zebra: Correct implication of SOL_NETLINK NETLINK_ADD_MEMBERSHIP usage

The usage of SOL_NETLINK for adding memberships of interest is
1 group per call.  The netink_socket function implied that
the call could be a bitfield of values.  This is not correct
at all.  This will trip someone else up in the future when
a new value is needed.  Let's get it right `now` before
it becomes a problem.

Let's also add a bit of extra code to give operator a better
understanding of what went wrong when a kernel does not
support the option.

Finally as a point of future reference should FRR just switch
over to a loop to add the required loops instead of having
this bastardized approach of some going in one way and some
going in another way?

Signed-off-by: Donald Sharp <sharpd@nvidia.com>

1 files changed, 32 insertions, 13 deletions

diff --git a/zebra/kernel_netlink.c b/zebra/kernel_netlink.c
index 4bd0ac27f..e372eaf6a 100644
--- a/zebra/kernel_netlink.c
+++ b/zebra/kernel_netlink.c
@@ -290,9 +290,20 @@ static int netlink_recvbuf(struct nlsock *nl, uint32_t newsize)
 	return 0;
 }
 
+static const char *group2str(uint32_t group)
+{
+	switch (group) {
+	case RTNLGRP_TUNNEL:
+		return "RTNLGRP_TUNNEL";
+	default:
+		return "UNKNOWN";
+	}
+}
+
 /* Make socket for Linux netlink interface. */
 static int netlink_socket(struct nlsock *nl, unsigned long groups,
-			  unsigned long ext_groups, ns_id_t ns_id)
+			  uint32_t ext_groups[], uint8_t ext_group_size,
+			  ns_id_t ns_id)
 {
 	int ret;
 	struct sockaddr_nl snl;
@@ -312,14 +323,21 @@ static int netlink_socket(struct nlsock *nl, unsigned long groups,
 		snl.nl_groups = groups;
 
 #if defined SOL_NETLINK
-		if (ext_groups) {
-			ret = setsockopt(sock, SOL_NETLINK,
-					 NETLINK_ADD_MEMBERSHIP, &ext_groups,
-					 sizeof(ext_groups));
-			if (ret < 0) {
-				zlog_notice(
-					"can't setsockopt NETLINK_ADD_MEMBERSHIP: %s(%d)",
-					safe_strerror(errno), errno);
+		if (ext_group_size) {
+			uint8_t i;
+
+			for (i = 0; i < ext_group_size; i++) {
+				ret = setsockopt(sock, SOL_NETLINK,
+						 NETLINK_ADD_MEMBERSHIP,
+						 &ext_groups[i],
+						 sizeof(ext_groups[i]));
+				if (ret < 0) {
+					zlog_notice(
+						"can't setsockopt NETLINK_ADD_MEMBERSHIP for group %s(%u), this linux kernel does not support it: %s(%d)",
+						group2str(ext_groups[i]),
+						ext_groups[i],
+						safe_strerror(errno), errno);
+				}
 			}
 		}
 #endif
@@ -1734,7 +1752,8 @@ void kernel_init(struct zebra_ns *zns)
 	snprintf(zns->netlink.name, sizeof(zns->netlink.name),
 		 "netlink-listen (NS %u)", zns->ns_id);
 	zns->netlink.sock = -1;
-	if (netlink_socket(&zns->netlink, groups, ext_groups, zns->ns_id) < 0) {
+	if (netlink_socket(&zns->netlink, groups, &ext_groups, 1, zns->ns_id) <
+	    0) {
 		zlog_err("Failure to create %s socket",
 			 zns->netlink.name);
 		exit(-1);
@@ -1745,7 +1764,7 @@ void kernel_init(struct zebra_ns *zns)
 	snprintf(zns->netlink_cmd.name, sizeof(zns->netlink_cmd.name),
 		 "netlink-cmd (NS %u)", zns->ns_id);
 	zns->netlink_cmd.sock = -1;
-	if (netlink_socket(&zns->netlink_cmd, 0, 0, zns->ns_id) < 0) {
+	if (netlink_socket(&zns->netlink_cmd, 0, 0, 0, zns->ns_id) < 0) {
 		zlog_err("Failure to create %s socket",
 			 zns->netlink_cmd.name);
 		exit(-1);
@@ -1758,7 +1777,7 @@ void kernel_init(struct zebra_ns *zns)
 		 sizeof(zns->netlink_dplane_out.name), "netlink-dp (NS %u)",
 		 zns->ns_id);
 	zns->netlink_dplane_out.sock = -1;
-	if (netlink_socket(&zns->netlink_dplane_out, 0, 0, zns->ns_id) < 0) {
+	if (netlink_socket(&zns->netlink_dplane_out, 0, 0, 0, zns->ns_id) < 0) {
 		zlog_err("Failure to create %s socket",
 			 zns->netlink_dplane_out.name);
 		exit(-1);
@@ -1771,7 +1790,7 @@ void kernel_init(struct zebra_ns *zns)
 		 sizeof(zns->netlink_dplane_in.name), "netlink-dp-in (NS %u)",
 		 zns->ns_id);
 	zns->netlink_dplane_in.sock = -1;
-	if (netlink_socket(&zns->netlink_dplane_in, dplane_groups, 0,
+	if (netlink_socket(&zns->netlink_dplane_in, dplane_groups, 0, 0,
 			   zns->ns_id) < 0) {
 		zlog_err("Failure to create %s socket",
 			 zns->netlink_dplane_in.name);