From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 771ECA0093; Mon, 3 Oct 2022 10:03:39 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 1882440695; Mon, 3 Oct 2022 10:03:39 +0200 (CEST) Received: from shelob.oktetlabs.ru (shelob.oktetlabs.ru [91.220.146.113]) by mails.dpdk.org (Postfix) with ESMTP id 8D38440693 for ; Mon, 3 Oct 2022 10:03:37 +0200 (CEST) Received: from [192.168.38.17] (aros.oktetlabs.ru [192.168.38.17]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by shelob.oktetlabs.ru (Postfix) with ESMTPSA id D4CAE5E; Mon, 3 Oct 2022 11:03:36 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 shelob.oktetlabs.ru D4CAE5E DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=oktetlabs.ru; s=default; t=1664784217; bh=3YNG3kj02nEyOSjCfWjY64Moean9uoiGnqarEnLttnw=; h=Date:Subject:To:Cc:References:From:In-Reply-To:From; b=N976qKFfuN5cGe43vrJcoUnQv5FrSvk9K8OyvxVdWFJWc1YWRJlGcGTYoT6j1Yx8Y r8ipL/Ou93pzAsITWfzqzmSgGZRa1hcvb4bX4WH8BZP24FGfjk0B017QEPUbXsHGqP aLJAkhOppANWOAl3oxA5ZJV8Vn07erg4Rc7+CZqw= Message-ID: <811afcdb-2081-b20d-5d49-8659760a7026@oktetlabs.ru> Date: Mon, 3 Oct 2022 11:03:35 +0300 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.13.0 Subject: Re: [PATCH 3/3] ethdev: add structure for indirect AGE update Content-Language: en-US To: Michael Baum , dev@dpdk.org Cc: Matan Azrad , Raslan Darawsheh , Ori Kam References: <20220921145409.511328-1-michaelba@nvidia.com> <20220921145409.511328-4-michaelba@nvidia.com> From: Andrew Rybchenko Organization: OKTET Labs In-Reply-To: <20220921145409.511328-4-michaelba@nvidia.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org 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 > --- > 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