Re: [dpdk-dev] [PATCH v4] net/memif: zero-copy slave

["Jakub Grajciar -X (jgrajcia - PANTHEON TECHNOLOGIES at Cisco)" <jgrajcia@cisco.com>]

> -----Original Message-----
> From: Ferruh Yigit <ferruh.yigit@intel.com>
> Sent: Wednesday, July 10, 2019 5:07 PM
> To: Jakub Grajciar -X (jgrajcia - PANTHEON TECHNOLOGIES at Cisco)
> <jgrajcia@cisco.com>; dev@dpdk.org
> Cc: anatoly.burakov@intel.com
> Subject: Re: [dpdk-dev] [PATCH v4] net/memif: zero-copy slave
> 
> On 7/9/2019 9:22 AM, Jakub Grajciar wrote:
> > Zero-copy slave support for memif PMD.
> > Slave interface exposes DPDK memory to master interface. Only single
> > file segments are supported (EAL option --single-file-segments).
> >
> > Signed-off-by: Jakub Grajciar <jgrajcia@cisco.com>
> 
> <...>
> 
> > +Zero-copy slave
> > +~~~~~~~~~~~~~~~
> > +
> > +**Shared memory format**
> > +
> > +Region 0 is created by memif driver and contains rings. Slave interface
> exposes DPDK memory (memseg).
> > +Instead of using memfd_create() to create new shared file, existing
> memsegs are used.
> > +Master interface functions the same as with zero-copy disabled.
> > +
> > +region 0:
> > +
> > ++-----------------------+
> > +| Rings                 |
> > ++-----------+-----------+
> > +| S2M rings | M2S rings |
> > ++-----------+-----------+
> > +
> > +region n:
> > +
> > ++-----------------+
> > +| Buffers         |
> > ++-----------------+
> > +|memseg           |
> > ++-----------------+
> > +
> > +Buffers are dequeued and enqueued as needed. Offset descriptor field is
> calculated at tx.
> > +Only single file segments mode (EAL option --single-file-segments) is
> > +supported, as calculating offset from multiple segments is too expensive.
> > +
> 
> Can you please add sample testpmd command for 'zerocopy' support, this can
> be useful to highlight '--single-file-segments' eal argument is required.
> 
> Also can you please document what happens if only master or slave provides
> the 'zero-copy=yes' devarg?
> 

Will do.

> <...>
> 
> > @@ -1127,6 +1522,10 @@ memif_set_zc(const char *key __rte_unused,
> const char *value, void *extra_args)
> >  	uint32_t *flags = (uint32_t *)extra_args;
> >
> >  	if (strstr(value, "yes") != NULL) {
> > +		if (!rte_mcfg_get_single_file_segments()) {
> > +			MIF_LOG(ERR, "Zero-copy doesn't support multi-file
> segments.");
> > +			return -ENOTSUP;
> > +		}
> 
> Why "--single-file-segments" is required? Is it to have big physically
> continuous chunk of memory? If so will it work with 2Mb hugepages?

Single file segments limitation will be explained in docs in next patch. Simply put zero-copy with multi file segments has worse performance than non zero-copy due to slow memseg lookup.

> btw, I am having a crash with 2Mb hugepages and zerocopy, but "1Gb &
> zerocopy" & "2Mb & non zerocopy" works without crash.

This is strange, I've only tested using 2Mb hugepages without a crash. Can you please provide more details?