On 10/3/22 18:13, Ori Kam wrote:
Hi Andrew,-----Original Message----- From: Andrew Rybchenko <andrew.rybche...@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.rybche...@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 <michae...@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.cindex 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 *, conststruct 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, conststructtoken *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_tport_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_tport_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.rstb/doc/guides/prog_guide/rte_flow.rstindex 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_ACTIONerror 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 theflow.| ``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 theAGEaction:| ``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.+1One 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 addanything+}; + /** * @warning * @b EXPERIMENTAL: this structure may change without prior notice
