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 PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Thu, 31 Mar 2022 13:31:05 +0000 From: Vladimir Oltean <vladimir.oltean@....com> To: Luiz Angelo Daros de Luca <luizluca@...il.com> CC: Alvin Šipraga <ALSI@...g-olufsen.dk>, "open list:NETWORKING DRIVERS" <netdev@...r.kernel.org>, "David S. Miller" <davem@...emloft.net>, Jakub Kicinski <kuba@...nel.org>, Andrew Lunn <andrew@...n.ch>, Vivien Didelot <vivien.didelot@...il.com>, Florian Fainelli <f.fainelli@...il.com> Subject: Re: [PATCH v2 net-next 3/6] net: dsa: stop updating master MTU from master.c On Thu, Mar 31, 2022 at 02:53:46AM -0300, Luiz Angelo Daros de Luca wrote: > Hi Vladimir, > > I think I found an issue with this patch. > > > At present there are two paths for changing the MTU of the DSA master. > > > > The first is: > > > > dsa_tree_setup > > -> dsa_tree_setup_ports > > -> dsa_port_setup > > -> dsa_slave_create > > -> dsa_slave_change_mtu > > -> dev_set_mtu(master) > > The first code from dsa_slave_change_mtu() is: > > if (!ds->ops->port_change_mtu) > return -EOPNOTSUPP; > > So, when the switch does not implement ds->ops->port_change_mtu, the > master MTU will never be updated. This is the case for > drivers/net/dsa/realtek/rtl8365mb.c. Before this patch, > ops->port_change_mtu was optional. We either need to turn it into a > mandatory function (even if it is a no-op that fails when mtu is > different) or change the dsa_slave_change_mtu to only return > -EOPNOTSUPP when the new slave MTU differs from current slave MTU. > > Regards, > > Luiz Thanks Luiz, you are right, I've sent a revert patch here: https://patchwork.kernel.org/project/netdevbpf/patch/20220331132854.1395040-1-vladimir.oltean@nxp.com/ My only problem with updating the MTU from dsa_master_setup() was that it was taking rtnl_lock(), but it looks like I went too far by deleting it entirely. I restored the old code structure, which should need no further changes.
Powered by blists - more mailing lists