Re: [dpdk-dev] [PATCH v12 1/6] ethdev: add Tx preparation

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

Hi Konstantin,

On Fri, 2 Dec 2016 01:06:30 +0000, "Ananyev, Konstantin"
<konstantin.ananyev@intel.com> wrote:
> > 
> > 2016-11-23 18:36, Tomasz Kulasek:  
> > > +/**
> > > + * Process a burst of output packets on a transmit queue of an
> > > Ethernet device.
> > > + *
> > > + * The rte_eth_tx_prepare() function is invoked to prepare
> > > output packets to be
> > > + * transmitted on the output queue *queue_id* of the Ethernet
> > > device designated
> > > + * by its *port_id*.
> > > + * The *nb_pkts* parameter is the number of packets to be
> > > prepared which are
> > > + * supplied in the *tx_pkts* array of *rte_mbuf* structures,
> > > each of them
> > > + * allocated from a pool created with rte_pktmbuf_pool_create().
> > > + * For each packet to send, the rte_eth_tx_prepare() function
> > > performs
> > > + * the following operations:
> > > + *
> > > + * - Check if packet meets devices requirements for tx offloads.
> > > + *
> > > + * - Check limitations about number of segments.
> > > + *
> > > + * - Check additional requirements when debug is enabled.
> > > + *
> > > + * - Update and/or reset required checksums when tx offload is
> > > set for packet.
> > > + *
> > > + * Since this function can modify packet data, provided mbufs
> > > must be safely
> > > + * writable (e.g. modified data cannot be in shared segment).  
> > 
> > I think we will have to remove this limitation in next releases.
> > As we don't know how it could affect the API, I suggest to declare
> > this API EXPERIMENTAL.  
> 
> While I don't really mind to mart it as experimental, I don't really
> understand the reasoning: Why " this function can modify packet data,
> provided mbufs must be safely writable" suddenly becomes a problem?
> That seems like and obvious limitation to me and let say tx_burst()
> has the same one. Second, I don't see how you are going to remove it
> without introducing a heavy performance impact. Konstantin 
> 

About tx_burst(), I don't think we should force the user to provide a
writable mbuf. There are many use cases where passing a clone
already works as of today and it avoids duplicating the mbuf data. For
instance: traffic generator, multicast, bridging/tap, etc...

Moreover, this requirement would be inconsistent with the model you are
proposing in case of pipeline:
 - tx_prepare() on core X, may update the data
 - tx_burst() on core Y, should not touch the data to avoid cache misses


Regards,
Olivier