Re: [dpdk-dev] [PATCH 1/9] lib: introduce vfio-user library

["Xia, Chenbo" <chenbo.xia@intel.com>]

Hi Stephen,

> -----Original Message-----
> From: Stephen Hemminger <stephen@networkplumber.org>
> Sent: Saturday, December 19, 2020 1:18 AM
> To: Xia, Chenbo <chenbo.xia@intel.com>
> Cc: dev@dpdk.org; thomas@monjalon.net; david.marchand@redhat.com; Liang,
> Cunming <cunming.liang@intel.com>; Lu, Xiuchun <xiuchun.lu@intel.com>; Li,
> Miao <miao.li@intel.com>; Wu, Jingjing <jingjing.wu@intel.com>
> Subject: Re: [PATCH 1/9] lib: introduce vfio-user library
> 
> On Fri, 18 Dec 2020 15:38:43 +0800
> Chenbo Xia <chenbo.xia@intel.com> wrote:
> 
> > +typedef struct vfio_user_msg {
> > +	uint16_t msg_id;
> > +	uint16_t cmd;
> > +	uint32_t size;
> > +#define VFIO_USER_TYPE_CMD	(0x0)		/* Message type is COMMAND */
> > +#define VFIO_USER_TYPE_REPLY	(0x1 << 0)	/* Message type is REPLY
> */
> > +#define VFIO_USER_NEED_NO_RP	(0x1 << 4)	/* Message needs no reply
> */
> > +#define VFIO_USER_ERROR		(0x1 << 5)	/* Reply message has error
> */
> > +	uint32_t flags;
> > +	uint32_t err;				/* Valid in reply, optional */
> > +	union {
> > +		struct vfio_user_version ver;
> > +	} payload;
> > +	int fds[VFIO_USER_MAX_FD];
> > +	int fd_num;
> > +} __attribute((packed)) VFIO_USER_MSG;
> 
> Please don't introduce all capitals typedefs.

OK. Will fix in v2.

> Don't use packed,  it generates slower code. Better to use tools
> like pahole and make the layout of the structure naturally packed.

Thanks for the heads up 😊. Will check pahole then.

> Also, if you put fds[] at the end you could use dynamic sized array
> and not be constrained by VFIO_USER_MAX_FD.

I put this constraint just because one vfio-user message in theory will not
send so many fds. And since I am just planning to optimize the message structure,
I will consider this to make it more flexible and reasonable.

> 
> 
> Since this is DPDK library the symbols should all start with rte_

I think structs in this file are not exposed. Do we add rte_ in this case?
If yes, I will fix it in v2 then.

Thanks for all the good catches!
Chenbo