[v2] ethdev: support flow aging
Checks
Commit Message
One of the reasons to destroy a flow is the fact that no packet matches the
flow for "timeout" time.
For example, when TCP\UDP sessions are suddenly closed.
Currently, there is not any DPDK mechanism for flow aging and the
applications use their own ways to detect and destroy aged-out flows.
The flow aging implementation need include:
- A new rte_flow action: RTE_FLOW_ACTION_TYPE_AGE to set the timeout and
the application flow context for each flow.
- A new ethdev event: RTE_ETH_EVENT_FLOW_AGED for the driver to report
that there are new aged-out flows.
- A new rte_flow API: rte_flow_get_aged_flows to get the aged-out flows
contexts from the port.
- Support input flow aging command line in Testpmd.
Signed-off-by: Dong Zhou <dongz@mellanox.com>
---
app/test-pmd/cmdline_flow.c | 26 ++++++++++
doc/guides/prog_guide/rte_flow.rst | 22 +++++++++
doc/guides/rel_notes/release_20_05.rst | 11 +++++
lib/librte_ethdev/rte_ethdev.h | 1 +
lib/librte_ethdev/rte_ethdev_version.map | 3 ++
lib/librte_ethdev/rte_flow.c | 18 +++++++
lib/librte_ethdev/rte_flow.h | 62 ++++++++++++++++++++++++
lib/librte_ethdev/rte_flow_driver.h | 6 +++
8 files changed, 149 insertions(+)
Comments
> -----Original Message-----
> From: Dong Zhou <dongz@mellanox.com>
> Sent: Tuesday, April 14, 2020 11:33 AM
> To: Ori Kam <orika@mellanox.com>; Matan Azrad <matan@mellanox.com>;
> wenzhuo.lu@intel.com; jingjing.wu@intel.com; bernard.iremonger@intel.com;
> john.mcnamara@intel.com; marko.kovacevic@intel.com; Thomas Monjalon
> <thomas@monjalon.net>; ferruh.yigit@intel.com; arybchenko@solarflare.com
> Cc: dev@dpdk.org
> Subject: [PATCH v2] ethdev: support flow aging
>
> One of the reasons to destroy a flow is the fact that no packet matches the
> flow for "timeout" time.
> For example, when TCP\UDP sessions are suddenly closed.
>
> Currently, there is not any DPDK mechanism for flow aging and the
> applications use their own ways to detect and destroy aged-out flows.
>
> The flow aging implementation need include:
> - A new rte_flow action: RTE_FLOW_ACTION_TYPE_AGE to set the timeout and
> the application flow context for each flow.
> - A new ethdev event: RTE_ETH_EVENT_FLOW_AGED for the driver to report
> that there are new aged-out flows.
> - A new rte_flow API: rte_flow_get_aged_flows to get the aged-out flows
> contexts from the port.
> - Support input flow aging command line in Testpmd.
>
> Signed-off-by: Dong Zhou <dongz@mellanox.com>
> ---
Like said before nice patch and hope to see more patches from you.
Just a small nit please next time add change log.
Acked-by: Ori Kam <orika@mellanox.com>
Thanks,
Ori
> app/test-pmd/cmdline_flow.c | 26 ++++++++++
> doc/guides/prog_guide/rte_flow.rst | 22 +++++++++
> doc/guides/rel_notes/release_20_05.rst | 11 +++++
> lib/librte_ethdev/rte_ethdev.h | 1 +
> lib/librte_ethdev/rte_ethdev_version.map | 3 ++
> lib/librte_ethdev/rte_flow.c | 18 +++++++
> lib/librte_ethdev/rte_flow.h | 62 ++++++++++++++++++++++++
> lib/librte_ethdev/rte_flow_driver.h | 6 +++
> 8 files changed, 149 insertions(+)
>
> diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
> index e6ab8ff2f7..45bcff3cf5 100644
> --- a/app/test-pmd/cmdline_flow.c
> +++ b/app/test-pmd/cmdline_flow.c
> @@ -343,6 +343,8 @@ enum index {
> ACTION_SET_IPV4_DSCP_VALUE,
> ACTION_SET_IPV6_DSCP,
> ACTION_SET_IPV6_DSCP_VALUE,
> + ACTION_AGE,
> + ACTION_AGE_TIMEOUT,
> };
>
> /** Maximum size for pattern in struct rte_flow_item_raw. */
> @@ -1145,6 +1147,7 @@ static const enum index next_action[] = {
> ACTION_SET_META,
> ACTION_SET_IPV4_DSCP,
> ACTION_SET_IPV6_DSCP,
> + ACTION_AGE,
> ZERO,
> };
>
> @@ -1370,6 +1373,13 @@ static const enum index action_set_ipv6_dscp[] = {
> ZERO,
> };
>
> +static const enum index action_age[] = {
> + ACTION_AGE,
> + ACTION_AGE_TIMEOUT,
> + ACTION_NEXT,
> + ZERO,
> +};
> +
> static int parse_set_raw_encap_decap(struct context *, const struct token *,
> const char *, unsigned int,
> void *, unsigned int);
> @@ -3694,6 +3704,22 @@ static const struct token token_list[] = {
> (struct rte_flow_action_set_dscp, dscp)),
> .call = parse_vc_conf,
> },
> + [ACTION_AGE] = {
> + .name = "age",
> + .help = "set a specific metadata header",
> + .next = NEXT(action_age),
> + .priv = PRIV_ACTION(AGE,
> + sizeof(struct rte_flow_action_age)),
> + .call = parse_vc,
> + },
> + [ACTION_AGE_TIMEOUT] = {
> + .name = "timeout",
> + .help = "flow age timeout value",
> + .args = ARGS(ARGS_ENTRY_BF(struct rte_flow_action_age,
> + timeout, 24)),
> + .next = NEXT(action_age, NEXT_ENTRY(UNSIGNED)),
> + .call = parse_vc_conf,
> + },
> };
>
> /** Remove and return last entry from argument stack. */
> diff --git a/doc/guides/prog_guide/rte_flow.rst
> b/doc/guides/prog_guide/rte_flow.rst
> index 41c147913c..cf4368e1c4 100644
> --- a/doc/guides/prog_guide/rte_flow.rst
> +++ b/doc/guides/prog_guide/rte_flow.rst
> @@ -2616,6 +2616,28 @@ Otherwise, RTE_FLOW_ERROR_TYPE_ACTION error
> will be returned.
> | ``dscp`` | DSCP in low 6 bits, rest ignore |
> +-----------+---------------------------------+
>
> +Action: ``AGE``
> +^^^^^^^^^^^^^^^^^^^^^^^^^
> +
> +Set ageing timeout configuration to a flow.
> +
> +Event RTE_ETH_EVENT_FLOW_AGED will be reported if
> +timeout passed without any matching on the flow.
> +
> +.. _table_rte_flow_action_age:
> +
> +.. table:: AGE
> +
> + +--------------+---------------------------------+
> + | Field | Value |
> + +==============+=================================+
> + | ``timeout`` | 24 bits timeout value |
> + +--------------+---------------------------------+
> + | ``reserved`` | 8 bits reserved, must be zero |
> + +--------------+---------------------------------+
> + | ``context`` | user input flow context |
> + +--------------+---------------------------------+
> +
> Negative types
> ~~~~~~~~~~~~~~
>
> diff --git a/doc/guides/rel_notes/release_20_05.rst
> b/doc/guides/rel_notes/release_20_05.rst
> index db885f3609..6b3cd8cda7 100644
> --- a/doc/guides/rel_notes/release_20_05.rst
> +++ b/doc/guides/rel_notes/release_20_05.rst
> @@ -100,6 +100,17 @@ New Features
>
> * Added generic filter support.
>
> +* **Added flow Aging Support.**
> +
> + Added flow Aging support to detect and report aged-out flows, including:
> +
> + * Added new action: RTE_FLOW_ACTION_TYPE_AGE to set the timeout and
> the
> + application flow context for each flow.
> + * Added new event: RTE_ETH_EVENT_FLOW_AGED for the driver to report
> that
> + there are new aged-out flows.
> + * Added new API: rte_flow_get_aged_flows to get the aged-out flows
> contexts
> + from the port.
> +
> Removed Items
> -------------
>
> diff --git a/lib/librte_ethdev/rte_ethdev.h b/lib/librte_ethdev/rte_ethdev.h
> index d1a593ad11..74c9d00f36 100644
> --- a/lib/librte_ethdev/rte_ethdev.h
> +++ b/lib/librte_ethdev/rte_ethdev.h
> @@ -3015,6 +3015,7 @@ enum rte_eth_event_type {
> RTE_ETH_EVENT_NEW, /**< port is probed */
> RTE_ETH_EVENT_DESTROY, /**< port is released */
> RTE_ETH_EVENT_IPSEC, /**< IPsec offload related event */
> + RTE_ETH_EVENT_FLOW_AGED,/**< New aged-out flows is detected */
> RTE_ETH_EVENT_MAX /**< max value of this enum */
> };
>
> diff --git a/lib/librte_ethdev/rte_ethdev_version.map
> b/lib/librte_ethdev/rte_ethdev_version.map
> index 3f32fdecf7..fa4b5816be 100644
> --- a/lib/librte_ethdev/rte_ethdev_version.map
> +++ b/lib/librte_ethdev/rte_ethdev_version.map
> @@ -230,4 +230,7 @@ EXPERIMENTAL {
>
> # added in 20.02
> rte_flow_dev_dump;
> +
> + # added in 20.05
> + rte_flow_get_aged_flows;
> };
> diff --git a/lib/librte_ethdev/rte_flow.c b/lib/librte_ethdev/rte_flow.c
> index a5ac1c7fbd..3699edce49 100644
> --- a/lib/librte_ethdev/rte_flow.c
> +++ b/lib/librte_ethdev/rte_flow.c
> @@ -172,6 +172,7 @@ static const struct rte_flow_desc_data
> rte_flow_desc_action[] = {
> MK_FLOW_ACTION(SET_META, sizeof(struct
> rte_flow_action_set_meta)),
> MK_FLOW_ACTION(SET_IPV4_DSCP, sizeof(struct
> rte_flow_action_set_dscp)),
> MK_FLOW_ACTION(SET_IPV6_DSCP, sizeof(struct
> rte_flow_action_set_dscp)),
> + MK_FLOW_ACTION(AGE, sizeof(struct rte_flow_action_age)),
> };
>
> int
> @@ -1232,3 +1233,20 @@ rte_flow_dev_dump(uint16_t port_id, FILE *file,
> struct rte_flow_error *error)
> RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
> NULL, rte_strerror(ENOSYS));
> }
> +
> +int
> +rte_flow_get_aged_flows(uint16_t port_id, void **contexts,
> + uint32_t nb_contexts, 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);
> +
> + if (unlikely(!ops))
> + return -rte_errno;
> + if (likely(!!ops->get_aged_flows))
> + return flow_err(port_id, ops->get_aged_flows(dev, contexts,
> + nb_contexts, error), error);
> + return rte_flow_error_set(error, ENOTSUP,
> + RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
> + NULL, rte_strerror(ENOTSUP));
> +}
> diff --git a/lib/librte_ethdev/rte_flow.h b/lib/librte_ethdev/rte_flow.h
> index 7f3e08fad3..fab44f6c0b 100644
> --- a/lib/librte_ethdev/rte_flow.h
> +++ b/lib/librte_ethdev/rte_flow.h
> @@ -2081,6 +2081,16 @@ enum rte_flow_action_type {
> * See struct rte_flow_action_set_dscp.
> */
> RTE_FLOW_ACTION_TYPE_SET_IPV6_DSCP,
> +
> + /**
> + * Report as aged flow if timeout passed without any matching on the
> + * flow.
> + *
> + * See struct rte_flow_action_age.
> + * See function rte_flow_get_aged_flows
> + * see enum RTE_ETH_EVENT_FLOW_AGED
> + */
> + RTE_FLOW_ACTION_TYPE_AGE,
> };
>
> /**
> @@ -2122,6 +2132,25 @@ struct rte_flow_action_queue {
> uint16_t index; /**< Queue index to use. */
> };
>
> +/**
> + * @warning
> + * @b EXPERIMENTAL: this structure may change without prior notice
> + *
> + * RTE_FLOW_ACTION_TYPE_AGE
> + *
> + * Report flow as aged-out if timeout passed without any matching
> + * on the flow. RTE_ETH_EVENT_FLOW_AGED event is triggered when a
> + * port detects new aged-out flows.
> + *
> + * The flow context and the flow handle will be reported by the
> + * rte_flow_get_aged_flows API.
> + */
> +struct rte_flow_action_age {
> + uint32_t timeout:24; /**< Time in seconds. */
> + uint32_t reserved:8; /**< Reserved, must be zero. */
> + void *context;
> + /**< The user flow context, NULL means the rte_flow pointer.
> */
> +};
>
> /**
> * @warning
> @@ -3254,6 +3283,39 @@ rte_flow_conv(enum rte_flow_conv_op op,
> const void *src,
> struct rte_flow_error *error);
>
> +/**
> + * Get aged-out flows of a given port.
> + *
> + * RTE_ETH_EVENT_FLOW_AGED event will be triggered when at least one
> new aged
> + * out flow was detected after the last call to rte_flow_get_aged_flows.
> + * This function can be called to get the aged flows usynchronously from the
> + * event callback or synchronously regardless the event.
> + * This is not safe to call rte_flow_get_aged_flows function with other flow
> + * functions from multiple threads simultaneously.
> + *
> + * @param port_id
> + * Port identifier of Ethernet device.
> + * @param[in, out] contexts
> + * The address of an array of pointers to the aged-out flows contexts.
> + * @param[in] nb_contexts
> + * The length of context array pointers.
> + * @param[out] error
> + * Perform verbose error reporting if not NULL. Initialized in case of
> + * error only.
> + *
> + * @return
> + * if nb_contexts is 0, return the amount of all aged contexts.
> + * if nb_contexts is not 0 , return the amount of aged flows reported
> + * in the context array, otherwise negative errno value.
> + *
> + * @see rte_flow_action_age
> + * @see RTE_ETH_EVENT_FLOW_AGED
> + */
> +__rte_experimental
> +int
> +rte_flow_get_aged_flows(uint16_t port_id, void **contexts,
> + uint32_t nb_contexts, struct rte_flow_error *error);
> +
> #ifdef __cplusplus
> }
> #endif
> diff --git a/lib/librte_ethdev/rte_flow_driver.h
> b/lib/librte_ethdev/rte_flow_driver.h
> index 51a9a57a0f..881cc469b7 100644
> --- a/lib/librte_ethdev/rte_flow_driver.h
> +++ b/lib/librte_ethdev/rte_flow_driver.h
> @@ -101,6 +101,12 @@ struct rte_flow_ops {
> (struct rte_eth_dev *dev,
> FILE *file,
> struct rte_flow_error *error);
> + /** See rte_flow_get_aged_flows() */
> + int (*get_aged_flows)
> + (struct rte_eth_dev *dev,
> + void **context,
> + uint32_t nb_contexts,
> + struct rte_flow_error *err);
> };
>
> /**
> --
> 2.21.0
> -----Original Message-----
> From: Ori Kam <orika@mellanox.com>
> Sent: Tuesday, April 14, 2020 4:50 PM
> To: Bill Zhou <dongz@mellanox.com>; Matan Azrad
> <matan@mellanox.com>; wenzhuo.lu@intel.com; jingjing.wu@intel.com;
> bernard.iremonger@intel.com; john.mcnamara@intel.com;
> marko.kovacevic@intel.com; Thomas Monjalon <thomas@monjalon.net>;
> ferruh.yigit@intel.com; arybchenko@solarflare.com
> Cc: dev@dpdk.org
> Subject: RE: [PATCH v2] ethdev: support flow aging
>
>
>
> > -----Original Message-----
> > From: Dong Zhou <dongz@mellanox.com>
> > Sent: Tuesday, April 14, 2020 11:33 AM
> > To: Ori Kam <orika@mellanox.com>; Matan Azrad
> <matan@mellanox.com>;
> > wenzhuo.lu@intel.com; jingjing.wu@intel.com;
> > bernard.iremonger@intel.com; john.mcnamara@intel.com;
> > marko.kovacevic@intel.com; Thomas Monjalon <thomas@monjalon.net>;
> > ferruh.yigit@intel.com; arybchenko@solarflare.com
> > Cc: dev@dpdk.org
> > Subject: [PATCH v2] ethdev: support flow aging
> >
> > One of the reasons to destroy a flow is the fact that no packet
> > matches the flow for "timeout" time.
> > For example, when TCP\UDP sessions are suddenly closed.
> >
> > Currently, there is not any DPDK mechanism for flow aging and the
> > applications use their own ways to detect and destroy aged-out flows.
> >
> > The flow aging implementation need include:
> > - A new rte_flow action: RTE_FLOW_ACTION_TYPE_AGE to set the
> timeout and
> > the application flow context for each flow.
> > - A new ethdev event: RTE_ETH_EVENT_FLOW_AGED for the driver to
> report
> > that there are new aged-out flows.
> > - A new rte_flow API: rte_flow_get_aged_flows to get the aged-out flows
> > contexts from the port.
> > - Support input flow aging command line in Testpmd.
> >
> > Signed-off-by: Dong Zhou <dongz@mellanox.com>
> > ---
> Like said before nice patch and hope to see more patches from you.
> Just a small nit please next time add change log.
>
Sorry for it.
---
v2: Removing "* Added support for flow Aging mechanism base on counter."
this line from doc/guides/rel_notes/release_20_05.rst, this patch does not
include this support.
---
> Acked-by: Ori Kam <orika@mellanox.com>
> Thanks,
> Ori
>
>
> > app/test-pmd/cmdline_flow.c | 26 ++++++++++
> > doc/guides/prog_guide/rte_flow.rst | 22 +++++++++
> > doc/guides/rel_notes/release_20_05.rst | 11 +++++
> > lib/librte_ethdev/rte_ethdev.h | 1 +
> > lib/librte_ethdev/rte_ethdev_version.map | 3 ++
> > lib/librte_ethdev/rte_flow.c | 18 +++++++
> > lib/librte_ethdev/rte_flow.h | 62 ++++++++++++++++++++++++
> > lib/librte_ethdev/rte_flow_driver.h | 6 +++
> > 8 files changed, 149 insertions(+)
> >
> > diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
> > index e6ab8ff2f7..45bcff3cf5 100644
> > --- a/app/test-pmd/cmdline_flow.c
> > +++ b/app/test-pmd/cmdline_flow.c
> > @@ -343,6 +343,8 @@ enum index {
> > ACTION_SET_IPV4_DSCP_VALUE,
> > ACTION_SET_IPV6_DSCP,
> > ACTION_SET_IPV6_DSCP_VALUE,
> > + ACTION_AGE,
> > + ACTION_AGE_TIMEOUT,
> > };
> >
> > /** Maximum size for pattern in struct rte_flow_item_raw. */ @@
> > -1145,6 +1147,7 @@ static const enum index next_action[] = {
> > ACTION_SET_META,
> > ACTION_SET_IPV4_DSCP,
> > ACTION_SET_IPV6_DSCP,
> > + ACTION_AGE,
> > ZERO,
> > };
> >
> > @@ -1370,6 +1373,13 @@ static const enum index
> action_set_ipv6_dscp[] = {
> > ZERO,
> > };
> >
> > +static const enum index action_age[] = {
> > + ACTION_AGE,
> > + ACTION_AGE_TIMEOUT,
> > + ACTION_NEXT,
> > + ZERO,
> > +};
> > +
> > static int parse_set_raw_encap_decap(struct context *, const struct
> token *,
> > const char *, unsigned int,
> > void *, unsigned int);
> > @@ -3694,6 +3704,22 @@ static const struct token token_list[] = {
> > (struct rte_flow_action_set_dscp, dscp)),
> > .call = parse_vc_conf,
> > },
> > + [ACTION_AGE] = {
> > + .name = "age",
> > + .help = "set a specific metadata header",
> > + .next = NEXT(action_age),
> > + .priv = PRIV_ACTION(AGE,
> > + sizeof(struct rte_flow_action_age)),
> > + .call = parse_vc,
> > + },
> > + [ACTION_AGE_TIMEOUT] = {
> > + .name = "timeout",
> > + .help = "flow age timeout value",
> > + .args = ARGS(ARGS_ENTRY_BF(struct rte_flow_action_age,
> > + timeout, 24)),
> > + .next = NEXT(action_age, NEXT_ENTRY(UNSIGNED)),
> > + .call = parse_vc_conf,
> > + },
> > };
> >
> > /** Remove and return last entry from argument stack. */ diff --git
> > a/doc/guides/prog_guide/rte_flow.rst
> > b/doc/guides/prog_guide/rte_flow.rst
> > index 41c147913c..cf4368e1c4 100644
> > --- a/doc/guides/prog_guide/rte_flow.rst
> > +++ b/doc/guides/prog_guide/rte_flow.rst
> > @@ -2616,6 +2616,28 @@ Otherwise, RTE_FLOW_ERROR_TYPE_ACTION
> error
> > will be returned.
> > | ``dscp`` | DSCP in low 6 bits, rest ignore |
> > +-----------+---------------------------------+
> >
> > +Action: ``AGE``
> > +^^^^^^^^^^^^^^^^^^^^^^^^^
> > +
> > +Set ageing timeout configuration to a flow.
> > +
> > +Event RTE_ETH_EVENT_FLOW_AGED will be reported if timeout passed
> > +without any matching on the flow.
> > +
> > +.. _table_rte_flow_action_age:
> > +
> > +.. table:: AGE
> > +
> > + +--------------+---------------------------------+
> > + | Field | Value |
> > + +==============+=================================+
> > + | ``timeout`` | 24 bits timeout value |
> > + +--------------+---------------------------------+
> > + | ``reserved`` | 8 bits reserved, must be zero |
> > + +--------------+---------------------------------+
> > + | ``context`` | user input flow context |
> > + +--------------+---------------------------------+
> > +
> > Negative types
> > ~~~~~~~~~~~~~~
> >
> > diff --git a/doc/guides/rel_notes/release_20_05.rst
> > b/doc/guides/rel_notes/release_20_05.rst
> > index db885f3609..6b3cd8cda7 100644
> > --- a/doc/guides/rel_notes/release_20_05.rst
> > +++ b/doc/guides/rel_notes/release_20_05.rst
> > @@ -100,6 +100,17 @@ New Features
> >
> > * Added generic filter support.
> >
> > +* **Added flow Aging Support.**
> > +
> > + Added flow Aging support to detect and report aged-out flows,
> including:
> > +
> > + * Added new action: RTE_FLOW_ACTION_TYPE_AGE to set the timeout
> and
> > the
> > + application flow context for each flow.
> > + * Added new event: RTE_ETH_EVENT_FLOW_AGED for the driver to
> report
> > that
> > + there are new aged-out flows.
> > + * Added new API: rte_flow_get_aged_flows to get the aged-out flows
> > contexts
> > + from the port.
> > +
> > Removed Items
> > -------------
> >
> > diff --git a/lib/librte_ethdev/rte_ethdev.h
> > b/lib/librte_ethdev/rte_ethdev.h index d1a593ad11..74c9d00f36 100644
> > --- a/lib/librte_ethdev/rte_ethdev.h
> > +++ b/lib/librte_ethdev/rte_ethdev.h
> > @@ -3015,6 +3015,7 @@ enum rte_eth_event_type {
> > RTE_ETH_EVENT_NEW, /**< port is probed */
> > RTE_ETH_EVENT_DESTROY, /**< port is released */
> > RTE_ETH_EVENT_IPSEC, /**< IPsec offload related event */
> > + RTE_ETH_EVENT_FLOW_AGED,/**< New aged-out flows is detected
> */
> > RTE_ETH_EVENT_MAX /**< max value of this enum */
> > };
> >
> > diff --git a/lib/librte_ethdev/rte_ethdev_version.map
> > b/lib/librte_ethdev/rte_ethdev_version.map
> > index 3f32fdecf7..fa4b5816be 100644
> > --- a/lib/librte_ethdev/rte_ethdev_version.map
> > +++ b/lib/librte_ethdev/rte_ethdev_version.map
> > @@ -230,4 +230,7 @@ EXPERIMENTAL {
> >
> > # added in 20.02
> > rte_flow_dev_dump;
> > +
> > + # added in 20.05
> > + rte_flow_get_aged_flows;
> > };
> > diff --git a/lib/librte_ethdev/rte_flow.c
> > b/lib/librte_ethdev/rte_flow.c index a5ac1c7fbd..3699edce49 100644
> > --- a/lib/librte_ethdev/rte_flow.c
> > +++ b/lib/librte_ethdev/rte_flow.c
> > @@ -172,6 +172,7 @@ static const struct rte_flow_desc_data
> > rte_flow_desc_action[] = {
> > MK_FLOW_ACTION(SET_META, sizeof(struct
> rte_flow_action_set_meta)),
> > MK_FLOW_ACTION(SET_IPV4_DSCP, sizeof(struct
> > rte_flow_action_set_dscp)),
> > MK_FLOW_ACTION(SET_IPV6_DSCP, sizeof(struct
> > rte_flow_action_set_dscp)),
> > + MK_FLOW_ACTION(AGE, sizeof(struct rte_flow_action_age)),
> > };
> >
> > int
> > @@ -1232,3 +1233,20 @@ rte_flow_dev_dump(uint16_t port_id, FILE
> *file,
> > struct rte_flow_error *error)
> > RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
> > NULL, rte_strerror(ENOSYS));
> > }
> > +
> > +int
> > +rte_flow_get_aged_flows(uint16_t port_id, void **contexts,
> > + uint32_t nb_contexts, 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);
> > +
> > + if (unlikely(!ops))
> > + return -rte_errno;
> > + if (likely(!!ops->get_aged_flows))
> > + return flow_err(port_id, ops->get_aged_flows(dev, contexts,
> > + nb_contexts, error), error);
> > + return rte_flow_error_set(error, ENOTSUP,
> > + RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
> > + NULL, rte_strerror(ENOTSUP));
> > +}
> > diff --git a/lib/librte_ethdev/rte_flow.h
> > b/lib/librte_ethdev/rte_flow.h index 7f3e08fad3..fab44f6c0b 100644
> > --- a/lib/librte_ethdev/rte_flow.h
> > +++ b/lib/librte_ethdev/rte_flow.h
> > @@ -2081,6 +2081,16 @@ enum rte_flow_action_type {
> > * See struct rte_flow_action_set_dscp.
> > */
> > RTE_FLOW_ACTION_TYPE_SET_IPV6_DSCP,
> > +
> > + /**
> > + * Report as aged flow if timeout passed without any matching on
> the
> > + * flow.
> > + *
> > + * See struct rte_flow_action_age.
> > + * See function rte_flow_get_aged_flows
> > + * see enum RTE_ETH_EVENT_FLOW_AGED
> > + */
> > + RTE_FLOW_ACTION_TYPE_AGE,
> > };
> >
> > /**
> > @@ -2122,6 +2132,25 @@ struct rte_flow_action_queue {
> > uint16_t index; /**< Queue index to use. */ };
> >
> > +/**
> > + * @warning
> > + * @b EXPERIMENTAL: this structure may change without prior notice
> > + *
> > + * RTE_FLOW_ACTION_TYPE_AGE
> > + *
> > + * Report flow as aged-out if timeout passed without any matching
> > + * on the flow. RTE_ETH_EVENT_FLOW_AGED event is triggered when a
> > + * port detects new aged-out flows.
> > + *
> > + * The flow context and the flow handle will be reported by the
> > + * rte_flow_get_aged_flows API.
> > + */
> > +struct rte_flow_action_age {
> > + uint32_t timeout:24; /**< Time in seconds. */
> > + uint32_t reserved:8; /**< Reserved, must be zero. */
> > + void *context;
> > + /**< The user flow context, NULL means the rte_flow
> pointer.
> > */
> > +};
> >
> > /**
> > * @warning
> > @@ -3254,6 +3283,39 @@ rte_flow_conv(enum rte_flow_conv_op op,
> > const void *src,
> > struct rte_flow_error *error);
> >
> > +/**
> > + * Get aged-out flows of a given port.
> > + *
> > + * RTE_ETH_EVENT_FLOW_AGED event will be triggered when at least
> one
> > new aged
> > + * out flow was detected after the last call to rte_flow_get_aged_flows.
> > + * This function can be called to get the aged flows usynchronously
> > +from the
> > + * event callback or synchronously regardless the event.
> > + * This is not safe to call rte_flow_get_aged_flows function with
> > +other flow
> > + * functions from multiple threads simultaneously.
> > + *
> > + * @param port_id
> > + * Port identifier of Ethernet device.
> > + * @param[in, out] contexts
> > + * The address of an array of pointers to the aged-out flows contexts.
> > + * @param[in] nb_contexts
> > + * The length of context array pointers.
> > + * @param[out] error
> > + * Perform verbose error reporting if not NULL. Initialized in case of
> > + * error only.
> > + *
> > + * @return
> > + * if nb_contexts is 0, return the amount of all aged contexts.
> > + * if nb_contexts is not 0 , return the amount of aged flows reported
> > + * in the context array, otherwise negative errno value.
> > + *
> > + * @see rte_flow_action_age
> > + * @see RTE_ETH_EVENT_FLOW_AGED
> > + */
> > +__rte_experimental
> > +int
> > +rte_flow_get_aged_flows(uint16_t port_id, void **contexts,
> > + uint32_t nb_contexts, struct rte_flow_error *error);
> > +
> > #ifdef __cplusplus
> > }
> > #endif
> > diff --git a/lib/librte_ethdev/rte_flow_driver.h
> > b/lib/librte_ethdev/rte_flow_driver.h
> > index 51a9a57a0f..881cc469b7 100644
> > --- a/lib/librte_ethdev/rte_flow_driver.h
> > +++ b/lib/librte_ethdev/rte_flow_driver.h
> > @@ -101,6 +101,12 @@ struct rte_flow_ops {
> > (struct rte_eth_dev *dev,
> > FILE *file,
> > struct rte_flow_error *error);
> > + /** See rte_flow_get_aged_flows() */
> > + int (*get_aged_flows)
> > + (struct rte_eth_dev *dev,
> > + void **context,
> > + uint32_t nb_contexts,
> > + struct rte_flow_error *err);
> > };
> >
> > /**
> > --
> > 2.21.0
On 4/14/2020 9:49 AM, Ori Kam wrote:
>
>
>> -----Original Message-----
>> From: Dong Zhou <dongz@mellanox.com>
>> Sent: Tuesday, April 14, 2020 11:33 AM
>> To: Ori Kam <orika@mellanox.com>; Matan Azrad <matan@mellanox.com>;
>> wenzhuo.lu@intel.com; jingjing.wu@intel.com; bernard.iremonger@intel.com;
>> john.mcnamara@intel.com; marko.kovacevic@intel.com; Thomas Monjalon
>> <thomas@monjalon.net>; ferruh.yigit@intel.com; arybchenko@solarflare.com
>> Cc: dev@dpdk.org
>> Subject: [PATCH v2] ethdev: support flow aging
>>
>> One of the reasons to destroy a flow is the fact that no packet matches the
>> flow for "timeout" time.
>> For example, when TCP\UDP sessions are suddenly closed.
>>
>> Currently, there is not any DPDK mechanism for flow aging and the
>> applications use their own ways to detect and destroy aged-out flows.
>>
>> The flow aging implementation need include:
>> - A new rte_flow action: RTE_FLOW_ACTION_TYPE_AGE to set the timeout and
>> the application flow context for each flow.
>> - A new ethdev event: RTE_ETH_EVENT_FLOW_AGED for the driver to report
>> that there are new aged-out flows.
>> - A new rte_flow API: rte_flow_get_aged_flows to get the aged-out flows
>> contexts from the port.
>> - Support input flow aging command line in Testpmd.
>>
>> Signed-off-by: Dong Zhou <dongz@mellanox.com>
>> ---
> Like said before nice patch and hope to see more patches from you.
> Just a small nit please next time add change log.
>
> Acked-by: Ori Kam <orika@mellanox.com>
Moved other acks from v1:
Acked-by: Andrew Rybchenko <arybchenko@solarflare.com>
Acked-by: Jerin Jacob <jerinj@marvell.com>
Acked-by: Matan Azrad <matan@mellanox.com>
Applied to dpdk-next-net/master, thanks.
On 4/14/2020 9:32 AM, Dong Zhou wrote:
> One of the reasons to destroy a flow is the fact that no packet matches the
> flow for "timeout" time.
> For example, when TCP\UDP sessions are suddenly closed.
>
> Currently, there is not any DPDK mechanism for flow aging and the
> applications use their own ways to detect and destroy aged-out flows.
>
> The flow aging implementation need include:
> - A new rte_flow action: RTE_FLOW_ACTION_TYPE_AGE to set the timeout and
> the application flow context for each flow.
> - A new ethdev event: RTE_ETH_EVENT_FLOW_AGED for the driver to report
> that there are new aged-out flows.
> - A new rte_flow API: rte_flow_get_aged_flows to get the aged-out flows
> contexts from the port.
> - Support input flow aging command line in Testpmd.
>
> Signed-off-by: Dong Zhou <dongz@mellanox.com>
<...>
> --- a/lib/librte_ethdev/rte_ethdev.h
> +++ b/lib/librte_ethdev/rte_ethdev.h
> @@ -3015,6 +3015,7 @@ enum rte_eth_event_type {
> RTE_ETH_EVENT_NEW, /**< port is probed */
> RTE_ETH_EVENT_DESTROY, /**< port is released */
> RTE_ETH_EVENT_IPSEC, /**< IPsec offload related event */
> + RTE_ETH_EVENT_FLOW_AGED,/**< New aged-out flows is detected */
> RTE_ETH_EVENT_MAX /**< max value of this enum */
> };
Just recognized that this is failing in ABI check [1], as far as last time for a
similar enum warning a QAT patch has been dropped, should this need to wait for
20.11 too?
[1]
[C]'function int _rte_eth_dev_callback_process(rte_eth_dev*,
rte_eth_event_type, void*)' at rte_ethdev.c:4063:1 has some indirect sub-type
changes:
parameter 2 of type 'enum rte_eth_event_type' has sub-type changes:
type size hasn't changed
1 enumerator insertion:
'rte_eth_event_type::RTE_ETH_EVENT_FLOW_AGED' value '10'
1 enumerator change:
'rte_eth_event_type::RTE_ETH_EVENT_MAX' from value '10' to '11' at
rte_ethdev.h:3008:1
On Fri, 17 Apr 2020 23:00:57 +0100
Ferruh Yigit <ferruh.yigit@intel.com> wrote:
> On 4/14/2020 9:32 AM, Dong Zhou wrote:
> > One of the reasons to destroy a flow is the fact that no packet matches the
> > flow for "timeout" time.
> > For example, when TCP\UDP sessions are suddenly closed.
> >
> > Currently, there is not any DPDK mechanism for flow aging and the
> > applications use their own ways to detect and destroy aged-out flows.
> >
> > The flow aging implementation need include:
> > - A new rte_flow action: RTE_FLOW_ACTION_TYPE_AGE to set the timeout and
> > the application flow context for each flow.
> > - A new ethdev event: RTE_ETH_EVENT_FLOW_AGED for the driver to report
> > that there are new aged-out flows.
> > - A new rte_flow API: rte_flow_get_aged_flows to get the aged-out flows
> > contexts from the port.
> > - Support input flow aging command line in Testpmd.
> >
> > Signed-off-by: Dong Zhou <dongz@mellanox.com>
>
> <...>
>
> > --- a/lib/librte_ethdev/rte_ethdev.h
> > +++ b/lib/librte_ethdev/rte_ethdev.h
> > @@ -3015,6 +3015,7 @@ enum rte_eth_event_type {
> > RTE_ETH_EVENT_NEW, /**< port is probed */
> > RTE_ETH_EVENT_DESTROY, /**< port is released */
> > RTE_ETH_EVENT_IPSEC, /**< IPsec offload related event */
> > + RTE_ETH_EVENT_FLOW_AGED,/**< New aged-out flows is detected */
> > RTE_ETH_EVENT_MAX /**< max value of this enum */
> > };
>
>
> Just recognized that this is failing in ABI check [1], as far as last time for a
> similar enum warning a QAT patch has been dropped, should this need to wait for
> 20.11 too?
>
>
> [1]
> [C]'function int _rte_eth_dev_callback_process(rte_eth_dev*,
> rte_eth_event_type, void*)' at rte_ethdev.c:4063:1 has some indirect sub-type
> changes:
> parameter 2 of type 'enum rte_eth_event_type' has sub-type changes:
> type size hasn't changed
> 1 enumerator insertion:
> 'rte_eth_event_type::RTE_ETH_EVENT_FLOW_AGED' value '10'
> 1 enumerator change:
> 'rte_eth_event_type::RTE_ETH_EVENT_MAX' from value '10' to '11' at
> rte_ethdev.h:3008:1
>
For 20.11, those _MAX values need to be removed from enums
> -----Original Message-----
> From: Ferruh Yigit <ferruh.yigit@intel.com>
> Sent: Saturday, April 18, 2020 6:01 AM
> To: Bill Zhou <dongz@mellanox.com>; Ori Kam <orika@mellanox.com>;
> Matan Azrad <matan@mellanox.com>; wenzhuo.lu@intel.com;
> jingjing.wu@intel.com; bernard.iremonger@intel.com;
> john.mcnamara@intel.com; marko.kovacevic@intel.com; Thomas Monjalon
> <thomas@monjalon.net>; arybchenko@solarflare.com
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v2] ethdev: support flow aging
>
> On 4/14/2020 9:32 AM, Dong Zhou wrote:
> > One of the reasons to destroy a flow is the fact that no packet
> > matches the flow for "timeout" time.
> > For example, when TCP\UDP sessions are suddenly closed.
> >
> > Currently, there is not any DPDK mechanism for flow aging and the
> > applications use their own ways to detect and destroy aged-out flows.
> >
> > The flow aging implementation need include:
> > - A new rte_flow action: RTE_FLOW_ACTION_TYPE_AGE to set the
> timeout and
> > the application flow context for each flow.
> > - A new ethdev event: RTE_ETH_EVENT_FLOW_AGED for the driver to
> report
> > that there are new aged-out flows.
> > - A new rte_flow API: rte_flow_get_aged_flows to get the aged-out flows
> > contexts from the port.
> > - Support input flow aging command line in Testpmd.
> >
> > Signed-off-by: Dong Zhou <dongz@mellanox.com>
>
> <...>
>
> > --- a/lib/librte_ethdev/rte_ethdev.h
> > +++ b/lib/librte_ethdev/rte_ethdev.h
> > @@ -3015,6 +3015,7 @@ enum rte_eth_event_type {
> > RTE_ETH_EVENT_NEW, /**< port is probed */
> > RTE_ETH_EVENT_DESTROY, /**< port is released */
> > RTE_ETH_EVENT_IPSEC, /**< IPsec offload related event */
> > + RTE_ETH_EVENT_FLOW_AGED,/**< New aged-out flows is detected
> */
> > RTE_ETH_EVENT_MAX /**< max value of this enum */
> > };
>
>
> Just recognized that this is failing in ABI check [1], as far as last time for a
> similar enum warning a QAT patch has been dropped, should this need to
> wait for
> 20.11 too?
This patch is commonly used for flow aging, there are 2 other patches have
implement flow aging in mlx5 driver reply to this patch.
In our schedule, this feature is merged in 20.05 for some customers. Can it
be fixed?
>
>
> [1]
> [C]'function int _rte_eth_dev_callback_process(rte_eth_dev*,
> rte_eth_event_type, void*)' at rte_ethdev.c:4063:1 has some indirect sub-
> type
> changes:
> parameter 2 of type 'enum rte_eth_event_type' has sub-type changes:
> type size hasn't changed
> 1 enumerator insertion:
> 'rte_eth_event_type::RTE_ETH_EVENT_FLOW_AGED' value '10'
> 1 enumerator change:
> 'rte_eth_event_type::RTE_ETH_EVENT_MAX' from value '10' to '11' at
> rte_ethdev.h:3008:1
18/04/2020 07:04, Bill Zhou:
> From: Ferruh Yigit <ferruh.yigit@intel.com>
> > On 4/14/2020 9:32 AM, Dong Zhou wrote:
> > > --- a/lib/librte_ethdev/rte_ethdev.h
> > > +++ b/lib/librte_ethdev/rte_ethdev.h
> > > @@ -3015,6 +3015,7 @@ enum rte_eth_event_type {
> > > RTE_ETH_EVENT_NEW, /**< port is probed */
> > > RTE_ETH_EVENT_DESTROY, /**< port is released */
> > > RTE_ETH_EVENT_IPSEC, /**< IPsec offload related event */
> > > + RTE_ETH_EVENT_FLOW_AGED,/**< New aged-out flows is detected
> > */
> > > RTE_ETH_EVENT_MAX /**< max value of this enum */
> > > };
> >
> >
> > Just recognized that this is failing in ABI check [1], as far as last time for a
> > similar enum warning a QAT patch has been dropped, should this need to
> > wait for
> > 20.11 too?
>
> This patch is commonly used for flow aging, there are 2 other patches have
> implement flow aging in mlx5 driver reply to this patch.
> In our schedule, this feature is merged in 20.05 for some customers. Can it
> be fixed?
These MAX values in enums are a pain.
We can try to think what can be done, waiting 20.11.
Not sure there is a solution, except hijacking an existing value
not used in the PMD, waiting the definitive value in 20.11...
On 4/18/2020 10:44 AM, Thomas Monjalon wrote:
> 18/04/2020 07:04, Bill Zhou:
>> From: Ferruh Yigit <ferruh.yigit@intel.com>
>>> On 4/14/2020 9:32 AM, Dong Zhou wrote:
>>>> --- a/lib/librte_ethdev/rte_ethdev.h
>>>> +++ b/lib/librte_ethdev/rte_ethdev.h
>>>> @@ -3015,6 +3015,7 @@ enum rte_eth_event_type {
>>>> RTE_ETH_EVENT_NEW, /**< port is probed */
>>>> RTE_ETH_EVENT_DESTROY, /**< port is released */
>>>> RTE_ETH_EVENT_IPSEC, /**< IPsec offload related event */
>>>> + RTE_ETH_EVENT_FLOW_AGED,/**< New aged-out flows is detected
>>> */
>>>> RTE_ETH_EVENT_MAX /**< max value of this enum */
>>>> };
>>>
>>>
>>> Just recognized that this is failing in ABI check [1], as far as last time for a
>>> similar enum warning a QAT patch has been dropped, should this need to
>>> wait for
>>> 20.11 too?
>>
>> This patch is commonly used for flow aging, there are 2 other patches have
>> implement flow aging in mlx5 driver reply to this patch.
>> In our schedule, this feature is merged in 20.05 for some customers. Can it
>> be fixed?
>
> These MAX values in enums are a pain.
> We can try to think what can be done, waiting 20.11.
> Not sure there is a solution, except hijacking an existing value
> not used in the PMD, waiting the definitive value in 20.11...
>
Dropping from the tree as of now, to not cause more merge conflicts, we can add
it later when issue is resolved.
20/04/2020 16:06, Ferruh Yigit:
> On 4/18/2020 10:44 AM, Thomas Monjalon wrote:
> > 18/04/2020 07:04, Bill Zhou:
> >> From: Ferruh Yigit <ferruh.yigit@intel.com>
> >>> On 4/14/2020 9:32 AM, Dong Zhou wrote:
> >>>> --- a/lib/librte_ethdev/rte_ethdev.h
> >>>> +++ b/lib/librte_ethdev/rte_ethdev.h
> >>>> @@ -3015,6 +3015,7 @@ enum rte_eth_event_type {
> >>>> RTE_ETH_EVENT_NEW, /**< port is probed */
> >>>> RTE_ETH_EVENT_DESTROY, /**< port is released */
> >>>> RTE_ETH_EVENT_IPSEC, /**< IPsec offload related event */
> >>>> + RTE_ETH_EVENT_FLOW_AGED,/**< New aged-out flows is detected
> >>> */
> >>>> RTE_ETH_EVENT_MAX /**< max value of this enum */
> >>>> };
> >>>
> >>>
> >>> Just recognized that this is failing in ABI check [1], as far as last time for a
> >>> similar enum warning a QAT patch has been dropped, should this need to
> >>> wait for
> >>> 20.11 too?
> >>
> >> This patch is commonly used for flow aging, there are 2 other patches have
> >> implement flow aging in mlx5 driver reply to this patch.
[...]
> > These MAX values in enums are a pain.
> > We can try to think what can be done, waiting 20.11.
> > Not sure there is a solution, except hijacking an existing value
> > not used in the PMD, waiting the definitive value in 20.11...
>
> Dropping from the tree as of now, to not cause more merge conflicts, we can add
> it later when issue is resolved.
Thanks for dropping, that's the right thing to do
when a patch is breaking ABI check.
After some thoughts, I think it is acceptable to make a v3
which ignore this specific enum change. I explain my thought below:
An enum can accept a new value at 2 conditions:
- added as last value (not changing old values)
- new value not used by existing API
The value RTE_ETH_EVENT_FLOW_AGED meet the above 2 conditions:
- only RTE_ETH_EVENT_MAX is changed, which is consistent
- new value sent to the app only if the app registered for it
So, except if I miss something, I suggest we add this exception:
Allow new value in rte_eth_event_type if added just before RTE_ETH_EVENT_MAX.
In other words, allow changing the value of RTE_ETH_EVENT_MAX.
The file to add such exception is devtools/libabigail.abignore.
On 4/20/2020 5:10 PM, Thomas Monjalon wrote:
> 20/04/2020 16:06, Ferruh Yigit:
>> On 4/18/2020 10:44 AM, Thomas Monjalon wrote:
>>> 18/04/2020 07:04, Bill Zhou:
>>>> From: Ferruh Yigit <ferruh.yigit@intel.com>
>>>>> On 4/14/2020 9:32 AM, Dong Zhou wrote:
>>>>>> --- a/lib/librte_ethdev/rte_ethdev.h
>>>>>> +++ b/lib/librte_ethdev/rte_ethdev.h
>>>>>> @@ -3015,6 +3015,7 @@ enum rte_eth_event_type {
>>>>>> RTE_ETH_EVENT_NEW, /**< port is probed */
>>>>>> RTE_ETH_EVENT_DESTROY, /**< port is released */
>>>>>> RTE_ETH_EVENT_IPSEC, /**< IPsec offload related event */
>>>>>> + RTE_ETH_EVENT_FLOW_AGED,/**< New aged-out flows is detected
>>>>> */
>>>>>> RTE_ETH_EVENT_MAX /**< max value of this enum */
>>>>>> };
>>>>>
>>>>>
>>>>> Just recognized that this is failing in ABI check [1], as far as last time for a
>>>>> similar enum warning a QAT patch has been dropped, should this need to
>>>>> wait for
>>>>> 20.11 too?
>>>>
>>>> This patch is commonly used for flow aging, there are 2 other patches have
>>>> implement flow aging in mlx5 driver reply to this patch.
> [...]
>>> These MAX values in enums are a pain.
>>> We can try to think what can be done, waiting 20.11.
>>> Not sure there is a solution, except hijacking an existing value
>>> not used in the PMD, waiting the definitive value in 20.11...
>>
>> Dropping from the tree as of now, to not cause more merge conflicts, we can add
>> it later when issue is resolved.
>
> Thanks for dropping, that's the right thing to do
> when a patch is breaking ABI check.
>
> After some thoughts, I think it is acceptable to make a v3
> which ignore this specific enum change. I explain my thought below:
>
> An enum can accept a new value at 2 conditions:
> - added as last value (not changing old values)
> - new value not used by existing API
>
> The value RTE_ETH_EVENT_FLOW_AGED meet the above 2 conditions:
> - only RTE_ETH_EVENT_MAX is changed, which is consistent
> - new value sent to the app only if the app registered for it
>
Same here, as far as I can see it is safe to get this change.
If any DPDK API returns this enum, either as return of the API or as output
parameter, this still can be problem, because application may use that returned
value, this was the concern in the QAT sample.
But here application registers an event and DPDK library process callback for
it, so application callbacks won't be called for anything that application
doesn't already know about, in that respect this should be safe for old
applications.
Not sure if we can generalize above two conditions for all enum changes, but we
can investigate them case by case as we get the warnings.
> So, except if I miss something, I suggest we add this exception:
> Allow new value in rte_eth_event_type if added just before RTE_ETH_EVENT_MAX.
> In other words, allow changing the value of RTE_ETH_EVENT_MAX.
> The file to add such exception is devtools/libabigail.abignore.
>
OK to exception.
21/04/2020 12:04, Ferruh Yigit:
> On 4/20/2020 5:10 PM, Thomas Monjalon wrote:
> > 20/04/2020 16:06, Ferruh Yigit:
> >> On 4/18/2020 10:44 AM, Thomas Monjalon wrote:
> >>> 18/04/2020 07:04, Bill Zhou:
> >>>> From: Ferruh Yigit <ferruh.yigit@intel.com>
> >>>>> On 4/14/2020 9:32 AM, Dong Zhou wrote:
> >>>>>> --- a/lib/librte_ethdev/rte_ethdev.h
> >>>>>> +++ b/lib/librte_ethdev/rte_ethdev.h
> >>>>>> @@ -3015,6 +3015,7 @@ enum rte_eth_event_type {
> >>>>>> RTE_ETH_EVENT_NEW, /**< port is probed */
> >>>>>> RTE_ETH_EVENT_DESTROY, /**< port is released */
> >>>>>> RTE_ETH_EVENT_IPSEC, /**< IPsec offload related event */
> >>>>>> + RTE_ETH_EVENT_FLOW_AGED,/**< New aged-out flows is detected
> >>>>> */
> >>>>>> RTE_ETH_EVENT_MAX /**< max value of this enum */
> >>>>>> };
> >>>>>
> >>>>>
> >>>>> Just recognized that this is failing in ABI check [1], as far as last time for a
> >>>>> similar enum warning a QAT patch has been dropped, should this need to
> >>>>> wait for
> >>>>> 20.11 too?
> >>>>
> >>>> This patch is commonly used for flow aging, there are 2 other patches have
> >>>> implement flow aging in mlx5 driver reply to this patch.
> > [...]
> >>> These MAX values in enums are a pain.
> >>> We can try to think what can be done, waiting 20.11.
> >>> Not sure there is a solution, except hijacking an existing value
> >>> not used in the PMD, waiting the definitive value in 20.11...
> >>
> >> Dropping from the tree as of now, to not cause more merge conflicts, we can add
> >> it later when issue is resolved.
> >
> > Thanks for dropping, that's the right thing to do
> > when a patch is breaking ABI check.
> >
> > After some thoughts, I think it is acceptable to make a v3
> > which ignore this specific enum change. I explain my thought below:
> >
> > An enum can accept a new value at 2 conditions:
> > - added as last value (not changing old values)
> > - new value not used by existing API
> >
> > The value RTE_ETH_EVENT_FLOW_AGED meet the above 2 conditions:
> > - only RTE_ETH_EVENT_MAX is changed, which is consistent
> > - new value sent to the app only if the app registered for it
> >
>
> Same here, as far as I can see it is safe to get this change.
>
> If any DPDK API returns this enum, either as return of the API or as output
> parameter, this still can be problem, because application may use that returned
> value, this was the concern in the QAT sample.
>
> But here application registers an event and DPDK library process callback for
> it, so application callbacks won't be called for anything that application
> doesn't already know about, in that respect this should be safe for old
> applications.
>
> Not sure if we can generalize above two conditions for all enum changes, but we
> can investigate them case by case as we get the warnings.
>
> > So, except if I miss something, I suggest we add this exception:
> > Allow new value in rte_eth_event_type if added just before RTE_ETH_EVENT_MAX.
> > In other words, allow changing the value of RTE_ETH_EVENT_MAX.
> > The file to add such exception is devtools/libabigail.abignore.
> >
>
> OK to exception.
v3 was sent.
I hope we'll get a v4 with justification for the exception.
On 4/21/20 1:04 PM, Ferruh Yigit wrote:
> On 4/20/2020 5:10 PM, Thomas Monjalon wrote:
>> 20/04/2020 16:06, Ferruh Yigit:
>>> On 4/18/2020 10:44 AM, Thomas Monjalon wrote:
>>>> 18/04/2020 07:04, Bill Zhou:
>>>>> From: Ferruh Yigit <ferruh.yigit@intel.com>
>>>>>> On 4/14/2020 9:32 AM, Dong Zhou wrote:
>>>>>>> --- a/lib/librte_ethdev/rte_ethdev.h
>>>>>>> +++ b/lib/librte_ethdev/rte_ethdev.h
>>>>>>> @@ -3015,6 +3015,7 @@ enum rte_eth_event_type {
>>>>>>> RTE_ETH_EVENT_NEW, /**< port is probed */
>>>>>>> RTE_ETH_EVENT_DESTROY, /**< port is released */
>>>>>>> RTE_ETH_EVENT_IPSEC, /**< IPsec offload related event */
>>>>>>> + RTE_ETH_EVENT_FLOW_AGED,/**< New aged-out flows is detected
>>>>>> */
>>>>>>> RTE_ETH_EVENT_MAX /**< max value of this enum */
>>>>>>> };
>>>>>>
>>>>>>
>>>>>> Just recognized that this is failing in ABI check [1], as far as last time for a
>>>>>> similar enum warning a QAT patch has been dropped, should this need to
>>>>>> wait for
>>>>>> 20.11 too?
>>>>>
>>>>> This patch is commonly used for flow aging, there are 2 other patches have
>>>>> implement flow aging in mlx5 driver reply to this patch.
>> [...]
>>>> These MAX values in enums are a pain.
>>>> We can try to think what can be done, waiting 20.11.
>>>> Not sure there is a solution, except hijacking an existing value
>>>> not used in the PMD, waiting the definitive value in 20.11...
>>>
>>> Dropping from the tree as of now, to not cause more merge conflicts, we can add
>>> it later when issue is resolved.
>>
>> Thanks for dropping, that's the right thing to do
>> when a patch is breaking ABI check.
>>
>> After some thoughts, I think it is acceptable to make a v3
>> which ignore this specific enum change. I explain my thought below:
>>
>> An enum can accept a new value at 2 conditions:
>> - added as last value (not changing old values)
>> - new value not used by existing API
>>
>> The value RTE_ETH_EVENT_FLOW_AGED meet the above 2 conditions:
>> - only RTE_ETH_EVENT_MAX is changed, which is consistent
>> - new value sent to the app only if the app registered for it
>>
>
> Same here, as far as I can see it is safe to get this change.
>
> If any DPDK API returns this enum, either as return of the API or as output
> parameter, this still can be problem, because application may use that returned
> value, this was the concern in the QAT sample.
>
> But here application registers an event and DPDK library process callback for
> it, so application callbacks won't be called for anything that application
> doesn't already know about, in that respect this should be safe for old
> applications.
>
> Not sure if we can generalize above two conditions for all enum changes, but we
> can investigate them case by case as we get the warnings.
>
>> So, except if I miss something, I suggest we add this exception:
>> Allow new value in rte_eth_event_type if added just before RTE_ETH_EVENT_MAX.
>> In other words, allow changing the value of RTE_ETH_EVENT_MAX.
>> The file to add such exception is devtools/libabigail.abignore.
>>
>
> OK to exception.
Me too
@@ -343,6 +343,8 @@ enum index {
ACTION_SET_IPV4_DSCP_VALUE,
ACTION_SET_IPV6_DSCP,
ACTION_SET_IPV6_DSCP_VALUE,
+ ACTION_AGE,
+ ACTION_AGE_TIMEOUT,
};
/** Maximum size for pattern in struct rte_flow_item_raw. */
@@ -1145,6 +1147,7 @@ static const enum index next_action[] = {
ACTION_SET_META,
ACTION_SET_IPV4_DSCP,
ACTION_SET_IPV6_DSCP,
+ ACTION_AGE,
ZERO,
};
@@ -1370,6 +1373,13 @@ static const enum index action_set_ipv6_dscp[] = {
ZERO,
};
+static const enum index action_age[] = {
+ ACTION_AGE,
+ ACTION_AGE_TIMEOUT,
+ ACTION_NEXT,
+ ZERO,
+};
+
static int parse_set_raw_encap_decap(struct context *, const struct token *,
const char *, unsigned int,
void *, unsigned int);
@@ -3694,6 +3704,22 @@ static const struct token token_list[] = {
(struct rte_flow_action_set_dscp, dscp)),
.call = parse_vc_conf,
},
+ [ACTION_AGE] = {
+ .name = "age",
+ .help = "set a specific metadata header",
+ .next = NEXT(action_age),
+ .priv = PRIV_ACTION(AGE,
+ sizeof(struct rte_flow_action_age)),
+ .call = parse_vc,
+ },
+ [ACTION_AGE_TIMEOUT] = {
+ .name = "timeout",
+ .help = "flow age timeout value",
+ .args = ARGS(ARGS_ENTRY_BF(struct rte_flow_action_age,
+ timeout, 24)),
+ .next = NEXT(action_age, NEXT_ENTRY(UNSIGNED)),
+ .call = parse_vc_conf,
+ },
};
/** Remove and return last entry from argument stack. */
@@ -2616,6 +2616,28 @@ Otherwise, RTE_FLOW_ERROR_TYPE_ACTION error will be returned.
| ``dscp`` | DSCP in low 6 bits, rest ignore |
+-----------+---------------------------------+
+Action: ``AGE``
+^^^^^^^^^^^^^^^^^^^^^^^^^
+
+Set ageing timeout configuration to a flow.
+
+Event RTE_ETH_EVENT_FLOW_AGED will be reported if
+timeout passed without any matching on the flow.
+
+.. _table_rte_flow_action_age:
+
+.. table:: AGE
+
+ +--------------+---------------------------------+
+ | Field | Value |
+ +==============+=================================+
+ | ``timeout`` | 24 bits timeout value |
+ +--------------+---------------------------------+
+ | ``reserved`` | 8 bits reserved, must be zero |
+ +--------------+---------------------------------+
+ | ``context`` | user input flow context |
+ +--------------+---------------------------------+
+
Negative types
~~~~~~~~~~~~~~
@@ -100,6 +100,17 @@ New Features
* Added generic filter support.
+* **Added flow Aging Support.**
+
+ Added flow Aging support to detect and report aged-out flows, including:
+
+ * Added new action: RTE_FLOW_ACTION_TYPE_AGE to set the timeout and the
+ application flow context for each flow.
+ * Added new event: RTE_ETH_EVENT_FLOW_AGED for the driver to report that
+ there are new aged-out flows.
+ * Added new API: rte_flow_get_aged_flows to get the aged-out flows contexts
+ from the port.
+
Removed Items
-------------
@@ -3015,6 +3015,7 @@ enum rte_eth_event_type {
RTE_ETH_EVENT_NEW, /**< port is probed */
RTE_ETH_EVENT_DESTROY, /**< port is released */
RTE_ETH_EVENT_IPSEC, /**< IPsec offload related event */
+ RTE_ETH_EVENT_FLOW_AGED,/**< New aged-out flows is detected */
RTE_ETH_EVENT_MAX /**< max value of this enum */
};
@@ -230,4 +230,7 @@ EXPERIMENTAL {
# added in 20.02
rte_flow_dev_dump;
+
+ # added in 20.05
+ rte_flow_get_aged_flows;
};
@@ -172,6 +172,7 @@ static const struct rte_flow_desc_data rte_flow_desc_action[] = {
MK_FLOW_ACTION(SET_META, sizeof(struct rte_flow_action_set_meta)),
MK_FLOW_ACTION(SET_IPV4_DSCP, sizeof(struct rte_flow_action_set_dscp)),
MK_FLOW_ACTION(SET_IPV6_DSCP, sizeof(struct rte_flow_action_set_dscp)),
+ MK_FLOW_ACTION(AGE, sizeof(struct rte_flow_action_age)),
};
int
@@ -1232,3 +1233,20 @@ rte_flow_dev_dump(uint16_t port_id, FILE *file, struct rte_flow_error *error)
RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
NULL, rte_strerror(ENOSYS));
}
+
+int
+rte_flow_get_aged_flows(uint16_t port_id, void **contexts,
+ uint32_t nb_contexts, 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);
+
+ if (unlikely(!ops))
+ return -rte_errno;
+ if (likely(!!ops->get_aged_flows))
+ return flow_err(port_id, ops->get_aged_flows(dev, contexts,
+ nb_contexts, error), error);
+ return rte_flow_error_set(error, ENOTSUP,
+ RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
+ NULL, rte_strerror(ENOTSUP));
+}
@@ -2081,6 +2081,16 @@ enum rte_flow_action_type {
* See struct rte_flow_action_set_dscp.
*/
RTE_FLOW_ACTION_TYPE_SET_IPV6_DSCP,
+
+ /**
+ * Report as aged flow if timeout passed without any matching on the
+ * flow.
+ *
+ * See struct rte_flow_action_age.
+ * See function rte_flow_get_aged_flows
+ * see enum RTE_ETH_EVENT_FLOW_AGED
+ */
+ RTE_FLOW_ACTION_TYPE_AGE,
};
/**
@@ -2122,6 +2132,25 @@ struct rte_flow_action_queue {
uint16_t index; /**< Queue index to use. */
};
+/**
+ * @warning
+ * @b EXPERIMENTAL: this structure may change without prior notice
+ *
+ * RTE_FLOW_ACTION_TYPE_AGE
+ *
+ * Report flow as aged-out if timeout passed without any matching
+ * on the flow. RTE_ETH_EVENT_FLOW_AGED event is triggered when a
+ * port detects new aged-out flows.
+ *
+ * The flow context and the flow handle will be reported by the
+ * rte_flow_get_aged_flows API.
+ */
+struct rte_flow_action_age {
+ uint32_t timeout:24; /**< Time in seconds. */
+ uint32_t reserved:8; /**< Reserved, must be zero. */
+ void *context;
+ /**< The user flow context, NULL means the rte_flow pointer. */
+};
/**
* @warning
@@ -3254,6 +3283,39 @@ rte_flow_conv(enum rte_flow_conv_op op,
const void *src,
struct rte_flow_error *error);
+/**
+ * Get aged-out flows of a given port.
+ *
+ * RTE_ETH_EVENT_FLOW_AGED event will be triggered when at least one new aged
+ * out flow was detected after the last call to rte_flow_get_aged_flows.
+ * This function can be called to get the aged flows usynchronously from the
+ * event callback or synchronously regardless the event.
+ * This is not safe to call rte_flow_get_aged_flows function with other flow
+ * functions from multiple threads simultaneously.
+ *
+ * @param port_id
+ * Port identifier of Ethernet device.
+ * @param[in, out] contexts
+ * The address of an array of pointers to the aged-out flows contexts.
+ * @param[in] nb_contexts
+ * The length of context array pointers.
+ * @param[out] error
+ * Perform verbose error reporting if not NULL. Initialized in case of
+ * error only.
+ *
+ * @return
+ * if nb_contexts is 0, return the amount of all aged contexts.
+ * if nb_contexts is not 0 , return the amount of aged flows reported
+ * in the context array, otherwise negative errno value.
+ *
+ * @see rte_flow_action_age
+ * @see RTE_ETH_EVENT_FLOW_AGED
+ */
+__rte_experimental
+int
+rte_flow_get_aged_flows(uint16_t port_id, void **contexts,
+ uint32_t nb_contexts, struct rte_flow_error *error);
+
#ifdef __cplusplus
}
#endif
@@ -101,6 +101,12 @@ struct rte_flow_ops {
(struct rte_eth_dev *dev,
FILE *file,
struct rte_flow_error *error);
+ /** See rte_flow_get_aged_flows() */
+ int (*get_aged_flows)
+ (struct rte_eth_dev *dev,
+ void **context,
+ uint32_t nb_contexts,
+ struct rte_flow_error *err);
};
/**