[dpdk-dev] [RFC PATCH v2] vhost: Add VHOST PMD
On 2015/10/27 22:44, Traynor, Kevin wrote: >> -Original Message- >> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Tetsuya Mukawa > [snip] > >> Hi, >> >> I have submitted latest patches. >> I will keep vhost library until we will have agreement to merge it to >> vhost PMD. > Longer term there are pros and cons to keeping the vhost library. Personally > I think it would make sense to remove sometime as trying to maintain two API's > has a cost, but I think adding a deprecation notice in DPDK 2.2 for removal in > DPDK 2.3 is very premature. Until it's proven *in the field* that the vhost > PMD > is a suitable fully functioning replacement for the vhost library and users > have time to migrate, then please don't remove. Hi Kevin, Thanks for commenting. I agree it's not the time to add deprecation notice. (I haven't included it in the vhost PMD patches) Tetsuya
[dpdk-dev] [RFC PATCH v2] vhost: Add VHOST PMD
> -Original Message- > From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Tetsuya Mukawa [snip] > > Hi, > > I have submitted latest patches. > I will keep vhost library until we will have agreement to merge it to > vhost PMD. Longer term there are pros and cons to keeping the vhost library. Personally I think it would make sense to remove sometime as trying to maintain two API's has a cost, but I think adding a deprecation notice in DPDK 2.2 for removal in DPDK 2.3 is very premature. Until it's proven *in the field* that the vhost PMD is a suitable fully functioning replacement for the vhost library and users have time to migrate, then please don't remove. > > Regards, > Testuya
[dpdk-dev] [RFC PATCH v2] vhost: Add VHOST PMD
On 2015/10/21 19:22, Bruce Richardson wrote: > On Wed, Oct 21, 2015 at 09:25:12AM +0300, Panu Matilainen wrote: >> On 10/21/2015 07:35 AM, Tetsuya Mukawa wrote: >>> On 2015/10/19 22:27, Richardson, Bruce wrote: >>>>> -Original Message- >>>>> From: Panu Matilainen [mailto:pmatilai at redhat.com] >>>>> Sent: Monday, October 19, 2015 2:26 PM >>>>> To: Tetsuya Mukawa ; Richardson, Bruce >>>>> ; Loftus, Ciara >>>>> Cc: dev at dpdk.org; ann.zhuangyanying at huawei.com >>>>> Subject: Re: [dpdk-dev] [RFC PATCH v2] vhost: Add VHOST PMD >>>>> >>>>> On 10/19/2015 01:50 PM, Tetsuya Mukawa wrote: >>>>>> On 2015/10/19 18:45, Bruce Richardson wrote: >>>>>>> On Mon, Oct 19, 2015 at 10:32:50AM +0100, Loftus, Ciara wrote: >>>>>>>>> On 2015/10/16 21:52, Bruce Richardson wrote: >>>>>>>>>> On Mon, Aug 31, 2015 at 12:55:26PM +0900, Tetsuya Mukawa wrote: >>>>>>>>>>> The patch introduces a new PMD. This PMD is implemented as thin >>>>>>>>> wrapper >>>>>>>>>>> of librte_vhost. It means librte_vhost is also needed to compile >>>>> the PMD. >>>>>>>>>>> The PMD can have 'iface' parameter like below to specify a path >>>>>>>>>>> to >>>>>>>>> connect >>>>>>>>>>> to a virtio-net device. >>>>>>>>>>> >>>>>>>>>>> $ ./testpmd -c f -n 4 --vdev 'eth_vhost0,iface=/tmp/sock0' -- -i >>>>>>>>>>> >>>>>>>>>>> To connect above testpmd, here is qemu command example. >>>>>>>>>>> >>>>>>>>>>> $ qemu-system-x86_64 \ >>>>>>>>>>> >>>>>>>>>>> -chardev socket,id=chr0,path=/tmp/sock0 \ >>>>>>>>>>> -netdev vhost-user,id=net0,chardev=chr0,vhostforce \ >>>>>>>>>>> -device virtio-net-pci,netdev=net0 >>>>>>>>>>> >>>>>>>>>>> Signed-off-by: Tetsuya Mukawa >>>>>>>>>> With this PMD in place, is there any need to keep the existing >>>>>>>>>> vhost library around as a separate entity? Can the existing >>>>>>>>>> library be >>>>>>>>> subsumed/converted into >>>>>>>>>> a standard PMD? >>>>>>>>>> >>>>>>>>>> /Bruce >>>>>>>>> Hi Bruce, >>>>>>>>> >>>>>>>>> I concern about whether the PMD has all features of librte_vhost, >>>>>>>>> because librte_vhost provides more features and freedom than ethdev >>>>>>>>> API provides. >>>>>>>>> In some cases, user needs to choose limited implementation without >>>>>>>>> librte_vhost. >>>>>>>>> I am going to eliminate such cases while implementing the PMD. >>>>>>>>> But I don't have strong belief that we can remove librte_vhost now. >>>>>>>>> >>>>>>>>> So how about keeping current separation in next DPDK? >>>>>>>>> I guess people will try to replace librte_vhost to vhost PMD, >>>>>>>>> because apparently using ethdev APIs will be useful in many cases. >>>>>>>>> And we will get feedbacks like "vhost PMD needs to support like this >>>>> usage". >>>>>>>>> (Or we will not have feedbacks, but it's also OK.) Then, we will be >>>>>>>>> able to merge librte_vhost and vhost PMD. >>>>>>>> I agree with the above. One the concerns I had when reviewing the >>>>> patch was that the PMD removes some freedom that is available with the >>>>> library. Eg. Ability to implement the new_device and destroy_device >>>>> callbacks. If using the PMD you are constrained to the implementations of >>>>> these in the PMD driver, but if using librte_vhost, you can implement your >>>>> own with whatever functionality you like - a good example of this can be >>>>> seen
[dpdk-dev] [RFC PATCH v2] vhost: Add VHOST PMD
On 2015/10/19 22:27, Richardson, Bruce wrote: >> -Original Message- >> From: Panu Matilainen [mailto:pmatilai at redhat.com] >> Sent: Monday, October 19, 2015 2:26 PM >> To: Tetsuya Mukawa ; Richardson, Bruce >> ; Loftus, Ciara >> Cc: dev at dpdk.org; ann.zhuangyanying at huawei.com >> Subject: Re: [dpdk-dev] [RFC PATCH v2] vhost: Add VHOST PMD >> >> On 10/19/2015 01:50 PM, Tetsuya Mukawa wrote: >>> On 2015/10/19 18:45, Bruce Richardson wrote: >>>> On Mon, Oct 19, 2015 at 10:32:50AM +0100, Loftus, Ciara wrote: >>>>>> On 2015/10/16 21:52, Bruce Richardson wrote: >>>>>>> On Mon, Aug 31, 2015 at 12:55:26PM +0900, Tetsuya Mukawa wrote: >>>>>>>> The patch introduces a new PMD. This PMD is implemented as thin >>>>>> wrapper >>>>>>>> of librte_vhost. It means librte_vhost is also needed to compile >> the PMD. >>>>>>>> The PMD can have 'iface' parameter like below to specify a path >>>>>>>> to >>>>>> connect >>>>>>>> to a virtio-net device. >>>>>>>> >>>>>>>> $ ./testpmd -c f -n 4 --vdev 'eth_vhost0,iface=/tmp/sock0' -- -i >>>>>>>> >>>>>>>> To connect above testpmd, here is qemu command example. >>>>>>>> >>>>>>>> $ qemu-system-x86_64 \ >>>>>>>> >>>>>>>> -chardev socket,id=chr0,path=/tmp/sock0 \ >>>>>>>> -netdev vhost-user,id=net0,chardev=chr0,vhostforce \ >>>>>>>> -device virtio-net-pci,netdev=net0 >>>>>>>> >>>>>>>> Signed-off-by: Tetsuya Mukawa >>>>>>> With this PMD in place, is there any need to keep the existing >>>>>>> vhost library around as a separate entity? Can the existing >>>>>>> library be >>>>>> subsumed/converted into >>>>>>> a standard PMD? >>>>>>> >>>>>>> /Bruce >>>>>> Hi Bruce, >>>>>> >>>>>> I concern about whether the PMD has all features of librte_vhost, >>>>>> because librte_vhost provides more features and freedom than ethdev >>>>>> API provides. >>>>>> In some cases, user needs to choose limited implementation without >>>>>> librte_vhost. >>>>>> I am going to eliminate such cases while implementing the PMD. >>>>>> But I don't have strong belief that we can remove librte_vhost now. >>>>>> >>>>>> So how about keeping current separation in next DPDK? >>>>>> I guess people will try to replace librte_vhost to vhost PMD, >>>>>> because apparently using ethdev APIs will be useful in many cases. >>>>>> And we will get feedbacks like "vhost PMD needs to support like this >> usage". >>>>>> (Or we will not have feedbacks, but it's also OK.) Then, we will be >>>>>> able to merge librte_vhost and vhost PMD. >>>>> I agree with the above. One the concerns I had when reviewing the >> patch was that the PMD removes some freedom that is available with the >> library. Eg. Ability to implement the new_device and destroy_device >> callbacks. If using the PMD you are constrained to the implementations of >> these in the PMD driver, but if using librte_vhost, you can implement your >> own with whatever functionality you like - a good example of this can be >> seen in the vhost sample app. >>>>> On the other hand, the PMD is useful in that it removes a lot of >> complexity for the user and may work for some more general use cases. So I >> would be in favour of having both options available too. >>>>> Ciara >>>>> >>>> Thanks. >>>> However, just because the libraries are merged does not mean that you >>>> need be limited by PMD functionality. Many PMDs provide additional >>>> library-specific functions over and above their PMD capabilities. The >>>> bonded PMD is a good example here, as it has a whole set of extra >>>> functions to create and manipulate bonded devices - things that are >>>> obviously not part of the general ethdev API. Other vPMDs similarly >> include functions to allow them to be created on the fly too. >>>> regards, >&g
[dpdk-dev] [RFC PATCH v2] vhost: Add VHOST PMD
On 2015/10/20 23:13, Loftus, Ciara wrote:
>
+
+static uint16_t
+eth_vhost_tx(void *q, struct rte_mbuf **bufs, uint16_t nb_bufs)
+{
+ struct vhost_queue *r = q;
+ uint16_t i, nb_tx = 0;
+
+ if (unlikely(r->internal == NULL))
+ return 0;
+
+ if (unlikely(rte_atomic16_read(&r->internal->xfer) == 0))
+ return 0;
+
+ rte_atomic16_set(&r->tx_executing, 1);
+
+ if (unlikely(rte_atomic16_read(&r->internal->xfer) == 0))
+ goto out;
+
+ nb_tx = (uint16_t)rte_vhost_enqueue_burst(r->device,
+ VIRTIO_RXQ, bufs, nb_bufs);
+
+ rte_atomic64_add(&(r->tx_pkts), nb_tx);
+ rte_atomic64_add(&(r->err_pkts), nb_bufs - nb_tx);
+
+ for (i = 0; likely(i < nb_tx); i++)
+ rte_pktmbuf_free(bufs[i]);
>>> We may not always want to free these mbufs. For example, if a call is made
>> to rte_eth_tx_burst with buffers from another (non DPDK) source, they may
>> not be ours to free.
>>
>> Sorry, I am not sure what type of buffer you want to transfer.
>>
>> This is a PMD that wraps librte_vhost.
>> And I guess other PMDs cannot handle buffers from another non DPDK
>> source.
>> Should we take care such buffers?
>>
>> I have also checked af_packet PMD.
>> It seems the tx function of af_packet PMD just frees mbuf.
> For example if using the PMD with an application that receives buffers from
> another source. Eg. a virtual switch receiving packets from an interface
> using the kernel driver.
For example, if a software switch on host tries to send data to DPDK
application on guest using vhost PMD and virtio-net PMD.
Also let's assume transfer data of software switch is come from kernel
driver.
In this case, these data on software switch will be copied and
transferred to virio-net PMD through virtqueue.
Because of this, we can free data after sending.
Could you please also check API documentation rte_eth_tx_burst?
(Freeing buffer is default behavior)
> I see that af_packet also frees the mbuf. I've checked the ixgbe and ring
> pmds though and they don't seem to free the buffers, although I may have
> missed something, the code for these is rather large and I am unfamiliar with
> most of it. If I am correct though, should this behaviour vary from PMD to
> PMD I wonder?
I guess ring PMD is something special.
Because we don't want to copy data with this PMD, RX function doesn't
allocate buffers, also TX function doesn't free buffers.
But other normal PMD will allocate buffers when RX is called, and free
buffers when TX is called.
+
+
+ eth_dev = rte_eth_dev_allocated(internal->dev_name);
+ if (eth_dev == NULL) {
+ RTE_LOG(INFO, PMD, "failuer to find ethdev\n");
>>> Typo: Failure. Same for the destroy_device function
>> Thanks, I will fix it in next patches.
>>
+ return -1;
+ }
+
+ internal = eth_dev->data->dev_private;
+
+ for (i = 0; i < internal->nb_rx_queues; i++) {
+ vq = &internal->rx_vhost_queues[i];
+ vq->device = dev;
+ vq->internal = internal;
+ }
+ for (i = 0; i < internal->nb_tx_queues; i++) {
+ vq = &internal->tx_vhost_queues[i];
+ vq->device = dev;
+ vq->internal = internal;
+ }
+
+ dev->flags |= VIRTIO_DEV_RUNNING;
+ dev->priv = eth_dev;
+
+ eth_dev->data->dev_link.link_status = 1;
+ rte_atomic16_set(&internal->xfer, 1);
+
+ RTE_LOG(INFO, PMD, "New connection established\n");
+
+ return 0;
>>> Some freedom is taken away if the new_device and destroy_device
>> callbacks are implemented in the driver.
>>> For example if one wishes to call the rte_vhost_enable_guest_notification
>> function when a new device is brought up. They cannot now as there is no
>> scope to modify these callbacks, as is done in for example the vHost sample
>> app. Is this correct?
>>
>> So how about adding one more parameter to be able to choose guest
>> notification behavior?
>>
>> ex)
>> ./testpmd --vdev 'eth_vhost0,iface=/tmp/sock0,guest_notification=0'
>>
>> In above case, all queues in this device will have
>> VRING_USED_F_NO_NOTIFY.
> I'm not too concerned about this particular function, I was just making an
> example. The main concern I was expressing here and in the other thread with
> Bruce, is the risk that we will lose some functionality available in the
> library but not in the PMD. This function is an example of that. If we could
> find some way to retain the functionality available in the library, it would
> be ideal.
I will reply to an other thread.
Anyway, I am going to keep current vhost library APIs.
Thanks,
Tetsuya
[dpdk-dev] [RFC PATCH v2] vhost: Add VHOST PMD
On Wed, Oct 21, 2015 at 09:25:12AM +0300, Panu Matilainen wrote: > On 10/21/2015 07:35 AM, Tetsuya Mukawa wrote: > >On 2015/10/19 22:27, Richardson, Bruce wrote: > >>>-Original Message- > >>>From: Panu Matilainen [mailto:pmatilai at redhat.com] > >>>Sent: Monday, October 19, 2015 2:26 PM > >>>To: Tetsuya Mukawa ; Richardson, Bruce > >>>; Loftus, Ciara > >>>Cc: dev at dpdk.org; ann.zhuangyanying at huawei.com > >>>Subject: Re: [dpdk-dev] [RFC PATCH v2] vhost: Add VHOST PMD > >>> > >>>On 10/19/2015 01:50 PM, Tetsuya Mukawa wrote: > >>>>On 2015/10/19 18:45, Bruce Richardson wrote: > >>>>>On Mon, Oct 19, 2015 at 10:32:50AM +0100, Loftus, Ciara wrote: > >>>>>>>On 2015/10/16 21:52, Bruce Richardson wrote: > >>>>>>>>On Mon, Aug 31, 2015 at 12:55:26PM +0900, Tetsuya Mukawa wrote: > >>>>>>>>>The patch introduces a new PMD. This PMD is implemented as thin > >>>>>>>wrapper > >>>>>>>>>of librte_vhost. It means librte_vhost is also needed to compile > >>>the PMD. > >>>>>>>>>The PMD can have 'iface' parameter like below to specify a path > >>>>>>>>>to > >>>>>>>connect > >>>>>>>>>to a virtio-net device. > >>>>>>>>> > >>>>>>>>>$ ./testpmd -c f -n 4 --vdev 'eth_vhost0,iface=/tmp/sock0' -- -i > >>>>>>>>> > >>>>>>>>>To connect above testpmd, here is qemu command example. > >>>>>>>>> > >>>>>>>>>$ qemu-system-x86_64 \ > >>>>>>>>> > >>>>>>>>> -chardev socket,id=chr0,path=/tmp/sock0 \ > >>>>>>>>> -netdev vhost-user,id=net0,chardev=chr0,vhostforce \ > >>>>>>>>> -device virtio-net-pci,netdev=net0 > >>>>>>>>> > >>>>>>>>>Signed-off-by: Tetsuya Mukawa > >>>>>>>>With this PMD in place, is there any need to keep the existing > >>>>>>>>vhost library around as a separate entity? Can the existing > >>>>>>>>library be > >>>>>>>subsumed/converted into > >>>>>>>>a standard PMD? > >>>>>>>> > >>>>>>>>/Bruce > >>>>>>>Hi Bruce, > >>>>>>> > >>>>>>>I concern about whether the PMD has all features of librte_vhost, > >>>>>>>because librte_vhost provides more features and freedom than ethdev > >>>>>>>API provides. > >>>>>>>In some cases, user needs to choose limited implementation without > >>>>>>>librte_vhost. > >>>>>>>I am going to eliminate such cases while implementing the PMD. > >>>>>>>But I don't have strong belief that we can remove librte_vhost now. > >>>>>>> > >>>>>>>So how about keeping current separation in next DPDK? > >>>>>>>I guess people will try to replace librte_vhost to vhost PMD, > >>>>>>>because apparently using ethdev APIs will be useful in many cases. > >>>>>>>And we will get feedbacks like "vhost PMD needs to support like this > >>>usage". > >>>>>>>(Or we will not have feedbacks, but it's also OK.) Then, we will be > >>>>>>>able to merge librte_vhost and vhost PMD. > >>>>>>I agree with the above. One the concerns I had when reviewing the > >>>patch was that the PMD removes some freedom that is available with the > >>>library. Eg. Ability to implement the new_device and destroy_device > >>>callbacks. If using the PMD you are constrained to the implementations of > >>>these in the PMD driver, but if using librte_vhost, you can implement your > >>>own with whatever functionality you like - a good example of this can be > >>>seen in the vhost sample app. > >>>>>>On the other hand, the PMD is useful in that it removes a lot of > >>>complexity for the user and may work for some more general use cases. So I > >>>would be in favour of having both options available too. > >>>
[dpdk-dev] [RFC PATCH v2] vhost: Add VHOST PMD
On Wed, Oct 21, 2015 at 01:30:54PM +0900, Tetsuya Mukawa wrote: > On 2015/10/20 23:13, Loftus, Ciara wrote: > > > > I see that af_packet also frees the mbuf. I've checked the ixgbe and ring > > pmds though and they don't seem to free the buffers, although I may have > > missed something, the code for these is rather large and I am unfamiliar > > with most of it. If I am correct though, should this behaviour vary from > > PMD to PMD I wonder? > > I guess ring PMD is something special. > Because we don't want to copy data with this PMD, RX function doesn't > allocate buffers, also TX function doesn't free buffers. > But other normal PMD will allocate buffers when RX is called, and free > buffers when TX is called. > Yes, this is correct. Ring pmd is the exception since it automatically recycles buffers, and so does not need to alloc/free mbufs. (ixgbe frees the buffers post-TX as part of the TX ring cleanup) /Bruce
[dpdk-dev] [RFC PATCH v2] vhost: Add VHOST PMD
On 10/21/2015 07:35 AM, Tetsuya Mukawa wrote: > On 2015/10/19 22:27, Richardson, Bruce wrote: >>> -Original Message- >>> From: Panu Matilainen [mailto:pmatilai at redhat.com] >>> Sent: Monday, October 19, 2015 2:26 PM >>> To: Tetsuya Mukawa ; Richardson, Bruce >>> ; Loftus, Ciara >>> Cc: dev at dpdk.org; ann.zhuangyanying at huawei.com >>> Subject: Re: [dpdk-dev] [RFC PATCH v2] vhost: Add VHOST PMD >>> >>> On 10/19/2015 01:50 PM, Tetsuya Mukawa wrote: >>>> On 2015/10/19 18:45, Bruce Richardson wrote: >>>>> On Mon, Oct 19, 2015 at 10:32:50AM +0100, Loftus, Ciara wrote: >>>>>>> On 2015/10/16 21:52, Bruce Richardson wrote: >>>>>>>> On Mon, Aug 31, 2015 at 12:55:26PM +0900, Tetsuya Mukawa wrote: >>>>>>>>> The patch introduces a new PMD. This PMD is implemented as thin >>>>>>> wrapper >>>>>>>>> of librte_vhost. It means librte_vhost is also needed to compile >>> the PMD. >>>>>>>>> The PMD can have 'iface' parameter like below to specify a path >>>>>>>>> to >>>>>>> connect >>>>>>>>> to a virtio-net device. >>>>>>>>> >>>>>>>>> $ ./testpmd -c f -n 4 --vdev 'eth_vhost0,iface=/tmp/sock0' -- -i >>>>>>>>> >>>>>>>>> To connect above testpmd, here is qemu command example. >>>>>>>>> >>>>>>>>> $ qemu-system-x86_64 \ >>>>>>>>> >>>>>>>>> -chardev socket,id=chr0,path=/tmp/sock0 \ >>>>>>>>> -netdev vhost-user,id=net0,chardev=chr0,vhostforce \ >>>>>>>>> -device virtio-net-pci,netdev=net0 >>>>>>>>> >>>>>>>>> Signed-off-by: Tetsuya Mukawa >>>>>>>> With this PMD in place, is there any need to keep the existing >>>>>>>> vhost library around as a separate entity? Can the existing >>>>>>>> library be >>>>>>> subsumed/converted into >>>>>>>> a standard PMD? >>>>>>>> >>>>>>>> /Bruce >>>>>>> Hi Bruce, >>>>>>> >>>>>>> I concern about whether the PMD has all features of librte_vhost, >>>>>>> because librte_vhost provides more features and freedom than ethdev >>>>>>> API provides. >>>>>>> In some cases, user needs to choose limited implementation without >>>>>>> librte_vhost. >>>>>>> I am going to eliminate such cases while implementing the PMD. >>>>>>> But I don't have strong belief that we can remove librte_vhost now. >>>>>>> >>>>>>> So how about keeping current separation in next DPDK? >>>>>>> I guess people will try to replace librte_vhost to vhost PMD, >>>>>>> because apparently using ethdev APIs will be useful in many cases. >>>>>>> And we will get feedbacks like "vhost PMD needs to support like this >>> usage". >>>>>>> (Or we will not have feedbacks, but it's also OK.) Then, we will be >>>>>>> able to merge librte_vhost and vhost PMD. >>>>>> I agree with the above. One the concerns I had when reviewing the >>> patch was that the PMD removes some freedom that is available with the >>> library. Eg. Ability to implement the new_device and destroy_device >>> callbacks. If using the PMD you are constrained to the implementations of >>> these in the PMD driver, but if using librte_vhost, you can implement your >>> own with whatever functionality you like - a good example of this can be >>> seen in the vhost sample app. >>>>>> On the other hand, the PMD is useful in that it removes a lot of >>> complexity for the user and may work for some more general use cases. So I >>> would be in favour of having both options available too. >>>>>> Ciara >>>>>> >>>>> Thanks. >>>>> However, just because the libraries are merged does not mean that you >>>>> need be limited by PMD functionality. Many PMDs provide additional >>>>> library-specific functions over and above their PMD capabilities. The >>>
[dpdk-dev] [RFC PATCH v2] vhost: Add VHOST PMD
>
> On 2015/09/24 2:47, Loftus, Ciara wrote:
> >> The patch introduces a new PMD. This PMD is implemented as thin
> wrapper
> >> of librte_vhost. It means librte_vhost is also needed to compile the PMD.
> >> The PMD can have 'iface' parameter like below to specify a path to
> connect
> >> to a virtio-net device.
> >>
> >> $ ./testpmd -c f -n 4 --vdev 'eth_vhost0,iface=/tmp/sock0' -- -i
> >>
> >> To connect above testpmd, here is qemu command example.
> >>
> >> $ qemu-system-x86_64 \
> >>
> >> -chardev socket,id=chr0,path=/tmp/sock0 \
> >> -netdev vhost-user,id=net0,chardev=chr0,vhostforce \
> >> -device virtio-net-pci,netdev=net0
> >>
> >> Signed-off-by: Tetsuya Mukawa
> >> ---
> >> config/common_linuxapp | 6 +
> >> drivers/net/Makefile| 4 +
> >> drivers/net/vhost/Makefile | 61 +++
> >> drivers/net/vhost/rte_eth_vhost.c | 640
> >>
> >> drivers/net/vhost/rte_pmd_vhost_version.map | 4 +
> >> mk/rte.app.mk | 8 +-
> >> 6 files changed, 722 insertions(+), 1 deletion(-)
> >> create mode 100644 drivers/net/vhost/Makefile
> >> create mode 100644 drivers/net/vhost/rte_eth_vhost.c
> >> create mode 100644 drivers/net/vhost/rte_pmd_vhost_version.map
> >>
> >> +struct pmd_internal {
> >> + TAILQ_ENTRY(pmd_internal) next;
> >> + char *dev_name;
> >> + char *iface_name;
> >> + unsigned nb_rx_queues;
> >> + unsigned nb_tx_queues;
> >> + rte_atomic16_t xfer;
> > Is this flag just used to indicate the state of the virtio_net device?
> > Ie. if =0 then virtio_dev=NULL and if =1 then virtio_net !=NULL & the
> VIRTIO_DEV_RUNNING flag is set?
>
> Hi Clara,
>
> I am sorry for very late reply.
>
> Yes, it is. Probably we can optimize it more.
> I will change this implementation a bit in next patches.
> Could you please check it?
Of course, thanks.
>
> >> +
> >> +static uint16_t
> >> +eth_vhost_tx(void *q, struct rte_mbuf **bufs, uint16_t nb_bufs)
> >> +{
> >> + struct vhost_queue *r = q;
> >> + uint16_t i, nb_tx = 0;
> >> +
> >> + if (unlikely(r->internal == NULL))
> >> + return 0;
> >> +
> >> + if (unlikely(rte_atomic16_read(&r->internal->xfer) == 0))
> >> + return 0;
> >> +
> >> + rte_atomic16_set(&r->tx_executing, 1);
> >> +
> >> + if (unlikely(rte_atomic16_read(&r->internal->xfer) == 0))
> >> + goto out;
> >> +
> >> + nb_tx = (uint16_t)rte_vhost_enqueue_burst(r->device,
> >> + VIRTIO_RXQ, bufs, nb_bufs);
> >> +
> >> + rte_atomic64_add(&(r->tx_pkts), nb_tx);
> >> + rte_atomic64_add(&(r->err_pkts), nb_bufs - nb_tx);
> >> +
> >> + for (i = 0; likely(i < nb_tx); i++)
> >> + rte_pktmbuf_free(bufs[i]);
> > We may not always want to free these mbufs. For example, if a call is made
> to rte_eth_tx_burst with buffers from another (non DPDK) source, they may
> not be ours to free.
>
> Sorry, I am not sure what type of buffer you want to transfer.
>
> This is a PMD that wraps librte_vhost.
> And I guess other PMDs cannot handle buffers from another non DPDK
> source.
> Should we take care such buffers?
>
> I have also checked af_packet PMD.
> It seems the tx function of af_packet PMD just frees mbuf.
For example if using the PMD with an application that receives buffers from
another source. Eg. a virtual switch receiving packets from an interface using
the kernel driver.
I see that af_packet also frees the mbuf. I've checked the ixgbe and ring pmds
though and they don't seem to free the buffers, although I may have missed
something, the code for these is rather large and I am unfamiliar with most of
it. If I am correct though, should this behaviour vary from PMD to PMD I wonder?
>
> >> +
> >> +
> >> + eth_dev = rte_eth_dev_allocated(internal->dev_name);
> >> + if (eth_dev == NULL) {
> >> + RTE_LOG(INFO, PMD, "failuer to find ethdev\n");
> > Typo: Failure. Same for the destroy_device function
>
> Thanks, I will fix it in next patches.
>
> >> + return -1;
> >> + }
> >> +
> >> + internal = eth_dev->data->dev_private;
> >> +
> >> + for (i = 0; i < internal->nb_rx_queues; i++) {
> >> + vq = &internal->rx_vhost_queues[i];
> >> + vq->device = dev;
> >> + vq->internal = internal;
> >> + }
> >> + for (i = 0; i < internal->nb_tx_queues; i++) {
> >> + vq = &internal->tx_vhost_queues[i];
> >> + vq->device = dev;
> >> + vq->internal = internal;
> >> + }
> >> +
> >> + dev->flags |= VIRTIO_DEV_RUNNING;
> >> + dev->priv = eth_dev;
> >> +
> >> + eth_dev->data->dev_link.link_status = 1;
> >> + rte_atomic16_set(&internal->xfer, 1);
> >> +
> >> + RTE_LOG(INFO, PMD, "New connection established\n");
> >> +
> >> + return 0;
> > Some freedom is taken away if the new_device and destroy_device
> callbacks are implemented in the driver.
> > For example if one wishes to call the rte_vhost_enable_guest_notificatio
[dpdk-dev] [RFC PATCH v2] vhost: Add VHOST PMD
On 2015/10/19 18:45, Bruce Richardson wrote: > On Mon, Oct 19, 2015 at 10:32:50AM +0100, Loftus, Ciara wrote: >>> On 2015/10/16 21:52, Bruce Richardson wrote: On Mon, Aug 31, 2015 at 12:55:26PM +0900, Tetsuya Mukawa wrote: > The patch introduces a new PMD. This PMD is implemented as thin >>> wrapper > of librte_vhost. It means librte_vhost is also needed to compile the PMD. > The PMD can have 'iface' parameter like below to specify a path to >>> connect > to a virtio-net device. > > $ ./testpmd -c f -n 4 --vdev 'eth_vhost0,iface=/tmp/sock0' -- -i > > To connect above testpmd, here is qemu command example. > > $ qemu-system-x86_64 \ > > -chardev socket,id=chr0,path=/tmp/sock0 \ > -netdev vhost-user,id=net0,chardev=chr0,vhostforce \ > -device virtio-net-pci,netdev=net0 > > Signed-off-by: Tetsuya Mukawa With this PMD in place, is there any need to keep the existing vhost library around as a separate entity? Can the existing library be >>> subsumed/converted into a standard PMD? /Bruce >>> Hi Bruce, >>> >>> I concern about whether the PMD has all features of librte_vhost, >>> because librte_vhost provides more features and freedom than ethdev API >>> provides. >>> In some cases, user needs to choose limited implementation without >>> librte_vhost. >>> I am going to eliminate such cases while implementing the PMD. >>> But I don't have strong belief that we can remove librte_vhost now. >>> >>> So how about keeping current separation in next DPDK? >>> I guess people will try to replace librte_vhost to vhost PMD, because >>> apparently using ethdev APIs will be useful in many cases. >>> And we will get feedbacks like "vhost PMD needs to support like this usage". >>> (Or we will not have feedbacks, but it's also OK.) >>> Then, we will be able to merge librte_vhost and vhost PMD. >> I agree with the above. One the concerns I had when reviewing the patch was >> that the PMD removes some freedom that is available with the library. Eg. >> Ability to implement the new_device and destroy_device callbacks. If using >> the PMD you are constrained to the implementations of these in the PMD >> driver, but if using librte_vhost, you can implement your own with whatever >> functionality you like - a good example of this can be seen in the vhost >> sample app. >> On the other hand, the PMD is useful in that it removes a lot of complexity >> for the user and may work for some more general use cases. So I would be in >> favour of having both options available too. >> >> Ciara >> > Thanks. > However, just because the libraries are merged does not mean that you need > be limited by PMD functionality. Many PMDs provide additional library-specific > functions over and above their PMD capabilities. The bonded PMD is a good > example > here, as it has a whole set of extra functions to create and manipulate bonded > devices - things that are obviously not part of the general ethdev API. Other > vPMDs similarly include functions to allow them to be created on the fly too. > > regards, > /Bruce Hi Bruce, I appreciate for showing a good example. I haven't noticed the PMD. I will check the bonding PMD, and try to remove librte_vhost without losing freedom and features of the library. Regards, Tetsuya
[dpdk-dev] [RFC PATCH v2] vhost: Add VHOST PMD
On 10/19/2015 01:50 PM, Tetsuya Mukawa wrote: > On 2015/10/19 18:45, Bruce Richardson wrote: >> On Mon, Oct 19, 2015 at 10:32:50AM +0100, Loftus, Ciara wrote: On 2015/10/16 21:52, Bruce Richardson wrote: > On Mon, Aug 31, 2015 at 12:55:26PM +0900, Tetsuya Mukawa wrote: >> The patch introduces a new PMD. This PMD is implemented as thin wrapper >> of librte_vhost. It means librte_vhost is also needed to compile the PMD. >> The PMD can have 'iface' parameter like below to specify a path to connect >> to a virtio-net device. >> >> $ ./testpmd -c f -n 4 --vdev 'eth_vhost0,iface=/tmp/sock0' -- -i >> >> To connect above testpmd, here is qemu command example. >> >> $ qemu-system-x86_64 \ >> >> -chardev socket,id=chr0,path=/tmp/sock0 \ >> -netdev vhost-user,id=net0,chardev=chr0,vhostforce \ >> -device virtio-net-pci,netdev=net0 >> >> Signed-off-by: Tetsuya Mukawa > With this PMD in place, is there any need to keep the existing vhost > library > around as a separate entity? Can the existing library be subsumed/converted into > a standard PMD? > > /Bruce Hi Bruce, I concern about whether the PMD has all features of librte_vhost, because librte_vhost provides more features and freedom than ethdev API provides. In some cases, user needs to choose limited implementation without librte_vhost. I am going to eliminate such cases while implementing the PMD. But I don't have strong belief that we can remove librte_vhost now. So how about keeping current separation in next DPDK? I guess people will try to replace librte_vhost to vhost PMD, because apparently using ethdev APIs will be useful in many cases. And we will get feedbacks like "vhost PMD needs to support like this usage". (Or we will not have feedbacks, but it's also OK.) Then, we will be able to merge librte_vhost and vhost PMD. >>> I agree with the above. One the concerns I had when reviewing the patch was >>> that the PMD removes some freedom that is available with the library. Eg. >>> Ability to implement the new_device and destroy_device callbacks. If using >>> the PMD you are constrained to the implementations of these in the PMD >>> driver, but if using librte_vhost, you can implement your own with whatever >>> functionality you like - a good example of this can be seen in the vhost >>> sample app. >>> On the other hand, the PMD is useful in that it removes a lot of complexity >>> for the user and may work for some more general use cases. So I would be in >>> favour of having both options available too. >>> >>> Ciara >>> >> Thanks. >> However, just because the libraries are merged does not mean that you need >> be limited by PMD functionality. Many PMDs provide additional >> library-specific >> functions over and above their PMD capabilities. The bonded PMD is a good >> example >> here, as it has a whole set of extra functions to create and manipulate >> bonded >> devices - things that are obviously not part of the general ethdev API. Other >> vPMDs similarly include functions to allow them to be created on the fly too. >> >> regards, >> /Bruce > > Hi Bruce, > > I appreciate for showing a good example. I haven't noticed the PMD. > I will check the bonding PMD, and try to remove librte_vhost without > losing freedom and features of the library. Hi, Just a gentle reminder - if you consider removing (even if by just replacing/renaming) an entire library, it needs to happen the ABI deprecation process. It seems obvious enough. But for all the ABI policing here, somehow we all failed to notice the two compatibility breaking rename-elephants in the room during 2.1 development: - libintel_dpdk was renamed to libdpdk - librte_pmd_virtio_uio was renamed to librte_pmd_virtio Of course these cases are easy to work around with symlinks, and are unrelated to the matter at hand. Just wanting to make sure such things dont happen again. - Panu -
[dpdk-dev] [RFC PATCH v2] vhost: Add VHOST PMD
> -Original Message- > From: Panu Matilainen [mailto:pmatilai at redhat.com] > Sent: Monday, October 19, 2015 2:26 PM > To: Tetsuya Mukawa ; Richardson, Bruce > ; Loftus, Ciara > Cc: dev at dpdk.org; ann.zhuangyanying at huawei.com > Subject: Re: [dpdk-dev] [RFC PATCH v2] vhost: Add VHOST PMD > > On 10/19/2015 01:50 PM, Tetsuya Mukawa wrote: > > On 2015/10/19 18:45, Bruce Richardson wrote: > >> On Mon, Oct 19, 2015 at 10:32:50AM +0100, Loftus, Ciara wrote: > >>>> On 2015/10/16 21:52, Bruce Richardson wrote: > >>>>> On Mon, Aug 31, 2015 at 12:55:26PM +0900, Tetsuya Mukawa wrote: > >>>>>> The patch introduces a new PMD. This PMD is implemented as thin > >>>> wrapper > >>>>>> of librte_vhost. It means librte_vhost is also needed to compile > the PMD. > >>>>>> The PMD can have 'iface' parameter like below to specify a path > >>>>>> to > >>>> connect > >>>>>> to a virtio-net device. > >>>>>> > >>>>>> $ ./testpmd -c f -n 4 --vdev 'eth_vhost0,iface=/tmp/sock0' -- -i > >>>>>> > >>>>>> To connect above testpmd, here is qemu command example. > >>>>>> > >>>>>> $ qemu-system-x86_64 \ > >>>>>> > >>>>>> -chardev socket,id=chr0,path=/tmp/sock0 \ > >>>>>> -netdev vhost-user,id=net0,chardev=chr0,vhostforce \ > >>>>>> -device virtio-net-pci,netdev=net0 > >>>>>> > >>>>>> Signed-off-by: Tetsuya Mukawa > >>>>> With this PMD in place, is there any need to keep the existing > >>>>> vhost library around as a separate entity? Can the existing > >>>>> library be > >>>> subsumed/converted into > >>>>> a standard PMD? > >>>>> > >>>>> /Bruce > >>>> Hi Bruce, > >>>> > >>>> I concern about whether the PMD has all features of librte_vhost, > >>>> because librte_vhost provides more features and freedom than ethdev > >>>> API provides. > >>>> In some cases, user needs to choose limited implementation without > >>>> librte_vhost. > >>>> I am going to eliminate such cases while implementing the PMD. > >>>> But I don't have strong belief that we can remove librte_vhost now. > >>>> > >>>> So how about keeping current separation in next DPDK? > >>>> I guess people will try to replace librte_vhost to vhost PMD, > >>>> because apparently using ethdev APIs will be useful in many cases. > >>>> And we will get feedbacks like "vhost PMD needs to support like this > usage". > >>>> (Or we will not have feedbacks, but it's also OK.) Then, we will be > >>>> able to merge librte_vhost and vhost PMD. > >>> I agree with the above. One the concerns I had when reviewing the > patch was that the PMD removes some freedom that is available with the > library. Eg. Ability to implement the new_device and destroy_device > callbacks. If using the PMD you are constrained to the implementations of > these in the PMD driver, but if using librte_vhost, you can implement your > own with whatever functionality you like - a good example of this can be > seen in the vhost sample app. > >>> On the other hand, the PMD is useful in that it removes a lot of > complexity for the user and may work for some more general use cases. So I > would be in favour of having both options available too. > >>> > >>> Ciara > >>> > >> Thanks. > >> However, just because the libraries are merged does not mean that you > >> need be limited by PMD functionality. Many PMDs provide additional > >> library-specific functions over and above their PMD capabilities. The > >> bonded PMD is a good example here, as it has a whole set of extra > >> functions to create and manipulate bonded devices - things that are > >> obviously not part of the general ethdev API. Other vPMDs similarly > include functions to allow them to be created on the fly too. > >> > >> regards, > >> /Bruce > > > > Hi Bruce, > > > > I appreciate for showing a good example. I haven't noticed the PMD. > > I will check the bonding PMD, and try to remove librte_vhost without > > losing freedom and features of the library. > > Hi, > > Just a gentle reminder - if you consider removing (even if by just > replacing/renaming) an entire library, it needs to happen the ABI > deprecation process. > > It seems obvious enough. But for all the ABI policing here, somehow we all > failed to notice the two compatibility breaking rename-elephants in the > room during 2.1 development: > - libintel_dpdk was renamed to libdpdk > - librte_pmd_virtio_uio was renamed to librte_pmd_virtio > > Of course these cases are easy to work around with symlinks, and are > unrelated to the matter at hand. Just wanting to make sure such things > dont happen again. > > - Panu - Still doesn't hurt to remind us, Panu! Thanks. :-)
[dpdk-dev] [RFC PATCH v2] vhost: Add VHOST PMD
On 2015/10/16 21:52, Bruce Richardson wrote: > On Mon, Aug 31, 2015 at 12:55:26PM +0900, Tetsuya Mukawa wrote: >> The patch introduces a new PMD. This PMD is implemented as thin wrapper >> of librte_vhost. It means librte_vhost is also needed to compile the PMD. >> The PMD can have 'iface' parameter like below to specify a path to connect >> to a virtio-net device. >> >> $ ./testpmd -c f -n 4 --vdev 'eth_vhost0,iface=/tmp/sock0' -- -i >> >> To connect above testpmd, here is qemu command example. >> >> $ qemu-system-x86_64 \ >> >> -chardev socket,id=chr0,path=/tmp/sock0 \ >> -netdev vhost-user,id=net0,chardev=chr0,vhostforce \ >> -device virtio-net-pci,netdev=net0 >> >> Signed-off-by: Tetsuya Mukawa > With this PMD in place, is there any need to keep the existing vhost library > around as a separate entity? Can the existing library be subsumed/converted > into > a standard PMD? > > /Bruce Hi Bruce, I concern about whether the PMD has all features of librte_vhost, because librte_vhost provides more features and freedom than ethdev API provides. In some cases, user needs to choose limited implementation without librte_vhost. I am going to eliminate such cases while implementing the PMD. But I don't have strong belief that we can remove librte_vhost now. So how about keeping current separation in next DPDK? I guess people will try to replace librte_vhost to vhost PMD, because apparently using ethdev APIs will be useful in many cases. And we will get feedbacks like "vhost PMD needs to support like this usage". (Or we will not have feedbacks, but it's also OK.) Then, we will be able to merge librte_vhost and vhost PMD. Thanks, Tetsuya
[dpdk-dev] [RFC PATCH v2] vhost: Add VHOST PMD
On Mon, Oct 19, 2015 at 10:32:50AM +0100, Loftus, Ciara wrote: > > On 2015/10/16 21:52, Bruce Richardson wrote: > > > On Mon, Aug 31, 2015 at 12:55:26PM +0900, Tetsuya Mukawa wrote: > > >> The patch introduces a new PMD. This PMD is implemented as thin > > wrapper > > >> of librte_vhost. It means librte_vhost is also needed to compile the PMD. > > >> The PMD can have 'iface' parameter like below to specify a path to > > connect > > >> to a virtio-net device. > > >> > > >> $ ./testpmd -c f -n 4 --vdev 'eth_vhost0,iface=/tmp/sock0' -- -i > > >> > > >> To connect above testpmd, here is qemu command example. > > >> > > >> $ qemu-system-x86_64 \ > > >> > > >> -chardev socket,id=chr0,path=/tmp/sock0 \ > > >> -netdev vhost-user,id=net0,chardev=chr0,vhostforce \ > > >> -device virtio-net-pci,netdev=net0 > > >> > > >> Signed-off-by: Tetsuya Mukawa > > > With this PMD in place, is there any need to keep the existing vhost > > > library > > > around as a separate entity? Can the existing library be > > subsumed/converted into > > > a standard PMD? > > > > > > /Bruce > > > > Hi Bruce, > > > > I concern about whether the PMD has all features of librte_vhost, > > because librte_vhost provides more features and freedom than ethdev API > > provides. > > In some cases, user needs to choose limited implementation without > > librte_vhost. > > I am going to eliminate such cases while implementing the PMD. > > But I don't have strong belief that we can remove librte_vhost now. > > > > So how about keeping current separation in next DPDK? > > I guess people will try to replace librte_vhost to vhost PMD, because > > apparently using ethdev APIs will be useful in many cases. > > And we will get feedbacks like "vhost PMD needs to support like this usage". > > (Or we will not have feedbacks, but it's also OK.) > > Then, we will be able to merge librte_vhost and vhost PMD. > > I agree with the above. One the concerns I had when reviewing the patch was > that the PMD removes some freedom that is available with the library. Eg. > Ability to implement the new_device and destroy_device callbacks. If using > the PMD you are constrained to the implementations of these in the PMD > driver, but if using librte_vhost, you can implement your own with whatever > functionality you like - a good example of this can be seen in the vhost > sample app. > On the other hand, the PMD is useful in that it removes a lot of complexity > for the user and may work for some more general use cases. So I would be in > favour of having both options available too. > > Ciara > Thanks. However, just because the libraries are merged does not mean that you need be limited by PMD functionality. Many PMDs provide additional library-specific functions over and above their PMD capabilities. The bonded PMD is a good example here, as it has a whole set of extra functions to create and manipulate bonded devices - things that are obviously not part of the general ethdev API. Other vPMDs similarly include functions to allow them to be created on the fly too. regards, /Bruce
[dpdk-dev] [RFC PATCH v2] vhost: Add VHOST PMD
> On 2015/10/16 21:52, Bruce Richardson wrote: > > On Mon, Aug 31, 2015 at 12:55:26PM +0900, Tetsuya Mukawa wrote: > >> The patch introduces a new PMD. This PMD is implemented as thin > wrapper > >> of librte_vhost. It means librte_vhost is also needed to compile the PMD. > >> The PMD can have 'iface' parameter like below to specify a path to > connect > >> to a virtio-net device. > >> > >> $ ./testpmd -c f -n 4 --vdev 'eth_vhost0,iface=/tmp/sock0' -- -i > >> > >> To connect above testpmd, here is qemu command example. > >> > >> $ qemu-system-x86_64 \ > >> > >> -chardev socket,id=chr0,path=/tmp/sock0 \ > >> -netdev vhost-user,id=net0,chardev=chr0,vhostforce \ > >> -device virtio-net-pci,netdev=net0 > >> > >> Signed-off-by: Tetsuya Mukawa > > With this PMD in place, is there any need to keep the existing vhost library > > around as a separate entity? Can the existing library be > subsumed/converted into > > a standard PMD? > > > > /Bruce > > Hi Bruce, > > I concern about whether the PMD has all features of librte_vhost, > because librte_vhost provides more features and freedom than ethdev API > provides. > In some cases, user needs to choose limited implementation without > librte_vhost. > I am going to eliminate such cases while implementing the PMD. > But I don't have strong belief that we can remove librte_vhost now. > > So how about keeping current separation in next DPDK? > I guess people will try to replace librte_vhost to vhost PMD, because > apparently using ethdev APIs will be useful in many cases. > And we will get feedbacks like "vhost PMD needs to support like this usage". > (Or we will not have feedbacks, but it's also OK.) > Then, we will be able to merge librte_vhost and vhost PMD. I agree with the above. One the concerns I had when reviewing the patch was that the PMD removes some freedom that is available with the library. Eg. Ability to implement the new_device and destroy_device callbacks. If using the PMD you are constrained to the implementations of these in the PMD driver, but if using librte_vhost, you can implement your own with whatever functionality you like - a good example of this can be seen in the vhost sample app. On the other hand, the PMD is useful in that it removes a lot of complexity for the user and may work for some more general use cases. So I would be in favour of having both options available too. Ciara > > Thanks, > Tetsuya
[dpdk-dev] [RFC PATCH v2] vhost: Add VHOST PMD
On 2015/09/24 2:47, Loftus, Ciara wrote:
>> The patch introduces a new PMD. This PMD is implemented as thin wrapper
>> of librte_vhost. It means librte_vhost is also needed to compile the PMD.
>> The PMD can have 'iface' parameter like below to specify a path to connect
>> to a virtio-net device.
>>
>> $ ./testpmd -c f -n 4 --vdev 'eth_vhost0,iface=/tmp/sock0' -- -i
>>
>> To connect above testpmd, here is qemu command example.
>>
>> $ qemu-system-x86_64 \
>>
>> -chardev socket,id=chr0,path=/tmp/sock0 \
>> -netdev vhost-user,id=net0,chardev=chr0,vhostforce \
>> -device virtio-net-pci,netdev=net0
>>
>> Signed-off-by: Tetsuya Mukawa
>> ---
>> config/common_linuxapp | 6 +
>> drivers/net/Makefile| 4 +
>> drivers/net/vhost/Makefile | 61 +++
>> drivers/net/vhost/rte_eth_vhost.c | 640
>>
>> drivers/net/vhost/rte_pmd_vhost_version.map | 4 +
>> mk/rte.app.mk | 8 +-
>> 6 files changed, 722 insertions(+), 1 deletion(-)
>> create mode 100644 drivers/net/vhost/Makefile
>> create mode 100644 drivers/net/vhost/rte_eth_vhost.c
>> create mode 100644 drivers/net/vhost/rte_pmd_vhost_version.map
>>
>> +struct pmd_internal {
>> +TAILQ_ENTRY(pmd_internal) next;
>> +char *dev_name;
>> +char *iface_name;
>> +unsigned nb_rx_queues;
>> +unsigned nb_tx_queues;
>> +rte_atomic16_t xfer;
> Is this flag just used to indicate the state of the virtio_net device?
> Ie. if =0 then virtio_dev=NULL and if =1 then virtio_net !=NULL & the
> VIRTIO_DEV_RUNNING flag is set?
Hi Clara,
I am sorry for very late reply.
Yes, it is. Probably we can optimize it more.
I will change this implementation a bit in next patches.
Could you please check it?
>> +
>> +static uint16_t
>> +eth_vhost_tx(void *q, struct rte_mbuf **bufs, uint16_t nb_bufs)
>> +{
>> +struct vhost_queue *r = q;
>> +uint16_t i, nb_tx = 0;
>> +
>> +if (unlikely(r->internal == NULL))
>> +return 0;
>> +
>> +if (unlikely(rte_atomic16_read(&r->internal->xfer) == 0))
>> +return 0;
>> +
>> +rte_atomic16_set(&r->tx_executing, 1);
>> +
>> +if (unlikely(rte_atomic16_read(&r->internal->xfer) == 0))
>> +goto out;
>> +
>> +nb_tx = (uint16_t)rte_vhost_enqueue_burst(r->device,
>> +VIRTIO_RXQ, bufs, nb_bufs);
>> +
>> +rte_atomic64_add(&(r->tx_pkts), nb_tx);
>> +rte_atomic64_add(&(r->err_pkts), nb_bufs - nb_tx);
>> +
>> +for (i = 0; likely(i < nb_tx); i++)
>> +rte_pktmbuf_free(bufs[i]);
> We may not always want to free these mbufs. For example, if a call is made to
> rte_eth_tx_burst with buffers from another (non DPDK) source, they may not be
> ours to free.
Sorry, I am not sure what type of buffer you want to transfer.
This is a PMD that wraps librte_vhost.
And I guess other PMDs cannot handle buffers from another non DPDK source.
Should we take care such buffers?
I have also checked af_packet PMD.
It seems the tx function of af_packet PMD just frees mbuf.
>> +
>> +
>> +eth_dev = rte_eth_dev_allocated(internal->dev_name);
>> +if (eth_dev == NULL) {
>> +RTE_LOG(INFO, PMD, "failuer to find ethdev\n");
> Typo: Failure. Same for the destroy_device function
Thanks, I will fix it in next patches.
>> +return -1;
>> +}
>> +
>> +internal = eth_dev->data->dev_private;
>> +
>> +for (i = 0; i < internal->nb_rx_queues; i++) {
>> +vq = &internal->rx_vhost_queues[i];
>> +vq->device = dev;
>> +vq->internal = internal;
>> +}
>> +for (i = 0; i < internal->nb_tx_queues; i++) {
>> +vq = &internal->tx_vhost_queues[i];
>> +vq->device = dev;
>> +vq->internal = internal;
>> +}
>> +
>> +dev->flags |= VIRTIO_DEV_RUNNING;
>> +dev->priv = eth_dev;
>> +
>> +eth_dev->data->dev_link.link_status = 1;
>> +rte_atomic16_set(&internal->xfer, 1);
>> +
>> +RTE_LOG(INFO, PMD, "New connection established\n");
>> +
>> +return 0;
> Some freedom is taken away if the new_device and destroy_device callbacks are
> implemented in the driver.
> For example if one wishes to call the rte_vhost_enable_guest_notification
> function when a new device is brought up. They cannot now as there is no
> scope to modify these callbacks, as is done in for example the vHost sample
> app. Is this correct?
So how about adding one more parameter to be able to choose guest
notification behavior?
ex)
./testpmd --vdev 'eth_vhost0,iface=/tmp/sock0,guest_notification=0'
In above case, all queues in this device will have VRING_USED_F_NO_NOTIFY.
Thanks,
Tetsuya
[dpdk-dev] [RFC PATCH v2] vhost: Add VHOST PMD
On Mon, Aug 31, 2015 at 12:55:26PM +0900, Tetsuya Mukawa wrote: > The patch introduces a new PMD. This PMD is implemented as thin wrapper > of librte_vhost. It means librte_vhost is also needed to compile the PMD. > The PMD can have 'iface' parameter like below to specify a path to connect > to a virtio-net device. > > $ ./testpmd -c f -n 4 --vdev 'eth_vhost0,iface=/tmp/sock0' -- -i > > To connect above testpmd, here is qemu command example. > > $ qemu-system-x86_64 \ > > -chardev socket,id=chr0,path=/tmp/sock0 \ > -netdev vhost-user,id=net0,chardev=chr0,vhostforce \ > -device virtio-net-pci,netdev=net0 > > Signed-off-by: Tetsuya Mukawa With this PMD in place, is there any need to keep the existing vhost library around as a separate entity? Can the existing library be subsumed/converted into a standard PMD? /Bruce
[dpdk-dev] [RFC PATCH v2] vhost: Add VHOST PMD
> The patch introduces a new PMD. This PMD is implemented as thin wrapper > of librte_vhost. It means librte_vhost is also needed to compile the PMD. > The PMD can have 'iface' parameter like below to specify a path to connect > to a virtio-net device. > > $ ./testpmd -c f -n 4 --vdev 'eth_vhost0,iface=/tmp/sock0' -- -i > > To connect above testpmd, here is qemu command example. > > $ qemu-system-x86_64 \ > > -chardev socket,id=chr0,path=/tmp/sock0 \ > -netdev vhost-user,id=net0,chardev=chr0,vhostforce \ > -device virtio-net-pci,netdev=net0 > > Signed-off-by: Tetsuya Mukawa > --- > config/common_linuxapp | 6 + > drivers/net/Makefile| 4 + > drivers/net/vhost/Makefile | 61 +++ > drivers/net/vhost/rte_eth_vhost.c | 640 > > drivers/net/vhost/rte_pmd_vhost_version.map | 4 + > mk/rte.app.mk | 8 +- > 6 files changed, 722 insertions(+), 1 deletion(-) > create mode 100644 drivers/net/vhost/Makefile > create mode 100644 drivers/net/vhost/rte_eth_vhost.c > create mode 100644 drivers/net/vhost/rte_pmd_vhost_version.map > > diff --git a/config/common_linuxapp b/config/common_linuxapp > index 0de43d5..7310240 100644 > --- a/config/common_linuxapp > +++ b/config/common_linuxapp > @@ -446,6 +446,12 @@ CONFIG_RTE_LIBRTE_VHOST_NUMA=n > CONFIG_RTE_LIBRTE_VHOST_DEBUG=n > > # > +# Compile vhost PMD > +# To compile, CONFIG_RTE_LIBRTE_VHOST should be enabled. > +# > +CONFIG_RTE_LIBRTE_PMD_VHOST=y > + > +# > #Compile Xen domain0 support > # > CONFIG_RTE_LIBRTE_XEN_DOM0=n > diff --git a/drivers/net/Makefile b/drivers/net/Makefile > index 5ebf963..e46a38e 100644 > --- a/drivers/net/Makefile > +++ b/drivers/net/Makefile > @@ -49,5 +49,9 @@ DIRS-$(CONFIG_RTE_LIBRTE_VIRTIO_PMD) += virtio > DIRS-$(CONFIG_RTE_LIBRTE_VMXNET3_PMD) += vmxnet3 > DIRS-$(CONFIG_RTE_LIBRTE_PMD_XENVIRT) += xenvirt > > +ifeq ($(CONFIG_RTE_LIBRTE_VHOST),y) > +DIRS-$(CONFIG_RTE_LIBRTE_PMD_VHOST) += vhost > +endif # $(CONFIG_RTE_LIBRTE_VHOST) > + > include $(RTE_SDK)/mk/rte.sharelib.mk > include $(RTE_SDK)/mk/rte.subdir.mk > diff --git a/drivers/net/vhost/Makefile b/drivers/net/vhost/Makefile > new file mode 100644 > index 000..018edde > --- /dev/null > +++ b/drivers/net/vhost/Makefile > @@ -0,0 +1,61 @@ > +# BSD LICENSE > +# > +# Copyright (c) 2010-2015 Intel Corporation. > +# All rights reserved. > +# > +# Redistribution and use in source and binary forms, with or without > +# modification, are permitted provided that the following conditions > +# are met: > +# > +# * Redistributions of source code must retain the above copyright > +# notice, this list of conditions and the following disclaimer. > +# * Redistributions in binary form must reproduce the above copyright > +# notice, this list of conditions and the following disclaimer in > +# the documentation and/or other materials provided with the > +# distribution. > +# * Neither the name of Intel corporation nor the names of its > +# contributors may be used to endorse or promote products derived > +# from this software without specific prior written permission. > +# > +# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND > CONTRIBUTORS > +# "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT > NOT > +# LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND > FITNESS FOR > +# A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE > COPYRIGHT > +# OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, > INCIDENTAL, > +# SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT > NOT > +# LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS > OF USE, > +# DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED > AND ON ANY > +# THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR > TORT > +# (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF > THE USE > +# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH > DAMAGE. > + > +include $(RTE_SDK)/mk/rte.vars.mk > + > +# > +# library name > +# > +LIB = librte_pmd_vhost.a > + > +CFLAGS += -O3 > +CFLAGS += $(WERROR_FLAGS) > + > +EXPORT_MAP := rte_pmd_vhost_version.map > + > +LIBABIVER := 1 > + > +# > +# all source are stored in SRCS-y > +# > +SRCS-$(CONFIG_RTE_LIBRTE_PMD_VHOST) += rte_eth_vhost.c > + > +# > +# Export include files > +# > +SYMLINK-y-include += > + > +# this lib depends upon: > +DEPDIRS-$(CONFIG_RTE_LIBRTE_PMD_VHOST) += lib/librte_mbuf > +DEPDIRS-$(CONFIG_RTE_LIBRTE_PMD_VHOST) += lib/librte_ether > +DEPDIRS-$(CONFIG_RTE_LIBRTE_PMD_VHOST) += lib/librte_kvargs > + > +include $(RTE_SDK)/mk/rte.lib.mk > diff --git a/drivers/net/vhost/rte_eth_vhost.c > b/drivers/net/vhost/rte_eth_vhost.c > new file mode 100644 > index 000..679e893 > --- /dev/null > +++ b/drivers/net/vhost/rte_eth_vhost.c > @@ -0,0 +1,640 @@
[dpdk-dev] [RFC PATCH v2] vhost: Add VHOST PMD
The patch introduces a new PMD. This PMD is implemented as thin wrapper of librte_vhost. It means librte_vhost is also needed to compile the PMD. The PMD can have 'iface' parameter like below to specify a path to connect to a virtio-net device. $ ./testpmd -c f -n 4 --vdev 'eth_vhost0,iface=/tmp/sock0' -- -i To connect above testpmd, here is qemu command example. $ qemu-system-x86_64 \ -chardev socket,id=chr0,path=/tmp/sock0 \ -netdev vhost-user,id=net0,chardev=chr0,vhostforce \ -device virtio-net-pci,netdev=net0 Signed-off-by: Tetsuya Mukawa --- config/common_linuxapp | 6 + drivers/net/Makefile| 4 + drivers/net/vhost/Makefile | 61 +++ drivers/net/vhost/rte_eth_vhost.c | 640 drivers/net/vhost/rte_pmd_vhost_version.map | 4 + mk/rte.app.mk | 8 +- 6 files changed, 722 insertions(+), 1 deletion(-) create mode 100644 drivers/net/vhost/Makefile create mode 100644 drivers/net/vhost/rte_eth_vhost.c create mode 100644 drivers/net/vhost/rte_pmd_vhost_version.map diff --git a/config/common_linuxapp b/config/common_linuxapp index 0de43d5..7310240 100644 --- a/config/common_linuxapp +++ b/config/common_linuxapp @@ -446,6 +446,12 @@ CONFIG_RTE_LIBRTE_VHOST_NUMA=n CONFIG_RTE_LIBRTE_VHOST_DEBUG=n # +# Compile vhost PMD +# To compile, CONFIG_RTE_LIBRTE_VHOST should be enabled. +# +CONFIG_RTE_LIBRTE_PMD_VHOST=y + +# #Compile Xen domain0 support # CONFIG_RTE_LIBRTE_XEN_DOM0=n diff --git a/drivers/net/Makefile b/drivers/net/Makefile index 5ebf963..e46a38e 100644 --- a/drivers/net/Makefile +++ b/drivers/net/Makefile @@ -49,5 +49,9 @@ DIRS-$(CONFIG_RTE_LIBRTE_VIRTIO_PMD) += virtio DIRS-$(CONFIG_RTE_LIBRTE_VMXNET3_PMD) += vmxnet3 DIRS-$(CONFIG_RTE_LIBRTE_PMD_XENVIRT) += xenvirt +ifeq ($(CONFIG_RTE_LIBRTE_VHOST),y) +DIRS-$(CONFIG_RTE_LIBRTE_PMD_VHOST) += vhost +endif # $(CONFIG_RTE_LIBRTE_VHOST) + include $(RTE_SDK)/mk/rte.sharelib.mk include $(RTE_SDK)/mk/rte.subdir.mk diff --git a/drivers/net/vhost/Makefile b/drivers/net/vhost/Makefile new file mode 100644 index 000..018edde --- /dev/null +++ b/drivers/net/vhost/Makefile @@ -0,0 +1,61 @@ +# BSD LICENSE +# +# Copyright (c) 2010-2015 Intel Corporation. +# All rights reserved. +# +# Redistribution and use in source and binary forms, with or without +# modification, are permitted provided that the following conditions +# are met: +# +# * Redistributions of source code must retain the above copyright +# notice, this list of conditions and the following disclaimer. +# * Redistributions in binary form must reproduce the above copyright +# notice, this list of conditions and the following disclaimer in +# the documentation and/or other materials provided with the +# distribution. +# * Neither the name of Intel corporation nor the names of its +# contributors may be used to endorse or promote products derived +# from this software without specific prior written permission. +# +# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS +# "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT +# LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR +# A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT +# OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +# SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT +# LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, +# DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY +# THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT +# (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + +include $(RTE_SDK)/mk/rte.vars.mk + +# +# library name +# +LIB = librte_pmd_vhost.a + +CFLAGS += -O3 +CFLAGS += $(WERROR_FLAGS) + +EXPORT_MAP := rte_pmd_vhost_version.map + +LIBABIVER := 1 + +# +# all source are stored in SRCS-y +# +SRCS-$(CONFIG_RTE_LIBRTE_PMD_VHOST) += rte_eth_vhost.c + +# +# Export include files +# +SYMLINK-y-include += + +# this lib depends upon: +DEPDIRS-$(CONFIG_RTE_LIBRTE_PMD_VHOST) += lib/librte_mbuf +DEPDIRS-$(CONFIG_RTE_LIBRTE_PMD_VHOST) += lib/librte_ether +DEPDIRS-$(CONFIG_RTE_LIBRTE_PMD_VHOST) += lib/librte_kvargs + +include $(RTE_SDK)/mk/rte.lib.mk diff --git a/drivers/net/vhost/rte_eth_vhost.c b/drivers/net/vhost/rte_eth_vhost.c new file mode 100644 index 000..679e893 --- /dev/null +++ b/drivers/net/vhost/rte_eth_vhost.c @@ -0,0 +1,640 @@ +/*- + * BSD LICENSE + * + * Copyright (c) 2010-2015 Intel Corporation. + * All rights reserved. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions + * are met: + * + * * Redistr
