On Mon, Jan 12, 2026 at 8:28 AM Eelco Chaudron <[email protected]> wrote: > > > > On 12 Jan 2026, at 14:26, Eelco Chaudron wrote: > > > On 20 Dec 2025, at 0:50, [email protected] wrote: > > > >> From: Numan Siddique <[email protected]> > > Forgot to mention the subject needs a component: > > ofproto-dpif: Limit the recirc_id to max of 2^28. > > >> 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 (2^28), kernel returns > > > > (2^28 - 1)
Ack. > > > >> EINVAL when ovs-vswitchd tries to offload the flow. > >> > >> This commit fixes this offload issue by setting the maximum > >> value of recirc id to 2^28. 2^28 is a fairly big number and > > > > value of recirc id to 2^28 - 1. 2^28 - 1 is a fairly big number and > > > >> we should not have that many active datapath flows at a time. > >> > >> Reported-at: > >> https://mail.openvswitch.org/pipermail/ovs-dev/2025-November/427485.html > >> Suggested-by: Ilya Maximets <[email protected]> > >> Suggested-by: Eelco Chaudron <[email protected]> > >> Signed-off-by: Numan Siddique <[email protected]> > >> --- > >> ofproto/ofproto-dpif-rid.c | 13 ++++++++++--- > >> 1 file changed, 10 insertions(+), 3 deletions(-) > >> > >> diff --git a/ofproto/ofproto-dpif-rid.c b/ofproto/ofproto-dpif-rid.c > >> index f01468025..09f8a84f1 100644 > >> --- a/ofproto/ofproto-dpif-rid.c > >> +++ b/ofproto/ofproto-dpif-rid.c > >> @@ -38,6 +38,13 @@ static uint32_t next_id OVS_GUARDED_BY(mutex) = 1; /* > >> Possible next free id. */ > >> > >> #define RECIRC_POOL_STATIC_IDS 1024 > >> > >> +/* Limit the recirc_ic maximum value to 268435455 (which is 2^^28). > > > > recirc_ic -> recirc_id > > > > Guess this is not correct, it should be 2^^28 - 1 (same below). > > > >> + * When kernel tc offload is enabled, we use the recirc_id as > >> + * the chain id and kernel limits the chain id maximum value > >> + * to 2^^28. > >> + */ > >> +#define RECIRC_ID_MAX_VALUE 268435455 > > > > Would it make sense to use TC_ACT_EXT_VAL_MASK here? Not 100% sure about > > this, Ilya? We could. I thought about it. I was not sure if its ok to include "include/uapi/linux/pkt_cls.h" (or appropriate linux header file) in ofproto/ofprot-dpif-rid.c. Is that ok ? > > > >> + > >> static void recirc_id_node_free(struct recirc_id_node *); > >> > >> /* This should be called by the revalidator once at each round (every > >> 500ms or > >> @@ -227,8 +234,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 ID space is limited to 2^^28, so there should never be a situation > > > > 2^^28 - 1, should we maybe also move to use a single carat for power of? > > Other comments in the code use a single caret. I used double carat because the existing comment used double carat (i.e 2^^32). I can change it to single. Please let me know. Thanks Numan > > > >> + * in which all the IDs are used up. 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 +254,7 @@ 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 || node->id > RECIRC_ID_MAX_VALUE)) { > >> next_id = RECIRC_POOL_STATIC_IDS + 1; > >> node->id = next_id++; > >> } > >> -- > >> 2.52.0 > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
