netdev - Re: [PATCH net] Revert "net: dsa: stop updating master MTU from master.c"

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]

Date:   Fri, 1 Apr 2022 00:52:20 -0300
From:   Luiz Angelo Daros de Luca <luizluca@...il.com>
To:     Vladimir Oltean <vladimir.oltean@....com>
Cc:     "open list:NETWORKING DRIVERS" <netdev@...r.kernel.org>,
        "David S . Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>,
        Paolo Abeni <pabeni@...hat.com>,
        Florian Fainelli <f.fainelli@...il.com>,
        Andrew Lunn <andrew@...n.ch>,
        Vivien Didelot <vivien.didelot@...il.com>
Subject: Re: [PATCH net] Revert "net: dsa: stop updating master MTU from master.c"

> This reverts commit a1ff94c2973c43bc1e2677ac63ebb15b1d1ff846.
>
> Switch drivers that don't implement ->port_change_mtu() will cause the
> DSA master to remain with an MTU of 1500, since we've deleted the other
> code path. In turn, this causes a regression for those systems, where
> MTU-sized traffic can no longer be terminated.
>
> Revert the change taking into account the fact that rtnl_lock() is now
> taken top-level from the callers of dsa_master_setup() and
> dsa_master_teardown(). Also add a comment in order for it to be
> absolutely clear why it is still needed.
>
> Fixes: a1ff94c2973c ("net: dsa: stop updating master MTU from master.c")
> Reported-by: Luiz Angelo Daros de Luca <luizluca@...il.com>
> Signed-off-by: Vladimir Oltean <vladimir.oltean@....com>
> ---
>  net/dsa/master.c | 25 ++++++++++++++++++++++++-
>  1 file changed, 24 insertions(+), 1 deletion(-)
>
> diff --git a/net/dsa/master.c b/net/dsa/master.c
> index 991c2930d631..2851e44c4cf0 100644
> --- a/net/dsa/master.c
> +++ b/net/dsa/master.c
> @@ -335,11 +335,24 @@ static const struct attribute_group dsa_group = {
>         .attrs  = dsa_slave_attrs,
>  };
>
> +static void dsa_master_reset_mtu(struct net_device *dev)
> +{
> +       int err;
> +
> +       err = dev_set_mtu(dev, ETH_DATA_LEN);
> +       if (err)
> +               netdev_dbg(dev,
> +                          "Unable to reset MTU to exclude DSA overheads\n");
> +}
> +
>  int dsa_master_setup(struct net_device *dev, struct dsa_port *cpu_dp)
>  {
> +       const struct dsa_device_ops *tag_ops = cpu_dp->tag_ops;
>         struct dsa_switch *ds = cpu_dp->ds;
>         struct device_link *consumer_link;
> -       int ret;
> +       int mtu, ret;
> +
> +       mtu = ETH_DATA_LEN + dsa_tag_protocol_overhead(tag_ops);
>
>         /* The DSA master must use SET_NETDEV_DEV for this to work. */
>         consumer_link = device_link_add(ds->dev, dev->dev.parent,
> @@ -349,6 +362,15 @@ int dsa_master_setup(struct net_device *dev, struct dsa_port *cpu_dp)
>                            "Failed to create a device link to DSA switch %s\n",
>                            dev_name(ds->dev));
>
> +       /* The switch driver may not implement ->port_change_mtu(), case in
> +        * which dsa_slave_change_mtu() will not update the master MTU either,
> +        * so we need to do that here.
> +        */
> +       ret = dev_set_mtu(dev, mtu);
> +       if (ret)
> +               netdev_warn(dev, "error %d setting MTU to %d to include DSA overhead\n",
> +                           ret, mtu);
> +
>         /* If we use a tagging format that doesn't have an ethertype
>          * field, make sure that all packets from this point on get
>          * sent to the tag format's receive function.
> @@ -384,6 +406,7 @@ void dsa_master_teardown(struct net_device *dev)
>         sysfs_remove_group(&dev->dev.kobj, &dsa_group);
>         dsa_netdev_ops_set(dev, NULL);
>         dsa_master_ethtool_teardown(dev);
> +       dsa_master_reset_mtu(dev);
>         dsa_master_set_promiscuity(dev, -1);
>
>         dev->dsa_ptr = NULL;
> --
> 2.25.1
>

It fixes the issue. Thanks a lot!

Tested-by: Luiz Angelo Daros de Luca <luizluca@...il.com>