On Tue, Mar 23, 2021 at 03:50:32PM +0100, Frode Nordahl wrote: > The devlink interface was introduced [0] in the Linux 4.6 time > frame and has since gained traction among multiple hardware > vendors. > > The devlink-port [1] and devlink-info[1] interfaces are > particularly useful for managing NICs connected to multiple > distinct CPUs such as SmartNICs. > > In such a topology it would be useful to be able to offload > Open vSwitch and OVN onto the NIC SoC operating system and this > library will help with discovering and managing ports representing > resources made available on the host from the NIC SoC side. > > The library will be consumed by upcoming proposed changes to OVN, > and I think it makes sense to maintain it together with the Open > vSwitch netlink library code. > > 0: > https://lore.kernel.org/netdev/[email protected]/ > 1: > https://github.com/torvalds/linux/blob/master/Documentation/networking/devlink/devlink-port.rst > 2: > https://github.com/torvalds/linux/blob/master/Documentation/networking/devlink/devlink-info.rst > > TODO: > - Polish a small (non-install) utility that can be used for testing > dump and monitoring of devlink changes. > - Write tests > > Signed-off-by: Frode Nordahl <[email protected]>
I took a quick look at this, which should not count as a full review. It looks to me like it fits well with the existing style and structure of the OVS code (great!). In my skim, I only noticed a couple of things that might be done better. One is that there is a pretty repetitive collection of AC_COMPILE_IFELSE calls in OVS_CHECK_LINUX_DEVLINK. I think that a straightforward m4_define would avoid repetition and make the whole thing easier to read. (Since the only point of these tests is to check whether enums exist so they can be #define'd if they don't, there's an alternate approach: just #define all of them unconditionally if !__KERNEL__. They'd override existing enums in some cases, but that is harmless. But I could understand if you thought this was a nasty approach and preferred to test for each of them.) It might make sense to supply a full devlink.h if it's missing. Then what OVS builds won't depend on what kernel headers are installed on the build machine. I also noticed that NL_UINT_ATTR_ASSIGN and NL_STR_ATTR_ASSIGN could be written as functions. The former could just return a uint64_t. _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
