[PATCH net 0/2] Fix up dev flags when add P2P down link

[PATCH net 0/2] Fix up dev flags when add P2P down link

[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

[PATCH net 1/2] bonding: reset bond's flags when down link is P2P device

[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

Re: [PATCH net 1/2] bonding: reset bond's flags when down link is P2P device

[Nikolay Aleksandrov]

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

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

[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

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

[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>

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

[Nikolay Aleksandrov]

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,

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

[Hangbin Liu]

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