From: Alexander Kozyrev <akozyrev@nvidia.com>
To: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>,
"dev@dpdk.org" <dev@dpdk.org>
Cc: "NBU-Contact-Thomas Monjalon (EXTERNAL)" <thomas@monjalon.net>,
"ferruh.yigit@amd.com" <ferruh.yigit@amd.com>,
Ori Kam <orika@nvidia.com>
Subject: RE: [PATCH v2 1/4] ethdev: add template table insertion type
Date: Tue, 7 Feb 2023 15:06:08 +0000 [thread overview]
Message-ID: <DM5PR12MB24051181B93AC2B3978A72F4AFDB9@DM5PR12MB2405.namprd12.prod.outlook.com> (raw)
In-Reply-To: <de77ecb8-6db3-1049-0eb0-91c46990c38d@oktetlabs.ru>
> > Allow user to specify insertion type used in template tables.
> > The insertion type is responsible for choosing the appropriate key
> > value used to map inserted flow rules into a template table.
> >
> > Flow rules can be inserted by calculating the hash value for
> > the pattern or inserted by index via the new create_by_index() API.
> > The idea of the index-based insertion is to avoid additional matches
> > and simply execute predefined actions after jumping to the index.
> >
> > The insertion into an already occupied index results in an error.
> > The old rule must be destroyed first. An index cannot be bigger than
> > the size of the table, otherwise, the rule is rejected as well.
>
> I'm sorry, but all my attempts to understand what happens here
> are unsuccessful. Unfortunately the structure is unclear.
> It sounds like your insert something by index into the template
> table, but I don't understand the idea why it makes sense and
> why it is needed.
Sorry, Andrew, for the confusion. Looks like I need to include this explanation from the RFC:
This is how the regular pattern-based table works:
you calculate hash on 5-tuple, go to the table entry with the corresponding hash,
check if there is a collision and search for the proper entry if so, execute actions.
Index-based table:
take an index value from a specified field, go to this index, and execute actions.
The whole idea is to skip any lookups for the packet if an user know exactly what should be executed.
> [snip]
>
> > diff --git a/doc/guides/rel_notes/release_23_03.rst
> b/doc/guides/rel_notes/release_23_03.rst
> > index c15f6fbb9f..fa9391de2b 100644
> > --- a/doc/guides/rel_notes/release_23_03.rst
> > +++ b/doc/guides/rel_notes/release_23_03.rst
> > @@ -69,6 +69,12 @@ New Features
> > ``rte_event_dev_config::nb_single_link_event_port_queues``
> parameter
> > required for eth_rx, eth_tx, crypto and timer eventdev adapters.
> >
> > +* **Added index-based rules insertion in flow API.**
> > +
> > + Added ``rte_flow_table_insertion_type`` to allow the creation
> > + of index-based template tables in addition to pattern-based tables.
> > + Introduced new function ``rte_flow_async_create_by_index()``
> > + to insert rules by index into index-based template tables.
>
> One more empty line here please, to have two before the next
> section.
No problem.
> > Removed Items
> > -------------
> > diff --git a/lib/ethdev/rte_flow.c b/lib/ethdev/rte_flow.c
> > index 7d0c24366c..013eb355ca 100644
> > --- a/lib/ethdev/rte_flow.c
> > +++ b/lib/ethdev/rte_flow.c
> > @@ -1765,6 +1765,30 @@ rte_flow_async_create(uint16_t port_id,
> > return flow;
> > }
> >
> > +struct rte_flow *
> > +rte_flow_async_create_by_index(uint16_t port_id,
> > + uint32_t queue_id,
> > + const struct rte_flow_op_attr *op_attr,
> > + struct rte_flow_template_table *template_table,
> > + uint32_t rule_index,
> > + const struct rte_flow_action actions[],
> > + uint8_t actions_template_index,
> > + void *user_data,
> > + struct rte_flow_error *error)
> > +{
> > + struct rte_eth_dev *dev = &rte_eth_devices[port_id];
> > + const struct rte_flow_ops *ops = rte_flow_ops_get(port_id, error);
> > + struct rte_flow *flow;
>
> All generic checks of arguments must be done here.
> I guess template_table cannot be NULL. Anything else?
> Can we check queue_id?
>
> ops must be checked vs NULL, callback must be checked vs NULL.
We are following the convention about asynchronous API.
No NULL checks are performed in any async Flow API for the sake of performance.
You can see this behaviour in any other async functions there.
> > +
> > + flow = ops->async_create_by_index(dev, queue_id,
> > + op_attr, template_table,
> rule_index,
> > + actions, actions_template_index,
> > + user_data, error);
> > + if (flow == NULL)
> > + flow_err(port_id, -rte_errno, error);
> > + return flow;
> > +}
> > +
> > int
> > rte_flow_async_destroy(uint16_t port_id,
> > uint32_t queue_id,
> > diff --git a/lib/ethdev/rte_flow.h b/lib/ethdev/rte_flow.h
> > index b60987db4b..2ba616eeb1 100644
> > --- a/lib/ethdev/rte_flow.h
> > +++ b/lib/ethdev/rte_flow.h
> > @@ -5187,6 +5187,23 @@ rte_flow_actions_template_destroy(uint16_t
> port_id,
> > */
> > struct rte_flow_template_table;
> >
> > +/**
> > + * @warning
> > + * @b EXPERIMENTAL: this API may change without prior notice.
> > + *
> > + * Template table flow rules insertion type.
> > + */
> > +enum rte_flow_table_insertion_type {
> > + /**
> > + * Pattern-based insertion.
> > + */
> > + RTE_FLOW_TABLE_INSERTION_TYPE_PATTERN,
> > + /**
> > + * Index-based insertion.
> > + */
> > + RTE_FLOW_TABLE_INSERTION_TYPE_INDEX,
> > +};
> > +
> > /**
> > * @warning
> > * @b EXPERIMENTAL: this API may change without prior notice.
> > @@ -5202,6 +5219,10 @@ struct rte_flow_template_table_attr {
> > * Maximum number of flow rules that this table holds.
> > */
> > uint32_t nb_flows;
> > + /**
> > + * Insertion type for flow rules.
> > + */
> > + enum rte_flow_table_insertion_type insertion_type;
> > };
> >
> > /**
> > @@ -5336,6 +5357,50 @@ rte_flow_async_create(uint16_t port_id,
> > void *user_data,
> > struct rte_flow_error *error);
> >
> > +/**
> > + * @warning
> > + * @b EXPERIMENTAL: this API may change without prior notice.
> > + *
> > + * Enqueue rule creation operation.
> > + *
> > + * @param port_id
> > + * Port identifier of Ethernet device.
> > + * @param queue_id
> > + * Flow queue used to insert the rule.
> > + * @param[in] op_attr
> > + * Rule creation operation attributes.
>
> May be NULL?
No.
> > + * @param[in] template_table
> > + * Template table to select templates from.
> > + * @param[in] rule_index
> > + * Rule index in the table.
> > + * @param[in] actions
> > + * List of actions to be used.
> > + * The list order should match the order in the actions template.
>
> Should we mention that it should be terminated with END?
Isn't redundant? Flow API assumes that pattern and actions always end with END.
> > + * @param[in] actions_template_index
> > + * Actions template index in the table.
> > + * @param[in] user_data
> > + * The user data that will be returned on the completion events.
>
> May be NULL?
Yes, it may be NULL if the user doesn’t want any data associated with the flow.
next prev parent reply other threads:[~2023-02-07 15:06 UTC|newest]
Thread overview: 31+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-12-14 2:21 [RFC] ethdev: add template table insertion and matching types Alexander Kozyrev
2023-01-08 14:22 ` Ori Kam
2023-01-18 8:50 ` Thomas Monjalon
2023-01-20 23:06 ` Alexander Kozyrev
2023-01-22 20:55 ` Thomas Monjalon
2023-01-23 22:02 ` Alexander Kozyrev
2023-01-30 14:47 ` Thomas Monjalon
2023-01-21 5:21 ` [PATCH 0/4] " Alexander Kozyrev
2023-01-21 5:21 ` [PATCH 1/4] ethdev: add template table insertion type Alexander Kozyrev
2023-01-21 5:21 ` [PATCH 2/4] ethdev: add template table hash calculation function Alexander Kozyrev
2023-01-21 5:21 ` [PATCH 3/4] app/testpmd: add template table insertion type Alexander Kozyrev
2023-01-21 5:21 ` [PATCH 4/4] app/testpmd: add template table hash calculation function Alexander Kozyrev
2023-01-26 23:27 ` [PATCH v2 0/4] ethdev: add template table insertion and matching types Alexander Kozyrev
2023-01-26 23:27 ` [PATCH v2 1/4] ethdev: add template table insertion type Alexander Kozyrev
2023-02-02 10:57 ` Andrew Rybchenko
2023-02-07 15:06 ` Alexander Kozyrev [this message]
2023-01-26 23:28 ` [PATCH v2 2/4] ethdev: add template table hash calculation function Alexander Kozyrev
2023-01-26 23:28 ` [PATCH v2 3/4] app/testpmd: add template table insertion type Alexander Kozyrev
2023-01-26 23:28 ` [PATCH v2 4/4] app/testpmd: add template table hash calculation function Alexander Kozyrev
2023-02-01 16:09 ` [PATCH v2 0/4] ethdev: add template table insertion and matching types Ori Kam
2023-02-08 2:47 ` [PATCH v3 " Alexander Kozyrev
2023-02-08 2:47 ` [PATCH v3 1/4] ethdev: add template table insertion type Alexander Kozyrev
2023-02-08 2:47 ` [PATCH v3 2/4] ethdev: add template table hash calculation function Alexander Kozyrev
2023-02-08 2:47 ` [PATCH v3 3/4] app/testpmd: add template table insertion type Alexander Kozyrev
2023-02-08 2:47 ` [PATCH v3 4/4] app/testpmd: add template table hash calculation function Alexander Kozyrev
2023-02-08 3:06 ` [PATCH v4 0/4] ethdev: add template table insertion and matching types Alexander Kozyrev
2023-02-08 3:06 ` [PATCH v4 1/4] ethdev: add template table insertion type Alexander Kozyrev
2023-02-08 3:06 ` [PATCH v4 2/4] ethdev: add template table hash calculation function Alexander Kozyrev
2023-02-08 3:06 ` [PATCH v4 3/4] app/testpmd: add template table insertion type Alexander Kozyrev
2023-02-08 3:06 ` [PATCH v4 4/4] app/testpmd: add template table hash calculation function Alexander Kozyrev
2023-02-10 17:42 ` [PATCH v4 0/4] ethdev: add template table insertion and matching types 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=DM5PR12MB24051181B93AC2B3978A72F4AFDB9@DM5PR12MB2405.namprd12.prod.outlook.com \
--to=akozyrev@nvidia.com \
--cc=andrew.rybchenko@oktetlabs.ru \
--cc=dev@dpdk.org \
--cc=ferruh.yigit@amd.com \
--cc=orika@nvidia.com \
--cc=thomas@monjalon.net \
/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).