* Re: [dpdk-dev] dev Digest, Vol 318, Issue 122
[not found] <mailman.8199.1600789484.18586.dev@dpdk.org>
@ 2020-09-22 15:55 ` McDaniel, Timothy
0 siblings, 0 replies; only message in thread
From: McDaniel, Timothy @ 2020-09-22 15:55 UTC (permalink / raw)
To: dev
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
> *************************************
^ permalink raw reply [flat|nested] only message in thread