On Wed, 3 Feb 2021 18:08:21 +0200 Vladimir Oltean wrote:
> From: Vladimir Oltean <vladimir.olt...@nxp.com>
>
> This is not fixing any actual bug that I know of, but having a DSA
> interface that is up even when its lower (master) interface is down is
> one of those things that just do not sound right.
>
> Yes, DSA checks if the master is up before actually bringing the
> user interface up, but nobody prevents bringing the master interface
> down immediately afterwards... Then the user ports would attempt
> dev_queue_xmit on an interface that is down, and wonder what's wrong.
>
> This patch prevents that from happening. NETDEV_GOING_DOWN is the
> notification emitted _before_ the master actually goes down, and we are
> protected by the rtnl_mutex, so all is well.
>
> $ ip link set eno2 down
> [ 763.672211] mscc_felix 0000:00:00.5 swp0: Link is Down
> [ 763.880137] mscc_felix 0000:00:00.5 swp1: Link is Down
> [ 764.078773] mscc_felix 0000:00:00.5 swp2: Link is Down
> [ 764.197106] mscc_felix 0000:00:00.5 swp3: Link is Down
> [ 764.299384] fsl_enetc 0000:00:00.2 eno2: Link is Down
>
> For those of you reading this because you were doing switch testing
> such as latency measurements for autonomously forwarded traffic, and you
> needed a controlled environment with no extra packets sent by the
> network stack, this patch breaks that, because now the user ports go
> down too, which may shut down the PHY etc. But please don't do it like
> that, just do instead:
>
> tc qdisc add dev eno2 clsact
> tc filter add dev eno2 egress flower action drop
>
> Signed-off-by: Vladimir Oltean <vladimir.olt...@nxp.com>
> Reviewed-by: Andrew Lunn <and...@lunn.ch>
> Reviewed-by: Florian Fainelli <f.faine...@gmail.com>
> ---
> Changes in v2:
> Fix typo: !dsa_is_user_port -> dsa_is_user_port.
>
> net/dsa/slave.c | 30 ++++++++++++++++++++++++++++++
> 1 file changed, 30 insertions(+)
>
> diff --git a/net/dsa/slave.c b/net/dsa/slave.c
> index 4616bd7c8684..aa7bd223073c 100644
> --- a/net/dsa/slave.c
> +++ b/net/dsa/slave.c
> @@ -2084,6 +2084,36 @@ static int dsa_slave_netdevice_event(struct
> notifier_block *nb,
> err = dsa_port_lag_change(dp, info->lower_state_info);
> return notifier_from_errno(err);
> }
> + case NETDEV_GOING_DOWN: {
> + struct dsa_port *dp, *cpu_dp;
> + struct dsa_switch_tree *dst;
> + int err = 0;
> +
> + if (!netdev_uses_dsa(dev))
> + return NOTIFY_DONE;
> +
> + cpu_dp = dev->dsa_ptr;
> + dst = cpu_dp->ds->dst;
> +
> + list_for_each_entry(dp, &dst->ports, list) {
> + if (dsa_is_user_port(dp->ds, dp->index)) {
> + struct net_device *slave = dp->slave;
> +
> + if (!(slave->flags & IFF_UP))
> + continue;
> +
> + err = dev_change_flags(slave,
> + slave->flags & ~IFF_UP,
> + NULL);
> + if (err)
> + break;
> + }
> + }
Perhaps:
LIST_HEAD(close_list);
list_for_each_entry(dp, &dst->ports, list)
list_add(&slave->close_list, &close_list);
dev_close_many(&close_list, true);
return NOTIFY_OK;
But we can keep as is if you prefer.
> + return notifier_from_errno(err);
> + }
> + default:
> + break;
> }
>
> return NOTIFY_DONE;
