All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH net 0/2] Fix up dev flags when add P2P down link
@ 2023-07-14  2:51 Hangbin Liu
  2023-07-14  2:52 ` [PATCH net 1/2] bonding: reset bond's flags when down link is P2P device Hangbin Liu
  2023-07-14  2:52 ` [PATCH net 2/2] team: reset team's " Hangbin Liu
  0 siblings, 2 replies; 7+ messages in thread
From: Hangbin Liu @ 2023-07-14  2:51 UTC (permalink / raw)
  To: netdev
  Cc: Jay Vosburgh, David S . Miller, Jakub Kicinski, Paolo Abeni,
	Eric Dumazet, Liang Li, Jiri Pirko, Hangbin Liu

When adding p2p interfaces to bond/team. The POINTOPOINT, NOARP flags are
not inherit to up devices. Which will trigger IPv6 DAD. Since there is
no ethernet MAC address for P2P devices. This will cause unexpected DAD
failures.

Hangbin Liu (2):
  bonding: reset bond's flags when down link is P2P device
  team: reset team's flags when down link is P2P device

 drivers/net/bonding/bond_main.c | 4 ++++
 drivers/net/team/team.c         | 4 ++++
 2 files changed, 8 insertions(+)

-- 
2.38.1


^ permalink raw reply	[flat|nested] 7+ messages in thread
* [PATCH net 1/2] bonding: reset bond's flags when down link is P2P device
  2023-07-14  2:51 [PATCH net 0/2] Fix up dev flags when add P2P down link Hangbin Liu
@ 2023-07-14  2:52 ` Hangbin Liu
  2023-07-14  6:51   ` Nikolay Aleksandrov
  2023-07-14  2:52 ` [PATCH net 2/2] team: reset team's " Hangbin Liu
  1 sibling, 1 reply; 7+ messages in thread
From: Hangbin Liu @ 2023-07-14  2:52 UTC (permalink / raw)
  To: netdev
  Cc: Jay Vosburgh, David S . Miller, Jakub Kicinski, Paolo Abeni,
	Eric Dumazet, Liang Li, Jiri Pirko, Hangbin Liu

When adding a point to point downlink to the bond, we neglected to reset
the bond'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 the bonding.

To address this issue, let's reset the bond's flags for P2P interfaces.

Before fix:
7: gre0@NONE: <POINTOPOINT,NOARP,SLAVE,UP,LOWER_UP> mtu 1500 qdisc noqueue master bond0 state UNKNOWN group default qlen 1000
    link/gre6 2006:70:10::1 peer 2006:70:10::2 permaddr 167f:18:f188::
8: bond0: <BROADCAST,MULTICAST,MASTER,UP,LOWER_UP> mtu 1500 qdisc noqueue state UP group default qlen 1000
    link/gre6 2006:70:10::1 brd 2006:70:10::2
    inet6 fe80::200:ff:fe00:0/64 scope link
       valid_lft forever preferred_lft forever

After fix:
7: gre0@NONE: <POINTOPOINT,NOARP,SLAVE,UP,LOWER_UP> mtu 1500 qdisc noqueue master bond2 state UNKNOWN group default qlen 1000
    link/gre6 2006:70:10::1 peer 2006:70:10::2 permaddr c29e:557a:e9d9::
8: bond0: <POINTOPOINT,NOARP,MASTER,UP,LOWER_UP> mtu 1500 qdisc noqueue state UP group default qlen 1000
    link/gre6 2006:70:10::1 peer 2006:70:10::2
    inet6 fe80::1/64 scope link
       valid_lft forever preferred_lft forever

Reported-by: Liang Li <liali@redhat.com>
Links: https://bugzilla.redhat.com/show_bug.cgi?id=2221438
Fixes: 872254dd6b1f ("net/bonding: Enable bonding to enslave non ARPHRD_ETHER")
Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
---
 drivers/net/bonding/bond_main.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 7a0f25301f7e..0186b2d19e8d 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -1508,6 +1508,10 @@ static void bond_setup_by_slave(struct net_device *bond_dev,
 
 	memcpy(bond_dev->broadcast, slave_dev->broadcast,
 		slave_dev->addr_len);
+
+	if (slave_dev->flags & IFF_POINTOPOINT)
+		bond_dev->flags &= ~(IFF_BROADCAST | IFF_MULTICAST);
+		bond_dev->flags |= (IFF_POINTOPOINT | IFF_NOARP);
 }
 
 /* On bonding slaves other than the currently active slave, suppress
-- 
2.38.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread
* [PATCH net 2/2] team: reset team's flags when down link is P2P device
  2023-07-14  2:51 [PATCH net 0/2] Fix up dev flags when add P2P down link Hangbin Liu
  2023-07-14  2:52 ` [PATCH net 1/2] bonding: reset bond's flags when down link is P2P device Hangbin Liu
@ 2023-07-14  2:52 ` Hangbin Liu
  2023-07-14  6:48   ` Jiri Pirko
  2023-07-14  6:52   ` Nikolay Aleksandrov
  1 sibling, 2 replies; 7+ messages in thread
From: Hangbin Liu @ 2023-07-14  2:52 UTC (permalink / raw)
  To: netdev
  Cc: Jay Vosburgh, David S . Miller, Jakub Kicinski, Paolo Abeni,
	Eric Dumazet, Liang Li, Jiri Pirko, Hangbin Liu

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")
Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
---
 drivers/net/team/team.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/net/team/team.c b/drivers/net/team/team.c
index 555b0b1e9a78..c11783efe13f 100644
--- a/drivers/net/team/team.c
+++ b/drivers/net/team/team.c
@@ -2135,6 +2135,10 @@ 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,
-- 
2.38.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread
* Re: [PATCH net 2/2] team: reset team's flags when down link is P2P device
  2023-07-14  2:52 ` [PATCH net 2/2] team: reset team's " Hangbin Liu
@ 2023-07-14  6:48   ` Jiri Pirko
  2023-07-14  6:52   ` Nikolay Aleksandrov
  1 sibling, 0 replies; 7+ messages in thread
From: Jiri Pirko @ 2023-07-14  6:48 UTC (permalink / raw)
  To: Hangbin Liu
  Cc: netdev, Jay Vosburgh, David S . Miller, Jakub Kicinski,
	Paolo Abeni, Eric Dumazet, Liang Li, Jiri Pirko

Fri, Jul 14, 2023 at 04:52:01AM CEST, liuhangbin@gmail.com 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")
>Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>

Reviewed-by: Jiri Pirko <jiri@nvidia.com>

^ permalink raw reply	[flat|nested] 7+ messages in thread
* Re: [PATCH net 1/2] bonding: reset bond's flags when down link is P2P device
  2023-07-14  2:52 ` [PATCH net 1/2] bonding: reset bond's flags when down link is P2P device Hangbin Liu
@ 2023-07-14  6:51   ` Nikolay Aleksandrov
  0 siblings, 0 replies; 7+ messages in thread
From: Nikolay Aleksandrov @ 2023-07-14  6:51 UTC (permalink / raw)
  To: Hangbin Liu, netdev
  Cc: Jay Vosburgh, David S . Miller, Jakub Kicinski, Paolo Abeni,
	Eric Dumazet, Liang Li, Jiri Pirko

On 14/07/2023 05:52, Hangbin Liu wrote:
> When adding a point to point downlink to the bond, we neglected to reset
> the bond'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 the bonding.
> 
> To address this issue, let's reset the bond's flags for P2P interfaces.
> 
> Before fix:
> 7: gre0@NONE: <POINTOPOINT,NOARP,SLAVE,UP,LOWER_UP> mtu 1500 qdisc noqueue master bond0 state UNKNOWN group default qlen 1000
>     link/gre6 2006:70:10::1 peer 2006:70:10::2 permaddr 167f:18:f188::
> 8: bond0: <BROADCAST,MULTICAST,MASTER,UP,LOWER_UP> mtu 1500 qdisc noqueue state UP group default qlen 1000
>     link/gre6 2006:70:10::1 brd 2006:70:10::2
>     inet6 fe80::200:ff:fe00:0/64 scope link
>        valid_lft forever preferred_lft forever
> 
> After fix:
> 7: gre0@NONE: <POINTOPOINT,NOARP,SLAVE,UP,LOWER_UP> mtu 1500 qdisc noqueue master bond2 state UNKNOWN group default qlen 1000
>     link/gre6 2006:70:10::1 peer 2006:70:10::2 permaddr c29e:557a:e9d9::
> 8: bond0: <POINTOPOINT,NOARP,MASTER,UP,LOWER_UP> mtu 1500 qdisc noqueue state UP group default qlen 1000
>     link/gre6 2006:70:10::1 peer 2006:70:10::2
>     inet6 fe80::1/64 scope link
>        valid_lft forever preferred_lft forever
> 
> Reported-by: Liang Li <liali@redhat.com>
> Links: https://bugzilla.redhat.com/show_bug.cgi?id=2221438
> Fixes: 872254dd6b1f ("net/bonding: Enable bonding to enslave non ARPHRD_ETHER")
> Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
> ---
>  drivers/net/bonding/bond_main.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
> index 7a0f25301f7e..0186b2d19e8d 100644
> --- a/drivers/net/bonding/bond_main.c
> +++ b/drivers/net/bonding/bond_main.c
> @@ -1508,6 +1508,10 @@ static void bond_setup_by_slave(struct net_device *bond_dev,
>  
>  	memcpy(bond_dev->broadcast, slave_dev->broadcast,
>  		slave_dev->addr_len);
> +
> +	if (slave_dev->flags & IFF_POINTOPOINT)
> +		bond_dev->flags &= ~(IFF_BROADCAST | IFF_MULTICAST);
> +		bond_dev->flags |= (IFF_POINTOPOINT | IFF_NOARP);

missing {} ?

>  }
>  
>  /* On bonding slaves other than the currently active slave, suppress


^ permalink raw reply	[flat|nested] 7+ messages in thread
* Re: [PATCH net 2/2] team: reset team's flags when down link is P2P device
  2023-07-14  2:52 ` [PATCH net 2/2] team: reset team's " Hangbin Liu
  2023-07-14  6:48   ` Jiri Pirko
@ 2023-07-14  6:52   ` Nikolay Aleksandrov
  2023-07-14  8:10     ` Hangbin Liu
  1 sibling, 1 reply; 7+ messages in thread
From: Nikolay Aleksandrov @ 2023-07-14  6:52 UTC (permalink / raw)
  To: Hangbin Liu, netdev
  Cc: Jay Vosburgh, David S . Miller, Jakub Kicinski, Paolo Abeni,
	Eric Dumazet, Liang Li, Jiri Pirko

On 14/07/2023 05:52, 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")
> Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
> ---
>  drivers/net/team/team.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/net/team/team.c b/drivers/net/team/team.c
> index 555b0b1e9a78..c11783efe13f 100644
> --- a/drivers/net/team/team.c
> +++ b/drivers/net/team/team.c
> @@ -2135,6 +2135,10 @@ 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);

here too, looks like missing {}

>  }
>  
>  static int team_dev_type_check_change(struct net_device *dev,


^ permalink raw reply	[flat|nested] 7+ messages in thread
* Re: [PATCH net 2/2] team: reset team's flags when down link is P2P device
  2023-07-14  6:52   ` Nikolay Aleksandrov
@ 2023-07-14  8:10     ` Hangbin Liu
  0 siblings, 0 replies; 7+ messages in thread
From: Hangbin Liu @ 2023-07-14  8:10 UTC (permalink / raw)
  To: Nikolay Aleksandrov
  Cc: netdev, Jay Vosburgh, David S . Miller, Jakub Kicinski,
	Paolo Abeni, Eric Dumazet, Liang Li, Jiri Pirko

On Fri, Jul 14, 2023 at 09:52:03AM +0300, Nikolay Aleksandrov wrote:
> > +	if (port_dev->flags & IFF_POINTOPOINT)
> > +		dev->flags &= ~(IFF_BROADCAST | IFF_MULTICAST);
> > +		dev->flags |= (IFF_POINTOPOINT | IFF_NOARP);
> 
> here too, looks like missing {}

Yes, you are right. I forgot to add the {}. When do testing before post the
patch. I just checked adding gre device to bonding and didn't check adding
ethernet interface...

Thanks for your review.

Hangbin

^ permalink raw reply	[flat|nested] 7+ messages in thread
end of thread, other threads:[~2023-07-14  8:10 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-07-14  2:51 [PATCH net 0/2] Fix up dev flags when add P2P down link Hangbin Liu
2023-07-14  2:52 ` [PATCH net 1/2] bonding: reset bond's flags when down link is P2P device Hangbin Liu
2023-07-14  6:51   ` Nikolay Aleksandrov
2023-07-14  2:52 ` [PATCH net 2/2] team: reset team's " Hangbin Liu
2023-07-14  6:48   ` Jiri Pirko
2023-07-14  6:52   ` Nikolay Aleksandrov
2023-07-14  8:10     ` Hangbin Liu

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.