| Message ID | 20230714081340.2064472-3-liuhangbin@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | Fix up dev flags when add P2P down link | expand |
Hi, On Fri, 2023-07-14 at 16:13 +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. > > Reported-by: Liang Li <liali@redhat.com> > Links: https://bugzilla.redhat.com/show_bug.cgi?id=2221438 > Fixes: 1d76efe1577b ("team: add support for non-ethernet devices") > Reviewed-by: Jiri Pirko <jiri@nvidia.com> > Signed-off-by: Hangbin Liu <liuhangbin@gmail.com> > --- > drivers/net/team/team.c | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/drivers/net/team/team.c b/drivers/net/team/team.c > index 555b0b1e9a78..9104e373c8cb 100644 > --- a/drivers/net/team/team.c > +++ b/drivers/net/team/team.c > @@ -2135,6 +2135,11 @@ 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); > + } It's unclear to me what happens with the following sequence of events: * p2p dev is enslaved to team (IFF_BROADCAST cleared) * p2p dev is removed from team * plain ether device is enslaved to team. I don't see where/when IFF_BROADCAST is set again. Could you please point it out? Thanks! Paolo
On Tue, Jul 18, 2023 at 10:01:27AM +0200, Paolo Abeni wrote: > > diff --git a/drivers/net/team/team.c b/drivers/net/team/team.c > > index 555b0b1e9a78..9104e373c8cb 100644 > > --- a/drivers/net/team/team.c > > +++ b/drivers/net/team/team.c > > @@ -2135,6 +2135,11 @@ 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); > > + } > > It's unclear to me what happens with the following sequence of events: > > * p2p dev is enslaved to team (IFF_BROADCAST cleared) > * p2p dev is removed from team > * plain ether device is enslaved to team. > > I don't see where/when IFF_BROADCAST is set again. Could you please > point it out? Hmm, you are right. Bonding will call bond_ether_setup(), ether_setup() to reset the dev flags. But team didn't do that. I will fix it. Thanks Hangbin
diff --git a/drivers/net/team/team.c b/drivers/net/team/team.c index 555b0b1e9a78..9104e373c8cb 100644 --- a/drivers/net/team/team.c +++ b/drivers/net/team/team.c @@ -2135,6 +2135,11 @@ 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 int team_dev_type_check_change(struct net_device *dev,
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. Reported-by: Liang Li <liali@redhat.com> Links: https://bugzilla.redhat.com/show_bug.cgi?id=2221438 Fixes: 1d76efe1577b ("team: add support for non-ethernet devices") Reviewed-by: Jiri Pirko <jiri@nvidia.com> Signed-off-by: Hangbin Liu <liuhangbin@gmail.com> --- drivers/net/team/team.c | 5 +++++ 1 file changed, 5 insertions(+)