[PATCH] net:bonding:support balance-alb with openvswitch

[PATCH] net:bonding:support balance-alb with openvswitch

[Mat Kowalski]

Commit d5410ac7b0ba ("net:bonding:support balance-alb interface with
vlan to bridge") introduced a support for balance-alb mode for
interfaces connected to the linux bridge by fixing missing matching of
MAC entry in FDB. In our testing we discovered that it still does not
work when the bond is connected to the OVS bridge as show in diagram
below:

eth1(mac:eth1_mac)--bond0(balance-alb,mac:eth0_mac)--eth0(mac:eth0_mac)
                       |
                     bond0.150(mac:eth0_mac)
                               |
                     ovs_bridge(ip:bridge_ip,mac:eth0_mac)

This patch fixes it by checking not only if the device is a bridge but
also if it is an openvswitch.

Signed-off-by: Mateusz Kowalski <mko@redhat.com>
---
  drivers/net/bonding/bond_alb.c | 3 +--
  include/linux/netdevice.h      | 5 +++++
  2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/net/bonding/bond_alb.c b/drivers/net/bonding/bond_alb.c
index b9dbad3a8af8..6f2ffcc4f19c 100644
--- a/drivers/net/bonding/bond_alb.c
+++ b/drivers/net/bonding/bond_alb.c
@@ -668,7 +668,7 @@ static struct slave *rlb_arp_xmit(struct sk_buff 
*skb, struct bonding *bond)

         dev = ip_dev_find(dev_net(bond->dev), arp->ip_src);
         if (dev) {
-               if (netif_is_bridge_master(dev)) {
+               if (netif_is_any_bridge_master(dev)) {
                         dev_put(dev);
                         return NULL;
                 }
@@ -1833,4 +1833,3 @@ void bond_alb_clear_vlan(struct bonding *bond, 
unsigned short vlan_id)
         if (bond->alb_info.rlb_enabled)
                 rlb_clear_vlan(bond, vlan_id);
  }
-
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index b828c7a75be2..d250a0b947f1 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -5102,6 +5102,11 @@ static inline bool netif_is_ovs_port(const struct 
net_device *dev)
         return dev->priv_flags & IFF_OVS_DATAPATH;
  }

+static inline bool netif_is_any_bridge_master(const struct net_device *dev)
+{
+       return netif_is_bridge_master(dev)) || netif_is_ovs_master(dev);
+}
+
  static inline bool netif_is_any_bridge_port(const struct net_device *dev)
  {
         return netif_is_bridge_port(dev) || netif_is_ovs_port(dev);
-- 
2.41.0

Re: [PATCH] net:bonding:support balance-alb with openvswitch

[Simon Horman]

Hi Mat,

+ Jay Vosburgh <j.vosburgh@gmail.com>
  Andy Gospodarek <andy@greyhouse.net>
  "David S. Miller" <davem@davemloft.net>
  Eric Dumazet <edumazet@google.com>
  Jakub Kicinski <kuba@kernel.org>
  Paolo Abeni <pabeni@redhat.com>
  netdev@vger.kernel.org

  As per the output of
  ./scripts/get_maintainer.pl --git-min-percent 25 this.patch
  which is the preferred method to determine the CC list for
  Networking patches. LKML can, in general, be excluded.

> Commit d5410ac7b0ba ("net:bonding:support balance-alb interface with
> vlan to bridge") introduced a support for balance-alb mode for
> interfaces connected to the linux bridge by fixing missing matching of
> MAC entry in FDB. In our testing we discovered that it still does not
> work when the bond is connected to the OVS bridge as show in diagram
> below:
> 
> eth1(mac:eth1_mac)--bond0(balance-alb,mac:eth0_mac)--eth0(mac:eth0_mac)
>                       |
>                     bond0.150(mac:eth0_mac)
>                               |
>                     ovs_bridge(ip:bridge_ip,mac:eth0_mac)
> 
> This patch fixes it by checking not only if the device is a bridge but
> also if it is an openvswitch.
> 
> Signed-off-by: Mateusz Kowalski <mko@redhat.com>

Hi,

unfortunately this does not seem to apply to net-next.
Perhaps it needs to be rebased.

Also.

1. For Networking patches, please include the target tree, in this case
   net-next, as opposed to net, which is for fixes, in the subject.

	Subject: [PATCH net-next] ...

2. Perhaps 'bonding; ' is a more appropriate prefix.

	Subject: [PATCH net-next] bonding: ...

...

Re: [PATCH] net:bonding:support balance-alb with openvswitch

[Mat Kowalski]

Hi Simon,

Thanks a lot for the pointers, not much experienced with contributing 
here so I really appreciate. Just a question inline regarding the net vs 
net-next

On 28/07/2023 14:04, Simon Horman wrote:
> Hi Mat,
>
> + Jay Vosburgh <j.vosburgh@gmail.com>
>    Andy Gospodarek <andy@greyhouse.net>
>    "David S. Miller" <davem@davemloft.net>
>    Eric Dumazet <edumazet@google.com>
>    Jakub Kicinski <kuba@kernel.org>
>    Paolo Abeni <pabeni@redhat.com>
>    netdev@vger.kernel.org
>
>    As per the output of
>    ./scripts/get_maintainer.pl --git-min-percent 25 this.patch
>    which is the preferred method to determine the CC list for
>    Networking patches. LKML can, in general, be excluded.
>
>> Commit d5410ac7b0ba ("net:bonding:support balance-alb interface with
>> vlan to bridge") introduced a support for balance-alb mode for
>> interfaces connected to the linux bridge by fixing missing matching of
>> MAC entry in FDB. In our testing we discovered that it still does not
>> work when the bond is connected to the OVS bridge as show in diagram
>> below:
>>
>> eth1(mac:eth1_mac)--bond0(balance-alb,mac:eth0_mac)--eth0(mac:eth0_mac)
>>                        |
>>                      bond0.150(mac:eth0_mac)
>>                                |
>>                      ovs_bridge(ip:bridge_ip,mac:eth0_mac)
>>
>> This patch fixes it by checking not only if the device is a bridge but
>> also if it is an openvswitch.
>>
>> Signed-off-by: Mateusz Kowalski <mko@redhat.com>
> Hi,
>
> unfortunately this does not seem to apply to net-next.
> Perhaps it needs to be rebased.
>
> Also.
>
> 1. For Networking patches, please include the target tree, in this case
>     net-next, as opposed to net, which is for fixes, in the subject.
>
> 	Subject: [PATCH net-next] ...
It makes me wonder as in my view this is a fix for something that 
doesn't work today, not necessarily a new feature. Is net-next still a 
preferred target?
>
> 2. Perhaps 'bonding; ' is a more appropriate prefix.
>
> 	Subject: [PATCH net-next] bonding: ...
>
> ...
>

Re: [PATCH] net:bonding:support balance-alb with openvswitch

[Simon Horman]

On Fri, Jul 28, 2023 at 02:17:03PM +0200, Mat Kowalski wrote:
> Hi Simon,
> 
> Thanks a lot for the pointers, not much experienced with contributing here
> so I really appreciate. Just a question inline regarding the net vs net-next
> 
> On 28/07/2023 14:04, Simon Horman wrote:
> > Hi Mat,
> > 
> > + Jay Vosburgh <j.vosburgh@gmail.com>
> >    Andy Gospodarek <andy@greyhouse.net>
> >    "David S. Miller" <davem@davemloft.net>
> >    Eric Dumazet <edumazet@google.com>
> >    Jakub Kicinski <kuba@kernel.org>
> >    Paolo Abeni <pabeni@redhat.com>
> >    netdev@vger.kernel.org
> > 
> >    As per the output of
> >    ./scripts/get_maintainer.pl --git-min-percent 25 this.patch
> >    which is the preferred method to determine the CC list for
> >    Networking patches. LKML can, in general, be excluded.
> > 
> > > Commit d5410ac7b0ba ("net:bonding:support balance-alb interface with
> > > vlan to bridge") introduced a support for balance-alb mode for
> > > interfaces connected to the linux bridge by fixing missing matching of
> > > MAC entry in FDB. In our testing we discovered that it still does not
> > > work when the bond is connected to the OVS bridge as show in diagram
> > > below:
> > > 
> > > eth1(mac:eth1_mac)--bond0(balance-alb,mac:eth0_mac)--eth0(mac:eth0_mac)
> > >                        |
> > >                      bond0.150(mac:eth0_mac)
> > >                                |
> > >                      ovs_bridge(ip:bridge_ip,mac:eth0_mac)
> > > 
> > > This patch fixes it by checking not only if the device is a bridge but
> > > also if it is an openvswitch.
> > > 
> > > Signed-off-by: Mateusz Kowalski <mko@redhat.com>
> > Hi,
> > 
> > unfortunately this does not seem to apply to net-next.
> > Perhaps it needs to be rebased.
> > 
> > Also.
> > 
> > 1. For Networking patches, please include the target tree, in this case
> >     net-next, as opposed to net, which is for fixes, in the subject.
> > 
> > 	Subject: [PATCH net-next] ...
> It makes me wonder as in my view this is a fix for something that doesn't
> work today, not necessarily a new feature. Is net-next still a preferred
> target?

Hi Mat,

Certainly there is a discussion to be had on if this is a fix or a feature.

I would argue that it is a feature as it makes something new work
that did not work before. As opposed to fixing something that worked
incorrectly.

But there is certainly room for interpretation.

> > 
> > 2. Perhaps 'bonding; ' is a more appropriate prefix.
> > 
> > 	Subject: [PATCH net-next] bonding: ...
> > 
> > ...
> > 
> 
> 

Re: [PATCH] net:bonding:support balance-alb with openvswitch

[Mat Kowalski]

On 28/07/2023 20:36, Simon Horman wrote:
> On Fri, Jul 28, 2023 at 02:17:03PM +0200, Mat Kowalski wrote:
>> Hi Simon,
>>
>> Thanks a lot for the pointers, not much experienced with contributing here
>> so I really appreciate. Just a question inline regarding the net vs net-next
>>
>> On 28/07/2023 14:04, Simon Horman wrote:
>>> Hi Mat,
>>>
>>> + Jay Vosburgh <j.vosburgh@gmail.com>
>>>     Andy Gospodarek <andy@greyhouse.net>
>>>     "David S. Miller" <davem@davemloft.net>
>>>     Eric Dumazet <edumazet@google.com>
>>>     Jakub Kicinski <kuba@kernel.org>
>>>     Paolo Abeni <pabeni@redhat.com>
>>>     netdev@vger.kernel.org
>>>
>>>     As per the output of
>>>     ./scripts/get_maintainer.pl --git-min-percent 25 this.patch
>>>     which is the preferred method to determine the CC list for
>>>     Networking patches. LKML can, in general, be excluded.
>>>
>>>> Commit d5410ac7b0ba ("net:bonding:support balance-alb interface with
>>>> vlan to bridge") introduced a support for balance-alb mode for
>>>> interfaces connected to the linux bridge by fixing missing matching of
>>>> MAC entry in FDB. In our testing we discovered that it still does not
>>>> work when the bond is connected to the OVS bridge as show in diagram
>>>> below:
>>>>
>>>> eth1(mac:eth1_mac)--bond0(balance-alb,mac:eth0_mac)--eth0(mac:eth0_mac)
>>>>                         |
>>>>                       bond0.150(mac:eth0_mac)
>>>>                                 |
>>>>                       ovs_bridge(ip:bridge_ip,mac:eth0_mac)
>>>>
>>>> This patch fixes it by checking not only if the device is a bridge but
>>>> also if it is an openvswitch.
>>>>
>>>> Signed-off-by: Mateusz Kowalski <mko@redhat.com>
>>> Hi,
>>>
>>> unfortunately this does not seem to apply to net-next.
>>> Perhaps it needs to be rebased.
>>>
>>> Also.
>>>
>>> 1. For Networking patches, please include the target tree, in this case
>>>      net-next, as opposed to net, which is for fixes, in the subject.
>>>
>>> 	Subject: [PATCH net-next] ...
>> It makes me wonder as in my view this is a fix for something that doesn't
>> work today, not necessarily a new feature. Is net-next still a preferred
>> target?
> Hi Mat,
>
> Certainly there is a discussion to be had on if this is a fix or a feature.
>
> I would argue that it is a feature as it makes something new work
> that did not work before. As opposed to fixing something that worked
> incorrectly.
>
> But there is certainly room for interpretation.
>
Of course, I am not pushing any way as I am perfectly fine with getting 
it only into net-next. An updated patch has already been submitted with 
the tag in subject fixed. Thanks for your input !
>>> 2. Perhaps 'bonding; ' is a more appropriate prefix.
>>>
>>> 	Subject: [PATCH net-next] bonding: ...
>>>
>>> ...
>>>
>>