On 9 Dec 2025, at 10:44, Eli Britstein wrote:
>> -----Original Message-----
>> From: dev <[email protected]> On Behalf Of Eelco Chaudron
>> Sent: Tuesday, 2 December 2025 16:05
>> To: [email protected]
>> Subject: [ovs-dev] [PATCH v2 06/41] dpif-offload: Add offload provider
>> set_config API.
>>
>> This patch introduces an API that allows offload providers to manage global
>> configurations. It also moves the 'n-offload-threads'
>> and 'tc-policy' setting to the appropriate providers using this new API.
>>
>> Signed-off-by: Eelco Chaudron <[email protected]>
>> ---
>> lib/dpif-offload-dpdk.c | 84 ++++++++++++++++++++++++++++++++++---
>> lib/dpif-offload-provider.h | 18 +++++++-
>> lib/dpif-offload-tc.c | 52 +++++++++++++++++++++--
>> lib/dpif-offload.c | 18 ++++++++
>> lib/dpif.c | 1 +
>> lib/netdev-offload.c | 37 ++++++----------
>> tests/dpif-netdev.at | 6 +--
>> tests/ofproto-macros.at | 2 +-
>> 8 files changed, 178 insertions(+), 40 deletions(-)
>>
>> diff --git a/lib/dpif-offload-dpdk.c b/lib/dpif-offload-dpdk.c index
>> 03c8f6492..6ac4af8d8 100644
>> --- a/lib/dpif-offload-dpdk.c
>> +++ b/lib/dpif-offload-dpdk.c
>> @@ -20,21 +20,84 @@
>> #include "dpif-offload-provider.h"
>> #include "util.h"
>>
>> +#include "openvswitch/vlog.h"
>> +
>> +VLOG_DEFINE_THIS_MODULE(dpif_offload_dpdk);
>> +
>> +#define DEFAULT_OFFLOAD_THREAD_NB 1
>> +#define MAX_OFFLOAD_THREAD_NB 10
>> +
>> +static unsigned int offload_thread_nb = DEFAULT_OFFLOAD_THREAD_NB;
>> +
>> +/* dpif offload interface for the dpdk rte_flow implementation. */
>> +struct dpif_offload_dpdk {
>> + struct dpif_offload offload;
>> +
>> + /* Configuration specific variables. */
>> + struct ovsthread_once once_enable; /* Track first-time enablement.
>> +*/ };
>> +
>> +static struct dpif_offload_dpdk *
>> +dpif_offload_dpdk_cast(const struct dpif_offload *offload) {
>> + dpif_offload_assert_class(offload, &dpif_offload_dpdk_class);
>> + return CONTAINER_OF(offload, struct dpif_offload_dpdk, offload); }
>> +
>> static int
>> dpif_offload_dpdk_open(const struct dpif_offload_class *offload_class,
>> - struct dpif *dpif, struct dpif_offload
>> **dpif_offload)
>> + struct dpif *dpif, struct dpif_offload
>> + **offload_)
> Change the name of the argument already in the first commit introduced it.
Done
>> {
>> - struct dpif_offload *offload = xmalloc(sizeof(struct dpif_offload));
>> + struct dpif_offload_dpdk *offload;
>> +
>> + offload = xmalloc(sizeof(struct dpif_offload_dpdk));
>> +
>> + dpif_offload_init(&offload->offload, offload_class, dpif);
>> + offload->once_enable = (struct ovsthread_once)
>> + OVSTHREAD_ONCE_INITIALIZER;
>>
>> - dpif_offload_init(offload, offload_class, dpif);
>> - *dpif_offload = offload;
>> + *offload_ = &offload->offload;
>> return 0;
>> }
>>
>> static void
>> -dpif_offload_dpdk_close(struct dpif_offload *dpif_offload)
>> +dpif_offload_dpdk_close(struct dpif_offload *offload_)
>> {
>> - free(dpif_offload);
>> + struct dpif_offload_dpdk *offload =
>> + dpif_offload_dpdk_cast(offload_);
>> +
>> + free(offload);
>> +}
>> +
>> +static void
>> +dpif_offload_dpdk_set_config(struct dpif_offload *offload_,
>> + const struct smap *other_cfg) {
>> + struct dpif_offload_dpdk *offload =
>> +dpif_offload_dpdk_cast(offload_);
>> +
>> + /* We maintain the existing behavior where global configurations
>> + * are only accepted when hardware offload is initially enabled.
>> + * Once enabled, they cannot be updated or reconfigured. */
>> + if (smap_get_bool(other_cfg, "hw-offload", false)) {
>> + if (ovsthread_once_start(&offload->once_enable)) {
>> +
>> + offload_thread_nb = smap_get_ullong(other_cfg,
>> + "n-offload-threads",
>> + DEFAULT_OFFLOAD_THREAD_NB);
>> + if (offload_thread_nb == 0 ||
>> + offload_thread_nb > MAX_OFFLOAD_THREAD_NB) {
>> + VLOG_WARN("netdev: Invalid number of threads requested: %u",
>> + offload_thread_nb);
>> + offload_thread_nb = DEFAULT_OFFLOAD_THREAD_NB;
>> + }
>> +
>> + if (smap_get(other_cfg, "n-offload-threads")) {
>> + VLOG_INFO("Flow API using %u thread%s",
>> + offload_thread_nb,
>> + offload_thread_nb > 1 ? "s" : "");
>> + }
>> +
>> + ovsthread_once_done(&offload->once_enable);
>> + }
>> + }
>> }
>>
>> struct dpif_offload_class dpif_offload_dpdk_class = { @@ -44,4 +107,13 @@
>> struct dpif_offload_class dpif_offload_dpdk_class = {
>> NULL},
>> .open = dpif_offload_dpdk_open,
>> .close = dpif_offload_dpdk_close,
>> + .set_config = dpif_offload_dpdk_set_config,
>> };
>> +
>> +/* XXX: Temporary functions below, which will be removed once fully
>> + * refactored. */
>> +unsigned int dpif_offload_dpdk_get_thread_nb(void);
>> +unsigned int dpif_offload_dpdk_get_thread_nb(void)
>> +{
>> + return offload_thread_nb;
>> +}
>> diff --git a/lib/dpif-offload-provider.h b/lib/dpif-offload-provider.h index
>> 73bb1a57c..09a7dfc2c 100644
>> --- a/lib/dpif-offload-provider.h
>> +++ b/lib/dpif-offload-provider.h
>> @@ -19,6 +19,8 @@
>>
>> #include "dpif-provider.h"
>> #include "ovs-thread.h"
>> +#include "smap.h"
>> +#include "util.h"
>> #include "openvswitch/list.h"
>>
>> /* The DPIF Offload Provider introduces an abstraction layer for hardware @@
>> -84,6 +86,11 @@ struct dpif_offload_class {
>> * open() above. If your implementation accesses this provider using
>> * RCU pointers, it's responsible for handling deferred deallocation. */
>> void (*close)(struct dpif_offload *);
>> + /* Pass custom configuration options to the offload provider. The
>> + * implementation might postpone applying the changes until run() is
>> + * called. */
>> + void (*set_config)(struct dpif_offload *,
>> + const struct smap *other_config);
>> };
>>
>>
>> @@ -92,8 +99,17 @@ extern struct dpif_offload_class
>> dpif_offload_dummy_x_class; extern struct dpif_offload_class
>> dpif_offload_dpdk_class; extern struct dpif_offload_class
>> dpif_offload_tc_class;
>>
>> -/* Global function, called by the dpif layer. */
>> +
>> +/* Global functions, called by the dpif layer or offload providers. */
>> void dp_offload_initialize(void);
>> +void dpif_offload_set_config(struct dpif *, const struct smap
>> +*other_cfg);
>> +
>> +static inline void dpif_offload_assert_class(
>> + const struct dpif_offload *dpif_offload,
>> + const struct dpif_offload_class *dpif_offload_class) {
>> + ovs_assert(dpif_offload->class == dpif_offload_class); }
>>
>>
>> #endif /* DPIF_OFFLOAD_PROVIDER_H */
>> diff --git a/lib/dpif-offload-tc.c b/lib/dpif-offload-tc.c
>> index f5d62566e..c09530daa 100644
>> --- a/lib/dpif-offload-tc.c
>> +++ b/lib/dpif-offload-tc.c
>> @@ -19,22 +19,65 @@
>> #include "dpif-offload.h"
>> #include "dpif-offload-provider.h"
>> #include "util.h"
>> +#include "tc.h"
> Move line up.
Done
>> +
>> +/* dpif offload interface for the tc implementation. */
>> +struct dpif_offload_tc {
>> + struct dpif_offload offload;
>> +
>> + /* Configuration specific variables. */
>> + struct ovsthread_once once_enable; /* Track first-time enablement. */
>> +};
>> +
>> +static struct dpif_offload_tc *
>> +dpif_offload_tc_cast(const struct dpif_offload *offload)
>> +{
>> + dpif_offload_assert_class(offload, &dpif_offload_tc_class);
>> + return CONTAINER_OF(offload, struct dpif_offload_tc, offload);
>> +}
>>
>> static int
>> dpif_offload_tc_open(const struct dpif_offload_class *offload_class,
>> struct dpif *dpif, struct dpif_offload **dpif_offload)
>> {
>> - struct dpif_offload *offload = xmalloc(sizeof(struct dpif_offload));
>> + struct dpif_offload_tc *offload_tc;
>> +
>> + offload_tc = xmalloc(sizeof(struct dpif_offload_tc));
>> +
>> + dpif_offload_init(&offload_tc->offload, offload_class, dpif);
>> + offload_tc->once_enable = (struct ovsthread_once) \
>> + OVSTHREAD_ONCE_INITIALIZER;
>>
>> - dpif_offload_init(offload, offload_class, dpif);
>> - *dpif_offload = offload;
>> + *dpif_offload = &offload_tc->offload;
>> return 0;
>> }
>>
>> static void
>> dpif_offload_tc_close(struct dpif_offload *dpif_offload)
>> {
>> - free(dpif_offload);
>> + struct dpif_offload_tc *offload_tc = dpif_offload_tc_cast(dpif_offload);
>> +
>> + free(offload_tc);
>> +}
>> +
>> +static void
>> +dpif_offload_tc_set_config(struct dpif_offload *offload,
>> + const struct smap *other_cfg)
>> +{
>> + struct dpif_offload_tc *offload_tc = dpif_offload_tc_cast(offload);
>> +
>> + /* We maintain the existing behavior where global configurations
>> + * are only accepted when hardware offload is initially enabled.
>> + * Once enabled, they cannot be updated or reconfigured. */
>> + if (smap_get_bool(other_cfg, "hw-offload", false)) {
>> + if (ovsthread_once_start(&offload_tc->once_enable)) {
>> +
>> + tc_set_policy(smap_get_def(other_cfg, "tc-policy",
>> + TC_POLICY_DEFAULT));
>> +
>> + ovsthread_once_done(&offload_tc->once_enable);
>> + }
>> + }
>> }
>>
>> struct dpif_offload_class dpif_offload_tc_class = {
>> @@ -44,4 +87,5 @@ struct dpif_offload_class dpif_offload_tc_class = {
>> NULL},
>> .open = dpif_offload_tc_open,
>> .close = dpif_offload_tc_close,
>> + .set_config = dpif_offload_tc_set_config,
>> };
>> diff --git a/lib/dpif-offload.c b/lib/dpif-offload.c
>> index 2d734beb0..4c89ae443 100644
>> --- a/lib/dpif-offload.c
>> +++ b/lib/dpif-offload.c
>> @@ -355,6 +355,23 @@ dpif_offload_detach_providers(struct dpif *dpif)
>> }
>> }
>>
>> +void
>> +dpif_offload_set_config(struct dpif *dpif, const struct smap *other_cfg)
>> +{
>> + struct dp_offload *dp_offload = dpif_offload_get_dp_offload(dpif);
>> + struct dpif_offload *offload;
>> +
>> + if (!dp_offload) {
>> + return;
>> + }
>> +
>> + LIST_FOR_EACH (offload, dpif_list_node, &dp_offload->offload_providers)
>> {
>> + if (offload->class->set_config) {
>> + offload->class->set_config(offload, other_cfg);
>> + }
>> + }
>> +}
>> +
>>
>>
>>
>> void
>> dpif_offload_init(struct dpif_offload *offload,
>> @@ -520,6 +537,7 @@ dpif_offload_set_global_cfg(const struct smap
>> *other_cfg)
>>
>> if (ovsthread_once_start(&once_enable)) {
>> atomic_store_relaxed(&dpif_offload_global_enabled, true);
>> + VLOG_INFO("hw-offload API Enabled");
> Can we move it to the commit introduced this code?
It is in this file because we remove the old message here and replace it with
the new one.
We could add it earlier, but that would complicate the test changes, and it
would be reverted later anyway.
>>
>> if (smap_get_bool(other_cfg, "offload-rebalance", false)) {
>> atomic_store_relaxed(&dpif_offload_rebalance_policy, true);
>> diff --git a/lib/dpif.c b/lib/dpif.c
>> index 91fa00888..4d4b5127b 100644
>> --- a/lib/dpif.c
>> +++ b/lib/dpif.c
>> @@ -1594,6 +1594,7 @@ dpif_set_config(struct dpif *dpif, const struct smap
>> *cfg)
>> if (error) {
>> log_operation(dpif, "set_config", error);
>> }
>> + dpif_offload_set_config(dpif, cfg);
>> }
>>
>> return error;
>> diff --git a/lib/netdev-offload.c b/lib/netdev-offload.c
>> index 01fdadbc3..0fad1b983 100644
>> --- a/lib/netdev-offload.c
>> +++ b/lib/netdev-offload.c
>> @@ -61,10 +61,16 @@ VLOG_DEFINE_THIS_MODULE(netdev_offload);
>>
>> static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
>>
>> -#define DEFAULT_OFFLOAD_THREAD_NB 1
>> +/* XXX: Temporarily duplicates definition in dpif-offload-dpdk.c. */
>> #define MAX_OFFLOAD_THREAD_NB 10
>> -
>> +#define DEFAULT_OFFLOAD_THREAD_NB 1
>> static unsigned int offload_thread_nb = DEFAULT_OFFLOAD_THREAD_NB;
>> +
>> +unsigned int dpif_offload_dpdk_get_thread_nb(void); /* XXX: Temporarily
>> + * external declaration
>> + * until fully
>> refactored.
>> + */
>> +
>> DEFINE_EXTERN_PER_THREAD_DATA(netdev_offload_thread_id,
>> OVSTHREAD_ID_UNSET);
>>
>> /* Protects 'netdev_flow_apis'. */
>> @@ -853,37 +859,18 @@ netdev_ports_flow_init(void)
>> }
>>
>> void
>> -netdev_set_flow_api_enabled(const struct smap *ovs_other_config)
>> +netdev_set_flow_api_enabled(const struct smap *ovs_other_config
>> OVS_UNUSED)
>> {
>> if (dpif_offload_is_offload_enabled()) {
>> static struct ovsthread_once once = OVSTHREAD_ONCE_INITIALIZER;
>>
>> if (ovsthread_once_start(&once)) {
>>
>> - offload_thread_nb = smap_get_ullong(ovs_other_config,
>> - "n-offload-threads",
>> - DEFAULT_OFFLOAD_THREAD_NB);
>> - if (offload_thread_nb == 0 ||
>> - offload_thread_nb > MAX_OFFLOAD_THREAD_NB) {
>> - VLOG_WARN("netdev: Invalid number of threads requested: %u",
>> - offload_thread_nb);
>> - offload_thread_nb = DEFAULT_OFFLOAD_THREAD_NB;
>> - }
>> -
>> - if (smap_get(ovs_other_config, "n-offload-threads")) {
>> - VLOG_INFO("netdev: Flow API Enabled, using %u thread%s",
>> - offload_thread_nb,
>> - offload_thread_nb > 1 ? "s" : "");
>> - } else {
>> - VLOG_INFO("netdev: Flow API Enabled");
>> - }
>> -
>> -#ifdef __linux__
>> - tc_set_policy(smap_get_def(ovs_other_config, "tc-policy",
>> - TC_POLICY_DEFAULT));
>> +#ifdef DPDK_NETDEV
>> + offload_thread_nb = dpif_offload_dpdk_get_thread_nb();
>> #endif
>> - netdev_ports_flow_init();
>>
>> + netdev_ports_flow_init();
>> ovsthread_once_done(&once);
>> }
>> }
>> diff --git a/tests/dpif-netdev.at b/tests/dpif-netdev.at
>> index 10dbbb4c7..6596af810 100644
>> --- a/tests/dpif-netdev.at
>> +++ b/tests/dpif-netdev.at
>> @@ -414,7 +414,7 @@ m4_define([DPIF_NETDEV_FLOW_HW_OFFLOAD],
>> AT_CHECK([ovs-appctl vlog/set dpif:file:dbg dpif_netdev:file:dbg
>> netdev_dummy:file:dbg])
>>
>> AT_CHECK([ovs-vsctl set Open_vSwitch . other_config:hw-offload=true])
>> - OVS_WAIT_UNTIL([grep "netdev: Flow API Enabled" ovs-vswitchd.log])
>> + OVS_WAIT_UNTIL([grep "hw-offload API Enabled" ovs-vswitchd.log])
>>
>> AT_CHECK([ovs-ofctl del-flows br0])
>> AT_CHECK([ovs-ofctl add-flow br0 in_port=1,actions=IN_PORT])
>> @@ -477,7 +477,7 @@
>> m4_define([DPIF_NETDEV_FLOW_HW_OFFLOAD_OFFSETS],
>> AT_CHECK([ovs-appctl vlog/set dpif:file:dbg dpif_netdev:file:dbg
>> netdev_dummy:file:dbg])
>>
>> AT_CHECK([ovs-vsctl set Open_vSwitch . other_config:hw-offload=true])
>> - OVS_WAIT_UNTIL([grep "netdev: Flow API Enabled" ovs-vswitchd.log])
>> + OVS_WAIT_UNTIL([grep "hw-offload API Enabled" ovs-vswitchd.log])
>>
>> AT_CHECK([ovs-ofctl del-flows br0])
>>
>> @@ -554,7 +554,7 @@
>> m4_define([DPIF_NETDEV_FLOW_HW_OFFLOAD_OFFSETS_VID_ARP],
>> AT_CHECK([ovs-appctl vlog/set dpif:file:dbg dpif_netdev:file:dbg
>> netdev_dummy:file:dbg])
>>
>> AT_CHECK([ovs-vsctl set Open_vSwitch . other_config:hw-offload=true])
>> - OVS_WAIT_UNTIL([grep "netdev: Flow API Enabled" ovs-vswitchd.log])
>> + OVS_WAIT_UNTIL([grep "hw-offload API Enabled" ovs-vswitchd.log])
>>
>> AT_CHECK([ovs-ofctl del-flows br0])
>>
>> diff --git a/tests/ofproto-macros.at b/tests/ofproto-macros.at
>> index 71cd2aed0..8894dd711 100644
>> --- a/tests/ofproto-macros.at
>> +++ b/tests/ofproto-macros.at
>> @@ -224,7 +224,7 @@ m4_define([_OVS_VSWITCHD_START],
>> /ofproto|INFO|datapath ID changed to fedcba9876543210/d
>> /dpdk|INFO|DPDK Disabled - Use other_config:dpdk-init to enable/d
>> /netlink_socket|INFO|netlink: could not enable listening to all nsid/d
>> -/netdev_offload|INFO|netdev: Flow API Enabled/d
>> +/dpif_offload|INFO|hw-offload API Enabled/d
>> /probe tc:/d
>> /setting extended ack support failed/d
>> /tc: Using policy/d
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
