On 21/01/2025 15:02, Ilya Maximets wrote: > On 1/20/25 16:13, Eelco Chaudron wrote: >> >> >> On 20 Jan 2025, at 14:50, Roi Dayan wrote: >> >>> On 20/01/2025 14:57, Eelco Chaudron wrote: >>>> >>>> >>>> On 15 Jan 2025, at 10:18, Roi Dayan via dev wrote: >>>> >>>>> Use this instead of directly using dpif_is_netdev() from dpif-netdev. >>>>> Return true in dpif-netdev. >>>> >>>> Not sure if we need an API for this, or maybe I should say, what is the >>>> definition >>>> of userspace? If this means anything but the kernel, we could probably use >>>> !dpif_is_netlink(). >>>> >>>> But I guess, the better question would be, what would we use this call >>>> for? If it’s similar >>>> to the existing use cases, we are probably better off by adding a features >>>> API to dpif. >>>> >>>> Thoughts? >>>> >>>> Cheers, >>>> >>>> Eelco >>> >>> >>> Hi, >>> >>> The meaning of dpif is a userspace means it needs to handle tunneling and >>> other stuff that >>> kernel dpif doesn't need as the kernel does it for it. >>> So while !dpif_is_netlink() is even more correct and better than current >>> dpif_is_netdev() >>> there was also a point here that dpif.c is not familiar with a specific >>> dpif and not accessing >>> a specific dpif function. >> >> So, this means a lot of potential stuff, so to me a general >> dpif_is_userspace() does not make sense. >> >>> I think introducing a features API to dpif could be an overkill at this >>> point. >>> So if future dpifs might need something specific other than handled by >>> kernel or not I think >>> this can be done at a later time. >> >> It might look like an overkill right now, but the current use cases might >> warrant one already. >> Or are you suggesting to use/introduce !dpif_is_netlink() instead for now? >> >> >> Ilya and other maintainers have any preferences or thoughts on this. > > In general, using either dpif_is_netlink or dpif_is_netdev outside of > dpif-netlink > and dpif-netdev respectively is icky. And I agree that even today the number > of > uses of these functions outside their modules justifies doing the feature > discovery > properly. Majority of the cases here in dpif.c are caused by the fact that > dpif-netdev doesn't validate actions, so it's not possible to probe them. It > may be > better to introduce some form of action validation in case of probing > instead, so > the features can be discovered normally. > > Best regards, Ilya Maximets.
I think this is out-of-scope for this patch. In this small commit I just wanted to do the separation from dpif.c knowing about existing dpifs and accessing them directly and not introducing new layers and mechanisem. Can we continue with this patch currently? a rename to the callback is also feasible though I think its saying exactly what it is. _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
