On Wed, Mar 23, 2022 at 10:12 PM Mike Pattrick <m...@redhat.com> wrote:
>
> On Thu, Mar 17, 2022 at 11:54 AM Christophe Fontaine
> <cfont...@redhat.com> wrote:
> >
> > This config param allows the delivery of broadcast and multicast packets
> > to the secondary interface of non-lacp bonds, identical to the same option
> > 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_slaves_active=false
> > (default unmodified behavior) 5 packets are received
> > - with other_config:all_slaves_active=true, all 10 packets are received.
> >
> > Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=1720935
> > Signed-off-by: Christophe Fontaine <cfont...@redhat.com>
> > ---
>
> Hello Chris,
>
> This feature seems like it would be useful, and looks correctly
> implemented. However, the nomenclature used throughout the code and
> documentation is member in place of slave.
>
Hi Mike,
Sure, I'll fix that in v2.
Thanks,
Christophe
>
> Cheers,
> M
>
> > NEWS | 1 +
> > ofproto/bond.c | 5 ++++-
> > ofproto/bond.h | 2 ++
> > vswitchd/bridge.c | 3 +++
> > vswitchd/vswitch.xml | 20 ++++++++++++++++++++
> > 5 files changed, 30 insertions(+), 1 deletion(-)
> >
> > diff --git a/NEWS b/NEWS
> > index df633e8e2..07f72fd5a 100644
> > --- a/NEWS
> > +++ b/NEWS
> > @@ -16,6 +16,7 @@ v2.17.0 - xx xxx xxxx
> > * Removed experimental tag for PMD Auto Load Balance.
> > * New configuration knob 'other_config:n-offload-threads' to change
> > the
> > number of HW offloading threads.
> > + * New configuration knob 'all_slaves_active' for non lacp bonds
> > - DPDK:
> > * EAL argument --socket-mem is no longer configured by default upon
> > start-up. If dpdk-socket-mem and dpdk-alloc-mem are not specified,
> > diff --git a/ofproto/bond.c b/ofproto/bond.c
> > index cdfdf0b9d..8bf166a41 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_slaves_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_slaves_active = s->all_slaves_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_slaves_active == false) {
> > goto out;
> > }
> > }
> > diff --git a/ofproto/bond.h b/ofproto/bond.h
> > index 1683ec878..4e8c1872a 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_slaves_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 5223aa897..53be7ddf6 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_slaves_active is disabled by default */
> > + s->all_slaves_active = smap_get_bool(&port->cfg->other_config,
> > + "all_slaves_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..d420397a3 100644
> > --- a/vswitchd/vswitch.xml
> > +++ b/vswitchd/vswitch.xml
> > @@ -2083,6 +2083,26 @@
> > <code>true</code>).
> > </column>
> >
> > + <column name="other_config" key="all_slaves_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 a broadcast packet 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
> > d...@openvswitch.org
> > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
> >
>
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
