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

[Paolo Abeni <pabeni@redhat.com>]

On Tue, 2023-07-18 at 18:17 +0800, Hangbin Liu wrote:
> When adding a point to point downlink to team device, we neglected to reset
> the team's flags, which were still using flags like BROADCAST and
> MULTICAST. Consequently, this would initiate ARP/DAD for P2P downlink
> interfaces, such as when adding a GRE device to team device. Fix this by
> remove multicast/broadcast flags and add p2p and noarp flags.
> 
> After removing the none ethernet interface and adding an ethernet interface
> to team, we need to reset team interface flags and hw address back. Unlike
> bonding interface, team do not need restore IFF_MASTER, IFF_SLAVE flags.
> 
> Reported-by: Liang Li <liali@redhat.com>
> Closes: https://bugzilla.redhat.com/show_bug.cgi?id=2221438
> Fixes: 1d76efe1577b ("team: add support for non-ethernet devices")
> Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
> ---
> v3: add function team_ether_setup to reset team back to ethernet.
> v2: Add the missed {} after if checking.
> ---
>  drivers/net/team/team.c | 23 ++++++++++++++++++++++-
>  1 file changed, 22 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/team/team.c b/drivers/net/team/team.c
> index 555b0b1e9a78..2e124a3b81d1 100644
> --- a/drivers/net/team/team.c
> +++ b/drivers/net/team/team.c
> @@ -2135,6 +2135,20 @@ static void team_setup_by_port(struct net_device *dev,
>  	dev->mtu = port_dev->mtu;
>  	memcpy(dev->broadcast, port_dev->broadcast, port_dev->addr_len);
>  	eth_hw_addr_inherit(dev, port_dev);
> +
> +	if (port_dev->flags & IFF_POINTOPOINT) {
> +		dev->flags &= ~(IFF_BROADCAST | IFF_MULTICAST);
> +		dev->flags |= (IFF_POINTOPOINT | IFF_NOARP);
> +	}
> +}
> +
> +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.

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

Yes, we have slightly different behaviours between bond and team, but
we can't change that easily.

Cheers,

Paolo