[dpdk-dev] [PATCH 4/4] virtio/vdev: add a new vdev named eth_cvio
On 1/11/2016 2:43 AM, Tan, Jianfeng wrote: > Add a new virtual device named eth_cvio, it can be used just like > eth_ring, eth_null, etc. > > Configured parameters include: > - rx (optional, 1 by default): number of rx, only allowed to be > 1 for now. > - tx (optional, 1 by default): number of tx, only allowed to be > 1 for now. >From APP side, virtio is something HW, in your implementation rx/tx is max queue numbers virtio supported. Does it make sense? Why need user tell HW, how much queues it support? We'd better make it un-configurable, only let users query it like the real HW, and then decide how much queues it need to enable. > - cq (optional, 0 by default): if ctrl queue is enabled, not > supported for now. > - mac (optional): mac address, random value will be given if not > specified. > - queue_num (optional, 256 by default): size of virtqueue. Better change it to queue_size. Thanks, Michael > - path (madatory): path of vhost, depends on the file type: > vhost-user is used if the given path points to > a unix socket; vhost-net is used if the given > path points to a char device. > > The major difference with original virtio for vm is that, here we > use virtual address instead of physical address for vhost to > calculate relative address. > > When enable CONFIG_RTE_VIRTIO_VDEV (enabled by default), the compiled > library can be used in both VM and container environment. > > Examples: > a. Use vhost-net as a backend > sudo numactl -N 1 -m 1 ./examples/l2fwd/build/l2fwd -c 0x10 -n 4 \ > -m 1024 --no-pci --single-file --file-prefix=l2fwd \ > --vdev=eth_cvio0,mac=00:01:02:03:04:05,path=/dev/vhost-net \ > -- -p 0x1 > > b. Use vhost-user as a backend > numactl -N 1 -m 1 ./examples/l2fwd/build/l2fwd -c 0x10 -n 4 -m 1024 \ > --no-pci --single-file --file-prefix=l2fwd \ > --vdev=eth_cvio0,mac=00:01:02:03:04:05,path= \ > -- -p 0x1 > > Signed-off-by: Huawei Xie > Signed-off-by: Jianfeng Tan > --- >
[dpdk-dev] [PATCH 4/4] virtio/vdev: add a new vdev named eth_cvio
Hi Fedin, On 1/12/2016 4:39 PM, Tan, Jianfeng wrote: > Hi Fedin, > > On 1/12/2016 3:45 PM, Pavel Fedin wrote: >> Hello! >> >> See inline >> >>> ... >>> } >>> >>> +struct rte_mbuf *m = NULL; >>> +if (dev->dev_type == RTE_ETH_DEV_PCI) >>> +vq->offset = (uintptr_t)&m->buf_addr; >>> +#ifdef RTE_VIRTIO_VDEV >>> +else { >>> +vq->offset = (uintptr_t)&m->buf_physaddr; >> Not sure, but shouldn't these be swapped? Originally, for PCI >> devices, we used buf_physaddr. > > Oops, seems that you are right. I'm trying to figure out why I can > rx/tx pkts using the wrong version. > I figure out why. When we run apps without root privilege, mempool's elt_pa is assigned the same of elt_va_start. So it happens to be right value to translate addresses. But it's definitely a bug. Thanks for pointing this out. Thanks, Jianfeng
[dpdk-dev] [PATCH 4/4] virtio/vdev: add a new vdev named eth_cvio
Hi Fedin, On 1/12/2016 3:45 PM, Pavel Fedin wrote: > Hello! > > See inline > >> ... >> } >> >> +struct rte_mbuf *m = NULL; >> +if (dev->dev_type == RTE_ETH_DEV_PCI) >> +vq->offset = (uintptr_t)&m->buf_addr; >> +#ifdef RTE_VIRTIO_VDEV >> +else { >> +vq->offset = (uintptr_t)&m->buf_physaddr; > Not sure, but shouldn't these be swapped? Originally, for PCI devices, we > used buf_physaddr. Oops, seems that you are right. I'm trying to figure out why I can rx/tx pkts using the wrong version. >> #define VIRTIO_READ_REG_1(hw, reg) \ >> -(hw->io_base != VIRTIO_VDEV_IO_BASE) ? \ >> +((hw->io_base != VIRTIO_VDEV_IO_BASE) ? \ >> inb((VIRTIO_PCI_REG_ADDR((hw), (reg \ >> -:virtio_ioport_read(hw, reg) >> +:virtio_ioport_read(hw, reg)) >> #define VIRTIO_WRITE_REG_1(hw, reg, value) \ >> -(hw->io_base != VIRTIO_VDEV_IO_BASE) ? \ >> +((hw->io_base != VIRTIO_VDEV_IO_BASE) ? \ >> outb_p((unsigned char)(value), (VIRTIO_PCI_REG_ADDR((hw), (reg \ >> -:virtio_ioport_write(hw, reg, value) >> +:virtio_ioport_write(hw, reg, value)) >> >> #define VIRTIO_READ_REG_2(hw, reg) \ >> -(hw->io_base != VIRTIO_VDEV_IO_BASE) ? \ >> +((hw->io_base != VIRTIO_VDEV_IO_BASE) ? \ >> inw((VIRTIO_PCI_REG_ADDR((hw), (reg \ >> -:virtio_ioport_read(hw, reg) >> +:virtio_ioport_read(hw, reg)) >> #define VIRTIO_WRITE_REG_2(hw, reg, value) \ >> -(hw->io_base != VIRTIO_VDEV_IO_BASE) ? \ >> +((hw->io_base != VIRTIO_VDEV_IO_BASE) ? \ >> outw_p((unsigned short)(value), (VIRTIO_PCI_REG_ADDR((hw), (reg \ >> -:virtio_ioport_write(hw, reg, value) >> +:virtio_ioport_write(hw, reg, value)) >> >> #define VIRTIO_READ_REG_4(hw, reg) \ >> -(hw->io_base != VIRTIO_VDEV_IO_BASE) ? \ >> +((hw->io_base != VIRTIO_VDEV_IO_BASE) ? \ >> inl((VIRTIO_PCI_REG_ADDR((hw), (reg \ >> -:virtio_ioport_read(hw, reg) >> +:virtio_ioport_read(hw, reg)) >> #define VIRTIO_WRITE_REG_4(hw, reg, value) \ >> -(hw->io_base != VIRTIO_VDEV_IO_BASE) ? \ >> +((hw->io_base != VIRTIO_VDEV_IO_BASE) ? \ >> outl_p((unsigned int)(value), (VIRTIO_PCI_REG_ADDR((hw), (reg \ >> -:virtio_ioport_write(hw, reg, value) >> +:virtio_ioport_write(hw, reg, value)) > These bracket fixups should be squashed into #3 > I'll rewrite this into function pointers according to Yuanhan's patch for virtio 1.0. Thanks, Jianfeng
[dpdk-dev] [PATCH 4/4] virtio/vdev: add a new vdev named eth_cvio
On Tue, Jan 12, 2016 at 10:45:59AM +0300, Pavel Fedin wrote: > Hello! > > See inline Hi, Please strip unrelated context, so that people could reach to your comments as quick as possible, otherwise, people could easily get lost from the long patch. > > > -Original Message- > > From: Jianfeng Tan [mailto:jianfeng.tan at intel.com] > > + struct rte_mbuf *m = NULL; > > + if (dev->dev_type == RTE_ETH_DEV_PCI) > > + vq->offset = (uintptr_t)&m->buf_addr; > > +#ifdef RTE_VIRTIO_VDEV > > + else { > > + vq->offset = (uintptr_t)&m->buf_physaddr; > > Not sure, but shouldn't these be swapped? Originally, for PCI devices, we > used buf_physaddr. And this reply just servers as an example only :) --yliu
[dpdk-dev] [PATCH 4/4] virtio/vdev: add a new vdev named eth_cvio
Hello! See inline > -Original Message- > From: Jianfeng Tan [mailto:jianfeng.tan at intel.com] > Sent: Sunday, January 10, 2016 2:43 PM > To: dev at dpdk.org > Cc: rich.lane at bigswitch.com; yuanhan.liu at linux.intel.com; mst at > redhat.com; > nakajima.yoshihiro at lab.ntt.co.jp; huawei.xie at intel.com; mukawa at > igel.co.jp; > p.fedin at samsung.com; michael.qiu at intel.com; ann.zhuangyanying at > huawei.com; Jianfeng Tan > Subject: [PATCH 4/4] virtio/vdev: add a new vdev named eth_cvio > > Add a new virtual device named eth_cvio, it can be used just like > eth_ring, eth_null, etc. > > Configured parameters include: > - rx (optional, 1 by default): number of rx, only allowed to be > 1 for now. > - tx (optional, 1 by default): number of tx, only allowed to be > 1 for now. > - cq (optional, 0 by default): if ctrl queue is enabled, not > supported for now. > - mac (optional): mac address, random value will be given if not > specified. > - queue_num (optional, 256 by default): size of virtqueue. > - path (madatory): path of vhost, depends on the file type: > vhost-user is used if the given path points to > a unix socket; vhost-net is used if the given > path points to a char device. > > The major difference with original virtio for vm is that, here we > use virtual address instead of physical address for vhost to > calculate relative address. > > When enable CONFIG_RTE_VIRTIO_VDEV (enabled by default), the compiled > library can be used in both VM and container environment. > > Examples: > a. Use vhost-net as a backend > sudo numactl -N 1 -m 1 ./examples/l2fwd/build/l2fwd -c 0x10 -n 4 \ > -m 1024 --no-pci --single-file --file-prefix=l2fwd \ > --vdev=eth_cvio0,mac=00:01:02:03:04:05,path=/dev/vhost-net \ > -- -p 0x1 > > b. Use vhost-user as a backend > numactl -N 1 -m 1 ./examples/l2fwd/build/l2fwd -c 0x10 -n 4 -m 1024 \ > --no-pci --single-file --file-prefix=l2fwd \ > --vdev=eth_cvio0,mac=00:01:02:03:04:05,path= \ > -- -p 0x1 > > Signed-off-by: Huawei Xie > Signed-off-by: Jianfeng Tan > --- > drivers/net/virtio/virtio_ethdev.c | 338 > +--- > drivers/net/virtio/virtio_ethdev.h | 1 + > drivers/net/virtio/virtio_pci.h | 24 +-- > drivers/net/virtio/virtio_rxtx.c| 11 +- > drivers/net/virtio/virtio_rxtx_simple.c | 14 +- > drivers/net/virtio/virtqueue.h | 13 +- > 6 files changed, 302 insertions(+), 99 deletions(-) > > diff --git a/drivers/net/virtio/virtio_ethdev.c > b/drivers/net/virtio/virtio_ethdev.c > index d928339..6e46060 100644 > --- a/drivers/net/virtio/virtio_ethdev.c > +++ b/drivers/net/virtio/virtio_ethdev.c > @@ -56,6 +56,7 @@ > #include > #include > #include > +#include > > #include "virtio_ethdev.h" > #include "virtio_pci.h" > @@ -174,14 +175,14 @@ virtio_send_command(struct virtqueue *vq, struct > virtio_pmd_ctrl *ctrl, >* One RX packet for ACK. >*/ > vq->vq_ring.desc[head].flags = VRING_DESC_F_NEXT; > - vq->vq_ring.desc[head].addr = vq->virtio_net_hdr_mz->phys_addr; > + vq->vq_ring.desc[head].addr = vq->virtio_net_hdr_mem; > vq->vq_ring.desc[head].len = sizeof(struct virtio_net_ctrl_hdr); > vq->vq_free_cnt--; > i = vq->vq_ring.desc[head].next; > > for (k = 0; k < pkt_num; k++) { > vq->vq_ring.desc[i].flags = VRING_DESC_F_NEXT; > - vq->vq_ring.desc[i].addr = vq->virtio_net_hdr_mz->phys_addr > + vq->vq_ring.desc[i].addr = vq->virtio_net_hdr_mem > + sizeof(struct virtio_net_ctrl_hdr) > + sizeof(ctrl->status) + sizeof(uint8_t)*sum; > vq->vq_ring.desc[i].len = dlen[k]; > @@ -191,7 +192,7 @@ virtio_send_command(struct virtqueue *vq, struct > virtio_pmd_ctrl *ctrl, > } > > vq->vq_ring.desc[i].flags = VRING_DESC_F_WRITE; > - vq->vq_ring.desc[i].addr = vq->virtio_net_hdr_mz->phys_addr > + vq->vq_ring.desc[i].addr = vq->virtio_net_hdr_mem > + sizeof(struct virtio_net_ctrl_hdr); > vq->vq_ring.desc[i].len = sizeof(ctrl->status); > vq->vq_free_cnt--; > @@ -374,68 +375,85 @@ int virtio_dev_queue_setup(struct rte_eth_dev *dev, > } > } > > - /* > - * Virtio PCI device VIRTIO_PCI_QUEUE_PF register is 32bit, > - * and only accepts 32 bit page frame number. > - * Check if the allocated physical memory exceeds 16TB. > - */ > - if ((mz->phys_addr + vq->vq_ring_size - 1) >> > (VIRTIO_PCI_QUEUE_ADDR_SHIFT + 32)) { > - PMD_INIT_LOG(ERR, "vring address shouldn't be above 16TB!"); > - rte_free(vq); > - return -ENOMEM; > - } > - > memset(mz->addr, 0, sizeof(mz->len)); > vq-
[dpdk-dev] [PATCH 4/4] virtio/vdev: add a new vdev named eth_cvio
Add a new virtual device named eth_cvio, it can be used just like eth_ring, eth_null, etc. Configured parameters include: - rx (optional, 1 by default): number of rx, only allowed to be 1 for now. - tx (optional, 1 by default): number of tx, only allowed to be 1 for now. - cq (optional, 0 by default): if ctrl queue is enabled, not supported for now. - mac (optional): mac address, random value will be given if not specified. - queue_num (optional, 256 by default): size of virtqueue. - path (madatory): path of vhost, depends on the file type: vhost-user is used if the given path points to a unix socket; vhost-net is used if the given path points to a char device. The major difference with original virtio for vm is that, here we use virtual address instead of physical address for vhost to calculate relative address. When enable CONFIG_RTE_VIRTIO_VDEV (enabled by default), the compiled library can be used in both VM and container environment. Examples: a. Use vhost-net as a backend sudo numactl -N 1 -m 1 ./examples/l2fwd/build/l2fwd -c 0x10 -n 4 \ -m 1024 --no-pci --single-file --file-prefix=l2fwd \ --vdev=eth_cvio0,mac=00:01:02:03:04:05,path=/dev/vhost-net \ -- -p 0x1 b. Use vhost-user as a backend numactl -N 1 -m 1 ./examples/l2fwd/build/l2fwd -c 0x10 -n 4 -m 1024 \ --no-pci --single-file --file-prefix=l2fwd \ --vdev=eth_cvio0,mac=00:01:02:03:04:05,path= \ -- -p 0x1 Signed-off-by: Huawei Xie Signed-off-by: Jianfeng Tan --- drivers/net/virtio/virtio_ethdev.c | 338 +--- drivers/net/virtio/virtio_ethdev.h | 1 + drivers/net/virtio/virtio_pci.h | 24 +-- drivers/net/virtio/virtio_rxtx.c| 11 +- drivers/net/virtio/virtio_rxtx_simple.c | 14 +- drivers/net/virtio/virtqueue.h | 13 +- 6 files changed, 302 insertions(+), 99 deletions(-) diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c index d928339..6e46060 100644 --- a/drivers/net/virtio/virtio_ethdev.c +++ b/drivers/net/virtio/virtio_ethdev.c @@ -56,6 +56,7 @@ #include #include #include +#include #include "virtio_ethdev.h" #include "virtio_pci.h" @@ -174,14 +175,14 @@ virtio_send_command(struct virtqueue *vq, struct virtio_pmd_ctrl *ctrl, * One RX packet for ACK. */ vq->vq_ring.desc[head].flags = VRING_DESC_F_NEXT; - vq->vq_ring.desc[head].addr = vq->virtio_net_hdr_mz->phys_addr; + vq->vq_ring.desc[head].addr = vq->virtio_net_hdr_mem; vq->vq_ring.desc[head].len = sizeof(struct virtio_net_ctrl_hdr); vq->vq_free_cnt--; i = vq->vq_ring.desc[head].next; for (k = 0; k < pkt_num; k++) { vq->vq_ring.desc[i].flags = VRING_DESC_F_NEXT; - vq->vq_ring.desc[i].addr = vq->virtio_net_hdr_mz->phys_addr + vq->vq_ring.desc[i].addr = vq->virtio_net_hdr_mem + sizeof(struct virtio_net_ctrl_hdr) + sizeof(ctrl->status) + sizeof(uint8_t)*sum; vq->vq_ring.desc[i].len = dlen[k]; @@ -191,7 +192,7 @@ virtio_send_command(struct virtqueue *vq, struct virtio_pmd_ctrl *ctrl, } vq->vq_ring.desc[i].flags = VRING_DESC_F_WRITE; - vq->vq_ring.desc[i].addr = vq->virtio_net_hdr_mz->phys_addr + vq->vq_ring.desc[i].addr = vq->virtio_net_hdr_mem + sizeof(struct virtio_net_ctrl_hdr); vq->vq_ring.desc[i].len = sizeof(ctrl->status); vq->vq_free_cnt--; @@ -374,68 +375,85 @@ int virtio_dev_queue_setup(struct rte_eth_dev *dev, } } - /* -* Virtio PCI device VIRTIO_PCI_QUEUE_PF register is 32bit, -* and only accepts 32 bit page frame number. -* Check if the allocated physical memory exceeds 16TB. -*/ - if ((mz->phys_addr + vq->vq_ring_size - 1) >> (VIRTIO_PCI_QUEUE_ADDR_SHIFT + 32)) { - PMD_INIT_LOG(ERR, "vring address shouldn't be above 16TB!"); - rte_free(vq); - return -ENOMEM; - } - memset(mz->addr, 0, sizeof(mz->len)); vq->mz = mz; - vq->vq_ring_mem = mz->phys_addr; vq->vq_ring_virt_mem = mz->addr; - PMD_INIT_LOG(DEBUG, "vq->vq_ring_mem: 0x%"PRIx64, (uint64_t)mz->phys_addr); - PMD_INIT_LOG(DEBUG, "vq->vq_ring_virt_mem: 0x%"PRIx64, (uint64_t)(uintptr_t)mz->addr); + + if (dev->dev_type == RTE_ETH_DEV_PCI) { + vq->vq_ring_mem = mz->phys_addr; + + /* Virtio PCI device VIRTIO_PCI_QUEUE_PF register is 32bit, +* and only accepts 32 bit page frame number. +* Check if the allocated physical memory exceeds 16TB. +*/ + uint64_t last_physaddr =