From: Ferruh Yigit <ferruh.yigit@amd.com>
To: "Etelson, Gregory" <getelson@nvidia.com>
Cc: dev@dpdk.org, mkashani@nvidia.com, Ori Kam <orika@nvidia.com>,
Aman Singh <aman.deep.singh@intel.com>,
Yuying Zhang <yuying.zhang@intel.com>,
Thomas Monjalon <thomas@monjalon.net>,
Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
Subject: Re: [PATCH v7] ethdev: add template table resize API
Date: Wed, 14 Feb 2024 21:59:14 +0000 [thread overview]
Message-ID: <f107542e-4b46-425f-b4fc-60df4176697b@amd.com> (raw)
In-Reply-To: <91415555-d916-b2a4-9fbf-0c98bb271543@nvidia.com>
On 2/14/2024 5:07 PM, Etelson, Gregory wrote:
> Hello Ferruh,
>
>>
>> Having conflict while applying the patch, can you please rebase it on
>> latest 'next-net'?
>
> Will rebase and update the patch.
>
>>
>>> + Query wheather template table can be resized.
>>>
>>
>> s/wheather/whether/
>>
>> <...>
>
> fix in updated patch
>
>>
>>> +__rte_experimental
>>> +bool
>>> +rte_flow_template_table_resizable
>>> + (__rte_unused uint16_t port_id,
>>> + const struct rte_flow_template_table_attr *tbl_attr);
>>> +
>>
>> Syntax above is odd, why not move parenthesis to first line.
>
> Agree that's odd style.
> DPDK prefers that way.
> Please check rte_flow_driver.h::rte_flow.ops.
>
True that is also odd but at least it is function pointers in a struct,
but this is regular function deceleration, why not:
```
bool
rte_flow_template_table_resizable(__rte_unused uint16_t port_id,
const struct rte_flow_template_table_attr *tbl_attr);
```
>>
>> This is different than previous version, I just want to confirm if this
>> is intentional.
>
> That API version was discussed with Thomas.
> The alternative was to introduce additional function call to query
> whether a flow rule needs conversion after table resize.
> As the result, the application still needs to iterate on all
> table flow rules.
>
Either it or application needs to manage it. Agree that application can
call update() on all flows is easier for app, that is why I asked for
this clarification.
But still it is different than API definition making it mandatory to
call for new flows.
What about something like, although it is a little longer:
```
Update flow for the new template table configuration after table resize.
Should be called for rules created before table resize. If called for
rules crated after table resize, API should return success, so
application is free to call this API for all flows.
```
>>
>> Rules created *before* resize must be updated.
>
> That part remains. The application must update
> flow rules created before resize.
>
>> Rules created *after* resize not need to be updated, but if API accepts
>> them and just returns a quick success,
>
> Calling update for new rules will return success without flow update.
>
>> this helps user and even user may
>> prefer to not keep track of flows as before and after resize,
>
> The application now does not need to differentiate table flow rules.
> All table flow rules must be updated after table resize.
>
>>
>>> +/**
>>> + * @warning
>>> + * @b EXPERIMENTAL: this API may change without prior notice.
>>> + *
>>> + * Resume normal operational mode after table was resized and
>>> + * table rules were updated for the new table configuration.
>>> + *
>>> + * @param port_id
>>> + * Port identifier of Ethernet device.
>>> + * @param table
>>> + * Template table that undergoing resize operation.
>>> + * @param error
>>> + * Perform verbose error reporting if not NULL.
>>> + * PMDs initialize this structure in case of error only.
>>> + *
>>> + * @return
>>> + * - (0) if success.
>>> + * - (-ENODEV) if *port_id* invalid.
>>> + * - (-ENOTSUP) if underlying device does not support this
>>> functionality.
>>> + * - (-EINVAL) if there are rules that were not updated or
>>> + * *table* cannot complete table resize,
>>> + * unrecoverable error.
>>> + */
>>> +__rte_experimental
>>> +int
>>> +rte_flow_template_table_resize_complete(uint16_t port_id,
>>> + struct rte_flow_template_table
>>> *table,
>>> + struct rte_flow_error *error);
>>
>> I think I asked this before but perhaps missed the response,
>> it is possible that user missed to update all flows and called this API,
>> it will return -EINVAL, in this case it is not clear for user if there
>> is a unrecoverable error or just need to update more flows.
>>
>> What do you think to send a different error for the case that there are
>> flows not updated, so user can action on this information.
>>
>>
>
> A different error is good.
> What about EBUSY ?
>
I don't know, can be EBUSY or EAGAIN.
Or we can overload EEXIST as there are existing not updated flows.
next prev parent reply other threads:[~2024-02-14 21:59 UTC|newest]
Thread overview: 33+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-12-17 9:32 [PATCH] " Gregory Etelson
2024-01-29 14:24 ` Ferruh Yigit
2024-01-29 15:08 ` Etelson, Gregory
2024-01-30 8:58 ` Ferruh Yigit
2024-01-30 12:46 ` Etelson, Gregory
2024-01-30 14:34 ` Ferruh Yigit
2024-01-30 18:15 ` Etelson, Gregory
2024-02-08 12:46 ` Ferruh Yigit
2024-02-09 5:55 ` Etelson, Gregory
2024-01-30 14:56 ` Ferruh Yigit
2024-01-30 18:49 ` Etelson, Gregory
2024-01-31 9:59 ` [PATCH v2] " Gregory Etelson
2024-02-06 22:31 ` Thomas Monjalon
2024-02-07 7:09 ` Etelson, Gregory
2024-02-07 7:03 ` [PATCH v3] " Gregory Etelson
2024-02-07 17:36 ` [PATCH v4] " Gregory Etelson
2024-02-11 9:30 ` [PATCH v5] " Gregory Etelson
2024-02-12 14:02 ` Thomas Monjalon
2024-02-12 14:48 ` Etelson, Gregory
2024-02-12 14:14 ` Ferruh Yigit
2024-02-12 15:01 ` Etelson, Gregory
2024-02-12 15:07 ` Ferruh Yigit
2024-02-12 18:12 ` [PATCH v6] " Gregory Etelson
2024-02-12 20:30 ` Ferruh Yigit
2024-02-13 11:51 ` Thomas Monjalon
2024-02-14 14:32 ` [PATCH v7] " Gregory Etelson
2024-02-14 14:42 ` Thomas Monjalon
2024-02-14 15:56 ` Ferruh Yigit
2024-02-14 17:07 ` Etelson, Gregory
2024-02-14 21:59 ` Ferruh Yigit [this message]
2024-02-15 5:41 ` Etelson, Gregory
2024-02-15 6:13 ` [PATCH v8] " Gregory Etelson
2024-02-15 13:13 ` Ferruh Yigit
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=f107542e-4b46-425f-b4fc-60df4176697b@amd.com \
--to=ferruh.yigit@amd.com \
--cc=aman.deep.singh@intel.com \
--cc=andrew.rybchenko@oktetlabs.ru \
--cc=dev@dpdk.org \
--cc=getelson@nvidia.com \
--cc=mkashani@nvidia.com \
--cc=orika@nvidia.com \
--cc=thomas@monjalon.net \
--cc=yuying.zhang@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).