DPDK patches and discussions
 help / color / mirror / Atom feed
From: "McDaniel, Timothy" <timothy.mcdaniel@intel.com>
To: "dev@dpdk.org" <dev@dpdk.org>
Subject: Re: [dpdk-dev] dev Digest, Vol 318, Issue 122
Date: Tue, 22 Sep 2020 15:55:04 +0000	[thread overview]
Message-ID: <SN6PR11MB310360015CF338ED3066CEAE9E3B0@SN6PR11MB3103.namprd11.prod.outlook.com> (raw)
In-Reply-To: <mailman.8199.1600789484.18586.dev@dpdk.org>

Thanks for the heads up

Tim

> -----Original Message-----
> From: dev <dev-bounces@dpdk.org> On Behalf Of dev-request@dpdk.org
> Sent: Tuesday, September 22, 2020 10:45 AM
> To: dev@dpdk.org
> Subject: dev Digest, Vol 318, Issue 122
> 
> Send dev mailing list submissions to
> 	dev@dpdk.org
> 
> To subscribe or unsubscribe via the World Wide Web, visit
> 	https://mails.dpdk.org/listinfo/dev
> or, via email, send a message with subject or body 'help' to
> 	dev-request@dpdk.org
> 
> You can reach the person managing the list at
> 	dev-owner@dpdk.org
> 
> When replying, please edit your Subject line so it is more specific
> than "Re: Contents of dev digest..."
> 
> 
> Today's Topics:
> 
>    1. Re: [PATCH v3 1/6] app/testpmd: fix missing verification of
>       port id (Ferruh Yigit)
>    2. Re: [PATCH v3 3/6] app/testpmd: remove restriction on txpkts
>       set (Ferruh Yigit)
>    3. Re: [PATCH v3 5/6] app/testpmd: fix valid desc id check
>       (Ferruh Yigit)
>    4. Re: [RFC v2 1/1] app/testpmd: distinguish ICMP identifier
>       fields in packet (Ferruh Yigit)
>    5. Re: [PATCH v3] app/testpmd: fix the default RSS key
>       configuration (Phil Yang)
> 
> 
> ----------------------------------------------------------------------
> 
> Message: 1
> Date: Tue, 22 Sep 2020 15:49:36 +0100
> From: Ferruh Yigit <ferruh.yigit@intel.com>
> To: "Wei Hu (Xavier)" <xavier_huwei@163.com>, dev@dpdk.org
> Cc: xavier.huwei@huawei.com
> Subject: Re: [dpdk-dev] [PATCH v3 1/6] app/testpmd: fix missing
> 	verification of port id
> Message-ID: <831d7d94-ce37-9ce4-16d1-e2de6f1162c3@intel.com>
> Content-Type: text/plain; charset=utf-8; format=flowed
> 
> On 9/19/2020 11:47 AM, Wei Hu (Xavier) wrote:
> > From: Chengchang Tang <tangchengchang@huawei.com>
> >
> > To set Tx vlan offloads, it is required to stop port firstly. But before
> > checking whether the port is stopped, the port id entered by the user
> > is not checked for validity. When the port id is illegal, it would lead
> > to a segmentation fault since it attempts to access a member of
> > non-existent port.
> >
> > This patch adds verification of port id in tx vlan offloads and remove
> > duplicated check.
> >
> > Fixes: 597f9fafe13b ("app/testpmd: convert to new Tx offloads API")
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Chengchang Tang <tangchengchang@huawei.com>
> > Signed-off-by: Wei Hu (Xavier) <xavier.huwei@huawei.com>
> 
> Reviewed-by: Ferruh Yigit <ferruh.yigit@intel.com>
> 
> 
> 
> ------------------------------
> 
> Message: 2
> Date: Tue, 22 Sep 2020 15:51:20 +0100
> From: Ferruh Yigit <ferruh.yigit@intel.com>
> To: "Wei Hu (Xavier)" <xavier_huwei@163.com>, dev@dpdk.org
> Cc: xavier.huwei@huawei.com
> Subject: Re: [dpdk-dev] [PATCH v3 3/6] app/testpmd: remove restriction
> 	on txpkts set
> Message-ID: <72499939-f3c1-1caa-cded-b46b489c1863@intel.com>
> Content-Type: text/plain; charset=utf-8; format=flowed
> 
> On 9/19/2020 11:47 AM, Wei Hu (Xavier) wrote:
> > From: Chengchang Tang <tangchengchang@huawei.com>
> >
> > Currently, if nb_txd is not set, the txpkts is not allowed to be set
> > because the nb_txd is used to avoid the numer of segments exceed the Tx
> > ring size and the default value of nb_txd is 0. And there is a bug that
> > nb_txd is the global configuration for Tx ring size and the ring size
> > could be changed by some command per queue. So these valid check is
> > unreliable and introduced unnecessary constraints.
> >
> > This patch adds a valid check function to use the real Tx ring size to
> > check the validity of txpkts.
> >
> > Fixes: af75078fece3 ("first public release")
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Chengchang Tang <tangchengchang@huawei.com>
> > Signed-off-by: Wei Hu (Xavier) <xavier.huwei@huawei.com>
> > ---
> >   app/test-pmd/config.c | 42 ++++++++++++++++++++++++++++++++++++++---
> -
> >   1 file changed, 38 insertions(+), 4 deletions(-)
> >
> > diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
> > index 4e33208..882de2d 100644
> > --- a/app/test-pmd/config.c
> > +++ b/app/test-pmd/config.c
> > @@ -2984,17 +2984,51 @@ show_tx_pkt_segments(void)
> >   	printf("Split packet: %s\n", split);
> >   }
> >
> > +static bool
> > +nb_segs_is_invalid(unsigned int nb_segs)
> > +{
> > +	uint16_t port_id;
> > +
> > +	RTE_ETH_FOREACH_DEV(port_id) {
> > +		struct rte_port *port = &ports[port_id];
> > +		uint16_t ring_size;
> > +		uint16_t queue_id;
> > +
> > +		/*
> > +		 * When configure the txq by rte_eth_tx_queue_setup with
> > +		 * nb_tx_desc being 0, it will use a default value provided by
> > +		 * PMDs to setup this txq. If the default value is 0, it will
> > +		 * use the RTE_ETH_DEV_FALLBACK_TX_RINGSIZE to setup this
> txq.
> > +		 */
> > +		for (queue_id = 0; queue_id < nb_txq; queue_id++) {
> > +			if (port->nb_tx_desc[queue_id])
> > +				ring_size = port->nb_tx_desc[queue_id];
> > +			else if (port->dev_info.default_txportconf.ring_size)
> > +				ring_size =
> > +				    port->dev_info.default_txportconf.ring_size;
> > +			else
> > +				ring_size =
> RTE_ETH_DEV_FALLBACK_TX_RINGSIZE;
> > +
> > +			if (ring_size < nb_segs) {
> > +				printf("nb segments per TX packets=%u >= TX "
> > +				       "queue(%u) ring_size=%u - ignored\n",
> > +					nb_segs, queue_id, ring_size);
> > +				return true;
> > +			}
> > +		}
> 
> What do you think calling 'rte_eth_rx_queue_info_get()' &
> 'rte_eth_tx_queue_info_get()' to get the 'nb_desc'?
> 
> 
> ------------------------------
> 
> Message: 3
> Date: Tue, 22 Sep 2020 15:53:04 +0100
> From: Ferruh Yigit <ferruh.yigit@intel.com>
> To: "Wei Hu (Xavier)" <xavier_huwei@163.com>, dev@dpdk.org
> Cc: xavier.huwei@huawei.com
> Subject: Re: [dpdk-dev] [PATCH v3 5/6] app/testpmd: fix valid desc id
> 	check
> Message-ID: <6e9c3318-1d89-3a50-9540-dbb80893ad8f@intel.com>
> Content-Type: text/plain; charset=utf-8; format=flowed
> 
> On 9/19/2020 11:47 AM, Wei Hu (Xavier) wrote:
> > From: Chengchang Tang <tangchengchang@huawei.com>
> >
> > The number of desc is a per queue configuration. But in the check function,
> > nb_txd & nb_rxd are used to check whether the desc_id is valid. nb_txd &
> > nb_rxd are the global configuration of number of desc. If the queue
> > configuration is changed by cmdline liks: "port config xx txq xx ring_size
> > xxx", the real value will be changed.
> >
> > This patch use the real value to check whether the desc_id is valid. And if
> > these are not configured by user. It will use the default value to check
> > it, since the rte_eth_rx_queue_setup & rte_eth_tx_queue_setup will use a
> > default value to confiure the queue if nb_rx_desc or nb_tx_desc is zero.
> >
> > Fixes: af75078fece3 ("first public release")
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Chengchang Tang <tangchengchang@huawei.com>
> > Signed-off-by: Wei Hu (Xavier) <xavier.huwei@huawei.com>
> > ---
> >   app/test-pmd/config.c | 53
> +++++++++++++++++++++++++++++++++++++++++----------
> >   1 file changed, 43 insertions(+), 10 deletions(-)
> >
> > diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
> > index 882de2d..b7851c7 100644
> > --- a/app/test-pmd/config.c
> > +++ b/app/test-pmd/config.c
> > @@ -1891,22 +1891,55 @@ tx_queue_id_is_invalid(queueid_t txq_id)
> >   }
> >
> >   static int
> > -rx_desc_id_is_invalid(uint16_t rxdesc_id)
> > +rx_desc_id_is_invalid(portid_t port_id, queueid_t rxq_id, uint16_t rxdesc_id)
> >   {
> > -	if (rxdesc_id < nb_rxd)
> > +	struct rte_port *port = &ports[port_id];
> > +	uint16_t ring_size;
> > +
> > +	/*
> > +	 * When configure the rxq by rte_eth_rx_queue_setup with nb_rx_desc
> > +	 * being 0, it will use a default value provided by PMDs to setup this
> > +	 * rxq. If the default value is 0, it will use the
> > +	 * RTE_ETH_DEV_FALLBACK_RX_RINGSIZE to setup this rxq.
> > +	 */
> > +	if (port->nb_rx_desc[rxq_id])
> > +		ring_size = port->nb_rx_desc[rxq_id];
> > +	else if (port->dev_info.default_rxportconf.ring_size)
> > +		ring_size = port->dev_info.default_rxportconf.ring_size;
> > +	else
> > +		ring_size =  RTE_ETH_DEV_FALLBACK_RX_RINGSIZE;
> > +
> > +	if (rxdesc_id < ring_size)
> >   		return 0;
> > -	printf("Invalid RX descriptor %d (must be < nb_rxd=%d)\n",
> > -	       rxdesc_id, nb_rxd);
> > +	printf("Invalid RX descriptor %d (must be < ring_size=%d)\n",
> > +	       rxdesc_id, ring_size);
> >   	return 1;
> 
> +1 to fix, but similar comment as previous patch, can
> 'rte_eth_rx_queue_info_get()' be used to detect the 'ring_size'?
> 
> 
> ------------------------------
> 
> Message: 4
> Date: Tue, 22 Sep 2020 16:38:13 +0100
> From: Ferruh Yigit <ferruh.yigit@intel.com>
> To: Li Zhang <lizh@nvidia.com>, dekelp@nvidia.com, orika@nvidia.com,
> 	viacheslavo@nvidia.com, matan@nvidia.com
> Cc: dev@dpdk.org, thomas@monjalon.net, rasland@nvidia.com
> Subject: Re: [dpdk-dev] [RFC v2 1/1] app/testpmd: distinguish ICMP
> 	identifier fields in packet
> Message-ID: <3def4458-0cb1-835e-1006-463ce4878361@intel.com>
> Content-Type: text/plain; charset=utf-8; format=flowed
> 
> On 9/9/2020 4:34 AM, Li Zhang wrote:
> > From: lizh <lizh@nvidia.com>
> >
> > Ability to distinguish ICMP identifier fields in packets.
> > Dstinguish ICMP sequence number field too.
> > Already supports ICMP code and type fields in current version.
> > Existing fields in ICMP header contain the required information.
> > ICMP header already is supported and no code change in RTE FLOW.
> > Extend testpmd CLI to include the fields of ident and sequence number.
> > One example:
> > flow create 0 ingress pattern eth / ipv4 /
> >   icmp code is 1 ident is 5 seq is 6 /
> >   end actions count / queue index 0 / end
> > The ICMP packet with code 1, identifier 5 and
> > sequence number 6 will be matched.
> > It will implement action counter and forward to queue 0.
> >
> 
> Overall looks good. @Ori, any objection?
> 
> @Li, is there any PMD implementation planned to use these icmp fields?
> 
> > Signed-off-by: Li Zhang <lizh@nvidia.com>
> > ---
> >   app/test-pmd/cmdline_flow.c | 18 ++++++++++++++++++
> >   1 file changed, 18 insertions(+)
> >
> > diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
> > index 6263d307ed..6e04d538ea 100644
> > --- a/app/test-pmd/cmdline_flow.c
> > +++ b/app/test-pmd/cmdline_flow.c
> > @@ -143,6 +143,8 @@ enum index {
> >   	ITEM_ICMP,
> >   	ITEM_ICMP_TYPE,
> >   	ITEM_ICMP_CODE,
> > +	ITEM_ICMP_IDENT,
> > +	ITEM_ICMP_SEQ,
> >   	ITEM_UDP,
> >   	ITEM_UDP_SRC,
> >   	ITEM_UDP_DST,
> > @@ -893,6 +895,8 @@ static const enum index item_ipv6[] = {
> >   static const enum index item_icmp[] = {
> >   	ITEM_ICMP_TYPE,
> >   	ITEM_ICMP_CODE,
> > +	ITEM_ICMP_IDENT,
> > +	ITEM_ICMP_SEQ,
> >   	ITEM_NEXT,
> >   	ZERO,
> >   };
> > @@ -2193,6 +2197,20 @@ static const struct token token_list[] = {
> >   		.args = ARGS(ARGS_ENTRY_HTON(struct rte_flow_item_icmp,
> >   					     hdr.icmp_code)),
> >   	},
> > +	[ITEM_ICMP_IDENT] = {
> > +		.name = "ident",
> > +		.help = "ICMP packet identifier",
> > +		.next = NEXT(item_icmp, NEXT_ENTRY(UNSIGNED),
> item_param),
> > +		.args = ARGS(ARGS_ENTRY_HTON(struct rte_flow_item_icmp,
> > +					     hdr.icmp_ident)),
> > +	},
> > +	[ITEM_ICMP_SEQ] = {
> > +		.name = "seq",
> > +		.help = "ICMP packet sequence number",
> > +		.next = NEXT(item_icmp, NEXT_ENTRY(UNSIGNED),
> item_param),
> > +		.args = ARGS(ARGS_ENTRY_HTON(struct rte_flow_item_icmp,
> > +					     hdr.icmp_seq_nb)),
> > +	},
> >   	[ITEM_UDP] = {
> >   		.name = "udp",
> >   		.help = "match UDP header",
> >
> 
> 
> 
> ------------------------------
> 
> Message: 5
> Date: Tue, 22 Sep 2020 15:44:29 +0000
> From: Phil Yang <Phil.Yang@arm.com>
> To: oulijun <oulijun@huawei.com>, "wenzhuo.lu@intel.com"
> 	<wenzhuo.lu@intel.com>, "beilei.xing@intel.com"
> 	<beilei.xing@intel.com>, "adrien.mazarguil@6wind.com"
> 	<adrien.mazarguil@6wind.com>, "ferruh.yigit@intel.com"
> 	<ferruh.yigit@intel.com>
> Cc: "dev@dpdk.org" <dev@dpdk.org>, "linuxarm@huawei.com"
> 	<linuxarm@huawei.com>, nd <nd@arm.com>, nd <nd@arm.com>
> Subject: Re: [dpdk-dev] [PATCH v3] app/testpmd: fix the default RSS
> 	key configuration
> Message-ID:
> 	<DB7PR08MB3865551BFBF69EEE1A690134E93B0@DB7PR08MB3865.e
> urprd08.prod.outlook.com>
> 
> Content-Type: text/plain; charset="us-ascii"
> 
> dev <dev-bounces@dpdk.org> On Behalf Of Lijun Ou writes:
> 
> 
> > >> Subject: [dpdk-dev] [PATCH v3] app/testpmd: fix the default RSS key
> > >> configuration
> > >
> > > Hi Lijun,
> > >
> > > Please fix the coding style issues.
> > >
> > > "Must be a reply to the first patch (--in-reply-to)."
> > >
> > >
> > >>
> > >> When a user runs a flow create cmd to configure an RSS rule
> > >> with specifying the empty rss actions in testpmd, this mean
> > >                                                                                                      ^^^ means
> > >> that the flow gets the default RSS functions from the valid
> > >> NIC default RSS hash key. However, the testpmd is not set
> > >                                                                                            ^^^ is set xxx incorrectly
> > >> the default RSS key incorrectly when RSS key is not specified
> > >> in flow create cmd.
> > >
> > > Use the NIC valid default RSS key instead of the testpmd dummy RSS key in
> > the flow configuration when the RSS key is not specified in the flow rule. If
> > the NIC RSS key is invalid, it will use testpmd dummy RSS key as the default
> > key.
> > >
> > > Is that good to put it in this way? Because I think it is not a bug, your patch
> > offers an approach to update the default testpmd RSS key.
> > >
> > Do you have any better advice?  or don't use my approach? I think the
> 
> 
> No, I think you misunderstood me. I agree with your proposal and your patch
> looks good to me.
> My suggestion is to reword the commit message to highlight that you mare
> making testpmd use the valid NIC RSS key as the default flow RSS key in this
> patch.
> In my perspective, if you don't specify any RSS key in your flow rule, it should
> allow any available RSS key work as the default key.
> So use a dummy RSS key is correct as well.
> 
> 
> > previous methods are easy to misunderstand.Can we use the NUL KEY
> > solution and fix the problem that the length is 0 and the RSS key is not
> > NULL ?
> >
> > Thanks
> > Lijun Ou
> > > I would also suggest making the commit message shorter and move the
> > test result into the cover letter.
> > > Because the checkepatch tool is not happy with the hex dumped below.
> > > http://mails.dpdk.org/archives/test-report/2020-September/151395.html
> > >
> > >
> > > Thanks,
> > > Phil
> > >
> > >> the cmdline as flows:
> > >> 1. first, startup testpmd:
> > >> testpmd> show port 0 rss-hash key
> > >> RSS functions:
> > >>   all ipv4-frag ipv4-other ipv6-frag ipv6-other ip
> > >> RSS key:
> > >>
> > 6D5A56DA255B0EC24167253D43A38FB0D0CA2BCBAE7B30B477CB2DA38030F
> > >> 20C6A42B73BBEAC01FA
> > >>
> > >> 2. create a rss rule
> > >> testpmd> flow create 0 ingress pattern eth / ipv4 / udp / end actions rss \
> > >> types ipv4-udp end queues end / end
> > >>
> > >> 3. show rss-hash key
> > >> testpmd> show port 0 rss-hash key
> > >> RSS functions:
> > >>   all ipv4-udp udp
> > >> RSS key:
> > >>
> >
> 74657374706D6427732064656661756C74205253532068617368206B65792C20
> 6F
> > >> 76657272696465
> > >>
> > >> Now, it uses rte_eth_dev_rss_hash_conf_get to correctly the
> > >> default rss key. the cmdline and result as flows:
> > >> testpmd> show port 0 rss-hash key
> > >> RSS functions:
> > >>   all ipv4-frag ipv4-other ipv6-frag ipv6-other ip
> > >> RSS key:
> > >>
> >
> 6D5A56DA255B0EC24167253D43A38FB0D0CA2BCBAE7B30B477CB2DA38030F2
> > >> 0C
> > >> 6A42B73BBEAC01FA
> > >> testpmd> flow create 0 ingress pattern eth / ipv4 / udp / end actions rss \
> > >> types ipv4-udp end queues end / end
> > >> testpmd> show port 0 rss-hash key
> > >> RSS functions:
> > >>   all ipv4-udp udp
> > >> RSS key:
> > >>
> > 6D5A56DA255B0EC24167253D43A38FB0D0CA2BCBAE7B30B477CB2DA38030F
> > >> 20C6A42B73BBEAC01FA
> > >>
> > >> Fixes: ac8d22de2394 ("ethdev: flatten RSS configuration in flow API")
> > >> Cc: stable@dpdk.org
> > >>
> > >> Signed-off-by: Lijun Ou <oulijun@huawei.com>
> > >> ---
> > >> V2->V3:
> > >> -fix checkpatch warning.
> > >>
> > >> V1->V2:
> > >> -fix the commit.
> > >> ---
> > >>   app/test-pmd/cmdline_flow.c | 8 ++++++++
> > >>   1 file changed, 8 insertions(+)
> > >>
> > >> diff --git a/app/test-pmd/cmdline_flow.c b/app/test-
> > pmd/cmdline_flow.c
> > >> index 6263d30..e6648da 100644
> > >> --- a/app/test-pmd/cmdline_flow.c
> > >> +++ b/app/test-pmd/cmdline_flow.c
> > >> @@ -4312,6 +4312,7 @@ parse_vc_action_rss(struct context *ctx, const
> > >> struct token *token,
> > >>   		action_rss_data->queue[i] = i;
> > >>   	if (!port_id_is_invalid(ctx->port, DISABLED_WARN) &&
> > >>   	    ctx->port != (portid_t)RTE_PORT_ALL) {
> > >> +		struct rte_eth_rss_conf rss_conf = {0};
> > >>   		struct rte_eth_dev_info info;
> > >>   		int ret2;
> > >>
> > >> @@ -4322,6 +4323,13 @@ parse_vc_action_rss(struct context *ctx, const
> > >> struct token *token,
> > >>   		action_rss_data->conf.key_len =
> > >>   			RTE_MIN(sizeof(action_rss_data->key),
> > >>   				info.hash_key_size);
> > >> +
> > >> +		rss_conf.rss_key_len = sizeof(action_rss_data->key);
> > >> +		rss_conf.rss_key = action_rss_data->key;
> > >> +		ret2 = rte_eth_dev_rss_hash_conf_get(ctx->port,
> > >> &rss_conf);
> > >> +		if (ret2 != 0)
> > >> +			return ret2;
> > >> +		action_rss_data->conf.key = rss_conf.rss_key;
> > >>   	}
> > >>   	action->conf = &action_rss_data->conf;
> > >>   	return ret;
> > >> --
> > >> 2.7.4
> > >
> > > .
> > >
> 
> 
> End of dev Digest, Vol 318, Issue 122
> *************************************

           reply	other threads:[~2020-09-22 15:55 UTC|newest]

Thread overview: expand[flat|nested]  mbox.gz  Atom feed
 [parent not found: <mailman.8199.1600789484.18586.dev@dpdk.org>]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=SN6PR11MB310360015CF338ED3066CEAE9E3B0@SN6PR11MB3103.namprd11.prod.outlook.com \
    --to=timothy.mcdaniel@intel.com \
    --cc=dev@dpdk.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).