On 3/27/2018 11:53 PM, Stefano Brivio wrote:
> On Tue, 27 Mar 2018 23:06:30 +0530
> Atul Gupta <atul.gupta@xxxxxxxxxxx> wrote:
>
>> +static struct tls_context *create_ctx(struct sock *sk)
>> +{
>> + struct inet_connection_sock *icsk = inet_csk(sk);
>> + struct tls_context *ctx;
>> +
>> + /* allocate tls context */
>> + ctx = kzalloc(sizeof(*ctx), GFP_KERNEL);
>> + if (!ctx)
>> + return NULL;
>> +
>> + icsk->icsk_ulp_data = ctx;
>> + return ctx;
>> +}
>>
>> [...]
>>
>> static int tls_init(struct sock *sk)
>> {
>> int ip_ver = sk->sk_family == AF_INET6 ? TLSV6 : TLSV4;
>> - struct inet_connection_sock *icsk = inet_csk(sk);
>> struct tls_context *ctx;
>> int rc = 0;
>>
>> + if (tls_hw_prot(sk))
>> + goto out;
>> +
>> /* The TLS ulp is currently supported only for TCP sockets
>> * in ESTABLISHED state.
>> * Supporting sockets in LISTEN state will require us
>> @@ -530,12 +624,11 @@ static int tls_init(struct sock *sk)
>> return -ENOTSUPP;
>>
>> /* allocate tls context */
>> - ctx = kzalloc(sizeof(*ctx), GFP_KERNEL);
>> + ctx = create_ctx(sk);
>> if (!ctx) {
>> rc = -ENOMEM;
>> goto out;
>> }
>> - icsk->icsk_ulp_data = ctx;
> Why are you changing this?
since create_ctx is called at two place it is assigned in allocating function than duplicate the assignment.
>
> This is now equivalent to the original implementation, except that you
> are "hiding" the assignment of icsk->icsk_ulp_data into a function named
> "create_ctx".
>
> Please also note that you are duplicating the "allocate tls context"
> comment.
will remove this comment.
>
