From: Numan Siddique <[email protected]> netdev-offload-tc when offloading a flow to tc, uses the flow's recirc_id as flower chain id. recirc_id is of type 'uint32_t'. Kernel tc reserves the upper nibble (4 bits) of the tc flower's chain index for extended action opcode [1]. If a flow's recirc_id value is greater than 268,435,455 (0x0fffffff), kernel returns EINVAL when ovs-vswitchd tries to offload the flow.
This commit fixes this offload issue by setting the maximum value of recirc id to 268,435,455 when hw-offload is enabled. Note: This commit renames a static function - mirror_destroy() to mirror_delete() in vswitchd/bridge.c since a public function with the same name exists in ofproto/ofproto-dpif-mirror.c. [1] - https://elixir.bootlin.com/linux/v6.17.6/source/tools/include/uapi/linux/pkt_cls.h#L50-L64 Reported-at: https://mail.openvswitch.org/pipermail/ovs-dev/2025-November/427485.html Suggested-by: Ilya Maximets <[email protected]> Signed-off-by: Numan Siddique <[email protected]> --- lib/netdev-offload-provider.h | 4 ++++ lib/netdev-offload-tc.c | 6 ++++++ lib/netdev-offload.c | 23 +++++++++++++++++++++++ lib/netdev-offload.h | 1 + ofproto/ofproto-dpif-rid.c | 18 ++++++++++++++++-- ofproto/ofproto-dpif-rid.h | 2 ++ vswitchd/bridge.c | 12 +++++++----- 7 files changed, 59 insertions(+), 7 deletions(-) diff --git a/lib/netdev-offload-provider.h b/lib/netdev-offload-provider.h index 9108856d18..9def5858c7 100644 --- a/lib/netdev-offload-provider.h +++ b/lib/netdev-offload-provider.h @@ -121,6 +121,10 @@ struct netdev_flow_api { int (*meter_del)(ofproto_meter_id meter_id, struct ofputil_meter_stats *stats); + /* Returns the maximum value of the flow recirc_id which the provider can + * support, if it uses the flow recirc_id to offload. */ + uint32_t (*get_max_recirc_id)(void); + /* Initializies the netdev flow api. * Return 0 if successful, otherwise returns a positive errno value. */ int (*init_flow_api)(struct netdev *); diff --git a/lib/netdev-offload-tc.c b/lib/netdev-offload-tc.c index d4b6e72f0a..ba157ce2c8 100644 --- a/lib/netdev-offload-tc.c +++ b/lib/netdev-offload-tc.c @@ -3430,6 +3430,11 @@ meter_tc_del_policer(ofproto_meter_id meter_id, return err; } +static uint32_t +netdev_tc_get_max_recird_id(void) { + return TC_ACT_EXT_VAL_MASK; +} + const struct netdev_flow_api netdev_offload_tc = { .type = "linux_tc", .flow_flush = netdev_tc_flow_flush, @@ -3443,5 +3448,6 @@ const struct netdev_flow_api netdev_offload_tc = { .meter_set = meter_tc_set_policer, .meter_get = meter_tc_get_policer, .meter_del = meter_tc_del_policer, + .get_max_recirc_id = netdev_tc_get_max_recird_id, .init_flow_api = netdev_tc_init_flow_api, }; diff --git a/lib/netdev-offload.c b/lib/netdev-offload.c index 8a9d365559..c6c5ae486d 100644 --- a/lib/netdev-offload.c +++ b/lib/netdev-offload.c @@ -908,3 +908,26 @@ netdev_set_flow_api_enabled(const struct smap *ovs_other_config) } } } + +/* Returns the minimum of the max recirc id supported by each registered flow, + * API, if it supports get_max_recirc_id(). + * Returns 0, if none of the registered flow API supports get_max_recirc_id(). + */ +uint32_t +netdev_get_max_recird_id(void) +{ + uint32_t max_recirc_id = 0; + + struct netdev_registered_flow_api *rfa; + + CMAP_FOR_EACH (rfa, cmap_node, &netdev_flow_apis) { + if (rfa->flow_api->get_max_recirc_id) { + uint32_t rfa_max_recirc_id = rfa->flow_api->get_max_recirc_id(); + if (!max_recirc_id || rfa_max_recirc_id < max_recirc_id) { + max_recirc_id = rfa_max_recirc_id; + } + } + } + + return max_recirc_id; +} diff --git a/lib/netdev-offload.h b/lib/netdev-offload.h index 7fc30a4243..f62a6f5d94 100644 --- a/lib/netdev-offload.h +++ b/lib/netdev-offload.h @@ -127,6 +127,7 @@ bool netdev_is_flow_api_enabled(void); void netdev_set_flow_api_enabled(const struct smap *ovs_other_config); bool netdev_is_offload_rebalance_policy_enabled(void); int netdev_flow_get_n_flows(struct netdev *netdev, uint64_t *n_flows); +uint32_t netdev_get_max_recird_id(void); struct dpif_port; int netdev_ports_insert(struct netdev *, struct dpif_port *); diff --git a/ofproto/ofproto-dpif-rid.c b/ofproto/ofproto-dpif-rid.c index f01468025c..f7bef09c3d 100644 --- a/ofproto/ofproto-dpif-rid.c +++ b/ofproto/ofproto-dpif-rid.c @@ -36,6 +36,8 @@ static struct ovs_list expired OVS_GUARDED_BY(mutex) static uint32_t next_id OVS_GUARDED_BY(mutex) = 1; /* Possible next free id. */ +static uint32_t max_recirc_id; + #define RECIRC_POOL_STATIC_IDS 1024 static void recirc_id_node_free(struct recirc_id_node *); @@ -83,6 +85,16 @@ recirc_run(void) ovs_mutex_unlock(&mutex); } +/* Sets the max recirc id. Caller cannnot reduce the max recirc id once set. + */ +void +recirc_set_max_recirc_id(uint32_t max_recirc_id_) +{ + if (max_recirc_id_ > max_recirc_id) { + max_recirc_id = max_recirc_id_; + } +} + /* We use the id as the hash value, which works due to cmap internal rehashing. * We also only insert nodes with unique IDs, so all possible hash collisions * remain internal to the cmap. */ @@ -228,7 +240,8 @@ frozen_state_free(struct frozen_state *state) /* Allocate a unique recirculation id for the given set of flow metadata. * The ID space is 2^^32, so there should never be a situation in which all - * the IDs are used up. We loop until we find a free one. */ + * the IDs are used up. Although, the ID space could be limited to less + * than 2^^32, if max_recirc_id is set. We loop until we find a free one. */ static struct recirc_id_node * recirc_alloc_id__(const struct frozen_state *state, uint32_t hash) { @@ -247,7 +260,8 @@ recirc_alloc_id__(const struct frozen_state *state, uint32_t hash) RECIRC_POOL_STATIC_IDS IDs on the later rounds, though, as some of the initial allocations may be for long term uses (like bonds). */ node->id = next_id++; - if (OVS_UNLIKELY(!node->id)) { + if (OVS_UNLIKELY(!node->id || + (max_recirc_id && node->id >= max_recirc_id))) { next_id = RECIRC_POOL_STATIC_IDS + 1; node->id = next_id++; } diff --git a/ofproto/ofproto-dpif-rid.h b/ofproto/ofproto-dpif-rid.h index 4df630c62b..712f5b81e8 100644 --- a/ofproto/ofproto-dpif-rid.h +++ b/ofproto/ofproto-dpif-rid.h @@ -212,6 +212,8 @@ static inline bool recirc_id_node_try_ref_rcu(const struct recirc_id_node *n_) void recirc_id_node_unref(const struct recirc_id_node *); +void recirc_set_max_recirc_id(uint32_t max_recirc_id_); + void recirc_run(void); /* Recirculation IDs on which references are held. */ diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c index 475eefefaa..c00ee4b7c9 100644 --- a/vswitchd/bridge.c +++ b/vswitchd/bridge.c @@ -43,6 +43,7 @@ #include "odp-execute.h" #include "ofproto/bond.h" #include "ofproto/ofproto.h" +#include "ofproto/ofproto-dpif-rid.h" #include "openvswitch/dynamic-string.h" #include "openvswitch/list.h" #include "openvswitch/meta-flow.h" @@ -328,7 +329,7 @@ static void run_system_stats(void); static void bridge_configure_mirrors(struct bridge *); static struct mirror *mirror_create(struct bridge *, const struct ovsrec_mirror *); -static void mirror_destroy(struct mirror *); +static void mirror_delete(struct mirror *); static bool mirror_configure(struct mirror *); static void mirror_refresh_stats(struct mirror *); @@ -3398,6 +3399,7 @@ bridge_run(void) netdev_set_flow_api_enabled(&cfg->other_config); dpdk_init(&cfg->other_config); userspace_tso_init(&cfg->other_config); + recirc_set_max_recirc_id(netdev_get_max_recird_id()); } /* Initialize the ofproto library. This only needs to run once, but @@ -3708,7 +3710,7 @@ bridge_destroy(struct bridge *br, bool del) port_destroy(port); } HMAP_FOR_EACH_SAFE (mirror, hmap_node, &br->mirrors) { - mirror_destroy(mirror); + mirror_delete(mirror); } hmap_remove(&all_bridges, &br->node); @@ -5153,7 +5155,7 @@ bridge_configure_mirrors(struct bridge *br) atom.uuid = m->uuid; if (!ovsdb_datum_find_key(mc, &atom, OVSDB_TYPE_UUID, NULL)) { - mirror_destroy(m); + mirror_delete(m); } } @@ -5166,7 +5168,7 @@ bridge_configure_mirrors(struct bridge *br) } m->cfg = cfg; if (!mirror_configure(m)) { - mirror_destroy(m); + mirror_delete(m); } } @@ -5192,7 +5194,7 @@ mirror_create(struct bridge *br, const struct ovsrec_mirror *cfg) } static void -mirror_destroy(struct mirror *m) +mirror_delete(struct mirror *m) { if (m) { struct bridge *br = m->bridge; -- 2.52.0 _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
