On Tue, Apr 12, 2022 at 7:49 AM Christophe Fontaine <[email protected]> wrote:
>
> 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.
>
> Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=1720935
> Signed-off-by: Christophe Fontaine <[email protected]>

Thanks for changing the nomenclature!

Acked-by: Mike Pattrick <[email protected]>

> ---
>  NEWS                 |   2 +
>  ofproto/bond.c       |   5 +-
>  ofproto/bond.h       |   2 +
>  tests/automake.mk    |   3 +-
>  tests/bond.at        | 125 +++++++++++++++++++++++++++++++++++++++++++
>  tests/testsuite.at   |   1 +
>  vswitchd/bridge.c    |   3 ++
>  vswitchd/vswitch.xml |  20 +++++++
>  8 files changed, 159 insertions(+), 2 deletions(-)
>  create mode 100644 tests/bond.at
>
> 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/tests/automake.mk b/tests/automake.mk
> index 8a9151f81..5c6400c0d 100644
> --- a/tests/automake.mk
> +++ b/tests/automake.mk
> @@ -109,7 +109,8 @@ TESTSUITE_AT = \
>         tests/mcast-snooping.at \
>         tests/packet-type-aware.at \
>         tests/nsh.at \
> -       tests/drop-stats.at
> +       tests/drop-stats.at \
> +       tests/bond.at
>
>  EXTRA_DIST += $(FUZZ_REGRESSION_TESTS)
>  FUZZ_REGRESSION_TESTS = \
> diff --git a/tests/bond.at b/tests/bond.at
> new file mode 100644
> index 000000000..be0be8433
> --- /dev/null
> +++ b/tests/bond.at
> @@ -0,0 +1,125 @@
> +AT_BANNER([bond])
> +
> +# Strips out Recirculation ID information since it may change over time.
> +m4_define([STRIP_RECIRC_ID], [[sed '
> +    s/Recirc-ID.*$/<del>/
> +' ]])
> +
> +# Strips out rebalance time information since it changes over time.
> +m4_define([STRIP_REBALANCE_TIME], [[sed '
> +    s/next rebalance.*$/<next rebalance del>/
> +' ]])
> +
> +# Strips out active member mac address since it may change over time.
> +m4_define([STRIP_ACTIVE_MEMBER_MAC], [[sed '
> +    s/active member mac.*$/<active member mac del>/
> +' ]])
> +
> +AT_SETUP([bond - discard duplicated frames])
> +# With an active/active non-lacp bond, the default behaviour
> +# is to discard multicast frames on the secondary interface
> +OVS_VSWITCHD_START([dnl
> +        add-bond br0 bond0 p1 p2 --\
> +        set Port bond0 bond-mode=balance-slb --\
> +        set Interface p1 type=dummy ofport_request=1 --\
> +        set Interface p2 type=dummy ofport_request=2 ])
> +
> +ovs-appctl bond/set-active-member bond0 p1
> +ovs-ofctl add-flow br0 actions=NORMAL
> +
> +AT_CHECK([ovs-appctl bond/show bond0 | STRIP_REBALANCE_TIME | 
> STRIP_ACTIVE_MEMBER_MAC], [0], [dnl
> +---- bond0 ----
> +bond_mode: balance-slb
> +bond may use recirculation: no, Recirc-ID : -1
> +bond-hash-basis: 0
> +lb_output action: disabled, bond-id: -1
> +updelay: 0 ms
> +downdelay: 0 ms
> +<next rebalance del>
> +lacp_status: off
> +lacp_fallback_ab: false
> +active-backup primary: <none>
> +<active member mac del>
> +
> +member p1: enabled
> +  active member
> +  may_enable: true
> +
> +member p2: enabled
> +  may_enable: true
> +
> +])
> +
> +AT_CHECK([ovs-appctl ofproto/trace br0 in_port=1,dl_dst=ff:ff:ff:ff:ff:ff], 
> [0], [dnl
> +Flow: 
> in_port=1,vlan_tci=0x0000,dl_src=00:00:00:00:00:00,dl_dst=ff:ff:ff:ff:ff:ff,dl_type=0x0000
> +
> +bridge("br0")
> +-------------
> + 0. priority 32768
> +    NORMAL
> +     -> no learned MAC for destination, flooding
> +
> +Final flow: unchanged
> +Megaflow: 
> recirc_id=0,eth,in_port=1,dl_src=00:00:00:00:00:00,dl_dst=ff:ff:ff:ff:ff:ff,dl_type=0x0000
> +Datapath actions: 100
> +])
> +
> +AT_CHECK([ovs-appctl ofproto/trace br0 in_port=2,dl_dst=ff:ff:ff:ff:ff:ff], 
> [0], [stdout])
> +AT_CHECK([cat stdout], [0], [dnl
> +Flow: 
> in_port=2,vlan_tci=0x0000,dl_src=00:00:00:00:00:00,dl_dst=ff:ff:ff:ff:ff:ff,dl_type=0x0000
> +
> +bridge("br0")
> +-------------
> + 0. priority 32768
> +    NORMAL
> +     -> bonding refused admissibility, dropping
> +
> +Final flow: unchanged
> +Megaflow: 
> recirc_id=0,eth,in_port=2,dl_src=00:00:00:00:00:00,dl_dst=ff:ff:ff:ff:ff:ff,dl_type=0x0000
> +Datapath actions: drop
> +])
> +OVS_VSWITCHD_STOP
> +AT_CLEANUP
> +
> +AT_SETUP([bond - allow duplicated frames])
> +# In some specific deployments, we may want to allow these multicast
> +# frames on the second interface
> +OVS_VSWITCHD_START([dnl
> +        add-bond br0 bond0 p1 p2 --\
> +        set Port bond0 bond-mode=balance-slb \
> +                       other_config:all_members_active=true --\
> +        set Interface p1 type=dummy ofport_request=1 --\
> +        set Interface p2 type=dummy ofport_request=2 ])
> +
> +ovs-appctl bond/set-active-member bond0 p1
> +ovs-ofctl add-flow br0 actions=NORMAL
> +AT_CHECK([ovs-appctl ofproto/trace br0 in_port=1,dl_dst=ff:ff:ff:ff:ff:ff], 
> [0], [dnl
> +Flow: 
> in_port=1,vlan_tci=0x0000,dl_src=00:00:00:00:00:00,dl_dst=ff:ff:ff:ff:ff:ff,dl_type=0x0000
> +
> +bridge("br0")
> +-------------
> + 0. priority 32768
> +    NORMAL
> +     -> no learned MAC for destination, flooding
> +
> +Final flow: unchanged
> +Megaflow: 
> recirc_id=0,eth,in_port=1,dl_src=00:00:00:00:00:00,dl_dst=ff:ff:ff:ff:ff:ff,dl_type=0x0000
> +Datapath actions: 100
> +])
> +
> +AT_CHECK([ovs-appctl ofproto/trace br0 in_port=2,dl_dst=ff:ff:ff:ff:ff:ff], 
> [0], [dnl
> +Flow: 
> in_port=2,vlan_tci=0x0000,dl_src=00:00:00:00:00:00,dl_dst=ff:ff:ff:ff:ff:ff,dl_type=0x0000
> +
> +bridge("br0")
> +-------------
> + 0. priority 32768
> +    NORMAL
> +     -> no learned MAC for destination, flooding
> +
> +Final flow: unchanged
> +Megaflow: 
> recirc_id=0,eth,in_port=2,dl_src=00:00:00:00:00:00,dl_dst=ff:ff:ff:ff:ff:ff,dl_type=0x0000
> +Datapath actions: 100
> +])
> +
> +OVS_VSWITCHD_STOP
> +AT_CLEANUP
> diff --git a/tests/testsuite.at b/tests/testsuite.at
> index 58adfa09c..b91edc5f2 100644
> --- a/tests/testsuite.at
> +++ b/tests/testsuite.at
> @@ -78,3 +78,4 @@ m4_include([tests/mcast-snooping.at])
>  m4_include([tests/packet-type-aware.at])
>  m4_include([tests/nsh.at])
>  m4_include([tests/drop-stats.at])
> +m4_include([tests/bond.at])
> 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
> --
> 2.35.1
>

_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to