netdev - Re: [PATCHv3 net 2/2] team: reset team's flags when down link is P2P device

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: <ZLkCXLsf+K6GLS/6@Laptop-X1>
Date: Thu, 20 Jul 2023 17:46:04 +0800
From: Hangbin Liu <liuhangbin@...il.com>
To: Paolo Abeni <pabeni@...hat.com>
Cc: netdev@...r.kernel.org, Jay Vosburgh <j.vosburgh@...il.com>,
	"David S . Miller" <davem@...emloft.net>,
	Jakub Kicinski <kuba@...nel.org>,
	Eric Dumazet <edumazet@...gle.com>, Liang Li <liali@...hat.com>,
	Jiri Pirko <jiri@...dia.com>,
	Nikolay Aleksandrov <razor@...ckwall.org>
Subject: Re: [PATCHv3 net 2/2] team: reset team's flags when down link is P2P
 device

On Thu, Jul 20, 2023 at 10:29:19AM +0200, Paolo Abeni wrote:
> > +static void team_ether_setup(struct net_device *dev)
> > +{
> > +	unsigned int flags = dev->flags & IFF_UP;
> > +
> > +	ether_setup(dev);
> > +	dev->flags |= flags;
> > +	dev->priv_flags &= ~IFF_TX_SKB_SHARING;
> 
> I think we can't do the above. e.g. ether_setup() sets dev->mtu to
> ethernet default, while prior to this patch dev inherited mtu from the
> slaved device. The change may affect the user-space in bad ways.

Hi Paolo,

I don't see the reason why we should inherited a none ethernet dev's mtu
to an ethernet dev (i.e. add a none ethernet dev to team, then delete it and
re-add a ethernet dev to team). I think the dev type has changed, so the
mtu should also be updated.

BTW, after adding the port, team will also set port's mtu to team's mtu.

> 
> I think we just need an 'else' branch in the point2point check above,
> restoring the bcast/mcast flags as needed.

Reset the flags is not enough. All the dev header_ops, type, etc are
all need to be reset back, that's why we need to call ether_setup().

Thanks
Hangbin