Re: [dpdk-dev] Question regarding mempool changes impact to XEN PMD

[Olivier Matz <olivier.matz@6wind.com>]

Hi Christian,

On 06/13/2016 09:34 AM, Christian Ehrhardt wrote:
> Hi David,

I guess this mail is for me, not for David :)

> it seems to be the first time I compiled with
> CONFIG_RTE_LIBRTE_PMD_XENVIRT=y sinec the bigger mempool changes around
> "587d684d doc: update release notes about mempool allocation".
> 
> I've seen related patch to mempool / xen in that regard "c042ba20 mempool:
> rework support of Xen dom0"
> 
> But with above config symbol enabled I got:
> drivers/net/xenvirt/rte_xen_lib.c: In function ‘grant_gntalloc_mbuf_pool’:
> drivers/net/xenvirt/rte_xen_lib.c:440:69: error: ‘struct rte_mempool’ has
> no member named ‘elt_va_start’
>   if (snprintf(val_str, sizeof(val_str), "%"PRIxPTR,
> (uintptr_t)mpool->elt_va_start) == -1)
>                                                                      ^
>   SYMLINK-FILE include/rte_eth_bond.h
> mk/internal/rte.compile-pre.mk:126: recipe for target 'rte_xen_lib.o' failed
> make[4]: *** [rte_xen_lib.o] Error 1
> make[4]: *** Waiting for unfinished jobs....
> 
> The change around the mempools is complex, so I don't see on the first look
> if that needs a minor or major rework in the xen sources.
> I mean I don't want it to compile, but to work and that could be more than
> just fixing that changed structure :-)
> 
> So I wanted to ask if you as author would see if it is a trivial change
> that has to be made?

Sorry, I missed this reference to elt_va_start in my patches.

I'm not very familiar with the xen code in dpdk, but from what I see:

- in the PMD, grant_gntalloc_mbuf_pool() stores the mempool virtual
  address in the xen key/value database
- in examples/vhost_xen, the function parse_mpool_va() retrieves it
- this address is used in new_device()

I think the patch would be almost similar to what I did in mlx
drivers in this commit:
http://dpdk.org/browse/dpdk/commit/?id=84121f1971873c9f45b2939c316c66126d8754a1

or in librte_kni in this commit:
http://dpdk.org/browse/dpdk/commit?id=d1d914ebbc2514f334a3ed24057e63c8bb76363d

To give more precisions:

- before the patchset, mp->elt_va_start was the virtual address of the
  mempool objects table. It was always virtually contiguous

- now, a mempool can be fragmented in several virtually contiguous
  chunks. In case there is only one chunk, it can be safely replaced
  by STAILQ_FIRST(&mp->mem_list)->addr (= the virtual address of the
  first chunk).

In case there are more chunks in the mempool, it would require deeper
modifications I think. But we should keep in mind that having a
virtually fragmented mempool was not possible before the patchset
(it would have fail at init). If if fails later in xen code because
xen does not support fragmented mempools, there is no regression
compared to what we had before.

I'll send a draft patch, if you could give it a try, it would be great!

Thanks for reporting,
Olivier