* [PATCH iproute2 v2 0/3] iplink: bond: fix partner LACP state decoding
@ 2026-10-19 20:38 Dave Seddon
2026-10-19 20:38 ` [PATCH iproute2 v2 1/3] iplink_bond_slave: read ad_partner_oper_port_state as u16 Dave Seddon
` (3 more replies)
0 siblings, 4 replies; 5+ messages in thread
From: Dave Seddon @ 2026-10-19 20:38 UTC (permalink / raw)
To: netdev; +Cc: Dave Seddon, Stephen Hemminger, David Ahern
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
^ permalink raw reply [flat|nested] 5+ messages in thread* [PATCH iproute2 v2 1/3] iplink_bond_slave: read ad_partner_oper_port_state as u16 2026-10-19 20:38 [PATCH iproute2 v2 0/3] iplink: bond: fix partner LACP state decoding Dave Seddon @ 2026-10-19 20:38 ` Dave Seddon 2026-10-19 20:38 ` [PATCH iproute2 v2 2/3] testsuite: fix test_on_not on multi-line output Dave Seddon ` (2 subsequent siblings) 3 siblings, 0 replies; 5+ messages in thread From: Dave Seddon @ 2026-10-19 20:38 UTC (permalink / raw) To: netdev; +Cc: Dave Seddon, Stephen Hemminger, David Ahern 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 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH iproute2 v2 2/3] testsuite: fix test_on_not on multi-line output 2026-10-19 20:38 [PATCH iproute2 v2 0/3] iplink: bond: fix partner LACP state decoding Dave Seddon 2026-10-19 20:38 ` [PATCH iproute2 v2 1/3] iplink_bond_slave: read ad_partner_oper_port_state as u16 Dave Seddon @ 2026-10-19 20:38 ` Dave Seddon 2026-10-19 20:38 ` [PATCH iproute2 v2 3/3] testsuite: add bond slave LACP port state test Dave Seddon 2026-10-21 2:50 ` [PATCH iproute2 v2 0/3] iplink: bond: fix partner LACP state decoding patchwork-bot+netdevbpf 3 siblings, 0 replies; 5+ messages in thread From: Dave Seddon @ 2026-10-19 20:38 UTC (permalink / raw) To: netdev; +Cc: Dave Seddon, Stephen Hemminger, David Ahern 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 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH iproute2 v2 3/3] testsuite: add bond slave LACP port state test 2026-10-19 20:38 [PATCH iproute2 v2 0/3] iplink: bond: fix partner LACP state decoding Dave Seddon 2026-10-19 20:38 ` [PATCH iproute2 v2 1/3] iplink_bond_slave: read ad_partner_oper_port_state as u16 Dave Seddon 2026-10-19 20:38 ` [PATCH iproute2 v2 2/3] testsuite: fix test_on_not on multi-line output Dave Seddon @ 2026-10-19 20:38 ` Dave Seddon 2026-10-21 2:50 ` [PATCH iproute2 v2 0/3] iplink: bond: fix partner LACP state decoding patchwork-bot+netdevbpf 3 siblings, 0 replies; 5+ messages in thread From: Dave Seddon @ 2026-10-19 20:38 UTC (permalink / raw) To: netdev; +Cc: Dave Seddon, Stephen Hemminger, David Ahern 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 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH iproute2 v2 0/3] iplink: bond: fix partner LACP state decoding 2026-10-19 20:38 [PATCH iproute2 v2 0/3] iplink: bond: fix partner LACP state decoding Dave Seddon ` (2 preceding siblings ...) 2026-10-19 20:38 ` [PATCH iproute2 v2 3/3] testsuite: add bond slave LACP port state test Dave Seddon @ 2026-10-21 2:50 ` patchwork-bot+netdevbpf 3 siblings, 0 replies; 5+ messages in thread From: patchwork-bot+netdevbpf @ 2026-10-21 2:50 UTC (permalink / raw) To: dave seddon; +Cc: netdev, stephen, dsahern Hello: This series was applied to iproute2/iproute2.git (main) by Stephen Hemminger <stephen@networkplumber.org>: On Mon, 19 Oct 2026 13:38:40 -0700 you wrote: > 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. > > [...] Here is the summary with links: - [iproute2,v2,1/3] iplink_bond_slave: read ad_partner_oper_port_state as u16 https://git.kernel.org/pub/scm/network/iproute2/iproute2.git/commit/?id=0067cbc8166e - [iproute2,v2,2/3] testsuite: fix test_on_not on multi-line output https://git.kernel.org/pub/scm/network/iproute2/iproute2.git/commit/?id=075f1b6f6a79 - [iproute2,v2,3/3] testsuite: add bond slave LACP port state test https://git.kernel.org/pub/scm/network/iproute2/iproute2.git/commit/?id=4f49441d2be0 You are awesome, thank you! -- Deet-doot-dot, I am a bot. https://korg.docs.kernel.org/patchwork/pwbot.html ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2026-10-21 2:50 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-10-19 20:38 [PATCH iproute2 v2 0/3] iplink: bond: fix partner LACP state decoding Dave Seddon 2026-10-19 20:38 ` [PATCH iproute2 v2 1/3] iplink_bond_slave: read ad_partner_oper_port_state as u16 Dave Seddon 2026-10-19 20:38 ` [PATCH iproute2 v2 2/3] testsuite: fix test_on_not on multi-line output Dave Seddon 2026-10-19 20:38 ` [PATCH iproute2 v2 3/3] testsuite: add bond slave LACP port state test Dave Seddon 2026-10-21 2:50 ` [PATCH iproute2 v2 0/3] iplink: bond: fix partner LACP state decoding patchwork-bot+netdevbpf
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox