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 b8ca445bcd07..6bfccf5da5c0 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 8204a5cc35e5..c191b142e60a 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);)
-- 
2.38.1

_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to