[dpdk-dev,v5,2/8] virtio: clean up virtio_dev_queue_setup
Commit Message
Abstract vring hdr desc init as an inline method.
Signed-off-by: Huawei Xie <huawei.xie@intel.com>
Signed-off-by: Jianfeng Tan <jianfeng.tan@intel.com>
---
drivers/net/virtio/virtio_ethdev.c | 42 ++++++++++++++++++++++----------------
1 file changed, 24 insertions(+), 18 deletions(-)
Comments
On Mon, May 30, 2016 at 10:55:33AM +0000, Jianfeng Tan wrote:
> Abstract vring hdr desc init as an inline method.
What's this patch for then? In your last version, it will be invoked
twice, but it turned out to be wrong. So, why keeping this change?
I didn't see it improves anything.
--yliu
Hi Yuanhan,
> -----Original Message-----
> From: Yuanhan Liu [mailto:yuanhan.liu@linux.intel.com]
> Sent: Wednesday, June 1, 2016 3:38 PM
> To: Tan, Jianfeng
> Cc: dev@dpdk.org; Xie, Huawei; rich.lane@bigswitch.com; mst@redhat.com;
> nakajima.yoshihiro@lab.ntt.co.jp; p.fedin@samsung.com;
> ann.zhuangyanying@huawei.com; mukawa@igel.co.jp;
> nhorman@tuxdriver.com
> Subject: Re: [PATCH v5 2/8] virtio: clean up virtio_dev_queue_setup
>
> On Mon, May 30, 2016 at 10:55:33AM +0000, Jianfeng Tan wrote:
> > Abstract vring hdr desc init as an inline method.
>
> What's this patch for then? In your last version, it will be invoked
> twice, but it turned out to be wrong. So, why keeping this change?
> I didn't see it improves anything.
>
Yes, for now, only one version is kept because the position to call this function is changed. And I think this segment of code functions as a special purpose, which can be abstracted as a function, make sense?
Thanks,
Jianfeng
On Wed, Jun 01, 2016 at 07:44:33AM +0000, Tan, Jianfeng wrote:
> Hi Yuanhan,
>
> > -----Original Message-----
> > From: Yuanhan Liu [mailto:yuanhan.liu@linux.intel.com]
> > Sent: Wednesday, June 1, 2016 3:38 PM
> > To: Tan, Jianfeng
> > Cc: dev@dpdk.org; Xie, Huawei; rich.lane@bigswitch.com; mst@redhat.com;
> > nakajima.yoshihiro@lab.ntt.co.jp; p.fedin@samsung.com;
> > ann.zhuangyanying@huawei.com; mukawa@igel.co.jp;
> > nhorman@tuxdriver.com
> > Subject: Re: [PATCH v5 2/8] virtio: clean up virtio_dev_queue_setup
> >
> > On Mon, May 30, 2016 at 10:55:33AM +0000, Jianfeng Tan wrote:
> > > Abstract vring hdr desc init as an inline method.
> >
> > What's this patch for then? In your last version, it will be invoked
> > twice, but it turned out to be wrong. So, why keeping this change?
> > I didn't see it improves anything.
> >
>
> Yes, for now, only one version is kept because the position to call this function is changed. And I think this segment of code functions as a special purpose, which can be abstracted as a function, make sense?
Yeah, maybe. But idealy, we should move it to tx_queue_setup() function.
Let's see what we might do after applying Huawei's rx/tx split patch: it
needs a while (I saw bugs).
--yliu
@@ -278,6 +278,26 @@ virtio_dev_queue_release(struct virtqueue *vq)
}
}
+static void
+vring_hdr_desc_init(struct virtqueue *vq)
+{
+ int i;
+ struct virtio_tx_region *txr = vq->virtio_net_hdr_mz->addr;
+
+ for (i = 0; i < vq->vq_nentries; i++) {
+ struct vring_desc *start_dp = txr[i].tx_indir;
+
+ vring_desc_init(start_dp, RTE_DIM(txr[i].tx_indir));
+
+ /* first indirect descriptor is always the tx header */
+ start_dp->addr = vq->virtio_net_hdr_mem + i * sizeof(*txr) +
+ offsetof(struct virtio_tx_region, tx_hdr);
+
+ start_dp->len = vq->hw->vtnet_hdr_size;
+ start_dp->flags = VRING_DESC_F_NEXT;
+ }
+}
+
int virtio_dev_queue_setup(struct rte_eth_dev *dev,
int queue_type,
uint16_t queue_idx,
@@ -375,8 +395,7 @@ int virtio_dev_queue_setup(struct rte_eth_dev *dev,
if (queue_type == VTNET_TQ) {
const struct rte_memzone *hdr_mz;
- struct virtio_tx_region *txr;
- unsigned int i;
+ size_t hdr_mz_sz = vq_size * sizeof(struct virtio_tx_region);
/*
* For each xmit packet, allocate a virtio_net_hdr
@@ -385,7 +404,7 @@ int virtio_dev_queue_setup(struct rte_eth_dev *dev,
snprintf(vq_name, sizeof(vq_name), "port%d_tvq%d_hdrzone",
dev->data->port_id, queue_idx);
hdr_mz = rte_memzone_reserve_aligned(vq_name,
- vq_size * sizeof(*txr),
+ hdr_mz_sz,
socket_id, 0,
RTE_CACHE_LINE_SIZE);
if (hdr_mz == NULL) {
@@ -399,21 +418,8 @@ int virtio_dev_queue_setup(struct rte_eth_dev *dev,
vq->virtio_net_hdr_mz = hdr_mz;
vq->virtio_net_hdr_mem = hdr_mz->phys_addr;
- txr = hdr_mz->addr;
- memset(txr, 0, vq_size * sizeof(*txr));
- for (i = 0; i < vq_size; i++) {
- struct vring_desc *start_dp = txr[i].tx_indir;
-
- vring_desc_init(start_dp, RTE_DIM(txr[i].tx_indir));
-
- /* first indirect descriptor is always the tx header */
- start_dp->addr = vq->virtio_net_hdr_mem
- + i * sizeof(*txr)
- + offsetof(struct virtio_tx_region, tx_hdr);
-
- start_dp->len = vq->hw->vtnet_hdr_size;
- start_dp->flags = VRING_DESC_F_NEXT;
- }
+ memset(hdr_mz->addr, 0, hdr_mz_sz);
+ vring_hdr_desc_init(vq);
} else if (queue_type == VTNET_CQ) {
/* Allocate a page for control vq command, data and status */