On Thu, May 9, 2024 at 5:54 AM Shibir Basak <shibir.ba...@nutanix.com> wrote:
>
> Hi team,
>
> Could someone please review these changes?

Sorry for the delay.  I'll take a look and come back.

Numan

>
> Thanks,
> Shibir Basak
>
> > On 24-Apr-2024, at 12:15 PM, Shibir Basak <shibir.ba...@nutanix.com> wrote:
> >
> > This option is applicable only if the lsp is of default 'type'
> > i.e. type=empty_string (which is a VM (or VIF) interface) and the
> > lsp also has 'unknown' addresses configured.
> > If lsp option 'force_fdb_lookup' is set to true, mac addresses
> > of the lsp (if configured) are not installed in the l2 lookup
> > table of the Logical Switch pipeline. However, MAC addresses
> > are learnt using the FDB Table.
> >
> > Usecase:
> > =========
> > This option is required to reduce packet loss when VM is being
> > migrated across AZs (via VXLAN tunnel). lsp is configured in both
> > AZs on different logical switches which are sharing the same IP
> > subnet. If the port has unknown address set along with MAC, IP
> > then, any packet destined to VM's MAC on destination AZ will get
> > forwarded locally, which may lead to packet loss during VM migration.
> >
> > This option is useful to reduce packet loss during VM migration
> > by forcing the logical switch to learn MAC using FDB.
> >
> > The other way to address this issue is to use pkt_clone_type
> > option but this causes too much of packet duplication when there
> > are multiple ports with unknown address in the logical switch.
> >
> > Signed-off-by: shibir-basak <shibir.ba...@nutanix.com>
> > ---
> > northd/northd.c     | 16 ++++++++++++++++
> > ovn-nb.xml          | 11 +++++++++++
> > tests/ovn-northd.at | 38 ++++++++++++++++++++++++++++++++++++++
> > 3 files changed, 65 insertions(+)
> >
> > diff --git a/northd/northd.c b/northd/northd.c
> > index 37f443e70..c29396716 100644
> > --- a/northd/northd.c
> > +++ b/northd/northd.c
> > @@ -1380,6 +1380,16 @@ lrport_is_enabled(const struct 
> > nbrec_logical_router_port *lrport)
> >     return !lrport->enabled || *lrport->enabled;
> > }
> >
> > +static bool
> > +lsp_force_fdb_lookup(const struct ovn_port *op)
> > +{
> > +    /* To enable FDB Table lookup on a logical switch port, it has to be
> > +     * of 'type' empty_string and "addresses" must have "unknown".
> > +     */
> > +    return !op->nbsp->type[0] && op->has_unknown &&
> > +        smap_get_bool(&op->nbsp->options, "force_fdb_lookup", false);
> > +}
> > +
> > static struct ovn_port *
> > ovn_port_get_peer(const struct hmap *lr_ports, struct ovn_port *op)
> > {
> > @@ -9479,6 +9489,12 @@ build_lswitch_ip_unicast_lookup(struct ovn_port *op,
> >
> >         if (ovs_scan(op->nbsp->addresses[i],
> >                     ETH_ADDR_SCAN_FMT, ETH_ADDR_SCAN_ARGS(mac))) {
> > +            /* Skip adding flows related to the MAC address
> > +             * as force FDB Lookup is enabled on the lsp.
> > +             */
> > +            if (lsp_force_fdb_lookup(op)) {
> > +                continue;
> > +            }
> >             ds_clear(match);
> >             ds_put_format(match, "eth.dst == "ETH_ADDR_FMT,
> >                           ETH_ADDR_ARGS(mac));
> > diff --git a/ovn-nb.xml b/ovn-nb.xml
> > index b652046a7..4724d0537 100644
> > --- a/ovn-nb.xml
> > +++ b/ovn-nb.xml
> > @@ -1262,6 +1262,17 @@
> >           </p>
> >         </column>
> >
> > +        <column name="options" key="force_fdb_lookup"
> > +                type='{"type": "boolean"}'>
> > +          This option is supported only if the Logical Switch Port is of
> > +          default <ref column="type"/> (i.e. type set to empty_string) and 
> > also
> > +          <ref column="addresses"/> column contains <code>unknown</code>.
> > +          If set to <code>true</code>, MAC addresses (if configured)
> > +          are not installed in the l2 lookup table but the MAC addresses 
> > are
> > +          learnt and stored in the FDB table.
> > +          The default value is <code>false</code>.
> > +        </column>
> > +
> >         <column name="options" key="pkt_clone_type"
> >                 type='{"type": "string", "enum": ["set", ["mc_unknown"]]}'>
> >           If set to mc_unknown, packets going to this VIF get cloned to all
> > diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
> > index be006fb32..82e7a6574 100644
> > --- a/tests/ovn-northd.at
> > +++ b/tests/ovn-northd.at
> > @@ -6886,6 +6886,44 @@ AT_CLEANUP
> > ])
> >
> >
> > +OVN_FOR_EACH_NORTHD_NO_HV([
> > +AT_SETUP([check options: force_fdb_lookup for LSP])
> > +ovn_start NORTHD_TYPE
> > +ovn-nbctl ls-add S1
> > +ovn-nbctl --wait=sb lsp-add S1 S1-vm1
> > +ovn-nbctl --wait=sb lsp-add S1 S1-localnet
> > +ovn-nbctl --wait=sb lsp-set-addresses S1-vm1 "50:54:00:00:00:01 
> > 192.168.0.1" unknown
> > +ovn-nbctl --wait=sb lsp-set-type S1-localnet localnet
> > +
> > +#Verify the flows before setting force_fdb_lookup option
> > +ovn-sbctl dump-flows S1 > S1flows
> > +AT_CAPTURE_FILE([S1flows])
> > +
> > +AT_CHECK([grep -e "ls_in_l2_lkup.*S1-vm1" S1flows | ovn_strip_lflows], 
> > [0], [dnl
> > +  table=??(ls_in_l2_lkup      ), priority=50   , match=(eth.dst == 
> > 50:54:00:00:00:01), action=(outport = "S1-vm1"; output;)
> > +])
> > +
> > +#Set the force_fdb_lookup option and verify the flows
> > +ovn-nbctl --wait=sb set logical_switch_port S1-vm1 
> > options:force_fdb_lookup=true
> > +ovn-nbctl --wait=sb set logical_switch_port S1-localnet 
> > options:force_fdb_lookup=true
> > +
> > +ovn-sbctl dump-flows S1 > S1flows
> > +AT_CAPTURE_FILE([S1flows])
> > +
> > +#Verify the flows for default port type (VM port)
> > +AT_CHECK([grep -e "ls_in_l2_lkup.*S1-vm1" S1flows | grep -e 
> > "match=(eth.dst == 50:54:00:00:00:01)"], [1], [])
> > +AT_CHECK([grep -e "ls_in_.*_fdb.*S1-vm1" S1flows | ovn_strip_lflows], [0], 
> > [dnl
> > +  table=??(ls_in_lookup_fdb   ), priority=100  , match=(inport == 
> > "S1-vm1"), action=(reg0[[11]] = lookup_fdb(inport, eth.src); next;)
> > +  table=??(ls_in_put_fdb      ), priority=100  , match=(inport == "S1-vm1" 
> > && reg0[[11]] == 0), action=(put_fdb(inport, eth.src); next;)
> > +])
> > +
> > +#Verify the flows for a non-default port type (localnet port)
> > +AT_CHECK([grep -e "ls_in_.*_fdb.*S1-localnet" S1flows], [1], [])
> > +
> > +AT_CLEANUP
> > +])
> > +
> > +
> > OVN_FOR_EACH_NORTHD_NO_HV([
> > AT_SETUP([check options:pkt_clone_type for LSP])
> > ovn_start NORTHD_TYPE
> > --
> > 2.22.3
> >
>
> _______________________________________________
> 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

Reply via email to