netdev - Re: [PATCHv3 net-next 3/3] selftests: bonding: add test for LACP actor port priority

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <ea44c100-34fc-4dec-a749-454d224fa84e@redhat.com>
Date: Thu, 21 Aug 2025 11:14:34 +0200
From: Paolo Abeni <pabeni@...hat.com>
To: Hangbin Liu <liuhangbin@...il.com>, netdev@...r.kernel.org
Cc: Jay Vosburgh <jv@...sburgh.net>, Andrew Lunn <andrew+netdev@...n.ch>,
 "David S. Miller" <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>,
 Jakub Kicinski <kuba@...nel.org>, Nikolay Aleksandrov <razor@...ckwall.org>,
 Simon Horman <horms@...nel.org>, Shuah Khan <shuah@...nel.org>,
 Jonathan Corbet <corbet@....net>, Petr Machata <petrm@...dia.com>,
 Amit Cohen <amcohen@...dia.com>, Vladimir Oltean <vladimir.oltean@....com>,
 Stephen Hemminger <stephen@...workplumber.org>,
 David Ahern <dsahern@...il.com>, Jonas Gorski <jonas.gorski@...il.com>,
 linux-doc@...r.kernel.org, linux-kselftest@...r.kernel.org
Subject: Re: [PATCHv3 net-next 3/3] selftests: bonding: add test for LACP
 actor port priority

On 8/18/25 11:23 AM, Hangbin Liu wrote:
> +# Trigger link state change to reselect the aggregator
> +ip -n "${c_ns}" link set eth1 down
> +sleep 1
> +ip -n "${c_ns}" link set eth1 up
> +# the active agg should be connect to switch
> +bond_agg_id=$(cmd_jq "ip -n ${c_ns} -d -j link show bond0" ".[].linkinfo.info_data.ad_info.aggregator")
> +eth0_agg_id=$(cmd_jq "ip -n ${c_ns} -d -j link show eth0" ".[].linkinfo.info_slave_data.ad_aggregator_id")
> +[ "${bond_agg_id}" -ne "${eth0_agg_id}" ] && RET=1

A few lines above exceed 100 chars, it would be better to wrap them

> +log_test "bond 802.3ad" "actor_port_prio select"
> +
> +# Change the actor port prio and re-test
> +ip -n "${c_ns}" link set eth0 type bond_slave actor_port_prio 10
> +ip -n "${c_ns}" link set eth2 type bond_slave actor_port_prio 1000
> +# Trigger link state change to reselect the aggregator
> +ip -n "${c_ns}" link set eth1 down
> +sleep 1
> +ip -n "${c_ns}" link set eth1 up
> +# now the active agg should be connect to backup switch
> +bond_agg_id=$(cmd_jq "ip -n ${c_ns} -d -j link show bond0" ".[].linkinfo.info_data.ad_info.aggregator")
> +eth2_agg_id=$(cmd_jq "ip -n ${c_ns} -d -j link show eth2" ".[].linkinfo.info_slave_data.ad_aggregator_id")
> +# shellcheck disable=SC2034
> +if [ "${bond_agg_id}" -ne "${eth2_agg_id}" ]; then
> +	RET=1
> +fi
> +log_test "bond 802.3ad" "actor_port_prio switch"

The test above is basically the same of the previous one, with a
slightly different style, it would be better to factor the whole
status cycling, data fetching and comparison in an helper to avoid code
duplication and the mentioned confusing difference.

Thanks,

Paolo