Commit 8e6f9a835 added a logical switch stage that extracts the
original conntrack protocol and destination port for every new IP
connection whenever the switch has a load balancer. This makes
unrelated traffic depend on conntrack tuple fields even though the
fields are only consumed by port-bearing load balancer rules.
Some hardware offload implementations cannot match these fields.
This is especially visible for unidirectional UDP, which may remain
in ct.new while still being eligible for hardware offload.
Generate extraction flows per port-bearing VIP instead. Match the VIP
address and load balancer protocol so unrelated traffic takes the
fallback flow without accessing conntrack tuple fields.
Add coverage for direct and grouped load balancers, removal, and
portless VIPs.
Fixes: 8e6f9a8355e2 ("northd: Fix HW offload problem related to ct_tuple.")
Assisted-by: OpenAI Codex
Signed-off-by: Tim Rozet <[email protected]>
---
Documentation/ref/ovn-logical-flows.7.rst | 11 ++++++----
northd/northd.c | 26 ++++++++++++++---------
tests/ovn-northd.at | 15 +++++++++++++
3 files changed, 38 insertions(+), 14 deletions(-)
diff --git a/Documentation/ref/ovn-logical-flows.7.rst
b/Documentation/ref/ovn-logical-flows.7.rst
index 0ec69e39e..f58feee70 100644
--- a/Documentation/ref/ovn-logical-flows.7.rst
+++ b/Documentation/ref/ovn-logical-flows.7.rst
@@ -551,10 +551,13 @@ Ingress Table 13: Connection Tracking Field Extraction
This table extracts connection tracking fields for new connections to be used
by
subsequent load balancing stages.
-- A priority-100 flow matches ``ct.new && ip`` and extracts connection tracking
- protocol and destination port information into registers ``reg1[16..23]``
- (protocol) and ``reg1[0..15]`` (destination port) using the actions
- ``reg1[16..23] = ct_proto(); reg1[0..15] = ct_tp_dst(); next;``.
+- For every load balancer VIP on the switch that includes an L4 port *PORT* of
+ protocol *P* and IP address *VIP*, a priority-100 flow is added. For IPv4
+ *VIPs*, the flow matches ``ct.new && ip4.dst == VIP && P``. For IPv6 *VIPs*,
+ the flow matches ``ct.new && ip6.dst == VIP && P``. It extracts connection
+ tracking protocol and destination port information into registers
+ ``reg1[16..23]`` (protocol) and ``reg1[0..15]`` (destination port) using the
+ actions ``reg1[16..23] = ct_proto(); reg1[0..15] = ct_tp_dst(); next;``.
- A priority-0 flow matches all packets and advances to the next table.
diff --git a/northd/northd.c b/northd/northd.c
index 8cd85a328..8a16793f1 100644
--- a/northd/northd.c
+++ b/northd/northd.c
@@ -9154,6 +9154,22 @@ build_lb_rules(struct lflow_table *lflows, struct
ovn_lb_datapaths *lb_dps,
ds_clear(action);
ds_clear(match);
+ /* Extract the original L4 tuple only for traffic that can match this
+ * port-bearing VIP. Accessing conntrack tuple fields for every new
+ * connection prevents unrelated traffic from being offloaded by some
+ * hardware implementations. */
+ if (lb_vip->port_str) {
+ ds_put_format(match, "ct.new && %s.dst == %s && %s", ip_match,
+ lb_vip->vip_str, lb->proto);
+ ovn_lflow_add_with_dp_group(
+ lflows, lb_dps->nb_ls_map.map, ods_size(ls_datapaths),
+ S_SWITCH_IN_CT_EXTRACT, 100, ds_cstr(match),
+ REG_CT_PROTO " = ct_proto(); "
+ REG_CT_TP_DST " = ct_tp_dst(); next;",
+ lb_dps->lflow_ref, WITH_HINT(&lb->nlb->header_));
+ ds_clear(match);
+ }
+
/* New connections in Ingress table. */
const char *meter = NULL;
bool reject = build_lb_vip_actions(lb, lb_vip, lb_vip_nb, action,
@@ -19248,16 +19264,6 @@ build_ls_stateful_flows(const struct
ls_stateful_record *ls_stateful_rec,
sampling_apps, features, ls_stateful_rec->lflow_ref,
sbrec_acl_id_table);
- /* Build CT extraction flows - only needed if this datapath has load
- * balancers. */
- if (ls_stateful_rec->has_lb_vip) {
- ovn_lflow_add(lflows, od, S_SWITCH_IN_CT_EXTRACT, 100,
- "ct.new && ip",
- REG_CT_PROTO " = ct_proto(); "
- REG_CT_TP_DST " = ct_tp_dst(); next;",
- ls_stateful_rec->lflow_ref);
- }
-
build_lb_hairpin(ls_stateful_rec, od, lflows, ls_stateful_rec->lflow_ref);
}
diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
index 13400ff5e..a5d2092b6 100644
--- a/tests/ovn-northd.at
+++ b/tests/ovn-northd.at
@@ -5124,6 +5124,12 @@ check_stateful_flows() {
table=??(ls_in_pre_stateful ), priority=120 , match=(reg0[[2]] == 1 &&
ip4.dst == 10.0.0.20 && tcp.dst == 80), action=(reg4 = 10.0.0.20; reg2[[0..15]]
= 80; ct_lb_mark;)
])
+ AT_CHECK([grep "ls_in_ct_extract" sw0flows | ovn_strip_lflows], [0], [dnl
+ table=??(ls_in_ct_extract ), priority=0 , match=(1), action=(next;)
+ table=??(ls_in_ct_extract ), priority=100 , match=(ct.new && ip4.dst ==
10.0.0.10 && tcp), action=(reg1[[16..23]] = ct_proto(); reg1[[0..15]] =
ct_tp_dst(); next;)
+ table=??(ls_in_ct_extract ), priority=100 , match=(ct.new && ip4.dst ==
10.0.0.20 && tcp), action=(reg1[[16..23]] = ct_proto(); reg1[[0..15]] =
ct_tp_dst(); next;)
+])
+
AT_CHECK([grep "ls_in_lb " sw0flows | ovn_strip_lflows], [0], [dnl
table=??(ls_in_lb ), priority=0 , match=(1), action=(next;)
table=??(ls_in_lb ), priority=110 , match=(ct.trk && ct.est &&
!ct.rpl && reg0[[19]] == 1 && ip4), action=(reg4 = ct_nw_dst(); reg2[[0..15]] =
ct_tp_dst(); next;)
@@ -5200,6 +5206,10 @@ AT_CHECK([grep "ls_in_pre_stateful" sw0flows |
ovn_strip_lflows], [0], [dnl
table=??(ls_in_pre_stateful ), priority=115 , match=(reg0[[2]] == 1 &&
ip.is_frag), action=(reg0[[19]] = 1; ct_lb_mark;)
])
+AT_CHECK([grep "ls_in_ct_extract" sw0flows | ovn_strip_lflows], [0], [dnl
+ table=??(ls_in_ct_extract ), priority=0 , match=(1), action=(next;)
+])
+
AT_CHECK([grep "ls_in_lb " sw0flows | ovn_strip_lflows], [0], [dnl
table=??(ls_in_lb ), priority=0 , match=(1), action=(next;)
])
@@ -5246,6 +5256,11 @@ AT_CHECK([ovn-sbctl dump-flows sw0 | grep "ls_in_lb " |
ovn_strip_lflows ], [0],
table=??(ls_in_lb ), priority=110 , match=(ct.trk && ct.est &&
!ct.rpl && reg0[[19]] == 1 && ip6), action=(xxreg1 = ct_ip6_dst();
reg2[[0..15]] = ct_tp_dst(); next;)
])
+# A VIP without an L4 port does not use the extracted fields.
+AT_CHECK([ovn-sbctl dump-flows sw0 | grep "ls_in_ct_extract" |
ovn_strip_lflows], [0], [dnl
+ table=??(ls_in_ct_extract ), priority=0 , match=(1), action=(next;)
+])
+
OVN_CLEANUP_NORTHD
AT_CLEANUP
])
--
2.55.0
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
