netdev - Re: [PATCH vhost v4 03/10] virtio_ring: packed: structure the indirect desc table

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1711009465.784253-4-xuanzhuo@linux.alibaba.com>
Date: Thu, 21 Mar 2024 16:24:25 +0800
From: Xuan Zhuo <xuanzhuo@...ux.alibaba.com>
To: Jason Wang <jasowang@...hat.com>
Cc: virtualization@...ts.linux.dev,
 "Michael S. Tsirkin" <mst@...hat.com>,
 "David S. Miller" <davem@...emloft.net>,
 Eric Dumazet <edumazet@...gle.com>,
 Jakub Kicinski <kuba@...nel.org>,
 Paolo Abeni <pabeni@...hat.com>,
 netdev@...r.kernel.org
Subject: Re: [PATCH vhost v4 03/10] virtio_ring: packed: structure the indirect desc table

On Thu, 21 Mar 2024 12:47:18 +0800, Jason Wang <jasowang@...hat.com> wrote:
> On Tue, Mar 12, 2024 at 11:36 AM Xuan Zhuo <xuanzhuo@...ux.alibaba.com> wrote:
> >
> > This commit structure the indirect desc table.
> > Then we can get the desc num directly when doing unmap.
> >
> > And save the dma info to the struct, then the indirect
> > will not use the dma fields of the desc_extra. The subsequent
> > commits will make the dma fields are optional. But for
> > the indirect case, we must record the dma info.
> >
> > Signed-off-by: Xuan Zhuo <xuanzhuo@...ux.alibaba.com>
> > ---
> >  drivers/virtio/virtio_ring.c | 66 +++++++++++++++++++++---------------
> >  1 file changed, 38 insertions(+), 28 deletions(-)
> >
> > diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
> > index 0dfbd17e5a87..22a588bba166 100644
> > --- a/drivers/virtio/virtio_ring.c
> > +++ b/drivers/virtio/virtio_ring.c
> > @@ -72,9 +72,16 @@ struct vring_desc_state_split {
> >         struct vring_desc *indir_desc;  /* Indirect descriptor, if any. */
> >  };
> >
> > +struct vring_packed_desc_indir {
> > +       dma_addr_t addr;                /* Descriptor Array DMA addr. */
> > +       u32 len;                        /* Descriptor Array length. */
> > +       u32 num;
> > +       struct vring_packed_desc desc[];
> > +};
> > +
> >  struct vring_desc_state_packed {
> >         void *data;                     /* Data for callback. */
> > -       struct vring_packed_desc *indir_desc; /* Indirect descriptor, if any. */
> > +       struct vring_packed_desc_indir *indir_desc; /* Indirect descriptor, if any. */
>
> Maybe it's better just to have a vring_desc_extra here.

Do you mean replacing vring_packed_desc_indir by vring_desc_extra?

I am ok for that. But vring_desc_extra has two extra items:

	u16 flags;			/* Descriptor flags. */
	u16 next;			/* The next desc state in a list. */

vring_packed_desc_indir has "desc". I think that is more convenient.

So, I think vring_packed_desc_indir is appropriate.
Or I missed something.

Thanks.

>
> Thanks
>