patches for DPDK stable branches
 help / color / mirror / Atom feed
From: "Ding, Xuan" <xuan.ding@intel.com>
To: Maxime Coquelin <maxime.coquelin@redhat.com>,
	"Xia, Chenbo" <chenbo.xia@intel.com>
Cc: "dev@dpdk.org" <dev@dpdk.org>, "Hu, Jiayu" <jiayu.hu@intel.com>,
	"Wang, Yinan" <yinan.wang@intel.com>,
	"stable@dpdk.org" <stable@dpdk.org>,
	"Liu, Yong" <yong.liu@intel.com>
Subject: Re: [dpdk-stable] [PATCH] net/virtio: fix indirect descriptors reconnection
Date: Thu, 23 Sep 2021 09:21:29 +0000
Message-ID: <BN9PR11MB5513D37BB07B74C71F81EEEDE7A39@BN9PR11MB5513.namprd11.prod.outlook.com> (raw)
In-Reply-To: <BN9PR11MB5513A5DB06A53C0735689E78E7A39@BN9PR11MB5513.namprd11.prod.outlook.com>

Hi Maxime,

> -----Original Message-----
> From: Ding, Xuan
> Sent: Thursday, September 23, 2021 4:36 PM
> To: Maxime Coquelin <maxime.coquelin@redhat.com>; Xia, Chenbo
> <Chenbo.Xia@intel.com>
> Cc: dev@dpdk.org; Hu, Jiayu <Jiayu.Hu@intel.com>; Wang, Yinan
> <yinan.wang@intel.com>; stable@dpdk.org; Liu, Yong <yong.liu@intel.com>
> Subject: RE: [PATCH] net/virtio: fix indirect descriptors reconnection
> 
> Hi Maxime,
> 
> > -----Original Message-----
> > From: Maxime Coquelin <maxime.coquelin@redhat.com>
> > Sent: Thursday, September 23, 2021 4:16 PM
> > To: Ding, Xuan <xuan.ding@intel.com>; Xia, Chenbo <chenbo.xia@intel.com>
> > Cc: dev@dpdk.org; Hu, Jiayu <jiayu.hu@intel.com>; Wang, Yinan
> > <yinan.wang@intel.com>; stable@dpdk.org; Liu, Yong <yong.liu@intel.com>
> > Subject: Re: [PATCH] net/virtio: fix indirect descriptors reconnection
> >
> > Hi Xuan,
> >
> > On 8/19/21 07:35, Xuan Ding wrote:
> > > Since packed indirect descriptors are added and initialized when
> > > initializing vring, the reconnection path also needs to be considered.
> > >
> > > Fixes: 381f39ebb78a ("net/virtio: fix packed ring indirect descricptors setup")
> > > Cc: stable@dpdk.org
> > > Cc: yong.liu@intel.com
> > >
> > > Signed-off-by: Xuan Ding <xuan.ding@intel.com>
> > > ---
> > >   drivers/net/virtio/virtqueue.c | 14 ++++++++++++++
> > >   1 file changed, 14 insertions(+)
> > >
> > > diff --git a/drivers/net/virtio/virtqueue.c b/drivers/net/virtio/virtqueue.c
> > > index 1f9af3c31b..47229f35c4 100644
> > > --- a/drivers/net/virtio/virtqueue.c
> > > +++ b/drivers/net/virtio/virtqueue.c
> > > @@ -208,6 +208,20 @@ virtqueue_txvq_reset_packed(struct virtqueue *vq)
> > >   			rte_pktmbuf_free(dxp->cookie);
> > >   			dxp->cookie = NULL;
> > >   		}
> >
> > It makes me think we might save quite some bytes by not allocating
> > indirect descriptors when feature is not negotiated, but it might have
> > a cost in term of performance.
> 
> Do you mean we allocate the indirect descriptors after checking the indirect
> feature in virtio TX queue,
> instead of allocating it at initialization?
> If so, that makes sense, and hence we don't to initialize it in two places.

Sorry for replying myself, as for performance, I can have a try and test the
impact on performance.

Thanks,
Xuan

> 
> >
> > > +
> > > +		struct virtio_tx_region *txr;
> >
> > Don't mix declarations within code.
> >
> > > +		txr = txvq->virtio_net_hdr_mz->addr;
> > > +		/* first indirect descriptor is always the tx header */
> > > +		struct vring_packed_desc *start_dp =
> > > +			txr[desc_idx].tx_packed_indir;
> >
> > Ditto.
> 
> Thanks, will update in next version.
> 
> Regards,
> Xuan
> 
> >
> > > +		vring_desc_init_indirect_packed(start_dp,
> > > +		      RTE_DIM(txr[desc_idx].tx_packed_indir));
> > > +		start_dp->addr = txvq->virtio_net_hdr_mem
> > > +			+ desc_idx * sizeof(*txr)
> > > +			+ offsetof(struct virtio_tx_region,
> > > +				   tx_hdr);
> > > +		start_dp->len = vq->hw->vtnet_hdr_size;
> > > +
> > >   	}
> > >
> > >   	vring_desc_init_packed(vq, size);
> > >
> >
> > Thanks,
> > Maxime


  reply	other threads:[~2021-09-23  9:21 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-08-19  5:35 Xuan Ding
2021-09-23  8:15 ` Maxime Coquelin
2021-09-23  8:35   ` Ding, Xuan
2021-09-23  9:21     ` Ding, Xuan [this message]
2021-10-12  8:48       ` Maxime Coquelin
2021-10-13  0:28 ` Wang, Yinan
2021-10-13  1:36 ` [dpdk-stable] [PATCH v2] " Xuan Ding
2021-10-14  7:59   ` Maxime Coquelin
2021-10-21 12:32   ` Maxime Coquelin

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=BN9PR11MB5513D37BB07B74C71F81EEEDE7A39@BN9PR11MB5513.namprd11.prod.outlook.com \
    --to=xuan.ding@intel.com \
    --cc=chenbo.xia@intel.com \
    --cc=dev@dpdk.org \
    --cc=jiayu.hu@intel.com \
    --cc=maxime.coquelin@redhat.com \
    --cc=stable@dpdk.org \
    --cc=yinan.wang@intel.com \
    --cc=yong.liu@intel.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

patches for DPDK stable branches

This inbox may be cloned and mirrored by anyone:

	git clone --mirror http://inbox.dpdk.org/stable/0 stable/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 stable stable/ http://inbox.dpdk.org/stable \
		stable@dpdk.org
	public-inbox-index stable

Example config snippet for mirrors.
Newsgroup available over NNTP:
	nntp://inbox.dpdk.org/inbox.dpdk.stable


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git