From: Slava Ovsiienko <viacheslavo@nvidia.com>
To: Haifei Luo <haifeil@nvidia.com>, "dev@dpdk.org" <dev@dpdk.org>
Cc: Ori Kam <orika@nvidia.com>, Raslan Darawsheh <rasland@nvidia.com>,
"Xueming(Steven) Li" <xuemingl@nvidia.com>,
Matan Azrad <matan@nvidia.com>,
Shahaf Shuler <shahafs@nvidia.com>
Subject: Re: [dpdk-dev] [PATCH v2 4/5] net/mlx5: add mlx5 APIs for single flow dump feature
Date: Tue, 13 Apr 2021 07:39:54 +0000 [thread overview]
Message-ID: <DM6PR12MB3753700843C56D84256DB1E5DF4F9@DM6PR12MB3753.namprd12.prod.outlook.com> (raw)
In-Reply-To: <MW2PR12MB466798177226B0267C02C306CE4F9@MW2PR12MB4667.namprd12.prod.outlook.com>
> -----Original Message-----
> From: Haifei Luo
> Sent: Tuesday, April 13, 2021 9:29 AM
> To: Slava Ovsiienko <viacheslavo@nvidia.com>; dev@dpdk.org
> Cc: Ori Kam <orika@nvidia.com>; Raslan Darawsheh <rasland@nvidia.com>;
> Xueming(Steven) Li <xuemingl@nvidia.com>; Matan Azrad
> <matan@nvidia.com>; Shahaf Shuler <shahafs@nvidia.com>
> Subject: RE: [PATCH v2 4/5] net/mlx5: add mlx5 APIs for single flow dump
> feature
>
> Hi Slava,
> For #1, The steering tool send messages to DPDK to request dump.
> Server/Client use data structure "struct msghdr"
> to communicate. It has " msg_iov " ," msg_iovlen" and etc.
> In the tool side, Msg_iov is constructed as 1 byte for port_id, 8 bytes for
> flowptr. In DPDK, then we parse the message this way.
Yes, it is clear. In my opinion we should not use byte array and parse
we should present the some structure instead:
struct mlx5_flow_dump_req {
uint8_t port_id;
void *flow_id;
} __rte_packed;
BTW, why port_id is 1 byte? port_id in DPDK is 16-bit value.
If the request dump tool uses somr structure - it should be defined in some
common file. IMO, it is not a good practice to rely on raw byte array layout.
With best regards, Slava
> For #2, I will move them to the beginning.
>
> -----Original Message-----
> From: Slava Ovsiienko <viacheslavo@nvidia.com>
> Sent: Monday, April 12, 2021 3:38 PM
> To: Haifei Luo <haifeil@nvidia.com>; dev@dpdk.org
> Cc: Ori Kam <orika@nvidia.com>; Raslan Darawsheh <rasland@nvidia.com>;
> Xueming(Steven) Li <xuemingl@nvidia.com>; Haifei Luo
> <haifeil@nvidia.com>; Matan Azrad <matan@nvidia.com>; Shahaf Shuler
> <shahafs@nvidia.com>
> Subject: RE: [PATCH v2 4/5] net/mlx5: add mlx5 APIs for single flow dump
> feature
>
> > -----Original Message-----
> > From: Haifei Luo <haifeil@nvidia.com>
> > Sent: Wednesday, April 7, 2021 9:09
> > To: dev@dpdk.org
> > Cc: Ori Kam <orika@nvidia.com>; Slava Ovsiienko
> > <viacheslavo@nvidia.com>; Raslan Darawsheh <rasland@nvidia.com>;
> > Xueming(Steven) Li <xuemingl@nvidia.com>; Haifei Luo
> > <haifeil@nvidia.com>; Matan Azrad <matan@nvidia.com>; Shahaf Shuler
> > <shahafs@nvidia.com>
> > Subject: [PATCH v2 4/5] net/mlx5: add mlx5 APIs for single flow dump
> > feature
> >
> > Modify API mlx5_flow_dev_dump to support the feature.
> > Modify mlx5_socket since one extra arg flow_ptr is added.
> >
> > Signed-off-by: Haifei Luo <haifeil@nvidia.com>
>
> Sorry, this patch is errorneously acked instead of the
> "common/mlx5: add mlx5 APIs for single flow dump feature"
>
> I have comment for this one.
>
> > +#ifndef _GNU_SOURCE
> > +#define _GNU_SOURCE
> > +#endif
> > +
> > #include <sys/types.h>
> > #include <sys/socket.h>
> > #include <sys/un.h>
> > @@ -29,11 +33,15 @@
> > {
> > int conn_sock;
> > int ret;
> > + int j;
> > struct cmsghdr *cmsg = NULL;
> > - int data;
> > + #define LENGTH 9
> > + /* The first byte for port_id and the rest for flowptr. */
> > + int data[LENGTH];
>
> So, we define 36/72 bytes array? And then use each int as byte to save
> flow_idx value?
> I suppose the correct way would be to define the structure of message in
> stead of using ints array, something likle this:
>
> struct mlx5_ipc_msg {
> int status;
> void* flow_idx;
> }
>
> > + /* The first byte in data for port_id and the following 8 for flowptr */
> > + for (j = 1; j < LENGTH; j++)
> > + flow_ptr = (flow_ptr << 8) + data[j];
> If structure is define, there should be:
> flow_ptr = msg->flow_idx
>
> > + if (flow_ptr == 0)
> > + ret = mlx5_flow_dev_dump(dev, NULL, file, NULL);
> > + else
> > + ret = mlx5_flow_dev_dump(dev,
> > + (struct rte_flow *)((uintptr_t)flow_ptr), file, &err);
> > +
>
> > + /*dump one*/
> > + uint32_t handle_idx;
> > + int ret;
> > + struct mlx5_flow_handle *dh;
> > + struct rte_flow *flow = mlx5_ipool_get(priv->sh->ipool
> > + [MLX5_IPOOL_RTE_FLOW], (uintptr_t)(void *)flow_idx);
> > +
> Please, move variable declarations to the routine beginning, to others
>
> With best regards, Slava
next prev parent reply other threads:[~2021-04-13 7:39 UTC|newest]
Thread overview: 53+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-03-09 8:15 [dpdk-dev] [PATCH 0/4] *** Support for one flow dump *** Haifei Luo
2021-03-09 8:15 ` [dpdk-dev] [PATCH 1/4] ethdev: add rte API for single flow dump Haifei Luo
2021-03-10 0:39 ` Stephen Hemminger
2021-03-10 7:05 ` Ori Kam
2021-03-09 8:15 ` [dpdk-dev] [PATCH 2/4] app/testpmd: add CLIs for single flow dump feature Haifei Luo
2021-03-09 8:15 ` [dpdk-dev] [PATCH 3/4] common/mlx5: add mlx5 APIs " Haifei Luo
2021-03-09 8:15 ` [dpdk-dev] [PATCH 4/4] net/mlx5: " Haifei Luo
2021-04-07 6:09 ` [dpdk-dev] [PATCH v2 0/5] support single flow dump Haifei Luo
2021-04-07 6:09 ` [dpdk-dev] [PATCH v2 1/5] ethdev: modify rte API for " Haifei Luo
2021-04-12 19:33 ` Ori Kam
2021-04-13 4:38 ` Ajit Khaparde
2021-04-07 6:09 ` [dpdk-dev] [PATCH v2 2/5] app/testpmd: add CLIs for single flow dump feature Haifei Luo
2021-04-13 16:36 ` Ajit Khaparde
2021-04-07 6:09 ` [dpdk-dev] [PATCH v2 3/5] common/mlx5: add mlx5 APIs " Haifei Luo
2021-04-12 7:32 ` Slava Ovsiienko
2021-04-13 16:44 ` Kinsella, Ray
2021-04-14 2:40 ` Haifei Luo
2021-04-14 6:21 ` David Marchand
2021-04-14 6:23 ` Haifei Luo
2021-04-07 6:09 ` [dpdk-dev] [PATCH v2 4/5] net/mlx5: " Haifei Luo
2021-04-12 7:29 ` Slava Ovsiienko
2021-04-12 7:37 ` Slava Ovsiienko
2021-04-13 1:29 ` Haifei Luo
2021-04-13 7:22 ` Haifei Luo
2021-04-13 7:39 ` Slava Ovsiienko [this message]
2021-04-07 6:09 ` [dpdk-dev] [PATCH v2 5/5] doc: add single flow dump to guides Haifei Luo
2021-04-14 6:13 ` [dpdk-dev] [PATCH v3 0/3] support single flow dump Haifei Luo
2021-04-14 6:13 ` [dpdk-dev] [PATCH v3 1/3] ethdev: modify rte API for " Haifei Luo
2021-04-14 6:13 ` [dpdk-dev] [PATCH v3 2/3] app/testpmd: add CLIs for single flow dump feature Haifei Luo
2021-04-14 6:13 ` [dpdk-dev] [PATCH v3 3/3] doc: add single flow dump to guides Haifei Luo
2021-04-14 8:23 ` Ferruh Yigit
2021-04-14 8:25 ` Haifei Luo
2021-04-14 10:00 ` Haifei Luo
2021-04-14 8:41 ` [dpdk-dev] [PATCH v4 0/3] support single flow dump Haifei Luo
2021-04-14 8:41 ` [dpdk-dev] [PATCH v4 1/3] ethdev: modify rte API for " Haifei Luo
2021-04-14 8:57 ` Thomas Monjalon
2021-04-14 9:10 ` Haifei Luo
2021-04-14 9:01 ` Thomas Monjalon
2021-04-14 9:07 ` Haifei Luo
2021-04-14 9:31 ` Thomas Monjalon
2021-04-14 8:41 ` [dpdk-dev] [PATCH v4 2/3] app/testpmd: add CLIs for single flow dump feature Haifei Luo
2021-04-14 8:41 ` [dpdk-dev] [PATCH v4 3/3] doc: add single flow dump to guides Haifei Luo
2021-04-14 9:15 ` Ferruh Yigit
2021-04-14 9:51 ` [dpdk-dev] [PATCH v5 0/3] support single flow dump Haifei Luo
2021-04-14 9:51 ` [dpdk-dev] [PATCH v5 1/3] ethdev: dump single flow rule Haifei Luo
2021-04-14 10:33 ` Thomas Monjalon
2021-04-14 10:37 ` Haifei Luo
2021-04-14 9:51 ` [dpdk-dev] [PATCH v5 2/3] app/testpmd: add CLIs for single flow dump feature Haifei Luo
2021-04-14 9:51 ` [dpdk-dev] [PATCH v5 3/3] doc: add single flow dump to guides Haifei Luo
2021-04-14 10:19 ` [dpdk-dev] [PATCH v6 0/2] single flow dump Haifei Luo
2021-04-14 10:19 ` [dpdk-dev] [PATCH v6 1/2] ethdev: dump single flow rule Haifei Luo
2021-04-14 10:20 ` [dpdk-dev] [PATCH v6 2/2] app/testpmd: add CLIs for single flow dump feature Haifei Luo
2021-04-14 11:26 ` [dpdk-dev] [PATCH v6 0/2] single flow dump Ferruh Yigit
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=DM6PR12MB3753700843C56D84256DB1E5DF4F9@DM6PR12MB3753.namprd12.prod.outlook.com \
--to=viacheslavo@nvidia.com \
--cc=dev@dpdk.org \
--cc=haifeil@nvidia.com \
--cc=matan@nvidia.com \
--cc=orika@nvidia.com \
--cc=rasland@nvidia.com \
--cc=shahafs@nvidia.com \
--cc=xuemingl@nvidia.com \
/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).