Thanks Ferruh, I'll send an update later this week. I also want to add a "Suggested-by: Dan Gora <d...@adax.com>" as it was his idea.
Dan, please let me know if you don't want this tag to be added. Thanks, Igor On Mon, Jun 28, 2021 at 3:55 PM Ferruh Yigit <ferruh.yi...@intel.com> wrote: > On 10/27/2019 8:16 PM, Igor Ryzhov wrote: > > Hi Ferruh, Dan, > > > > Sure, I remember last year discussion but now I see the problem in > current > > implementation. > > > > Ferruh, here is an example: > > > > We have a thread in the application that processes KNI commands from the > > kernel. > > It receives config_network_if command to set interface up, calls > > rte_eth_dev_start, and here is the problem. > > We cannot call current rte_kni_update_link from here as the interface is > > not yet up in the kernel, > > as we didn't send a response for config_network_if yet. So we need to > send > > a response first and only > > after that, we can use rte_kni_update_link. Actually, we don't even know > > the exact time between we > > send a response and the moment when the kernel receives it and the > > interface becomes up. > > We always have a dependency on the interface state in the kernel. With > > ioctl approach, we don't > > have such dependency - we can call rte_kni_update_link whenever we want, > > even when the interface is > > down in the kernel. As I explained, it's common when processing > > config_network_if to set interface up. > > > > Hi Igor, > > I agree with the mentioned problem. When the KNI interface is down, not > able to > update the link carrier status is not convenient. > > For a physical interface this may make sense, since interface won't be > used by > the OS, no need to power on the PHY and trace the carrier status. But for > the > intention of the original link set feature, it requires to be able to > update the > carrier status independent from the interface up/down status. > > Overall, also agree to not introduce a new ioctl and use existing > interface, but > for this case existing interface doesn't exactly fit to the intended use > case > and I am OK have the ioctl. > > Can you please send a new version rebasing latest head, we can continue on > that one? > > Thanks, > ferruh > > > > Igor > > > > On Mon, Oct 14, 2019 at 11:56 PM Dan Gora <d...@adax.com> wrote: > > > >> Here's another link to the thread where this was discussed last year.. > >> Igor was actually on this thread as well... > >> > >> https://mails.dpdk.org/archives/dev/2018-August/110383.html > >> > >> On Mon, Oct 14, 2019 at 4:01 PM Dan Gora <d...@adax.com> wrote: > >>> > >>> My original patch to add this feature was basically the same thing as > >>> this: setting the link status via a KNI ioctl. That method was > >>> rejected after _much_ discussion and we eventually settled on the > >>> currently implementation. > >>> > >>> My original patch was here: Message-Id: < > >> 20180628225548.21885-1...@adax.com> > >>> > >>> If you search for KNI and d...@adax.com in the DPDK devel list you > >>> should be able to suss out the whole discussion that lead to the > >>> current implementation. > >>> > >>> thanks > >>> dan > >>> > >>> On Mon, Oct 14, 2019 at 1:17 PM Ferruh Yigit <ferruh.yi...@intel.com> > >> wrote: > >>>> > >>>> On 10/14/2019 5:10 PM, Ferruh Yigit wrote: > >>>>> On 9/25/2019 10:36 AM, Igor Ryzhov wrote: > >>>>>> Current implementation doesn't allow us to update KNI carrier if the > >>>>>> interface is not yet UP in kernel. It means that we can't use it in > >> the > >>>>>> same thread which is processing rte_kni_ops.config_network_if, > >> which is > >>>>>> very convenient, because it allows us to have correct carrier status > >>>>>> of the interface right after we enabled it and we don't have to use > >> any > >>>>>> additional thread to track link status. > >>>>> > >>>>> Hi Igor, > >>>>> > >>>>> The existing thread tracks the link status of the physical device > >> and reflects > >>>>> the changes to the kni netdev, but the "struct rte_kni_ops" > >>>>> (rte_kni_ops.config_network_if) works other way around, it captures > >> (some) > >>>>> requests to kni netdev and reflects them to the underlying physical > >> device. > >>>>> Even 'rte_kni_update_link()' updated to use ioctl, the thread still > >> looks > >>>>> required and this patch doesn't really changes that part. > >>>>> > >>>>> Also I am reluctant to extend the KNI ioctl interface when there is > >> a generic > >>>>> way to do that work. > >>>>> > >>>>> What is the use case of updating kni netdev carrier status when the > >> interface is > >>>>> down? > >>>> > >>>> btw, if the problem is status of the interface being 'no-carrier' by > >> default, > >>>> this can be changed by "carrier=on" parameter of the kni kernel > module: > >>>> "insmod ./build/kmod/rte_kni.ko carrier=on" > >> > >
