On Thu, May 18, 2023 at 10:11 PM Mark Michelson <mmich...@redhat.com
<mailto:mmich...@redhat.com>> wrote:
Current code always skips conntrack for traffic that ingresses or
egresses on a localnet port. However, this makes it impossible for
traffic to be load-balanced when it arrives on a localnet port.
This patch allows for traffic to be load balanced on localnet ports by
making two changes:
* Localnet ports now have a conntrack zone assigned.
* When a load balancer is configured on a logical switch containing a
localnet port, then conntrack is no longer skipped for traffic
involving the localnet port.
Co-authored by: Dumitru Ceara <dce...@redhat.com
<mailto:dce...@redhat.com>>
Signed-off-by: Dumitru Ceara <dce...@redhat.com
<mailto:dce...@redhat.com>>
Signed-off-by: Mark Michelson <mmich...@redhat.com
<mailto:mmich...@redhat.com>>
---
controller/ovn-controller.c | 16 ++++---
northd/northd.c | 18 ++++++--
tests/ovn-northd.at <http://ovn-northd.at> | 60
++++++++++++++++++++++++
tests/system-ovn.at <http://system-ovn.at> | 91
++++++++++++++++++++++++++++++++++++-
4 files changed, 172 insertions(+), 13 deletions(-)
diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
index de90025f0..662029597 100644
--- a/controller/ovn-controller.c
+++ b/controller/ovn-controller.c
@@ -708,7 +708,7 @@ get_snat_ct_zone(const struct
sbrec_datapath_binding *dp)
}
static void
-update_ct_zones(const struct shash *binding_lports,
+update_ct_zones(const struct sset *local_lports,
const struct hmap *local_datapaths,
struct simap *ct_zones, unsigned long *ct_zone_bitmap,
struct shash *pending_ct_zones)
@@ -721,9 +721,9 @@ update_ct_zones(const struct shash *binding_lports,
unsigned long unreq_snat_zones_map[BITMAP_N_LONGS(MAX_CT_ZONES)];
struct simap unreq_snat_zones =
SIMAP_INITIALIZER(&unreq_snat_zones);
- struct shash_node *shash_node;
- SHASH_FOR_EACH (shash_node, binding_lports) {
- sset_add(&all_users, shash_node->name);
+ const char *local_lport;
+ SSET_FOR_EACH (local_lport, local_lports) {
+ sset_add(&all_users, local_lport);
}
/* Local patched datapath (gateway routers) need zones
assigned. */
@@ -2377,7 +2377,7 @@ en_ct_zones_run(struct engine_node *node, void
*data)
EN_OVSDB_GET(engine_get_input("OVS_bridge", node));
restore_ct_zones(bridge_table, ovs_table, ct_zones_data);
- update_ct_zones(&rt_data->lbinding_data.lports,
&rt_data->local_datapaths,
+ update_ct_zones(&rt_data->local_lports, &rt_data->local_datapaths,
&ct_zones_data->current, ct_zones_data->bitmap,
&ct_zones_data->pending);
@@ -2467,8 +2467,10 @@ ct_zones_runtime_data_handler(struct
engine_node *node, void *data)
SHASH_FOR_EACH (shash_node, &tdp->lports) {
struct tracked_lport *t_lport = shash_node->data;
if (strcmp(t_lport->pb->type, "")
- && strcmp(t_lport->pb->type, "localport")) {
- /* We allocate zone-id's only to VIF and localport
lports. */
+ && strcmp(t_lport->pb->type, "localport")
+ && strcmp(t_lport->pb->type, "localnet")) {
+ /* We allocate zone-id's only to VIF, localport,
and localnet
+ * lports. */
continue;
}
diff --git a/northd/northd.c b/northd/northd.c
index b69fcf321..41d0f5994 100644
--- a/northd/northd.c
+++ b/northd/northd.c
@@ -5968,7 +5968,8 @@ build_pre_acls(struct ovn_datapath *od, const
struct hmap *port_groups,
}
for (size_t i = 0; i < od->n_localnet_ports; i++) {
skip_port_from_conntrack(od, od->localnet_ports[i],
- S_SWITCH_IN_PRE_ACL,
S_SWITCH_OUT_PRE_ACL,
+ S_SWITCH_IN_PRE_ACL,
+ S_SWITCH_OUT_PRE_ACL,
110, lflows);
}
@@ -6137,10 +6138,17 @@ build_pre_lb(struct ovn_datapath *od, const
struct shash *meter_groups,
S_SWITCH_IN_PRE_LB,
S_SWITCH_OUT_PRE_LB,
110, lflows);
}
- for (size_t i = 0; i < od->n_localnet_ports; i++) {
- skip_port_from_conntrack(od, od->localnet_ports[i],
- S_SWITCH_IN_PRE_LB,
S_SWITCH_OUT_PRE_LB,
- 110, lflows);
+ /* Localnet ports have no need for going through conntrack, unless
+ * the logical switch has a load balancer. Then, conntrack is
necessary
+ * so that traffic arriving via the localnet port can be load
+ * balanced.
+ */
+ if (!od->has_lb_vip) {
+ for (size_t i = 0; i < od->n_localnet_ports; i++) {
+ skip_port_from_conntrack(od, od->localnet_ports[i],
+ S_SWITCH_IN_PRE_LB,
S_SWITCH_OUT_PRE_LB,
+ 110, lflows);
+ }
}
/* Do not sent statless flows via conntrack */
diff --git a/tests/ovn-northd.at <http://ovn-northd.at>
b/tests/ovn-northd.at <http://ovn-northd.at>
index 047b8b6ad..850bc25a4 100644
--- a/tests/ovn-northd.at <http://ovn-northd.at>
+++ b/tests/ovn-northd.at <http://ovn-northd.at>
@@ -8975,3 +8975,63 @@ mac_binding_timestamp: true
AT_CLEANUP
])
+
+OVN_FOR_EACH_NORTHD_NO_HV([
+AT_SETUP([Localnet ports on LS with LB])
+ovn_start
+# In the past, traffic arriving on localnet ports has skipped
conntrack.
+# This test ensures that we still skip conntrack for localnet ports,
+# *except* for the case where the logical switch has a load balancer
+# configured. In this case, the localnet port will not skip conntrack,
+# allowing for traffic to be load balanced on the localnet port.
+
+check ovn-nbctl ls-add sw
+check ovn-nbctl lsp-add sw sw-ln
+check ovn-nbctl lsp-set-type sw-ln localnet
+check ovn-nbctl lsp-set-addresses sw-ln unknown
+check ovn-nbctl --wait=sb sync
+
+# Since this test is only concerned with logical flows, we don't
need to
+# configure anything else that we normally would with regards to
localnet
+# ports
+
+
+# First, ensure that conntrack is skipped for the localnet port
since there
+# isn't a load balancer configured.
+
+AT_CHECK([ovn-sbctl lflow-list sw | grep ls_in_pre_lb | grep
priority=110 | grep sw-ln | sed 's/table=../table=??/'], [0], [dnl
+ table=??(ls_in_pre_lb ), priority=110 , match=(ip &&
inport == "sw-ln"), action=(next;)
+])
+
+AT_CHECK([ovn-sbctl lflow-list sw | grep ls_out_pre_lb | grep
priority=110 | grep sw-ln | sed 's/table=../table=??/'], [0], [dnl
+ table=??(ls_out_pre_lb ), priority=110 , match=(ip &&
outport == "sw-ln"), action=(ct_clear; next;)
+])
+
+# Now add a load balancer and ensure that we no longer are skipping
conntrack
+# for the localnet port
+
+check ovn-nbctl lb-add lb 10.0.0.1:80 <http://10.0.0.1:80>
10.0.0.100:8080 <http://10.0.0.100:8080> tcp
+check ovn-nbctl ls-lb-add sw lb
+check ovn-nbctl --wait=sb sync
+
+AT_CHECK([ovn-sbctl lflow-list sw | grep ls_in_pre_lb | grep
priority=110 | grep sw-ln | sed 's/table=../table=??/'], [0], [dnl
+])
+
+AT_CHECK([ovn-sbctl lflow-list sw | grep ls_out_pre_lb | grep
priority=110 | grep sw-ln | sed 's/table=../table=??/'], [0], [dnl
+])
+
+# And ensure that removing the load balancer from the switch
results in skipping
+# conntrack again
+check ovn-nbctl ls-lb-del sw lb
+check ovn-nbctl --wait=sb sync
+
+AT_CHECK([ovn-sbctl lflow-list sw | grep ls_in_pre_lb | grep
priority=110 | grep sw-ln | sed 's/table=../table=??/'], [0], [dnl
+ table=??(ls_in_pre_lb ), priority=110 , match=(ip &&
inport == "sw-ln"), action=(next;)
+])
+
+AT_CHECK([ovn-sbctl lflow-list sw | grep ls_out_pre_lb | grep
priority=110 | grep sw-ln | sed 's/table=../table=??/'], [0], [dnl
+ table=??(ls_out_pre_lb ), priority=110 , match=(ip &&
outport == "sw-ln"), action=(ct_clear; next;)
+])
+
+AT_CLEANUP
+])
diff --git a/tests/system-ovn.at <http://system-ovn.at>
b/tests/system-ovn.at <http://system-ovn.at>
index df0dd99fb..61fb47865 100644
--- a/tests/system-ovn.at <http://system-ovn.at>
+++ b/tests/system-ovn.at <http://system-ovn.at>
@@ -10692,7 +10692,7 @@ ovn_start
ADD_BR([br-int])
# Set external-ids in br-int needed for ovn-controller
-check ovs-vsctl \
+ovs-vsctl \
-- set Open_vSwitch . external-ids:system-id=hv1 \
-- set Open_vSwitch .
external-ids:ovn-remote=unix:$ovs_base/ovn-sb/ovn-sb.sock \
-- set Open_vSwitch . external-ids:ovn-encap-type=geneve \
@@ -11009,3 +11009,92 @@ OVS_TRAFFIC_VSWITCHD_STOP(["/failed to
query port patch-.*/d
AT_CLEANUP
])
+OVN_FOR_EACH_NORTHD([
+AT_SETUP([load balancer with localnet port])
+CHECK_CONNTRACK()
+CHECK_CONNTRACK_NAT()
+ovn_start
+OVS_TRAFFIC_VSWITCHD_START()
+ADD_BR([br-int])
+ADD_BR([br-phys], [set Bridge br-phys fail-mode=standalone])
+
+# Set external-ids in br-int needed for ovn-controller
+ovs-vsctl \
+ -- set Open_vSwitch . external-ids:system-id=hv1 \
+ -- set Open_vSwitch .
external-ids:ovn-remote=unix:$ovs_base/ovn-sb/ovn-sb.sock \
+ -- set Open_vSwitch . external-ids:ovn-encap-type=geneve \
+ -- set Open_vSwitch . external-ids:ovn-encap-ip=169.0.0.1 \
+ -- set bridge br-int fail-mode=secure
other-config:disable-in-band=true
+
+start_daemon ovn-controller
+
+check ovn-nbctl lr-add ro
+check ovn-nbctl lrp-add ro ro-sw 00:00:00:00:00:01 192.168.0.1/24
<http://192.168.0.1/24>
+check ovn-nbctl lrp-add ro ro-pub 00:00:00:00:01:01 10.0.0.1/24
<http://10.0.0.1/24>
+
+check ovn-nbctl ls-add sw
+check ovn-nbctl lsp-add sw sw-vm1 \
+ -- lsp-set-addresses sw-vm1 "00:00:00:00:00:02 192.168.0.2"
+check ovn-nbctl lsp-add sw sw-ro \
+ -- lsp-set-type sw-ro router \
+ -- lsp-set-addresses sw-ro router \
+ -- lsp-set-options sw-ro router-port=ro-sw
+
+check ovn-nbctl ls-add pub
+check ovn-nbctl lsp-add pub sw-ln \
+ -- lsp-set-type sw-ln localnet \
+ -- lsp-set-addresses sw-ln unknown \
+ -- lsp-set-options sw-ln network_name=phys
+check ovn-nbctl lsp-add pub pub-ro \
+ -- lsp-set-type pub-ro router \
+ -- lsp-set-addresses pub-ro router \
+ -- lsp-set-options pub-ro router-port=ro-pub
+
+check ovs-vsctl set open .
external-ids:ovn-bridge-mappings=phys:br-phys
+
+ADD_NAMESPACES(sw-vm1)
+ADD_VETH(sw-vm1, sw-vm1, br-int, "192.168.0.2/24
<http://192.168.0.2/24>", "00:00:00:00:00:02", \
+ "192.168.0.1")
+
+ADD_NAMESPACES(ln)
+ADD_VETH(ln, ln, br-phys, "10.0.0.2/24 <http://10.0.0.2/24>",
"00:00:00:00:01:02", \
+ "10.0.0.1")
+
+# We have the basic network set up. Now let's add a load balancer
+# on the "pub" logical switch.
+
+check ovn-nbctl lb-add ln-lb 172.16.0.1:80 <http://172.16.0.1:80>
192.168.0.2:80 <http://192.168.0.2:80> tcp
+check ovn-nbctl ls-lb-add pub ln-lb
+check ovn-nbctl --wait=hv sync
+
+# Add a route so that the localnet port can reach the load balancer
+# VIP.
+NS_CHECK_EXEC([ln], [ip route add 172.16.0.1 via 10.0.0.1])
+NS_CHECK_EXEC([ln], [ip route add 192.168.0.0/24
<http://192.168.0.0/24> via 10.0.0.1])
+
+OVS_START_L7([sw-vm1], [http])
+
+NS_CHECK_EXEC([ln], [wget 172.16.0.1 -t 5 -T 1 --retry-connrefused
-v -o wget.log])
+
+AT_CHECK([ovs-appctl dpctl/dump-conntrack | FORMAT_CT(172.16.0.1) | \
+sed -e 's/zone=[[0-9]]*/zone=<cleared>/'], [0], [dnl
+tcp,orig=(src=10.0.0.2,dst=172.16.0.1,sport=<cleared>,dport=<cleared>),reply=(src=192.168.0.2,dst=10.0.0.2,sport=<cleared>,dport=<cleared>),zone=<cleared>,mark=2,protoinfo=(state=<cleared>)
+])
+
+OVS_APP_EXIT_AND_WAIT([ovn-controller])
+
+as ovn-sb
+OVS_APP_EXIT_AND_WAIT([ovsdb-server])
+
+as ovn-nb
+OVS_APP_EXIT_AND_WAIT([ovsdb-server])
+
+as northd
+OVS_APP_EXIT_AND_WAIT([NORTHD_TYPE])
+
+as
+OVS_TRAFFIC_VSWITCHD_STOP(["/failed to query port patch-.*/d
+/connection dropped.*/d"])
+
+AT_CLEANUP
+])
--
2.39.2
_______________________________________________
dev mailing list
d...@openvswitch.org <mailto:d...@openvswitch.org>
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
<https://mail.openvswitch.org/mailman/listinfo/ovs-dev>
Looks good to me, thanks.
Acked-by: Ales Musil <amu...@redhat.com <mailto:amu...@redhat.com>>
--
Ales Musil
Senior Software Engineer - OVN Core
Red Hat EMEA <https://www.redhat.com>
amu...@redhat.com <mailto:amu...@redhat.com> IM: amusil
<https://red.ht/sig>
