-----Original Message----- From: linux-crypto-owner@xxxxxxxxxxxxxxx [mailto:linux-crypto-owner@xxxxxxxxxxxxxxx] On Behalf Of David Miller Sent: Tuesday, February 20, 2018 9:46 PM To: Atul Gupta <atul.gupta@xxxxxxxxxxx> Cc: davejwatson@xxxxxx; herbert@xxxxxxxxxxxxxxxxxxx; sd@xxxxxxxxxxxxxxx; linux-crypto@xxxxxxxxxxxxxxx; netdev@xxxxxxxxxxxxxxx; Ganesh GR <ganeshgr@xxxxxxxxxxx> Subject: Re: [Crypto v6 03/12] tls: support for inline tls From: Atul Gupta <atul.gupta@xxxxxxxxxxx> 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. Sure. > +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. Will rename to no_tls_offload_dev, return 0 is used as default condition in calling function. 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); Done 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); Done Likewise. Thanks
