diff mbox series

[ovs-dev,v2] ofproto/bond: Add knob "all_members_active"

Message ID 20220411135206.9566-2-cfontain@redhat.com
State Superseded
Headers show
Series [ovs-dev,v2] ofproto/bond: Add knob "all_members_active" | expand

Checks

Context Check Description
ovsrobot/apply-robot success apply and check: success
ovsrobot/intel-ovs-compilation success test: success
ovsrobot/github-robot-_Build_and_Test success github build: passed

Commit Message

Christophe Fontaine April 11, 2022, 1:52 p.m. UTC
This config param allows the delivery of broadcast and multicast packets
to the secondary interface of non-lacp bonds, equivalent to the option
"all_slaves_active" for kernel bonds.

Tested with an ovs-dpdk balance-slb bond with 2 virtual functions,
and a kernel bond with LACP on 2 other virtual functions.

Scapy sending 10 broadcast packets from 10 different mac addresses:
- with ovs-vsctl set port dpdkbond other_config:all_members_active=false
(default unmodified behavior) 5 packets are received
- with other_config:all_members_active=true, all 10 packets are received.

Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=1720935
Signed-off-by: Christophe Fontaine <cfontain@redhat.com>
---
 NEWS                 |  2 ++
 ofproto/bond.c       |  5 ++++-
 ofproto/bond.h       |  2 ++
 vswitchd/bridge.c    |  3 +++
 vswitchd/vswitch.xml | 20 ++++++++++++++++++++
 5 files changed, 31 insertions(+), 1 deletion(-)
diff mbox series

Patch

diff --git a/NEWS b/NEWS
index 5074b97aa..38777eea8 100644
--- a/NEWS
+++ b/NEWS
@@ -14,6 +14,8 @@  Post-v2.17.0
    - OVSDB:
      * 'relay' service model now supports transaction history, i.e. honors the
        'last-txn-id' field in 'monitor_cond_since' requests from clients.
+   - Userspace datapath:
+     * New configuration knob 'all_members_active' for non lacp bonds
 
 
 v2.17.0 - 17 Feb 2022
diff --git a/ofproto/bond.c b/ofproto/bond.c
index 845f69e21..aee420890 100644
--- a/ofproto/bond.c
+++ b/ofproto/bond.c
@@ -145,6 +145,7 @@  struct bond {
     struct eth_addr active_member_mac; /* MAC address of the active member. */
     /* Legacy compatibility. */
     bool lacp_fallback_ab; /* Fallback to active-backup on LACP failure. */
+    bool all_members_active;
 
     struct ovs_refcount ref_cnt;
 };
@@ -448,6 +449,7 @@  bond_reconfigure(struct bond *bond, const struct bond_settings *s)
 
     bond->updelay = s->up_delay;
     bond->downdelay = s->down_delay;
+    bond->all_members_active = s->all_members_active;
 
     if (bond->lacp_fallback_ab != s->lacp_fallback_ab_cfg) {
         bond->lacp_fallback_ab = s->lacp_fallback_ab_cfg;
@@ -893,7 +895,8 @@  bond_check_admissibility(struct bond *bond, const void *member_,
 
     /* Drop all multicast packets on inactive members. */
     if (eth_addr_is_multicast(eth_dst)) {
-        if (bond->active_member != member) {
+        if (bond->active_member != member &&
+            bond->all_members_active == false) {
             goto out;
         }
     }
diff --git a/ofproto/bond.h b/ofproto/bond.h
index 1683ec878..ce4a25e73 100644
--- a/ofproto/bond.h
+++ b/ofproto/bond.h
@@ -62,6 +62,8 @@  struct bond_settings {
                                    ovs run. */
     bool use_lb_output_action;  /* Use lb_output action. Only applicable for
                                    bond mode BALANCE TCP. */
+    bool all_members_active;    /* For non LACP bond, also accept multicast
+                                   packets on secondary interface. */
 };
 
 /* Program startup. */
diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
index e328d8ead..6034bb032 100644
--- a/vswitchd/bridge.c
+++ b/vswitchd/bridge.c
@@ -4615,6 +4615,9 @@  port_configure_bond(struct port *port, struct bond_settings *s)
     s->use_lb_output_action = (s->balance == BM_TCP)
                               && smap_get_bool(&port->cfg->other_config,
                                                "lb-output-action", false);
+    /* all_members_active is disabled by default */
+    s->all_members_active = smap_get_bool(&port->cfg->other_config,
+                                       "all_members_active", false);
 }
 
 /* Returns true if 'port' is synthetic, that is, if we constructed it locally
diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml
index 0c6632617..fe6d6f3b8 100644
--- a/vswitchd/vswitch.xml
+++ b/vswitchd/vswitch.xml
@@ -2083,6 +2083,26 @@ 
         <code>true</code>).
       </column>
 
+      <column name="other_config" key="all_members_active"
+              type='{"type": "boolean"}'>
+        <p>
+          Enable/Disable delivery of broadcast/multicast packets on secondary
+          interface of a bond. Relevant only when <ref column="lacp"/> is
+          <code>off</code>.
+        </p>
+
+        <p>
+          This parameter is identical to <code>all_slaves_active</code> for
+          kernel bonds. This is useful when we need to share 2 physical
+          functions between an ovs bond and a linux bond, as 2 LACP sessions
+          can't be negotiated over the same physical link.
+          LACP session will be managed by the kernel bond, while an
+          active-active bond is used for ovs. In that particular configuration,
+          the physical switch will send a unique copy of broadcast packets over
+          the 2 physical links, eventually to the secondary link of the bond.
+        </p>
+      </column>
+
       <group title="Link Failure Detection">
         <p>
           An important part of link bonding is detecting that links are down so