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

[Dave Seddon <dave.seddon.ca@gmail.com>]

I have been using an LLM to help port iproute2's netlink handling to
Go. While comparing the two decoders it flagged that ip reads
IFLA_BOND_SLAVE_AD_PARTNER_OPER_PORT_STATE one byte wide. I may well be
missing something; details below in case it is real.

Patch 1 is the one-line fix and stands on its own. Patches 2 and 3 add
a test for it. Please take just patch 1 if you would rather not have
the test.

Issue
- kernel sends u16: nla_put_u16() in bond_fill_slave_info(),
  drivers/net/bonding/bond_netlink.c:79. The field is
  struct port_params.port_state, u16, include/net/bond_3ad.h:205.
- iproute2 reads u8: rta_getattr_u8() in bond_slave_print_opt(),
  ip/iplink_bond_slave.c:148.
- little endian: first byte is the low byte. Output is right because
  the kernel never sets bits above 0xff (the value comes from the
  one-byte LACPDU actor_state, bond_3ad.c:562).
- big endian: first byte is the always-zero high byte. Every bond port
  prints:
    ad_partner_oper_port_state 0 ad_partner_oper_port_state_str <>
  Reproduced on s390x under qemu-user by replaying a generated message.
- regressed in 39ac2d2b802b (v5.6.0). That commit added the _str
  output and copied the actor (u8) block for the partner. The code
  before it used rta_getattr_u16().
- the kernel width is old: u16 in the struct since 1055c9aba3b6
  (2008), exported as u16 since 46ea297ed67c (2015). sysfs and the
  YNL spec agree with it, so the fix belongs in iproute2.
- /proc/net/bonding and sysfs read the struct directly and are fine,
  which is probably why nobody noticed.
- the other bond and bond slave attributes match bond_netlink.c; this
  is the only width mismatch.

Test
- tools/generate_nlmsg writes four RTM_NEWLINK blobs for a bond slave
  with partner state 0x003f, 0x0000, 0x0100 and absent.
- tests/ip/link/show_bond_slave_lacp_state.t decodes them with
  "ip -d monitor file" and checks the printed value. No root needed.
- 0x0100 cannot occur in real LACP. A one-byte reader prints 0 for it
  on little endian and 1 on big endian; only a u16 reader prints 256.
  That makes the bug visible on x86_64 too.

How it was tested
- x86_64, native: new test red with patch 1 reverted (0x0100 prints 0),
  green with it.
- s390x, qemu-user: the blobs were written by an s390x build of
  generate_nlmsg, so they are big endian (partner payload 01 00 for
  0x0100). ip built with and without patch 1:
    without: 0x3f prints "0 <>", 0x0100 prints "1 <active>"
    with:    0x3f prints "63 <active,...>", 0x0100 prints "256"
- results: without patch 1, 1 of 6 checks fails on x86_64 and 2 of 6
  on s390x. With it, all pass on both.
- whole testsuite, rootless, with the series: no change. add_tunnel.t
  and the two netns/set_nsid tests fail (no module load, no
  /var/run/netns), the same as on unpatched e11870d5.
- test_on_not fix checked against matching, mixed, absent and empty
  output, a bad regex and a missing file.
- builds with no warnings. checkpatch --strict is clean apart from the
  exec bit on the new .t, which matches the existing .t files.
- the new .t is shellcheck clean and passes under dash, busybox sh and
  bash --posix, so it does not depend on what /bin/sh points to.

Fix
- patch 1: rta_getattr_u8 -> rta_getattr_u16.
- patch 2: lib/generic.sh test_on_not uses "grep -vqE", which passes
  whenever any line does not match. Fixed so the new test can use it.
  Its one caller, tests/bridge/vlan/show.t, still passes.
- patch 3: the test.

LLM use
- The LLM spotted the mismatch, wrote the patches and drafted this
  letter. Each patch carries Assisted-by: LLM.
- Every result above comes from running the code, natively and under
  qemu-s390x. I have read all of it and can answer for it.

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                                    |   1 +
 ip/iplink_bond_slave.c                        |   2 +-
 testsuite/Makefile                            |   1 +
 testsuite/lib/generic.sh                      |   5 +-
 .../ip/link/show_bond_slave_lacp_state.t      |  33 ++++++
 testsuite/tools/generate_nlmsg.c              | 103 ++++++++++++++++--
 6 files changed, 133 insertions(+), 12 deletions(-)
 create mode 100755 testsuite/tests/ip/link/show_bond_slave_lacp_state.t

-- 
2.55.0