| Message ID | 20240122171242.203457-1-naveen.yerramneni@nutanix.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [ovs-dev] Add "pkt_clone_type" option to LSP. | expand |
On Mon, Jan 22, 2024 at 12:13 PM Naveen Yerramneni <naveen.yerramneni@nutanix.com> wrote: > > Currently only valid value for this option is mc_unknown. > If set to mc_unknown, packets destined to the port gets cloned > to all unknown ports connected to the same Logical Switch. > > Usecase: > ========= > It is required to reduce packet loss when VM is being migrated to > different AZ via VXLAN tunnel. Port is configured in both AZs > on different logical switches which are sharing same IP subnet. > VTEP device is connected to the same logical switch on both AZs > and its port is set to unknown. > > Signed-off-by: Naveen Yerramneni <naveen.yerramneni@nutanix.com> Thanks for the patch. I applied this patch to the main with the below changes ----------------------------------------------- diff --git a/northd/northd.c b/northd/northd.c index 40676d4da6..b0f3d89346 100644 --- a/northd/northd.c +++ b/northd/northd.c @@ -1260,10 +1260,13 @@ ovn_port_find(const struct hmap *ports, const char *name) static bool lsp_is_clone_to_unknown(const struct nbrec_logical_switch_port *nbsp) { - const char *pkt_clone_type = smap_get( ->options, - "pkt_clone_type"); - if (pkt_clone_type && !strcasecmp(pkt_clone_type, "mc_unknown")) { - return true; + if (!nbsp->type[0]) { + /* Check this option only for VIF logical port. */ + const char *pkt_clone_type = smap_get( ->options, + "pkt_clone_type"); + if (pkt_clone_type && !strcasecmp(pkt_clone_type, "mc_unknown")) { + return true; + } } return false; } @@ -9451,21 +9454,20 @@ build_lswitch_ip_unicast_lookup(struct ovn_port *op, &op->nbsp->header_); } + bool lsp_clone_to_unknown = lsp_is_clone_to_unknown(op->nbsp); + for (size_t i = 0; i < op->nbsp->n_addresses; i++) { /* Addresses are owned by the logical port. * Ethernet address followed by zero or more IPv4 * or IPv6 addresses (or both). */ struct eth_addr mac; bool lsp_enabled = lsp_is_enabled(op->nbsp); - bool lsp_clone_to_unknown = lsp_is_clone_to_unknown(op->nbsp); - const char *action = lsp_enabled ? - ((lsp_clone_to_unknown && op->od->has_unknown - && !strcmp(op->nbsp->type, "")) ? - "clone {outport = %s; output; };" - "outport = \""MC_UNKNOWN "\"; output;" : - "outport = %s; output;") : - debug_drop_action(); - + const char *action = lsp_enabled + ? ((lsp_clone_to_unknown && op->od->has_unknown) + ? "clone {outport = %s; output; };" + "outport = \""MC_UNKNOWN "\"; output;" + : "outport = %s; output;") + : debug_drop_action(); if (ovs_scan(op->nbsp->addresses[i], ETH_ADDR_SCAN_FMT, ETH_ADDR_SCAN_ARGS(mac))) { diff --git a/northd/ovn-northd.8.xml b/northd/ovn-northd.8.xml index 9583abeffd..98799e91c1 100644 --- a/northd/ovn-northd.8.xml +++ b/northd/ovn-northd.8.xml @@ -2017,7 +2017,10 @@ output; One priority-50 flow that matches each known Ethernet address against <code>eth.dst</code>. Action of this flow outputs the packet to the single associated output port if it is enabled. <code>drop;</code> - action is applied if LSP is disabled. + action is applied if LSP is disabled. If the logical switch port + of type VIF has the option <code>options:pkt_clone_type</code> + is set to the value <code>mc_unknown</code>, then the packet is + also forwarded to the <code>MC_UNKNOWN</code> multicast group. </p> <p> diff --git a/NEWS b/NEWS index 7114b96d11..680ae49cbc 100644 --- a/NEWS +++ b/NEWS @@ -1,5 +1,8 @@ Post v24.03.0 ------------- + - Added a new logical switch port option "pkt_clone_type". + If the value is set to "mc_unknown", packets destined to the port gets + cloned to all unknown ports connected to the same Logical Switch. OVN v24.03.0 - xx xxx xxxx -------------------------- diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at index 6b2f6244e9..8fdb6e8cee 100644 --- a/tests/ovn-northd.at +++ b/tests/ovn-northd.at @@ -6850,7 +6850,7 @@ ovn-nbctl --wait=sb lsp-set-addresses S1-vm2 "unknown" ovn-sbctl dump-flows S1 > S1flows AT_CAPTURE_FILE([S1flows]) -AT_CHECK([grep -e "ls_in_l2_lkup.*S1-vm1" S1flows | sed 's/table=../table=??/'], [0], [dnl +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;) ]) @@ -6860,10 +6860,20 @@ ovn-nbctl --wait=sb set logical_switch_port S1-vm1 options:pkt_clone_type=mc_unk ovn-sbctl dump-flows S1 > S1flows AT_CAPTURE_FILE([S1flows]) -AT_CHECK([grep -e "ls_in_l2_lkup.*S1-vm1" S1flows | sed 's/table=../table=??/'], [0], [dnl +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=(clone {outport = "S1-vm1"; output; };outport = "_MC_unknown"; output;) ]) +#Set the pkt_clone_type option to an invalid value +ovn-nbctl --wait=sb set logical_switch_port S1-vm1 options:pkt_clone_type=foo + +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;) +]) + AT_CLEANUP ]) ----------------------------------------------- Thanks Numan > --- > northd/northd.c | 23 +++++++++++++++++++++-- > ovn-nb.xml | 6 ++++++ > tests/ovn-northd.at | 32 ++++++++++++++++++++++++++++++++ > 3 files changed, 59 insertions(+), 2 deletions(-) > > diff --git a/northd/northd.c b/northd/northd.c > index 952f8200d..98e97fd6b 100644 > --- a/northd/northd.c > +++ b/northd/northd.c > @@ -1786,6 +1786,17 @@ ovn_port_find(const struct hmap *ports, const char *name) > return ovn_port_find__(ports, name, false); > } > > +static bool > +lsp_is_clone_to_unknown(const struct nbrec_logical_switch_port *nbsp) > +{ > + const char *pkt_clone_type = smap_get( ->options, > + "pkt_clone_type"); > + if (pkt_clone_type && !strcasecmp(pkt_clone_type, "mc_unknown")) { > + return true; > + } > + return false; > +} > + > static struct ovn_port * > ovn_port_find_bound(const struct hmap *ports, const char *name) > { > @@ -10529,8 +10540,16 @@ build_lswitch_ip_unicast_lookup(struct ovn_port *op, > * or IPv6 addresses (or both). */ > struct eth_addr mac; > bool lsp_enabled = lsp_is_enabled(op->nbsp); > - const char *action = lsp_enabled ? "outport = %s; output;" : > - debug_drop_action(); > + bool lsp_clone_to_unknown = lsp_is_clone_to_unknown(op->nbsp); > + const char *action = lsp_enabled ? > + ((lsp_clone_to_unknown && op->od->has_unknown > + && !strcmp(op->nbsp->type, "")) ? > + "clone {outport = %s; output; };" > + "outport = \""MC_UNKNOWN "\"; output;" : > + "outport = %s; output;") : > + debug_drop_action(); > + > + > if (ovs_scan(op->nbsp->addresses[i], > ETH_ADDR_SCAN_FMT, ETH_ADDR_SCAN_ARGS(mac))) { > ds_clear(match); > diff --git a/ovn-nb.xml b/ovn-nb.xml > index 765ffcf2e..dc55c8c99 100644 > --- a/ovn-nb.xml > +++ b/ovn-nb.xml > @@ -1262,6 +1262,12 @@ > </p> > </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 > + unknown ports connected to the same Logical Switch. > + </column> > + > <group title="VIF Plugging Options"> > <column name="options" key="vif-plug-type"> > If set, OVN will attempt to perform plugging of this VIF. In order > diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at > index 9a0d418e4..0ca9f82bb 100644 > --- a/tests/ovn-northd.at > +++ b/tests/ovn-northd.at > @@ -6650,6 +6650,38 @@ wait_row_count Port_binding 1 logical-port=S1-vm2 requested_chassis=$ch2_uuid > AT_CLEANUP > ]) > > + > +OVN_FOR_EACH_NORTHD_NO_HV([ > +AT_SETUP([check options:pkt_clone_type 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-vm2 > +ovn-nbctl --wait=sb lsp-set-addresses S1-vm1 "50:54:00:00:00:01 192.168.0.1" > +ovn-nbctl --wait=sb lsp-set-addresses S1-vm2 "unknown" > + > +ovn-sbctl dump-flows S1 > S1flows > +AT_CAPTURE_FILE([S1flows]) > + > +AT_CHECK([grep -e "ls_in_l2_lkup.*S1-vm1" S1flows | sed 's/table=../table=??/'], [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 pkt_clone_type option and verify the flow > +ovn-nbctl --wait=sb set logical_switch_port S1-vm1 options:pkt_clone_type=mc_unknown > + > +ovn-sbctl dump-flows S1 > S1flows > +AT_CAPTURE_FILE([S1flows]) > + > +AT_CHECK([grep -e "ls_in_l2_lkup.*S1-vm1" S1flows | sed 's/table=../table=??/'], [0], [dnl > + table=??(ls_in_l2_lkup ), priority=50 , match=(eth.dst == 50:54:00:00:00:01), action=(clone {outport = "S1-vm1"; output; };outport = "_MC_unknown"; output;) > +]) > + > +AT_CLEANUP > +]) > + > + > + > # Duplicated datapaths shouldn't be created, but in case it is created because > # of bug or dirty data, it should be properly deleted instead of causing > # permanent failure in northd. > -- > 2.36.6 > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev >
diff --git a/northd/northd.c b/northd/northd.c index 952f8200d..98e97fd6b 100644 --- a/northd/northd.c +++ b/northd/northd.c @@ -1786,6 +1786,17 @@ ovn_port_find(const struct hmap *ports, const char *name) return ovn_port_find__(ports, name, false); } +static bool +lsp_is_clone_to_unknown(const struct nbrec_logical_switch_port *nbsp) +{ + const char *pkt_clone_type = smap_get( ->options, + "pkt_clone_type"); + if (pkt_clone_type && !strcasecmp(pkt_clone_type, "mc_unknown")) { + return true; + } + return false; +} + static struct ovn_port * ovn_port_find_bound(const struct hmap *ports, const char *name) { @@ -10529,8 +10540,16 @@ build_lswitch_ip_unicast_lookup(struct ovn_port *op, * or IPv6 addresses (or both). */ struct eth_addr mac; bool lsp_enabled = lsp_is_enabled(op->nbsp); - const char *action = lsp_enabled ? "outport = %s; output;" : - debug_drop_action(); + bool lsp_clone_to_unknown = lsp_is_clone_to_unknown(op->nbsp); + const char *action = lsp_enabled ? + ((lsp_clone_to_unknown && op->od->has_unknown + && !strcmp(op->nbsp->type, "")) ? + "clone {outport = %s; output; };" + "outport = \""MC_UNKNOWN "\"; output;" : + "outport = %s; output;") : + debug_drop_action(); + + if (ovs_scan(op->nbsp->addresses[i], ETH_ADDR_SCAN_FMT, ETH_ADDR_SCAN_ARGS(mac))) { ds_clear(match); diff --git a/ovn-nb.xml b/ovn-nb.xml index 765ffcf2e..dc55c8c99 100644 --- a/ovn-nb.xml +++ b/ovn-nb.xml @@ -1262,6 +1262,12 @@ </p> </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 + unknown ports connected to the same Logical Switch. + </column> + <group title="VIF Plugging Options"> <column name="options" key="vif-plug-type"> If set, OVN will attempt to perform plugging of this VIF. In order diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at index 9a0d418e4..0ca9f82bb 100644 --- a/tests/ovn-northd.at +++ b/tests/ovn-northd.at @@ -6650,6 +6650,38 @@ wait_row_count Port_binding 1 logical-port=S1-vm2 requested_chassis=$ch2_uuid AT_CLEANUP ]) + +OVN_FOR_EACH_NORTHD_NO_HV([ +AT_SETUP([check options:pkt_clone_type 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-vm2 +ovn-nbctl --wait=sb lsp-set-addresses S1-vm1 "50:54:00:00:00:01 192.168.0.1" +ovn-nbctl --wait=sb lsp-set-addresses S1-vm2 "unknown" + +ovn-sbctl dump-flows S1 > S1flows +AT_CAPTURE_FILE([S1flows]) + +AT_CHECK([grep -e "ls_in_l2_lkup.*S1-vm1" S1flows | sed 's/table=../table=??/'], [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 pkt_clone_type option and verify the flow +ovn-nbctl --wait=sb set logical_switch_port S1-vm1 options:pkt_clone_type=mc_unknown + +ovn-sbctl dump-flows S1 > S1flows +AT_CAPTURE_FILE([S1flows]) + +AT_CHECK([grep -e "ls_in_l2_lkup.*S1-vm1" S1flows | sed 's/table=../table=??/'], [0], [dnl + table=??(ls_in_l2_lkup ), priority=50 , match=(eth.dst == 50:54:00:00:00:01), action=(clone {outport = "S1-vm1"; output; };outport = "_MC_unknown"; output;) +]) + +AT_CLEANUP +]) + + + # Duplicated datapaths shouldn't be created, but in case it is created because # of bug or dirty data, it should be properly deleted instead of causing # permanent failure in northd.
Currently only valid value for this option is mc_unknown. If set to mc_unknown, packets destined to the port gets cloned to all unknown ports connected to the same Logical Switch. Usecase: ========= It is required to reduce packet loss when VM is being migrated to different AZ via VXLAN tunnel. Port is configured in both AZs on different logical switches which are sharing same IP subnet. VTEP device is connected to the same logical switch on both AZs and its port is set to unknown. Signed-off-by: Naveen Yerramneni <naveen.yerramneni@nutanix.com> --- northd/northd.c | 23 +++++++++++++++++++++-- ovn-nb.xml | 6 ++++++ tests/ovn-northd.at | 32 ++++++++++++++++++++++++++++++++ 3 files changed, 59 insertions(+), 2 deletions(-)