Hi Stephen,
> -----Original Message-----
> From: Stephen Hemminger <step...@networkplumber.org>
> Sent: Saturday, December 19, 2020 1:18 AM
> To: Xia, Chenbo <chenbo....@intel.com>
> Cc: dev@dpdk.org; tho...@monjalon.net; david.march...@redhat.com; Liang,
> Cunming <cunming.li...@intel.com>; Lu, Xiuchun <xiuchun...@intel.com>; Li,
> Miao <miao...@intel.com>; Wu, Jingjing <jingjing...@intel.com>
> Subject: Re: [PATCH 1/9] lib: introduce vfio-user library
>
> On Fri, 18 Dec 2020 15:38:43 +0800
> Chenbo Xia <chenbo....@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
