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

[Igor Ryzhov <iryzhov@nfware.com>]

Could you please clarify where exactly do I need to use rtnl lock?
From what I understand, netif_carrier_on/off can be called without the lock.

On Thu, Aug 26, 2021 at 8:15 PM Stephen Hemminger <
stephen@networkplumber.org> wrote:

> 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.
>
>