From: Atul Gupta <atul.gu...@chelsio.com>
Date: Mon, 19 Feb 2018 12:19:41 +0530

> +     struct net_device *netdev = NULL;
> +
> +     netdev = dev_get_by_index(sock_net(sk), inet->cork.fl.flowi_oif);

No need for an assignment in the variable declaration here.
You immediately set it to something else unconditionally.

> +static int get_tls_offload_dev(struct sock *sk)
> +{
> +     struct net_device *netdev;
> +     struct tls_device *dev;
> +     int rc = 0;
> +
> +     netdev = get_netdev(sk);
> +     if (!netdev)
> +             return -EINVAL;
> +
> +     mutex_lock(&device_mutex);
> +     list_for_each_entry(dev, &device_list, dev_list) {
> +             if (dev->netdev && dev->netdev(dev, netdev)) {
> +                     rc = -EEXIST;
> +                     break;
> +             }
> +     }
> +     mutex_unlock(&device_mutex);
> +     dev_put(netdev);
> +     return rc;
> +}

This is really a confusing function.

It's name suggests that it "gets" the offload device.  In that case,
if it is found it should return success.  Instead we get an -EEXIST
error in that case.  And it returns 0 if not found.

Better to make this do what it says it does, which would be to return
'0' when the device is found and return -ENODEV when it is not found.

> +     tcp_prot.unhash(sk);

Do not force this to the ipv4 TCP instance, use the pointer through
the socket to call the proper unhash method.

> +     err = tcp_prot.hash(sk);

Likewise.

Reply via email to