Re: [PATCH net-next iproute2 1/3] netlink: import netlink message parsing from kernel

From: David Ahern <dsa@cumulusnetworks.com>
To: Stephen Hemminger <stephen@networkplumber.org>,
	David Miller <davem@davemloft.net>
Cc: netdev@vger.kernel.org, jakub.kicinski@netronome.com
Subject: Re: [PATCH net-next iproute2 1/3] netlink: import netlink message parsing from kernel
Date: Tue, 2 May 2017 12:39:51 -0600	[thread overview]
Message-ID: <3acddce9-5421-637e-8b2b-ae8dd5c7b29a@cumulusnetworks.com> (raw)
In-Reply-To: <20170502110314.66d9ee89@xeon-e3>

On 5/2/17 12:03 PM, Stephen Hemminger wrote:
> Then use libmnl it is already used in several other places in iproute2.
> Eventually, I would like to use it everywhere and get rid of old netlink parser.
> 

Why? libmnl is not going to simplify the iproute2 code.

Look at attribute validation. Importing the kernel code into iproute2,
the API is very familiar to anyone hacking on the kernel side:

+       if (nla_parse(tb, NLMSGERR_ATTR_MAX, attr, alen, extack_policy)
!= 0) {
+               fprintf(stderr,
+                       "Failed to parse extended error attributes\n");
+               return 0;
+       }
+

ie., you pass a policy to the parse routine and the checking is part of
nla_parse. The implementation of nla_parse and validate_nla are quite
easy to read.

Now take a look at what devlink has for validation - attr_cb. IMO very
unreadable and puts the burden on the app using the libmnl API.