Messages in this thread Patch in this message | | | Date | Tue, 21 Nov 2023 13:53:14 +0200 | From | Vladimir Oltean <> | Subject | Re: [PATCH v6 net-next 6/7] net: ethernet: ti: am65-cpsw-qos: Add Frame Preemption MAC Merge support |
| |
On Tue, Nov 21, 2023 at 01:02:50PM +0200, Roger Quadros wrote: > Yes I'm using openlldp master. > > So I just dumped the "ethtool --show-mm" right before the "lldptool -i $h1 -t -n -V addEthCaps" > and this is what I see > > # MAC Merge layer state for eth0: > # pMAC enabled: on > # TX enabled: off > # TX active: off > # TX minimum fragment size: 252 > # RX minimum fragment size: 124 > # Verify enabled: off > # Verify time: 10 > # Max verify time: 134 > # Verification status: DISABLED > # > # MAC Merge layer state for eth1: > # pMAC enabled: on > # TX enabled: off > # TX active: off > # TX minimum fragment size: 124 > # RX minimum fragment size: 124 > # Verify enabled: off > # Verify time: 10 > # Max verify time: 134 > # Verification status: DISABLED > # > # Additional Ethernet Capabilities TLV > # Preemption capability supported > # Preemption capability not enabled > # Preemption capability not active > # Additional fragment size: 3 (252 octets) > # Additional Ethernet Capabilities TLV > # Preemption capability supported > # Preemption capability not enabled > # Preemption capability not active > # Additional fragment size: 1 (124 octets) > # Warning: Stopping lldpad.service, but it can still be activated by: > # lldpad.socket > # TEST: LLDP [FAIL] > > > If I add the following lines at the beginning of lldp() routine, > then it works. > > lldp() > { > RET=0 > > + ethtool --set-mm $h1 tx-enabled on verify-enabled on > + ethtool --set-mm $h2 tx-enabled on verify-enabled on > ... > } > > Is lldp supposed to turn on tx-enabled and verify-enabled for us > or it is test scritps responsibility?
lldpad should absolutely do that. https://github.com/intel/openlldp/blob/master/lldp_8023.c#L701
Try to see what goes on and if there isn't, in fact, an error during the netlink communication with the kernel.
Edit /usr/local/lib/systemd/system/lldpad.service: ExecStart=/usr/local/sbin/lldpad -t -V 7 ~~~~~ increases log level Then run:
$ systemctl daemon-reload $ journalctl -u lldpad.service -f & $ ./ethtool_mm.sh eno0 swp0
During the test you should see:
lldpad[4764]: eno0: Link partner preemption capability supported lldpad[4764]: eno0: Link partner preemption capability not enabled lldpad[4764]: eno0: Link partner preemption capability not active lldpad[4764]: eno0: Link partner minimum fragment size: 252 octets lldpad[4764]: eno0: initiating MM verification with a retry interval of 127 ms... lldpad[4764]: rxProcessFrame: allocated TLV 0 was not stored! 0xaaaafd7cfbe0 lldpad[4764]: swp0: Link partner preemption capability supported lldpad[4764]: swp0: Link partner preemption capability not enabled lldpad[4764]: swp0: Link partner preemption capability not active lldpad[4764]: swp0: Link partner minimum fragment size: 60 octets lldpad[4764]: swp0: initiating MM verification with a retry interval of 128 ms... lldpad[4764]: rxProcessFrame: allocated TLV 0 was not stored! 0xaaaafd7cfd30
> > The test fails later at "addFragSize 0", but that is because we don't > support RX fragment size 60 due to errata. > If I skip that test then all the rest of the tests pass.
Hmm, yeah, the test is dumb. lldpad has this logic, so if we request 0 it should still advertise 1.
if (config_add_frag_size < add_frag_size) { LLDPAD_WARN("%s: Configured addFragSize (%d) smaller than the minimum value requested by kernel (%d). Using the latter\n", bd->ifname, config_add_frag_size, add_frag_size); config_add_frag_size = add_frag_size; }
I guess that logic does engage, but the selftest doesn't expect that it will, because it expects that lldpad will report back exactly the requested value - and it will report the true value instead.
Luckily I know what to do here, see the patch below.
From 0ed218345f16a0f2c0efd5eba1838ccb3d8e4921 Mon Sep 17 00:00:00 2001 From: Vladimir Oltean <vladimir.oltean@nxp.com> Date: Tue, 21 Nov 2023 13:42:14 +0200 Subject: [PATCH] selftests: forwarding: ethtool_mm: support devices with higher rx-min-frag-size
Some devices have errata due to which they cannot report ETH_ZLEN (60) in the rx-min-frag-size. This was foreseen of course, and lldpad has logic that when we request it to advertise addFragSize 0, it will round it up to the lowest value that is _actually_ supported by the hardware.
The problem is that the selftest expects lldpad to report back to us the same value as we requested.
Make the selftest smarter by figuring out on its own what is a reasonable value to expect.
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com> --- .../selftests/net/forwarding/ethtool_mm.sh | 37 ++++++++++++++++++- 1 file changed, 35 insertions(+), 2 deletions(-)
diff --git a/tools/testing/selftests/net/forwarding/ethtool_mm.sh b/tools/testing/selftests/net/forwarding/ethtool_mm.sh index 39e736f30322..6212913f4ad1 100755 --- a/tools/testing/selftests/net/forwarding/ethtool_mm.sh +++ b/tools/testing/selftests/net/forwarding/ethtool_mm.sh @@ -155,15 +155,48 @@ manual_failed_verification_h2_to_h1() manual_failed_verification $h2 $h1 } +smallest_supported_add_frag_size() +{ + local iface=$1 + local rx_min_frag_size= + + rx_min_frag_size=$(ethtool --json --show-mm $iface | \ + jq '.[]."rx-min-frag-size"') + + if [ $rx_min_frag_size -le 60 ]; then + echo 0 + elif [ $rx_min_frag_size -le 124 ]; then + echo 1 + elif [ $rx_min_frag_size -le 188 ]; then + echo 2 + elif [ $rx_min_frag_size -le 252 ]; then + echo 3 + else + echo "$iface: RX min frag size $rx_min_frag_size cannot be advertised over LLDP" + exit 1 + fi +} + +expected_add_frag_size() +{ + local iface=$1 + local requested=$2 + local min=$(smallest_supported_add_frag_size $iface) + + [ $requested -le $min ] && echo $min || echo $requested +} + lldp_change_add_frag_size() { local add_frag_size=$1 + local pattern= lldptool -T -i $h1 -V addEthCaps addFragSize=$add_frag_size >/dev/null # Wait for TLVs to be received sleep 2 - lldptool -i $h2 -t -n -V addEthCaps | \ - grep -q "Additional fragment size: $add_frag_size" + pattern=$(printf "Additional fragment size: %d" \ + $(expected_add_frag_size $h1 $add_frag_size)) + lldptool -i $h2 -t -n -V addEthCaps | grep -q "$pattern" } lldp() -- 2.34.1
| |