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)
>
>> 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?
>
>> +
>> 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.
>
>> + * 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
