On 20 Dec 2025, at 0:50, [email protected] wrote:
> 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 (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
