Thanks for the heads up Tim
> -----Original Message----- > From: dev <dev-boun...@dpdk.org> On Behalf Of dev-requ...@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-requ...@dpdk.org > > You can reach the person managing the list at > dev-ow...@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.yi...@intel.com> > To: "Wei Hu (Xavier)" <xavier_hu...@163.com>, dev@dpdk.org > Cc: xavier.hu...@huawei.com > Subject: Re: [dpdk-dev] [PATCH v3 1/6] app/testpmd: fix missing > verification of port id > Message-ID: <831d7d94-ce37-9ce4-16d1-e2de6f116...@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 <tangchengch...@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: sta...@dpdk.org > > > > Signed-off-by: Chengchang Tang <tangchengch...@huawei.com> > > Signed-off-by: Wei Hu (Xavier) <xavier.hu...@huawei.com> > > Reviewed-by: Ferruh Yigit <ferruh.yi...@intel.com> > > > > ------------------------------ > > Message: 2 > Date: Tue, 22 Sep 2020 15:51:20 +0100 > From: Ferruh Yigit <ferruh.yi...@intel.com> > To: "Wei Hu (Xavier)" <xavier_hu...@163.com>, dev@dpdk.org > Cc: xavier.hu...@huawei.com > Subject: Re: [dpdk-dev] [PATCH v3 3/6] app/testpmd: remove restriction > on txpkts set > Message-ID: <72499939-f3c1-1caa-cded-b46b489c1...@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 <tangchengch...@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: sta...@dpdk.org > > > > Signed-off-by: Chengchang Tang <tangchengch...@huawei.com> > > Signed-off-by: Wei Hu (Xavier) <xavier.hu...@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.yi...@intel.com> > To: "Wei Hu (Xavier)" <xavier_hu...@163.com>, dev@dpdk.org > Cc: xavier.hu...@huawei.com > Subject: Re: [dpdk-dev] [PATCH v3 5/6] app/testpmd: fix valid desc id > check > Message-ID: <6e9c3318-1d89-3a50-9540-dbb80893a...@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 <tangchengch...@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: sta...@dpdk.org > > > > Signed-off-by: Chengchang Tang <tangchengch...@huawei.com> > > Signed-off-by: Wei Hu (Xavier) <xavier.hu...@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.yi...@intel.com> > To: Li Zhang <l...@nvidia.com>, dek...@nvidia.com, or...@nvidia.com, > viachesl...@nvidia.com, ma...@nvidia.com > Cc: dev@dpdk.org, tho...@monjalon.net, rasl...@nvidia.com > Subject: Re: [dpdk-dev] [RFC v2 1/1] app/testpmd: distinguish ICMP > identifier fields in packet > Message-ID: <3def4458-0cb1-835e-1006-463ce4878...@intel.com> > Content-Type: text/plain; charset=utf-8; format=flowed > > On 9/9/2020 4:34 AM, Li Zhang wrote: > > From: lizh <l...@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 <l...@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.y...@arm.com> > To: oulijun <ouli...@huawei.com>, "wenzhuo...@intel.com" > <wenzhuo...@intel.com>, "beilei.x...@intel.com" > <beilei.x...@intel.com>, "adrien.mazarg...@6wind.com" > <adrien.mazarg...@6wind.com>, "ferruh.yi...@intel.com" > <ferruh.yi...@intel.com> > Cc: "dev@dpdk.org" <dev@dpdk.org>, "linux...@huawei.com" > <linux...@huawei.com>, nd <n...@arm.com>, nd <n...@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-boun...@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: sta...@dpdk.org > > >> > > >> Signed-off-by: Lijun Ou <ouli...@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 > *************************************
