[dpdk-dev,v6,4/4] ethdev: add shared counter support to rte_flow
Checks
Commit Message
Add rte_flow_action_count action data structure to enable shared
counters across multiple flows on a single port or across multiple
flows on multiple ports within the same switch domain. Also this enables
multiple count actions to be specified in a single flow action.
This patch also modifies the existing rte_flow_query API to take the
rte_flow_action structure as an input parameter instead of the
rte_flow_action_type enumeration to allow querying a specific action
from a flow rule when multiple actions of the same type are specified.
This patch also contains updates for the bonding and failsafe PMDs and
testpmd application which are affected by this API change.
Signed-off-by: Declan Doherty <declan.doherty@intel.com>
---
app/test-pmd/cmdline_flow.c | 6 ++--
app/test-pmd/config.c | 15 +++++----
app/test-pmd/testpmd.h | 2 +-
doc/guides/prog_guide/rte_flow.rst | 59 +++++++++++++++++++++------------
drivers/net/bonding/rte_eth_bond_flow.c | 9 ++---
drivers/net/failsafe/failsafe_flow.c | 4 +--
lib/librte_ether/rte_flow.c | 2 +-
lib/librte_ether/rte_flow.h | 38 +++++++++++++++++++--
lib/librte_ether/rte_flow_driver.h | 2 +-
9 files changed, 93 insertions(+), 44 deletions(-)
Comments
Hi Declan,
You are changing API (port_flow_query) which is in use in
both MLX5 and MLX4 this results in breaking the build.
Best,
Ori
> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Declan Doherty
> Sent: Thursday, April 26, 2018 3:08 PM
> To: dev@dpdk.org
> Cc: Ferruh Yigit <ferruh.yigit@intel.com>; Declan Doherty
> <declan.doherty@intel.com>
> Subject: [dpdk-dev] [PATCH v6 4/4] ethdev: add shared counter support to
> rte_flow
>
> Add rte_flow_action_count action data structure to enable shared
> counters across multiple flows on a single port or across multiple
> flows on multiple ports within the same switch domain. Also this enables
> multiple count actions to be specified in a single flow action.
>
> This patch also modifies the existing rte_flow_query API to take the
> rte_flow_action structure as an input parameter instead of the
> rte_flow_action_type enumeration to allow querying a specific action
> from a flow rule when multiple actions of the same type are specified.
>
> This patch also contains updates for the bonding and failsafe PMDs and
> testpmd application which are affected by this API change.
>
> Signed-off-by: Declan Doherty <declan.doherty@intel.com>
> ---
> app/test-pmd/cmdline_flow.c | 6 ++--
> app/test-pmd/config.c | 15 +++++----
> app/test-pmd/testpmd.h | 2 +-
> doc/guides/prog_guide/rte_flow.rst | 59 +++++++++++++++++++++------
> ------
> drivers/net/bonding/rte_eth_bond_flow.c | 9 ++---
> drivers/net/failsafe/failsafe_flow.c | 4 +--
> lib/librte_ether/rte_flow.c | 2 +-
> lib/librte_ether/rte_flow.h | 38 +++++++++++++++++++--
> lib/librte_ether/rte_flow_driver.h | 2 +-
> 9 files changed, 93 insertions(+), 44 deletions(-)
>
> diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
> index 1ac04a0ab..5754e7858 100644
> --- a/app/test-pmd/cmdline_flow.c
> +++ b/app/test-pmd/cmdline_flow.c
> @@ -420,7 +420,7 @@ struct buffer {
> } destroy; /**< Destroy arguments. */
> struct {
> uint32_t rule;
> - enum rte_flow_action_type action;
> + struct rte_flow_action action;
> } query; /**< Query arguments. */
> struct {
> uint32_t *group;
> @@ -1101,7 +1101,7 @@ static const struct token token_list[] = {
> .next = NEXT(NEXT_ENTRY(QUERY_ACTION),
> NEXT_ENTRY(RULE_ID),
> NEXT_ENTRY(PORT_ID)),
> - .args = ARGS(ARGS_ENTRY(struct buffer, args.query.action),
> + .args = ARGS(ARGS_ENTRY(struct buffer,
> args.query.action.type),
> ARGS_ENTRY(struct buffer, args.query.rule),
> ARGS_ENTRY(struct buffer, port)),
> .call = parse_query,
> @@ -3842,7 +3842,7 @@ cmd_flow_parsed(const struct buffer *in)
> break;
> case QUERY:
> port_flow_query(in->port, in->args.query.rule,
> - in->args.query.action);
> + &in->args.query.action);
> break;
> case LIST:
> port_flow_list(in->port, in->args.list.group_n,
> diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
> index 0f2425229..cd6102dfc 100644
> --- a/app/test-pmd/config.c
> +++ b/app/test-pmd/config.c
> @@ -1452,7 +1452,7 @@ port_flow_flush(portid_t port_id)
> /** Query a flow rule. */
> int
> port_flow_query(portid_t port_id, uint32_t rule,
> - enum rte_flow_action_type action)
> + const struct rte_flow_action *action)
> {
> struct rte_flow_error error;
> struct rte_port *port;
> @@ -1474,15 +1474,16 @@ port_flow_query(portid_t port_id, uint32_t rule,
> return -ENOENT;
> }
> if ((unsigned int)action >= RTE_DIM(flow_action) ||
> - !flow_action[action].name)
> + !flow_action[action->type].name)
> name = "unknown";
> else
> - name = flow_action[action].name;
> - switch (action) {
> + name = flow_action[action->type].name;
> + switch (action->type) {
> case RTE_FLOW_ACTION_TYPE_COUNT:
> break;
> default:
> - printf("Cannot query action type %d (%s)\n", action, name);
> + printf("Cannot query action type %d (%s)\n",
> + action->type, name);
> return -ENOTSUP;
> }
> /* Poisoning to make sure PMDs update it in case of error. */
> @@ -1490,7 +1491,7 @@ port_flow_query(portid_t port_id, uint32_t rule,
> memset(&query, 0, sizeof(query));
> if (rte_flow_query(port_id, pf->flow, action, &query, &error))
> return port_flow_complain(&error);
> - switch (action) {
> + switch (action->type) {
> case RTE_FLOW_ACTION_TYPE_COUNT:
> printf("%s:\n"
> " hits_set: %u\n"
> @@ -1505,7 +1506,7 @@ port_flow_query(portid_t port_id, uint32_t rule,
> break;
> default:
> printf("Cannot display result for action type %d (%s)\n",
> - action, name);
> + action->type, name);
> break;
> }
> return 0;
> diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
> index a33b525e2..1af87b8f4 100644
> --- a/app/test-pmd/testpmd.h
> +++ b/app/test-pmd/testpmd.h
> @@ -620,7 +620,7 @@ int port_flow_create(portid_t port_id,
> int port_flow_destroy(portid_t port_id, uint32_t n, const uint32_t *rule);
> int port_flow_flush(portid_t port_id);
> int port_flow_query(portid_t port_id, uint32_t rule,
> - enum rte_flow_action_type action);
> + const struct rte_flow_action *action);
> void port_flow_list(portid_t port_id, uint32_t n, const uint32_t *group);
> int port_flow_isolate(portid_t port_id, int set);
>
> diff --git a/doc/guides/prog_guide/rte_flow.rst
> b/doc/guides/prog_guide/rte_flow.rst
> index 301f8762e..88bfc87eb 100644
> --- a/doc/guides/prog_guide/rte_flow.rst
> +++ b/doc/guides/prog_guide/rte_flow.rst
> @@ -1277,17 +1277,19 @@ Actions are performed in list order:
>
> .. table:: Mark, count then redirect
>
> - +-------+--------+-----------+-------+
> - | Index | Action | Field | Value |
> - +=======+========+===========+=======+
> - | 0 | MARK | ``mark`` | 0x2a |
> - +-------+--------+-----------+-------+
> - | 1 | COUNT |
> - +-------+--------+-----------+-------+
> - | 2 | QUEUE | ``queue`` | 10 |
> - +-------+--------+-----------+-------+
> - | 3 | END |
> - +-------+----------------------------+
> + +-------+--------+------------+-------+
> + | Index | Action | Field | Value |
> + +=======+========+============+=======+
> + | 0 | MARK | ``mark`` | 0x2a |
> + +-------+--------+------------+-------+
> + | 1 | COUNT | ``shared`` | 0 |
> + | | +------------+-------+
> + | | | ``id`` | 0 |
> + +-------+--------+------------+-------+
> + | 2 | QUEUE | ``queue`` | 10 |
> + +-------+--------+------------+-------+
> + | 3 | END |
> + +-------+-----------------------------+
>
> |
>
> @@ -1516,23 +1518,36 @@ Drop packets.
> Action: ``COUNT``
> ^^^^^^^^^^^^^^^^^
>
> -Enables counters for this rule.
> +Adds a counter action to a matched flow.
> +
> +If more than one count action is specified in a single flow rule, then each
> +action must specify a unique id.
>
> -These counters can be retrieved and reset through ``rte_flow_query()``, see
> +Counters can be retrieved and reset through ``rte_flow_query()``, see
> ``struct rte_flow_query_count``.
>
> -- Counters can be retrieved with ``rte_flow_query()``.
> -- No configurable properties.
> +The shared flag indicates whether the counter is unique to the flow rule the
> +action is specified with, or whether it is a shared counter.
> +
> +For a count action with the shared flag set, then then a global device
> +namespace is assumed for the counter id, so that any matched flow rules
> using
> +a count action with the same counter id on the same port will contribute to
> +that counter.
> +
> +For ports within the same switch domain then the counter id namespace
> extends
> +to all ports within that switch domain.
>
> .. _table_rte_flow_action_count:
>
> .. table:: COUNT
>
> - +---------------+
> - | Field |
> - +===============+
> - | no properties |
> - +---------------+
> + +------------+---------------------+
> + | Field | Value |
> + +============+=====================+
> + | ``shared`` | shared counter flag |
> + +------------+---------------------+
> + | ``id`` | counter id |
> + +------------+---------------------+
>
> Query structure to retrieve and reset flow rule counters:
>
> @@ -2282,7 +2297,7 @@ definition.
> int
> rte_flow_query(uint16_t port_id,
> struct rte_flow *flow,
> - enum rte_flow_action_type action,
> + const struct rte_flow_action *action,
> void *data,
> struct rte_flow_error *error);
>
> @@ -2290,7 +2305,7 @@ Arguments:
>
> - ``port_id``: port identifier of Ethernet device.
> - ``flow``: flow rule handle to query.
> -- ``action``: action type to query.
> +- ``action``: action to query, this must match prototype from flow rule.
> - ``data``: pointer to storage for the associated query data type.
> - ``error``: perform verbose error reporting if not NULL. PMDs initialize
> this structure in case of error only.
> diff --git a/drivers/net/bonding/rte_eth_bond_flow.c
> b/drivers/net/bonding/rte_eth_bond_flow.c
> index 8093c04f5..31e4bcaeb 100644
> --- a/drivers/net/bonding/rte_eth_bond_flow.c
> +++ b/drivers/net/bonding/rte_eth_bond_flow.c
> @@ -152,6 +152,7 @@ bond_flow_flush(struct rte_eth_dev *dev, struct
> rte_flow_error *err)
>
> static int
> bond_flow_query_count(struct rte_eth_dev *dev, struct rte_flow *flow,
> + const struct rte_flow_action *action,
> struct rte_flow_query_count *count,
> struct rte_flow_error *err)
> {
> @@ -165,7 +166,7 @@ bond_flow_query_count(struct rte_eth_dev *dev,
> struct rte_flow *flow,
> rte_memcpy(&slave_count, count, sizeof(slave_count));
> for (i = 0; i < internals->slave_count; i++) {
> ret = rte_flow_query(internals->slaves[i].port_id,
> - flow->flows[i],
> RTE_FLOW_ACTION_TYPE_COUNT,
> + flow->flows[i], action,
> &slave_count, err);
> if (unlikely(ret != 0)) {
> RTE_BOND_LOG(ERR, "Failed to query flow on"
> @@ -182,12 +183,12 @@ bond_flow_query_count(struct rte_eth_dev *dev,
> struct rte_flow *flow,
>
> static int
> bond_flow_query(struct rte_eth_dev *dev, struct rte_flow *flow,
> - enum rte_flow_action_type type, void *arg,
> + const struct rte_flow_action *action, void *arg,
> struct rte_flow_error *err)
> {
> - switch (type) {
> + switch (action->type) {
> case RTE_FLOW_ACTION_TYPE_COUNT:
> - return bond_flow_query_count(dev, flow, arg, err);
> + return bond_flow_query_count(dev, flow, action, arg, err);
> default:
> return rte_flow_error_set(err, ENOTSUP,
> RTE_FLOW_ERROR_TYPE_ACTION,
> arg,
> diff --git a/drivers/net/failsafe/failsafe_flow.c
> b/drivers/net/failsafe/failsafe_flow.c
> index a97f4075d..bfe42fcee 100644
> --- a/drivers/net/failsafe/failsafe_flow.c
> +++ b/drivers/net/failsafe/failsafe_flow.c
> @@ -174,7 +174,7 @@ fs_flow_flush(struct rte_eth_dev *dev,
> static int
> fs_flow_query(struct rte_eth_dev *dev,
> struct rte_flow *flow,
> - enum rte_flow_action_type type,
> + const struct rte_flow_action *action,
> void *arg,
> struct rte_flow_error *error)
> {
> @@ -185,7 +185,7 @@ fs_flow_query(struct rte_eth_dev *dev,
> if (sdev != NULL) {
> int ret = rte_flow_query(PORT_ID(sdev),
> flow->flows[SUB_ID(sdev)],
> - type, arg, error);
> + action, arg, error);
>
> if ((ret = fs_err(sdev, ret))) {
> fs_unlock(dev, 0);
> diff --git a/lib/librte_ether/rte_flow.c b/lib/librte_ether/rte_flow.c
> index 4f94ac9b5..7947529da 100644
> --- a/lib/librte_ether/rte_flow.c
> +++ b/lib/librte_ether/rte_flow.c
> @@ -233,7 +233,7 @@ rte_flow_flush(uint16_t port_id,
> int
> rte_flow_query(uint16_t port_id,
> struct rte_flow *flow,
> - enum rte_flow_action_type action,
> + const struct rte_flow_action *action,
> void *data,
> struct rte_flow_error *error)
> {
> diff --git a/lib/librte_ether/rte_flow.h b/lib/librte_ether/rte_flow.h
> index d390bbf5a..f8ba71cdb 100644
> --- a/lib/librte_ether/rte_flow.h
> +++ b/lib/librte_ether/rte_flow.h
> @@ -1314,7 +1314,7 @@ enum rte_flow_action_type {
> * These counters can be retrieved and reset through
> rte_flow_query(),
> * see struct rte_flow_query_count.
> *
> - * No associated configuration structure.
> + * See struct rte_flow_action_count.
> */
> RTE_FLOW_ACTION_TYPE_COUNT,
>
> @@ -1546,6 +1546,38 @@ 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_COUNT
> + *
> + * Adds a counter action to a matched flow.
> + *
> + * If more than one count action is specified in a single flow rule, then each
> + * action must specify a unique id.
> + *
> + * Counters can be retrieved and reset through ``rte_flow_query()``, see
> + * ``struct rte_flow_query_count``.
> + *
> + * The shared flag indicates whether the counter is unique to the flow rule
> the
> + * action is specified with, or whether it is a shared counter.
> + *
> + * For a count action with the shared flag set, then then a global device
> + * namespace is assumed for the counter id, so that any matched flow rules
> using
> + * a count action with the same counter id on the same port will contribute
> to
> + * that counter.
> + *
> + * For ports within the same switch domain then the counter id namespace
> extends
> + * to all ports within that switch domain.
> + */
> +struct rte_flow_action_count {
> + uint32_t shared:1; /**< Share counter ID with other flow rules. */
> + uint32_t reserved:31; /**< Reserved, must be zero. */
> + uint32_t id; /**< Counter ID. */
> +};
> +
> /**
> * RTE_FLOW_ACTION_TYPE_COUNT (query)
> *
> @@ -2044,7 +2076,7 @@ rte_flow_flush(uint16_t port_id,
> * @param flow
> * Flow rule handle to query.
> * @param action
> - * Action type to query.
> + * Action definition as defined in original flow rule.
> * @param[in, out] data
> * Pointer to storage for the associated query data type.
> * @param[out] error
> @@ -2057,7 +2089,7 @@ rte_flow_flush(uint16_t port_id,
> int
> rte_flow_query(uint16_t port_id,
> struct rte_flow *flow,
> - enum rte_flow_action_type action,
> + const struct rte_flow_action *action,
> void *data,
> struct rte_flow_error *error);
>
> diff --git a/lib/librte_ether/rte_flow_driver.h
> b/lib/librte_ether/rte_flow_driver.h
> index 3800310ba..1c90c600d 100644
> --- a/lib/librte_ether/rte_flow_driver.h
> +++ b/lib/librte_ether/rte_flow_driver.h
> @@ -88,7 +88,7 @@ struct rte_flow_ops {
> int (*query)
> (struct rte_eth_dev *,
> struct rte_flow *,
> - enum rte_flow_action_type,
> + const struct rte_flow_action *,
> void *,
> struct rte_flow_error *);
> /** See rte_flow_isolate(). */
> --
> 2.14.3
On 4/26/2018 3:06 PM, Ori Kam wrote:
> Hi Declan,
>
> You are changing API (port_flow_query) which is in use in
> both MLX5 and MLX4 this results in breaking the build.
Hi Ori,
Do you mean "rte_flow_query"? port_flow_query() is a function in testpmd, how
mlx is using it?
>
> Best,
> Ori
>
>> -----Original Message-----
>> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Declan Doherty
>> Sent: Thursday, April 26, 2018 3:08 PM
>> To: dev@dpdk.org
>> Cc: Ferruh Yigit <ferruh.yigit@intel.com>; Declan Doherty
>> <declan.doherty@intel.com>
>> Subject: [dpdk-dev] [PATCH v6 4/4] ethdev: add shared counter support to
>> rte_flow
>>
>> Add rte_flow_action_count action data structure to enable shared
>> counters across multiple flows on a single port or across multiple
>> flows on multiple ports within the same switch domain. Also this enables
>> multiple count actions to be specified in a single flow action.
>>
>> This patch also modifies the existing rte_flow_query API to take the
>> rte_flow_action structure as an input parameter instead of the
>> rte_flow_action_type enumeration to allow querying a specific action
>> from a flow rule when multiple actions of the same type are specified.
>>
>> This patch also contains updates for the bonding and failsafe PMDs and
>> testpmd application which are affected by this API change.
>>
>> Signed-off-by: Declan Doherty <declan.doherty@intel.com>
>> ---
>> app/test-pmd/cmdline_flow.c | 6 ++--
>> app/test-pmd/config.c | 15 +++++----
>> app/test-pmd/testpmd.h | 2 +-
>> doc/guides/prog_guide/rte_flow.rst | 59 +++++++++++++++++++++------
>> ------
>> drivers/net/bonding/rte_eth_bond_flow.c | 9 ++---
>> drivers/net/failsafe/failsafe_flow.c | 4 +--
>> lib/librte_ether/rte_flow.c | 2 +-
>> lib/librte_ether/rte_flow.h | 38 +++++++++++++++++++--
>> lib/librte_ether/rte_flow_driver.h | 2 +-
>> 9 files changed, 93 insertions(+), 44 deletions(-)
>>
>> diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
>> index 1ac04a0ab..5754e7858 100644
>> --- a/app/test-pmd/cmdline_flow.c
>> +++ b/app/test-pmd/cmdline_flow.c
>> @@ -420,7 +420,7 @@ struct buffer {
>> } destroy; /**< Destroy arguments. */
>> struct {
>> uint32_t rule;
>> - enum rte_flow_action_type action;
>> + struct rte_flow_action action;
>> } query; /**< Query arguments. */
>> struct {
>> uint32_t *group;
>> @@ -1101,7 +1101,7 @@ static const struct token token_list[] = {
>> .next = NEXT(NEXT_ENTRY(QUERY_ACTION),
>> NEXT_ENTRY(RULE_ID),
>> NEXT_ENTRY(PORT_ID)),
>> - .args = ARGS(ARGS_ENTRY(struct buffer, args.query.action),
>> + .args = ARGS(ARGS_ENTRY(struct buffer,
>> args.query.action.type),
>> ARGS_ENTRY(struct buffer, args.query.rule),
>> ARGS_ENTRY(struct buffer, port)),
>> .call = parse_query,
>> @@ -3842,7 +3842,7 @@ cmd_flow_parsed(const struct buffer *in)
>> break;
>> case QUERY:
>> port_flow_query(in->port, in->args.query.rule,
>> - in->args.query.action);
>> + &in->args.query.action);
>> break;
>> case LIST:
>> port_flow_list(in->port, in->args.list.group_n,
>> diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
>> index 0f2425229..cd6102dfc 100644
>> --- a/app/test-pmd/config.c
>> +++ b/app/test-pmd/config.c
>> @@ -1452,7 +1452,7 @@ port_flow_flush(portid_t port_id)
>> /** Query a flow rule. */
>> int
>> port_flow_query(portid_t port_id, uint32_t rule,
>> - enum rte_flow_action_type action)
>> + const struct rte_flow_action *action)
>> {
>> struct rte_flow_error error;
>> struct rte_port *port;
>> @@ -1474,15 +1474,16 @@ port_flow_query(portid_t port_id, uint32_t rule,
>> return -ENOENT;
>> }
>> if ((unsigned int)action >= RTE_DIM(flow_action) ||
>> - !flow_action[action].name)
>> + !flow_action[action->type].name)
>> name = "unknown";
>> else
>> - name = flow_action[action].name;
>> - switch (action) {
>> + name = flow_action[action->type].name;
>> + switch (action->type) {
>> case RTE_FLOW_ACTION_TYPE_COUNT:
>> break;
>> default:
>> - printf("Cannot query action type %d (%s)\n", action, name);
>> + printf("Cannot query action type %d (%s)\n",
>> + action->type, name);
>> return -ENOTSUP;
>> }
>> /* Poisoning to make sure PMDs update it in case of error. */
>> @@ -1490,7 +1491,7 @@ port_flow_query(portid_t port_id, uint32_t rule,
>> memset(&query, 0, sizeof(query));
>> if (rte_flow_query(port_id, pf->flow, action, &query, &error))
>> return port_flow_complain(&error);
>> - switch (action) {
>> + switch (action->type) {
>> case RTE_FLOW_ACTION_TYPE_COUNT:
>> printf("%s:\n"
>> " hits_set: %u\n"
>> @@ -1505,7 +1506,7 @@ port_flow_query(portid_t port_id, uint32_t rule,
>> break;
>> default:
>> printf("Cannot display result for action type %d (%s)\n",
>> - action, name);
>> + action->type, name);
>> break;
>> }
>> return 0;
>> diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
>> index a33b525e2..1af87b8f4 100644
>> --- a/app/test-pmd/testpmd.h
>> +++ b/app/test-pmd/testpmd.h
>> @@ -620,7 +620,7 @@ int port_flow_create(portid_t port_id,
>> int port_flow_destroy(portid_t port_id, uint32_t n, const uint32_t *rule);
>> int port_flow_flush(portid_t port_id);
>> int port_flow_query(portid_t port_id, uint32_t rule,
>> - enum rte_flow_action_type action);
>> + const struct rte_flow_action *action);
>> void port_flow_list(portid_t port_id, uint32_t n, const uint32_t *group);
>> int port_flow_isolate(portid_t port_id, int set);
>>
>> diff --git a/doc/guides/prog_guide/rte_flow.rst
>> b/doc/guides/prog_guide/rte_flow.rst
>> index 301f8762e..88bfc87eb 100644
>> --- a/doc/guides/prog_guide/rte_flow.rst
>> +++ b/doc/guides/prog_guide/rte_flow.rst
>> @@ -1277,17 +1277,19 @@ Actions are performed in list order:
>>
>> .. table:: Mark, count then redirect
>>
>> - +-------+--------+-----------+-------+
>> - | Index | Action | Field | Value |
>> - +=======+========+===========+=======+
>> - | 0 | MARK | ``mark`` | 0x2a |
>> - +-------+--------+-----------+-------+
>> - | 1 | COUNT |
>> - +-------+--------+-----------+-------+
>> - | 2 | QUEUE | ``queue`` | 10 |
>> - +-------+--------+-----------+-------+
>> - | 3 | END |
>> - +-------+----------------------------+
>> + +-------+--------+------------+-------+
>> + | Index | Action | Field | Value |
>> + +=======+========+============+=======+
>> + | 0 | MARK | ``mark`` | 0x2a |
>> + +-------+--------+------------+-------+
>> + | 1 | COUNT | ``shared`` | 0 |
>> + | | +------------+-------+
>> + | | | ``id`` | 0 |
>> + +-------+--------+------------+-------+
>> + | 2 | QUEUE | ``queue`` | 10 |
>> + +-------+--------+------------+-------+
>> + | 3 | END |
>> + +-------+-----------------------------+
>>
>> |
>>
>> @@ -1516,23 +1518,36 @@ Drop packets.
>> Action: ``COUNT``
>> ^^^^^^^^^^^^^^^^^
>>
>> -Enables counters for this rule.
>> +Adds a counter action to a matched flow.
>> +
>> +If more than one count action is specified in a single flow rule, then each
>> +action must specify a unique id.
>>
>> -These counters can be retrieved and reset through ``rte_flow_query()``, see
>> +Counters can be retrieved and reset through ``rte_flow_query()``, see
>> ``struct rte_flow_query_count``.
>>
>> -- Counters can be retrieved with ``rte_flow_query()``.
>> -- No configurable properties.
>> +The shared flag indicates whether the counter is unique to the flow rule the
>> +action is specified with, or whether it is a shared counter.
>> +
>> +For a count action with the shared flag set, then then a global device
>> +namespace is assumed for the counter id, so that any matched flow rules
>> using
>> +a count action with the same counter id on the same port will contribute to
>> +that counter.
>> +
>> +For ports within the same switch domain then the counter id namespace
>> extends
>> +to all ports within that switch domain.
>>
>> .. _table_rte_flow_action_count:
>>
>> .. table:: COUNT
>>
>> - +---------------+
>> - | Field |
>> - +===============+
>> - | no properties |
>> - +---------------+
>> + +------------+---------------------+
>> + | Field | Value |
>> + +============+=====================+
>> + | ``shared`` | shared counter flag |
>> + +------------+---------------------+
>> + | ``id`` | counter id |
>> + +------------+---------------------+
>>
>> Query structure to retrieve and reset flow rule counters:
>>
>> @@ -2282,7 +2297,7 @@ definition.
>> int
>> rte_flow_query(uint16_t port_id,
>> struct rte_flow *flow,
>> - enum rte_flow_action_type action,
>> + const struct rte_flow_action *action,
>> void *data,
>> struct rte_flow_error *error);
>>
>> @@ -2290,7 +2305,7 @@ Arguments:
>>
>> - ``port_id``: port identifier of Ethernet device.
>> - ``flow``: flow rule handle to query.
>> -- ``action``: action type to query.
>> +- ``action``: action to query, this must match prototype from flow rule.
>> - ``data``: pointer to storage for the associated query data type.
>> - ``error``: perform verbose error reporting if not NULL. PMDs initialize
>> this structure in case of error only.
>> diff --git a/drivers/net/bonding/rte_eth_bond_flow.c
>> b/drivers/net/bonding/rte_eth_bond_flow.c
>> index 8093c04f5..31e4bcaeb 100644
>> --- a/drivers/net/bonding/rte_eth_bond_flow.c
>> +++ b/drivers/net/bonding/rte_eth_bond_flow.c
>> @@ -152,6 +152,7 @@ bond_flow_flush(struct rte_eth_dev *dev, struct
>> rte_flow_error *err)
>>
>> static int
>> bond_flow_query_count(struct rte_eth_dev *dev, struct rte_flow *flow,
>> + const struct rte_flow_action *action,
>> struct rte_flow_query_count *count,
>> struct rte_flow_error *err)
>> {
>> @@ -165,7 +166,7 @@ bond_flow_query_count(struct rte_eth_dev *dev,
>> struct rte_flow *flow,
>> rte_memcpy(&slave_count, count, sizeof(slave_count));
>> for (i = 0; i < internals->slave_count; i++) {
>> ret = rte_flow_query(internals->slaves[i].port_id,
>> - flow->flows[i],
>> RTE_FLOW_ACTION_TYPE_COUNT,
>> + flow->flows[i], action,
>> &slave_count, err);
>> if (unlikely(ret != 0)) {
>> RTE_BOND_LOG(ERR, "Failed to query flow on"
>> @@ -182,12 +183,12 @@ bond_flow_query_count(struct rte_eth_dev *dev,
>> struct rte_flow *flow,
>>
>> static int
>> bond_flow_query(struct rte_eth_dev *dev, struct rte_flow *flow,
>> - enum rte_flow_action_type type, void *arg,
>> + const struct rte_flow_action *action, void *arg,
>> struct rte_flow_error *err)
>> {
>> - switch (type) {
>> + switch (action->type) {
>> case RTE_FLOW_ACTION_TYPE_COUNT:
>> - return bond_flow_query_count(dev, flow, arg, err);
>> + return bond_flow_query_count(dev, flow, action, arg, err);
>> default:
>> return rte_flow_error_set(err, ENOTSUP,
>> RTE_FLOW_ERROR_TYPE_ACTION,
>> arg,
>> diff --git a/drivers/net/failsafe/failsafe_flow.c
>> b/drivers/net/failsafe/failsafe_flow.c
>> index a97f4075d..bfe42fcee 100644
>> --- a/drivers/net/failsafe/failsafe_flow.c
>> +++ b/drivers/net/failsafe/failsafe_flow.c
>> @@ -174,7 +174,7 @@ fs_flow_flush(struct rte_eth_dev *dev,
>> static int
>> fs_flow_query(struct rte_eth_dev *dev,
>> struct rte_flow *flow,
>> - enum rte_flow_action_type type,
>> + const struct rte_flow_action *action,
>> void *arg,
>> struct rte_flow_error *error)
>> {
>> @@ -185,7 +185,7 @@ fs_flow_query(struct rte_eth_dev *dev,
>> if (sdev != NULL) {
>> int ret = rte_flow_query(PORT_ID(sdev),
>> flow->flows[SUB_ID(sdev)],
>> - type, arg, error);
>> + action, arg, error);
>>
>> if ((ret = fs_err(sdev, ret))) {
>> fs_unlock(dev, 0);
>> diff --git a/lib/librte_ether/rte_flow.c b/lib/librte_ether/rte_flow.c
>> index 4f94ac9b5..7947529da 100644
>> --- a/lib/librte_ether/rte_flow.c
>> +++ b/lib/librte_ether/rte_flow.c
>> @@ -233,7 +233,7 @@ rte_flow_flush(uint16_t port_id,
>> int
>> rte_flow_query(uint16_t port_id,
>> struct rte_flow *flow,
>> - enum rte_flow_action_type action,
>> + const struct rte_flow_action *action,
>> void *data,
>> struct rte_flow_error *error)
>> {
>> diff --git a/lib/librte_ether/rte_flow.h b/lib/librte_ether/rte_flow.h
>> index d390bbf5a..f8ba71cdb 100644
>> --- a/lib/librte_ether/rte_flow.h
>> +++ b/lib/librte_ether/rte_flow.h
>> @@ -1314,7 +1314,7 @@ enum rte_flow_action_type {
>> * These counters can be retrieved and reset through
>> rte_flow_query(),
>> * see struct rte_flow_query_count.
>> *
>> - * No associated configuration structure.
>> + * See struct rte_flow_action_count.
>> */
>> RTE_FLOW_ACTION_TYPE_COUNT,
>>
>> @@ -1546,6 +1546,38 @@ 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_COUNT
>> + *
>> + * Adds a counter action to a matched flow.
>> + *
>> + * If more than one count action is specified in a single flow rule, then each
>> + * action must specify a unique id.
>> + *
>> + * Counters can be retrieved and reset through ``rte_flow_query()``, see
>> + * ``struct rte_flow_query_count``.
>> + *
>> + * The shared flag indicates whether the counter is unique to the flow rule
>> the
>> + * action is specified with, or whether it is a shared counter.
>> + *
>> + * For a count action with the shared flag set, then then a global device
>> + * namespace is assumed for the counter id, so that any matched flow rules
>> using
>> + * a count action with the same counter id on the same port will contribute
>> to
>> + * that counter.
>> + *
>> + * For ports within the same switch domain then the counter id namespace
>> extends
>> + * to all ports within that switch domain.
>> + */
>> +struct rte_flow_action_count {
>> + uint32_t shared:1; /**< Share counter ID with other flow rules. */
>> + uint32_t reserved:31; /**< Reserved, must be zero. */
>> + uint32_t id; /**< Counter ID. */
>> +};
>> +
>> /**
>> * RTE_FLOW_ACTION_TYPE_COUNT (query)
>> *
>> @@ -2044,7 +2076,7 @@ rte_flow_flush(uint16_t port_id,
>> * @param flow
>> * Flow rule handle to query.
>> * @param action
>> - * Action type to query.
>> + * Action definition as defined in original flow rule.
>> * @param[in, out] data
>> * Pointer to storage for the associated query data type.
>> * @param[out] error
>> @@ -2057,7 +2089,7 @@ rte_flow_flush(uint16_t port_id,
>> int
>> rte_flow_query(uint16_t port_id,
>> struct rte_flow *flow,
>> - enum rte_flow_action_type action,
>> + const struct rte_flow_action *action,
>> void *data,
>> struct rte_flow_error *error);
>>
>> diff --git a/lib/librte_ether/rte_flow_driver.h
>> b/lib/librte_ether/rte_flow_driver.h
>> index 3800310ba..1c90c600d 100644
>> --- a/lib/librte_ether/rte_flow_driver.h
>> +++ b/lib/librte_ether/rte_flow_driver.h
>> @@ -88,7 +88,7 @@ struct rte_flow_ops {
>> int (*query)
>> (struct rte_eth_dev *,
>> struct rte_flow *,
>> - enum rte_flow_action_type,
>> + const struct rte_flow_action *,
>> void *,
>> struct rte_flow_error *);
>> /** See rte_flow_isolate(). */
>> --
>> 2.14.3
>
Hi,
PSB
Ori
> -----Original Message-----
> From: Ferruh Yigit [mailto:ferruh.yigit@intel.com]
> Sent: Thursday, April 26, 2018 5:28 PM
> To: Ori Kam <orika@mellanox.com>; Declan Doherty
> <declan.doherty@intel.com>; dev@dpdk.org; Matan Azrad
> <matan@mellanox.com>
> Subject: Re: [dpdk-dev] [PATCH v6 4/4] ethdev: add shared counter support
> to rte_flow
>
> On 4/26/2018 3:06 PM, Ori Kam wrote:
> > Hi Declan,
> >
> > You are changing API (port_flow_query) which is in use in both MLX5
> > and MLX4 this results in breaking the build.
>
> Hi Ori,
>
> Do you mean "rte_flow_query"? port_flow_query() is a function in testpmd,
> how mlx is using it?
>
My bad let me be clearer.
MLX5 and MLX4 are implementing the rte_flow_ops query function.
This patch changes the prototype for the query function which results in
compilation error and should also result in some change in the MLX5 and MLX4 implementation.
> >
> > Best,
> > Ori
> >
> >> -----Original Message-----
> >> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Declan Doherty
> >> Sent: Thursday, April 26, 2018 3:08 PM
> >> To: dev@dpdk.org
> >> Cc: Ferruh Yigit <ferruh.yigit@intel.com>; Declan Doherty
> >> <declan.doherty@intel.com>
> >> Subject: [dpdk-dev] [PATCH v6 4/4] ethdev: add shared counter support
> >> to rte_flow
> >>
> >> Add rte_flow_action_count action data structure to enable shared
> >> counters across multiple flows on a single port or across multiple
> >> flows on multiple ports within the same switch domain. Also this
> >> enables multiple count actions to be specified in a single flow action.
> >>
> >> This patch also modifies the existing rte_flow_query API to take the
> >> rte_flow_action structure as an input parameter instead of the
> >> rte_flow_action_type enumeration to allow querying a specific action
> >> from a flow rule when multiple actions of the same type are specified.
> >>
> >> This patch also contains updates for the bonding and failsafe PMDs
> >> and testpmd application which are affected by this API change.
> >>
> >> Signed-off-by: Declan Doherty <declan.doherty@intel.com>
> >> ---
> >> app/test-pmd/cmdline_flow.c | 6 ++--
> >> app/test-pmd/config.c | 15 +++++----
> >> app/test-pmd/testpmd.h | 2 +-
> >> doc/guides/prog_guide/rte_flow.rst | 59 +++++++++++++++++++++--
> ----
> >> ------
> >> drivers/net/bonding/rte_eth_bond_flow.c | 9 ++---
> >> drivers/net/failsafe/failsafe_flow.c | 4 +--
> >> lib/librte_ether/rte_flow.c | 2 +-
> >> lib/librte_ether/rte_flow.h | 38 +++++++++++++++++++--
> >> lib/librte_ether/rte_flow_driver.h | 2 +-
> >> 9 files changed, 93 insertions(+), 44 deletions(-)
> >>
> >> diff --git a/app/test-pmd/cmdline_flow.c
> >> b/app/test-pmd/cmdline_flow.c index 1ac04a0ab..5754e7858 100644
> >> --- a/app/test-pmd/cmdline_flow.c
> >> +++ b/app/test-pmd/cmdline_flow.c
> >> @@ -420,7 +420,7 @@ struct buffer {
> >> } destroy; /**< Destroy arguments. */
> >> struct {
> >> uint32_t rule;
> >> - enum rte_flow_action_type action;
> >> + struct rte_flow_action action;
> >> } query; /**< Query arguments. */
> >> struct {
> >> uint32_t *group;
> >> @@ -1101,7 +1101,7 @@ static const struct token token_list[] = {
> >> .next = NEXT(NEXT_ENTRY(QUERY_ACTION),
> >> NEXT_ENTRY(RULE_ID),
> >> NEXT_ENTRY(PORT_ID)),
> >> - .args = ARGS(ARGS_ENTRY(struct buffer, args.query.action),
> >> + .args = ARGS(ARGS_ENTRY(struct buffer,
> >> args.query.action.type),
> >> ARGS_ENTRY(struct buffer, args.query.rule),
> >> ARGS_ENTRY(struct buffer, port)),
> >> .call = parse_query,
> >> @@ -3842,7 +3842,7 @@ cmd_flow_parsed(const struct buffer *in)
> >> break;
> >> case QUERY:
> >> port_flow_query(in->port, in->args.query.rule,
> >> - in->args.query.action);
> >> + &in->args.query.action);
> >> break;
> >> case LIST:
> >> port_flow_list(in->port, in->args.list.group_n, diff --git
> >> a/app/test-pmd/config.c b/app/test-pmd/config.c index
> >> 0f2425229..cd6102dfc 100644
> >> --- a/app/test-pmd/config.c
> >> +++ b/app/test-pmd/config.c
> >> @@ -1452,7 +1452,7 @@ port_flow_flush(portid_t port_id)
> >> /** Query a flow rule. */
> >> int
> >> port_flow_query(portid_t port_id, uint32_t rule,
> >> - enum rte_flow_action_type action)
> >> + const struct rte_flow_action *action)
> >> {
> >> struct rte_flow_error error;
> >> struct rte_port *port;
> >> @@ -1474,15 +1474,16 @@ port_flow_query(portid_t port_id, uint32_t
> rule,
> >> return -ENOENT;
> >> }
> >> if ((unsigned int)action >= RTE_DIM(flow_action) ||
> >> - !flow_action[action].name)
> >> + !flow_action[action->type].name)
> >> name = "unknown";
> >> else
> >> - name = flow_action[action].name;
> >> - switch (action) {
> >> + name = flow_action[action->type].name;
> >> + switch (action->type) {
> >> case RTE_FLOW_ACTION_TYPE_COUNT:
> >> break;
> >> default:
> >> - printf("Cannot query action type %d (%s)\n", action, name);
> >> + printf("Cannot query action type %d (%s)\n",
> >> + action->type, name);
> >> return -ENOTSUP;
> >> }
> >> /* Poisoning to make sure PMDs update it in case of error. */ @@
> >> -1490,7 +1491,7 @@ port_flow_query(portid_t port_id, uint32_t rule,
> >> memset(&query, 0, sizeof(query));
> >> if (rte_flow_query(port_id, pf->flow, action, &query, &error))
> >> return port_flow_complain(&error);
> >> - switch (action) {
> >> + switch (action->type) {
> >> case RTE_FLOW_ACTION_TYPE_COUNT:
> >> printf("%s:\n"
> >> " hits_set: %u\n"
> >> @@ -1505,7 +1506,7 @@ port_flow_query(portid_t port_id, uint32_t
> rule,
> >> break;
> >> default:
> >> printf("Cannot display result for action type %d (%s)\n",
> >> - action, name);
> >> + action->type, name);
> >> break;
> >> }
> >> return 0;
> >> diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h index
> >> a33b525e2..1af87b8f4 100644
> >> --- a/app/test-pmd/testpmd.h
> >> +++ b/app/test-pmd/testpmd.h
> >> @@ -620,7 +620,7 @@ int port_flow_create(portid_t port_id, int
> >> port_flow_destroy(portid_t port_id, uint32_t n, const uint32_t
> >> *rule); int port_flow_flush(portid_t port_id); int
> >> port_flow_query(portid_t port_id, uint32_t rule,
> >> - enum rte_flow_action_type action);
> >> + const struct rte_flow_action *action);
> >> void port_flow_list(portid_t port_id, uint32_t n, const uint32_t
> >> *group); int port_flow_isolate(portid_t port_id, int set);
> >>
> >> diff --git a/doc/guides/prog_guide/rte_flow.rst
> >> b/doc/guides/prog_guide/rte_flow.rst
> >> index 301f8762e..88bfc87eb 100644
> >> --- a/doc/guides/prog_guide/rte_flow.rst
> >> +++ b/doc/guides/prog_guide/rte_flow.rst
> >> @@ -1277,17 +1277,19 @@ Actions are performed in list order:
> >>
> >> .. table:: Mark, count then redirect
> >>
> >> - +-------+--------+-----------+-------+
> >> - | Index | Action | Field | Value |
> >> - +=======+========+===========+=======+
> >> - | 0 | MARK | ``mark`` | 0x2a |
> >> - +-------+--------+-----------+-------+
> >> - | 1 | COUNT |
> >> - +-------+--------+-----------+-------+
> >> - | 2 | QUEUE | ``queue`` | 10 |
> >> - +-------+--------+-----------+-------+
> >> - | 3 | END |
> >> - +-------+----------------------------+
> >> + +-------+--------+------------+-------+
> >> + | Index | Action | Field | Value |
> >> + +=======+========+============+=======+
> >> + | 0 | MARK | ``mark`` | 0x2a |
> >> + +-------+--------+------------+-------+
> >> + | 1 | COUNT | ``shared`` | 0 |
> >> + | | +------------+-------+
> >> + | | | ``id`` | 0 |
> >> + +-------+--------+------------+-------+
> >> + | 2 | QUEUE | ``queue`` | 10 |
> >> + +-------+--------+------------+-------+
> >> + | 3 | END |
> >> + +-------+-----------------------------+
> >>
> >> |
> >>
> >> @@ -1516,23 +1518,36 @@ Drop packets.
> >> Action: ``COUNT``
> >> ^^^^^^^^^^^^^^^^^
> >>
> >> -Enables counters for this rule.
> >> +Adds a counter action to a matched flow.
> >> +
> >> +If more than one count action is specified in a single flow rule,
> >> +then each action must specify a unique id.
> >>
> >> -These counters can be retrieved and reset through
> >> ``rte_flow_query()``, see
> >> +Counters can be retrieved and reset through ``rte_flow_query()``,
> >> +see
> >> ``struct rte_flow_query_count``.
> >>
> >> -- Counters can be retrieved with ``rte_flow_query()``.
> >> -- No configurable properties.
> >> +The shared flag indicates whether the counter is unique to the flow
> >> +rule the action is specified with, or whether it is a shared counter.
> >> +
> >> +For a count action with the shared flag set, then then a global
> >> +device namespace is assumed for the counter id, so that any matched
> >> +flow rules
> >> using
> >> +a count action with the same counter id on the same port will
> >> +contribute to that counter.
> >> +
> >> +For ports within the same switch domain then the counter id
> >> +namespace
> >> extends
> >> +to all ports within that switch domain.
> >>
> >> .. _table_rte_flow_action_count:
> >>
> >> .. table:: COUNT
> >>
> >> - +---------------+
> >> - | Field |
> >> - +===============+
> >> - | no properties |
> >> - +---------------+
> >> + +------------+---------------------+
> >> + | Field | Value |
> >> + +============+=====================+
> >> + | ``shared`` | shared counter flag |
> >> + +------------+---------------------+
> >> + | ``id`` | counter id |
> >> + +------------+---------------------+
> >>
> >> Query structure to retrieve and reset flow rule counters:
> >>
> >> @@ -2282,7 +2297,7 @@ definition.
> >> int
> >> rte_flow_query(uint16_t port_id,
> >> struct rte_flow *flow,
> >> - enum rte_flow_action_type action,
> >> + const struct rte_flow_action *action,
> >> void *data,
> >> struct rte_flow_error *error);
> >>
> >> @@ -2290,7 +2305,7 @@ Arguments:
> >>
> >> - ``port_id``: port identifier of Ethernet device.
> >> - ``flow``: flow rule handle to query.
> >> -- ``action``: action type to query.
> >> +- ``action``: action to query, this must match prototype from flow rule.
> >> - ``data``: pointer to storage for the associated query data type.
> >> - ``error``: perform verbose error reporting if not NULL. PMDs initialize
> >> this structure in case of error only.
> >> diff --git a/drivers/net/bonding/rte_eth_bond_flow.c
> >> b/drivers/net/bonding/rte_eth_bond_flow.c
> >> index 8093c04f5..31e4bcaeb 100644
> >> --- a/drivers/net/bonding/rte_eth_bond_flow.c
> >> +++ b/drivers/net/bonding/rte_eth_bond_flow.c
> >> @@ -152,6 +152,7 @@ bond_flow_flush(struct rte_eth_dev *dev, struct
> >> rte_flow_error *err)
> >>
> >> static int
> >> bond_flow_query_count(struct rte_eth_dev *dev, struct rte_flow
> >> *flow,
> >> + const struct rte_flow_action *action,
> >> struct rte_flow_query_count *count,
> >> struct rte_flow_error *err)
> >> {
> >> @@ -165,7 +166,7 @@ bond_flow_query_count(struct rte_eth_dev
> *dev,
> >> struct rte_flow *flow,
> >> rte_memcpy(&slave_count, count, sizeof(slave_count));
> >> for (i = 0; i < internals->slave_count; i++) {
> >> ret = rte_flow_query(internals->slaves[i].port_id,
> >> - flow->flows[i],
> >> RTE_FLOW_ACTION_TYPE_COUNT,
> >> + flow->flows[i], action,
> >> &slave_count, err);
> >> if (unlikely(ret != 0)) {
> >> RTE_BOND_LOG(ERR, "Failed to query flow on"
> >> @@ -182,12 +183,12 @@ bond_flow_query_count(struct rte_eth_dev
> *dev,
> >> struct rte_flow *flow,
> >>
> >> static int
> >> bond_flow_query(struct rte_eth_dev *dev, struct rte_flow *flow,
> >> - enum rte_flow_action_type type, void *arg,
> >> + const struct rte_flow_action *action, void *arg,
> >> struct rte_flow_error *err)
> >> {
> >> - switch (type) {
> >> + switch (action->type) {
> >> case RTE_FLOW_ACTION_TYPE_COUNT:
> >> - return bond_flow_query_count(dev, flow, arg, err);
> >> + return bond_flow_query_count(dev, flow, action, arg, err);
> >> default:
> >> return rte_flow_error_set(err, ENOTSUP,
> >> RTE_FLOW_ERROR_TYPE_ACTION,
> >> arg,
> >> diff --git a/drivers/net/failsafe/failsafe_flow.c
> >> b/drivers/net/failsafe/failsafe_flow.c
> >> index a97f4075d..bfe42fcee 100644
> >> --- a/drivers/net/failsafe/failsafe_flow.c
> >> +++ b/drivers/net/failsafe/failsafe_flow.c
> >> @@ -174,7 +174,7 @@ fs_flow_flush(struct rte_eth_dev *dev, static
> >> int fs_flow_query(struct rte_eth_dev *dev,
> >> struct rte_flow *flow,
> >> - enum rte_flow_action_type type,
> >> + const struct rte_flow_action *action,
> >> void *arg,
> >> struct rte_flow_error *error) { @@ -185,7 +185,7 @@
> >> fs_flow_query(struct rte_eth_dev *dev,
> >> if (sdev != NULL) {
> >> int ret = rte_flow_query(PORT_ID(sdev),
> >> flow->flows[SUB_ID(sdev)],
> >> - type, arg, error);
> >> + action, arg, error);
> >>
> >> if ((ret = fs_err(sdev, ret))) {
> >> fs_unlock(dev, 0);
> >> diff --git a/lib/librte_ether/rte_flow.c
> >> b/lib/librte_ether/rte_flow.c index 4f94ac9b5..7947529da 100644
> >> --- a/lib/librte_ether/rte_flow.c
> >> +++ b/lib/librte_ether/rte_flow.c
> >> @@ -233,7 +233,7 @@ rte_flow_flush(uint16_t port_id, int
> >> rte_flow_query(uint16_t port_id,
> >> struct rte_flow *flow,
> >> - enum rte_flow_action_type action,
> >> + const struct rte_flow_action *action,
> >> void *data,
> >> struct rte_flow_error *error) { diff --git
> >> a/lib/librte_ether/rte_flow.h b/lib/librte_ether/rte_flow.h index
> >> d390bbf5a..f8ba71cdb 100644
> >> --- a/lib/librte_ether/rte_flow.h
> >> +++ b/lib/librte_ether/rte_flow.h
> >> @@ -1314,7 +1314,7 @@ enum rte_flow_action_type {
> >> * These counters can be retrieved and reset through
> >> rte_flow_query(),
> >> * see struct rte_flow_query_count.
> >> *
> >> - * No associated configuration structure.
> >> + * See struct rte_flow_action_count.
> >> */
> >> RTE_FLOW_ACTION_TYPE_COUNT,
> >>
> >> @@ -1546,6 +1546,38 @@ 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_COUNT
> >> + *
> >> + * Adds a counter action to a matched flow.
> >> + *
> >> + * If more than one count action is specified in a single flow rule,
> >> +then each
> >> + * action must specify a unique id.
> >> + *
> >> + * Counters can be retrieved and reset through ``rte_flow_query()``,
> >> +see
> >> + * ``struct rte_flow_query_count``.
> >> + *
> >> + * The shared flag indicates whether the counter is unique to the
> >> +flow rule
> >> the
> >> + * action is specified with, or whether it is a shared counter.
> >> + *
> >> + * For a count action with the shared flag set, then then a global
> >> + device
> >> + * namespace is assumed for the counter id, so that any matched flow
> >> + rules
> >> using
> >> + * a count action with the same counter id on the same port will
> >> + contribute
> >> to
> >> + * that counter.
> >> + *
> >> + * For ports within the same switch domain then the counter id
> >> + namespace
> >> extends
> >> + * to all ports within that switch domain.
> >> + */
> >> +struct rte_flow_action_count {
> >> + uint32_t shared:1; /**< Share counter ID with other flow rules. */
> >> + uint32_t reserved:31; /**< Reserved, must be zero. */
> >> + uint32_t id; /**< Counter ID. */
> >> +};
> >> +
> >> /**
> >> * RTE_FLOW_ACTION_TYPE_COUNT (query)
> >> *
> >> @@ -2044,7 +2076,7 @@ rte_flow_flush(uint16_t port_id,
> >> * @param flow
> >> * Flow rule handle to query.
> >> * @param action
> >> - * Action type to query.
> >> + * Action definition as defined in original flow rule.
> >> * @param[in, out] data
> >> * Pointer to storage for the associated query data type.
> >> * @param[out] error
> >> @@ -2057,7 +2089,7 @@ rte_flow_flush(uint16_t port_id, int
> >> rte_flow_query(uint16_t port_id,
> >> struct rte_flow *flow,
> >> - enum rte_flow_action_type action,
> >> + const struct rte_flow_action *action,
> >> void *data,
> >> struct rte_flow_error *error);
> >>
> >> diff --git a/lib/librte_ether/rte_flow_driver.h
> >> b/lib/librte_ether/rte_flow_driver.h
> >> index 3800310ba..1c90c600d 100644
> >> --- a/lib/librte_ether/rte_flow_driver.h
> >> +++ b/lib/librte_ether/rte_flow_driver.h
> >> @@ -88,7 +88,7 @@ struct rte_flow_ops {
> >> int (*query)
> >> (struct rte_eth_dev *,
> >> struct rte_flow *,
> >> - enum rte_flow_action_type,
> >> + const struct rte_flow_action *,
> >> void *,
> >> struct rte_flow_error *);
> >> /** See rte_flow_isolate(). */
> >> --
> >> 2.14.3
> >
On 26/04/2018 3:43 PM, Ori Kam wrote:
> Hi,
>
> PSB
>
> Ori
>> -----Original Message-----
>> From: Ferruh Yigit [mailto:ferruh.yigit@intel.com]
>> Sent: Thursday, April 26, 2018 5:28 PM
>> To: Ori Kam <orika@mellanox.com>; Declan Doherty
>> <declan.doherty@intel.com>; dev@dpdk.org; Matan Azrad
>> <matan@mellanox.com>
>> Subject: Re: [dpdk-dev] [PATCH v6 4/4] ethdev: add shared counter support
>> to rte_flow
>>
>> On 4/26/2018 3:06 PM, Ori Kam wrote:
>>> Hi Declan,
>>>
>>> You are changing API (port_flow_query) which is in use in both MLX5
>>> and MLX4 this results in breaking the build.
>>
>> Hi Ori,
>>
>> Do you mean "rte_flow_query"? port_flow_query() is a function in testpmd,
>> how mlx is using it?
>>
>
> My bad let me be clearer.
> MLX5 and MLX4 are implementing the rte_flow_ops query function.
> This patch changes the prototype for the query function which results in
> compilation error and should also result in some change in the MLX5 and MLX4 implementation.
>
>
Hey Ori, I don't see any references to the query in MLX4 code.
static const struct rte_flow_ops mlx4_flow_ops = {
.validate = mlx4_flow_validate,
.create = mlx4_flow_create,
.destroy = mlx4_flow_destroy,
.flush = mlx4_flow_flush,
.isolate = mlx4_flow_isolate,
};
I just looking at fixing the MLX5 code compilation, I missed that due to
the function being ifdefined out with
HAVE_IBV_DEVICE_COUNTERS_SET_SUPPORT the flag and I missed the reference.
>>>
>>> Best,
>>> Ori
>>>
>>>> -----Original Message-----
>>>> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Declan Doherty
>>>> Sent: Thursday, April 26, 2018 3:08 PM
>>>> To: dev@dpdk.org
>>>> Cc: Ferruh Yigit <ferruh.yigit@intel.com>; Declan Doherty
>>>> <declan.doherty@intel.com>
>>>> Subject: [dpdk-dev] [PATCH v6 4/4] ethdev: add shared counter support
>>>> to rte_flow
>>>>
>>>> Add rte_flow_action_count action data structure to enable shared
>>>> counters across multiple flows on a single port or across multiple
>>>> flows on multiple ports within the same switch domain. Also this
>>>> enables multiple count actions to be specified in a single flow action.
>>>>
>>>> This patch also modifies the existing rte_flow_query API to take the
>>>> rte_flow_action structure as an input parameter instead of the
>>>> rte_flow_action_type enumeration to allow querying a specific action
>>>> from a flow rule when multiple actions of the same type are specified.
>>>>
>>>> This patch also contains updates for the bonding and failsafe PMDs
>>>> and testpmd application which are affected by this API change.
>>>>
>>>> Signed-off-by: Declan Doherty <declan.doherty@intel.com>
>>>> ---
>>>> app/test-pmd/cmdline_flow.c | 6 ++--
>>>> app/test-pmd/config.c | 15 +++++----
>>>> app/test-pmd/testpmd.h | 2 +-
>>>> doc/guides/prog_guide/rte_flow.rst | 59 +++++++++++++++++++++--
>> ----
>>>> ------
>>>> drivers/net/bonding/rte_eth_bond_flow.c | 9 ++---
>>>> drivers/net/failsafe/failsafe_flow.c | 4 +--
>>>> lib/librte_ether/rte_flow.c | 2 +-
>>>> lib/librte_ether/rte_flow.h | 38 +++++++++++++++++++--
>>>> lib/librte_ether/rte_flow_driver.h | 2 +-
>>>> 9 files changed, 93 insertions(+), 44 deletions(-)
>>>>
>>>> diff --git a/app/test-pmd/cmdline_flow.c
>>>> b/app/test-pmd/cmdline_flow.c index 1ac04a0ab..5754e7858 100644
>>>> --- a/app/test-pmd/cmdline_flow.c
>>>> +++ b/app/test-pmd/cmdline_flow.c
>>>> @@ -420,7 +420,7 @@ struct buffer {
>>>> } destroy; /**< Destroy arguments. */
>>>> struct {
>>>> uint32_t rule;
>>>> - enum rte_flow_action_type action;
>>>> + struct rte_flow_action action;
>>>> } query; /**< Query arguments. */
>>>> struct {
>>>> uint32_t *group;
>>>> @@ -1101,7 +1101,7 @@ static const struct token token_list[] = {
>>>> .next = NEXT(NEXT_ENTRY(QUERY_ACTION),
>>>> NEXT_ENTRY(RULE_ID),
>>>> NEXT_ENTRY(PORT_ID)),
>>>> - .args = ARGS(ARGS_ENTRY(struct buffer, args.query.action),
>>>> + .args = ARGS(ARGS_ENTRY(struct buffer,
>>>> args.query.action.type),
>>>> ARGS_ENTRY(struct buffer, args.query.rule),
>>>> ARGS_ENTRY(struct buffer, port)),
>>>> .call = parse_query,
>>>> @@ -3842,7 +3842,7 @@ cmd_flow_parsed(const struct buffer *in)
>>>> break;
>>>> case QUERY:
>>>> port_flow_query(in->port, in->args.query.rule,
>>>> - in->args.query.action);
>>>> + &in->args.query.action);
>>>> break;
>>>> case LIST:
>>>> port_flow_list(in->port, in->args.list.group_n, diff --git
>>>> a/app/test-pmd/config.c b/app/test-pmd/config.c index
>>>> 0f2425229..cd6102dfc 100644
>>>> --- a/app/test-pmd/config.c
>>>> +++ b/app/test-pmd/config.c
>>>> @@ -1452,7 +1452,7 @@ port_flow_flush(portid_t port_id)
>>>> /** Query a flow rule. */
>>>> int
>>>> port_flow_query(portid_t port_id, uint32_t rule,
>>>> - enum rte_flow_action_type action)
>>>> + const struct rte_flow_action *action)
>>>> {
>>>> struct rte_flow_error error;
>>>> struct rte_port *port;
>>>> @@ -1474,15 +1474,16 @@ port_flow_query(portid_t port_id, uint32_t
>> rule,
>>>> return -ENOENT;
>>>> }
>>>> if ((unsigned int)action >= RTE_DIM(flow_action) ||
>>>> - !flow_action[action].name)
>>>> + !flow_action[action->type].name)
>>>> name = "unknown";
>>>> else
>>>> - name = flow_action[action].name;
>>>> - switch (action) {
>>>> + name = flow_action[action->type].name;
>>>> + switch (action->type) {
>>>> case RTE_FLOW_ACTION_TYPE_COUNT:
>>>> break;
>>>> default:
>>>> - printf("Cannot query action type %d (%s)\n", action, name);
>>>> + printf("Cannot query action type %d (%s)\n",
>>>> + action->type, name);
>>>> return -ENOTSUP;
>>>> }
>>>> /* Poisoning to make sure PMDs update it in case of error. */ @@
>>>> -1490,7 +1491,7 @@ port_flow_query(portid_t port_id, uint32_t rule,
>>>> memset(&query, 0, sizeof(query));
>>>> if (rte_flow_query(port_id, pf->flow, action, &query, &error))
>>>> return port_flow_complain(&error);
>>>> - switch (action) {
>>>> + switch (action->type) {
>>>> case RTE_FLOW_ACTION_TYPE_COUNT:
>>>> printf("%s:\n"
>>>> " hits_set: %u\n"
>>>> @@ -1505,7 +1506,7 @@ port_flow_query(portid_t port_id, uint32_t
>> rule,
>>>> break;
>>>> default:
>>>> printf("Cannot display result for action type %d (%s)\n",
>>>> - action, name);
>>>> + action->type, name);
>>>> break;
>>>> }
>>>> return 0;
>>>> diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h index
>>>> a33b525e2..1af87b8f4 100644
>>>> --- a/app/test-pmd/testpmd.h
>>>> +++ b/app/test-pmd/testpmd.h
>>>> @@ -620,7 +620,7 @@ int port_flow_create(portid_t port_id, int
>>>> port_flow_destroy(portid_t port_id, uint32_t n, const uint32_t
>>>> *rule); int port_flow_flush(portid_t port_id); int
>>>> port_flow_query(portid_t port_id, uint32_t rule,
>>>> - enum rte_flow_action_type action);
>>>> + const struct rte_flow_action *action);
>>>> void port_flow_list(portid_t port_id, uint32_t n, const uint32_t
>>>> *group); int port_flow_isolate(portid_t port_id, int set);
>>>>
>>>> diff --git a/doc/guides/prog_guide/rte_flow.rst
>>>> b/doc/guides/prog_guide/rte_flow.rst
>>>> index 301f8762e..88bfc87eb 100644
>>>> --- a/doc/guides/prog_guide/rte_flow.rst
>>>> +++ b/doc/guides/prog_guide/rte_flow.rst
>>>> @@ -1277,17 +1277,19 @@ Actions are performed in list order:
>>>>
>>>> .. table:: Mark, count then redirect
>>>>
>>>> - +-------+--------+-----------+-------+
>>>> - | Index | Action | Field | Value |
>>>> - +=======+========+===========+=======+
>>>> - | 0 | MARK | ``mark`` | 0x2a |
>>>> - +-------+--------+-----------+-------+
>>>> - | 1 | COUNT |
>>>> - +-------+--------+-----------+-------+
>>>> - | 2 | QUEUE | ``queue`` | 10 |
>>>> - +-------+--------+-----------+-------+
>>>> - | 3 | END |
>>>> - +-------+----------------------------+
>>>> + +-------+--------+------------+-------+
>>>> + | Index | Action | Field | Value |
>>>> + +=======+========+============+=======+
>>>> + | 0 | MARK | ``mark`` | 0x2a |
>>>> + +-------+--------+------------+-------+
>>>> + | 1 | COUNT | ``shared`` | 0 |
>>>> + | | +------------+-------+
>>>> + | | | ``id`` | 0 |
>>>> + +-------+--------+------------+-------+
>>>> + | 2 | QUEUE | ``queue`` | 10 |
>>>> + +-------+--------+------------+-------+
>>>> + | 3 | END |
>>>> + +-------+-----------------------------+
>>>>
>>>> |
>>>>
>>>> @@ -1516,23 +1518,36 @@ Drop packets.
>>>> Action: ``COUNT``
>>>> ^^^^^^^^^^^^^^^^^
>>>>
>>>> -Enables counters for this rule.
>>>> +Adds a counter action to a matched flow.
>>>> +
>>>> +If more than one count action is specified in a single flow rule,
>>>> +then each action must specify a unique id.
>>>>
>>>> -These counters can be retrieved and reset through
>>>> ``rte_flow_query()``, see
>>>> +Counters can be retrieved and reset through ``rte_flow_query()``,
>>>> +see
>>>> ``struct rte_flow_query_count``.
>>>>
>>>> -- Counters can be retrieved with ``rte_flow_query()``.
>>>> -- No configurable properties.
>>>> +The shared flag indicates whether the counter is unique to the flow
>>>> +rule the action is specified with, or whether it is a shared counter.
>>>> +
>>>> +For a count action with the shared flag set, then then a global
>>>> +device namespace is assumed for the counter id, so that any matched
>>>> +flow rules
>>>> using
>>>> +a count action with the same counter id on the same port will
>>>> +contribute to that counter.
>>>> +
>>>> +For ports within the same switch domain then the counter id
>>>> +namespace
>>>> extends
>>>> +to all ports within that switch domain.
>>>>
>>>> .. _table_rte_flow_action_count:
>>>>
>>>> .. table:: COUNT
>>>>
>>>> - +---------------+
>>>> - | Field |
>>>> - +===============+
>>>> - | no properties |
>>>> - +---------------+
>>>> + +------------+---------------------+
>>>> + | Field | Value |
>>>> + +============+=====================+
>>>> + | ``shared`` | shared counter flag |
>>>> + +------------+---------------------+
>>>> + | ``id`` | counter id |
>>>> + +------------+---------------------+
>>>>
>>>> Query structure to retrieve and reset flow rule counters:
>>>>
>>>> @@ -2282,7 +2297,7 @@ definition.
>>>> int
>>>> rte_flow_query(uint16_t port_id,
>>>> struct rte_flow *flow,
>>>> - enum rte_flow_action_type action,
>>>> + const struct rte_flow_action *action,
>>>> void *data,
>>>> struct rte_flow_error *error);
>>>>
>>>> @@ -2290,7 +2305,7 @@ Arguments:
>>>>
>>>> - ``port_id``: port identifier of Ethernet device.
>>>> - ``flow``: flow rule handle to query.
>>>> -- ``action``: action type to query.
>>>> +- ``action``: action to query, this must match prototype from flow rule.
>>>> - ``data``: pointer to storage for the associated query data type.
>>>> - ``error``: perform verbose error reporting if not NULL. PMDs initialize
>>>> this structure in case of error only.
>>>> diff --git a/drivers/net/bonding/rte_eth_bond_flow.c
>>>> b/drivers/net/bonding/rte_eth_bond_flow.c
>>>> index 8093c04f5..31e4bcaeb 100644
>>>> --- a/drivers/net/bonding/rte_eth_bond_flow.c
>>>> +++ b/drivers/net/bonding/rte_eth_bond_flow.c
>>>> @@ -152,6 +152,7 @@ bond_flow_flush(struct rte_eth_dev *dev, struct
>>>> rte_flow_error *err)
>>>>
>>>> static int
>>>> bond_flow_query_count(struct rte_eth_dev *dev, struct rte_flow
>>>> *flow,
>>>> + const struct rte_flow_action *action,
>>>> struct rte_flow_query_count *count,
>>>> struct rte_flow_error *err)
>>>> {
>>>> @@ -165,7 +166,7 @@ bond_flow_query_count(struct rte_eth_dev
>> *dev,
>>>> struct rte_flow *flow,
>>>> rte_memcpy(&slave_count, count, sizeof(slave_count));
>>>> for (i = 0; i < internals->slave_count; i++) {
>>>> ret = rte_flow_query(internals->slaves[i].port_id,
>>>> - flow->flows[i],
>>>> RTE_FLOW_ACTION_TYPE_COUNT,
>>>> + flow->flows[i], action,
>>>> &slave_count, err);
>>>> if (unlikely(ret != 0)) {
>>>> RTE_BOND_LOG(ERR, "Failed to query flow on"
>>>> @@ -182,12 +183,12 @@ bond_flow_query_count(struct rte_eth_dev
>> *dev,
>>>> struct rte_flow *flow,
>>>>
>>>> static int
>>>> bond_flow_query(struct rte_eth_dev *dev, struct rte_flow *flow,
>>>> - enum rte_flow_action_type type, void *arg,
>>>> + const struct rte_flow_action *action, void *arg,
>>>> struct rte_flow_error *err)
>>>> {
>>>> - switch (type) {
>>>> + switch (action->type) {
>>>> case RTE_FLOW_ACTION_TYPE_COUNT:
>>>> - return bond_flow_query_count(dev, flow, arg, err);
>>>> + return bond_flow_query_count(dev, flow, action, arg, err);
>>>> default:
>>>> return rte_flow_error_set(err, ENOTSUP,
>>>> RTE_FLOW_ERROR_TYPE_ACTION,
>>>> arg,
>>>> diff --git a/drivers/net/failsafe/failsafe_flow.c
>>>> b/drivers/net/failsafe/failsafe_flow.c
>>>> index a97f4075d..bfe42fcee 100644
>>>> --- a/drivers/net/failsafe/failsafe_flow.c
>>>> +++ b/drivers/net/failsafe/failsafe_flow.c
>>>> @@ -174,7 +174,7 @@ fs_flow_flush(struct rte_eth_dev *dev, static
>>>> int fs_flow_query(struct rte_eth_dev *dev,
>>>> struct rte_flow *flow,
>>>> - enum rte_flow_action_type type,
>>>> + const struct rte_flow_action *action,
>>>> void *arg,
>>>> struct rte_flow_error *error) { @@ -185,7 +185,7 @@
>>>> fs_flow_query(struct rte_eth_dev *dev,
>>>> if (sdev != NULL) {
>>>> int ret = rte_flow_query(PORT_ID(sdev),
>>>> flow->flows[SUB_ID(sdev)],
>>>> - type, arg, error);
>>>> + action, arg, error);
>>>>
>>>> if ((ret = fs_err(sdev, ret))) {
>>>> fs_unlock(dev, 0);
>>>> diff --git a/lib/librte_ether/rte_flow.c
>>>> b/lib/librte_ether/rte_flow.c index 4f94ac9b5..7947529da 100644
>>>> --- a/lib/librte_ether/rte_flow.c
>>>> +++ b/lib/librte_ether/rte_flow.c
>>>> @@ -233,7 +233,7 @@ rte_flow_flush(uint16_t port_id, int
>>>> rte_flow_query(uint16_t port_id,
>>>> struct rte_flow *flow,
>>>> - enum rte_flow_action_type action,
>>>> + const struct rte_flow_action *action,
>>>> void *data,
>>>> struct rte_flow_error *error) { diff --git
>>>> a/lib/librte_ether/rte_flow.h b/lib/librte_ether/rte_flow.h index
>>>> d390bbf5a..f8ba71cdb 100644
>>>> --- a/lib/librte_ether/rte_flow.h
>>>> +++ b/lib/librte_ether/rte_flow.h
>>>> @@ -1314,7 +1314,7 @@ enum rte_flow_action_type {
>>>> * These counters can be retrieved and reset through
>>>> rte_flow_query(),
>>>> * see struct rte_flow_query_count.
>>>> *
>>>> - * No associated configuration structure.
>>>> + * See struct rte_flow_action_count.
>>>> */
>>>> RTE_FLOW_ACTION_TYPE_COUNT,
>>>>
>>>> @@ -1546,6 +1546,38 @@ 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_COUNT
>>>> + *
>>>> + * Adds a counter action to a matched flow.
>>>> + *
>>>> + * If more than one count action is specified in a single flow rule,
>>>> +then each
>>>> + * action must specify a unique id.
>>>> + *
>>>> + * Counters can be retrieved and reset through ``rte_flow_query()``,
>>>> +see
>>>> + * ``struct rte_flow_query_count``.
>>>> + *
>>>> + * The shared flag indicates whether the counter is unique to the
>>>> +flow rule
>>>> the
>>>> + * action is specified with, or whether it is a shared counter.
>>>> + *
>>>> + * For a count action with the shared flag set, then then a global
>>>> + device
>>>> + * namespace is assumed for the counter id, so that any matched flow
>>>> + rules
>>>> using
>>>> + * a count action with the same counter id on the same port will
>>>> + contribute
>>>> to
>>>> + * that counter.
>>>> + *
>>>> + * For ports within the same switch domain then the counter id
>>>> + namespace
>>>> extends
>>>> + * to all ports within that switch domain.
>>>> + */
>>>> +struct rte_flow_action_count {
>>>> + uint32_t shared:1; /**< Share counter ID with other flow rules. */
>>>> + uint32_t reserved:31; /**< Reserved, must be zero. */
>>>> + uint32_t id; /**< Counter ID. */
>>>> +};
>>>> +
>>>> /**
>>>> * RTE_FLOW_ACTION_TYPE_COUNT (query)
>>>> *
>>>> @@ -2044,7 +2076,7 @@ rte_flow_flush(uint16_t port_id,
>>>> * @param flow
>>>> * Flow rule handle to query.
>>>> * @param action
>>>> - * Action type to query.
>>>> + * Action definition as defined in original flow rule.
>>>> * @param[in, out] data
>>>> * Pointer to storage for the associated query data type.
>>>> * @param[out] error
>>>> @@ -2057,7 +2089,7 @@ rte_flow_flush(uint16_t port_id, int
>>>> rte_flow_query(uint16_t port_id,
>>>> struct rte_flow *flow,
>>>> - enum rte_flow_action_type action,
>>>> + const struct rte_flow_action *action,
>>>> void *data,
>>>> struct rte_flow_error *error);
>>>>
>>>> diff --git a/lib/librte_ether/rte_flow_driver.h
>>>> b/lib/librte_ether/rte_flow_driver.h
>>>> index 3800310ba..1c90c600d 100644
>>>> --- a/lib/librte_ether/rte_flow_driver.h
>>>> +++ b/lib/librte_ether/rte_flow_driver.h
>>>> @@ -88,7 +88,7 @@ struct rte_flow_ops {
>>>> int (*query)
>>>> (struct rte_eth_dev *,
>>>> struct rte_flow *,
>>>> - enum rte_flow_action_type,
>>>> + const struct rte_flow_action *,
>>>> void *,
>>>> struct rte_flow_error *);
>>>> /** See rte_flow_isolate(). */
>>>> --
>>>> 2.14.3
>>>
>
@@ -420,7 +420,7 @@ struct buffer {
} destroy; /**< Destroy arguments. */
struct {
uint32_t rule;
- enum rte_flow_action_type action;
+ struct rte_flow_action action;
} query; /**< Query arguments. */
struct {
uint32_t *group;
@@ -1101,7 +1101,7 @@ static const struct token token_list[] = {
.next = NEXT(NEXT_ENTRY(QUERY_ACTION),
NEXT_ENTRY(RULE_ID),
NEXT_ENTRY(PORT_ID)),
- .args = ARGS(ARGS_ENTRY(struct buffer, args.query.action),
+ .args = ARGS(ARGS_ENTRY(struct buffer, args.query.action.type),
ARGS_ENTRY(struct buffer, args.query.rule),
ARGS_ENTRY(struct buffer, port)),
.call = parse_query,
@@ -3842,7 +3842,7 @@ cmd_flow_parsed(const struct buffer *in)
break;
case QUERY:
port_flow_query(in->port, in->args.query.rule,
- in->args.query.action);
+ &in->args.query.action);
break;
case LIST:
port_flow_list(in->port, in->args.list.group_n,
@@ -1452,7 +1452,7 @@ port_flow_flush(portid_t port_id)
/** Query a flow rule. */
int
port_flow_query(portid_t port_id, uint32_t rule,
- enum rte_flow_action_type action)
+ const struct rte_flow_action *action)
{
struct rte_flow_error error;
struct rte_port *port;
@@ -1474,15 +1474,16 @@ port_flow_query(portid_t port_id, uint32_t rule,
return -ENOENT;
}
if ((unsigned int)action >= RTE_DIM(flow_action) ||
- !flow_action[action].name)
+ !flow_action[action->type].name)
name = "unknown";
else
- name = flow_action[action].name;
- switch (action) {
+ name = flow_action[action->type].name;
+ switch (action->type) {
case RTE_FLOW_ACTION_TYPE_COUNT:
break;
default:
- printf("Cannot query action type %d (%s)\n", action, name);
+ printf("Cannot query action type %d (%s)\n",
+ action->type, name);
return -ENOTSUP;
}
/* Poisoning to make sure PMDs update it in case of error. */
@@ -1490,7 +1491,7 @@ port_flow_query(portid_t port_id, uint32_t rule,
memset(&query, 0, sizeof(query));
if (rte_flow_query(port_id, pf->flow, action, &query, &error))
return port_flow_complain(&error);
- switch (action) {
+ switch (action->type) {
case RTE_FLOW_ACTION_TYPE_COUNT:
printf("%s:\n"
" hits_set: %u\n"
@@ -1505,7 +1506,7 @@ port_flow_query(portid_t port_id, uint32_t rule,
break;
default:
printf("Cannot display result for action type %d (%s)\n",
- action, name);
+ action->type, name);
break;
}
return 0;
@@ -620,7 +620,7 @@ int port_flow_create(portid_t port_id,
int port_flow_destroy(portid_t port_id, uint32_t n, const uint32_t *rule);
int port_flow_flush(portid_t port_id);
int port_flow_query(portid_t port_id, uint32_t rule,
- enum rte_flow_action_type action);
+ const struct rte_flow_action *action);
void port_flow_list(portid_t port_id, uint32_t n, const uint32_t *group);
int port_flow_isolate(portid_t port_id, int set);
@@ -1277,17 +1277,19 @@ Actions are performed in list order:
.. table:: Mark, count then redirect
- +-------+--------+-----------+-------+
- | Index | Action | Field | Value |
- +=======+========+===========+=======+
- | 0 | MARK | ``mark`` | 0x2a |
- +-------+--------+-----------+-------+
- | 1 | COUNT |
- +-------+--------+-----------+-------+
- | 2 | QUEUE | ``queue`` | 10 |
- +-------+--------+-----------+-------+
- | 3 | END |
- +-------+----------------------------+
+ +-------+--------+------------+-------+
+ | Index | Action | Field | Value |
+ +=======+========+============+=======+
+ | 0 | MARK | ``mark`` | 0x2a |
+ +-------+--------+------------+-------+
+ | 1 | COUNT | ``shared`` | 0 |
+ | | +------------+-------+
+ | | | ``id`` | 0 |
+ +-------+--------+------------+-------+
+ | 2 | QUEUE | ``queue`` | 10 |
+ +-------+--------+------------+-------+
+ | 3 | END |
+ +-------+-----------------------------+
|
@@ -1516,23 +1518,36 @@ Drop packets.
Action: ``COUNT``
^^^^^^^^^^^^^^^^^
-Enables counters for this rule.
+Adds a counter action to a matched flow.
+
+If more than one count action is specified in a single flow rule, then each
+action must specify a unique id.
-These counters can be retrieved and reset through ``rte_flow_query()``, see
+Counters can be retrieved and reset through ``rte_flow_query()``, see
``struct rte_flow_query_count``.
-- Counters can be retrieved with ``rte_flow_query()``.
-- No configurable properties.
+The shared flag indicates whether the counter is unique to the flow rule the
+action is specified with, or whether it is a shared counter.
+
+For a count action with the shared flag set, then then a global device
+namespace is assumed for the counter id, so that any matched flow rules using
+a count action with the same counter id on the same port will contribute to
+that counter.
+
+For ports within the same switch domain then the counter id namespace extends
+to all ports within that switch domain.
.. _table_rte_flow_action_count:
.. table:: COUNT
- +---------------+
- | Field |
- +===============+
- | no properties |
- +---------------+
+ +------------+---------------------+
+ | Field | Value |
+ +============+=====================+
+ | ``shared`` | shared counter flag |
+ +------------+---------------------+
+ | ``id`` | counter id |
+ +------------+---------------------+
Query structure to retrieve and reset flow rule counters:
@@ -2282,7 +2297,7 @@ definition.
int
rte_flow_query(uint16_t port_id,
struct rte_flow *flow,
- enum rte_flow_action_type action,
+ const struct rte_flow_action *action,
void *data,
struct rte_flow_error *error);
@@ -2290,7 +2305,7 @@ Arguments:
- ``port_id``: port identifier of Ethernet device.
- ``flow``: flow rule handle to query.
-- ``action``: action type to query.
+- ``action``: action to query, this must match prototype from flow rule.
- ``data``: pointer to storage for the associated query data type.
- ``error``: perform verbose error reporting if not NULL. PMDs initialize
this structure in case of error only.
@@ -152,6 +152,7 @@ bond_flow_flush(struct rte_eth_dev *dev, struct rte_flow_error *err)
static int
bond_flow_query_count(struct rte_eth_dev *dev, struct rte_flow *flow,
+ const struct rte_flow_action *action,
struct rte_flow_query_count *count,
struct rte_flow_error *err)
{
@@ -165,7 +166,7 @@ bond_flow_query_count(struct rte_eth_dev *dev, struct rte_flow *flow,
rte_memcpy(&slave_count, count, sizeof(slave_count));
for (i = 0; i < internals->slave_count; i++) {
ret = rte_flow_query(internals->slaves[i].port_id,
- flow->flows[i], RTE_FLOW_ACTION_TYPE_COUNT,
+ flow->flows[i], action,
&slave_count, err);
if (unlikely(ret != 0)) {
RTE_BOND_LOG(ERR, "Failed to query flow on"
@@ -182,12 +183,12 @@ bond_flow_query_count(struct rte_eth_dev *dev, struct rte_flow *flow,
static int
bond_flow_query(struct rte_eth_dev *dev, struct rte_flow *flow,
- enum rte_flow_action_type type, void *arg,
+ const struct rte_flow_action *action, void *arg,
struct rte_flow_error *err)
{
- switch (type) {
+ switch (action->type) {
case RTE_FLOW_ACTION_TYPE_COUNT:
- return bond_flow_query_count(dev, flow, arg, err);
+ return bond_flow_query_count(dev, flow, action, arg, err);
default:
return rte_flow_error_set(err, ENOTSUP,
RTE_FLOW_ERROR_TYPE_ACTION, arg,
@@ -174,7 +174,7 @@ fs_flow_flush(struct rte_eth_dev *dev,
static int
fs_flow_query(struct rte_eth_dev *dev,
struct rte_flow *flow,
- enum rte_flow_action_type type,
+ const struct rte_flow_action *action,
void *arg,
struct rte_flow_error *error)
{
@@ -185,7 +185,7 @@ fs_flow_query(struct rte_eth_dev *dev,
if (sdev != NULL) {
int ret = rte_flow_query(PORT_ID(sdev),
flow->flows[SUB_ID(sdev)],
- type, arg, error);
+ action, arg, error);
if ((ret = fs_err(sdev, ret))) {
fs_unlock(dev, 0);
@@ -233,7 +233,7 @@ rte_flow_flush(uint16_t port_id,
int
rte_flow_query(uint16_t port_id,
struct rte_flow *flow,
- enum rte_flow_action_type action,
+ const struct rte_flow_action *action,
void *data,
struct rte_flow_error *error)
{
@@ -1314,7 +1314,7 @@ enum rte_flow_action_type {
* These counters can be retrieved and reset through rte_flow_query(),
* see struct rte_flow_query_count.
*
- * No associated configuration structure.
+ * See struct rte_flow_action_count.
*/
RTE_FLOW_ACTION_TYPE_COUNT,
@@ -1546,6 +1546,38 @@ 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_COUNT
+ *
+ * Adds a counter action to a matched flow.
+ *
+ * If more than one count action is specified in a single flow rule, then each
+ * action must specify a unique id.
+ *
+ * Counters can be retrieved and reset through ``rte_flow_query()``, see
+ * ``struct rte_flow_query_count``.
+ *
+ * The shared flag indicates whether the counter is unique to the flow rule the
+ * action is specified with, or whether it is a shared counter.
+ *
+ * For a count action with the shared flag set, then then a global device
+ * namespace is assumed for the counter id, so that any matched flow rules using
+ * a count action with the same counter id on the same port will contribute to
+ * that counter.
+ *
+ * For ports within the same switch domain then the counter id namespace extends
+ * to all ports within that switch domain.
+ */
+struct rte_flow_action_count {
+ uint32_t shared:1; /**< Share counter ID with other flow rules. */
+ uint32_t reserved:31; /**< Reserved, must be zero. */
+ uint32_t id; /**< Counter ID. */
+};
+
/**
* RTE_FLOW_ACTION_TYPE_COUNT (query)
*
@@ -2044,7 +2076,7 @@ rte_flow_flush(uint16_t port_id,
* @param flow
* Flow rule handle to query.
* @param action
- * Action type to query.
+ * Action definition as defined in original flow rule.
* @param[in, out] data
* Pointer to storage for the associated query data type.
* @param[out] error
@@ -2057,7 +2089,7 @@ rte_flow_flush(uint16_t port_id,
int
rte_flow_query(uint16_t port_id,
struct rte_flow *flow,
- enum rte_flow_action_type action,
+ const struct rte_flow_action *action,
void *data,
struct rte_flow_error *error);
@@ -88,7 +88,7 @@ struct rte_flow_ops {
int (*query)
(struct rte_eth_dev *,
struct rte_flow *,
- enum rte_flow_action_type,
+ const struct rte_flow_action *,
void *,
struct rte_flow_error *);
/** See rte_flow_isolate(). */