[3/3] ethdev: add structure for indirect AGE update
Checks
Commit Message
Add a new structure for indirect AGE update.
This new structure enables:
1. Update timeout value.
2. Stop AGE checking.
3. Start AGE checking.
4. restart AGE checking.
Signed-off-by: Michael Baum <michaelba@nvidia.com>
---
app/test-pmd/cmdline_flow.c | 66 ++++++++++++++++++++++++++++++
app/test-pmd/config.c | 18 +++++++-
doc/guides/prog_guide/rte_flow.rst | 25 +++++++++--
lib/ethdev/rte_flow.h | 27 ++++++++++++
4 files changed, 132 insertions(+), 4 deletions(-)
Comments
Hi Michael,
> -----Original Message-----
> From: Michael Baum <michaelba@nvidia.com>
> Sent: Wednesday, 21 September 2022 17:54
>
> Add a new structure for indirect AGE update.
>
> This new structure enables:
> 1. Update timeout value.
> 2. Stop AGE checking.
> 3. Start AGE checking.
> 4. restart AGE checking.
>
> Signed-off-by: Michael Baum <michaelba@nvidia.com>
> ---
Acked-by: Ori Kam <orika@nvidia.com>
Best,
Ori
On 9/21/22 17:54, Michael Baum wrote:
> Add a new structure for indirect AGE update.
>
> This new structure enables:
> 1. Update timeout value.
> 2. Stop AGE checking.
> 3. Start AGE checking.
> 4. restart AGE checking.
>
> Signed-off-by: Michael Baum <michaelba@nvidia.com>
> ---
> app/test-pmd/cmdline_flow.c | 66 ++++++++++++++++++++++++++++++
> app/test-pmd/config.c | 18 +++++++-
> doc/guides/prog_guide/rte_flow.rst | 25 +++++++++--
> lib/ethdev/rte_flow.h | 27 ++++++++++++
> 4 files changed, 132 insertions(+), 4 deletions(-)
>
> diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
> index 4fb90a92cb..a315fd9ded 100644
> --- a/app/test-pmd/cmdline_flow.c
> +++ b/app/test-pmd/cmdline_flow.c
> @@ -583,6 +583,9 @@ enum index {
> ACTION_SET_IPV6_DSCP_VALUE,
> ACTION_AGE,
> ACTION_AGE_TIMEOUT,
> + ACTION_AGE_UPDATE,
> + ACTION_AGE_UPDATE_TIMEOUT,
> + ACTION_AGE_UPDATE_TOUCH,
> ACTION_SAMPLE,
> ACTION_SAMPLE_RATIO,
> ACTION_SAMPLE_INDEX,
> @@ -1869,6 +1872,7 @@ static const enum index next_action[] = {
> ACTION_SET_IPV4_DSCP,
> ACTION_SET_IPV6_DSCP,
> ACTION_AGE,
> + ACTION_AGE_UPDATE,
> ACTION_SAMPLE,
> ACTION_INDIRECT,
> ACTION_MODIFY_FIELD,
> @@ -2113,6 +2117,14 @@ static const enum index action_age[] = {
> ZERO,
> };
>
> +static const enum index action_age_update[] = {
> + ACTION_AGE_UPDATE,
> + ACTION_AGE_UPDATE_TIMEOUT,
> + ACTION_AGE_UPDATE_TOUCH,
> + ACTION_NEXT,
> + ZERO,
> +};
> +
> static const enum index action_sample[] = {
> ACTION_SAMPLE,
> ACTION_SAMPLE_RATIO,
> @@ -2191,6 +2203,9 @@ static int parse_vc_spec(struct context *, const struct token *,
> const char *, unsigned int, void *, unsigned int);
> static int parse_vc_conf(struct context *, const struct token *,
> const char *, unsigned int, void *, unsigned int);
> +static int parse_vc_conf_timeout(struct context *, const struct token *,
> + const char *, unsigned int, void *,
> + unsigned int);
> static int parse_vc_item_ecpri_type(struct context *, const struct token *,
> const char *, unsigned int,
> void *, unsigned int);
> @@ -6194,6 +6209,30 @@ static const struct token token_list[] = {
> .next = NEXT(action_age, NEXT_ENTRY(COMMON_UNSIGNED)),
> .call = parse_vc_conf,
> },
> + [ACTION_AGE_UPDATE] = {
> + .name = "age_update",
> + .help = "update aging parameter",
> + .next = NEXT(action_age_update),
> + .priv = PRIV_ACTION(AGE,
> + sizeof(struct rte_flow_update_age)),
> + .call = parse_vc,
> + },
> + [ACTION_AGE_UPDATE_TIMEOUT] = {
> + .name = "timeout",
> + .help = "age timeout update value",
> + .args = ARGS(ARGS_ENTRY_BF(struct rte_flow_update_age,
> + timeout, 24)),
> + .next = NEXT(action_age_update, NEXT_ENTRY(COMMON_UNSIGNED)),
> + .call = parse_vc_conf_timeout,
> + },
> + [ACTION_AGE_UPDATE_TOUCH] = {
> + .name = "touch",
> + .help = "this flow is touched",
> + .next = NEXT(action_age_update, NEXT_ENTRY(COMMON_BOOLEAN)),
> + .args = ARGS(ARGS_ENTRY_BF(struct rte_flow_update_age,
> + touch, 1)),
> + .call = parse_vc_conf,
> + },
> [ACTION_SAMPLE] = {
> .name = "sample",
> .help = "set a sample action",
> @@ -7031,6 +7070,33 @@ parse_vc_conf(struct context *ctx, const struct token *token,
> return len;
> }
>
> +/** Parse action configuration field. */
> +static int
> +parse_vc_conf_timeout(struct context *ctx, const struct token *token,
> + const char *str, unsigned int len,
> + void *buf, unsigned int size)
> +{
> + struct buffer *out = buf;
> + struct rte_flow_update_age *update;
> +
> + (void)size;
> + if (ctx->curr != ACTION_AGE_UPDATE_TIMEOUT)
> + return -1;
> + /* Token name must match. */
> + if (parse_default(ctx, token, str, len, NULL, 0) < 0)
> + return -1;
> + /* Nothing else to do if there is no buffer. */
> + if (!out)
> + return len;
> + /* Point to selected object. */
> + ctx->object = out->args.vc.data;
> + ctx->objmask = NULL;
> + /* Update the timeout is valid. */
> + update = (struct rte_flow_update_age *)out->args.vc.data;
> + update->timeout_valid = 1;
> + return len;
> +}
> +
> /** Parse eCPRI common header type field. */
> static int
> parse_vc_item_ecpri_type(struct context *ctx, const struct token *token,
> diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
> index 31952467fb..45495385d7 100644
> --- a/app/test-pmd/config.c
> +++ b/app/test-pmd/config.c
> @@ -2065,6 +2065,7 @@ port_action_handle_update(portid_t port_id, uint32_t id,
> if (!pia)
> return -EINVAL;
> switch (pia->type) {
> + case RTE_FLOW_ACTION_TYPE_AGE:
> case RTE_FLOW_ACTION_TYPE_CONNTRACK:
> update = action->conf;
> break;
> @@ -2904,6 +2905,8 @@ port_queue_action_handle_update(portid_t port_id,
> struct rte_port *port;
> struct rte_flow_error error;
> struct rte_flow_action_handle *action_handle;
> + struct port_indirect_action *pia;
> + const void *update;
>
> action_handle = port_action_handle_get_by_id(port_id, id);
> if (!action_handle)
> @@ -2915,8 +2918,21 @@ port_queue_action_handle_update(portid_t port_id,
> return -EINVAL;
> }
>
> + pia = action_get_by_id(port_id, id);
> + if (!pia)
> + return -EINVAL;
> +
> + switch (pia->type) {
> + case RTE_FLOW_ACTION_TYPE_AGE:
> + update = action->conf;
> + break;
> + default:
> + update = action;
> + break;
> + }
> +
> if (rte_flow_async_action_handle_update(port_id, queue_id, &attr,
> - action_handle, action, NULL, &error)) {
> + action_handle, update, NULL, &error)) {
> return port_flow_complain(&error);
> }
> printf("Indirect action #%u update queued\n", id);
> diff --git a/doc/guides/prog_guide/rte_flow.rst b/doc/guides/prog_guide/rte_flow.rst
> index 588914b231..dae9121279 100644
> --- a/doc/guides/prog_guide/rte_flow.rst
> +++ b/doc/guides/prog_guide/rte_flow.rst
> @@ -2958,7 +2958,7 @@ Otherwise, RTE_FLOW_ERROR_TYPE_ACTION error will be returned.
> Action: ``AGE``
> ^^^^^^^^^^^^^^^
>
> -Set ageing timeout configuration to a flow.
> +Set aging timeout configuration to a flow.
>
> Event RTE_ETH_EVENT_FLOW_AGED will be reported if
> timeout passed without any matching on the flow.
> @@ -2977,8 +2977,8 @@ timeout passed without any matching on the flow.
> | ``context`` | user input flow context |
> +--------------+---------------------------------+
>
> -Query structure to retrieve ageing status information of a
> -shared AGE action, or a flow rule using the AGE action:
> +Query structure to retrieve aging status information of an
> +indirect AGE action, or a flow rule using the AGE action:
>
> .. _table_rte_flow_query_age:
>
> @@ -2994,6 +2994,25 @@ shared AGE action, or a flow rule using the AGE action:
> | ``sec_since_last_hit`` | out | Seconds since last traffic hit |
> +------------------------------+-----+----------------------------------------+
>
> +Update structure to modify the parameters of an indirect AGE action.
> +The update structure is used by ``rte_flow_action_handle_update()`` function.
> +
> +.. _table_rte_flow_update_age:
> +
> +.. table:: AGE update
> +
> + +-------------------+--------------------------------------------------------------+
> + | Field | Value |
> + +===================+==============================================================+
> + | ``timeout`` | 24 bits timeout value |
> + +-------------------+--------------------------------------------------------------+
> + | ``timeout_valid`` | 1 bit, timeout value is valid |
> + +-------------------+--------------------------------------------------------------+
> + | ``touch`` | 1 bit, touch the AGE action to set ``sec_since_last_hit`` 0 |
> + +-------------------+--------------------------------------------------------------+
> + | ``reserved`` | 6 bits reserved, must be zero |
> + +-------------------+--------------------------------------------------------------+
> +
> Action: ``SAMPLE``
> ^^^^^^^^^^^^^^^^^^
>
> diff --git a/lib/ethdev/rte_flow.h b/lib/ethdev/rte_flow.h
> index d830b02321..a21d437cf8 100644
> --- a/lib/ethdev/rte_flow.h
> +++ b/lib/ethdev/rte_flow.h
> @@ -2954,6 +2954,33 @@ struct rte_flow_query_age {
> uint32_t sec_since_last_hit:24; /**< Seconds since last traffic hit. */
> };
>
> +/**
> + * @warning
> + * @b EXPERIMENTAL: this structure may change without prior notice
> + *
> + * RTE_FLOW_ACTION_TYPE_AGE
> + *
> + * Update indirect AGE action attributes:
> + * - Timeout can be updated including stop/start action:
> + * +-------------+-------------+------------------------------+
> + * | Old Timeout | New Timeout | Updating |
> + * +=============+=============+==============================+
> + * | 0 | positive | Start aging with new value |
> + * +-------------+-------------+------------------------------+
> + * | positive | 0 | Stop aging |
> + * +-------------+-------------+------------------------------+
> + * | positive | positive | Change timeout to new value |
> + * +-------------+-------------+------------------------------+
> + * - sec_since_last_hit can be reset.
> + */
> +struct rte_flow_update_age {
I think it worse to mention the structure in
RTE_FLOW_ACTION_TYPE_AGE description.
> + uint32_t reserved:6; /**< Reserved, must be zero. */
> + uint32_t timeout_valid:1; /**< The timeout is valid for update. */
> + uint32_t timeout:24; /**< Time in seconds. */
> + uint32_t touch:1;
> + /**< Means that aging should assume packet passed the aging. */
Documentation after code makes sense if it is in the same line.
If the documentation is in its own line, it should be before
the code and use /** prefix.
One more question: why order in the documentation above does
not match order here?
> +};
> +
> /**
> * @warning
> * @b EXPERIMENTAL: this structure may change without prior notice
Hi Andrew,
> -----Original Message-----
> From: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
> Sent: Monday, 3 October 2022 11:04
>
> On 9/21/22 17:54, Michael Baum wrote:
> > Add a new structure for indirect AGE update.
> >
> > This new structure enables:
> > 1. Update timeout value.
> > 2. Stop AGE checking.
> > 3. Start AGE checking.
> > 4. restart AGE checking.
> >
> > Signed-off-by: Michael Baum <michaelba@nvidia.com>
> > ---
> > app/test-pmd/cmdline_flow.c | 66
> ++++++++++++++++++++++++++++++
> > app/test-pmd/config.c | 18 +++++++-
> > doc/guides/prog_guide/rte_flow.rst | 25 +++++++++--
> > lib/ethdev/rte_flow.h | 27 ++++++++++++
> > 4 files changed, 132 insertions(+), 4 deletions(-)
> >
> > diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
> > index 4fb90a92cb..a315fd9ded 100644
> > --- a/app/test-pmd/cmdline_flow.c
> > +++ b/app/test-pmd/cmdline_flow.c
> > @@ -583,6 +583,9 @@ enum index {
> > ACTION_SET_IPV6_DSCP_VALUE,
> > ACTION_AGE,
> > ACTION_AGE_TIMEOUT,
> > + ACTION_AGE_UPDATE,
> > + ACTION_AGE_UPDATE_TIMEOUT,
> > + ACTION_AGE_UPDATE_TOUCH,
> > ACTION_SAMPLE,
> > ACTION_SAMPLE_RATIO,
> > ACTION_SAMPLE_INDEX,
> > @@ -1869,6 +1872,7 @@ static const enum index next_action[] = {
> > ACTION_SET_IPV4_DSCP,
> > ACTION_SET_IPV6_DSCP,
> > ACTION_AGE,
> > + ACTION_AGE_UPDATE,
> > ACTION_SAMPLE,
> > ACTION_INDIRECT,
> > ACTION_MODIFY_FIELD,
> > @@ -2113,6 +2117,14 @@ static const enum index action_age[] = {
> > ZERO,
> > };
> >
> > +static const enum index action_age_update[] = {
> > + ACTION_AGE_UPDATE,
> > + ACTION_AGE_UPDATE_TIMEOUT,
> > + ACTION_AGE_UPDATE_TOUCH,
> > + ACTION_NEXT,
> > + ZERO,
> > +};
> > +
> > static const enum index action_sample[] = {
> > ACTION_SAMPLE,
> > ACTION_SAMPLE_RATIO,
> > @@ -2191,6 +2203,9 @@ static int parse_vc_spec(struct context *, const
> struct token *,
> > const char *, unsigned int, void *, unsigned int);
> > static int parse_vc_conf(struct context *, const struct token *,
> > const char *, unsigned int, void *, unsigned int);
> > +static int parse_vc_conf_timeout(struct context *, const struct token *,
> > + const char *, unsigned int, void *,
> > + unsigned int);
> > static int parse_vc_item_ecpri_type(struct context *, const struct token *,
> > const char *, unsigned int,
> > void *, unsigned int);
> > @@ -6194,6 +6209,30 @@ static const struct token token_list[] = {
> > .next = NEXT(action_age,
> NEXT_ENTRY(COMMON_UNSIGNED)),
> > .call = parse_vc_conf,
> > },
> > + [ACTION_AGE_UPDATE] = {
> > + .name = "age_update",
> > + .help = "update aging parameter",
> > + .next = NEXT(action_age_update),
> > + .priv = PRIV_ACTION(AGE,
> > + sizeof(struct rte_flow_update_age)),
> > + .call = parse_vc,
> > + },
> > + [ACTION_AGE_UPDATE_TIMEOUT] = {
> > + .name = "timeout",
> > + .help = "age timeout update value",
> > + .args = ARGS(ARGS_ENTRY_BF(struct rte_flow_update_age,
> > + timeout, 24)),
> > + .next = NEXT(action_age_update,
> NEXT_ENTRY(COMMON_UNSIGNED)),
> > + .call = parse_vc_conf_timeout,
> > + },
> > + [ACTION_AGE_UPDATE_TOUCH] = {
> > + .name = "touch",
> > + .help = "this flow is touched",
> > + .next = NEXT(action_age_update,
> NEXT_ENTRY(COMMON_BOOLEAN)),
> > + .args = ARGS(ARGS_ENTRY_BF(struct rte_flow_update_age,
> > + touch, 1)),
> > + .call = parse_vc_conf,
> > + },
> > [ACTION_SAMPLE] = {
> > .name = "sample",
> > .help = "set a sample action",
> > @@ -7031,6 +7070,33 @@ parse_vc_conf(struct context *ctx, const struct
> token *token,
> > return len;
> > }
> >
> > +/** Parse action configuration field. */
> > +static int
> > +parse_vc_conf_timeout(struct context *ctx, const struct token *token,
> > + const char *str, unsigned int len,
> > + void *buf, unsigned int size)
> > +{
> > + struct buffer *out = buf;
> > + struct rte_flow_update_age *update;
> > +
> > + (void)size;
> > + if (ctx->curr != ACTION_AGE_UPDATE_TIMEOUT)
> > + return -1;
> > + /* Token name must match. */
> > + if (parse_default(ctx, token, str, len, NULL, 0) < 0)
> > + return -1;
> > + /* Nothing else to do if there is no buffer. */
> > + if (!out)
> > + return len;
> > + /* Point to selected object. */
> > + ctx->object = out->args.vc.data;
> > + ctx->objmask = NULL;
> > + /* Update the timeout is valid. */
> > + update = (struct rte_flow_update_age *)out->args.vc.data;
> > + update->timeout_valid = 1;
> > + return len;
> > +}
> > +
> > /** Parse eCPRI common header type field. */
> > static int
> > parse_vc_item_ecpri_type(struct context *ctx, const struct token *token,
> > diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
> > index 31952467fb..45495385d7 100644
> > --- a/app/test-pmd/config.c
> > +++ b/app/test-pmd/config.c
> > @@ -2065,6 +2065,7 @@ port_action_handle_update(portid_t port_id,
> uint32_t id,
> > if (!pia)
> > return -EINVAL;
> > switch (pia->type) {
> > + case RTE_FLOW_ACTION_TYPE_AGE:
> > case RTE_FLOW_ACTION_TYPE_CONNTRACK:
> > update = action->conf;
> > break;
> > @@ -2904,6 +2905,8 @@ port_queue_action_handle_update(portid_t
> port_id,
> > struct rte_port *port;
> > struct rte_flow_error error;
> > struct rte_flow_action_handle *action_handle;
> > + struct port_indirect_action *pia;
> > + const void *update;
> >
> > action_handle = port_action_handle_get_by_id(port_id, id);
> > if (!action_handle)
> > @@ -2915,8 +2918,21 @@ port_queue_action_handle_update(portid_t
> port_id,
> > return -EINVAL;
> > }
> >
> > + pia = action_get_by_id(port_id, id);
> > + if (!pia)
> > + return -EINVAL;
> > +
> > + switch (pia->type) {
> > + case RTE_FLOW_ACTION_TYPE_AGE:
> > + update = action->conf;
> > + break;
> > + default:
> > + update = action;
> > + break;
> > + }
> > +
> > if (rte_flow_async_action_handle_update(port_id, queue_id, &attr,
> > - action_handle, action, NULL, &error)) {
> > + action_handle, update, NULL, &error)) {
> > return port_flow_complain(&error);
> > }
> > printf("Indirect action #%u update queued\n", id);
> > diff --git a/doc/guides/prog_guide/rte_flow.rst
> b/doc/guides/prog_guide/rte_flow.rst
> > index 588914b231..dae9121279 100644
> > --- a/doc/guides/prog_guide/rte_flow.rst
> > +++ b/doc/guides/prog_guide/rte_flow.rst
> > @@ -2958,7 +2958,7 @@ Otherwise, RTE_FLOW_ERROR_TYPE_ACTION
> error will be returned.
> > Action: ``AGE``
> > ^^^^^^^^^^^^^^^
> >
> > -Set ageing timeout configuration to a flow.
> > +Set aging timeout configuration to a flow.
> >
> > Event RTE_ETH_EVENT_FLOW_AGED will be reported if
> > timeout passed without any matching on the flow.
> > @@ -2977,8 +2977,8 @@ timeout passed without any matching on the
> flow.
> > | ``context`` | user input flow context |
> > +--------------+---------------------------------+
> >
> > -Query structure to retrieve ageing status information of a
> > -shared AGE action, or a flow rule using the AGE action:
> > +Query structure to retrieve aging status information of an
> > +indirect AGE action, or a flow rule using the AGE action:
> >
> > .. _table_rte_flow_query_age:
> >
> > @@ -2994,6 +2994,25 @@ shared AGE action, or a flow rule using the AGE
> action:
> > | ``sec_since_last_hit`` | out | Seconds since last traffic hit |
> > +------------------------------+-----+----------------------------------------+
> >
> > +Update structure to modify the parameters of an indirect AGE action.
> > +The update structure is used by ``rte_flow_action_handle_update()``
> function.
> > +
> > +.. _table_rte_flow_update_age:
> > +
> > +.. table:: AGE update
> > +
> > + +-------------------+--------------------------------------------------------------+
> > + | Field | Value |
> > +
> +===================+=====================================
> =========================+
> > + | ``timeout`` | 24 bits timeout value |
> > + +-------------------+--------------------------------------------------------------+
> > + | ``timeout_valid`` | 1 bit, timeout value is valid |
> > + +-------------------+--------------------------------------------------------------+
> > + | ``touch`` | 1 bit, touch the AGE action to set ``sec_since_last_hit`` 0
> |
> > + +-------------------+--------------------------------------------------------------+
> > + | ``reserved`` | 6 bits reserved, must be zero |
> > + +-------------------+--------------------------------------------------------------+
> > +
> > Action: ``SAMPLE``
> > ^^^^^^^^^^^^^^^^^^
> >
> > diff --git a/lib/ethdev/rte_flow.h b/lib/ethdev/rte_flow.h
> > index d830b02321..a21d437cf8 100644
> > --- a/lib/ethdev/rte_flow.h
> > +++ b/lib/ethdev/rte_flow.h
> > @@ -2954,6 +2954,33 @@ struct rte_flow_query_age {
> > uint32_t sec_since_last_hit:24; /**< Seconds since last traffic hit. */
> > };
> >
> > +/**
> > + * @warning
> > + * @b EXPERIMENTAL: this structure may change without prior notice
> > + *
> > + * RTE_FLOW_ACTION_TYPE_AGE
> > + *
> > + * Update indirect AGE action attributes:
> > + * - Timeout can be updated including stop/start action:
> > + * +-------------+-------------+------------------------------+
> > + * | Old Timeout | New Timeout | Updating |
> > + *
> +=============+=============+=============================
> =+
> > + * | 0 | positive | Start aging with new value |
> > + * +-------------+-------------+------------------------------+
> > + * | positive | 0 | Stop aging |
> > + * +-------------+-------------+------------------------------+
> > + * | positive | positive | Change timeout to new value |
> > + * +-------------+-------------+------------------------------+
> > + * - sec_since_last_hit can be reset.
> > + */
> > +struct rte_flow_update_age {
>
> I think it worse to mention the structure in
> RTE_FLOW_ACTION_TYPE_AGE description.
What do you mean?
>
> > + uint32_t reserved:6; /**< Reserved, must be zero. */
> > + uint32_t timeout_valid:1; /**< The timeout is valid for update. */
> > + uint32_t timeout:24; /**< Time in seconds. */
> > + uint32_t touch:1;
> > + /**< Means that aging should assume packet passed the aging. */
>
> Documentation after code makes sense if it is in the same line.
> If the documentation is in its own line, it should be before
> the code and use /** prefix.
>
+1
> One more question: why order in the documentation above does
> not match order here?
>
I think we can remove it from the documentation since it doesn't add anything
> > +};
> > +
> > /**
> > * @warning
> > * @b EXPERIMENTAL: this structure may change without prior notice
On 10/3/22 11:30, Ori Kam wrote:
> Hi Andrew,
>
>> -----Original Message-----
>> From: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
>> Sent: Monday, 3 October 2022 11:04
>>
>> On 9/21/22 17:54, Michael Baum wrote:
>>> Add a new structure for indirect AGE update.
>>>
>>> This new structure enables:
>>> 1. Update timeout value.
>>> 2. Stop AGE checking.
>>> 3. Start AGE checking.
>>> 4. restart AGE checking.
>>>
>>> Signed-off-by: Michael Baum <michaelba@nvidia.com>
>>> ---
>>> app/test-pmd/cmdline_flow.c | 66
>> ++++++++++++++++++++++++++++++
>>> app/test-pmd/config.c | 18 +++++++-
>>> doc/guides/prog_guide/rte_flow.rst | 25 +++++++++--
>>> lib/ethdev/rte_flow.h | 27 ++++++++++++
>>> 4 files changed, 132 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
>>> index 4fb90a92cb..a315fd9ded 100644
>>> --- a/app/test-pmd/cmdline_flow.c
>>> +++ b/app/test-pmd/cmdline_flow.c
>>> @@ -583,6 +583,9 @@ enum index {
>>> ACTION_SET_IPV6_DSCP_VALUE,
>>> ACTION_AGE,
>>> ACTION_AGE_TIMEOUT,
>>> + ACTION_AGE_UPDATE,
>>> + ACTION_AGE_UPDATE_TIMEOUT,
>>> + ACTION_AGE_UPDATE_TOUCH,
>>> ACTION_SAMPLE,
>>> ACTION_SAMPLE_RATIO,
>>> ACTION_SAMPLE_INDEX,
>>> @@ -1869,6 +1872,7 @@ static const enum index next_action[] = {
>>> ACTION_SET_IPV4_DSCP,
>>> ACTION_SET_IPV6_DSCP,
>>> ACTION_AGE,
>>> + ACTION_AGE_UPDATE,
>>> ACTION_SAMPLE,
>>> ACTION_INDIRECT,
>>> ACTION_MODIFY_FIELD,
>>> @@ -2113,6 +2117,14 @@ static const enum index action_age[] = {
>>> ZERO,
>>> };
>>>
>>> +static const enum index action_age_update[] = {
>>> + ACTION_AGE_UPDATE,
>>> + ACTION_AGE_UPDATE_TIMEOUT,
>>> + ACTION_AGE_UPDATE_TOUCH,
>>> + ACTION_NEXT,
>>> + ZERO,
>>> +};
>>> +
>>> static const enum index action_sample[] = {
>>> ACTION_SAMPLE,
>>> ACTION_SAMPLE_RATIO,
>>> @@ -2191,6 +2203,9 @@ static int parse_vc_spec(struct context *, const
>> struct token *,
>>> const char *, unsigned int, void *, unsigned int);
>>> static int parse_vc_conf(struct context *, const struct token *,
>>> const char *, unsigned int, void *, unsigned int);
>>> +static int parse_vc_conf_timeout(struct context *, const struct token *,
>>> + const char *, unsigned int, void *,
>>> + unsigned int);
>>> static int parse_vc_item_ecpri_type(struct context *, const struct token *,
>>> const char *, unsigned int,
>>> void *, unsigned int);
>>> @@ -6194,6 +6209,30 @@ static const struct token token_list[] = {
>>> .next = NEXT(action_age,
>> NEXT_ENTRY(COMMON_UNSIGNED)),
>>> .call = parse_vc_conf,
>>> },
>>> + [ACTION_AGE_UPDATE] = {
>>> + .name = "age_update",
>>> + .help = "update aging parameter",
>>> + .next = NEXT(action_age_update),
>>> + .priv = PRIV_ACTION(AGE,
>>> + sizeof(struct rte_flow_update_age)),
>>> + .call = parse_vc,
>>> + },
>>> + [ACTION_AGE_UPDATE_TIMEOUT] = {
>>> + .name = "timeout",
>>> + .help = "age timeout update value",
>>> + .args = ARGS(ARGS_ENTRY_BF(struct rte_flow_update_age,
>>> + timeout, 24)),
>>> + .next = NEXT(action_age_update,
>> NEXT_ENTRY(COMMON_UNSIGNED)),
>>> + .call = parse_vc_conf_timeout,
>>> + },
>>> + [ACTION_AGE_UPDATE_TOUCH] = {
>>> + .name = "touch",
>>> + .help = "this flow is touched",
>>> + .next = NEXT(action_age_update,
>> NEXT_ENTRY(COMMON_BOOLEAN)),
>>> + .args = ARGS(ARGS_ENTRY_BF(struct rte_flow_update_age,
>>> + touch, 1)),
>>> + .call = parse_vc_conf,
>>> + },
>>> [ACTION_SAMPLE] = {
>>> .name = "sample",
>>> .help = "set a sample action",
>>> @@ -7031,6 +7070,33 @@ parse_vc_conf(struct context *ctx, const struct
>> token *token,
>>> return len;
>>> }
>>>
>>> +/** Parse action configuration field. */
>>> +static int
>>> +parse_vc_conf_timeout(struct context *ctx, const struct token *token,
>>> + const char *str, unsigned int len,
>>> + void *buf, unsigned int size)
>>> +{
>>> + struct buffer *out = buf;
>>> + struct rte_flow_update_age *update;
>>> +
>>> + (void)size;
>>> + if (ctx->curr != ACTION_AGE_UPDATE_TIMEOUT)
>>> + return -1;
>>> + /* Token name must match. */
>>> + if (parse_default(ctx, token, str, len, NULL, 0) < 0)
>>> + return -1;
>>> + /* Nothing else to do if there is no buffer. */
>>> + if (!out)
>>> + return len;
>>> + /* Point to selected object. */
>>> + ctx->object = out->args.vc.data;
>>> + ctx->objmask = NULL;
>>> + /* Update the timeout is valid. */
>>> + update = (struct rte_flow_update_age *)out->args.vc.data;
>>> + update->timeout_valid = 1;
>>> + return len;
>>> +}
>>> +
>>> /** Parse eCPRI common header type field. */
>>> static int
>>> parse_vc_item_ecpri_type(struct context *ctx, const struct token *token,
>>> diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
>>> index 31952467fb..45495385d7 100644
>>> --- a/app/test-pmd/config.c
>>> +++ b/app/test-pmd/config.c
>>> @@ -2065,6 +2065,7 @@ port_action_handle_update(portid_t port_id,
>> uint32_t id,
>>> if (!pia)
>>> return -EINVAL;
>>> switch (pia->type) {
>>> + case RTE_FLOW_ACTION_TYPE_AGE:
>>> case RTE_FLOW_ACTION_TYPE_CONNTRACK:
>>> update = action->conf;
>>> break;
>>> @@ -2904,6 +2905,8 @@ port_queue_action_handle_update(portid_t
>> port_id,
>>> struct rte_port *port;
>>> struct rte_flow_error error;
>>> struct rte_flow_action_handle *action_handle;
>>> + struct port_indirect_action *pia;
>>> + const void *update;
>>>
>>> action_handle = port_action_handle_get_by_id(port_id, id);
>>> if (!action_handle)
>>> @@ -2915,8 +2918,21 @@ port_queue_action_handle_update(portid_t
>> port_id,
>>> return -EINVAL;
>>> }
>>>
>>> + pia = action_get_by_id(port_id, id);
>>> + if (!pia)
>>> + return -EINVAL;
>>> +
>>> + switch (pia->type) {
>>> + case RTE_FLOW_ACTION_TYPE_AGE:
>>> + update = action->conf;
>>> + break;
>>> + default:
>>> + update = action;
>>> + break;
>>> + }
>>> +
>>> if (rte_flow_async_action_handle_update(port_id, queue_id, &attr,
>>> - action_handle, action, NULL, &error)) {
>>> + action_handle, update, NULL, &error)) {
>>> return port_flow_complain(&error);
>>> }
>>> printf("Indirect action #%u update queued\n", id);
>>> diff --git a/doc/guides/prog_guide/rte_flow.rst
>> b/doc/guides/prog_guide/rte_flow.rst
>>> index 588914b231..dae9121279 100644
>>> --- a/doc/guides/prog_guide/rte_flow.rst
>>> +++ b/doc/guides/prog_guide/rte_flow.rst
>>> @@ -2958,7 +2958,7 @@ Otherwise, RTE_FLOW_ERROR_TYPE_ACTION
>> error will be returned.
>>> Action: ``AGE``
>>> ^^^^^^^^^^^^^^^
>>>
>>> -Set ageing timeout configuration to a flow.
>>> +Set aging timeout configuration to a flow.
>>>
>>> Event RTE_ETH_EVENT_FLOW_AGED will be reported if
>>> timeout passed without any matching on the flow.
>>> @@ -2977,8 +2977,8 @@ timeout passed without any matching on the
>> flow.
>>> | ``context`` | user input flow context |
>>> +--------------+---------------------------------+
>>>
>>> -Query structure to retrieve ageing status information of a
>>> -shared AGE action, or a flow rule using the AGE action:
>>> +Query structure to retrieve aging status information of an
>>> +indirect AGE action, or a flow rule using the AGE action:
>>>
>>> .. _table_rte_flow_query_age:
>>>
>>> @@ -2994,6 +2994,25 @@ shared AGE action, or a flow rule using the AGE
>> action:
>>> | ``sec_since_last_hit`` | out | Seconds since last traffic hit |
>>> +------------------------------+-----+----------------------------------------+
>>>
>>> +Update structure to modify the parameters of an indirect AGE action.
>>> +The update structure is used by ``rte_flow_action_handle_update()``
>> function.
>>> +
>>> +.. _table_rte_flow_update_age:
>>> +
>>> +.. table:: AGE update
>>> +
>>> + +-------------------+--------------------------------------------------------------+
>>> + | Field | Value |
>>> +
>> +===================+=====================================
>> =========================+
>>> + | ``timeout`` | 24 bits timeout value |
>>> + +-------------------+--------------------------------------------------------------+
>>> + | ``timeout_valid`` | 1 bit, timeout value is valid |
>>> + +-------------------+--------------------------------------------------------------+
>>> + | ``touch`` | 1 bit, touch the AGE action to set ``sec_since_last_hit`` 0
>> |
>>> + +-------------------+--------------------------------------------------------------+
>>> + | ``reserved`` | 6 bits reserved, must be zero |
>>> + +-------------------+--------------------------------------------------------------+
>>> +
>>> Action: ``SAMPLE``
>>> ^^^^^^^^^^^^^^^^^^
>>>
>>> diff --git a/lib/ethdev/rte_flow.h b/lib/ethdev/rte_flow.h
>>> index d830b02321..a21d437cf8 100644
>>> --- a/lib/ethdev/rte_flow.h
>>> +++ b/lib/ethdev/rte_flow.h
>>> @@ -2954,6 +2954,33 @@ struct rte_flow_query_age {
>>> uint32_t sec_since_last_hit:24; /**< Seconds since last traffic hit. */
>>> };
>>>
>>> +/**
>>> + * @warning
>>> + * @b EXPERIMENTAL: this structure may change without prior notice
>>> + *
>>> + * RTE_FLOW_ACTION_TYPE_AGE
>>> + *
>>> + * Update indirect AGE action attributes:
>>> + * - Timeout can be updated including stop/start action:
>>> + * +-------------+-------------+------------------------------+
>>> + * | Old Timeout | New Timeout | Updating |
>>> + *
>> +=============+=============+=============================
>> =+
>>> + * | 0 | positive | Start aging with new value |
>>> + * +-------------+-------------+------------------------------+
>>> + * | positive | 0 | Stop aging |
>>> + * +-------------+-------------+------------------------------+
>>> + * | positive | positive | Change timeout to new value |
>>> + * +-------------+-------------+------------------------------+
>>> + * - sec_since_last_hit can be reset.
>>> + */
>>> +struct rte_flow_update_age {
>>
>> I think it worse to mention the structure in
>> RTE_FLOW_ACTION_TYPE_AGE description.
>
> What do you mean?
RTE_FLOW_ACTION_TYPE_AGE has a description as enum member.
Typically configuration structures are mentioned there.
However, I think that it would be useful to mention specific
update structure if any. Like this one. Just to make it clear
that if a user wants to update the action configuration it
should use specific structure. Or am I missing something?
Misunderstand something?
>
>>
>>> + uint32_t reserved:6; /**< Reserved, must be zero. */
>>> + uint32_t timeout_valid:1; /**< The timeout is valid for update. */
>>> + uint32_t timeout:24; /**< Time in seconds. */
>>> + uint32_t touch:1;
>>> + /**< Means that aging should assume packet passed the aging. */
>>
>> Documentation after code makes sense if it is in the same line.
>> If the documentation is in its own line, it should be before
>> the code and use /** prefix.
>>
>
> +1
>
>> One more question: why order in the documentation above does
>> not match order here?
>>
>
> I think we can remove it from the documentation since it doesn't add anything
>
>>> +};
>>> +
>>> /**
>>> * @warning
>>> * @b EXPERIMENTAL: this structure may change without prior notice
>
Hi Andrew,
> -----Original Message-----
> From: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
> Sent: Monday, 3 October 2022 16:18
>
> On 10/3/22 11:30, Ori Kam wrote:
> > Hi Andrew,
> >
> >> -----Original Message-----
> >> From: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
> >> Sent: Monday, 3 October 2022 11:04
> >>
> >> On 9/21/22 17:54, Michael Baum wrote:
> >>> Add a new structure for indirect AGE update.
> >>>
> >>> This new structure enables:
> >>> 1. Update timeout value.
> >>> 2. Stop AGE checking.
> >>> 3. Start AGE checking.
> >>> 4. restart AGE checking.
> >>>
> >>> Signed-off-by: Michael Baum <michaelba@nvidia.com>
> >>> ---
> >>> app/test-pmd/cmdline_flow.c | 66
> >> ++++++++++++++++++++++++++++++
> >>> app/test-pmd/config.c | 18 +++++++-
> >>> doc/guides/prog_guide/rte_flow.rst | 25 +++++++++--
> >>> lib/ethdev/rte_flow.h | 27 ++++++++++++
> >>> 4 files changed, 132 insertions(+), 4 deletions(-)
> >>>
> >>> diff --git a/app/test-pmd/cmdline_flow.c b/app/test-
> pmd/cmdline_flow.c
> >>> index 4fb90a92cb..a315fd9ded 100644
> >>> --- a/app/test-pmd/cmdline_flow.c
> >>> +++ b/app/test-pmd/cmdline_flow.c
> >>> @@ -583,6 +583,9 @@ enum index {
> >>> ACTION_SET_IPV6_DSCP_VALUE,
> >>> ACTION_AGE,
> >>> ACTION_AGE_TIMEOUT,
> >>> + ACTION_AGE_UPDATE,
> >>> + ACTION_AGE_UPDATE_TIMEOUT,
> >>> + ACTION_AGE_UPDATE_TOUCH,
> >>> ACTION_SAMPLE,
> >>> ACTION_SAMPLE_RATIO,
> >>> ACTION_SAMPLE_INDEX,
> >>> @@ -1869,6 +1872,7 @@ static const enum index next_action[] = {
> >>> ACTION_SET_IPV4_DSCP,
> >>> ACTION_SET_IPV6_DSCP,
> >>> ACTION_AGE,
> >>> + ACTION_AGE_UPDATE,
> >>> ACTION_SAMPLE,
> >>> ACTION_INDIRECT,
> >>> ACTION_MODIFY_FIELD,
> >>> @@ -2113,6 +2117,14 @@ static const enum index action_age[] = {
> >>> ZERO,
> >>> };
> >>>
> >>> +static const enum index action_age_update[] = {
> >>> + ACTION_AGE_UPDATE,
> >>> + ACTION_AGE_UPDATE_TIMEOUT,
> >>> + ACTION_AGE_UPDATE_TOUCH,
> >>> + ACTION_NEXT,
> >>> + ZERO,
> >>> +};
> >>> +
> >>> static const enum index action_sample[] = {
> >>> ACTION_SAMPLE,
> >>> ACTION_SAMPLE_RATIO,
> >>> @@ -2191,6 +2203,9 @@ static int parse_vc_spec(struct context *, const
> >> struct token *,
> >>> const char *, unsigned int, void *, unsigned int);
> >>> static int parse_vc_conf(struct context *, const struct token *,
> >>> const char *, unsigned int, void *, unsigned int);
> >>> +static int parse_vc_conf_timeout(struct context *, const struct token *,
> >>> + const char *, unsigned int, void *,
> >>> + unsigned int);
> >>> static int parse_vc_item_ecpri_type(struct context *, const struct token
> *,
> >>> const char *, unsigned int,
> >>> void *, unsigned int);
> >>> @@ -6194,6 +6209,30 @@ static const struct token token_list[] = {
> >>> .next = NEXT(action_age,
> >> NEXT_ENTRY(COMMON_UNSIGNED)),
> >>> .call = parse_vc_conf,
> >>> },
> >>> + [ACTION_AGE_UPDATE] = {
> >>> + .name = "age_update",
> >>> + .help = "update aging parameter",
> >>> + .next = NEXT(action_age_update),
> >>> + .priv = PRIV_ACTION(AGE,
> >>> + sizeof(struct rte_flow_update_age)),
> >>> + .call = parse_vc,
> >>> + },
> >>> + [ACTION_AGE_UPDATE_TIMEOUT] = {
> >>> + .name = "timeout",
> >>> + .help = "age timeout update value",
> >>> + .args = ARGS(ARGS_ENTRY_BF(struct rte_flow_update_age,
> >>> + timeout, 24)),
> >>> + .next = NEXT(action_age_update,
> >> NEXT_ENTRY(COMMON_UNSIGNED)),
> >>> + .call = parse_vc_conf_timeout,
> >>> + },
> >>> + [ACTION_AGE_UPDATE_TOUCH] = {
> >>> + .name = "touch",
> >>> + .help = "this flow is touched",
> >>> + .next = NEXT(action_age_update,
> >> NEXT_ENTRY(COMMON_BOOLEAN)),
> >>> + .args = ARGS(ARGS_ENTRY_BF(struct rte_flow_update_age,
> >>> + touch, 1)),
> >>> + .call = parse_vc_conf,
> >>> + },
> >>> [ACTION_SAMPLE] = {
> >>> .name = "sample",
> >>> .help = "set a sample action",
> >>> @@ -7031,6 +7070,33 @@ parse_vc_conf(struct context *ctx, const
> struct
> >> token *token,
> >>> return len;
> >>> }
> >>>
> >>> +/** Parse action configuration field. */
> >>> +static int
> >>> +parse_vc_conf_timeout(struct context *ctx, const struct token *token,
> >>> + const char *str, unsigned int len,
> >>> + void *buf, unsigned int size)
> >>> +{
> >>> + struct buffer *out = buf;
> >>> + struct rte_flow_update_age *update;
> >>> +
> >>> + (void)size;
> >>> + if (ctx->curr != ACTION_AGE_UPDATE_TIMEOUT)
> >>> + return -1;
> >>> + /* Token name must match. */
> >>> + if (parse_default(ctx, token, str, len, NULL, 0) < 0)
> >>> + return -1;
> >>> + /* Nothing else to do if there is no buffer. */
> >>> + if (!out)
> >>> + return len;
> >>> + /* Point to selected object. */
> >>> + ctx->object = out->args.vc.data;
> >>> + ctx->objmask = NULL;
> >>> + /* Update the timeout is valid. */
> >>> + update = (struct rte_flow_update_age *)out->args.vc.data;
> >>> + update->timeout_valid = 1;
> >>> + return len;
> >>> +}
> >>> +
> >>> /** Parse eCPRI common header type field. */
> >>> static int
> >>> parse_vc_item_ecpri_type(struct context *ctx, const struct token
> *token,
> >>> diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
> >>> index 31952467fb..45495385d7 100644
> >>> --- a/app/test-pmd/config.c
> >>> +++ b/app/test-pmd/config.c
> >>> @@ -2065,6 +2065,7 @@ port_action_handle_update(portid_t port_id,
> >> uint32_t id,
> >>> if (!pia)
> >>> return -EINVAL;
> >>> switch (pia->type) {
> >>> + case RTE_FLOW_ACTION_TYPE_AGE:
> >>> case RTE_FLOW_ACTION_TYPE_CONNTRACK:
> >>> update = action->conf;
> >>> break;
> >>> @@ -2904,6 +2905,8 @@ port_queue_action_handle_update(portid_t
> >> port_id,
> >>> struct rte_port *port;
> >>> struct rte_flow_error error;
> >>> struct rte_flow_action_handle *action_handle;
> >>> + struct port_indirect_action *pia;
> >>> + const void *update;
> >>>
> >>> action_handle = port_action_handle_get_by_id(port_id, id);
> >>> if (!action_handle)
> >>> @@ -2915,8 +2918,21 @@ port_queue_action_handle_update(portid_t
> >> port_id,
> >>> return -EINVAL;
> >>> }
> >>>
> >>> + pia = action_get_by_id(port_id, id);
> >>> + if (!pia)
> >>> + return -EINVAL;
> >>> +
> >>> + switch (pia->type) {
> >>> + case RTE_FLOW_ACTION_TYPE_AGE:
> >>> + update = action->conf;
> >>> + break;
> >>> + default:
> >>> + update = action;
> >>> + break;
> >>> + }
> >>> +
> >>> if (rte_flow_async_action_handle_update(port_id, queue_id, &attr,
> >>> - action_handle, action, NULL, &error)) {
> >>> + action_handle, update, NULL, &error)) {
> >>> return port_flow_complain(&error);
> >>> }
> >>> printf("Indirect action #%u update queued\n", id);
> >>> diff --git a/doc/guides/prog_guide/rte_flow.rst
> >> b/doc/guides/prog_guide/rte_flow.rst
> >>> index 588914b231..dae9121279 100644
> >>> --- a/doc/guides/prog_guide/rte_flow.rst
> >>> +++ b/doc/guides/prog_guide/rte_flow.rst
> >>> @@ -2958,7 +2958,7 @@ Otherwise, RTE_FLOW_ERROR_TYPE_ACTION
> >> error will be returned.
> >>> Action: ``AGE``
> >>> ^^^^^^^^^^^^^^^
> >>>
> >>> -Set ageing timeout configuration to a flow.
> >>> +Set aging timeout configuration to a flow.
> >>>
> >>> Event RTE_ETH_EVENT_FLOW_AGED will be reported if
> >>> timeout passed without any matching on the flow.
> >>> @@ -2977,8 +2977,8 @@ timeout passed without any matching on the
> >> flow.
> >>> | ``context`` | user input flow context |
> >>> +--------------+---------------------------------+
> >>>
> >>> -Query structure to retrieve ageing status information of a
> >>> -shared AGE action, or a flow rule using the AGE action:
> >>> +Query structure to retrieve aging status information of an
> >>> +indirect AGE action, or a flow rule using the AGE action:
> >>>
> >>> .. _table_rte_flow_query_age:
> >>>
> >>> @@ -2994,6 +2994,25 @@ shared AGE action, or a flow rule using the
> AGE
> >> action:
> >>> | ``sec_since_last_hit`` | out | Seconds since last traffic hit |
> >>> +------------------------------+-----+----------------------------------------+
> >>>
> >>> +Update structure to modify the parameters of an indirect AGE action.
> >>> +The update structure is used by ``rte_flow_action_handle_update()``
> >> function.
> >>> +
> >>> +.. _table_rte_flow_update_age:
> >>> +
> >>> +.. table:: AGE update
> >>> +
> >>> + +-------------------+--------------------------------------------------------------+
> >>> + | Field | Value |
> >>> +
> >> +===================+=====================================
> >> =========================+
> >>> + | ``timeout`` | 24 bits timeout value |
> >>> + +-------------------+--------------------------------------------------------------+
> >>> + | ``timeout_valid`` | 1 bit, timeout value is valid |
> >>> + +-------------------+--------------------------------------------------------------+
> >>> + | ``touch`` | 1 bit, touch the AGE action to set
> ``sec_since_last_hit`` 0
> >> |
> >>> + +-------------------+--------------------------------------------------------------+
> >>> + | ``reserved`` | 6 bits reserved, must be zero |
> >>> + +-------------------+--------------------------------------------------------------+
> >>> +
> >>> Action: ``SAMPLE``
> >>> ^^^^^^^^^^^^^^^^^^
> >>>
> >>> diff --git a/lib/ethdev/rte_flow.h b/lib/ethdev/rte_flow.h
> >>> index d830b02321..a21d437cf8 100644
> >>> --- a/lib/ethdev/rte_flow.h
> >>> +++ b/lib/ethdev/rte_flow.h
> >>> @@ -2954,6 +2954,33 @@ struct rte_flow_query_age {
> >>> uint32_t sec_since_last_hit:24; /**< Seconds since last traffic hit. */
> >>> };
> >>>
> >>> +/**
> >>> + * @warning
> >>> + * @b EXPERIMENTAL: this structure may change without prior notice
> >>> + *
> >>> + * RTE_FLOW_ACTION_TYPE_AGE
> >>> + *
> >>> + * Update indirect AGE action attributes:
> >>> + * - Timeout can be updated including stop/start action:
> >>> + * +-------------+-------------+------------------------------+
> >>> + * | Old Timeout | New Timeout | Updating |
> >>> + *
> >> +=============+=============+=============================
> >> =+
> >>> + * | 0 | positive | Start aging with new value |
> >>> + * +-------------+-------------+------------------------------+
> >>> + * | positive | 0 | Stop aging |
> >>> + * +-------------+-------------+------------------------------+
> >>> + * | positive | positive | Change timeout to new value |
> >>> + * +-------------+-------------+------------------------------+
> >>> + * - sec_since_last_hit can be reset.
> >>> + */
> >>> +struct rte_flow_update_age {
> >>
> >> I think it worse to mention the structure in
> >> RTE_FLOW_ACTION_TYPE_AGE description.
> >
> > What do you mean?
>
> RTE_FLOW_ACTION_TYPE_AGE has a description as enum member.
> Typically configuration structures are mentioned there.
> However, I think that it would be useful to mention specific
> update structure if any. Like this one. Just to make it clear
> that if a user wants to update the action configuration it
> should use specific structure. Or am I missing something?
> Misunderstand something?
>
Do you mean something like:
* see enum RTE_ETH_EVENT_FLOW_AGED
* See struct rte_flow_query_age
+ See struct rte_flow_update_age
*/
RTE_FLOW_ACTION_TYPE_AGE,
If so, I fully agree.
> >
> >>
> >>> + uint32_t reserved:6; /**< Reserved, must be zero. */
> >>> + uint32_t timeout_valid:1; /**< The timeout is valid for update. */
> >>> + uint32_t timeout:24; /**< Time in seconds. */
> >>> + uint32_t touch:1;
> >>> + /**< Means that aging should assume packet passed the aging. */
> >>
> >> Documentation after code makes sense if it is in the same line.
> >> If the documentation is in its own line, it should be before
> >> the code and use /** prefix.
> >>
> >
> > +1
> >
> >> One more question: why order in the documentation above does
> >> not match order here?
> >>
> >
> > I think we can remove it from the documentation since it doesn't add
> anything
> >
> >>> +};
> >>> +
> >>> /**
> >>> * @warning
> >>> * @b EXPERIMENTAL: this structure may change without prior notice
> >
On 10/3/22 18:13, Ori Kam wrote:
> Hi Andrew,
>
>> -----Original Message-----
>> From: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
>> Sent: Monday, 3 October 2022 16:18
>>
>> On 10/3/22 11:30, Ori Kam wrote:
>>> Hi Andrew,
>>>
>>>> -----Original Message-----
>>>> From: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
>>>> Sent: Monday, 3 October 2022 11:04
>>>>
>>>> On 9/21/22 17:54, Michael Baum wrote:
>>>>> Add a new structure for indirect AGE update.
>>>>>
>>>>> This new structure enables:
>>>>> 1. Update timeout value.
>>>>> 2. Stop AGE checking.
>>>>> 3. Start AGE checking.
>>>>> 4. restart AGE checking.
>>>>>
>>>>> Signed-off-by: Michael Baum <michaelba@nvidia.com>
>>>>> ---
>>>>> app/test-pmd/cmdline_flow.c | 66
>>>> ++++++++++++++++++++++++++++++
>>>>> app/test-pmd/config.c | 18 +++++++-
>>>>> doc/guides/prog_guide/rte_flow.rst | 25 +++++++++--
>>>>> lib/ethdev/rte_flow.h | 27 ++++++++++++
>>>>> 4 files changed, 132 insertions(+), 4 deletions(-)
>>>>>
>>>>> diff --git a/app/test-pmd/cmdline_flow.c b/app/test-
>> pmd/cmdline_flow.c
>>>>> index 4fb90a92cb..a315fd9ded 100644
>>>>> --- a/app/test-pmd/cmdline_flow.c
>>>>> +++ b/app/test-pmd/cmdline_flow.c
>>>>> @@ -583,6 +583,9 @@ enum index {
>>>>> ACTION_SET_IPV6_DSCP_VALUE,
>>>>> ACTION_AGE,
>>>>> ACTION_AGE_TIMEOUT,
>>>>> + ACTION_AGE_UPDATE,
>>>>> + ACTION_AGE_UPDATE_TIMEOUT,
>>>>> + ACTION_AGE_UPDATE_TOUCH,
>>>>> ACTION_SAMPLE,
>>>>> ACTION_SAMPLE_RATIO,
>>>>> ACTION_SAMPLE_INDEX,
>>>>> @@ -1869,6 +1872,7 @@ static const enum index next_action[] = {
>>>>> ACTION_SET_IPV4_DSCP,
>>>>> ACTION_SET_IPV6_DSCP,
>>>>> ACTION_AGE,
>>>>> + ACTION_AGE_UPDATE,
>>>>> ACTION_SAMPLE,
>>>>> ACTION_INDIRECT,
>>>>> ACTION_MODIFY_FIELD,
>>>>> @@ -2113,6 +2117,14 @@ static const enum index action_age[] = {
>>>>> ZERO,
>>>>> };
>>>>>
>>>>> +static const enum index action_age_update[] = {
>>>>> + ACTION_AGE_UPDATE,
>>>>> + ACTION_AGE_UPDATE_TIMEOUT,
>>>>> + ACTION_AGE_UPDATE_TOUCH,
>>>>> + ACTION_NEXT,
>>>>> + ZERO,
>>>>> +};
>>>>> +
>>>>> static const enum index action_sample[] = {
>>>>> ACTION_SAMPLE,
>>>>> ACTION_SAMPLE_RATIO,
>>>>> @@ -2191,6 +2203,9 @@ static int parse_vc_spec(struct context *, const
>>>> struct token *,
>>>>> const char *, unsigned int, void *, unsigned int);
>>>>> static int parse_vc_conf(struct context *, const struct token *,
>>>>> const char *, unsigned int, void *, unsigned int);
>>>>> +static int parse_vc_conf_timeout(struct context *, const struct token *,
>>>>> + const char *, unsigned int, void *,
>>>>> + unsigned int);
>>>>> static int parse_vc_item_ecpri_type(struct context *, const struct token
>> *,
>>>>> const char *, unsigned int,
>>>>> void *, unsigned int);
>>>>> @@ -6194,6 +6209,30 @@ static const struct token token_list[] = {
>>>>> .next = NEXT(action_age,
>>>> NEXT_ENTRY(COMMON_UNSIGNED)),
>>>>> .call = parse_vc_conf,
>>>>> },
>>>>> + [ACTION_AGE_UPDATE] = {
>>>>> + .name = "age_update",
>>>>> + .help = "update aging parameter",
>>>>> + .next = NEXT(action_age_update),
>>>>> + .priv = PRIV_ACTION(AGE,
>>>>> + sizeof(struct rte_flow_update_age)),
>>>>> + .call = parse_vc,
>>>>> + },
>>>>> + [ACTION_AGE_UPDATE_TIMEOUT] = {
>>>>> + .name = "timeout",
>>>>> + .help = "age timeout update value",
>>>>> + .args = ARGS(ARGS_ENTRY_BF(struct rte_flow_update_age,
>>>>> + timeout, 24)),
>>>>> + .next = NEXT(action_age_update,
>>>> NEXT_ENTRY(COMMON_UNSIGNED)),
>>>>> + .call = parse_vc_conf_timeout,
>>>>> + },
>>>>> + [ACTION_AGE_UPDATE_TOUCH] = {
>>>>> + .name = "touch",
>>>>> + .help = "this flow is touched",
>>>>> + .next = NEXT(action_age_update,
>>>> NEXT_ENTRY(COMMON_BOOLEAN)),
>>>>> + .args = ARGS(ARGS_ENTRY_BF(struct rte_flow_update_age,
>>>>> + touch, 1)),
>>>>> + .call = parse_vc_conf,
>>>>> + },
>>>>> [ACTION_SAMPLE] = {
>>>>> .name = "sample",
>>>>> .help = "set a sample action",
>>>>> @@ -7031,6 +7070,33 @@ parse_vc_conf(struct context *ctx, const
>> struct
>>>> token *token,
>>>>> return len;
>>>>> }
>>>>>
>>>>> +/** Parse action configuration field. */
>>>>> +static int
>>>>> +parse_vc_conf_timeout(struct context *ctx, const struct token *token,
>>>>> + const char *str, unsigned int len,
>>>>> + void *buf, unsigned int size)
>>>>> +{
>>>>> + struct buffer *out = buf;
>>>>> + struct rte_flow_update_age *update;
>>>>> +
>>>>> + (void)size;
>>>>> + if (ctx->curr != ACTION_AGE_UPDATE_TIMEOUT)
>>>>> + return -1;
>>>>> + /* Token name must match. */
>>>>> + if (parse_default(ctx, token, str, len, NULL, 0) < 0)
>>>>> + return -1;
>>>>> + /* Nothing else to do if there is no buffer. */
>>>>> + if (!out)
>>>>> + return len;
>>>>> + /* Point to selected object. */
>>>>> + ctx->object = out->args.vc.data;
>>>>> + ctx->objmask = NULL;
>>>>> + /* Update the timeout is valid. */
>>>>> + update = (struct rte_flow_update_age *)out->args.vc.data;
>>>>> + update->timeout_valid = 1;
>>>>> + return len;
>>>>> +}
>>>>> +
>>>>> /** Parse eCPRI common header type field. */
>>>>> static int
>>>>> parse_vc_item_ecpri_type(struct context *ctx, const struct token
>> *token,
>>>>> diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
>>>>> index 31952467fb..45495385d7 100644
>>>>> --- a/app/test-pmd/config.c
>>>>> +++ b/app/test-pmd/config.c
>>>>> @@ -2065,6 +2065,7 @@ port_action_handle_update(portid_t port_id,
>>>> uint32_t id,
>>>>> if (!pia)
>>>>> return -EINVAL;
>>>>> switch (pia->type) {
>>>>> + case RTE_FLOW_ACTION_TYPE_AGE:
>>>>> case RTE_FLOW_ACTION_TYPE_CONNTRACK:
>>>>> update = action->conf;
>>>>> break;
>>>>> @@ -2904,6 +2905,8 @@ port_queue_action_handle_update(portid_t
>>>> port_id,
>>>>> struct rte_port *port;
>>>>> struct rte_flow_error error;
>>>>> struct rte_flow_action_handle *action_handle;
>>>>> + struct port_indirect_action *pia;
>>>>> + const void *update;
>>>>>
>>>>> action_handle = port_action_handle_get_by_id(port_id, id);
>>>>> if (!action_handle)
>>>>> @@ -2915,8 +2918,21 @@ port_queue_action_handle_update(portid_t
>>>> port_id,
>>>>> return -EINVAL;
>>>>> }
>>>>>
>>>>> + pia = action_get_by_id(port_id, id);
>>>>> + if (!pia)
>>>>> + return -EINVAL;
>>>>> +
>>>>> + switch (pia->type) {
>>>>> + case RTE_FLOW_ACTION_TYPE_AGE:
>>>>> + update = action->conf;
>>>>> + break;
>>>>> + default:
>>>>> + update = action;
>>>>> + break;
>>>>> + }
>>>>> +
>>>>> if (rte_flow_async_action_handle_update(port_id, queue_id, &attr,
>>>>> - action_handle, action, NULL, &error)) {
>>>>> + action_handle, update, NULL, &error)) {
>>>>> return port_flow_complain(&error);
>>>>> }
>>>>> printf("Indirect action #%u update queued\n", id);
>>>>> diff --git a/doc/guides/prog_guide/rte_flow.rst
>>>> b/doc/guides/prog_guide/rte_flow.rst
>>>>> index 588914b231..dae9121279 100644
>>>>> --- a/doc/guides/prog_guide/rte_flow.rst
>>>>> +++ b/doc/guides/prog_guide/rte_flow.rst
>>>>> @@ -2958,7 +2958,7 @@ Otherwise, RTE_FLOW_ERROR_TYPE_ACTION
>>>> error will be returned.
>>>>> Action: ``AGE``
>>>>> ^^^^^^^^^^^^^^^
>>>>>
>>>>> -Set ageing timeout configuration to a flow.
>>>>> +Set aging timeout configuration to a flow.
>>>>>
>>>>> Event RTE_ETH_EVENT_FLOW_AGED will be reported if
>>>>> timeout passed without any matching on the flow.
>>>>> @@ -2977,8 +2977,8 @@ timeout passed without any matching on the
>>>> flow.
>>>>> | ``context`` | user input flow context |
>>>>> +--------------+---------------------------------+
>>>>>
>>>>> -Query structure to retrieve ageing status information of a
>>>>> -shared AGE action, or a flow rule using the AGE action:
>>>>> +Query structure to retrieve aging status information of an
>>>>> +indirect AGE action, or a flow rule using the AGE action:
>>>>>
>>>>> .. _table_rte_flow_query_age:
>>>>>
>>>>> @@ -2994,6 +2994,25 @@ shared AGE action, or a flow rule using the
>> AGE
>>>> action:
>>>>> | ``sec_since_last_hit`` | out | Seconds since last traffic hit |
>>>>> +------------------------------+-----+----------------------------------------+
>>>>>
>>>>> +Update structure to modify the parameters of an indirect AGE action.
>>>>> +The update structure is used by ``rte_flow_action_handle_update()``
>>>> function.
>>>>> +
>>>>> +.. _table_rte_flow_update_age:
>>>>> +
>>>>> +.. table:: AGE update
>>>>> +
>>>>> + +-------------------+--------------------------------------------------------------+
>>>>> + | Field | Value |
>>>>> +
>>>> +===================+=====================================
>>>> =========================+
>>>>> + | ``timeout`` | 24 bits timeout value |
>>>>> + +-------------------+--------------------------------------------------------------+
>>>>> + | ``timeout_valid`` | 1 bit, timeout value is valid |
>>>>> + +-------------------+--------------------------------------------------------------+
>>>>> + | ``touch`` | 1 bit, touch the AGE action to set
>> ``sec_since_last_hit`` 0
>>>> |
>>>>> + +-------------------+--------------------------------------------------------------+
>>>>> + | ``reserved`` | 6 bits reserved, must be zero |
>>>>> + +-------------------+--------------------------------------------------------------+
>>>>> +
>>>>> Action: ``SAMPLE``
>>>>> ^^^^^^^^^^^^^^^^^^
>>>>>
>>>>> diff --git a/lib/ethdev/rte_flow.h b/lib/ethdev/rte_flow.h
>>>>> index d830b02321..a21d437cf8 100644
>>>>> --- a/lib/ethdev/rte_flow.h
>>>>> +++ b/lib/ethdev/rte_flow.h
>>>>> @@ -2954,6 +2954,33 @@ struct rte_flow_query_age {
>>>>> uint32_t sec_since_last_hit:24; /**< Seconds since last traffic hit. */
>>>>> };
>>>>>
>>>>> +/**
>>>>> + * @warning
>>>>> + * @b EXPERIMENTAL: this structure may change without prior notice
>>>>> + *
>>>>> + * RTE_FLOW_ACTION_TYPE_AGE
>>>>> + *
>>>>> + * Update indirect AGE action attributes:
>>>>> + * - Timeout can be updated including stop/start action:
>>>>> + * +-------------+-------------+------------------------------+
>>>>> + * | Old Timeout | New Timeout | Updating |
>>>>> + *
>>>> +=============+=============+=============================
>>>> =+
>>>>> + * | 0 | positive | Start aging with new value |
>>>>> + * +-------------+-------------+------------------------------+
>>>>> + * | positive | 0 | Stop aging |
>>>>> + * +-------------+-------------+------------------------------+
>>>>> + * | positive | positive | Change timeout to new value |
>>>>> + * +-------------+-------------+------------------------------+
>>>>> + * - sec_since_last_hit can be reset.
>>>>> + */
>>>>> +struct rte_flow_update_age {
>>>>
>>>> I think it worse to mention the structure in
>>>> RTE_FLOW_ACTION_TYPE_AGE description.
>>>
>>> What do you mean?
>>
>> RTE_FLOW_ACTION_TYPE_AGE has a description as enum member.
>> Typically configuration structures are mentioned there.
>> However, I think that it would be useful to mention specific
>> update structure if any. Like this one. Just to make it clear
>> that if a user wants to update the action configuration it
>> should use specific structure. Or am I missing something?
>> Misunderstand something?
>>
>
> Do you mean something like:
> * see enum RTE_ETH_EVENT_FLOW_AGED
> * See struct rte_flow_query_age
> + See struct rte_flow_update_age
> */
> RTE_FLOW_ACTION_TYPE_AGE,
> If so, I fully agree.
Yes, exactly.
>
>>>
>>>>
>>>>> + uint32_t reserved:6; /**< Reserved, must be zero. */
>>>>> + uint32_t timeout_valid:1; /**< The timeout is valid for update. */
>>>>> + uint32_t timeout:24; /**< Time in seconds. */
>>>>> + uint32_t touch:1;
>>>>> + /**< Means that aging should assume packet passed the aging. */
>>>>
>>>> Documentation after code makes sense if it is in the same line.
>>>> If the documentation is in its own line, it should be before
>>>> the code and use /** prefix.
>>>>
>>>
>>> +1
>>>
>>>> One more question: why order in the documentation above does
>>>> not match order here?
>>>>
>>>
>>> I think we can remove it from the documentation since it doesn't add
>> anything
>>>
>>>>> +};
>>>>> +
>>>>> /**
>>>>> * @warning
>>>>> * @b EXPERIMENTAL: this structure may change without prior notice
>>>
>
Hi Andrew and Ori,
On 10/4/22 9:37, Andrew Rybchenko wrote:
>
> On 10/3/22 18:13, Ori Kam wrote:
> > Hi Andrew,
> >
> >> -----Original Message-----
> >> From: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
> >> Sent: Monday, 3 October 2022 16:18
> >>
> >> On 10/3/22 11:30, Ori Kam wrote:
> >>> Hi Andrew,
> >>>
> >>>> -----Original Message-----
> >>>> From: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
> >>>> Sent: Monday, 3 October 2022 11:04
> >>>>
> >>>> On 9/21/22 17:54, Michael Baum wrote:
> >>>>> Add a new structure for indirect AGE update.
> >>>>>
> >>>>> This new structure enables:
> >>>>> 1. Update timeout value.
> >>>>> 2. Stop AGE checking.
> >>>>> 3. Start AGE checking.
> >>>>> 4. restart AGE checking.
> >>>>>
> >>>>> Signed-off-by: Michael Baum <michaelba@nvidia.com>
> >>>>> ---
> >>>>> app/test-pmd/cmdline_flow.c | 66
> >>>> ++++++++++++++++++++++++++++++
> >>>>> app/test-pmd/config.c | 18 +++++++-
> >>>>> doc/guides/prog_guide/rte_flow.rst | 25 +++++++++--
> >>>>> lib/ethdev/rte_flow.h | 27 ++++++++++++
> >>>>> 4 files changed, 132 insertions(+), 4 deletions(-)
> >>>>>
> >>>>> diff --git a/app/test-pmd/cmdline_flow.c b/app/test-
> >> pmd/cmdline_flow.c
> >>>>> index 4fb90a92cb..a315fd9ded 100644
> >>>>> --- a/app/test-pmd/cmdline_flow.c
> >>>>> +++ b/app/test-pmd/cmdline_flow.c
> >>>>> @@ -583,6 +583,9 @@ enum index {
> >>>>> ACTION_SET_IPV6_DSCP_VALUE,
> >>>>> ACTION_AGE,
> >>>>> ACTION_AGE_TIMEOUT,
> >>>>> + ACTION_AGE_UPDATE,
> >>>>> + ACTION_AGE_UPDATE_TIMEOUT,
> >>>>> + ACTION_AGE_UPDATE_TOUCH,
> >>>>> ACTION_SAMPLE,
> >>>>> ACTION_SAMPLE_RATIO,
> >>>>> ACTION_SAMPLE_INDEX,
> >>>>> @@ -1869,6 +1872,7 @@ static const enum index next_action[] = {
> >>>>> ACTION_SET_IPV4_DSCP,
> >>>>> ACTION_SET_IPV6_DSCP,
> >>>>> ACTION_AGE,
> >>>>> + ACTION_AGE_UPDATE,
> >>>>> ACTION_SAMPLE,
> >>>>> ACTION_INDIRECT,
> >>>>> ACTION_MODIFY_FIELD,
> >>>>> @@ -2113,6 +2117,14 @@ static const enum index action_age[] = {
> >>>>> ZERO,
> >>>>> };
> >>>>>
> >>>>> +static const enum index action_age_update[] = {
> >>>>> +ACTION_AGE_UPDATE, ACTION_AGE_UPDATE_TIMEOUT,
> >>>>> +ACTION_AGE_UPDATE_TOUCH, ACTION_NEXT, ZERO, };
> >>>>> +
> >>>>> static const enum index action_sample[] = {
> >>>>> ACTION_SAMPLE,
> >>>>> ACTION_SAMPLE_RATIO,
> >>>>> @@ -2191,6 +2203,9 @@ static int parse_vc_spec(struct context *,
> >>>>> const
> >>>> struct token *,
> >>>>> const char *, unsigned int, void *, unsigned int);
> >>>>> static int parse_vc_conf(struct context *, const struct token *,
> >>>>> const char *, unsigned int, void *,
> >>>>> unsigned int);
> >>>>> +static int parse_vc_conf_timeout(struct context *, const struct token
> *,
> >>>>> + const char *, unsigned int, void *,
> >>>>> + unsigned int);
> >>>>> static int parse_vc_item_ecpri_type(struct context *, const
> >>>>> struct token
> >> *,
> >>>>> const char *, unsigned int,
> >>>>> void *, unsigned int); @@
> >>>>> -6194,6 +6209,30 @@ static const struct token token_list[] = {
> >>>>> .next = NEXT(action_age,
> >>>> NEXT_ENTRY(COMMON_UNSIGNED)),
> >>>>> .call = parse_vc_conf,
> >>>>> },
> >>>>> + [ACTION_AGE_UPDATE] = {
> >>>>> + .name = "age_update",
> >>>>> + .help = "update aging parameter",
> >>>>> + .next = NEXT(action_age_update),
> >>>>> + .priv = PRIV_ACTION(AGE,
> >>>>> + sizeof(struct rte_flow_update_age)),
> >>>>> + .call = parse_vc,
> >>>>> + },
> >>>>> + [ACTION_AGE_UPDATE_TIMEOUT] = {
> >>>>> + .name = "timeout",
> >>>>> + .help = "age timeout update value",
> >>>>> + .args = ARGS(ARGS_ENTRY_BF(struct rte_flow_update_age,
> >>>>> + timeout, 24)),
> >>>>> + .next = NEXT(action_age_update,
> >>>> NEXT_ENTRY(COMMON_UNSIGNED)),
> >>>>> + .call = parse_vc_conf_timeout, },
> >>>>> + [ACTION_AGE_UPDATE_TOUCH] = {
> >>>>> + .name = "touch",
> >>>>> + .help = "this flow is touched",
> >>>>> + .next = NEXT(action_age_update,
> >>>> NEXT_ENTRY(COMMON_BOOLEAN)),
> >>>>> + .args = ARGS(ARGS_ENTRY_BF(struct rte_flow_update_age,
> >>>>> + touch, 1)),
> >>>>> + .call = parse_vc_conf,
> >>>>> + },
> >>>>> [ACTION_SAMPLE] = {
> >>>>> .name = "sample",
> >>>>> .help = "set a sample action", @@ -7031,6
> >>>>> +7070,33 @@ parse_vc_conf(struct context *ctx, const
> >> struct
> >>>> token *token,
> >>>>> return len;
> >>>>> }
> >>>>>
> >>>>> +/** Parse action configuration field. */ static int
> >>>>> +parse_vc_conf_timeout(struct context *ctx, const struct token
> *token,
> >>>>> + const char *str, unsigned int len,
> >>>>> + void *buf, unsigned int size) { struct buffer
> >>>>> +*out = buf; struct rte_flow_update_age *update;
> >>>>> +
> >>>>> + (void)size;
> >>>>> + if (ctx->curr != ACTION_AGE_UPDATE_TIMEOUT)
> >>>>> + return -1;
> >>>>> + /* Token name must match. */
> >>>>> + if (parse_default(ctx, token, str, len, NULL, 0) < 0)
> >>>>> + return -1;
> >>>>> + /* Nothing else to do if there is no buffer. */ if (!out)
> >>>>> + return len;
> >>>>> + /* Point to selected object. */
> >>>>> + ctx->object = out->args.vc.data; objmask = NULL;
> >>>>> + /* Update the timeout is valid. */ update = (struct
> >>>>> + rte_flow_update_age *)out->args.vc.data;
> >>>>> + update->timeout_valid = 1;
> >>>>> + return len;
> >>>>> +}
> >>>>> +
> >>>>> /** Parse eCPRI common header type field. */
> >>>>> static int
> >>>>> parse_vc_item_ecpri_type(struct context *ctx, const struct
> >>>>> token
> >> *token,
> >>>>> diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c index
> >>>>> 31952467fb..45495385d7 100644
> >>>>> --- a/app/test-pmd/config.c
> >>>>> +++ b/app/test-pmd/config.c
> >>>>> @@ -2065,6 +2065,7 @@ port_action_handle_update(portid_t
> port_id,
> >>>> uint32_t id,
> >>>>> if (!pia)
> >>>>> return -EINVAL;
> >>>>> switch (pia->type) {
> >>>>> + case RTE_FLOW_ACTION_TYPE_AGE:
> >>>>> case RTE_FLOW_ACTION_TYPE_CONNTRACK:
> >>>>> update = action->conf;
> >>>>> break;
> >>>>> @@ -2904,6 +2905,8 @@
> port_queue_action_handle_update(portid_t
> >>>> port_id,
> >>>>> struct rte_port *port;
> >>>>> struct rte_flow_error error;
> >>>>> struct rte_flow_action_handle *action_handle;
> >>>>> + struct port_indirect_action *pia; const void *update;
> >>>>>
> >>>>> action_handle = port_action_handle_get_by_id(port_id, id);
> >>>>> if (!action_handle)
> >>>>> @@ -2915,8 +2918,21 @@
> port_queue_action_handle_update(portid_t
> >>>> port_id,
> >>>>> return -EINVAL;
> >>>>> }
> >>>>>
> >>>>> + pia = action_get_by_id(port_id, id); if (!pia)
> >>>>> + return -EINVAL;
> >>>>> +
> >>>>> + switch (pia->type) {
> >>>>> + case RTE_FLOW_ACTION_TYPE_AGE:
> >>>>> + update = action->conf;
> >>>>> + break;
> >>>>> + default:
> >>>>> + update = action;
> >>>>> + break;
> >>>>> + }
> >>>>> +
> >>>>> if (rte_flow_async_action_handle_update(port_id, queue_id,
> &attr,
> >>>>> - action_handle, action, NULL, &error)) {
> >>>>> + action_handle, update, NULL,
> >>>>> + &error)) {
> >>>>> return port_flow_complain(&error);
> >>>>> }
> >>>>> printf("Indirect action #%u update queued\n", id); diff
> >>>>> --git a/doc/guides/prog_guide/rte_flow.rst
> >>>> b/doc/guides/prog_guide/rte_flow.rst
> >>>>> index 588914b231..dae9121279 100644
> >>>>> --- a/doc/guides/prog_guide/rte_flow.rst
> >>>>> +++ b/doc/guides/prog_guide/rte_flow.rst
> >>>>> @@ -2958,7 +2958,7 @@ Otherwise,
> RTE_FLOW_ERROR_TYPE_ACTION
> >>>> error will be returned.
> >>>>> Action: ``AGE``
> >>>>> ^^^^^^^^^^^^^^^
> >>>>>
> >>>>> -Set ageing timeout configuration to a flow.
> >>>>> +Set aging timeout configuration to a flow.
> >>>>>
> >>>>> Event RTE_ETH_EVENT_FLOW_AGED will be reported if
> >>>>> timeout passed without any matching on the flow.
> >>>>> @@ -2977,8 +2977,8 @@ timeout passed without any matching on
> the
> >>>> flow.
> >>>>> | ``context`` | user input flow context |
> >>>>> +--------------+---------------------------------+
> >>>>>
> >>>>> -Query structure to retrieve ageing status information of a
> >>>>> -shared AGE action, or a flow rule using the AGE action:
> >>>>> +Query structure to retrieve aging status information of an
> >>>>> +indirect AGE action, or a flow rule using the AGE action:
> >>>>>
> >>>>> .. _table_rte_flow_query_age:
> >>>>>
> >>>>> @@ -2994,6 +2994,25 @@ shared AGE action, or a flow rule using the
> >> AGE
> >>>> action:
> >>>>> | ``sec_since_last_hit`` | out | Seconds since last traffic hit |
> >>>>>
> >>>>> +------------------------------+-----+----------------------------
> >>>>> ------------+
> >>>>>
> >>>>> +Update structure to modify the parameters of an indirect AGE
> action.
> >>>>> +The update structure is used by
> >>>>> +``rte_flow_action_handle_update()``
> >>>> function.
> >>>>> +
> >>>>> +.. _table_rte_flow_update_age:
> >>>>> +
> >>>>> +.. table:: AGE update
> >>>>> +
> >>>>> + +-------------------+-----------------------------------------------------------
> ---+
> >>>>> + | Field | Value |
> >>>>> +
> >>>>
> +===================+=====================================
> >>>> =========================+
> >>>>> + | ``timeout`` | 24 bits timeout value |
> >>>>> + +-------------------+-----------------------------------------------------------
> ---+
> >>>>> + | ``timeout_valid`` | 1 bit, timeout value is valid |
> >>>>> + +-------------------+-----------------------------------------------------------
> ---+
> >>>>> + | ``touch`` | 1 bit, touch the AGE action to set
> >> ``sec_since_last_hit`` 0
> >>>> |
> >>>>> + +-------------------+-----------------------------------------------------------
> ---+
> >>>>> + | ``reserved`` | 6 bits reserved, must be zero |
> >>>>> +
> >>>>> + +-------------------+-------------------------------------------
> >>>>> + -------------------+
> >>>>> +
> >>>>> Action: ``SAMPLE``
> >>>>> ^^^^^^^^^^^^^^^^^^
> >>>>>
> >>>>> diff --git a/lib/ethdev/rte_flow.h b/lib/ethdev/rte_flow.h index
> >>>>> d830b02321..a21d437cf8 100644
> >>>>> --- a/lib/ethdev/rte_flow.h
> >>>>> +++ b/lib/ethdev/rte_flow.h
> >>>>> @@ -2954,6 +2954,33 @@ struct rte_flow_query_age {
> >>>>> uint32_t sec_since_last_hit:24; /**< Seconds since last traffic hit.
> */
> >>>>> };
> >>>>>
> >>>>> +/**
> >>>>> + * @warning
> >>>>> + * @b EXPERIMENTAL: this structure may change without prior
> >>>>> +notice
> >>>>> + *
> >>>>> + * RTE_FLOW_ACTION_TYPE_AGE
> >>>>> + *
> >>>>> + * Update indirect AGE action attributes:
> >>>>> + * - Timeout can be updated including stop/start action:
> >>>>> + * +-------------+-------------+------------------------------+
> >>>>> + * | Old Timeout | New Timeout | Updating |
> >>>>> + *
> >>>>
> +=============+=============+=============================
> >>>> =+
> >>>>> + * | 0 | positive | Start aging with new value |
> >>>>> + * +-------------+-------------+------------------------------+
> >>>>> + * | positive | 0 | Stop aging |
> >>>>> + * +-------------+-------------+------------------------------+
> >>>>> + * | positive | positive | Change timeout to new value |
> >>>>> + * +-------------+-------------+------------------------------+
> >>>>> + * - sec_since_last_hit can be reset.
> >>>>> + */
> >>>>> +struct rte_flow_update_age {
> >>>>
> >>>> I think it worse to mention the structure in
> >>>> RTE_FLOW_ACTION_TYPE_AGE description.
> >>>
> >>> What do you mean?
> >>
> >> RTE_FLOW_ACTION_TYPE_AGE has a description as enum member.
> >> Typically configuration structures are mentioned there.
> >> However, I think that it would be useful to mention specific update
> >> structure if any. Like this one. Just to make it clear that if a user
> >> wants to update the action configuration it should use specific
> >> structure. Or am I missing something?
> >> Misunderstand something?
> >>
> >
> > Do you mean something like:
> > * see enum RTE_ETH_EVENT_FLOW_AGED
> > * See struct rte_flow_query_age
> > + See struct rte_flow_update_age
> > */
> > RTE_FLOW_ACTION_TYPE_AGE,
> > If so, I fully agree.
>
> Yes, exactly.
I have updated it according your suggestion, thank you.
>
> >
> >>>
> >>>>
> >>>>> + uint32_t reserved:6; /**< Reserved, must be zero. */ uint32_t
> >>>>> + timeout_valid:1; /**< The timeout is valid for update. */
> >>>>> + uint32_t timeout:24; /**< Time in seconds. */ uint32_t touch:1;
> >>>>> + /**< Means that aging should assume packet passed the aging. */
> >>>>
> >>>> Documentation after code makes sense if it is in the same line.
> >>>> If the documentation is in its own line, it should be before the
> >>>> code and use /** prefix.
Updated, thanks.
> >>>
> >>> +1
> >>>
> >>>> One more question: why order in the documentation above does not
> >>>> match order here?
I updated the documentation to be align to this order.
> >>>
> >>> I think we can remove it from the documentation since it doesn't add
> >> anything
> >>>
> >>>>> +};
> >>>>> +
> >>>>> /**
> >>>>> * @warning
> >>>>> * @b EXPERIMENTAL: this structure may change without prior
> >>>>> notice
> >>>
> >
@@ -583,6 +583,9 @@ enum index {
ACTION_SET_IPV6_DSCP_VALUE,
ACTION_AGE,
ACTION_AGE_TIMEOUT,
+ ACTION_AGE_UPDATE,
+ ACTION_AGE_UPDATE_TIMEOUT,
+ ACTION_AGE_UPDATE_TOUCH,
ACTION_SAMPLE,
ACTION_SAMPLE_RATIO,
ACTION_SAMPLE_INDEX,
@@ -1869,6 +1872,7 @@ static const enum index next_action[] = {
ACTION_SET_IPV4_DSCP,
ACTION_SET_IPV6_DSCP,
ACTION_AGE,
+ ACTION_AGE_UPDATE,
ACTION_SAMPLE,
ACTION_INDIRECT,
ACTION_MODIFY_FIELD,
@@ -2113,6 +2117,14 @@ static const enum index action_age[] = {
ZERO,
};
+static const enum index action_age_update[] = {
+ ACTION_AGE_UPDATE,
+ ACTION_AGE_UPDATE_TIMEOUT,
+ ACTION_AGE_UPDATE_TOUCH,
+ ACTION_NEXT,
+ ZERO,
+};
+
static const enum index action_sample[] = {
ACTION_SAMPLE,
ACTION_SAMPLE_RATIO,
@@ -2191,6 +2203,9 @@ static int parse_vc_spec(struct context *, const struct token *,
const char *, unsigned int, void *, unsigned int);
static int parse_vc_conf(struct context *, const struct token *,
const char *, unsigned int, void *, unsigned int);
+static int parse_vc_conf_timeout(struct context *, const struct token *,
+ const char *, unsigned int, void *,
+ unsigned int);
static int parse_vc_item_ecpri_type(struct context *, const struct token *,
const char *, unsigned int,
void *, unsigned int);
@@ -6194,6 +6209,30 @@ static const struct token token_list[] = {
.next = NEXT(action_age, NEXT_ENTRY(COMMON_UNSIGNED)),
.call = parse_vc_conf,
},
+ [ACTION_AGE_UPDATE] = {
+ .name = "age_update",
+ .help = "update aging parameter",
+ .next = NEXT(action_age_update),
+ .priv = PRIV_ACTION(AGE,
+ sizeof(struct rte_flow_update_age)),
+ .call = parse_vc,
+ },
+ [ACTION_AGE_UPDATE_TIMEOUT] = {
+ .name = "timeout",
+ .help = "age timeout update value",
+ .args = ARGS(ARGS_ENTRY_BF(struct rte_flow_update_age,
+ timeout, 24)),
+ .next = NEXT(action_age_update, NEXT_ENTRY(COMMON_UNSIGNED)),
+ .call = parse_vc_conf_timeout,
+ },
+ [ACTION_AGE_UPDATE_TOUCH] = {
+ .name = "touch",
+ .help = "this flow is touched",
+ .next = NEXT(action_age_update, NEXT_ENTRY(COMMON_BOOLEAN)),
+ .args = ARGS(ARGS_ENTRY_BF(struct rte_flow_update_age,
+ touch, 1)),
+ .call = parse_vc_conf,
+ },
[ACTION_SAMPLE] = {
.name = "sample",
.help = "set a sample action",
@@ -7031,6 +7070,33 @@ parse_vc_conf(struct context *ctx, const struct token *token,
return len;
}
+/** Parse action configuration field. */
+static int
+parse_vc_conf_timeout(struct context *ctx, const struct token *token,
+ const char *str, unsigned int len,
+ void *buf, unsigned int size)
+{
+ struct buffer *out = buf;
+ struct rte_flow_update_age *update;
+
+ (void)size;
+ if (ctx->curr != ACTION_AGE_UPDATE_TIMEOUT)
+ return -1;
+ /* Token name must match. */
+ if (parse_default(ctx, token, str, len, NULL, 0) < 0)
+ return -1;
+ /* Nothing else to do if there is no buffer. */
+ if (!out)
+ return len;
+ /* Point to selected object. */
+ ctx->object = out->args.vc.data;
+ ctx->objmask = NULL;
+ /* Update the timeout is valid. */
+ update = (struct rte_flow_update_age *)out->args.vc.data;
+ update->timeout_valid = 1;
+ return len;
+}
+
/** Parse eCPRI common header type field. */
static int
parse_vc_item_ecpri_type(struct context *ctx, const struct token *token,
@@ -2065,6 +2065,7 @@ port_action_handle_update(portid_t port_id, uint32_t id,
if (!pia)
return -EINVAL;
switch (pia->type) {
+ case RTE_FLOW_ACTION_TYPE_AGE:
case RTE_FLOW_ACTION_TYPE_CONNTRACK:
update = action->conf;
break;
@@ -2904,6 +2905,8 @@ port_queue_action_handle_update(portid_t port_id,
struct rte_port *port;
struct rte_flow_error error;
struct rte_flow_action_handle *action_handle;
+ struct port_indirect_action *pia;
+ const void *update;
action_handle = port_action_handle_get_by_id(port_id, id);
if (!action_handle)
@@ -2915,8 +2918,21 @@ port_queue_action_handle_update(portid_t port_id,
return -EINVAL;
}
+ pia = action_get_by_id(port_id, id);
+ if (!pia)
+ return -EINVAL;
+
+ switch (pia->type) {
+ case RTE_FLOW_ACTION_TYPE_AGE:
+ update = action->conf;
+ break;
+ default:
+ update = action;
+ break;
+ }
+
if (rte_flow_async_action_handle_update(port_id, queue_id, &attr,
- action_handle, action, NULL, &error)) {
+ action_handle, update, NULL, &error)) {
return port_flow_complain(&error);
}
printf("Indirect action #%u update queued\n", id);
@@ -2958,7 +2958,7 @@ Otherwise, RTE_FLOW_ERROR_TYPE_ACTION error will be returned.
Action: ``AGE``
^^^^^^^^^^^^^^^
-Set ageing timeout configuration to a flow.
+Set aging timeout configuration to a flow.
Event RTE_ETH_EVENT_FLOW_AGED will be reported if
timeout passed without any matching on the flow.
@@ -2977,8 +2977,8 @@ timeout passed without any matching on the flow.
| ``context`` | user input flow context |
+--------------+---------------------------------+
-Query structure to retrieve ageing status information of a
-shared AGE action, or a flow rule using the AGE action:
+Query structure to retrieve aging status information of an
+indirect AGE action, or a flow rule using the AGE action:
.. _table_rte_flow_query_age:
@@ -2994,6 +2994,25 @@ shared AGE action, or a flow rule using the AGE action:
| ``sec_since_last_hit`` | out | Seconds since last traffic hit |
+------------------------------+-----+----------------------------------------+
+Update structure to modify the parameters of an indirect AGE action.
+The update structure is used by ``rte_flow_action_handle_update()`` function.
+
+.. _table_rte_flow_update_age:
+
+.. table:: AGE update
+
+ +-------------------+--------------------------------------------------------------+
+ | Field | Value |
+ +===================+==============================================================+
+ | ``timeout`` | 24 bits timeout value |
+ +-------------------+--------------------------------------------------------------+
+ | ``timeout_valid`` | 1 bit, timeout value is valid |
+ +-------------------+--------------------------------------------------------------+
+ | ``touch`` | 1 bit, touch the AGE action to set ``sec_since_last_hit`` 0 |
+ +-------------------+--------------------------------------------------------------+
+ | ``reserved`` | 6 bits reserved, must be zero |
+ +-------------------+--------------------------------------------------------------+
+
Action: ``SAMPLE``
^^^^^^^^^^^^^^^^^^
@@ -2954,6 +2954,33 @@ struct rte_flow_query_age {
uint32_t sec_since_last_hit:24; /**< Seconds since last traffic hit. */
};
+/**
+ * @warning
+ * @b EXPERIMENTAL: this structure may change without prior notice
+ *
+ * RTE_FLOW_ACTION_TYPE_AGE
+ *
+ * Update indirect AGE action attributes:
+ * - Timeout can be updated including stop/start action:
+ * +-------------+-------------+------------------------------+
+ * | Old Timeout | New Timeout | Updating |
+ * +=============+=============+==============================+
+ * | 0 | positive | Start aging with new value |
+ * +-------------+-------------+------------------------------+
+ * | positive | 0 | Stop aging |
+ * +-------------+-------------+------------------------------+
+ * | positive | positive | Change timeout to new value |
+ * +-------------+-------------+------------------------------+
+ * - sec_since_last_hit can be reset.
+ */
+struct rte_flow_update_age {
+ uint32_t reserved:6; /**< Reserved, must be zero. */
+ uint32_t timeout_valid:1; /**< The timeout is valid for update. */
+ uint32_t timeout:24; /**< Time in seconds. */
+ uint32_t touch:1;
+ /**< Means that aging should assume packet passed the aging. */
+};
+
/**
* @warning
* @b EXPERIMENTAL: this structure may change without prior notice