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

[Hangbin Liu <liuhangbin@gmail.com>]

On Thu, Jul 20, 2023 at 12:40:56PM +0200, Paolo Abeni wrote:
> > 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.
> 
> Let suppose the user has set the lower dev MTU to some N (< 1500) for
> whatever reason (e.g. lower is a vxlan tunnel). After this change, team
> will use mtu = 1500 breaking the connectivity in such scenario/

This looks like a team bug here. Team will not inherited port mtu if
they are some dev type. This would cause adding vxlan to team failed.

But if we change the team dev type and then adding vxlan. Team will
reset the mtu and add vxlan success.

With my patch, if we reset team to 1500, the later adding will failed.
So, as consistency, you suggestion is right.

> As far as I can see team_setup_by_port() takes care of that, inheriting
> such infor from the current slave. What I mean is something alike the
> following.
> 
> Cheers,
> 
> Paolo
> 
> ---
> diff --git a/drivers/net/team/team.c b/drivers/net/team/team.c
> index 555b0b1e9a78..17c8056adbe9 100644
> --- a/drivers/net/team/team.c
> +++ b/drivers/net/team/team.c
> @@ -2135,6 +2135,15 @@ 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);
> +	} else if ((port_dev->flags & (IFF_BROADCAST | IFF_MULTICAST)) ==
> +		   (IFF_BROADCAST | IFF_MULTICAST)) {
> +		dev->flags |= IFF_BROADCAST | IFF_MULTICAST;
> +		dev->flags &= ~(IFF_POINTOPOINT | IFF_NOARP)
> +	}
>  }

Thanks, this looks good to me. I will update the patch.

Hangbin