[PATCH iproute2 v2 0/3] iplink: bond: fix partner LACP state decoding

[PATCH iproute2 v2 0/3] iplink: bond: fix partner LACP state decoding

[Dave Seddon]

G'day,

Thanks Stephen for the feedback.

While porting iproute2 to Go, I noticed ip reads the u16
IFLA_BOND_SLAVE_AD_PARTNER_OPER_PORT_STATE as a u8. For normal
LACP state values, this makes the partner state print as 0 on
big-endian systems.

Patch 1 fixes the attribute read. Patch 2 fixes the absence check
used by the regression test in patch 3.

v2:
- .gitignore and make clean: use tests/ip/link/*.nl
- trim the test comment
- shorter commit messages

v1: https://lore.kernel.org/netdev/20261011032906.1398174-1-dave.seddon.ca@gmail.com/

Dave Seddon (3):
  iplink_bond_slave: read ad_partner_oper_port_state as u16
  testsuite: fix test_on_not on multi-line output
  testsuite: add bond slave LACP port state test

 .gitignore                                    |   2 +-
 ip/iplink_bond_slave.c                        |   2 +-
 testsuite/Makefile                            |   2 +-
 testsuite/lib/generic.sh                      |   5 +-
 .../ip/link/show_bond_slave_lacp_state.t      |  26 +++++
 testsuite/tools/generate_nlmsg.c              | 103 ++++++++++++++++--
 6 files changed, 126 insertions(+), 14 deletions(-)
 create mode 100755 testsuite/tests/ip/link/show_bond_slave_lacp_state.t

-- 
2.55.0

[PATCH iproute2 v2 1/3] iplink_bond_slave: read ad_partner_oper_port_state as u16

[Dave Seddon]

The kernel sends IFLA_BOND_SLAVE_AD_PARTNER_OPER_PORT_STATE as a u16,
but ip reads it as a u8. On big endian that is the high byte, so the
partner state always prints as 0.

Fixes: 39ac2d2b802b ("iplink: bond: print lacp actor/partner oper states as strings")
Assisted-by: Claude:claude-opus-5-5
Signed-off-by: Dave Seddon <dave.seddon.ca@gmail.com>
---
 ip/iplink_bond_slave.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ip/iplink_bond_slave.c b/ip/iplink_bond_slave.c
index f4b28fbc..24e73575 100644
--- a/ip/iplink_bond_slave.c
+++ b/ip/iplink_bond_slave.c
@@ -145,7 +145,7 @@ static void bond_slave_print_opt(struct link_util *lu, FILE *f, struct rtattr *t
 	}
 
 	if (tb[IFLA_BOND_SLAVE_AD_PARTNER_OPER_PORT_STATE]) {
-		__u16 state = rta_getattr_u8(tb[IFLA_BOND_SLAVE_AD_PARTNER_OPER_PORT_STATE]);
+		__u16 state = rta_getattr_u16(tb[IFLA_BOND_SLAVE_AD_PARTNER_OPER_PORT_STATE]);
 
 		print_int(PRINT_ANY,
 			  "ad_partner_oper_port_state",
-- 
2.55.0

[PATCH iproute2 v2 2/3] testsuite: fix test_on_not on multi-line output

[Dave Seddon]

test_on_not used grep -vq, which passes if any line does not match,
even when another line does. Pass only when no line matches.

Assisted-by: Claude:claude-opus-5-5
Signed-off-by: Dave Seddon <dave.seddon.ca@gmail.com>
---
 testsuite/lib/generic.sh | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/testsuite/lib/generic.sh b/testsuite/lib/generic.sh
index 8b339ec1..ac48f023 100644
--- a/testsuite/lib/generic.sh
+++ b/testsuite/lib/generic.sh
@@ -113,8 +113,9 @@ test_on()
 
 test_on_not()
 {
-	echo -n "test on: \"$1\""
-	if cat "$STD_OUT" | grep -vqE "$1"
+	echo -n "test on absence: \"$1\""
+	grep -qE -- "$1" "$STD_OUT"
+	if [ "$?" -eq 1 ]
 	then
 		pr_success
 	else
-- 
2.55.0

[PATCH iproute2 v2 3/3] testsuite: add bond slave LACP port state test

[Dave Seddon]

Decode generated bond slave messages with ip -d monitor file.
Use a synthetic partner state of 0x0100 to catch a u8 reader on
either endianness. Also check normal and zero states, actor state
output, and an absent partner attribute.

Assisted-by: Claude:claude-opus-5-5
Signed-off-by: Dave Seddon <dave.seddon.ca@gmail.com>
---
 .gitignore                                    |   2 +-
 testsuite/Makefile                            |   2 +-
 .../ip/link/show_bond_slave_lacp_state.t      |  26 +++++
 testsuite/tools/generate_nlmsg.c              | 103 ++++++++++++++++--
 4 files changed, 122 insertions(+), 11 deletions(-)
 create mode 100755 testsuite/tests/ip/link/show_bond_slave_lacp_state.t

diff --git a/.gitignore b/.gitignore
index e5234a3d..48064c45 100644
--- a/.gitignore
+++ b/.gitignore
@@ -38,4 +38,4 @@ series
 testsuite/results
 testsuite/iproute2/iproute2-this
 testsuite/tools/generate_nlmsg
-testsuite/tests/ip/link/dev_wo_vf_rate.nl
+testsuite/tests/ip/link/*.nl
diff --git a/testsuite/Makefile b/testsuite/Makefile
index fb50f618..82f3f98c 100644
--- a/testsuite/Makefile
+++ b/testsuite/Makefile
@@ -47,7 +47,7 @@ testclean:
 
 clean: testclean
 	@rm -f iproute2/iproute2-this
-	@rm -f tests/ip/link/dev_wo_vf_rate.nl
+	@rm -f tests/ip/link/*.nl
 	$(MAKE) -C tools clean
 
 distclean: clean
diff --git a/testsuite/tests/ip/link/show_bond_slave_lacp_state.t b/testsuite/tests/ip/link/show_bond_slave_lacp_state.t
new file mode 100755
index 00000000..71c6f0bc
--- /dev/null
+++ b/testsuite/tests/ip/link/show_bond_slave_lacp_state.t
@@ -0,0 +1,26 @@
+#!/bin/sh
+# SPDX-License-Identifier: GPL-2.0
+
+. lib/generic.sh
+
+# suffix|description|expected regex (leading ! = must not match)
+while IFS='|' read -r suffix desc expect; do
+	[ -z "$suffix" ] && continue
+	NL_FILE="tests/ip/link/bond_slave_partner_${suffix}.nl"
+	ts_ip "$0" "$desc" -d monitor file "$NL_FILE"
+	case "$expect" in
+	!*)
+		test_on "ad_actor_oper_port_state 63[^0-9]"
+		test_on_not "${expect#!}"
+		;;
+	*)
+		test_on "$expect"
+		;;
+	esac
+done <<EOF
+fast|partner state 0x3f|ad_partner_oper_port_state 63 ad_partner_oper_port_state_str <active,short_timeout,aggregating,in_sync,collecting,distributing>
+fast|actor state stays u8|ad_actor_oper_port_state 63[^0-9]
+zero|partner state 0|ad_partner_oper_port_state 0 ad_partner_oper_port_state_str <>
+hionly|partner state 0x0100, high byte only|ad_partner_oper_port_state 256[^0-9]
+absent|no partner attribute|!ad_partner_oper_port_state
+EOF
diff --git a/testsuite/tools/generate_nlmsg.c b/testsuite/tools/generate_nlmsg.c
index 106dca63..7cb9689b 100644
--- a/testsuite/tools/generate_nlmsg.c
+++ b/testsuite/tools/generate_nlmsg.c
@@ -11,6 +11,9 @@
 #include <linux/if.h>
 #include <errno.h>
 #include <stdio.h>
+#include <string.h>
+#include <linux/if_link.h>
+#include "utils.h"
 
 int fill_vf_rate_test(void *buf, size_t buflen)
 {
@@ -40,6 +43,7 @@ int fill_vf_rate_test(void *buf, size_t buflen)
 #define ASSERT(x) if (x < 0) return -1
 #define ATTR_L(t, v, l)	ASSERT(addattr_l(h, buflen, t, v, l))
 #define ATTR_8(t, v)	ASSERT(addattr8(h, buflen, t, v))
+#define ATTR_16(t, v)	ASSERT(addattr16(h, buflen, t, v))
 #define ATTR_32(t, v)	ASSERT(addattr32(h, buflen, t, v))
 #define ATTR_STRZ(t, v)	ASSERT(addattrstrz(h, buflen, t, v))
 
@@ -87,26 +91,107 @@ int fill_vf_rate_test(void *buf, size_t buflen)
 	return h->nlmsg_len;
 }
 
+/* 0x0100 catches a u8 reader on either endianness */
+struct bond_slave_case {
+	const char *file;
+	__u16 partner_state;
+	bool has_partner;
+};
+
+static const struct bond_slave_case bond_slave_cases[] = {
+	{ "tests/ip/link/bond_slave_partner_fast.nl",	0x003f, true },
+	{ "tests/ip/link/bond_slave_partner_zero.nl",	0x0000, true },
+	{ "tests/ip/link/bond_slave_partner_hionly.nl",	0x0100, true },
+	{ "tests/ip/link/bond_slave_partner_absent.nl",	0x0000, false },
+};
+
+static int fill_bond_slave_test(void *buf, size_t buflen,
+				const struct bond_slave_case *c)
+{
+	__u8 mac[ETH_ALEN] = { 0x02, 0x00, 0x00, 0x00, 0x00, 0x01 };
+	struct rtattr *linkinfo, *slave_data;
+	struct nlmsghdr *h = buf;
+	struct ifinfomsg *ifi;
+
+	memset(buf, 0, buflen);
+	h->nlmsg_type = RTM_NEWLINK;
+	h->nlmsg_len = NLMSG_LENGTH(sizeof(*ifi));
+
+	ifi = NLMSG_DATA(h);
+	ifi->ifi_type = ARPHRD_ETHER;
+	ifi->ifi_index = 2;
+	ifi->ifi_flags = IFF_RUNNING | IFF_BROADCAST | IFF_MULTICAST |
+			 IFF_UP | IFF_LOWER_UP | IFF_SLAVE;
+
+	ATTR_STRZ(IFLA_IFNAME, "eth1");
+	ATTR_32(IFLA_MTU, 1500);
+	ATTR_L(IFLA_ADDRESS, mac, sizeof(mac));
+
+	linkinfo = NEST(IFLA_LINKINFO);
+	ATTR_STRZ(IFLA_INFO_SLAVE_KIND, "bond");
+	slave_data = NEST(IFLA_INFO_SLAVE_DATA);
+	ATTR_8(IFLA_BOND_SLAVE_STATE, 0);
+	ATTR_8(IFLA_BOND_SLAVE_MII_STATUS, 0);
+	ATTR_32(IFLA_BOND_SLAVE_LINK_FAILURE_COUNT, 0);
+	ATTR_L(IFLA_BOND_SLAVE_PERM_HWADDR, mac, sizeof(mac));
+	ATTR_16(IFLA_BOND_SLAVE_QUEUE_ID, 0);
+	ATTR_16(IFLA_BOND_SLAVE_AD_AGGREGATOR_ID, 1);
+	ATTR_8(IFLA_BOND_SLAVE_AD_ACTOR_OPER_PORT_STATE, 0x3f);
+	if (c->has_partner)
+		ATTR_16(IFLA_BOND_SLAVE_AD_PARTNER_OPER_PORT_STATE,
+			c->partner_state);
+	NEST_END(slave_data);
+	NEST_END(linkinfo);
+
+	return h->nlmsg_len;
+}
+
+static int write_nlmsg(const char *file, const void *buf, int msglen)
+{
+	FILE *fp;
+
+	fp = fopen(file, "wb");
+	if (!fp) {
+		perror("fopen()");
+		return -1;
+	}
+	if (fwrite(buf, msglen, 1, fp) != 1) {
+		perror("fwrite()");
+		fclose(fp);
+		return -1;
+	}
+	if (fclose(fp)) {
+		perror("fclose()");
+		return -1;
+	}
+	return 0;
+}
+
 int main(void)
 {
 	char buf[16384] = { 0 };
+	unsigned int i;
 	int msglen;
-	FILE *fp;
 
 	msglen = fill_vf_rate_test(buf, sizeof(buf));
 	if (msglen < 0) {
 		fprintf(stderr, "fill_vf_rate_test() failed!\n");
 		return 1;
 	}
-	fp = fopen("tests/ip/link/dev_wo_vf_rate.nl", "w");
-	if (!fp) {
-		perror("fopen()");
-		return 1;
-	}
-	if (fwrite(buf, msglen, 1, fp) != 1) {
-		perror("fwrite()");
+	if (write_nlmsg("tests/ip/link/dev_wo_vf_rate.nl", buf, msglen))
 		return 1;
+
+	for (i = 0; i < ARRAY_SIZE(bond_slave_cases); i++) {
+		const struct bond_slave_case *c = &bond_slave_cases[i];
+
+		msglen = fill_bond_slave_test(buf, sizeof(buf), c);
+		if (msglen < 0) {
+			fprintf(stderr, "fill_bond_slave_test(%s) failed!\n",
+				c->file);
+			return 1;
+		}
+		if (write_nlmsg(c->file, buf, msglen))
+			return 1;
 	}
-	fclose(fp);
 	return 0;
 }
-- 
2.55.0