Re: [SPDK] SPDK virtio-vhost-user series

Re: [SPDK] SPDK virtio-vhost-user series

[Nikos Dragazis]

[-- Attachment #1: Type: text/plain, Size: 7521 bytes --]

On 7/3/19 3:13 μ.μ., Stojaczyk, Dariusz wrote:
> Hi Nikos!
>
> I was actually about to write you a status email, but first wanted to
> try to finish up the related development and bring you some good news.
> Apparently we'll have to wait for those good news a bit more :( Please
> see below.
>
>> -----Original Message-----
>> From: SPDK [mailto:spdk-bounces(a)lists.01.org] On Behalf Of Nikos Dragazis
>> Sent: Wednesday, March 6, 2019 9:24 PM
>> To: Stojaczyk, Dariusz <dariusz.stojaczyk(a)intel.com>
>> Cc: Storage Performance Development Kit <spdk(a)lists.01.org>
>> Subject: Re: [SPDK] SPDK virtio-vhost-user series
>>
>> On 18/2/19 10:38 π.μ., Stojaczyk, Dariusz wrote:
>>>> -----Original Message-----
>>>> From: Nikos Dragazis [mailto:ndragazis(a)arrikto.com]
>>>> Sent: Friday, February 15, 2019 1:17 PM
>>>> To: Stojaczyk, Dariusz <dariusz.stojaczyk(a)intel.com>
>>>> Cc: Storage Performance Development Kit <spdk(a)lists.01.org>
>>>> Subject: Re: SPDK virtio-vhost-user series
>>>>
>>>>
>>>> On 12/2/19 11:46 μ.μ., Stojaczyk, Dariusz wrote:
>>>>> Other than that, you might already want to try rebasing on top of that
>>>>> 19.02-test-vhost branch, but the initial ETA for the first SPDK
>>>>> patches integrating it remains the same - the end of February, it is.
>>>> I will give it a try. Should I post them on GerritHub?
>>> Yes, please. Those patches won't get merged to our DPDK fork, but will
>>> allow for easier testing and validation as you'll be able to point SPDK's
>>> DPDK submodule directly at them.
>>>
>>> Thanks!
>>> D.
>>
>> Hi Darek,
>>
>> sorry for the quite delayed response. I hope I didn’t stall you.
>>
>> I just pushed a patchset to the 19.02-test-vhost branch in spdk/dpdk
>> repo on GerritHub that adds support for the virtio-vhost-user transport.
>> SPDK’s rte_vhost and DPDK’s librte_vhost have diverged more that I
>> expected, so it was easier for me to start working on top of Stefan’s
>> branch [1] rather than rely on my rte_vhost patches.
>>
>> I think this patchset is a good starting point. A lot of design issues
>> could and should be reconsidered though. I am awaiting for any review
>> comments of yours.
> I'll do a quick review yet this week.
>
>> How about you? Have you made any progress with the external message
>> handlers?
> About two weeks ago I pushed an initial SPDK patch series enabling
> SPDK to run with upstream rte_vhost. It passed the basic I/O tests on
> my local setup, but was just a PoC and wasn't even capable of running
> through the full vhost test suite on Jenkins. This week I finished it
> up, started to run it through those tests on Jenkins, and now I'm
> struggling with different failures. At the moment it's QEMU that hangs
> indefinitely on boot in our of our test cases. I'm hoping to get it
> all fixed by the end of the week. I think all the DPDK patches are in
> place and we're only missing some storage-specific workarounds within
> those external message handlers.
>
> Generally, the SPDK changes introduce a new configuration option to
> stop using the internal rte_vhost copy. It's just a new, experimental
> feature and I think after I get +1 from Jenkins, the patches will be ready
> to merge as they can't break any existing SPDK use cases. We'll keep this
> entire feature as experimental for SPDK 19.04 and announce it as
> complete probably for SPDK 19.07.
>
> The patch series starts here:
> https://review.gerrithub.io/c/spdk/spdk/+/446082
>
>> Have you opened the discussion about unconditional device
>> interrupts with the DPDK community? Is there anything that I could do to
>> help?
> I haven't started that DPDK discussion yet. I was thinking maybe we
> could get away with rte_vhost_vring_call() just telling us whether it
> did send the interrupt or not, but I didn't check if it's a viable
> solution for all the SPDK cases where we need to send an interrupt.
> Could you maybe try looking into that? I think we have automated tests
> for all the tricky vhost corner cases - including live migration - so
> as long as you get +1 from Jenkins, all the changes are fine.

I will have a look.

>
>> If I get this right, the overall plan for migrating from SPDK's
>> rte_vhost copy to DPDK’s librte_vhost and adding support for the
>> virtio-vhost-user transport goes as follows:
>>
>> 1. write external storage-specific message handlers for vhost-user
>>    messages
> Right. I'm still working on this.
>
>> 2. extend the librte_vhost API to support unconditional vhost device
>>    interrupts
>> 3. possibly refactor, tidy up and push in dpdk-next-virtio the patchset
>>    that adds support for the virtio-vhost-user transport
>> 4. support registering non-2MB aligned virtual addresses in SPDK’s
>>    registration and vtophys maps
>> 5. push another patchset in SPDK that will integrate the
>>    virtio-vhost-user transport with the vhost app
> I think the plan is fine. SPDK could also use some test script to get two
> QEMU instances running and setup virtio-vhost-user between them, but
> the virtio-vhost-user-specific changes in SPDK are actually very small
> so I don't feel a strong need for that test.

I see your point. I guess we can see that again later.

> #4 will take me some time and I can't give you a specific deadline, but
> for the initial virtio-vhost-user version we can use your DPDK workaround,
> do you agree?

Yes. My DPDK workaround will do the job for now.

What do you have in mind about this? I was thinking about adding a third
4KB layer in the memory map and just skipping this layer when we are
playing with 2MB regions. I am pretty sure that this is what the kernel
does for the multi-level page tables.

> Since we're here - how is QEMU aligned to this plan? Are the QEMU
> patches merged?

Currently, I am using Stefan's virtio-vhost-user device code [1]. The
device code is fully functional for our use-cases, but it is not
complete. As Stefan says [2], the whole process has been stalled a year
ago.

I have been looking into the code over the last few days. There are some
things that need to be done in order to follow up with the device spec
[3]. Primarily:

1. support slave guest interrupts in response to master kickfd
   notifications

2. add PCI capabilities for the additional device resources (doorbells,
   notifications, shared memory)

So, there is some work that needs to be done before we try merging it in
upstream QEMU. I am working on it. But I have to admit that I am not
familiar with the development process in QEMU. So, any help of yours
would be great. We could also add the device in spdk/qemu as an
intermediate step. What do you think?

[1] https://github.com/stefanha/qemu/tree/virtio-vhost-user
[2] https://lists.01.org/pipermail/spdk/2018-December/002822.html
[3] https://stefanha.github.io/virtio/vhost-user-slave.html#x1-2830007


Nikos

>
> Thanks!
> D.
>
>> Is this plan right? And if so, what is our current state?
>>
>> Looking forward to your feedback.
>>
>> Best regards,
>> Nikos
>>
>> [1] https://github.com/stefanha/dpdk/tree/virtio-vhost-user
>>
>> _______________________________________________
>> SPDK mailing list
>> SPDK(a)lists.01.org
>> https://lists.01.org/mailman/listinfo/spdk
> _______________________________________________
> SPDK mailing list
> SPDK(a)lists.01.org
> https://lists.01.org/mailman/listinfo/spdk

Re: [SPDK] SPDK virtio-vhost-user series

[Stojaczyk, Dariusz]

[-- Attachment #1: Type: text/plain, Size: 4774 bytes --]

Hi Nikos,

That's good news! I reviewed your SPDK patches - let's continue some of
this discussion on gerrithub.

Thanks!
D.

> -----Original Message-----
> From: Nikos Dragazis [mailto:ndragazis(a)arrikto.com]
> Sent: Tuesday, May 14, 2019 10:53 PM
> To: Stojaczyk, Dariusz <dariusz.stojaczyk(a)intel.com>
> Cc: Storage Performance Development Kit <spdk(a)lists.01.org>
> Subject: Re: [SPDK] SPDK virtio-vhost-user series
> 
> Hi Darek,
> 
> you haven’t heard from me for a while. I am pleased to tell you that I
> have managed to make some progress on our plan for the virtio-vhost-user
> transport integration. Let me give you an overview so that we get
> synced.
> 
> I spent enough time over the last few weeks on the virtio-vhost-user
> QEMU device code. I got myself more familiar with the QEMU code and I
> have made some progress on implementing Stefan’s TODO list [1]. I have
> also made some changes in Stefan’s draft virtio device spec [2] and I
> have submitted a revised version to the virtio-dev mailing list [3].
> Getting the spec approved will aid in getting the code merged into
> qemu:master.
> 
> Concerning SPDK, I have started working on top of your patches. I have
> already submitted a revised patchset on GerritHub for review.
> 
> An overview of what I did on SPDK/DPDK:
> 
> - sync the virtio-vhost-user device driver with the updated QEMU code so
>   that it utilizes the new virtio PCI capabilities of the
>   virtio-vhost-user device
> 
> - use my DPDK hack in order to overcome the 2MB alignment restriction
>   (mapping all PCI BARs into 2MB aligned virtual addresses)
> 
> - override the check for the 2MB alignment of the mmap_size in case of
>   the virtio-vhost-user transport
> 
> - register the virtio-vhost-user device in SPDK’s internal DMA-capable
>   PCI device list (necessary for finding VA-to-PA translations in case
>   of vfio no-IOMMU mode)
> 
> A quick thought about the unconditional vhost device interrupts we were
> talking about:
> 
> It is true that DPDK’s `rte_vhost_vring_call()` uses the event idx
> mechanism in order to support interrupt suppression. However, this is
> not a problem for the SPDK vhost devices, which perform interrupt
> coalescing internally, provided that they do not negotiate the
> VIRTIO_F_EVENT_IDX feature flag during device initialization. This holds
> true for all SPDK vhost devices (`SPDK_VHOST_DISABLED_FEATURES`), so
> there is no reason to worry about it.
> 
> One more thing (quite irrelevant):
> 
> I noticed that you have disabled the VHOST_CONFIG INFO log messages [4].
> I think that one should still be able to re-activate these log messages
> in case he wants to play with the vhost library. I see that one can
> provide custom EAL options to the DPDK runtime with the
> `opts->env_context` field. However, this requires modifying the code.
> So, I thought about giving the end user the ability to activate the
> VHOST_CONFIG INFO and DEBUG messages without having to change the
> code.
> Therefore, I suggest inserting a new command-line option in the vhost
> app. I have submitted a patch on GerritHub for this purpose [5].
> 
> About future plans:
> 
> a quick refresh on the initial end-to-end plan:
> 
> 1. write external storage-specific message handlers for vhost-user
>    messages
> 
> 2. extend the librte_vhost API to support unconditional vhost device
>    interrupts
> 
> 3. refactor, tidy up and push in dpdk:dpdk-next-virtio the patchset that
>    adds support for the virtio-vhost-user transport
> 
> 4. support registering non-2MB aligned virtual addresses in SPDK’s
>    registration and vtophys maps
> 
> 5. push another patchset in SPDK that will integrate the
>    virtio-vhost-user transport with the vhost app
> 
> 6. get the virtio device spec approved
> 
> 7. merge QEMU device code into qemu:master
> 
> I cannot estimate the time interval needed for getting the virtio device
> spec approved and the QEMU device code reviewed and merged. We will
> see
> how it goes. Nevertheless, I do not think this is stalling us, at least
> for now.
> 
> I think the next step is finishing with the patchset for DPDK
> librte_vhost. Please have a look at my patches on GerritHub so that we
> can talk about them.
> 
> Thanks for reading so far. :)
> 
> Best regards,
> Nikos
> 
> [1] http://lists.nongnu.org/archive/html/qemu-devel/2018-
> 01/msg04806.html
> [2] https://stefanha.github.io/virtio/vhost-user-slave.html#x1-2830007
> [3] https://lists.oasis-open.org/archives/virtio-dev/201905/msg00022.html
> [4] https://review.gerrithub.io/c/spdk/spdk/+/371691/
> [5] https://review.gerrithub.io/c/spdk/spdk/+/454476

Re: [SPDK] SPDK virtio-vhost-user series

[Nikos Dragazis]

[-- Attachment #1: Type: text/plain, Size: 4125 bytes --]

Hi Darek,

you haven’t heard from me for a while. I am pleased to tell you that I
have managed to make some progress on our plan for the virtio-vhost-user
transport integration. Let me give you an overview so that we get
synced.

I spent enough time over the last few weeks on the virtio-vhost-user
QEMU device code. I got myself more familiar with the QEMU code and I
have made some progress on implementing Stefan’s TODO list [1]. I have
also made some changes in Stefan’s draft virtio device spec [2] and I
have submitted a revised version to the virtio-dev mailing list [3].
Getting the spec approved will aid in getting the code merged into
qemu:master.

Concerning SPDK, I have started working on top of your patches. I have
already submitted a revised patchset on GerritHub for review.

An overview of what I did on SPDK/DPDK:

- sync the virtio-vhost-user device driver with the updated QEMU code so
  that it utilizes the new virtio PCI capabilities of the
  virtio-vhost-user device

- use my DPDK hack in order to overcome the 2MB alignment restriction
  (mapping all PCI BARs into 2MB aligned virtual addresses)

- override the check for the 2MB alignment of the mmap_size in case of
  the virtio-vhost-user transport

- register the virtio-vhost-user device in SPDK’s internal DMA-capable
  PCI device list (necessary for finding VA-to-PA translations in case
  of vfio no-IOMMU mode)

A quick thought about the unconditional vhost device interrupts we were
talking about:

It is true that DPDK’s `rte_vhost_vring_call()` uses the event idx
mechanism in order to support interrupt suppression. However, this is
not a problem for the SPDK vhost devices, which perform interrupt
coalescing internally, provided that they do not negotiate the
VIRTIO_F_EVENT_IDX feature flag during device initialization. This holds
true for all SPDK vhost devices (`SPDK_VHOST_DISABLED_FEATURES`), so
there is no reason to worry about it.

One more thing (quite irrelevant):

I noticed that you have disabled the VHOST_CONFIG INFO log messages [4].
I think that one should still be able to re-activate these log messages
in case he wants to play with the vhost library. I see that one can
provide custom EAL options to the DPDK runtime with the
`opts->env_context` field. However, this requires modifying the code.
So, I thought about giving the end user the ability to activate the
VHOST_CONFIG INFO and DEBUG messages without having to change the code.
Therefore, I suggest inserting a new command-line option in the vhost
app. I have submitted a patch on GerritHub for this purpose [5].

About future plans:

a quick refresh on the initial end-to-end plan:

1. write external storage-specific message handlers for vhost-user
   messages

2. extend the librte_vhost API to support unconditional vhost device
   interrupts

3. refactor, tidy up and push in dpdk:dpdk-next-virtio the patchset that
   adds support for the virtio-vhost-user transport

4. support registering non-2MB aligned virtual addresses in SPDK’s
   registration and vtophys maps

5. push another patchset in SPDK that will integrate the
   virtio-vhost-user transport with the vhost app

6. get the virtio device spec approved

7. merge QEMU device code into qemu:master

I cannot estimate the time interval needed for getting the virtio device
spec approved and the QEMU device code reviewed and merged. We will see
how it goes. Nevertheless, I do not think this is stalling us, at least
for now.

I think the next step is finishing with the patchset for DPDK
librte_vhost. Please have a look at my patches on GerritHub so that we
can talk about them.

Thanks for reading so far. :)

Best regards,
Nikos

[1] http://lists.nongnu.org/archive/html/qemu-devel/2018-01/msg04806.html
[2] https://stefanha.github.io/virtio/vhost-user-slave.html#x1-2830007
[3] https://lists.oasis-open.org/archives/virtio-dev/201905/msg00022.html
[4] https://review.gerrithub.io/c/spdk/spdk/+/371691/
[5] https://review.gerrithub.io/c/spdk/spdk/+/454476

Re: [SPDK] SPDK virtio-vhost-user series

[Stojaczyk, Dariusz]

[-- Attachment #1: Type: text/plain, Size: 9368 bytes --]

The SPDK patch series utilizing the upstream rte_vhost is ready! Top here:
https://review.gerrithub.io/c/spdk/spdk/+/447250

Please see the answers below.

> -----Original Message-----
> From: SPDK [mailto:spdk-bounces(a)lists.01.org] On Behalf Of Nikos Dragazis
> Sent: Monday, March 11, 2019 2:40 PM
> To: spdk(a)lists.01.org
> Subject: Re: [SPDK] SPDK virtio-vhost-user series
> 
> On 7/3/19 3:13 μ.μ., Stojaczyk, Dariusz wrote:
> > Hi Nikos!
> >
> > I was actually about to write you a status email, but first wanted to
> > try to finish up the related development and bring you some good news.
> > Apparently we'll have to wait for those good news a bit more :( Please
> > see below.
> >
> >> -----Original Message-----
> >> From: SPDK [mailto:spdk-bounces(a)lists.01.org] On Behalf Of Nikos
> Dragazis
> >> Sent: Wednesday, March 6, 2019 9:24 PM
> >> To: Stojaczyk, Dariusz <dariusz.stojaczyk(a)intel.com>
> >> Cc: Storage Performance Development Kit <spdk(a)lists.01.org>
> >> Subject: Re: [SPDK] SPDK virtio-vhost-user series
> >>
> >> On 18/2/19 10:38 π.μ., Stojaczyk, Dariusz wrote:
> >>>> -----Original Message-----
> >>>> From: Nikos Dragazis [mailto:ndragazis(a)arrikto.com]
> >>>> Sent: Friday, February 15, 2019 1:17 PM
> >>>> To: Stojaczyk, Dariusz <dariusz.stojaczyk(a)intel.com>
> >>>> Cc: Storage Performance Development Kit <spdk(a)lists.01.org>
> >>>> Subject: Re: SPDK virtio-vhost-user series
> >>>>
> >>>>
> >>>> On 12/2/19 11:46 μ.μ., Stojaczyk, Dariusz wrote:
> >>>>> Other than that, you might already want to try rebasing on top of that
> >>>>> 19.02-test-vhost branch, but the initial ETA for the first SPDK
> >>>>> patches integrating it remains the same - the end of February, it is.
> >>>> I will give it a try. Should I post them on GerritHub?
> >>> Yes, please. Those patches won't get merged to our DPDK fork, but will
> >>> allow for easier testing and validation as you'll be able to point SPDK's
> >>> DPDK submodule directly at them.
> >>>
> >>> Thanks!
> >>> D.
> >>
> >> Hi Darek,
> >>
> >> sorry for the quite delayed response. I hope I didn’t stall you.
> >>
> >> I just pushed a patchset to the 19.02-test-vhost branch in spdk/dpdk
> >> repo on GerritHub that adds support for the virtio-vhost-user transport.
> >> SPDK’s rte_vhost and DPDK’s librte_vhost have diverged more that I
> >> expected, so it was easier for me to start working on top of Stefan’s
> >> branch [1] rather than rely on my rte_vhost patches.
> >>
> >> I think this patchset is a good starting point. A lot of design issues
> >> could and should be reconsidered though. I am awaiting for any review
> >> comments of yours.
> > I'll do a quick review yet this week.
> >
> >> How about you? Have you made any progress with the external message
> >> handlers?
> > About two weeks ago I pushed an initial SPDK patch series enabling
> > SPDK to run with upstream rte_vhost. It passed the basic I/O tests on
> > my local setup, but was just a PoC and wasn't even capable of running
> > through the full vhost test suite on Jenkins. This week I finished it
> > up, started to run it through those tests on Jenkins, and now I'm
> > struggling with different failures. At the moment it's QEMU that hangs
> > indefinitely on boot in our of our test cases. I'm hoping to get it
> > all fixed by the end of the week. I think all the DPDK patches are in
> > place and we're only missing some storage-specific workarounds within
> > those external message handlers.
> >
> > Generally, the SPDK changes introduce a new configuration option to
> > stop using the internal rte_vhost copy. It's just a new, experimental
> > feature and I think after I get +1 from Jenkins, the patches will be ready
> > to merge as they can't break any existing SPDK use cases. We'll keep this
> > entire feature as experimental for SPDK 19.04 and announce it as
> > complete probably for SPDK 19.07.
> >
> > The patch series starts here:
> > https://review.gerrithub.io/c/spdk/spdk/+/446082
> >
> >> Have you opened the discussion about unconditional device
> >> interrupts with the DPDK community? Is there anything that I could do to
> >> help?
> > I haven't started that DPDK discussion yet. I was thinking maybe we
> > could get away with rte_vhost_vring_call() just telling us whether it
> > did send the interrupt or not, but I didn't check if it's a viable
> > solution for all the SPDK cases where we need to send an interrupt.
> > Could you maybe try looking into that? I think we have automated tests
> > for all the tricky vhost corner cases - including live migration - so
> > as long as you get +1 from Jenkins, all the changes are fine.
> 
> I will have a look.
> 
> >
> >> If I get this right, the overall plan for migrating from SPDK's
> >> rte_vhost copy to DPDK’s librte_vhost and adding support for the
> >> virtio-vhost-user transport goes as follows:
> >>
> >> 1. write external storage-specific message handlers for vhost-user
> >>    messages
> > Right. I'm still working on this.
> >
> >> 2. extend the librte_vhost API to support unconditional vhost device
> >>    interrupts
> >> 3. possibly refactor, tidy up and push in dpdk-next-virtio the patchset
> >>    that adds support for the virtio-vhost-user transport
> >> 4. support registering non-2MB aligned virtual addresses in SPDK’s
> >>    registration and vtophys maps
> >> 5. push another patchset in SPDK that will integrate the
> >>    virtio-vhost-user transport with the vhost app
> > I think the plan is fine. SPDK could also use some test script to get two
> > QEMU instances running and setup virtio-vhost-user between them, but
> > the virtio-vhost-user-specific changes in SPDK are actually very small
> > so I don't feel a strong need for that test.
> 
> I see your point. I guess we can see that again later.
> 
> > #4 will take me some time and I can't give you a specific deadline, but
> > for the initial virtio-vhost-user version we can use your DPDK workaround,
> > do you agree?
> 
> Yes. My DPDK workaround will do the job for now.
> 
> What do you have in mind about this? I was thinking about adding a third
> 4KB layer in the memory map and just skipping this layer when we are
> playing with 2MB regions. I am pretty sure that this is what the kernel
> does for the multi-level page tables.

I was hoping to somewhat stick to the idea I had back in October -
https://review.gerrithub.io/c/spdk/spdk/+/427816 
Reusing the remaining 62 bits inside the reference count map, that is.

If we were to support just non-2MB aligned PCI resources, then
I'd say that's the way to go.

Another 4KB layer in mem_maps would potentially require some more
effort to implement, but would suit all kinds of non-2MB-aligned memory.

The question is how do we prioritize that generic non-2MB-aligned mem
support and how much effort does it exactly take to implement - there's
still some research to be done first.

> 
> > Since we're here - how is QEMU aligned to this plan? Are the QEMU
> > patches merged?
> 
> Currently, I am using Stefan's virtio-vhost-user device code [1]. The
> device code is fully functional for our use-cases, but it is not
> complete. As Stefan says [2], the whole process has been stalled a year
> ago.
> 
> I have been looking into the code over the last few days. There are some
> things that need to be done in order to follow up with the device spec
> [3]. Primarily:
> 
> 1. support slave guest interrupts in response to master kickfd
>    notifications
> 
> 2. add PCI capabilities for the additional device resources (doorbells,
>    notifications, shared memory)
> 
> So, there is some work that needs to be done before we try merging it in
> upstream QEMU. I am working on it. But I have to admit that I am not
> familiar with the development process in QEMU. So, any help of yours
> would be great. We could also add the device in spdk/qemu as an
> intermediate step. What do you think?

I'm not familiar with QEMU development either. You can certainly push
patches to spdk/qemu, but please note it's currently based on qemu-3.0.0
and its vhost codebase might differ from upstream qemu master. Keeping
all the virtio-vhost-user patches in spdk repositories would be handy, but
I'm not sure if it's worth the extra work.

> 
> [1] https://github.com/stefanha/qemu/tree/virtio-vhost-user
> [2] https://lists.01.org/pipermail/spdk/2018-December/002822.html
> [3] https://stefanha.github.io/virtio/vhost-user-slave.html#x1-2830007
> 
> 
> Nikos
> 
> >
> > Thanks!
> > D.
> >
> >> Is this plan right? And if so, what is our current state?
> >>
> >> Looking forward to your feedback.
> >>
> >> Best regards,
> >> Nikos
> >>
> >> [1] https://github.com/stefanha/dpdk/tree/virtio-vhost-user
> >>
> >> _______________________________________________
> >> SPDK mailing list
> >> SPDK(a)lists.01.org
> >> https://lists.01.org/mailman/listinfo/spdk
> > _______________________________________________
> > SPDK mailing list
> > SPDK(a)lists.01.org
> > https://lists.01.org/mailman/listinfo/spdk
> _______________________________________________
> SPDK mailing list
> SPDK(a)lists.01.org
> https://lists.01.org/mailman/listinfo/spdk

Re: [SPDK] SPDK virtio-vhost-user series

[Stojaczyk, Dariusz]

[-- Attachment #1: Type: text/plain, Size: 6780 bytes --]

Nikos,

Just to give you a quick update - I fixed the issue, stressed the patch
series on Jenkins, verified it's working and now I have to clean up
the patches a bit. I will resubmit the whole series today so it'll start
start making its way towards merge. I'll let you know once it's all
pushed so that you can rebase on top and start testing
virtio-vhost-user.

Thanks,
D. 

> -----Original Message-----
> From: SPDK [mailto:spdk-bounces(a)lists.01.org] On Behalf Of Stojaczyk,
> Dariusz
> Sent: Thursday, March 7, 2019 2:13 PM
> To: Storage Performance Development Kit <spdk(a)lists.01.org>
> Subject: Re: [SPDK] SPDK virtio-vhost-user series
> 
> Hi Nikos!
> 
> I was actually about to write you a status email, but first wanted to
> try to finish up the related development and bring you some good news.
> Apparently we'll have to wait for those good news a bit more :( Please
> see below.
> 
> > -----Original Message-----
> > From: SPDK [mailto:spdk-bounces(a)lists.01.org] On Behalf Of Nikos
> Dragazis
> > Sent: Wednesday, March 6, 2019 9:24 PM
> > To: Stojaczyk, Dariusz <dariusz.stojaczyk(a)intel.com>
> > Cc: Storage Performance Development Kit <spdk(a)lists.01.org>
> > Subject: Re: [SPDK] SPDK virtio-vhost-user series
> >
> > On 18/2/19 10:38 π.μ., Stojaczyk, Dariusz wrote:
> > >> -----Original Message-----
> > >> From: Nikos Dragazis [mailto:ndragazis(a)arrikto.com]
> > >> Sent: Friday, February 15, 2019 1:17 PM
> > >> To: Stojaczyk, Dariusz <dariusz.stojaczyk(a)intel.com>
> > >> Cc: Storage Performance Development Kit <spdk(a)lists.01.org>
> > >> Subject: Re: SPDK virtio-vhost-user series
> > >>
> > >>
> > >> On 12/2/19 11:46 μ.μ., Stojaczyk, Dariusz wrote:
> > >>> Other than that, you might already want to try rebasing on top of that
> > >>> 19.02-test-vhost branch, but the initial ETA for the first SPDK
> > >>> patches integrating it remains the same - the end of February, it is.
> > >> I will give it a try. Should I post them on GerritHub?
> > > Yes, please. Those patches won't get merged to our DPDK fork, but will
> > > allow for easier testing and validation as you'll be able to point SPDK's
> > > DPDK submodule directly at them.
> > >
> > > Thanks!
> > > D.
> >
> >
> > Hi Darek,
> >
> > sorry for the quite delayed response. I hope I didn’t stall you.
> >
> > I just pushed a patchset to the 19.02-test-vhost branch in spdk/dpdk
> > repo on GerritHub that adds support for the virtio-vhost-user transport.
> > SPDK’s rte_vhost and DPDK’s librte_vhost have diverged more that I
> > expected, so it was easier for me to start working on top of Stefan’s
> > branch [1] rather than rely on my rte_vhost patches.
> >
> > I think this patchset is a good starting point. A lot of design issues
> > could and should be reconsidered though. I am awaiting for any review
> > comments of yours.
> 
> I'll do a quick review yet this week.
> 
> >
> > How about you? Have you made any progress with the external message
> > handlers?
> 
> About two weeks ago I pushed an initial SPDK patch series enabling
> SPDK to run with upstream rte_vhost. It passed the basic I/O tests on
> my local setup, but was just a PoC and wasn't even capable of running
> through the full vhost test suite on Jenkins. This week I finished it
> up, started to run it through those tests on Jenkins, and now I'm
> struggling with different failures. At the moment it's QEMU that hangs
> indefinitely on boot in our of our test cases. I'm hoping to get it
> all fixed by the end of the week. I think all the DPDK patches are in
> place and we're only missing some storage-specific workarounds within
> those external message handlers.
> 
> Generally, the SPDK changes introduce a new configuration option to
> stop using the internal rte_vhost copy. It's just a new, experimental
> feature and I think after I get +1 from Jenkins, the patches will be ready
> to merge as they can't break any existing SPDK use cases. We'll keep this
> entire feature as experimental for SPDK 19.04 and announce it as
> complete probably for SPDK 19.07.
> 
> The patch series starts here:
> https://review.gerrithub.io/c/spdk/spdk/+/446082
> 
> > Have you opened the discussion about unconditional device
> > interrupts with the DPDK community? Is there anything that I could do to
> > help?
> 
> I haven't started that DPDK discussion yet. I was thinking maybe we
> could get away with rte_vhost_vring_call() just telling us whether it
> did send the interrupt or not, but I didn't check if it's a viable
> solution for all the SPDK cases where we need to send an interrupt.
> Could you maybe try looking into that? I think we have automated tests
> for all the tricky vhost corner cases - including live migration - so
> as long as you get +1 from Jenkins, all the changes are fine.
> 
> >
> > If I get this right, the overall plan for migrating from SPDK's
> > rte_vhost copy to DPDK’s librte_vhost and adding support for the
> > virtio-vhost-user transport goes as follows:
> >
> > 1. write external storage-specific message handlers for vhost-user
> >    messages
> 
> Right. I'm still working on this.
> 
> > 2. extend the librte_vhost API to support unconditional vhost device
> >    interrupts
> > 3. possibly refactor, tidy up and push in dpdk-next-virtio the patchset
> >    that adds support for the virtio-vhost-user transport
> > 4. support registering non-2MB aligned virtual addresses in SPDK’s
> >    registration and vtophys maps
> > 5. push another patchset in SPDK that will integrate the
> >    virtio-vhost-user transport with the vhost app
> 
> I think the plan is fine. SPDK could also use some test script to get two
> QEMU instances running and setup virtio-vhost-user between them, but
> the virtio-vhost-user-specific changes in SPDK are actually very small
> so I don't feel a strong need for that test.
> 
> #4 will take me some time and I can't give you a specific deadline, but
> for the initial virtio-vhost-user version we can use your DPDK workaround,
> do you agree?
> 
> Since we're here - how is QEMU aligned to this plan? Are the QEMU
> patches merged?
> 
> Thanks!
> D.
> 
> >
> > Is this plan right? And if so, what is our current state?
> >
> > Looking forward to your feedback.
> >
> > Best regards,
> > Nikos
> >
> > [1] https://github.com/stefanha/dpdk/tree/virtio-vhost-user
> >
> > _______________________________________________
> > SPDK mailing list
> > SPDK(a)lists.01.org
> > https://lists.01.org/mailman/listinfo/spdk
> _______________________________________________
> SPDK mailing list
> SPDK(a)lists.01.org
> https://lists.01.org/mailman/listinfo/spdk

Re: [SPDK] SPDK virtio-vhost-user series

[Stojaczyk, Dariusz]

[-- Attachment #1: Type: text/plain, Size: 5708 bytes --]

Hi Nikos!

I was actually about to write you a status email, but first wanted to
try to finish up the related development and bring you some good news.
Apparently we'll have to wait for those good news a bit more :( Please
see below.

> -----Original Message-----
> From: SPDK [mailto:spdk-bounces(a)lists.01.org] On Behalf Of Nikos Dragazis
> Sent: Wednesday, March 6, 2019 9:24 PM
> To: Stojaczyk, Dariusz <dariusz.stojaczyk(a)intel.com>
> Cc: Storage Performance Development Kit <spdk(a)lists.01.org>
> Subject: Re: [SPDK] SPDK virtio-vhost-user series
> 
> On 18/2/19 10:38 π.μ., Stojaczyk, Dariusz wrote:
> >> -----Original Message-----
> >> From: Nikos Dragazis [mailto:ndragazis(a)arrikto.com]
> >> Sent: Friday, February 15, 2019 1:17 PM
> >> To: Stojaczyk, Dariusz <dariusz.stojaczyk(a)intel.com>
> >> Cc: Storage Performance Development Kit <spdk(a)lists.01.org>
> >> Subject: Re: SPDK virtio-vhost-user series
> >>
> >>
> >> On 12/2/19 11:46 μ.μ., Stojaczyk, Dariusz wrote:
> >>> Other than that, you might already want to try rebasing on top of that
> >>> 19.02-test-vhost branch, but the initial ETA for the first SPDK
> >>> patches integrating it remains the same - the end of February, it is.
> >> I will give it a try. Should I post them on GerritHub?
> > Yes, please. Those patches won't get merged to our DPDK fork, but will
> > allow for easier testing and validation as you'll be able to point SPDK's
> > DPDK submodule directly at them.
> >
> > Thanks!
> > D.
> 
> 
> Hi Darek,
> 
> sorry for the quite delayed response. I hope I didn’t stall you.
> 
> I just pushed a patchset to the 19.02-test-vhost branch in spdk/dpdk
> repo on GerritHub that adds support for the virtio-vhost-user transport.
> SPDK’s rte_vhost and DPDK’s librte_vhost have diverged more that I
> expected, so it was easier for me to start working on top of Stefan’s
> branch [1] rather than rely on my rte_vhost patches.
> 
> I think this patchset is a good starting point. A lot of design issues
> could and should be reconsidered though. I am awaiting for any review
> comments of yours.

I'll do a quick review yet this week.

> 
> How about you? Have you made any progress with the external message
> handlers?

About two weeks ago I pushed an initial SPDK patch series enabling
SPDK to run with upstream rte_vhost. It passed the basic I/O tests on
my local setup, but was just a PoC and wasn't even capable of running
through the full vhost test suite on Jenkins. This week I finished it
up, started to run it through those tests on Jenkins, and now I'm
struggling with different failures. At the moment it's QEMU that hangs
indefinitely on boot in our of our test cases. I'm hoping to get it
all fixed by the end of the week. I think all the DPDK patches are in
place and we're only missing some storage-specific workarounds within
those external message handlers.

Generally, the SPDK changes introduce a new configuration option to
stop using the internal rte_vhost copy. It's just a new, experimental
feature and I think after I get +1 from Jenkins, the patches will be ready
to merge as they can't break any existing SPDK use cases. We'll keep this
entire feature as experimental for SPDK 19.04 and announce it as
complete probably for SPDK 19.07.

The patch series starts here:
https://review.gerrithub.io/c/spdk/spdk/+/446082

> Have you opened the discussion about unconditional device
> interrupts with the DPDK community? Is there anything that I could do to
> help?

I haven't started that DPDK discussion yet. I was thinking maybe we
could get away with rte_vhost_vring_call() just telling us whether it
did send the interrupt or not, but I didn't check if it's a viable
solution for all the SPDK cases where we need to send an interrupt.
Could you maybe try looking into that? I think we have automated tests
for all the tricky vhost corner cases - including live migration - so
as long as you get +1 from Jenkins, all the changes are fine.

> 
> If I get this right, the overall plan for migrating from SPDK's
> rte_vhost copy to DPDK’s librte_vhost and adding support for the
> virtio-vhost-user transport goes as follows:
> 
> 1. write external storage-specific message handlers for vhost-user
>    messages

Right. I'm still working on this.

> 2. extend the librte_vhost API to support unconditional vhost device
>    interrupts
> 3. possibly refactor, tidy up and push in dpdk-next-virtio the patchset
>    that adds support for the virtio-vhost-user transport
> 4. support registering non-2MB aligned virtual addresses in SPDK’s
>    registration and vtophys maps
> 5. push another patchset in SPDK that will integrate the
>    virtio-vhost-user transport with the vhost app

I think the plan is fine. SPDK could also use some test script to get two
QEMU instances running and setup virtio-vhost-user between them, but
the virtio-vhost-user-specific changes in SPDK are actually very small
so I don't feel a strong need for that test.

#4 will take me some time and I can't give you a specific deadline, but
for the initial virtio-vhost-user version we can use your DPDK workaround,
do you agree?

Since we're here - how is QEMU aligned to this plan? Are the QEMU
patches merged?

Thanks!
D.

> 
> Is this plan right? And if so, what is our current state?
> 
> Looking forward to your feedback.
> 
> Best regards,
> Nikos
> 
> [1] https://github.com/stefanha/dpdk/tree/virtio-vhost-user
> 
> _______________________________________________
> SPDK mailing list
> SPDK(a)lists.01.org
> https://lists.01.org/mailman/listinfo/spdk

Re: [SPDK] SPDK virtio-vhost-user series

[Nikos Dragazis]

[-- Attachment #1: Type: text/plain, Size: 2754 bytes --]

On 18/2/19 10:38 π.μ., Stojaczyk, Dariusz wrote:
>> -----Original Message-----
>> From: Nikos Dragazis [mailto:ndragazis(a)arrikto.com]
>> Sent: Friday, February 15, 2019 1:17 PM
>> To: Stojaczyk, Dariusz <dariusz.stojaczyk(a)intel.com>
>> Cc: Storage Performance Development Kit <spdk(a)lists.01.org>
>> Subject: Re: SPDK virtio-vhost-user series
>>
>>
>> On 12/2/19 11:46 μ.μ., Stojaczyk, Dariusz wrote:
>>> Other than that, you might already want to try rebasing on top of that
>>> 19.02-test-vhost branch, but the initial ETA for the first SPDK
>>> patches integrating it remains the same - the end of February, it is.
>> I will give it a try. Should I post them on GerritHub?
> Yes, please. Those patches won't get merged to our DPDK fork, but will
> allow for easier testing and validation as you'll be able to point SPDK's
> DPDK submodule directly at them.
>
> Thanks!
> D.


Hi Darek,
                                                                       
sorry for the quite delayed response. I hope I didn’t stall you.

I just pushed a patchset to the 19.02-test-vhost branch in spdk/dpdk
repo on GerritHub that adds support for the virtio-vhost-user transport.
SPDK’s rte_vhost and DPDK’s librte_vhost have diverged more that I
expected, so it was easier for me to start working on top of Stefan’s
branch [1] rather than rely on my rte_vhost patches.

I think this patchset is a good starting point. A lot of design issues
could and should be reconsidered though. I am awaiting for any review
comments of yours.

How about you? Have you made any progress with the external message
handlers? Have you opened the discussion about unconditional device
interrupts with the DPDK community? Is there anything that I could do to
help?

If I get this right, the overall plan for migrating from SPDK's
rte_vhost copy to DPDK’s librte_vhost and adding support for the
virtio-vhost-user transport goes as follows:

1. write external storage-specific message handlers for vhost-user
   messages
2. extend the librte_vhost API to support unconditional vhost device
   interrupts
3. possibly refactor, tidy up and push in dpdk-next-virtio the patchset
   that adds support for the virtio-vhost-user transport
4. support registering non-2MB aligned virtual addresses in SPDK’s
   registration and vtophys maps
5. push another patchset in SPDK that will integrate the
   virtio-vhost-user transport with the vhost app

Is this plan right? And if so, what is our current state?

Looking forward to your feedback.

Best regards,
Nikos

[1] https://github.com/stefanha/dpdk/tree/virtio-vhost-user

Re: [SPDK] SPDK virtio-vhost-user series

[Stojaczyk, Dariusz]

[-- Attachment #1: Type: text/plain, Size: 14595 bytes --]



> -----Original Message-----
> From: Nikos Dragazis [mailto:ndragazis(a)arrikto.com]
> Sent: Friday, February 15, 2019 1:17 PM
> To: Stojaczyk, Dariusz <dariusz.stojaczyk(a)intel.com>
> Cc: Storage Performance Development Kit <spdk(a)lists.01.org>
> Subject: Re: SPDK virtio-vhost-user series
> 
> [switching to a new email account]
> 
> Hi Darek,
> 
> thanks for the heads-up. Some quick thoughts ...
> 
> On 12/2/19 11:46 μ.μ., Stojaczyk, Dariusz wrote:
> > Nikos,
> >
> > The DPDK patch enabling rte_vhost to be used directly by SPDK has
> > landed in dpdk-next-virtio [1]. I have just pushed a branch
> > 19.02-test-vhost [2] to our dpdk fork that's based on it. It also
> > contains a bunch of spdk-specific patches required to run other SPDK
> > tests, so that it will pass our Jenkins CI if we run the entire SPDK
> > autotest against it. I will very likely use that particular branch to
> > develop the SPDK part.
> >
> > The exact patch that enables direct SPDK integration with rte_vhost is
> > "vhost: add external message handling to the API" [1]. It exposes an
> > rte_vhost_extern_callback_register() function used to install
> > additional message parsing handlers that SPDK will use to intercept
> > some vhost-user messages. This means we will still alter the rte_vhost
> > flow, but the code to do that will be in SPDK and won't affect any
> > existing DPDK use cases. It also shouldn't conflict with
> > any virtio-vhost-user changes.
> 
> Yes, it doesn't seem to conflict with virtio-vhost-user. So, I guess
> your next step is to start writing the storage-specific
> message handlers in SPDK. Right?

Right

> 
> > When it comes to updating our internal rte_vhost copy, I think we'll
> > just add rte_vhost_extern_callback_register() as a stub that doesn't
> > do anything at all. Considering that the rte_vhost copy already works,
> > it doesn't really need those hooks. We may face some issues there, but
> > generally that's what I think we should try first.
> >
> > I see one problem with the new rte_vhost though. The function used for
> > interrupting the guest - rte_vhost_vring_call() - internally checks
> > the event idx or the AVAIL_F_NO_INTERRUPT flag. This won't work with
> > SPDK's interrupt coalescing, in which we need to send an interrupt
> > unconditionally, with no regard to guest's preferences.
> 
> Sorry for my unawareness, but where exactly does SPDK send coalesced
> interrupts unconditionally? Are you referring to this point here:
> https://github.com/spdk/spdk/blob/master/lib/vhost/vhost.c#L1116

That's a yet another case for live migration which I'm not entirely familiar
with. I was thinking of https://github.com/spdk/spdk/blob/master/lib/vhost/vhost.c#L326 

> 
> 
> > For Unix
> > domain sockets we can workaround this by using the deprecated callfd
> > field in rte_vhost_vring struct or - once it is finally removed - by
> > intercepting the callfd directly from SET_VRING_CALL vhost-user
> > message. This obviously won't work with virtio-vhost-user. We'll need
> > to either change rte_vhost_vring_call() to call the vring
> > unconditionally or introduce a new function to do that. I can get the
> > discussion about this started on the DPDK mailing list - I'll be sure
> > to CC you.
> 
> I new function sounds like a good choice to me. I am looking forward
> to hearing news.
> 
> 
> > Other than that, you might already want to try rebasing on top of that
> > 19.02-test-vhost branch, but the initial ETA for the first SPDK
> > patches integrating it remains the same - the end of February, it is.
> 
> I will give it a try. Should I post them on GerritHub?

Yes, please. Those patches won't get merged to our DPDK fork, but will
allow for easier testing and validation as you'll be able to point SPDK's
DPDK submodule directly at them.

Thanks!
D.

> 
> Thanks,
> Nikos
> 
> 
> >
> > Thanks,
> > D.
> >
> > [1] http://git.dpdk.org/next/dpdk-next-
> virtio/commit/?id=f6108041714f7bef4bf59d9aaaefbb2efbb27b57
> > [2] https://github.com/spdk/dpdk/tree/19.02-test-vhost
> >
> >> -----Original Message-----
> >> From: SPDK [mailto:spdk-bounces(a)lists.01.org] On Behalf Of Stojaczyk,
> >> Dariusz
> >> Sent: Monday, February 4, 2019 2:19 PM
> >> To: Nikos Dragazis <ndragazis(a)outlook.com.gr>
> >> Cc: Storage Performance Development Kit <spdk(a)lists.01.org>
> >> Subject: Re: [SPDK] SPDK virtio-vhost-user series
> >>
> >> Nikos,
> >>
> >>> -----Original Message-----
> >>> From: Nikos Dragazis [mailto:ndragazis(a)outlook.com.gr]
> >>> Sent: Friday, February 1, 2019 7:09 PM
> >>> Subject: Re: SPDK virtio-vhost-user series
> >>>
> >>> Hi Darek,
> >>>
> >>> thanks for keeping me in the loop. I was completely unaware of your
> >>> effort to sync SPDK rte_vhost with DPDK rte_vhost. I have a couple of
> >>> questions for you.
> >>>
> >>> On 31/1/19 1:09 μ.μ., Stojaczyk, Dariusz wrote:
> >>>> Hi Nikos,
> >>>>
> >>>> I'm SPDK core maintainer responsible for the vhost library.
> >>>>
> >>>> I saw your virtio-vhost-user patch series on gerrithub. I know you've
> >>>> been talking about it on SPDK community meeting over a month ago,
> >>>> although I was on holiday at that time.
> >>>>
> >>>> I wanted to give you some background of what is currently going on
> >>>> around SPDK vhost.
> >>>>
> >>>> SPDK currently keeps an internal copy of DPDK's rte_vhost with a
> >>>> couple of storage specific changes. We have tried to upstream those
> >>>> changes to DPDK, but they were rejected [1].
> >>> Yes, I have noticed a lot of differences. What is the point of
> >>> upstreaming the storage-specific changes to DPDK? I thought DPDK is
> >>> focused on networking.
> >>>
> >>>> Although they were
> >>>> critical to support vhost-scsi or vhost-blk, they also altered how
> >>>> vhost-net operated and that was DPDK's major concern. We kept the
> >>>> internal rte_vhost copy but still haven't decided whether to try to
> >>>> switch to DPDK's version or to completely derive from DPDK and
> >>>> maintain our own vhost library.
> >>> Could you shed some light on this? I would like to know more about the
> >>> community’s thoughts on this subject. What’s the reason that
> discourages
> >>> you from diverging from DPDK’s rte_vhost? Why should those two
> projects
> >>> be related at all (not generally speaking, just in case of vhost)?
> >> We already have DPDK as an dependency and we're trying to use as much
> >> out of it as possible. Having a separate rte_vhost-like library in
> >> SPDK means just extra lines of code to maintain for SPDK maintainers -
> >> we generally don't want that. A lot of rte_vhost code is really
> >> device-agnostic. Right now we're using an outdated version of
> >> rte_vhost and we're already missing out on fairly recent, generic
> >> vhost features like vIOMMU or post-copy migration. Trying to implement
> >> all those features independently in SPDK would be a solid undertaking
> >> (and would probably base on DPDK rte_vhost anyway).
> >>
> >> Virtio-vhost-user is device-agnostic as well. It was initially
> >> designed for networking and has its usages in there, so if it was to
> >> be merged into SPDK only, then DPDK rte_vhost would have to
> >> potentially catch up at some point. Putting it in DPDK in the first
> >> place serves both SPDK and DPDK.
> >>
> >>>> At one point we've also put together a
> >>>> list of rte_vhost issues - one of which was vhost-user specification
> >>>> incompliance that eventually made our vhost-scsi unusable with QEMU
> >>>> 2.12+. The amount of "fixes" that rte_vhost required was huge.
> >>>> Instead, we tried to create a new, even lower level vhost library in
> >>>> DPDK [2].
> >>> I will have a closer look at this.
> >>>
> >>>> The initial API proposal was warmly welcomed [3], but a few
> >>>> months later, after a PoC implementation was ready, the whole library
> >>>> was rejected as well [4]. [One of the concerns the new library would
> >>>> address was creating an abstraction and environment for
> >>>> virtio-vhost-user, but apparently DPDK team didn't find that useful at
> >>>> the time]
> >>>>
> >>>> We still have the rte_vhost copy in SPDK and we still haven't decided
> >>>> on its future strategy, which is why we were so reluctant to reviewing
> >>>> your patches.
> >>> Never mind.
> >>>
> >>>> Just last week we seem to have finally made some progress, as a DPDK
> >>>> patch that would potentially allow SPDK to use DPDK's rte_vhost
> >>>> directly [5] was approved for DPDK 19.05. Around the end of February I
> >>>> believe SPDK will try to stop using its rte_vhost copy and switch to
> >>>> DPDK's rte_vhost with the mentioned patch.
> >>> That is great news.
> >>>
> >>> What exactly do you mean by “switch to DPDK’s rte_vhost”? How are
> you
> >>> planning to use DPDK's rte_vhost? Are you going to export the DPDK
> >>> rte_vhost public API through SPDK env_dpdk? Can you give me a
> roadmap
> >>> about the upcoming patches?
> >> I don't think there's a need to create env wrappers around rte_vhost.
> >> SPDK's rte_vhost already has direct dependencies on external DPDK
> >> components like librte_net, ether, and mbuf, so making lib/vhost in
> >> SPDK depend directly on librte_vhost in DPDK is good enough to me. We
> >> can reconsider once there's someone wanting to use his own vhost-user
> >> implementation.
> >>
> >> We will also need to support DPDK versions < 19.05 and for those we'll
> >> probably backport all the API changes to our rte_vhost copy, so it can
> >> be used just like librte_vhost from DPDK 19.05. This is critical for
> >> systems where DPDK comes from a system package. It's fine if we don't
> >> provide any of the new features within our rte_vhost copy, but we just
> >> can't introduce a sudden requirement of DPDK 19.05+.
> >>
> >> Let me give you an update by the end of the week. I expect to have
> >> more details worked out by then.
> >>
> >>>> After that happens, I would like to ask you to rebase your patches on
> >>>> latest DPDK's rte_vhost and resubmit them to DPDK.
> >>> Sure.
> >>>
> >>>> I can certainly
> >>>> help with upstreaming vfio no-iommu support in SPDK and am even
> >>>> willing to implement registering non-2MB-aligned memory,
> >>> That would be great. I think that those two changes are irrelevant to
> >>> virtio-vhost-user. Let me express my thoughts.
> >>>
> >>> vfio no-IOMMU support makes perfect sense to me and it is quite easy
> to
> >>> support it. I used it a lot in the Storage Appliance VM, especially in
> >>> cases I wanted to have an SPDK vhost-scsi device with a virtio-scsi
> >>> storage backend (terminating in QEMU user space SCSI target) attached
> as
> >>> vhost SCSI LUN. I originally tried to use a vIOMMU, but I think I hit
> >>> into a QEMU bug. I reported it in qemu-devel mailing list here:
> >>>
> >>> http://lists.nongnu.org/archive/html/qemu-devel/2018-
> 10/msg05382.html
> >>>
> >>> but I got no answer.  So, vfio no-IOMMU was the only way for me to
> have
> >>> virtio-scsi storage backends. And I think that DPDK already supports
> >>> vfio no-IOMMU mode.
> >> Yes, DPDK does support vfio no-IOMMU. There are rte_vfio memory
> >> mapping APIs that automatically work with all VFIO modes and we will
> >> likely start using those eventually. For now we have to stick with our
> >> VFIO code though, because we still support DPDK versions which don't
> >> provide rte_vfio. Your patch will need some other means of checking
> >> noiommu (reading sysfs directly?), but otherwise it's good to go.
> >> Let's continue the discussion on gerrithub.
> >>
> >>> Speaking of the 2MB alignment restriction, I know that you are
> >>> working towards this direction:
> >>>
> >>> https://review.gerrithub.io/c/spdk/spdk/+/427816/1
> >>>
> >>> In case of vhost, the vhost shared memory is 2MB-aligned given that the
> >>> VM’s memory is hugepage backed. But is this restriction necessary? I
> >>> think that the whole configuration would still work if the VM’s memory
> >>> was backed by a tmpfs file (normal 4KB pages) given that we use vfio
> >>> with IOMMU support. The vhost memory regions would be mapped by
> the
> >>> SPDK
> >>> vhost target and then registered to vfio as DMA-able memory with
> >>> MAP_DMA
> >>> ioctl. vfio would take care of making this memory DMA-able. This
> >>> basically involves pinning the memory and updating the device's IOVA
> >>> domain to grant access to this memory.
> >>>
> >>> I haven't tried to implement this. I will have a look at the code. If
> >>> you have any pointers on this or you have made any progress, I would
> >>> love to hear.
> >> Agreed on all points. Vhost code would still need minor adjustments to
> >> handle some corner cases with non-2MB aligned memory, but should
> >> generally work fine with it.
> >>
> >>>> but rte_vhost changes belong in DPDK.
> >>> Yes, but what about the rest of my patches in the vhost library? Except
> >>> for inserting the virtio-vhost-user transport in rte_vhost, this new
> >>> transport has to be exported somehow to the end user. Currently, I am
> >>> using a command-line option in the vhost app, but I think the best would
> >>> be to choose the vhost-user transport from the RPC calls for the
> >>> construction of the vhost controller. Anyways, I guess we are going to
> >>> see that later.
> >> Right, SPDK still needs an abstraction for vring call and some work
> >> around socket path handling. I'll post my comments on gerrithub.
> >>
> >> Thanks,
> >> D.
> >>
> >>>> I'm sorry for the previous lack of transparency in this matter.
> >>>>
> >>>> D.
> >>> Thanks again for the above outline. I would appreciate if you could keep
> >>> me in the loop for any progress with rte_vhost. In the meantime, I am
> >>> looking forward to any comments on the vfio no-IOMMU related
> patches.
> >>>
> >>> Nikos
> >>>
> >>>>
> >>>> [1] https://www.mail-archive.com/dev(a)dpdk.org/msg91788.html
> >>>> [2] https://www.mail-archive.com/dev(a)dpdk.org/msg101943.html
> >>>> [3] https://www.mail-archive.com/dev(a)dpdk.org/msg102042.html
> >>>> [4] https://www.mail-archive.com/dev(a)dpdk.org/msg104886.html
> >>>> [5] http://patches.dpdk.org/patch/49921/
> >> _______________________________________________
> >> SPDK mailing list
> >> SPDK(a)lists.01.org
> >> https://lists.01.org/mailman/listinfo/spdk

Re: [SPDK] SPDK virtio-vhost-user series

[Nikos Dragazis]

[-- Attachment #1: Type: text/plain, Size: 13313 bytes --]

[switching to a new email account]

Hi Darek,

thanks for the heads-up. Some quick thoughts ...

On 12/2/19 11:46 μ.μ., Stojaczyk, Dariusz wrote:
> Nikos,
>
> The DPDK patch enabling rte_vhost to be used directly by SPDK has
> landed in dpdk-next-virtio [1]. I have just pushed a branch
> 19.02-test-vhost [2] to our dpdk fork that's based on it. It also
> contains a bunch of spdk-specific patches required to run other SPDK
> tests, so that it will pass our Jenkins CI if we run the entire SPDK
> autotest against it. I will very likely use that particular branch to
> develop the SPDK part.
>
> The exact patch that enables direct SPDK integration with rte_vhost is
> "vhost: add external message handling to the API" [1]. It exposes an
> rte_vhost_extern_callback_register() function used to install
> additional message parsing handlers that SPDK will use to intercept
> some vhost-user messages. This means we will still alter the rte_vhost
> flow, but the code to do that will be in SPDK and won't affect any
> existing DPDK use cases. It also shouldn't conflict with
> any virtio-vhost-user changes.

Yes, it doesn't seem to conflict with virtio-vhost-user. So, I guess
your next step is to start writing the storage-specific
message handlers in SPDK. Right?

> When it comes to updating our internal rte_vhost copy, I think we'll
> just add rte_vhost_extern_callback_register() as a stub that doesn't
> do anything at all. Considering that the rte_vhost copy already works,
> it doesn't really need those hooks. We may face some issues there, but
> generally that's what I think we should try first.
>
> I see one problem with the new rte_vhost though. The function used for
> interrupting the guest - rte_vhost_vring_call() - internally checks
> the event idx or the AVAIL_F_NO_INTERRUPT flag. This won't work with
> SPDK's interrupt coalescing, in which we need to send an interrupt
> unconditionally, with no regard to guest's preferences. 

Sorry for my unawareness, but where exactly does SPDK send coalesced
interrupts unconditionally? Are you referring to this point here:
https://github.com/spdk/spdk/blob/master/lib/vhost/vhost.c#L1116


> For Unix
> domain sockets we can workaround this by using the deprecated callfd
> field in rte_vhost_vring struct or - once it is finally removed - by
> intercepting the callfd directly from SET_VRING_CALL vhost-user
> message. This obviously won't work with virtio-vhost-user. We'll need
> to either change rte_vhost_vring_call() to call the vring
> unconditionally or introduce a new function to do that. I can get the
> discussion about this started on the DPDK mailing list - I'll be sure
> to CC you.

I new function sounds like a good choice to me. I am looking forward
to hearing news.


> Other than that, you might already want to try rebasing on top of that
> 19.02-test-vhost branch, but the initial ETA for the first SPDK
> patches integrating it remains the same - the end of February, it is.

I will give it a try. Should I post them on GerritHub?

Thanks,
Nikos


>
> Thanks,
> D.
>
> [1] http://git.dpdk.org/next/dpdk-next-virtio/commit/?id=f6108041714f7bef4bf59d9aaaefbb2efbb27b57
> [2] https://github.com/spdk/dpdk/tree/19.02-test-vhost
>
>> -----Original Message-----
>> From: SPDK [mailto:spdk-bounces(a)lists.01.org] On Behalf Of Stojaczyk,
>> Dariusz
>> Sent: Monday, February 4, 2019 2:19 PM
>> To: Nikos Dragazis <ndragazis(a)outlook.com.gr>
>> Cc: Storage Performance Development Kit <spdk(a)lists.01.org>
>> Subject: Re: [SPDK] SPDK virtio-vhost-user series
>>
>> Nikos,
>>
>>> -----Original Message-----
>>> From: Nikos Dragazis [mailto:ndragazis(a)outlook.com.gr]
>>> Sent: Friday, February 1, 2019 7:09 PM
>>> Subject: Re: SPDK virtio-vhost-user series
>>>
>>> Hi Darek,
>>>
>>> thanks for keeping me in the loop. I was completely unaware of your
>>> effort to sync SPDK rte_vhost with DPDK rte_vhost. I have a couple of
>>> questions for you.
>>>
>>> On 31/1/19 1:09 μ.μ., Stojaczyk, Dariusz wrote:
>>>> Hi Nikos,
>>>>
>>>> I'm SPDK core maintainer responsible for the vhost library.
>>>>
>>>> I saw your virtio-vhost-user patch series on gerrithub. I know you've
>>>> been talking about it on SPDK community meeting over a month ago,
>>>> although I was on holiday at that time.
>>>>
>>>> I wanted to give you some background of what is currently going on
>>>> around SPDK vhost.
>>>>
>>>> SPDK currently keeps an internal copy of DPDK's rte_vhost with a
>>>> couple of storage specific changes. We have tried to upstream those
>>>> changes to DPDK, but they were rejected [1].
>>> Yes, I have noticed a lot of differences. What is the point of
>>> upstreaming the storage-specific changes to DPDK? I thought DPDK is
>>> focused on networking.
>>>
>>>> Although they were
>>>> critical to support vhost-scsi or vhost-blk, they also altered how
>>>> vhost-net operated and that was DPDK's major concern. We kept the
>>>> internal rte_vhost copy but still haven't decided whether to try to
>>>> switch to DPDK's version or to completely derive from DPDK and
>>>> maintain our own vhost library.
>>> Could you shed some light on this? I would like to know more about the
>>> community’s thoughts on this subject. What’s the reason that discourages
>>> you from diverging from DPDK’s rte_vhost? Why should those two projects
>>> be related at all (not generally speaking, just in case of vhost)?
>> We already have DPDK as an dependency and we're trying to use as much
>> out of it as possible. Having a separate rte_vhost-like library in
>> SPDK means just extra lines of code to maintain for SPDK maintainers -
>> we generally don't want that. A lot of rte_vhost code is really
>> device-agnostic. Right now we're using an outdated version of
>> rte_vhost and we're already missing out on fairly recent, generic
>> vhost features like vIOMMU or post-copy migration. Trying to implement
>> all those features independently in SPDK would be a solid undertaking
>> (and would probably base on DPDK rte_vhost anyway).
>>
>> Virtio-vhost-user is device-agnostic as well. It was initially
>> designed for networking and has its usages in there, so if it was to
>> be merged into SPDK only, then DPDK rte_vhost would have to
>> potentially catch up at some point. Putting it in DPDK in the first
>> place serves both SPDK and DPDK.
>>
>>>> At one point we've also put together a
>>>> list of rte_vhost issues - one of which was vhost-user specification
>>>> incompliance that eventually made our vhost-scsi unusable with QEMU
>>>> 2.12+. The amount of "fixes" that rte_vhost required was huge.
>>>> Instead, we tried to create a new, even lower level vhost library in
>>>> DPDK [2].
>>> I will have a closer look at this.
>>>
>>>> The initial API proposal was warmly welcomed [3], but a few
>>>> months later, after a PoC implementation was ready, the whole library
>>>> was rejected as well [4]. [One of the concerns the new library would
>>>> address was creating an abstraction and environment for
>>>> virtio-vhost-user, but apparently DPDK team didn't find that useful at
>>>> the time]
>>>>
>>>> We still have the rte_vhost copy in SPDK and we still haven't decided
>>>> on its future strategy, which is why we were so reluctant to reviewing
>>>> your patches.
>>> Never mind.
>>>
>>>> Just last week we seem to have finally made some progress, as a DPDK
>>>> patch that would potentially allow SPDK to use DPDK's rte_vhost
>>>> directly [5] was approved for DPDK 19.05. Around the end of February I
>>>> believe SPDK will try to stop using its rte_vhost copy and switch to
>>>> DPDK's rte_vhost with the mentioned patch.
>>> That is great news.
>>>
>>> What exactly do you mean by “switch to DPDK’s rte_vhost”? How are you
>>> planning to use DPDK's rte_vhost? Are you going to export the DPDK
>>> rte_vhost public API through SPDK env_dpdk? Can you give me a roadmap
>>> about the upcoming patches?
>> I don't think there's a need to create env wrappers around rte_vhost.
>> SPDK's rte_vhost already has direct dependencies on external DPDK
>> components like librte_net, ether, and mbuf, so making lib/vhost in
>> SPDK depend directly on librte_vhost in DPDK is good enough to me. We
>> can reconsider once there's someone wanting to use his own vhost-user
>> implementation.
>>
>> We will also need to support DPDK versions < 19.05 and for those we'll
>> probably backport all the API changes to our rte_vhost copy, so it can
>> be used just like librte_vhost from DPDK 19.05. This is critical for
>> systems where DPDK comes from a system package. It's fine if we don't
>> provide any of the new features within our rte_vhost copy, but we just
>> can't introduce a sudden requirement of DPDK 19.05+.
>>
>> Let me give you an update by the end of the week. I expect to have
>> more details worked out by then.
>>
>>>> After that happens, I would like to ask you to rebase your patches on
>>>> latest DPDK's rte_vhost and resubmit them to DPDK.
>>> Sure.
>>>
>>>> I can certainly
>>>> help with upstreaming vfio no-iommu support in SPDK and am even
>>>> willing to implement registering non-2MB-aligned memory,
>>> That would be great. I think that those two changes are irrelevant to
>>> virtio-vhost-user. Let me express my thoughts.
>>>
>>> vfio no-IOMMU support makes perfect sense to me and it is quite easy to
>>> support it. I used it a lot in the Storage Appliance VM, especially in
>>> cases I wanted to have an SPDK vhost-scsi device with a virtio-scsi
>>> storage backend (terminating in QEMU user space SCSI target) attached as
>>> vhost SCSI LUN. I originally tried to use a vIOMMU, but I think I hit
>>> into a QEMU bug. I reported it in qemu-devel mailing list here:
>>>
>>> http://lists.nongnu.org/archive/html/qemu-devel/2018-10/msg05382.html
>>>
>>> but I got no answer.  So, vfio no-IOMMU was the only way for me to have
>>> virtio-scsi storage backends. And I think that DPDK already supports
>>> vfio no-IOMMU mode.
>> Yes, DPDK does support vfio no-IOMMU. There are rte_vfio memory
>> mapping APIs that automatically work with all VFIO modes and we will
>> likely start using those eventually. For now we have to stick with our
>> VFIO code though, because we still support DPDK versions which don't
>> provide rte_vfio. Your patch will need some other means of checking
>> noiommu (reading sysfs directly?), but otherwise it's good to go.
>> Let's continue the discussion on gerrithub.
>>
>>> Speaking of the 2MB alignment restriction, I know that you are
>>> working towards this direction:
>>>
>>> https://review.gerrithub.io/c/spdk/spdk/+/427816/1
>>>
>>> In case of vhost, the vhost shared memory is 2MB-aligned given that the
>>> VM’s memory is hugepage backed. But is this restriction necessary? I
>>> think that the whole configuration would still work if the VM’s memory
>>> was backed by a tmpfs file (normal 4KB pages) given that we use vfio
>>> with IOMMU support. The vhost memory regions would be mapped by the
>>> SPDK
>>> vhost target and then registered to vfio as DMA-able memory with
>>> MAP_DMA
>>> ioctl. vfio would take care of making this memory DMA-able. This
>>> basically involves pinning the memory and updating the device's IOVA
>>> domain to grant access to this memory.
>>>
>>> I haven't tried to implement this. I will have a look at the code. If
>>> you have any pointers on this or you have made any progress, I would
>>> love to hear.
>> Agreed on all points. Vhost code would still need minor adjustments to
>> handle some corner cases with non-2MB aligned memory, but should
>> generally work fine with it.
>>
>>>> but rte_vhost changes belong in DPDK.
>>> Yes, but what about the rest of my patches in the vhost library? Except
>>> for inserting the virtio-vhost-user transport in rte_vhost, this new
>>> transport has to be exported somehow to the end user. Currently, I am
>>> using a command-line option in the vhost app, but I think the best would
>>> be to choose the vhost-user transport from the RPC calls for the
>>> construction of the vhost controller. Anyways, I guess we are going to
>>> see that later.
>> Right, SPDK still needs an abstraction for vring call and some work
>> around socket path handling. I'll post my comments on gerrithub.
>>
>> Thanks,
>> D.
>>
>>>> I'm sorry for the previous lack of transparency in this matter.
>>>>
>>>> D.
>>> Thanks again for the above outline. I would appreciate if you could keep
>>> me in the loop for any progress with rte_vhost. In the meantime, I am
>>> looking forward to any comments on the vfio no-IOMMU related patches.
>>>
>>> Nikos
>>>
>>>>
>>>> [1] https://www.mail-archive.com/dev(a)dpdk.org/msg91788.html
>>>> [2] https://www.mail-archive.com/dev(a)dpdk.org/msg101943.html
>>>> [3] https://www.mail-archive.com/dev(a)dpdk.org/msg102042.html
>>>> [4] https://www.mail-archive.com/dev(a)dpdk.org/msg104886.html
>>>> [5] http://patches.dpdk.org/patch/49921/
>> _______________________________________________
>> SPDK mailing list
>> SPDK(a)lists.01.org
>> https://lists.01.org/mailman/listinfo/spdk

Re: [SPDK] SPDK virtio-vhost-user series

[Stojaczyk, Dariusz]

[-- Attachment #1: Type: text/plain, Size: 12646 bytes --]

Nikos,

The DPDK patch enabling rte_vhost to be used directly by SPDK has
landed in dpdk-next-virtio [1]. I have just pushed a branch
19.02-test-vhost [2] to our dpdk fork that's based on it. It also
contains a bunch of spdk-specific patches required to run other SPDK
tests, so that it will pass our Jenkins CI if we run the entire SPDK
autotest against it. I will very likely use that particular branch to
develop the SPDK part.

The exact patch that enables direct SPDK integration with rte_vhost is
"vhost: add external message handling to the API" [1]. It exposes an
rte_vhost_extern_callback_register() function used to install
additional message parsing handlers that SPDK will use to intercept
some vhost-user messages. This means we will still alter the rte_vhost
flow, but the code to do that will be in SPDK and won't affect any
existing DPDK use cases. It also shouldn't conflict with
any virtio-vhost-user changes.

When it comes to updating our internal rte_vhost copy, I think we'll
just add rte_vhost_extern_callback_register() as a stub that doesn't
do anything at all. Considering that the rte_vhost copy already works,
it doesn't really need those hooks. We may face some issues there, but
generally that's what I think we should try first.

I see one problem with the new rte_vhost though. The function used for
interrupting the guest - rte_vhost_vring_call() - internally checks
the event idx or the AVAIL_F_NO_INTERRUPT flag. This won't work with
SPDK's interrupt coalescing, in which we need to send an interrupt
unconditionally, with no regard to guest's preferences. For Unix
domain sockets we can workaround this by using the deprecated callfd
field in rte_vhost_vring struct or - once it is finally removed - by
intercepting the callfd directly from SET_VRING_CALL vhost-user
message. This obviously won't work with virtio-vhost-user. We'll need
to either change rte_vhost_vring_call() to call the vring
unconditionally or introduce a new function to do that. I can get the
discussion about this started on the DPDK mailing list - I'll be sure
to CC you.

Other than that, you might already want to try rebasing on top of that
19.02-test-vhost branch, but the initial ETA for the first SPDK
patches integrating it remains the same - the end of February, it is.

Thanks,
D.

[1] http://git.dpdk.org/next/dpdk-next-virtio/commit/?id=f6108041714f7bef4bf59d9aaaefbb2efbb27b57
[2] https://github.com/spdk/dpdk/tree/19.02-test-vhost

> -----Original Message-----
> From: SPDK [mailto:spdk-bounces(a)lists.01.org] On Behalf Of Stojaczyk,
> Dariusz
> Sent: Monday, February 4, 2019 2:19 PM
> To: Nikos Dragazis <ndragazis(a)outlook.com.gr>
> Cc: Storage Performance Development Kit <spdk(a)lists.01.org>
> Subject: Re: [SPDK] SPDK virtio-vhost-user series
> 
> Nikos,
> 
> > -----Original Message-----
> > From: Nikos Dragazis [mailto:ndragazis(a)outlook.com.gr]
> > Sent: Friday, February 1, 2019 7:09 PM
> > Subject: Re: SPDK virtio-vhost-user series
> >
> > Hi Darek,
> >
> > thanks for keeping me in the loop. I was completely unaware of your
> > effort to sync SPDK rte_vhost with DPDK rte_vhost. I have a couple of
> > questions for you.
> >
> > On 31/1/19 1:09 μ.μ., Stojaczyk, Dariusz wrote:
> > > Hi Nikos,
> > >
> > > I'm SPDK core maintainer responsible for the vhost library.
> > >
> > > I saw your virtio-vhost-user patch series on gerrithub. I know you've
> > > been talking about it on SPDK community meeting over a month ago,
> > > although I was on holiday at that time.
> > >
> > > I wanted to give you some background of what is currently going on
> > > around SPDK vhost.
> > >
> > > SPDK currently keeps an internal copy of DPDK's rte_vhost with a
> > > couple of storage specific changes. We have tried to upstream those
> > > changes to DPDK, but they were rejected [1].
> >
> > Yes, I have noticed a lot of differences. What is the point of
> > upstreaming the storage-specific changes to DPDK? I thought DPDK is
> > focused on networking.
> >
> > > Although they were
> > > critical to support vhost-scsi or vhost-blk, they also altered how
> > > vhost-net operated and that was DPDK's major concern. We kept the
> > > internal rte_vhost copy but still haven't decided whether to try to
> > > switch to DPDK's version or to completely derive from DPDK and
> > > maintain our own vhost library.
> >
> > Could you shed some light on this? I would like to know more about the
> > community’s thoughts on this subject. What’s the reason that discourages
> > you from diverging from DPDK’s rte_vhost? Why should those two projects
> > be related at all (not generally speaking, just in case of vhost)?
> 
> We already have DPDK as an dependency and we're trying to use as much
> out of it as possible. Having a separate rte_vhost-like library in
> SPDK means just extra lines of code to maintain for SPDK maintainers -
> we generally don't want that. A lot of rte_vhost code is really
> device-agnostic. Right now we're using an outdated version of
> rte_vhost and we're already missing out on fairly recent, generic
> vhost features like vIOMMU or post-copy migration. Trying to implement
> all those features independently in SPDK would be a solid undertaking
> (and would probably base on DPDK rte_vhost anyway).
> 
> Virtio-vhost-user is device-agnostic as well. It was initially
> designed for networking and has its usages in there, so if it was to
> be merged into SPDK only, then DPDK rte_vhost would have to
> potentially catch up at some point. Putting it in DPDK in the first
> place serves both SPDK and DPDK.
> 
> >
> > > At one point we've also put together a
> > > list of rte_vhost issues - one of which was vhost-user specification
> > > incompliance that eventually made our vhost-scsi unusable with QEMU
> > > 2.12+. The amount of "fixes" that rte_vhost required was huge.
> > > Instead, we tried to create a new, even lower level vhost library in
> > > DPDK [2].
> >
> > I will have a closer look at this.
> >
> > > The initial API proposal was warmly welcomed [3], but a few
> > > months later, after a PoC implementation was ready, the whole library
> > > was rejected as well [4]. [One of the concerns the new library would
> > > address was creating an abstraction and environment for
> > > virtio-vhost-user, but apparently DPDK team didn't find that useful at
> > > the time]
> > >
> > > We still have the rte_vhost copy in SPDK and we still haven't decided
> > > on its future strategy, which is why we were so reluctant to reviewing
> > > your patches.
> >
> > Never mind.
> >
> > >
> > > Just last week we seem to have finally made some progress, as a DPDK
> > > patch that would potentially allow SPDK to use DPDK's rte_vhost
> > > directly [5] was approved for DPDK 19.05. Around the end of February I
> > > believe SPDK will try to stop using its rte_vhost copy and switch to
> > > DPDK's rte_vhost with the mentioned patch.
> >
> > That is great news.
> >
> > What exactly do you mean by “switch to DPDK’s rte_vhost”? How are you
> > planning to use DPDK's rte_vhost? Are you going to export the DPDK
> > rte_vhost public API through SPDK env_dpdk? Can you give me a roadmap
> > about the upcoming patches?
> 
> I don't think there's a need to create env wrappers around rte_vhost.
> SPDK's rte_vhost already has direct dependencies on external DPDK
> components like librte_net, ether, and mbuf, so making lib/vhost in
> SPDK depend directly on librte_vhost in DPDK is good enough to me. We
> can reconsider once there's someone wanting to use his own vhost-user
> implementation.
> 
> We will also need to support DPDK versions < 19.05 and for those we'll
> probably backport all the API changes to our rte_vhost copy, so it can
> be used just like librte_vhost from DPDK 19.05. This is critical for
> systems where DPDK comes from a system package. It's fine if we don't
> provide any of the new features within our rte_vhost copy, but we just
> can't introduce a sudden requirement of DPDK 19.05+.
> 
> Let me give you an update by the end of the week. I expect to have
> more details worked out by then.
> 
> >
> > >
> > > After that happens, I would like to ask you to rebase your patches on
> > > latest DPDK's rte_vhost and resubmit them to DPDK.
> >
> > Sure.
> >
> > > I can certainly
> > > help with upstreaming vfio no-iommu support in SPDK and am even
> > > willing to implement registering non-2MB-aligned memory,
> >
> > That would be great. I think that those two changes are irrelevant to
> > virtio-vhost-user. Let me express my thoughts.
> >
> > vfio no-IOMMU support makes perfect sense to me and it is quite easy to
> > support it. I used it a lot in the Storage Appliance VM, especially in
> > cases I wanted to have an SPDK vhost-scsi device with a virtio-scsi
> > storage backend (terminating in QEMU user space SCSI target) attached as
> > vhost SCSI LUN. I originally tried to use a vIOMMU, but I think I hit
> > into a QEMU bug. I reported it in qemu-devel mailing list here:
> >
> > http://lists.nongnu.org/archive/html/qemu-devel/2018-10/msg05382.html
> >
> > but I got no answer.  So, vfio no-IOMMU was the only way for me to have
> > virtio-scsi storage backends. And I think that DPDK already supports
> > vfio no-IOMMU mode.
> 
> Yes, DPDK does support vfio no-IOMMU. There are rte_vfio memory
> mapping APIs that automatically work with all VFIO modes and we will
> likely start using those eventually. For now we have to stick with our
> VFIO code though, because we still support DPDK versions which don't
> provide rte_vfio. Your patch will need some other means of checking
> noiommu (reading sysfs directly?), but otherwise it's good to go.
> Let's continue the discussion on gerrithub.
> 
> >
> > Speaking of the 2MB alignment restriction, I know that you are
> > working towards this direction:
> >
> > https://review.gerrithub.io/c/spdk/spdk/+/427816/1
> >
> > In case of vhost, the vhost shared memory is 2MB-aligned given that the
> > VM’s memory is hugepage backed. But is this restriction necessary? I
> > think that the whole configuration would still work if the VM’s memory
> > was backed by a tmpfs file (normal 4KB pages) given that we use vfio
> > with IOMMU support. The vhost memory regions would be mapped by the
> > SPDK
> > vhost target and then registered to vfio as DMA-able memory with
> > MAP_DMA
> > ioctl. vfio would take care of making this memory DMA-able. This
> > basically involves pinning the memory and updating the device's IOVA
> > domain to grant access to this memory.
> >
> > I haven't tried to implement this. I will have a look at the code. If
> > you have any pointers on this or you have made any progress, I would
> > love to hear.
> 
> Agreed on all points. Vhost code would still need minor adjustments to
> handle some corner cases with non-2MB aligned memory, but should
> generally work fine with it.
> 
> >
> > > but rte_vhost changes belong in DPDK.
> >
> > Yes, but what about the rest of my patches in the vhost library? Except
> > for inserting the virtio-vhost-user transport in rte_vhost, this new
> > transport has to be exported somehow to the end user. Currently, I am
> > using a command-line option in the vhost app, but I think the best would
> > be to choose the vhost-user transport from the RPC calls for the
> > construction of the vhost controller. Anyways, I guess we are going to
> > see that later.
> 
> Right, SPDK still needs an abstraction for vring call and some work
> around socket path handling. I'll post my comments on gerrithub.
> 
> Thanks,
> D.
> 
> >
> > >
> > > I'm sorry for the previous lack of transparency in this matter.
> > >
> > > D.
> >
> > Thanks again for the above outline. I would appreciate if you could keep
> > me in the loop for any progress with rte_vhost. In the meantime, I am
> > looking forward to any comments on the vfio no-IOMMU related patches.
> >
> > Nikos
> >
> > >
> > >
> > > [1] https://www.mail-archive.com/dev(a)dpdk.org/msg91788.html
> > > [2] https://www.mail-archive.com/dev(a)dpdk.org/msg101943.html
> > > [3] https://www.mail-archive.com/dev(a)dpdk.org/msg102042.html
> > > [4] https://www.mail-archive.com/dev(a)dpdk.org/msg104886.html
> > > [5] http://patches.dpdk.org/patch/49921/
> 
> _______________________________________________
> SPDK mailing list
> SPDK(a)lists.01.org
> https://lists.01.org/mailman/listinfo/spdk

Re: [SPDK] SPDK virtio-vhost-user series

[Stojaczyk, Dariusz]

[-- Attachment #1: Type: text/plain, Size: 9274 bytes --]

Nikos,

> -----Original Message-----
> From: Nikos Dragazis [mailto:ndragazis(a)outlook.com.gr]
> Sent: Friday, February 1, 2019 7:09 PM
> Subject: Re: SPDK virtio-vhost-user series
> 
> Hi Darek,
> 
> thanks for keeping me in the loop. I was completely unaware of your
> effort to sync SPDK rte_vhost with DPDK rte_vhost. I have a couple of
> questions for you.
> 
> On 31/1/19 1:09 μ.μ., Stojaczyk, Dariusz wrote:
> > Hi Nikos,
> >
> > I'm SPDK core maintainer responsible for the vhost library.
> >
> > I saw your virtio-vhost-user patch series on gerrithub. I know you've
> > been talking about it on SPDK community meeting over a month ago,
> > although I was on holiday at that time.
> >
> > I wanted to give you some background of what is currently going on
> > around SPDK vhost.
> >
> > SPDK currently keeps an internal copy of DPDK's rte_vhost with a
> > couple of storage specific changes. We have tried to upstream those
> > changes to DPDK, but they were rejected [1].
> 
> Yes, I have noticed a lot of differences. What is the point of
> upstreaming the storage-specific changes to DPDK? I thought DPDK is
> focused on networking.
> 
> > Although they were
> > critical to support vhost-scsi or vhost-blk, they also altered how
> > vhost-net operated and that was DPDK's major concern. We kept the
> > internal rte_vhost copy but still haven't decided whether to try to
> > switch to DPDK's version or to completely derive from DPDK and
> > maintain our own vhost library.
> 
> Could you shed some light on this? I would like to know more about the
> community’s thoughts on this subject. What’s the reason that discourages
> you from diverging from DPDK’s rte_vhost? Why should those two projects
> be related at all (not generally speaking, just in case of vhost)?

We already have DPDK as an dependency and we're trying to use as much
out of it as possible. Having a separate rte_vhost-like library in
SPDK means just extra lines of code to maintain for SPDK maintainers -
we generally don't want that. A lot of rte_vhost code is really
device-agnostic. Right now we're using an outdated version of
rte_vhost and we're already missing out on fairly recent, generic
vhost features like vIOMMU or post-copy migration. Trying to implement
all those features independently in SPDK would be a solid undertaking
(and would probably base on DPDK rte_vhost anyway).

Virtio-vhost-user is device-agnostic as well. It was initially
designed for networking and has its usages in there, so if it was to
be merged into SPDK only, then DPDK rte_vhost would have to
potentially catch up at some point. Putting it in DPDK in the first
place serves both SPDK and DPDK.

> 
> > At one point we've also put together a
> > list of rte_vhost issues - one of which was vhost-user specification
> > incompliance that eventually made our vhost-scsi unusable with QEMU
> > 2.12+. The amount of "fixes" that rte_vhost required was huge.
> > Instead, we tried to create a new, even lower level vhost library in
> > DPDK [2].
> 
> I will have a closer look at this.
> 
> > The initial API proposal was warmly welcomed [3], but a few
> > months later, after a PoC implementation was ready, the whole library
> > was rejected as well [4]. [One of the concerns the new library would
> > address was creating an abstraction and environment for
> > virtio-vhost-user, but apparently DPDK team didn't find that useful at
> > the time]
> >
> > We still have the rte_vhost copy in SPDK and we still haven't decided
> > on its future strategy, which is why we were so reluctant to reviewing
> > your patches.
> 
> Never mind.
> 
> >
> > Just last week we seem to have finally made some progress, as a DPDK
> > patch that would potentially allow SPDK to use DPDK's rte_vhost
> > directly [5] was approved for DPDK 19.05. Around the end of February I
> > believe SPDK will try to stop using its rte_vhost copy and switch to
> > DPDK's rte_vhost with the mentioned patch.
> 
> That is great news.
> 
> What exactly do you mean by “switch to DPDK’s rte_vhost”? How are you
> planning to use DPDK's rte_vhost? Are you going to export the DPDK
> rte_vhost public API through SPDK env_dpdk? Can you give me a roadmap
> about the upcoming patches?

I don't think there's a need to create env wrappers around rte_vhost.
SPDK's rte_vhost already has direct dependencies on external DPDK
components like librte_net, ether, and mbuf, so making lib/vhost in
SPDK depend directly on librte_vhost in DPDK is good enough to me. We
can reconsider once there's someone wanting to use his own vhost-user
implementation.

We will also need to support DPDK versions < 19.05 and for those we'll
probably backport all the API changes to our rte_vhost copy, so it can
be used just like librte_vhost from DPDK 19.05. This is critical for
systems where DPDK comes from a system package. It's fine if we don't
provide any of the new features within our rte_vhost copy, but we just
can't introduce a sudden requirement of DPDK 19.05+.

Let me give you an update by the end of the week. I expect to have
more details worked out by then.

> 
> >
> > After that happens, I would like to ask you to rebase your patches on
> > latest DPDK's rte_vhost and resubmit them to DPDK.
> 
> Sure.
> 
> > I can certainly
> > help with upstreaming vfio no-iommu support in SPDK and am even
> > willing to implement registering non-2MB-aligned memory,
> 
> That would be great. I think that those two changes are irrelevant to
> virtio-vhost-user. Let me express my thoughts.
> 
> vfio no-IOMMU support makes perfect sense to me and it is quite easy to
> support it. I used it a lot in the Storage Appliance VM, especially in
> cases I wanted to have an SPDK vhost-scsi device with a virtio-scsi
> storage backend (terminating in QEMU user space SCSI target) attached as
> vhost SCSI LUN. I originally tried to use a vIOMMU, but I think I hit
> into a QEMU bug. I reported it in qemu-devel mailing list here:
> 
> http://lists.nongnu.org/archive/html/qemu-devel/2018-10/msg05382.html
> 
> but I got no answer.  So, vfio no-IOMMU was the only way for me to have
> virtio-scsi storage backends. And I think that DPDK already supports
> vfio no-IOMMU mode.

Yes, DPDK does support vfio no-IOMMU. There are rte_vfio memory
mapping APIs that automatically work with all VFIO modes and we will
likely start using those eventually. For now we have to stick with our
VFIO code though, because we still support DPDK versions which don't
provide rte_vfio. Your patch will need some other means of checking
noiommu (reading sysfs directly?), but otherwise it's good to go.
Let's continue the discussion on gerrithub.

> 
> Speaking of the 2MB alignment restriction, I know that you are
> working towards this direction:
> 
> https://review.gerrithub.io/c/spdk/spdk/+/427816/1
> 
> In case of vhost, the vhost shared memory is 2MB-aligned given that the
> VM’s memory is hugepage backed. But is this restriction necessary? I
> think that the whole configuration would still work if the VM’s memory
> was backed by a tmpfs file (normal 4KB pages) given that we use vfio
> with IOMMU support. The vhost memory regions would be mapped by the
> SPDK
> vhost target and then registered to vfio as DMA-able memory with
> MAP_DMA
> ioctl. vfio would take care of making this memory DMA-able. This
> basically involves pinning the memory and updating the device's IOVA
> domain to grant access to this memory.
> 
> I haven't tried to implement this. I will have a look at the code. If
> you have any pointers on this or you have made any progress, I would
> love to hear.

Agreed on all points. Vhost code would still need minor adjustments to
handle some corner cases with non-2MB aligned memory, but should
generally work fine with it.

> 
> > but rte_vhost changes belong in DPDK.
> 
> Yes, but what about the rest of my patches in the vhost library? Except
> for inserting the virtio-vhost-user transport in rte_vhost, this new
> transport has to be exported somehow to the end user. Currently, I am
> using a command-line option in the vhost app, but I think the best would
> be to choose the vhost-user transport from the RPC calls for the
> construction of the vhost controller. Anyways, I guess we are going to
> see that later.

Right, SPDK still needs an abstraction for vring call and some work
around socket path handling. I'll post my comments on gerrithub.

Thanks,
D.

> 
> >
> > I'm sorry for the previous lack of transparency in this matter.
> >
> > D.
> 
> Thanks again for the above outline. I would appreciate if you could keep
> me in the loop for any progress with rte_vhost. In the meantime, I am
> looking forward to any comments on the vfio no-IOMMU related patches.
> 
> Nikos
> 
> >
> >
> > [1] https://www.mail-archive.com/dev(a)dpdk.org/msg91788.html
> > [2] https://www.mail-archive.com/dev(a)dpdk.org/msg101943.html
> > [3] https://www.mail-archive.com/dev(a)dpdk.org/msg102042.html
> > [4] https://www.mail-archive.com/dev(a)dpdk.org/msg104886.html
> > [5] http://patches.dpdk.org/patch/49921/

Re: [SPDK] SPDK virtio-vhost-user series

[Nikos Dragazis]

[-- Attachment #1: Type: text/plain, Size: 6202 bytes --]

Hi Darek,

thanks for keeping me in the loop. I was completely unaware of your
effort to sync SPDK rte_vhost with DPDK rte_vhost. I have a couple of
questions for you.

On 31/1/19 1:09 μ.μ., Stojaczyk, Dariusz wrote:
> Hi Nikos,
>
> I'm SPDK core maintainer responsible for the vhost library.
>
> I saw your virtio-vhost-user patch series on gerrithub. I know you've
> been talking about it on SPDK community meeting over a month ago,
> although I was on holiday at that time.
>
> I wanted to give you some background of what is currently going on
> around SPDK vhost.
>
> SPDK currently keeps an internal copy of DPDK's rte_vhost with a
> couple of storage specific changes. We have tried to upstream those
> changes to DPDK, but they were rejected [1].

Yes, I have noticed a lot of differences. What is the point of
upstreaming the storage-specific changes to DPDK? I thought DPDK is
focused on networking.

> Although they were
> critical to support vhost-scsi or vhost-blk, they also altered how
> vhost-net operated and that was DPDK's major concern. We kept the
> internal rte_vhost copy but still haven't decided whether to try to
> switch to DPDK's version or to completely derive from DPDK and
> maintain our own vhost library.

Could you shed some light on this? I would like to know more about the
community’s thoughts on this subject. What’s the reason that discourages
you from diverging from DPDK’s rte_vhost? Why should those two projects
be related at all (not generally speaking, just in case of vhost)?

> At one point we've also put together a
> list of rte_vhost issues - one of which was vhost-user specification
> incompliance that eventually made our vhost-scsi unusable with QEMU
> 2.12+. The amount of "fixes" that rte_vhost required was huge.
> Instead, we tried to create a new, even lower level vhost library in
> DPDK [2].

I will have a closer look at this.

> The initial API proposal was warmly welcomed [3], but a few
> months later, after a PoC implementation was ready, the whole library
> was rejected as well [4]. [One of the concerns the new library would
> address was creating an abstraction and environment for
> virtio-vhost-user, but apparently DPDK team didn't find that useful at
> the time]
>
> We still have the rte_vhost copy in SPDK and we still haven't decided
> on its future strategy, which is why we were so reluctant to reviewing
> your patches.

Never mind.

>
> Just last week we seem to have finally made some progress, as a DPDK
> patch that would potentially allow SPDK to use DPDK's rte_vhost
> directly [5] was approved for DPDK 19.05. Around the end of February I
> believe SPDK will try to stop using its rte_vhost copy and switch to
> DPDK's rte_vhost with the mentioned patch.

That is great news.

What exactly do you mean by “switch to DPDK’s rte_vhost”? How are you
planning to use DPDK's rte_vhost? Are you going to export the DPDK
rte_vhost public API through SPDK env_dpdk? Can you give me a roadmap
about the upcoming patches?

>
> After that happens, I would like to ask you to rebase your patches on
> latest DPDK's rte_vhost and resubmit them to DPDK.

Sure.

> I can certainly
> help with upstreaming vfio no-iommu support in SPDK and am even
> willing to implement registering non-2MB-aligned memory,

That would be great. I think that those two changes are irrelevant to
virtio-vhost-user. Let me express my thoughts.

vfio no-IOMMU support makes perfect sense to me and it is quite easy to
support it. I used it a lot in the Storage Appliance VM, especially in
cases I wanted to have an SPDK vhost-scsi device with a virtio-scsi
storage backend (terminating in QEMU user space SCSI target) attached as
vhost SCSI LUN. I originally tried to use a vIOMMU, but I think I hit
into a QEMU bug. I reported it in qemu-devel mailing list here:

http://lists.nongnu.org/archive/html/qemu-devel/2018-10/msg05382.html

but I got no answer.  So, vfio no-IOMMU was the only way for me to have
virtio-scsi storage backends. And I think that DPDK already supports
vfio no-IOMMU mode.

Speaking of the 2MB alignment restriction, I know that you are
working towards this direction:

https://review.gerrithub.io/c/spdk/spdk/+/427816/1

In case of vhost, the vhost shared memory is 2MB-aligned given that the
VM’s memory is hugepage backed. But is this restriction necessary? I
think that the whole configuration would still work if the VM’s memory
was backed by a tmpfs file (normal 4KB pages) given that we use vfio
with IOMMU support. The vhost memory regions would be mapped by the SPDK
vhost target and then registered to vfio as DMA-able memory with MAP_DMA
ioctl. vfio would take care of making this memory DMA-able. This
basically involves pinning the memory and updating the device's IOVA
domain to grant access to this memory.

I haven't tried to implement this. I will have a look at the code. If
you have any pointers on this or you have made any progress, I would
love to hear.

> but rte_vhost changes belong in DPDK.

Yes, but what about the rest of my patches in the vhost library? Except
for inserting the virtio-vhost-user transport in rte_vhost, this new
transport has to be exported somehow to the end user. Currently, I am
using a command-line option in the vhost app, but I think the best would
be to choose the vhost-user transport from the RPC calls for the
construction of the vhost controller. Anyways, I guess we are going to
see that later.

>
> I'm sorry for the previous lack of transparency in this matter.
>
> D.

Thanks again for the above outline. I would appreciate if you could keep
me in the loop for any progress with rte_vhost. In the meantime, I am
looking forward to any comments on the vfio no-IOMMU related patches.

Nikos

>
>
> [1] https://www.mail-archive.com/dev(a)dpdk.org/msg91788.html
> [2] https://www.mail-archive.com/dev(a)dpdk.org/msg101943.html
> [3] https://www.mail-archive.com/dev(a)dpdk.org/msg102042.html
> [4] https://www.mail-archive.com/dev(a)dpdk.org/msg104886.html
> [5] http://patches.dpdk.org/patch/49921/

[SPDK] SPDK virtio-vhost-user series

[Stojaczyk, Dariusz]

[-- Attachment #1: Type: text/plain, Size: 2672 bytes --]

Hi Nikos,

I'm SPDK core maintainer responsible for the vhost library.

I saw your virtio-vhost-user patch series on gerrithub. I know you've
been talking about it on SPDK community meeting over a month ago,
although I was on holiday at that time.

I wanted to give you some background of what is currently going on
around SPDK vhost.

SPDK currently keeps an internal copy of DPDK's rte_vhost with a
couple of storage specific changes. We have tried to upstream those
changes to DPDK, but they were rejected [1]. Although they were
critical to support vhost-scsi or vhost-blk, they also altered how
vhost-net operated and that was DPDK's major concern. We kept the
internal rte_vhost copy but still haven't decided whether to try to
switch to DPDK's version or to completely derive from DPDK and
maintain our own vhost library. At one point we've also put together a
list of rte_vhost issues - one of which was vhost-user specification
incompliance that eventually made our vhost-scsi unusable with QEMU
2.12+. The amount of "fixes" that rte_vhost required was huge.
Instead, we tried to create a new, even lower level vhost library in
DPDK [2]. The initial API proposal was warmly welcomed [3], but a few
months later, after a PoC implementation was ready, the whole library
was rejected as well [4]. [One of the concerns the new library would
address was creating an abstraction and environment for
virtio-vhost-user, but apparently DPDK team didn't find that useful at
the time]

We still have the rte_vhost copy in SPDK and we still haven't decided
on its future strategy, which is why we were so reluctant to reviewing
your patches.

Just last week we seem to have finally made some progress, as a DPDK
patch that would potentially allow SPDK to use DPDK's rte_vhost
directly [5] was approved for DPDK 19.05. Around the end of February I
believe SPDK will try to stop using its rte_vhost copy and switch to
DPDK's rte_vhost with the mentioned patch.

After that happens, I would like to ask you to rebase your patches on
latest DPDK's rte_vhost and resubmit them to DPDK. I can certainly
help with upstreaming vfio no-iommu support in SPDK and am even
willing to implement registering non-2MB-aligned memory, but rte_vhost
changes belong in DPDK.

I'm sorry for the previous lack of transparency in this matter.

D.


[1] https://www.mail-archive.com/dev(a)dpdk.org/msg91788.html
[2] https://www.mail-archive.com/dev(a)dpdk.org/msg101943.html
[3] https://www.mail-archive.com/dev(a)dpdk.org/msg102042.html
[4] https://www.mail-archive.com/dev(a)dpdk.org/msg104886.html
[5] http://patches.dpdk.org/patch/49921/