[ovs-dev] [RFC PATCH] lib: Introduce netlink-devlink library
blp at ovn.org
Tue Mar 23 16:23:10 UTC 2021
On Tue, Mar 23, 2021 at 03:50:32PM +0100, Frode Nordahl wrote:
> The devlink interface was introduced  in the Linux 4.6 time
> frame and has since gained traction among multiple hardware
> The devlink-port  and devlink-info 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://email@example.com/
> 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
> - 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 <frode.nordahl at canonical.com>
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
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.
More information about the dev