[PATCH iproute2-next 0/5] ip bridge: Updates to neigh and fdb dumps

[PATCH iproute2-next 0/5] ip bridge: Updates to neigh and fdb dumps

[David Ahern]

From: David Ahern <dsahern@gmail.com>

First 3 patches fixup ip neigh and bridge fdb dumps to use
rtnl_neighdump_req.

Patch 4 adds a new flag for commands to know when strict checking is
enabled.

Patch 5 fixes fdb dumps to work on older kernels.

David Ahern (5):
  libnetlink: Add filter function to rtnl_neighdump_req
  ip neigh: Convert do_show_or_flush to use rtnl_neighdump_req
  bridge: Update fdb show to use rtnl_neighdump_req
  libnetlink: Add RTNL_HANDLE_F_STRICT_CHK flag
  bridge: fdb: Fix filtering with strict checking disabled

 bridge/fdb.c         | 62 +++++++++++++++++++++++++++++++++++++++-------------
 include/libnetlink.h |  4 +++-
 ip/ipneigh.c         | 42 +++++++++++++++++++++--------------
 lib/libnetlink.c     | 19 +++++++++++++---
 misc/arpd.c          |  2 +-
 5 files changed, 93 insertions(+), 36 deletions(-)

-- 
2.11.0

[PATCH iproute2-next 1/5] libnetlink: Add filter function to rtnl_neighdump_req

[David Ahern]

From: David Ahern <dsahern@gmail.com>

Add filter function to rtnl_neighdump_req and a buffer to the
request for the filter functions to append attributes.

Signed-off-by: David Ahern <dsahern@gmail.com>
---
 include/libnetlink.h |  3 ++-
 lib/libnetlink.c     | 12 +++++++++++-
 misc/arpd.c          |  2 +-
 3 files changed, 14 insertions(+), 3 deletions(-)

diff --git a/include/libnetlink.h b/include/libnetlink.h
index dc0c9c4eb3f5..148951510d1e 100644
--- a/include/libnetlink.h
+++ b/include/libnetlink.h
@@ -60,7 +60,8 @@ int rtnl_routedump_req(struct rtnl_handle *rth, int family,
 	__attribute__((warn_unused_result));
 int rtnl_ruledump_req(struct rtnl_handle *rth, int family)
 	__attribute__((warn_unused_result));
-int rtnl_neighdump_req(struct rtnl_handle *rth, int family)
+int rtnl_neighdump_req(struct rtnl_handle *rth, int family,
+		       req_filter_fn_t filter_fn)
 	__attribute__((warn_unused_result));
 int rtnl_neightbldump_req(struct rtnl_handle *rth, int family)
 	__attribute__((warn_unused_result));
diff --git a/lib/libnetlink.c b/lib/libnetlink.c
index 4d7d081054fd..19318b445266 100644
--- a/lib/libnetlink.c
+++ b/lib/libnetlink.c
@@ -327,11 +327,13 @@ int rtnl_ruledump_req(struct rtnl_handle *rth, int family)
 	return send(rth->fd, &req, sizeof(req), 0);
 }
 
-int rtnl_neighdump_req(struct rtnl_handle *rth, int family)
+int rtnl_neighdump_req(struct rtnl_handle *rth, int family,
+		       req_filter_fn_t filter_fn)
 {
 	struct {
 		struct nlmsghdr nlh;
 		struct ndmsg ndm;
+		char buf[256];
 	} req = {
 		.nlh.nlmsg_len = NLMSG_LENGTH(sizeof(struct ndmsg)),
 		.nlh.nlmsg_type = RTM_GETNEIGH,
@@ -340,6 +342,14 @@ int rtnl_neighdump_req(struct rtnl_handle *rth, int family)
 		.ndm.ndm_family = family,
 	};
 
+	if (filter_fn) {
+		int err;
+
+		err = filter_fn(&req.nlh, sizeof(req));
+		if (err)
+			return err;
+	}
+
 	return send(rth->fd, &req, sizeof(req), 0);
 }
 
diff --git a/misc/arpd.c b/misc/arpd.c
index ce7c09978c2b..504961cb5e3a 100644
--- a/misc/arpd.c
+++ b/misc/arpd.c
@@ -424,7 +424,7 @@ static int do_one_request(struct nlmsghdr *n)
 
 static void load_initial_table(void)
 {
-	if (rtnl_neighdump_req(&rth, AF_INET) < 0) {
+	if (rtnl_neighdump_req(&rth, AF_INET, NULL) < 0) {
 		perror("dump request failed");
 		exit(1);
 	}
-- 
2.11.0

[PATCH iproute2-next 2/5] ip neigh: Convert do_show_or_flush to use rtnl_neighdump_req

[David Ahern]

From: David Ahern <dsahern@gmail.com>

Add ipneigh_dump_filter to add filter attributes to the neighbor
dump request and update do_show_or_flush to use rtnl_neighdump_req.

Signed-off-by: David Ahern <dsahern@gmail.com>
---
 ip/ipneigh.c | 42 ++++++++++++++++++++++++++----------------
 1 file changed, 26 insertions(+), 16 deletions(-)

diff --git a/ip/ipneigh.c b/ip/ipneigh.c
index 26ac2d1be21f..2d717d2d2aeb 100644
--- a/ip/ipneigh.c
+++ b/ip/ipneigh.c
@@ -41,6 +41,7 @@ static struct
 	int flushe;
 	int master;
 	int protocol;
+	__u8 ndm_flags;
 } filter;
 
 static void usage(void) __attribute__((noreturn));
@@ -408,16 +409,29 @@ void ipneigh_reset_filter(int ifindex)
 	filter.index = ifindex;
 }
 
+static int ipneigh_dump_filter(struct nlmsghdr *nlh, int reqlen)
+{
+	struct ndmsg *ndm = NLMSG_DATA(nlh);
+	int err;
+
+	ndm->ndm_flags = filter.ndm_flags;
+
+	if (filter.index) {
+		err = addattr32(nlh, reqlen, NDA_IFINDEX, filter.index);
+		if (err)
+			return err;
+	}
+	if (filter.master) {
+		err = addattr32(nlh, reqlen, NDA_MASTER, filter.master);
+		if (err)
+			return err;
+	}
+
+	return 0;
+}
+
 static int do_show_or_flush(int argc, char **argv, int flush)
 {
-	struct {
-		struct nlmsghdr	n;
-		struct ndmsg		ndm;
-		char			buf[256];
-	} req = {
-		.n.nlmsg_type = RTM_GETNEIGH,
-		.n.nlmsg_len = NLMSG_LENGTH(sizeof(struct ndmsg)),
-	};
 	char *filter_dev = NULL;
 	int state_given = 0;
 
@@ -448,7 +462,6 @@ static int do_show_or_flush(int argc, char **argv, int flush)
 			ifindex = ll_name_to_index(*argv);
 			if (!ifindex)
 				invarg("Device does not exist\n", *argv);
-			addattr32(&req.n, sizeof(req), NDA_MASTER, ifindex);
 			filter.master = ifindex;
 		} else if (strcmp(*argv, "vrf") == 0) {
 			int ifindex;
@@ -459,7 +472,6 @@ static int do_show_or_flush(int argc, char **argv, int flush)
 				invarg("Not a valid VRF name\n", *argv);
 			if (!name_is_vrf(*argv))
 				invarg("Not a valid VRF name\n", *argv);
-			addattr32(&req.n, sizeof(req), NDA_MASTER, ifindex);
 			filter.master = ifindex;
 		} else if (strcmp(*argv, "unused") == 0) {
 			filter.unused_only = 1;
@@ -482,7 +494,7 @@ static int do_show_or_flush(int argc, char **argv, int flush)
 				state = 0x100;
 			filter.state |= state;
 		} else if (strcmp(*argv, "proxy") == 0) {
-			req.ndm.ndm_flags = NTF_PROXY;
+			filter.ndm_flags = NTF_PROXY;
 		} else if (matches(*argv, "protocol") == 0) {
 			__u32 prot;
 
@@ -513,11 +525,8 @@ static int do_show_or_flush(int argc, char **argv, int flush)
 		filter.index = ll_name_to_index(filter_dev);
 		if (!filter.index)
 			return nodev(filter_dev);
-		addattr32(&req.n, sizeof(req), NDA_IFINDEX, filter.index);
 	}
 
-	req.ndm.ndm_family = filter.family;
-
 	if (flush) {
 		int round = 0;
 		char flushb[4096-512];
@@ -527,7 +536,8 @@ static int do_show_or_flush(int argc, char **argv, int flush)
 		filter.flushe = sizeof(flushb);
 
 		while (round < MAX_ROUNDS) {
-			if (rtnl_dump_request_n(&rth, &req.n) < 0) {
+			if (rtnl_neighdump_req(&rth, filter.family,
+					       ipneigh_dump_filter) < 0) {
 				perror("Cannot send dump request");
 				exit(1);
 			}
@@ -560,7 +570,7 @@ static int do_show_or_flush(int argc, char **argv, int flush)
 		return 1;
 	}
 
-	if (rtnl_dump_request_n(&rth, &req.n) < 0) {
+	if (rtnl_neighdump_req(&rth, filter.family, ipneigh_dump_filter) < 0) {
 		perror("Cannot send dump request");
 		exit(1);
 	}
-- 
2.11.0

[PATCH iproute2-next 3/5] bridge: Update fdb show to use rtnl_neighdump_req

[David Ahern]

From: David Ahern <dsahern@gmail.com>

Add fdb_dump_filter to set filter attributes in dump request
and convert fdb_show to use rtnl_neighdump_req.

Signed-off-by: David Ahern <dsahern@gmail.com>
---
 bridge/fdb.c | 37 ++++++++++++++++++++++---------------
 1 file changed, 22 insertions(+), 15 deletions(-)

diff --git a/bridge/fdb.c b/bridge/fdb.c
index a7a0d8052307..676267e15ddc 100644
--- a/bridge/fdb.c
+++ b/bridge/fdb.c
@@ -30,7 +30,7 @@
 #include "rt_names.h"
 #include "utils.h"
 
-static unsigned int filter_index, filter_vlan, filter_state;
+static unsigned int filter_index, filter_vlan, filter_state, filter_master;
 
 static void usage(void)
 {
@@ -256,20 +256,29 @@ int print_fdb(struct nlmsghdr *n, void *arg)
 	return 0;
 }
 
-static int fdb_show(int argc, char **argv)
+static int fdb_dump_filter(struct nlmsghdr *nlh, int reqlen)
 {
-	struct {
-		struct nlmsghdr	n;
-		struct ndmsg		ndm;
-		char			buf[256];
-	} req = {
-		.n.nlmsg_len = NLMSG_LENGTH(sizeof(struct ndmsg)),
-		.ndm.ndm_family = PF_BRIDGE,
-	};
+	int err;
+
+	if (filter_index) {
+		struct ndmsg *ndm = NLMSG_DATA(nlh);
 
+		ndm->ndm_ifindex = filter_index;
+	}
+
+	if (filter_master) {
+		err = addattr32(nlh, reqlen, NDA_MASTER, filter_master);
+		if (err)
+			return err;
+	}
+
+	return 0;
+}
+
+static int fdb_show(int argc, char **argv)
+{
 	char *filter_dev = NULL;
 	char *br = NULL;
-	int msg_size = sizeof(struct ndmsg);
 
 	while (argc > 0) {
 		if ((strcmp(*argv, "brport") == 0) || strcmp(*argv, "dev") == 0) {
@@ -304,8 +313,7 @@ static int fdb_show(int argc, char **argv)
 			fprintf(stderr, "Cannot find bridge device \"%s\"\n", br);
 			return -1;
 		}
-		addattr32(&req.n, sizeof(req), IFLA_MASTER, br_ifindex);
-		msg_size += RTA_LENGTH(4);
+		filter_master = br_ifindex;
 	}
 
 	/*we'll keep around filter_dev for older kernels */
@@ -313,10 +321,9 @@ static int fdb_show(int argc, char **argv)
 		filter_index = ll_name_to_index(filter_dev);
 		if (!filter_index)
 			return nodev(filter_dev);
-		req.ndm.ndm_ifindex = filter_index;
 	}
 
-	if (rtnl_dump_request(&rth, RTM_GETNEIGH, &req.ndm, msg_size) < 0) {
+	if (rtnl_neighdump_req(&rth, PF_BRIDGE, fdb_dump_filter) < 0) {
 		perror("Cannot send dump request");
 		exit(1);
 	}
-- 
2.11.0

[PATCH iproute2-next 4/5] libnetlink: Add RTNL_HANDLE_F_STRICT_CHK flag

[David Ahern]

From: David Ahern <dsahern@gmail.com>

Add RTNL_HANDLE_F_STRICT_CHK flag and set in rth flags to let know
commands know if the kernel supports strict checking.

Extracted from patch from Ido to fix filtering with strict checking
enabled.

Cc: Ido Schimmel <idosch@mellanox.com>
Signed-off-by: David Ahern <dsahern@gmail.com>
---
 include/libnetlink.h | 1 +
 lib/libnetlink.c     | 7 +++++--
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/include/libnetlink.h b/include/libnetlink.h
index 148951510d1e..0854d6ad2fab 100644
--- a/include/libnetlink.h
+++ b/include/libnetlink.h
@@ -23,6 +23,7 @@ struct rtnl_handle {
 	FILE		       *dump_fp;
 #define RTNL_HANDLE_F_LISTEN_ALL_NSID		0x01
 #define RTNL_HANDLE_F_SUPPRESS_NLERR		0x02
+#define RTNL_HANDLE_F_STRICT_CHK		0x04
 	int			flags;
 };
 
diff --git a/lib/libnetlink.c b/lib/libnetlink.c
index 19318b445266..600e9a2476ba 100644
--- a/lib/libnetlink.c
+++ b/lib/libnetlink.c
@@ -166,8 +166,11 @@ void rtnl_set_strict_dump(struct rtnl_handle *rth)
 {
 	int one = 1;
 
-	setsockopt(rth->fd, SOL_NETLINK, NETLINK_GET_STRICT_CHK,
-		   &one, sizeof(one));
+	if (setsockopt(rth->fd, SOL_NETLINK, NETLINK_GET_STRICT_CHK,
+		       &one, sizeof(one)) < 0)
+		return;
+
+	rth->flags |= RTNL_HANDLE_F_STRICT_CHK;
 }
 
 void rtnl_close(struct rtnl_handle *rth)
-- 
2.11.0

[PATCH iproute2-next 5/5] bridge: fdb: Fix filtering with strict checking disabled

[David Ahern]

From: David Ahern <dsahern@gmail.com>

Older kernels expect an ifinfomsg struct as the ancillary header, and
after kernel commit bd961c9bc664 ("rtnetlink: fix rtnl_fdb_dump() for ndmsg
header") can handle either ifinfomsg or ndmsg. Strict data checking only
allows ndmsg.

Use the new RTNL_HANDLE_F_STRICT_CHK flag to know which header to send.

Signed-off-by: David Ahern <dsahern@gmail.com>
---
 bridge/fdb.c | 27 ++++++++++++++++++++++++++-
 1 file changed, 26 insertions(+), 1 deletion(-)

diff --git a/bridge/fdb.c b/bridge/fdb.c
index 676267e15ddc..a0fdac00e094 100644
--- a/bridge/fdb.c
+++ b/bridge/fdb.c
@@ -256,6 +256,25 @@ int print_fdb(struct nlmsghdr *n, void *arg)
 	return 0;
 }
 
+static int fdb_linkdump_filter(struct nlmsghdr *nlh, int reqlen)
+{
+	int err;
+
+	if (filter_index) {
+		struct ifinfomsg *ifm = NLMSG_DATA(nlh);
+
+		ifm->ifi_index = filter_index;
+	}
+
+	if (filter_master) {
+		err = addattr32(nlh, reqlen, IFLA_MASTER, filter_master);
+		if (err)
+			return err;
+	}
+
+	return 0;
+}
+
 static int fdb_dump_filter(struct nlmsghdr *nlh, int reqlen)
 {
 	int err;
@@ -279,6 +298,7 @@ static int fdb_show(int argc, char **argv)
 {
 	char *filter_dev = NULL;
 	char *br = NULL;
+	int rc;
 
 	while (argc > 0) {
 		if ((strcmp(*argv, "brport") == 0) || strcmp(*argv, "dev") == 0) {
@@ -323,7 +343,12 @@ static int fdb_show(int argc, char **argv)
 			return nodev(filter_dev);
 	}
 
-	if (rtnl_neighdump_req(&rth, PF_BRIDGE, fdb_dump_filter) < 0) {
+	if (rth.flags & RTNL_HANDLE_F_STRICT_CHK)
+		rc = rtnl_neighdump_req(&rth, PF_BRIDGE, fdb_dump_filter);
+	else
+		rc = rtnl_linkdump_req_filter_fn(&rth, PF_BRIDGE,
+						 fdb_linkdump_filter);
+	if (rc < 0) {
 		perror("Cannot send dump request");
 		exit(1);
 	}
-- 
2.11.0

Re: [PATCH iproute2-next 5/5] bridge: fdb: Fix filtering with strict checking disabled

[Ido Schimmel]

On Wed, Jan 02, 2019 at 08:38:32PM -0800, David Ahern wrote:
> From: David Ahern <dsahern@gmail.com>
> 
> Older kernels expect an ifinfomsg struct as the ancillary header, and
> after kernel commit bd961c9bc664 ("rtnetlink: fix rtnl_fdb_dump() for ndmsg
> header") can handle either ifinfomsg or ndmsg. Strict data checking only
> allows ndmsg.
> 
> Use the new RTNL_HANDLE_F_STRICT_CHK flag to know which header to send.
> 
> Signed-off-by: David Ahern <dsahern@gmail.com>

Reviewed-by: Ido Schimmel <idosch@mellanox.com>
Tested-by: Ido Schimmel <idosch@mellanox.com>

Thanks!