On 22 Jan 2026, at 12:18, Ilya Maximets wrote:
> On 1/21/26 10:27 AM, Eelco Chaudron wrote:
>> The original series allowed the provider priority to be configured
>> on the port using the other-config:hw-offload-priority option.
>>
>> However, this is not the correct level. The configuration should be
>> applied at the interface level, allowing, for example, specific bond
>> members to have different configurations.
>>
>> This patch moves the configuration from the port to the interface.
>>
>> Fixes: 5c9b96ed5abd ("dpif-offload: Allow per-port offload provider priority
>> config.")
>> Signed-off-by: Eelco Chaudron <[email protected]>
Thanks, Ilya, for reviewing this. I’ll address all your comments in v2 (out
soon).
//Eelco
>> ---
>> NEWS | 7 +++---
>> lib/dpif-offload.c | 23 ++++++++++++--------
>> tests/ofproto-dpif.at | 8 +++----
>> vswitchd/vswitch.xml | 50 ++++++++++++++++++++++---------------------
>> 4 files changed, 48 insertions(+), 40 deletions(-)
>>
>> diff --git a/NEWS b/NEWS
>> index 3ff5b3633b..d648c54dde 100644
>> --- a/NEWS
>> +++ b/NEWS
>> @@ -27,9 +27,10 @@ v3.7.0 - xx xxx xxxx
>> * New global configuration option 'other-config:hw-offload-priority'
>> that allows to set the order of the hardware offload providers
>> to try when multiple exist for a given datapath implementation.
>> - * New per-port configuration option 'other-config:hw-offload-priority'
>> - that allows to set order of the hardware offload providers or disable
>> - flow offloading on a particular port.
>> + * New per-interface configuration option
>> + 'other-config:hw-offload-priority' that allows to set order of the
>> + hardware offload providers or disable flow offloading on a particular
>> + port.
>
> On a particular interface?
Ack
>> - OVSDB-IDL:
>> * New ovsdb_idl_txn_assert_read_only() interface to mark transactions
>> as read-only and trigger assertion failure when application attempts
>> diff --git a/lib/dpif-offload.c b/lib/dpif-offload.c
>> index f6ab65e980..9bea07fa02 100644
>> --- a/lib/dpif-offload.c
>> +++ b/lib/dpif-offload.c
>> @@ -59,7 +59,7 @@ static const struct dpif_offload_class
>> *base_dpif_offload_classes[] = {
>> static char *dpif_offload_provider_priority_list = NULL;
>> static atomic_bool offload_global_enabled = false;
>> static atomic_bool offload_rebalance_policy = false;
>> -static struct smap port_order_cfg = SMAP_INITIALIZER(&port_order_cfg);
>> +static struct smap iface_order_cfg = SMAP_INITIALIZER(&iface_order_cfg);
>>
>> static int
>> dpif_offload_register_provider__(const struct dpif_offload_class *class)
>> @@ -527,7 +527,7 @@ dpif_offload_port_add(struct dpif *dpif, struct netdev
>> *netdev,
>> odp_port_t port_no)
>> {
>> struct dpif_offload_provider_collection *collection;
>> - const char *port_priority = smap_get(&port_order_cfg,
>> + const char *port_priority = smap_get(&iface_order_cfg,
>> netdev_get_name(netdev));
>> struct dpif_offload *offload;
>>
>> @@ -773,21 +773,26 @@ dpif_offload_set_global_cfg(const struct
>> ovsrec_open_vswitch *cfg)
>> }
>> }
>>
>> - /* Filter out the 'hw-offload-priority' per port setting we need it
>> before
>> - * ports are added, so we can assign the correct offload-provider.
>> + /* Filter out the 'hw-offload-priority' per interface setting we need it
>> + * before ports are added, so we can assign the correct
>> offload-provider.
>> * Note that we can safely rebuild the map here, as we only access this
>> * from the same (main) thread. */
>> - smap_clear(&port_order_cfg);
>> + smap_clear(&iface_order_cfg);
>> for (int i = 0; i < cfg->n_bridges; i++) {
>> const struct ovsrec_bridge *br_cfg = cfg->bridges[i];
>>
>> for (int j = 0; j < br_cfg->n_ports; j++) {
>> const struct ovsrec_port *port_cfg = br_cfg->ports[j];
>>
>> - priority = smap_get(&port_cfg->other_config,
>> - "hw-offload-priority");
>> - if (priority) {
>> - smap_add(&port_order_cfg, port_cfg->name, priority);
>> + for (int k = 0; k < port_cfg->n_interfaces; k++) {
>> + const struct ovsrec_interface *iface_cfg
>> + = port_cfg->interfaces[k];
>
> nit: Might be better to split the definition and the assignment to avoid
> this long line.
Ack
>> +
>> + priority = smap_get(&iface_cfg->other_config,
>> + "hw-offload-priority");
>> + if (priority) {
>> + smap_add(&iface_order_cfg, iface_cfg->name, priority);
>> + }
>> }
>> }
>> }
>> diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at
>> index ff628c362d..15dbfd60cf 100644
>> --- a/tests/ofproto-dpif.at
>> +++ b/tests/ofproto-dpif.at
>> @@ -10242,16 +10242,16 @@ AT_SETUP([ofproto-dpif - offload - port priority
>> order])
>> AT_KEYWORDS([dpif-offload])
>> OVS_VSWITCHD_START([add-port br0 p1 -- \
>> set Interface p1 type=dummy ofport_request=1 -- \
>> - set port p1 other_config:hw-offload-priority=dummy_x,dummy -- \
>> + set Interface p1 other_config:hw-offload-priority=dummy_x,dummy -- \
>> add-port br0 p2 -- \
>> set Interface p2 type=dummy ofport_request=2 -- \
>> - set port p2 other_config:hw-offload-priority=none -- \
>> + set Interface p2 other_config:hw-offload-priority=none -- \
>> add-port br0 p3 -- \
>> set Interface p3 type=dummy ofport_request=3 -- \
>> - set port p3 other_config:hw-offload-priority=dummy_x -- \
>> + set Interface p3 other_config:hw-offload-priority=dummy_x -- \
>> add-port br0 p4 -- \
>> set Interface p4 type=dummy ofport_request=4 -- \
>> - set port p4 other_config:hw-offload-priority=dummy])
>> + set Interface p4 other_config:hw-offload-priority=dummy])
>>
>> AT_CHECK([ovs-appctl --format json --pretty dpif/offload/show], [0], [dnl
>> {
>> diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml
>> index 29ad9c13e5..53b6a08c53 100644
>> --- a/vswitchd/vswitch.xml
>> +++ b/vswitchd/vswitch.xml
>> @@ -2600,30 +2600,6 @@
>> e.g. <code>fake-bridge-bridge-id</code>.
>> </column>
>>
>> - <column name="other_config" key="hw-offload-priority"
>> - type='{"type": "string"}'>
>> - <p>
>> - This configuration sets an explicit list of hardware offload
>> - providers to try on this port. The argument should be a
>> - comma-separated list of hardware offload provider names, or the
>> - word <code>none</code>.
>> - </p>
>> - <p>
>> - If <code>none</code> is encountered in the list, further trying of
>> - offload providers is stopped. For example, if the list only
>> contains
>> - <code>none</code>, hardware offload is disabled on this port even
>> if
>> - it is globally enabled. Note that unknown hardware offload
>> provider
>> - names are ignored.
>> - </p>
>> - <p>
>> - If not set, uses the global
>> - <ref table="Open_vSwitch" column="other_config"
>> - key="hw-offload-priority"/>
>> - configuration. Changing this value after offload enablement
>> requires
>> - restarting the daemon.
>> - </p>
>> - </column>
>> -
>> <column name="other_config" key="transient"
>> type='{"type": "boolean"}'>
>> <p>
>> @@ -4681,6 +4657,32 @@ ovs-vsctl add-port br0 p0 -- set Interface p0
>> type=patch options:peer=p1 \
>> </column>
>> </group>
>>
>> + <group title="Hardware Offload Configuration">
>> + <column name="other_config" key="hw-offload-priority"
>> + type='{"type": "string"}'>
>> + <p>
>> + This configuration sets an explicit list of hardware offload
>> + providers to try on this Interface. The argument should be a
>> + comma-separated list of hardware offload provider names, or the
>> + word <code>none</code>.
>> + </p>
>> + <p>
>> + If <code>none</code> is encountered in the list, further trying of
>> + offload providers is stopped. For example, if the list only
>> contains
>> + <code>none</code>, hardware offload is disabled on this interface
>> + even if it is globally enabled. Note that unknown hardware
>> offload
>> + provider names are ignored.
>> + </p>
>> + <p>
>> + If not set, uses the global
>> + <ref table="Open_vSwitch" column="other_config"
>> + key="hw-offload-priority"/>
>> + configuration. Changing this value after offload enablement
>> requires
>> + restarting the daemon.
>
> Not necessarily in this patch (but can be in this one as well), but we
> probably
> should specify how this works when a new port is added while the hw offload is
> already enabled. It seems the requirement in this case is that the config
> should
> be set in the same transaction that adds the port.
Will add some text in the v2.
>> + </p>
>> + </column>
>> + </group>
>> +
>> <group title="Common Columns">
>> The overall purpose of these columns is described under <code>Common
>> Columns</code> at the beginning of this document.
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
