On 02/12/2025 16:04, Eelco Chaudron wrote:
Introduce per-offload-provider implementation types and remove the
synced_dp abstraction from the dpif level. The new API allows
querying the offload implementation type and which provider offloads
a given port.
Signed-off-by: Eelco Chaudron <[email protected]>
---
v2 changes:
- Fixed double spaces between two sentences in long comments.
---
lib/dpif-netdev.c | 1 -
lib/dpif-netlink.c | 1 -
lib/dpif-offload-dpdk.c | 1 +
lib/dpif-offload-dummy.c | 1 +
lib/dpif-offload-provider.h | 4 +++
lib/dpif-offload-tc.c | 1 +
lib/dpif-offload.c | 68 +++++++++++++++++++++++++++++++++++
lib/dpif-offload.h | 25 +++++++++++++
lib/dpif-provider.h | 8 -----
lib/dpif.c | 6 ----
lib/dpif.h | 1 -
ofproto/ofproto-dpif-upcall.c | 28 ++++++++++-----
12 files changed, 119 insertions(+), 26 deletions(-)
diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index fe9c8b7be..6575cb9e8 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -9958,7 +9958,6 @@ dpif_netdev_bond_stats_get(struct dpif *dpif, uint32_t
bond_id,
const struct dpif_class dpif_netdev_class = {
"netdev",
true, /* cleanup_required */
- true, /* synced_dp_layers */
dpif_netdev_init,
dpif_netdev_enumerate,
dpif_netdev_port_open_type,
diff --git a/lib/dpif-netlink.c b/lib/dpif-netlink.c
index 8a9acc904..04d7c8ed6 100644
--- a/lib/dpif-netlink.c
+++ b/lib/dpif-netlink.c
@@ -4363,7 +4363,6 @@ dpif_netlink_cache_set_size(struct dpif *dpif_, uint32_t
level, uint32_t size)
const struct dpif_class dpif_netlink_class = {
"system",
false, /* cleanup_required */
- false, /* synced_dp_layers */
NULL, /* init */
dpif_netlink_enumerate,
NULL,
diff --git a/lib/dpif-offload-dpdk.c b/lib/dpif-offload-dpdk.c
index 7a4cc7292..155f631b3 100644
--- a/lib/dpif-offload-dpdk.c
+++ b/lib/dpif-offload-dpdk.c
@@ -310,6 +310,7 @@ dpif_offload_dpdk_netdev_hw_miss_packet_postprocess(
struct dpif_offload_class dpif_offload_dpdk_class = {
.type = "dpdk",
+ .impl_type = DPIF_OFFLOAD_IMPL_SYNC,
.supported_dpif_types = (const char *const[]) {
"netdev",
NULL},
diff --git a/lib/dpif-offload-dummy.c b/lib/dpif-offload-dummy.c
index b7fff205d..cf4ac9df5 100644
--- a/lib/dpif-offload-dummy.c
+++ b/lib/dpif-offload-dummy.c
@@ -253,6 +253,7 @@ dpif_offload_dummy_can_offload(struct dpif_offload
*dpif_offload OVS_UNUSED,
#define DEFINE_DPIF_DUMMY_CLASS(NAME, TYPE_STR) \
struct dpif_offload_class NAME = { \
.type = TYPE_STR, \
+ .impl_type = DPIF_OFFLOAD_IMPL_HW_ONLY, \
.supported_dpif_types = (const char *const[]) { \
"dummy", \
NULL}, \
diff --git a/lib/dpif-offload-provider.h b/lib/dpif-offload-provider.h
index f47492106..bc6cb3574 100644
--- a/lib/dpif-offload-provider.h
+++ b/lib/dpif-offload-provider.h
@@ -18,6 +18,7 @@
#define DPIF_OFFLOAD_PROVIDER_H
#include "cmap.h"
+#include "dpif-offload.h"
#include "dpif-provider.h"
#include "ovs-thread.h"
#include "smap.h"
@@ -96,6 +97,9 @@ struct dpif_offload_class {
* 'struct dpif_class' definition. */
const char *const *supported_dpif_types;
+ /* Type of implementation for this DPIF offload provider. */
+ enum dpif_offload_impl_type impl_type;
+
/* Called when the dpif offload provider class is registered. Note that
* this is the global initialization, not the per dpif one. */
int (*init)(void);
diff --git a/lib/dpif-offload-tc.c b/lib/dpif-offload-tc.c
index 1c9483f26..e367a0ccd 100644
--- a/lib/dpif-offload-tc.c
+++ b/lib/dpif-offload-tc.c
@@ -560,6 +560,7 @@ dpif_offload_tc_flow_dump_thread_destroy(
struct dpif_offload_class dpif_offload_tc_class = {
.type = "tc",
+ .impl_type = DPIF_OFFLOAD_IMPL_HW_ONLY,
.supported_dpif_types = (const char *const[]) {
"system",
NULL},
diff --git a/lib/dpif-offload.c b/lib/dpif-offload.c
index 5eb9404c3..f519e3901 100644
--- a/lib/dpif-offload.c
+++ b/lib/dpif-offload.c
@@ -571,6 +571,26 @@ dpif_offload_port_set_config(struct dpif *dpif, odp_port_t
port_no,
}
}
+struct dpif_offload *
+dpif_offload_port_offloaded_by(const struct dpif *dpif, odp_port_t port_no)
+{
+ struct dp_offload *dp_offload = dpif_offload_get_dp_offload(dpif);
+ struct dpif_offload *offload, *offload_return = NULL;
+
+ if (!dp_offload || !dpif_offload_is_offload_enabled()) {
+ return NULL;
+ }
+
+ LIST_FOR_EACH (offload, dpif_list_node, &dp_offload->offload_providers) {
+ if (offload->class->get_netdev(offload, port_no)) {
+ offload_return = offload;
+ break;
+ }
+ }
+
+ return offload_return;
+}
+
void
dpif_offload_dump_start(struct dpif_offload_dump *dump,
const struct dpif *dpif)
@@ -749,6 +769,32 @@ dpif_offload_flow_flush(struct dpif *dpif)
}
}
+enum dpif_offload_impl_type
+dpif_offload_get_impl_type(const struct dpif_offload *offload)
+{
+ return offload->class->impl_type;
+}
+
+enum dpif_offload_impl_type
+dpif_offload_get_impl_type_by_class(const char *type)
+{
+ enum dpif_offload_impl_type impl_type = DPIF_OFFLOAD_IMPL_NONE;
+ struct shash_node *node;
+
+ ovs_mutex_lock(&dpif_offload_mutex);
+ SHASH_FOR_EACH (node, &dpif_offload_classes) {
+ const struct dpif_offload_class *class = node->data;
+
+ if (!strcmp(type, class->type)) {
+ impl_type = class->impl_type;
+ break;
+ }
+ }
+ ovs_mutex_unlock(&dpif_offload_mutex);
+
+ return impl_type;
+}
+
uint64_t
dpif_offload_flow_get_n_offloaded(const struct dpif *dpif)
{
@@ -769,6 +815,28 @@ dpif_offload_flow_get_n_offloaded(const struct dpif *dpif)
return flow_count;
}
+uint64_t
+dpif_offload_flow_get_n_offloaded_by_impl(const struct dpif *dpif,
+ enum dpif_offload_impl_type type)
+{
+ struct dp_offload *dp_offload = dpif_offload_get_dp_offload(dpif);
+ const struct dpif_offload *offload;
+ uint64_t flow_count = 0;
+
+ if (!dp_offload || !dpif_offload_is_offload_enabled()) {
+ return 0;
+ }
+
+ LIST_FOR_EACH (offload, dpif_list_node, &dp_offload->offload_providers) {
+ if (offload->class->flow_get_n_offloaded
+ && type == dpif_offload_get_impl_type(offload)) {
+ flow_count += offload->class->flow_get_n_offloaded(offload);
+ }
+ }
+
+ return flow_count;
+}
+
void
dpif_offload_meter_set(const struct dpif *dpif, ofproto_meter_id meter_id,
struct ofputil_meter_config *config)
diff --git a/lib/dpif-offload.h b/lib/dpif-offload.h
index 541b59679..29211c96a 100644
--- a/lib/dpif-offload.h
+++ b/lib/dpif-offload.h
@@ -30,6 +30,23 @@ struct dpif_offload_dump {
void *state;
};
+/* Definition of the DPIF offload implementation type.
+ *
+ * The 'DPIF_OFFLOAD_IMPL_SYNC' implementation has a single view, the offload
+ * provider is responsible for synchronizing flows and statistics through the
+ * dpif flow operations. An example of this is DPDK's rte_flow.
+ *
+ * The 'DPIF_OFFLOAD_IMPL_HW_ONLY' implementation tries to install a hardware
+ * flow first. If successful, no dpif-layer software flow will be installed.
+ * Offload-specific callbacks are then used to manage the flow and query
+ * statistics. An example of this is kernel TC.
+ */
+enum dpif_offload_impl_type {
+ DPIF_OFFLOAD_IMPL_NONE,
+ DPIF_OFFLOAD_IMPL_SYNC,
+ DPIF_OFFLOAD_IMPL_HW_ONLY,
+};
+
I don't understand neither the purpose of this property, nor the naming.
For naming, it is not "SYNC/HW_ONLY" but maybe "INTERNAL/EXTERNAL".
INTERNAL means the source of truth is OVS, and all the state is managed
by OVS, while EXTERNAL means it's in the kernel domain.
I can't actually think of another use-case that it won't be either
userspace/kernelspace.
Regarding purpose, it is not something that is changing, surely not in
the class level (per-flow, yes).
/* Global functions. */
void dpif_offload_set_global_cfg(const struct ovsrec_open_vswitch *);
@@ -52,6 +69,8 @@ bool dpif_offload_dump_next(struct dpif_offload_dump *,
struct dpif_offload **);
int dpif_offload_dump_done(struct dpif_offload_dump *);
uint64_t dpif_offload_flow_get_n_offloaded(const struct dpif *);
+uint64_t dpif_offload_flow_get_n_offloaded_by_impl(
+ const struct dpif *, enum dpif_offload_impl_type);
void dpif_offload_meter_set(const struct dpif *dpif, ofproto_meter_id
meter_id,
struct ofputil_meter_config *);
void dpif_offload_meter_get(const struct dpif *dpif, ofproto_meter_id
meter_id,
@@ -63,6 +82,12 @@ struct netdev *dpif_offload_get_netdev_by_port_id(struct
dpif *,
odp_port_t);
struct netdev *dpif_offload_offload_get_netdev_by_port_id(
struct dpif_offload *, odp_port_t);
+struct dpif_offload *dpif_offload_port_offloaded_by(const struct dpif *,
+ odp_port_t);
+enum dpif_offload_impl_type dpif_offload_get_impl_type(
+ const struct dpif_offload *);
+enum dpif_offload_impl_type dpif_offload_get_impl_type_by_class(
+ const char *type);
/* Iterates through each DPIF_OFFLOAD in DPIF, using DUMP as state.
*
diff --git a/lib/dpif-provider.h b/lib/dpif-provider.h
index 91e7ddcfa..02bcae12f 100644
--- a/lib/dpif-provider.h
+++ b/lib/dpif-provider.h
@@ -164,14 +164,6 @@ struct dpif_class {
* datapaths that can not exist without it (e.g. netdev datapath). */
bool cleanup_required;
- /* If 'true' the specific dpif implementation synchronizes the various
- * datapath implementation layers, i.e., the dpif's layer in combination
- * with the underlying netdev offload layers. For example, dpif-netlink
- * does not sync its kernel flows with the tc ones, i.e., only one gets
- * installed. On the other hand, dpif-netdev installs both flows,
- * internally keeps track of both, and represents them as one. */
- bool synced_dp_layers;
-
/* Called when the dpif provider is registered, typically at program
* startup. Returning an error from this function will prevent any
* datapath with this class from being created.
diff --git a/lib/dpif.c b/lib/dpif.c
index 5319d6b85..ddafa32c7 100644
--- a/lib/dpif.c
+++ b/lib/dpif.c
@@ -2137,9 +2137,3 @@ dpif_cache_set_size(struct dpif *dpif, uint32_t level,
uint32_t size)
? dpif->dpif_class->cache_set_size(dpif, level, size)
: EOPNOTSUPP;
}
-
-bool
-dpif_synced_dp_layers(struct dpif *dpif)
-{
- return dpif->dpif_class->synced_dp_layers;
-}
diff --git a/lib/dpif.h b/lib/dpif.h
index 473eacb2f..f3301ae85 100644
--- a/lib/dpif.h
+++ b/lib/dpif.h
@@ -943,7 +943,6 @@ char *dpif_get_dp_version(const struct dpif *);
bool dpif_supports_tnl_push_pop(const struct dpif *);
bool dpif_may_support_explicit_drop_action(const struct dpif *);
bool dpif_may_support_psample(const struct dpif *);
-bool dpif_synced_dp_layers(struct dpif *);
/* Log functions. */
struct vlog_module;
diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c
index 363314ad9..2aa77a4de 100644
--- a/ofproto/ofproto-dpif-upcall.c
+++ b/ofproto/ofproto-dpif-upcall.c
@@ -826,12 +826,8 @@ udpif_get_n_flows(struct udpif *udpif)
atomic_store_relaxed(&udpif->n_flows_timestamp, now);
dpif_get_dp_stats(udpif->dpif, &stats);
flow_count = stats.n_flows;
-
- if (!dpif_synced_dp_layers(udpif->dpif)) {
- /* If the dpif layer does not sync the flows, we need to include
- * the hardware offloaded flows separately. */
- flow_count += dpif_offload_flow_get_n_offloaded(udpif->dpif);
- }
+ flow_count += dpif_offload_flow_get_n_offloaded_by_impl(
+ udpif->dpif, DPIF_OFFLOAD_IMPL_HW_ONLY);
Why does ofproto layer cares about offload? It should be agnostic to it.
atomic_store_relaxed(&udpif->n_flows, flow_count);
ovs_mutex_unlock(&udpif->n_flows_mutex);
@@ -2802,6 +2798,21 @@ udpif_update_used(struct udpif *udpif, struct udpif_key
*ukey,
return stats->used;
}
+static bool
+did_dp_hw_only_offload_change(const char *old_type, const char *new_type)
+{
+ enum dpif_offload_impl_type old_impl = old_type ?
+ dpif_offload_get_impl_type_by_class(old_type) : DPIF_OFFLOAD_IMPL_NONE;
+ enum dpif_offload_impl_type new_impl = new_type ?
+ dpif_offload_get_impl_type_by_class(new_type) : DPIF_OFFLOAD_IMPL_NONE;
+
+ if (old_impl != new_impl && (old_impl == DPIF_OFFLOAD_IMPL_HW_ONLY ||
+ new_impl == DPIF_OFFLOAD_IMPL_HW_ONLY)) {
+ return true;
+ }
+ return false;
+}
+
static void
revalidate(struct revalidator *revalidator)
{
@@ -2900,9 +2911,8 @@ revalidate(struct revalidator *revalidator)
ukey->offloaded = f->attrs.offloaded;
if (!ukey->dp_layer
- || (!dpif_synced_dp_layers(udpif->dpif)
- && strcmp(ukey->dp_layer, f->attrs.dp_layer))) {
-
+ || did_dp_hw_only_offload_change(ukey->dp_layer,
+ f->attrs.dp_layer)) {
if (ukey->dp_layer) {
/* The dp_layer has changed this is probably due to an
* earlier revalidate cycle moving it to/from hw offload.
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
