[ovs-dev] [PATCH v2 ovn] Don't suppress localport traffic directed to external port

Ihar Hrachyshka ihrachys at redhat.com
Wed Jul 7 15:03:12 UTC 2021


Recently, we stopped leaking localport traffic through localnet ports
into fabric to avoid unnecessary flipping between chassis hosting the
same localport.

Despite the type name, in some scenarios localports are supposed to
talk outside the hosting chassis. Specifically, in OpenStack [1]
metadata service for SR-IOV ports is implemented as a localport hosted
on another chassis that is exposed to the chassis owning the SR-IOV
port through an "external" port. In this case, "leaking" localport
traffic into fabric is desirable.

This patch inserts a higher priority flow per external port on the
same datapath that avoids dropping localport traffic.

Fixes: 96959e56d634 ("physical: do not forward traffic from localport
to a localnet one")

[1] https://docs.openstack.org/neutron/latest/admin/ovn/sriov.html

Signed-off-by: Ihar Hrachyshka <ihrachys at redhat.com>

--

v1: initial version
v2: fixed code for unbound external ports
v2: rebased
---
 controller/physical.c | 50 +++++++++++++++++++++++++++++++++++++++
 tests/ovn.at          | 54 +++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 104 insertions(+)

diff --git a/controller/physical.c b/controller/physical.c
index 17ca5afbb..9235c48f4 100644
--- a/controller/physical.c
+++ b/controller/physical.c
@@ -920,6 +920,7 @@ get_binding_peer(struct ovsdb_idl_index *sbrec_port_binding_by_name,
 
 static void
 consider_port_binding(struct ovsdb_idl_index *sbrec_port_binding_by_name,
+                      const struct sbrec_port_binding_table *pb_table,
                       enum mf_field_id mff_ovn_geneve,
                       const struct simap *ct_zones,
                       const struct sset *active_tunnels,
@@ -1281,6 +1282,52 @@ consider_port_binding(struct ovsdb_idl_index *sbrec_port_binding_by_name,
             ofctrl_add_flow(flow_table, OFTABLE_CHECK_LOOPBACK, 160,
                             binding->header_.uuid.parts[0], &match,
                             ofpacts_p, &binding->header_.uuid);
+
+            /* Localport traffic directed to external is *not* local. */
+            const struct sbrec_port_binding *peer;
+            SBREC_PORT_BINDING_TABLE_FOR_EACH (peer, pb_table) {
+                if (strcmp(peer->type, "external")) {
+                    continue;
+                }
+                if (!peer->chassis) {
+                    continue;
+                }
+                if (peer->datapath->tunnel_key != dp_key) {
+                    continue;
+                }
+                if (strcmp(peer->chassis->name, chassis->name)) {
+                    continue;
+                }
+
+                ofpbuf_clear(ofpacts_p);
+                for (int i = 0; i < MFF_N_LOG_REGS; i++) {
+                    put_load(0, MFF_REG0 + i, 0, 32, ofpacts_p);
+                }
+                put_resubmit(OFTABLE_LOG_EGRESS_PIPELINE, ofpacts_p);
+
+                for (int i = 0; i < peer->n_mac; i++) {
+                    char *err_str;
+                    struct eth_addr peer_mac;
+                    if ((err_str = str_to_mac(peer->mac[i], &peer_mac))) {
+                        VLOG_WARN("Parsing MAC failed for external port: %s, "
+                                "with error: %s", peer->logical_port, err_str);
+                        free(err_str);
+                        continue;
+                    }
+
+                    match_init_catchall(&match);
+                    match_set_metadata(&match, htonll(dp_key));
+                    match_set_reg(&match, MFF_LOG_OUTPORT - MFF_REG0,
+                                  port_key);
+                    match_set_reg_masked(&match, MFF_LOG_FLAGS - MFF_REG0,
+                                         MLF_LOCALPORT, MLF_LOCALPORT);
+                    match_set_dl_dst(&match, peer_mac);
+
+                    ofctrl_add_flow(flow_table, OFTABLE_CHECK_LOOPBACK, 170,
+                                    binding->header_.uuid.parts[0], &match,
+                                    ofpacts_p, &binding->header_.uuid);
+                }
+            }
         }
 
     } else if (!tun && !is_ha_remote) {
@@ -1504,6 +1551,7 @@ physical_handle_port_binding_changes(struct physical_ctx *p_ctx,
                 ofctrl_remove_flows(flow_table, &binding->header_.uuid);
             }
             consider_port_binding(p_ctx->sbrec_port_binding_by_name,
+                                  p_ctx->port_binding_table,
                                   p_ctx->mff_ovn_geneve, p_ctx->ct_zones,
                                   p_ctx->active_tunnels,
                                   p_ctx->local_datapaths,
@@ -1684,6 +1732,7 @@ physical_run(struct physical_ctx *p_ctx,
     const struct sbrec_port_binding *binding;
     SBREC_PORT_BINDING_TABLE_FOR_EACH (binding, p_ctx->port_binding_table) {
         consider_port_binding(p_ctx->sbrec_port_binding_by_name,
+                              p_ctx->port_binding_table,
                               p_ctx->mff_ovn_geneve, p_ctx->ct_zones,
                               p_ctx->active_tunnels, p_ctx->local_datapaths,
                               binding, p_ctx->chassis,
@@ -1932,6 +1981,7 @@ physical_handle_ovs_iface_changes(struct physical_ctx *p_ctx,
 
             simap_put(&localvif_to_ofport, iface_id, ofport);
             consider_port_binding(p_ctx->sbrec_port_binding_by_name,
+                                  p_ctx->port_binding_table,
                                   p_ctx->mff_ovn_geneve, p_ctx->ct_zones,
                                   p_ctx->active_tunnels,
                                   p_ctx->local_datapaths,
diff --git a/tests/ovn.at b/tests/ovn.at
index b0f61f9ce..a65ec4214 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -12143,6 +12143,60 @@ OVN_CLEANUP([hv1])
 AT_CLEANUP
 ])
 
+OVN_FOR_EACH_NORTHD([
+AT_SETUP([localport doesn't suppress gARP directed to external port])
+
+send_garp() {
+    local inport=$1 eth_src=$2 eth_dst=$3 spa=$4 tpa=$5
+    local request=${eth_dst}${eth_src}08060001080006040001${eth_src}${spa}${eth_dst}${tpa}
+    ovs-appctl netdev-dummy/receive $inport $request
+}
+
+ovn_start
+net_add n1
+
+check ovs-vsctl add-br br-phys
+check ovs-vsctl set open . external-ids:ovn-bridge-mappings=phys:br-phys
+ovn_attach n1 br-phys 192.168.0.1
+
+check ovn-nbctl ls-add ls
+
+# create topology to allow to talk from localport through localnet to external
+check ovs-vsctl add-port br-int lp -- set Interface lp external-ids:iface-id=lp
+check ovn-nbctl lsp-add ls lp
+check ovn-nbctl lsp-set-addresses lp "00:00:00:00:00:01 10.0.0.1"
+check ovn-nbctl lsp-set-type lp localport
+
+check ovn-nbctl lsp-add ls ln
+check ovn-nbctl lsp-set-addresses ln unknown
+check ovn-nbctl lsp-set-type ln localnet
+check ovn-nbctl lsp-set-options ln network_name=phys
+
+check ovn-nbctl --wait=sb ha-chassis-group-add hagrp
+check ovn-nbctl --wait=sb ha-chassis-group-add-chassis hagrp main 10
+
+check ovn-nbctl lsp-add ls lext
+check ovn-nbctl lsp-set-addresses lext "00:00:00:00:00:02 10.0.0.2"
+check ovn-nbctl lsp-set-type lext external
+
+# own external port
+hagrp_uuid=`ovn-nbctl --bare --columns _uuid find ha_chassis_group name=hagrp`
+check ovn-nbctl set logical_switch_port lext ha_chassis_group=$hagrp_uuid
+
+check ovn-nbctl --wait=hv sync
+
+spa=$(ip_to_hex 10 0 0 1)
+tpa=$(ip_to_hex 10 0 0 2)
+send_garp lp 000000000001 000000000002 $spa $tpa
+
+dnl external traffic from localport should be sent to localnet
+AT_CHECK([tcpdump -r main/br-phys_n1-tx.pcap arp[[24:4]]=0x0a000002 | wc -l],[0],[dnl
+1
+],[ignore])
+
+AT_CLEANUP
+])
+
 OVN_FOR_EACH_NORTHD([
 AT_SETUP([1 LR with HA distributed router gateway port])
 ovn_start
-- 
2.31.1



More information about the dev mailing list