Re: [dpdk-dev] [PATCH v3 09/10] examples/flow_filtering: utilize shared RSS action

[Andrey Vesnovaty <andreyv@nvidia.com>]

Hi Ori,

PSB

Thanks,
Andrey

> -----Original Message-----
> From: Ori Kam <orika@nvidia.com>
> Sent: Sunday, October 4, 2020 2:22 PM
> To: Andrey Vesnovaty <andreyv@nvidia.com>; dev@dpdk.org
> Cc: jer@marvell.com; jerinjacobk@gmail.com; NBU-Contact-Thomas Monjalon
> <thomas@monjalon.net>; ferruh.yigit@intel.com;
> stephen@networkplumber.org; bruce.richardson@intel.com; Slava Ovsiienko
> <viacheslavo@nvidia.com>; andrey.vesnovaty@gmail.com; mdr@ashroe.eu;
> nhorman@tuxdriver.com; ajit.khaparde@broadcom.com;
> samik.gupta@broadcom.com; Andrey Vesnovaty <andreyv@mellanox.com>;
> Marko Kovacevic <marko.kovacevic@intel.com>; Ori Kam
> <orika@mellanox.com>; Radu Nicolau <radu.nicolau@intel.com>; Akhil Goyal
> <akhil.goyal@nxp.com>; Tomasz Kantecki <tomasz.kantecki@intel.com>; Sunil
> Kumar Kori <skori@marvell.com>; Pavan Nikhilesh
> <pbhagavatula@marvell.com>; John McNamara <john.mcnamara@intel.com>
> Subject: RE: [PATCH v3 09/10] examples/flow_filtering: utilize shared RSS action
> 
> Hi Andrey,
> 
> I don't think this change should be part of the series.

Now there is PMD support for shared action & having some example isn't critical
For this patch series. Removing it. 

> After you update this example will only work if the PMD support shared action.
> Not all PMD supports it, and in any case this example is to show basic rules and
> RSS
> your code changes the app that the RSS action can't be checked.

There is a fallback for PMDs without shared action support but I that
agree, it would be better to provide separate example for shared RSS.

> 
> Best,
> Ori
> 
> 
> > -----Original Message-----
> > From: Andrey Vesnovaty <andreyv@nvidia.com>
> > Sent: Sunday, October 4, 2020 1:06 AM
> > Subject: [PATCH v3 09/10] examples/flow_filtering: utilize shared RSS action
> >
> > From: Andrey Vesnovaty <andreyv@mellanox.com>
> >
> > This commit give very first shared RSS action usage example.
> > Queue action used by the flow replaced by shared RSS action
> > having single queue. On each RX burst queue switched 0 <-> 1
> > utilizing rte_flow_shared_action_update() API. User supposed
> > to observe consistent queue switches on each packet burst.
> >
> > Signed-off-by: Andrey Vesnovaty <andreyv@mellanox.com>
> > Signed-off-by: Andrey Vesnovaty <andreyv@nvidia.com>
> > ---
> >  doc/guides/sample_app_ug/flow_filtering.rst | 62 +++++++++++++---
> >  examples/flow_filtering/flow_blocks.c       | 81 +++++++++++++++++----
> >  examples/flow_filtering/main.c              | 13 +++-
> >  3 files changed, 128 insertions(+), 28 deletions(-)
> >
> > diff --git a/doc/guides/sample_app_ug/flow_filtering.rst
> > b/doc/guides/sample_app_ug/flow_filtering.rst
> > index 5e5a6cd8a0..cfe9334717 100644
> > --- a/doc/guides/sample_app_ug/flow_filtering.rst
> > +++ b/doc/guides/sample_app_ug/flow_filtering.rst
> > @@ -106,7 +106,7 @@ following code:
> >  .. code-block:: c
> >
> >     /* create flow for send packet with */
> > -   flow = generate_ipv4_flow(port_id, selected_queue,
> > +   flow = generate_ipv4_flow(port_id, shared_action,
> >                                  SRC_IP, EMPTY_MASK,
> >                                  DEST_IP, FULL_MASK, &error);
> >     if (!flow) {
> > @@ -242,7 +242,7 @@ The Ethernet port is configured with default settings
> > using the
> >     rxq_conf = dev_info.default_rxconf;
> >     rxq_conf.offloads = port_conf.rxmode.offloads;
> >
> > -For this example we are configuring number of rx and tx queues that are
> > connected
> > +For this example we are configuring 2 rx and 2 tx queues that are connected
> >  to a single port.
> >
> >  .. code-block:: c
> > @@ -270,13 +270,22 @@ to a single port.
> >            }
> >     }
> >
> > +Before we create the flow we create shared action in order to send it as
> > +actions argument when creating a flow. The action is single queue RSS action
> > +similar to action queue with the only difference that shared RSS action
> > +provides update capability after action creation.
> > +
> > +.. code-block:: c
> > +
> > +   shared_action = rte_flow_shared_action_create(port_id, &action, &error);
> > +
> >  In the next step we create and apply the flow rule. which is to send packets
> >  with destination ip equals to 192.168.1.1 to queue number 1. The detail
> >  explanation of the ``generate_ipv4_flow()`` appears later in this document:
> >
> >  .. code-block:: c
> >
> > -   flow = generate_ipv4_flow(port_id, selected_queue,
> > +   flow = generate_ipv4_flow(port_id, shared_action,
> >                               SRC_IP, EMPTY_MASK,
> >                               DEST_IP, FULL_MASK, &error);
> >
> > @@ -339,6 +348,21 @@ looks like the following:
> >                                             printf("\n");
> >                                             rte_pktmbuf_free(m);
> >                                     }
> > +                                   if (rss_queue[0] == 0) {
> > +                                           printf(">>> switching queue 0 -> 1\n");
> > +                                           rss_queue[0] = 1;
> > +                                   } else {
> > +                                           printf(">>> switching queue 1 -> 0\n");
> > +                                           rss_queue[0] = 0;
> > +                                   }
> > +                                   ret = rte_flow_shared_action_update
> > +                                           (port_id, shared_action, &action,
> > +                                            &error);
> > +                                   if (ret)
> > +                                           rte_exit(EXIT_FAILURE,
> > +                                                    ":: error: RSS action update "
> > +                                                    "failed: %s\n",
> > +                                                    rte_strerror(-ret));
> >                             }
> >                     }
> >             }
> > @@ -348,6 +372,8 @@ looks like the following:
> >             rte_eth_dev_close(port_id);
> >     }
> >
> > +On each loop eteration Rx queue switched using
> > +``rte_flow_shared_action_update()`` API.
> >  The main work of the application is reading the packets from all
> >  queues and printing for each packet the destination queue:
> >
> > @@ -365,6 +391,21 @@ queues and printing for each packet the destination
> > queue:
> >                               printf(" - queue=0x%x", (unsigned int)i);
> >                               printf("\n");
> >                               rte_pktmbuf_free(m);
> > +                             if (rss_queue[0] == 0) {
> > +                                     printf(">>> switching queue 0 -> 1\n");
> > +                                     rss_queue[0] = 1;
> > +                             } else {
> > +                                     printf(">>> switching queue 1 -> 0\n");
> > +                                     rss_queue[0] = 0;
> > +                             }
> > +                             ret = rte_flow_shared_action_update
> > +                                     (port_id, shared_action, &action,
> > +                                      &error);
> > +                             if (ret)
> > +                                     rte_exit(EXIT_FAILURE,
> > +                                              ":: error: RSS action update "
> > +                                              "failed: %s\n",
> > +                                              rte_strerror(-ret));
> >                          }
> >                  }
> >             }
> > @@ -378,13 +419,15 @@ The forwarding loop can be interrupted and the
> > application closed using
> >  The generate_ipv4_flow function
> >  ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> >
> > +
> >  The generate_ipv4_flow function is responsible for creating the flow rule.
> >  This function is located in the ``flow_blocks.c`` file.
> >
> >  .. code-block:: c
> >
> >     static struct rte_flow *
> > -   generate_ipv4_flow(uint8_t port_id, uint16_t rx_q,
> > +   generate_ipv4_flow(uint8_t port_id,
> > +                   cstructrte_flow_shared_action *shared_action,
> >                     uint32_t src_ip, uint32_t src_mask,
> >                     uint32_t dest_ip, uint32_t dest_mask,
> >                     struct rte_flow_error *error)
> > @@ -393,7 +436,6 @@ This function is located in the ``flow_blocks.c`` file.
> >             struct rte_flow_item pattern[MAX_PATTERN_NUM];
> >             struct rte_flow_action action[MAX_ACTION_NUM];
> >             struct rte_flow *flow = NULL;
> > -           struct rte_flow_action_queue queue = { .index = rx_q };
> >             struct rte_flow_item_ipv4 ip_spec;
> >             struct rte_flow_item_ipv4 ip_mask;
> >
> > @@ -411,8 +453,8 @@ This function is located in the ``flow_blocks.c`` file.
> >              * create the action sequence.
> >              * one action only,  move packet to queue
> >              */
> > -           action[0].type = RTE_FLOW_ACTION_TYPE_QUEUE;
> > -           action[0].conf = &queue;
> > +           action[0].type = RTE_FLOW_ACTION_TYPE_SHARED;
> > +           action[0].conf = shared_action;
> >             action[1].type = RTE_FLOW_ACTION_TYPE_END;
> >
> >             /*
> > @@ -468,12 +510,12 @@ The following part create the flow attributes, in our
> > case ingress.
> >     attr.ingress = 1;
> >
> >  The third part defines the action to be taken when a packet matches
> > -the rule. In this case send the packet to queue.
> > +the rule. In this case send the packet to single RSS queue.
> >
> >  .. code-block:: c
> >
> > -   action[0].type = RTE_FLOW_ACTION_TYPE_QUEUE;
> > -   action[0].conf = &queue;
> > +   action[0].type = RTE_FLOW_ACTION_TYPE_SHARED;
> > +   action[0].conf = shared_action;
> >     action[1].type = RTE_FLOW_ACTION_TYPE_END;
> >
> >  The fourth part is responsible for creating the pattern and is built from
> > diff --git a/examples/flow_filtering/flow_blocks.c
> > b/examples/flow_filtering/flow_blocks.c
> > index 575d792810..ed17ae073e 100644
> > --- a/examples/flow_filtering/flow_blocks.c
> > +++ b/examples/flow_filtering/flow_blocks.c
> > @@ -5,12 +5,30 @@
> >  #define MAX_PATTERN_NUM		3
> >  #define MAX_ACTION_NUM		2
> >
> > -struct rte_flow *
> > -generate_ipv4_flow(uint16_t port_id, uint16_t rx_q,
> > -		uint32_t src_ip, uint32_t src_mask,
> > -		uint32_t dest_ip, uint32_t dest_mask,
> > -		struct rte_flow_error *error);
> > +struct rte_flow_shared_action *shared_action;
> > +uint16_t rss_queue[1] = {0};
> > +
> > +struct rte_flow_action_rss rss_action = {
> > +		.func = RTE_ETH_HASH_FUNCTION_DEFAULT,
> > +		.level = 0,
> > +		.types = 0,
> > +		.key_len = 0,
> > +		.key = NULL,
> > +		.queue = rss_queue,
> > +		.queue_num = 1,
> > +};
> >
> > +struct rte_flow_action flow_action = {
> > +	.type = RTE_FLOW_ACTION_TYPE_RSS,
> > +	.conf = &rss_action,
> > +};
> > +
> > +struct rte_flow *
> > +generate_ipv4_flow(uint16_t port_id,
> > +		   uint32_t src_ip, uint32_t src_mask,
> > +		   uint32_t dest_ip, uint32_t dest_mask,
> > +		   struct rte_flow_error *error);
> > +int update_rss_action(uint16_t port_id, struct rte_flow_error *error);
> >
> >  /**
> >   * create a flow rule that sends packets with matching src and dest ip
> > @@ -18,8 +36,6 @@ generate_ipv4_flow(uint16_t port_id, uint16_t rx_q,
> >   *
> >   * @param port_id
> >   *   The selected port.
> > - * @param rx_q
> > - *   The selected target queue.
> >   * @param src_ip
> >   *   The src ip value to match the input packet.
> >   * @param src_mask
> > @@ -35,16 +51,15 @@ generate_ipv4_flow(uint16_t port_id, uint16_t rx_q,
> >   *   A flow if the rule could be created else return NULL.
> >   */
> >  struct rte_flow *
> > -generate_ipv4_flow(uint16_t port_id, uint16_t rx_q,
> > -		uint32_t src_ip, uint32_t src_mask,
> > -		uint32_t dest_ip, uint32_t dest_mask,
> > -		struct rte_flow_error *error)
> > +generate_ipv4_flow(uint16_t port_id,
> > +		   uint32_t src_ip, uint32_t src_mask,
> > +		   uint32_t dest_ip, uint32_t dest_mask,
> > +		   struct rte_flow_error *error)
> >  {
> >  	struct rte_flow_attr attr;
> >  	struct rte_flow_item pattern[MAX_PATTERN_NUM];
> >  	struct rte_flow_action action[MAX_ACTION_NUM];
> >  	struct rte_flow *flow = NULL;
> > -	struct rte_flow_action_queue queue = { .index = rx_q };
> >  	struct rte_flow_item_ipv4 ip_spec;
> >  	struct rte_flow_item_ipv4 ip_mask;
> >  	int res;
> > @@ -61,10 +76,19 @@ generate_ipv4_flow(uint16_t port_id, uint16_t rx_q,
> >
> >  	/*
> >  	 * create the action sequence.
> > -	 * one action only,  move packet to queue
> > +	 * one action only,  move packet to shared RSS queue
> >  	 */
> > -	action[0].type = RTE_FLOW_ACTION_TYPE_QUEUE;
> > -	action[0].conf = &queue;
> > +	shared_action = rte_flow_shared_action_create(port_id, NULL,
> > +						      &flow_action, error);
> > +	if (shared_action) {
> > +		action[0].conf = shared_action;
> > +		action[0].type = RTE_FLOW_ACTION_TYPE_SHARED;
> > +	} else {
> > +		if (rte_errno != ENOSYS)
> > +			rte_exit(EXIT_FAILURE, ":: action creation failure\n");
> > +		printf("PMD doesn't support shared actions.\n");
> > +		action[0] = flow_action;
> > +	}
> >  	action[1].type = RTE_FLOW_ACTION_TYPE_END;
> >
> >  	/*
> > @@ -98,3 +122,30 @@ generate_ipv4_flow(uint16_t port_id, uint16_t rx_q,
> >
> >  	return flow;
> >  }
> > +
> > +/**
> > + * updates shared RSS action if flow created with shared action
> > + *
> > + * @param port_id
> > + *   The selected port.
> > + * @param[out] error
> > + *   Perform verbose error reporting if not NULL.
> > + *
> > + * @return
> > + *   0 on sccess
> > + */
> > +int
> > +update_rss_action(uint16_t port_id, struct rte_flow_error *error)
> > +{
> > +	if (!shared_action)
> > +		return 0;
> > +	if (rss_queue[0] == 0) {
> > +		printf(">>> switching queue 0 -> 1\n");
> > +		rss_queue[0] = 1;
> > +	} else {
> > +		printf(">>> switching queue 1 -> 0\n");
> > +		rss_queue[0] = 0;
> > +	}
> > +	return rte_flow_shared_action_update(port_id, shared_action,
> > +					     &flow_action, error);
> > +}
> > diff --git a/examples/flow_filtering/main.c b/examples/flow_filtering/main.c
> > index cc9e7e7808..9ef7f099d9 100644
> > --- a/examples/flow_filtering/main.c
> > +++ b/examples/flow_filtering/main.c
> > @@ -32,8 +32,7 @@
> >  static volatile bool force_quit;
> >
> >  static uint16_t port_id;
> > -static uint16_t nr_queues = 5;
> > -static uint8_t selected_queue = 1;
> > +static uint16_t nr_queues = 2;
> >  struct rte_mempool *mbuf_pool;
> >  struct rte_flow *flow;
> >
> > @@ -61,6 +60,7 @@ main_loop(void)
> >  	uint16_t nb_rx;
> >  	uint16_t i;
> >  	uint16_t j;
> > +	int ret;
> >
> >  	while (!force_quit) {
> >  		for (i = 0; i < nr_queues; i++) {
> > @@ -82,6 +82,12 @@ main_loop(void)
> >
> >  					rte_pktmbuf_free(m);
> >  				}
> > +				ret = update_rss_action(port_id, &error);
> > +				if (ret)
> > +					rte_exit(EXIT_FAILURE,
> > +						 ":: error: RSS action update "
> > +						 "failed: %s\n",
> > +						 rte_strerror(-ret));
> >  			}
> >  		}
> >  	}
> > @@ -243,8 +249,9 @@ main(int argc, char **argv)
> >
> >  	init_port();
> >
> > +
> >  	/* create flow for send packet with */
> > -	flow = generate_ipv4_flow(port_id, selected_queue,
> > +	flow = generate_ipv4_flow(port_id,
> >  				SRC_IP, EMPTY_MASK,
> >  				DEST_IP, FULL_MASK, &error);
> >  	if (!flow) {
> > --
> > 2.26.2