diff mbox series

[ovs-dev,v9,4/5] dpif: Probe support for OVS_ACTION_ATTR_DROP.

Message ID 20240307135222.54976-5-eric@garver.life
State Superseded
Headers show
Series dpif: probe support for OVS_ACTION_ATTR_DROP | expand

Checks

Context Check Description
ovsrobot/apply-robot success apply and check: success
ovsrobot/github-robot-_Build_and_Test fail github build: failed
ovsrobot/intel-ovs-compilation success test: success

Commit Message

Eric Garver March 7, 2024, 1:52 p.m. UTC
Kernel support has been added for this action. As such, we need to probe
the datapath for support.

Signed-off-by: Eric Garver <eric@garver.life>
---
 include/linux/openvswitch.h |  2 +-
 lib/dpif.c                  |  6 ---
 lib/dpif.h                  |  1 -
 ofproto/ofproto-dpif.c      | 89 +++++++++++++++++++++++++++++++++++--
 ofproto/ofproto-dpif.h      |  4 +-
 5 files changed, 89 insertions(+), 13 deletions(-)
diff mbox series

Patch

diff --git a/include/linux/openvswitch.h b/include/linux/openvswitch.h
index a265e05ad253..fce6456f47c8 100644
--- a/include/linux/openvswitch.h
+++ b/include/linux/openvswitch.h
@@ -1086,11 +1086,11 @@  enum ovs_action_attr {
 	OVS_ACTION_ATTR_CHECK_PKT_LEN, /* Nested OVS_CHECK_PKT_LEN_ATTR_*. */
 	OVS_ACTION_ATTR_ADD_MPLS,     /* struct ovs_action_add_mpls. */
 	OVS_ACTION_ATTR_DEC_TTL,      /* Nested OVS_DEC_TTL_ATTR_*. */
+	OVS_ACTION_ATTR_DROP,         /* u32 xlate_error. */
 
 #ifndef __KERNEL__
 	OVS_ACTION_ATTR_TUNNEL_PUSH,   /* struct ovs_action_push_tnl*/
 	OVS_ACTION_ATTR_TUNNEL_POP,    /* u32 port number. */
-	OVS_ACTION_ATTR_DROP,          /* u32 xlate_error. */
 	OVS_ACTION_ATTR_LB_OUTPUT,     /* u32 bond-id. */
 #endif
 	__OVS_ACTION_ATTR_MAX,	      /* Nothing past this will be accepted
diff --git a/lib/dpif.c b/lib/dpif.c
index 0f480bec48d0..28fa40879655 100644
--- a/lib/dpif.c
+++ b/lib/dpif.c
@@ -1934,12 +1934,6 @@  dpif_supports_tnl_push_pop(const struct dpif *dpif)
     return dpif_is_netdev(dpif);
 }
 
-bool
-dpif_supports_explicit_drop_action(const struct dpif *dpif)
-{
-    return dpif_is_netdev(dpif);
-}
-
 bool
 dpif_supports_lb_output_action(const struct dpif *dpif)
 {
diff --git a/lib/dpif.h b/lib/dpif.h
index 0f2dc2ef3c55..8dc5da45b3ef 100644
--- a/lib/dpif.h
+++ b/lib/dpif.h
@@ -940,7 +940,6 @@  int dpif_get_pmds_for_port(const struct dpif * dpif, odp_port_t port_no,
 
 char *dpif_get_dp_version(const struct dpif *);
 bool dpif_supports_tnl_push_pop(const struct dpif *);
-bool dpif_supports_explicit_drop_action(const struct dpif *);
 bool dpif_synced_dp_layers(struct dpif *);
 
 /* Log functions. */
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index 718cc7bfd254..0c500fe5d8a6 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -72,6 +72,8 @@ 
 #include "util.h"
 #include "uuid.h"
 #include "vlan-bitmap.h"
+#include "dpif-netdev.h"
+#include "netdev-offload.h"
 
 VLOG_DEFINE_THIS_MODULE(ofproto_dpif);
 
@@ -221,6 +223,8 @@  static void ct_zone_config_init(struct dpif_backer *backer);
 static void ct_zone_config_uninit(struct dpif_backer *backer);
 static void ct_zone_timeout_policy_sweep(struct dpif_backer *backer);
 static void ct_zone_limits_commit(struct dpif_backer *backer);
+static bool check_drop_action(struct dpif_backer *backer);
+static bool recheck_support_explicit_drop_action(struct dpif_backer *backer);
 
 static inline struct ofproto_dpif *
 ofproto_dpif_cast(const struct ofproto *ofproto)
@@ -391,6 +395,10 @@  type_run(const char *type)
         udpif_set_threads(backer->udpif, n_handlers, n_revalidators);
     }
 
+    if (recheck_support_explicit_drop_action(backer)) {
+        backer->need_revalidate = REV_RECONFIGURE;
+    }
+
     if (backer->need_revalidate) {
         struct ofproto_dpif *ofproto;
         struct simap_node *node;
@@ -807,6 +815,7 @@  open_dpif_backer(const char *type, struct dpif_backer **backerp)
 
     shash_add(&all_dpif_backers, type, backer);
 
+    atomic_init(&backer->rt_support.explicit_drop_action, false);
     check_support(backer);
     atomic_count_init(&backer->tnl_count, 0);
 
@@ -855,7 +864,10 @@  ovs_native_tunneling_is_on(struct ofproto_dpif *ofproto)
 bool
 ovs_explicit_drop_action_supported(struct ofproto_dpif *ofproto)
 {
-    return ofproto->backer->rt_support.explicit_drop_action;
+    bool value;
+    atomic_read_relaxed(&ofproto->backer->rt_support.explicit_drop_action,
+                        &value);
+    return value;
 }
 
 bool
@@ -1379,6 +1391,43 @@  check_ct_timeout_policy(struct dpif_backer *backer)
     return !error;
 }
 
+/* Tests whether backer's datapath supports the OVS_ACTION_ATTR_DROP action. */
+static bool
+check_drop_action(struct dpif_backer *backer)
+{
+    struct odputil_keybuf keybuf;
+    uint8_t actbuf[NL_A_U32_SIZE];
+    struct ofpbuf actions;
+    struct ofpbuf key;
+    struct flow flow;
+    bool supported;
+    bool hw_offload;
+
+    struct odp_flow_key_parms odp_parms = {
+        .flow = &flow,
+        .probe = true,
+    };
+
+    memset(&flow, 0, sizeof flow);
+    ofpbuf_use_stack(&key, &keybuf, sizeof keybuf);
+    odp_flow_key_from_flow(&odp_parms, &key);
+
+    ofpbuf_use_stack(&actions, &actbuf, sizeof actbuf);
+    nl_msg_put_u32(&actions, OVS_ACTION_ATTR_DROP, XLATE_OK);
+
+    supported = dpif_probe_feature(backer->dpif, "drop", &key, &actions, NULL);
+
+    /* TC does not support offloading this action. */
+    hw_offload = netdev_is_flow_api_enabled() && !dpif_is_netdev(backer->dpif);
+
+    VLOG_INFO("%s: Datapath %s drop action%s", dpif_name(backer->dpif),
+              (supported) ? "supports" : "does not support",
+              (hw_offload) ? ", but not enabled due to hardware offload being "
+                             "enabled" : "");
+
+    return supported && !hw_offload;
+}
+
 /* Tests whether 'backer''s datapath supports the all-zero SNAT case. */
 static bool
 dpif_supports_ct_zero_snat(struct dpif_backer *backer)
@@ -1647,8 +1696,8 @@  check_support(struct dpif_backer *backer)
     backer->rt_support.max_hash_alg = check_max_dp_hash_alg(backer);
     backer->rt_support.check_pkt_len = check_check_pkt_len(backer);
     backer->rt_support.ct_timeout = check_ct_timeout_policy(backer);
-    backer->rt_support.explicit_drop_action =
-        dpif_supports_explicit_drop_action(backer->dpif);
+    atomic_store_relaxed(&backer->rt_support.explicit_drop_action,
+                         check_drop_action(backer));
     backer->rt_support.lb_output_action =
         dpif_supports_lb_output_action(backer->dpif);
     backer->rt_support.ct_zero_snat = dpif_supports_ct_zero_snat(backer);
@@ -1665,6 +1714,36 @@  check_support(struct dpif_backer *backer)
     backer->rt_support.odp.nd_ext = check_nd_extensions(backer);
 }
 
+/* For TC, if vswitchd started with other_config:hw-offload set to "false", and
+ * the configuration is now "true", then we need to re-probe datapath support
+ * for the "drop" action. */
+static bool
+recheck_support_explicit_drop_action(struct dpif_backer *backer) {
+    bool explicit_drop_action;
+
+    atomic_read_relaxed(&backer->rt_support.explicit_drop_action,
+                        &explicit_drop_action);
+
+    if (explicit_drop_action
+        && netdev_is_flow_api_enabled() && !dpif_is_netdev(backer->dpif)) {
+        bool new_explicit_drop_action = check_drop_action(backer);
+
+        if (!new_explicit_drop_action) {
+            atomic_compare_exchange_strong_relaxed(
+                                   &backer->rt_support.explicit_drop_action,
+                                   &explicit_drop_action,
+                                   new_explicit_drop_action);
+            /* If the cmpx fails, it means the value changed to false in a
+             * different thread. As such, we still want to trigger a
+             * reconfigure.
+             */
+            return true;
+        }
+    }
+
+    return false;
+}
+
 static int
 construct(struct ofproto *ofproto_)
 {
@@ -5706,6 +5785,7 @@  ct_zone_limit_protection_update(const char *datapath_type, bool protected)
 static void
 get_datapath_cap(const char *datapath_type, struct smap *cap)
 {
+    bool explicit_drop_action;
     struct dpif_backer_support *s;
     struct dpif_backer *backer = shash_find_data(&all_dpif_backers,
                                                  datapath_type);
@@ -5741,8 +5821,9 @@  get_datapath_cap(const char *datapath_type, struct smap *cap)
     smap_add_format(cap, "max_hash_alg", "%"PRIuSIZE, s->max_hash_alg);
     smap_add(cap, "check_pkt_len", s->check_pkt_len ? "true" : "false");
     smap_add(cap, "ct_timeout", s->ct_timeout ? "true" : "false");
+    atomic_read_relaxed(&s->explicit_drop_action, &explicit_drop_action);
     smap_add(cap, "explicit_drop_action",
-             s->explicit_drop_action ? "true" :"false");
+             explicit_drop_action ? "true" :"false");
     smap_add(cap, "lb_output_action", s->lb_output_action ? "true" : "false");
     smap_add(cap, "ct_zero_snat", s->ct_zero_snat ? "true" : "false");
     smap_add(cap, "add_mpls", s->add_mpls ? "true" : "false");
diff --git a/ofproto/ofproto-dpif.h b/ofproto/ofproto-dpif.h
index 92d33aa64709..d33f73df8aed 100644
--- a/ofproto/ofproto-dpif.h
+++ b/ofproto/ofproto-dpif.h
@@ -51,6 +51,7 @@ 
 #include "hmapx.h"
 #include "odp-util.h"
 #include "id-pool.h"
+#include "ovs-atomic.h"
 #include "ovs-thread.h"
 #include "ofproto-provider.h"
 #include "util.h"
@@ -202,7 +203,8 @@  struct group_dpif *group_dpif_lookup(struct ofproto_dpif *,
     DPIF_SUPPORT_FIELD(bool, ct_timeout, "Conntrack timeout policy")        \
                                                                             \
     /* True if the datapath supports explicit drop action. */               \
-    DPIF_SUPPORT_FIELD(bool, explicit_drop_action, "Explicit Drop action")  \
+    DPIF_SUPPORT_FIELD(atomic_bool, explicit_drop_action,                   \
+                       "Explicit Drop action")                              \
                                                                             \
     /* True if the datapath supports balance_tcp optimization */            \
     DPIF_SUPPORT_FIELD(bool, lb_output_action, "Optimized Balance TCP mode")\