On Thu, Aug 21, 2025 at 11:14:34AM +0200, Paolo Abeni wrote:
> 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.
>
OK, I will fix it.
Regards
Hangbin
