Re: [dpdk-dev] [PATCH v5 1/3] kni: rework rte_kni_update_link using ioctl

[Stephen Hemminger <stephen@networkplumber.org>]

On Thu, 26 Aug 2021 18:19:09 +0300
Igor Ryzhov <iryzhov@nfware.com> wrote:

>  
> +static int
> +kni_ioctl_link(struct net *net, uint32_t ioctl_num,
> +		unsigned long ioctl_param)
> +{
> +	struct kni_net *knet = net_generic(net, kni_net_id);
> +	int ret = -EINVAL;
> +	struct kni_dev *dev, *n;
> +	struct rte_kni_link_info link_info;
> +	struct net_device *netdev;
> +
> +	if (_IOC_SIZE(ioctl_num) > sizeof(link_info))
> +		return -EINVAL;
> +
> +	if (copy_from_user(&link_info, (void *)ioctl_param, sizeof(link_info)))
> +		return -EFAULT;
> +
> +	if (strlen(link_info.name) == 0)
> +		return -EINVAL;
> +
> +	down_read(&knet->kni_list_lock);
> +	list_for_each_entry_safe(dev, n, &knet->kni_list_head, list) {
> +		if (strncmp(dev->name, link_info.name, RTE_KNI_NAMESIZE) != 0)
> +			continue;
> +
> +		netdev = dev->net_dev;
> +
> +		if (link_info.status) {
> +			netif_carrier_on(netdev);
> +
> +			dev->speed = link_info.speed;
> +			dev->duplex = link_info.duplex;
> +			dev->autoneg = link_info.autoneg;
> +		} else {
> +			netif_carrier_off(netdev);
> +		}
> +
> +		ret = 0;
> +		break;
> +	}
> +	up_read(&knet->kni_list_lock);
> +
> +	return ret;
> +}
> +

You need to be using the RTNL mutex in KNI here (and probably elsewhere).
The use of semaphore for list lock should also be replaced by a mutex.

The KNI driver was written long ago and was never reviewed by people
knowledgeable about kernel networking. That is one reason IMHO KNI
should not be used in production systems.