On Thu, May 9, 2024 at 2:17 PM Numan Siddique <num...@ovn.org> wrote:
>
> 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.
Thanks for the patch. I applied it to the main with the below changes.
---------------------------------------------------------------------------------------------
diff --git a/AUTHORS.rst b/AUTHORS.rst
index b4e0a2695d..2f5713d8b6 100644
--- a/AUTHORS.rst
+++ b/AUTHORS.rst
@@ -366,6 +366,7 @@ Shan Wei davids...@tencent.com
Sharon Krendel thekaf...@gmail.com
Shashank Ram r...@vmware.com
Shashwat Srivastava shashwat.srivast...@tcs.com
+Shibir-basak shibir.ba...@nutanix.com
Shih-Hao Li shi...@vmware.com
Shu Shen shu.s...@radisys.com
Simon Horman ho...@ovn.org
diff --git a/NEWS b/NEWS
index 3b5e93dc9a..9883fd2ae9 100644
--- a/NEWS
+++ b/NEWS
@@ -17,6 +17,9 @@ Post v24.03.0
external-ids, the option is no longer needed as it became effectively
"true" for all scenarios.
- Added DHCPv4 relay support.
+ - A new LSP option "force_fdb_lookup" has been added to ensure the additional
+ MAC addresses configured on the LSP with "unknown", are learnt via the
+ OVN native FDB.
OVN v24.03.0 - 01 Mar 2024
--------------------------
diff --git a/northd/ovn-northd.8.xml b/northd/ovn-northd.8.xml
index 2e588e28f6..83cdbdd9f0 100644
--- a/northd/ovn-northd.8.xml
+++ b/northd/ovn-northd.8.xml
@@ -2038,6 +2038,12 @@ output;
also forwarded to the <code>MC_UNKNOWN</code> multicast group.
</p>
+ <p>
+ The above flow is not added if the logical switch port is of type
+ VIF, has <code>unknown</code> as one of its address and has the
+ option <code>options:force_fdb_lookup</code> set to true.
+ </p>
+
<p>
For the Ethernet address on a logical switch port of type
<code>router</code>, when that logical switch port's
---------------------------------------------------------------------------------------------
Numan
>
> 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
