The ct_nw_dst(), ct_ip6_dst(), and ct_tp_dst() logical actions are
implemented by jumping to physical flows in tables 81, 82, and 83
respectively. These physical flows previously only matched on
established connections (ct.trk && ct.est), limiting their use to
scenarios where connections were already established.
Extend the physical flow matches to support both established and new
connection states by changing from:
match = ct.trk && ct.est && ip4/ip6
to:
match = ct.trk && (ct.est || ct.new) && ip4/ip6
This allows the ct_xxx actions to work with new connections, while
maintaining compatibility with existing established connection use
cases.
The responsibility for matching specific connection states (e.g., ct.est
for fragmented packet handling) is moved to the logical flows in
northd.c, providing more flexibility in how these actions are used
across different scenarios.
Signed-off-by: Han Zhou <hz...@ovn.org>
---
controller/physical.c | 38 +++++++++++++++++++++++++++++++-------
northd/northd.c | 6 ++++--
tests/ovn-northd.at | 24 ++++++++++++------------
3 files changed, 47 insertions(+), 21 deletions(-)
diff --git a/controller/physical.c b/controller/physical.c
index 1cc4173b2b6b..7b9b66ffe7f4 100644
--- a/controller/physical.c
+++ b/controller/physical.c
@@ -3033,24 +3033,35 @@ physical_run(struct physical_ctx *p_ctx,
add_default_drop_flow(p_ctx, OFTABLE_LOG_TO_PHY, flow_table);
/* Table 81, 82 and 83
- * Match on ct.trk and ct.est and store the ct_nw_dst, ct_ip6_dst and
- * ct_tp_dst in the registers. */
- uint32_t ct_state = OVS_CS_F_TRACKED | OVS_CS_F_ESTABLISHED;
+ * Match on ct.trk and ct.est | ct.new and store the ct_nw_dst, ct_ip6_dst
+ * and ct_tp_dst in the registers. */
+ uint32_t ct_state_est = OVS_CS_F_TRACKED | OVS_CS_F_ESTABLISHED;
+ uint32_t ct_state_new = OVS_CS_F_TRACKED | OVS_CS_F_NEW;
+ struct match match_new = MATCH_CATCHALL_INITIALIZER;
match_init_catchall(&match);
ofpbuf_clear(&ofpacts);
- /* Add the flow:
+ /* Add the flows:
* match = (ct.trk && ct.est), action = (reg8 = ct_tp_dst)
* table = 83
+ * match = (ct.trk && ct.new), action = (reg8 = ct_tp_dst)
+ * table = 83
*/
- match_set_ct_state_masked(&match, ct_state, ct_state);
+ match_set_ct_state_masked(&match, ct_state_est, ct_state_est);
put_move(MFF_CT_TP_DST, 0, MFF_LOG_CT_ORIG_TP_DST_PORT, 0, 16, &ofpacts);
ofctrl_add_flow(flow_table, OFTABLE_CT_ORIG_TP_DST_LOAD, 100, 0, &match,
&ofpacts, hc_uuid);
- /* Add the flow:
+ match_set_ct_state_masked(&match_new, ct_state_new, ct_state_new);
+ put_move(MFF_CT_TP_DST, 0, MFF_LOG_CT_ORIG_TP_DST_PORT, 0, 16, &ofpacts);
+ ofctrl_add_flow(flow_table, OFTABLE_CT_ORIG_TP_DST_LOAD, 100, 0,
+ &match_new, &ofpacts, hc_uuid);
+
+ /* Add the flows:
* match = (ct.trk && ct.est && ip4), action = (reg4 = ct_nw_dst)
* table = 81
+ * match = (ct.trk && ct.new && ip4), action = (reg4 = ct_nw_dst)
+ * table = 81
*/
ofpbuf_clear(&ofpacts);
match_set_dl_type(&match, htons(ETH_TYPE_IP));
@@ -3058,9 +3069,16 @@ physical_run(struct physical_ctx *p_ctx,
ofctrl_add_flow(flow_table, OFTABLE_CT_ORIG_NW_DST_LOAD, 100, 0, &match,
&ofpacts, hc_uuid);
- /* Add the flow:
+ match_set_dl_type(&match_new, htons(ETH_TYPE_IP));
+ put_move(MFF_CT_NW_DST, 0, MFF_LOG_CT_ORIG_NW_DST_ADDR, 0, 32, &ofpacts);
+ ofctrl_add_flow(flow_table, OFTABLE_CT_ORIG_NW_DST_LOAD, 100, 0,
+ &match_new, &ofpacts, hc_uuid);
+
+ /* Add the flows:
* match = (ct.trk && ct.est && ip6), action = (xxreg0 = ct_ip6_dst)
* table = 82
+ * match = (ct.trk && ct.new && ip6), action = (xxreg0 = ct_ip6_dst)
+ * table = 82
*/
ofpbuf_clear(&ofpacts);
match_set_dl_type(&match, htons(ETH_TYPE_IPV6));
@@ -3069,6 +3087,12 @@ physical_run(struct physical_ctx *p_ctx,
ofctrl_add_flow(flow_table, OFTABLE_CT_ORIG_IP6_DST_LOAD, 100, 0, &match,
&ofpacts, hc_uuid);
+ match_set_dl_type(&match_new, htons(ETH_TYPE_IPV6));
+ put_move(MFF_CT_IPV6_DST, 0, MFF_LOG_CT_ORIG_IP6_DST_ADDR, 0,
+ 128, &ofpacts);
+ ofctrl_add_flow(flow_table, OFTABLE_CT_ORIG_IP6_DST_LOAD, 100, 0,
+ &match_new, &ofpacts, hc_uuid);
+
/* Implement the ct_state_save() logical action. */
/* Add the flow:
diff --git a/northd/northd.c b/northd/northd.c
index 9b21786fad05..b46895bfafbb 100644
--- a/northd/northd.c
+++ b/northd/northd.c
@@ -8731,12 +8731,14 @@ build_lb_hairpin(const struct ls_stateful_record
*ls_stateful_rec,
* We need to find a better way to handle the fragmented packets.
* */
ovn_lflow_add(lflows, od, S_SWITCH_IN_LB, 110,
- "ct.trk && !ct.rpl && "REGBIT_IP_FRAG" == 1 && ip4",
+ "ct.trk && ct.est && !ct.rpl && "REGBIT_IP_FRAG
+ " == 1 && ip4",
REG_LB_IPV4 " = ct_nw_dst(); "
REG_LB_PORT " = ct_tp_dst(); next;",
lflow_ref);
ovn_lflow_add(lflows, od, S_SWITCH_IN_LB, 110,
- "ct.trk && !ct.rpl && "REGBIT_IP_FRAG" == 1 && ip6",
+ "ct.trk && ct.est && !ct.rpl && "REGBIT_IP_FRAG
+ " == 1 && ip6",
REG_LB_IPV6 " = ct_ip6_dst(); "
REG_LB_PORT " = ct_tp_dst(); next;",
lflow_ref);
diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
index 5e0747fcc816..3a353368a92f 100644
--- a/tests/ovn-northd.at
+++ b/tests/ovn-northd.at
@@ -4789,8 +4789,8 @@ check_stateful_flows() {
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.rpl &&
reg0[[19]] == 1 && ip4), action=(reg4 = ct_nw_dst(); reg2[[0..15]] = ct_tp_dst(); next;)
- table=??(ls_in_lb ), priority=110 , match=(ct.trk && !ct.rpl &&
reg0[[19]] == 1 && ip6), action=(xxreg1 = ct_ip6_dst(); reg2[[0..15]] = ct_tp_dst(); 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;)
+ 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;)
table=??(ls_in_lb ), priority=120 , match=(ct.new && ip4.dst ==
10.0.0.10 && ct_tcp.dst == 80), action=(reg4 = 10.0.0.10; reg2[[0..15]] = 80;
ct_lb_mark(backends=10.0.0.4:8080);)
table=??(ls_in_lb ), priority=120 , match=(ct.new && ip4.dst ==
10.0.0.20 && ct_tcp.dst == 80), action=(reg4 = 10.0.0.20; reg2[[0..15]] = 80;
ct_lb_mark(backends=10.0.0.40:8080);)
])
@@ -4897,8 +4897,8 @@ check ovn-nbctl --wait=sb ls-lb-add sw0 lb1
AT_CHECK([ovn-sbctl dump-flows sw0 | grep "ls_in_lb " | ovn_strip_lflows ],
[0], [dnl
table=??(ls_in_lb ), priority=0 , match=(1), action=(next;)
table=??(ls_in_lb ), priority=110 , match=(ct.new && ip4.dst ==
10.0.0.20), action=(drop;)
- table=??(ls_in_lb ), priority=110 , match=(ct.trk && !ct.rpl &&
reg0[[19]] == 1 && ip4), action=(reg4 = ct_nw_dst(); reg2[[0..15]] = ct_tp_dst(); next;)
- table=??(ls_in_lb ), priority=110 , match=(ct.trk && !ct.rpl &&
reg0[[19]] == 1 && ip6), action=(xxreg1 = ct_ip6_dst(); reg2[[0..15]] = ct_tp_dst(); 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;)
+ 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;)
])
AT_CLEANUP
@@ -8149,8 +8149,8 @@ AT_CHECK([grep -e "ls_in_acl.*eval" -e "ls_in_acl_hint"
lsflows | ovn_strip_lflo
AT_CHECK([grep -e "ls_in_lb " lsflows | ovn_strip_lflows], [0], [dnl
table=??(ls_in_lb ), priority=0 , match=(1), action=(next;)
table=??(ls_in_lb ), priority=110 , match=(ct.new && ip4.dst ==
10.0.0.2), action=(reg4 = 10.0.0.2; ct_lb_mark(backends=10.0.0.10);)
- table=??(ls_in_lb ), priority=110 , match=(ct.trk && !ct.rpl &&
reg0[[19]] == 1 && ip4), action=(reg4 = ct_nw_dst(); reg2[[0..15]] = ct_tp_dst(); next;)
- table=??(ls_in_lb ), priority=110 , match=(ct.trk && !ct.rpl &&
reg0[[19]] == 1 && ip6), action=(xxreg1 = ct_ip6_dst(); reg2[[0..15]] = ct_tp_dst(); 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;)
+ 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;)
])
AT_CHECK([grep -e "ls_in_stateful" lsflows | ovn_strip_lflows], [0], [dnl
@@ -8208,8 +8208,8 @@ AT_CHECK([grep -e "ls_in_acl.*eval" -e "ls_in_acl_hint"
lsflows | ovn_strip_lflo
AT_CHECK([grep -e "ls_in_lb " lsflows | ovn_strip_lflows], [0], [dnl
table=??(ls_in_lb ), priority=0 , match=(1), action=(next;)
table=??(ls_in_lb ), priority=110 , match=(ct.new && ip4.dst ==
10.0.0.2), action=(reg4 = 10.0.0.2; ct_lb_mark(backends=10.0.0.10);)
- table=??(ls_in_lb ), priority=110 , match=(ct.trk && !ct.rpl &&
reg0[[19]] == 1 && ip4), action=(reg4 = ct_nw_dst(); reg2[[0..15]] = ct_tp_dst(); next;)
- table=??(ls_in_lb ), priority=110 , match=(ct.trk && !ct.rpl &&
reg0[[19]] == 1 && ip6), action=(xxreg1 = ct_ip6_dst(); reg2[[0..15]] = ct_tp_dst(); 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;)
+ 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;)
])
AT_CHECK([grep -e "ls_in_stateful" lsflows | ovn_strip_lflows], [0], [dnl
@@ -8267,8 +8267,8 @@ AT_CHECK([grep -e "ls_in_acl.*eval" -e "ls_in_acl_hint"
lsflows | ovn_strip_lflo
AT_CHECK([grep -e "ls_in_lb " lsflows | ovn_strip_lflows], [0], [dnl
table=??(ls_in_lb ), priority=0 , match=(1), action=(next;)
table=??(ls_in_lb ), priority=110 , match=(ct.new && ip4.dst ==
10.0.0.2), action=(reg4 = 10.0.0.2; ct_lb_mark(backends=10.0.0.10);)
- table=??(ls_in_lb ), priority=110 , match=(ct.trk && !ct.rpl &&
reg0[[19]] == 1 && ip4), action=(reg4 = ct_nw_dst(); reg2[[0..15]] = ct_tp_dst(); next;)
- table=??(ls_in_lb ), priority=110 , match=(ct.trk && !ct.rpl &&
reg0[[19]] == 1 && ip6), action=(xxreg1 = ct_ip6_dst(); reg2[[0..15]] = ct_tp_dst(); 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;)
+ 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;)
])
AT_CHECK([grep -e "ls_in_stateful" lsflows | ovn_strip_lflows], [0], [dnl
@@ -9800,8 +9800,8 @@ AT_CHECK([grep "ls_in_lb_aff_check" S0flows |
ovn_strip_lflows], [0], [dnl
])
AT_CHECK([grep "ls_in_lb " S0flows | 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.rpl &&
reg0[[19]] == 1 && ip4), action=(reg4 = ct_nw_dst(); reg2[[0..15]] = ct_tp_dst(); next;)
- table=??(ls_in_lb ), priority=110 , match=(ct.trk && !ct.rpl &&
reg0[[19]] == 1 && ip6), action=(xxreg1 = ct_ip6_dst(); reg2[[0..15]] = ct_tp_dst(); 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;)
+ 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;)
table=??(ls_in_lb ), priority=120 , match=(ct.new && ip4.dst ==
172.16.0.10 && ct_tcp.dst == 80), action=(reg4 = 172.16.0.10; reg2[[0..15]] = 80;
ct_lb_mark(backends=10.0.0.2:80,20.0.0.2:80);)
table=??(ls_in_lb ), priority=150 , match=(reg9[[6]] == 1 && ct.new && ip4.dst
== 172.16.0.10 && reg4 == 10.0.0.2 && reg2[[0..15]] == 80), action=(reg4 = 172.16.0.10;
reg2[[0..15]] = 80; ct_lb_mark(backends=10.0.0.2:80);)
table=??(ls_in_lb ), priority=150 , match=(reg9[[6]] == 1 && ct.new && ip4.dst
== 172.16.0.10 && reg4 == 20.0.0.2 && reg2[[0..15]] == 80), action=(reg4 = 172.16.0.10;
reg2[[0..15]] = 80; ct_lb_mark(backends=20.0.0.2:80);)
