This capability is not in all OVS versions, so we need to check for it
in order to use it.

A future commit will make use of this feature, so this lays the
groundwork for being able to confirm if chassis all support being able
to flush CT entries using label or mark bits.

Signed-off-by: Mark Michelson <mmich...@redhat.com>
Acked-by: Ales Musil <amu...@redhat.com>
---
v6 -> v7:
 * Rebased.

v5 -> v6:
 * Added .ip_proto = IPPROTO_ICMP when detecting if flushing using
   ct_label is supported.
 * Added Ales's Ack.

v4 -> v5:
 * Fixed failing tests

v3 -> v4:
 * Rebased, but no functional changes

v2 -> v3:
 * No changes

v1 -> v2:
 * Added this patch to the series
---
 controller/chassis.c      | 15 ++++++++++
 include/ovn/features.h    |  3 ++
 lib/features.c            | 58 ++++++++++++++++++++++++++++++++++++++-
 northd/en-global-config.c | 10 +++++++
 northd/en-global-config.h |  1 +
 tests/ovn-controller.at   |  8 +++---
 6 files changed, 90 insertions(+), 5 deletions(-)

diff --git a/controller/chassis.c b/controller/chassis.c
index 19a251f26..aa5980d92 100644
--- a/controller/chassis.c
+++ b/controller/chassis.c
@@ -71,6 +71,8 @@ struct ovs_chassis_cfg {
     bool is_interconn;
     /* Does OVS support sampling with ids taken from registers? */
     bool sample_with_regs;
+    /* Does OVS support flushing CT zones using label/mark? */
+    bool ct_label_flush;
 };
 
 static void
@@ -358,6 +360,8 @@ chassis_parse_ovs_config(const struct 
ovsrec_open_vswitch_table *ovs_table,
     ovs_cfg->is_interconn = get_is_interconn(&cfg->external_ids, chassis_id);
     ovs_cfg->sample_with_regs =
         ovs_feature_is_supported(OVS_SAMPLE_REG_SUPPORT);
+    ovs_cfg->ct_label_flush =
+        ovs_feature_is_supported(OVS_CT_LABEL_FLUSH_SUPPORT);
 
     return true;
 }
@@ -395,6 +399,8 @@ chassis_build_other_config(const struct ovs_chassis_cfg 
*ovs_cfg,
     smap_replace(config, OVN_FEATURE_SAMPLE_WITH_REGISTERS,
                  ovs_cfg->sample_with_regs ? "true" : "false");
     smap_replace(config, OVN_FEATURE_CT_NEXT_ZONE, "true");
+    smap_replace(config, OVN_FEATURE_CT_LABEL_FLUSH,
+                 ovs_cfg->ct_label_flush ? "true" :"false");
 }
 
 /*
@@ -560,6 +566,14 @@ chassis_other_config_changed(const struct ovs_chassis_cfg 
*ovs_cfg,
         return true;
     }
 
+    bool chassis_ct_label_flush =
+        smap_get_bool(&chassis_rec->other_config,
+                      OVN_FEATURE_CT_LABEL_FLUSH,
+                      false);
+    if (chassis_ct_label_flush != ovs_cfg->ct_label_flush) {
+        return true;
+    }
+
     return false;
 }
 
@@ -718,6 +732,7 @@ update_supported_sset(struct sset *supported)
     sset_add(supported, OVN_FEATURE_CT_COMMIT_TO_ZONE);
     sset_add(supported, OVN_FEATURE_SAMPLE_WITH_REGISTERS);
     sset_add(supported, OVN_FEATURE_CT_NEXT_ZONE);
+    sset_add(supported, OVN_FEATURE_CT_LABEL_FLUSH);
 }
 
 static void
diff --git a/include/ovn/features.h b/include/ovn/features.h
index 95f3704ca..24c72c221 100644
--- a/include/ovn/features.h
+++ b/include/ovn/features.h
@@ -31,6 +31,7 @@
 #define OVN_FEATURE_CT_COMMIT_TO_ZONE "ct-commit-to-zone"
 #define OVN_FEATURE_SAMPLE_WITH_REGISTERS "ovn-sample-with-registers"
 #define OVN_FEATURE_CT_NEXT_ZONE "ct-next-zone"
+#define OVN_FEATURE_CT_LABEL_FLUSH "ct-label-flush"
 
 /* OVS datapath supported features.  Based on availability OVN might generate
  * different types of openflows.
@@ -42,6 +43,7 @@ enum ovs_feature_support_bits {
     OVS_DP_HASH_L4_SYM_BIT,
     OVS_OF_GROUP_SUPPORT_BIT,
     OVS_SAMPLE_REG_SUPPORT_BIT,
+    OVS_CT_LABEL_FLUSH_BIT,
 };
 
 enum ovs_feature_value {
@@ -51,6 +53,7 @@ enum ovs_feature_value {
     OVS_DP_HASH_L4_SYM_SUPPORT = (1 << OVS_DP_HASH_L4_SYM_BIT),
     OVS_OF_GROUP_SUPPORT = (1 << OVS_OF_GROUP_SUPPORT_BIT),
     OVS_SAMPLE_REG_SUPPORT = (1 << OVS_SAMPLE_REG_SUPPORT_BIT),
+    OVS_CT_LABEL_FLUSH_SUPPORT = (1 << OVS_CT_LABEL_FLUSH_BIT),
 };
 
 void ovs_feature_support_destroy(void);
diff --git a/lib/features.c b/lib/features.c
index 7302599ac..4afb58a2a 100644
--- a/lib/features.c
+++ b/lib/features.c
@@ -35,6 +35,7 @@
 #include "openvswitch/ofp-group.h"
 #include "openvswitch/ofp-print.h"
 #include "openvswitch/ofp-util.h"
+#include "openvswitch/ofp-ct.h"
 #include "openvswitch/rconn.h"
 #include "ovn/features.h"
 #include "controller/ofctrl.h"
@@ -271,6 +272,53 @@ sample_with_reg_handle_barrier(struct ovs_openflow_feature 
*feature OVS_UNUSED)
     return true;
 }
 
+static void
+ct_label_flush_send_request(struct ovs_openflow_feature *feature)
+{
+    /* At the time of this code being written, the highest bits
+     * of the CT label are not used for anything. Setting these
+     * is most likely to minimize flushing a valid CT entry by
+     * mistake.
+     */
+    ovs_u128 ct_mask = {
+        .u64.hi = 0x10000000,
+    };
+    ovs_u128 ct_value = {
+        .u64.hi = 0x10000000,
+    };
+    struct ofp_ct_match match = {
+        .labels = ct_value,
+        .labels_mask = ct_mask,
+        /* ICMP type 1 is unassigned. This should reduce the
+         * risk of flushing legitimate CT entries by mistake
+         */
+        .ip_proto = IPPROTO_ICMP,
+        .tuple_orig.icmp_type = 1,
+    };
+
+    struct ofpbuf *msg = ofp_ct_match_encode(&match, NULL,
+                                             rconn_get_version(swconn));
+    const struct ofp_header *oh = msg->data;
+    feature->xid = oh->xid;
+    rconn_send(swconn, msg, NULL);
+}
+
+static bool
+ct_label_flush_handle_response(struct ovs_openflow_feature *feature,
+                               enum ofptype type, const struct ofp_header *oh)
+{
+    if (type != OFPTYPE_ERROR) {
+        log_unexpected_reply(feature, oh);
+    }
+    return false;
+}
+
+static bool
+ct_label_flush_handle_barrier(struct ovs_openflow_feature *feature OVS_UNUSED)
+{
+    return true;
+}
+
 static struct ovs_openflow_feature all_openflow_features[] = {
         {
             .value = OVS_DP_METER_SUPPORT,
@@ -292,7 +340,14 @@ static struct ovs_openflow_feature all_openflow_features[] 
= {
             .send_request = sample_with_reg_send_request,
             .handle_response = sample_with_reg_handle_response,
             .handle_barrier = sample_with_reg_handle_barrier,
-        }
+        },
+        {
+            .value = OVS_CT_LABEL_FLUSH_SUPPORT,
+            .name = "ct_label_flush",
+            .send_request = ct_label_flush_send_request,
+            .handle_response = ct_label_flush_handle_response,
+            .handle_barrier = ct_label_flush_handle_barrier,
+        },
 };
 
 static bool
@@ -365,6 +420,7 @@ ovs_feature_is_valid(enum ovs_feature_value feature)
     case OVS_DP_HASH_L4_SYM_SUPPORT:
     case OVS_OF_GROUP_SUPPORT:
     case OVS_SAMPLE_REG_SUPPORT:
+    case OVS_CT_LABEL_FLUSH_SUPPORT:
         return true;
     default:
         return false;
diff --git a/northd/en-global-config.c b/northd/en-global-config.c
index ce16c26f2..97d861eb1 100644
--- a/northd/en-global-config.c
+++ b/northd/en-global-config.c
@@ -383,6 +383,7 @@ northd_enable_all_features(struct ed_type_global_config 
*data)
         .ct_commit_to_zone = true,
         .sample_with_reg = true,
         .ct_next_zone = true,
+        .ct_label_flush = true,
     };
 }
 
@@ -462,6 +463,15 @@ build_chassis_features(const struct sbrec_chassis_table 
*sbrec_chassis_table,
             chassis_features->ct_next_zone) {
             chassis_features->ct_next_zone = false;
         }
+
+        bool ct_label_flush =
+                smap_get_bool(&chassis->other_config,
+                              OVN_FEATURE_CT_LABEL_FLUSH,
+                              false);
+        if (!ct_label_flush &&
+            chassis_features->ct_label_flush) {
+            chassis_features->ct_label_flush = false;
+        }
     }
 }
 
diff --git a/northd/en-global-config.h b/northd/en-global-config.h
index 767810542..a95ba2a6c 100644
--- a/northd/en-global-config.h
+++ b/northd/en-global-config.h
@@ -21,6 +21,7 @@ struct chassis_features {
     bool ct_commit_to_zone;
     bool sample_with_reg;
     bool ct_next_zone;
+    bool ct_label_flush;
 };
 
 struct global_config_tracked_data {
diff --git a/tests/ovn-controller.at b/tests/ovn-controller.at
index 73bd50358..9eac7844d 100644
--- a/tests/ovn-controller.at
+++ b/tests/ovn-controller.at
@@ -3327,7 +3327,7 @@ check_ct_zone_max 22
 AS_BOX([Check LR snat requested zone 2])
 AT_CHECK([test $(ovn-appctl -t ovn-controller ct-zone-list | awk 
'/lr_snat/{print $2}') -eq 2])
 
-n_flush=$(grep -c -i flush hv1/ovs-vswitchd.log)
+n_flush=$(grep -c -i ct_flush_zone hv1/ovs-vswitchd.log)
 
 # Stop ovn-controller on hv1 with --restart flag
 OVN_CONTROLLER_EXIT([hv1])
@@ -3337,7 +3337,7 @@ wait_for_ports_up
 check ovn-nbctl --wait=hv sync
 
 # Check we do not have unexpected ct-flush restarting ovn-controller
-AT_CHECK([test $(grep -c -i flush hv1/ovs-vswitchd.log) -eq ${n_flush}])
+AT_CHECK([test $(grep -c -i ct_flush_zone hv1/ovs-vswitchd.log) -eq 
${n_flush}])
 
 AS_BOX([Check LR snat allowed requested zone 0])
 check ovn-nbctl set logical_router lr options:snat-ct-zone=0
@@ -3347,14 +3347,14 @@ check_ct_zone_min 0
 check_ct_zone_max 22
 AT_CHECK([test $(ovn-appctl -t ovn-controller ct-zone-list | awk 
'/lr_snat/{print $2}') -eq 0])
 
-n_flush=$(grep -c -i flush hv1/ovs-vswitchd.log)
+n_flush=$(grep -c -i ct_flush_zone hv1/ovs-vswitchd.log)
 OVN_CONTROLLER_EXIT([hv1])
 start_daemon ovn-controller --verbose="ct_zone:dbg"
 wait_for_ports_up
 check ovn-nbctl --wait=hv sync
 
 # Check we do not have unexpected ct-flush restarting ovn-controller
-AT_CHECK([test $(grep -c -i flush hv1/ovs-vswitchd.log) -eq ${n_flush}])
+AT_CHECK([test $(grep -c -i ct_flush_zone hv1/ovs-vswitchd.log) -eq 
${n_flush}])
 
 OVN_CLEANUP([hv1])
 AT_CLEANUP
-- 
2.45.2

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

Reply via email to