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

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

> > On 8/22/2019 9:18 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).
> 
> Do you really want this additional configuration in your driver or can't you
> enable/disable the functional

	Performance with multi file segments is worse than non-zero copy, so there is no reason to implement.

> > > +/**
> > > + * @warning
> > > + * @b EXPERIMENTAL: this API may change without prior notice
> > > + *
> > > + * Get the single_file_segments parameter value from memory
> configuration.
> 
> I would prefer you describe what this actually means.
> We don't really care about the value itself.

	Ack.

> 
> 
> > > + */
> > > +__rte_experimental
> > > +uint32_t
> 
> And a boolean is enough, this is a flag.

	Ack.

> 
> 
> > > +rte_mcfg_get_single_file_segments(void);
> > > +
> > >  #ifdef __cplusplus
> > >  }
> > >  #endif
> > > diff --git a/lib/librte_eal/rte_eal_version.map
> > > b/lib/librte_eal/rte_eal_version.map
> > > index 7cbf82d37..c2b9d473f 100644
> > > --- a/lib/librte_eal/rte_eal_version.map
> > > +++ b/lib/librte_eal/rte_eal_version.map
> > > @@ -418,5 +418,6 @@ EXPERIMENTAL {
> > >       rte_lcore_to_cpu_id;
> > >       rte_mcfg_timer_lock;
> > >       rte_mcfg_timer_unlock;
> > > +     rte_mcfg_get_single_file_segments;
> >
> > This should be moved to 19.11 block in experimental
> 
> +1

	Ack.

> 
> 
> > cc'ed Dave for eal part,
> > @Dave, change looks straight forward but can you please check/comment?
> 
> I don't like the name of this API, since it gives the impression it returns
> "segments"..
> But on the other hand, this is aligned with the mcfg field: people touching the
> internals have more chances to see there is an exported API.

	Do you have any suggestions? How about rte_mcfg_get_single_file_segments_parameter()?

> 
> Cc: Anatoly (but I think he is off for this week).
> 
> Other than that I am ok with this change.
> 
> 
> --
> David Marchand