Re: [dpdk-dev] [PATCH v4 18/29] node: add ethdev Rx and Tx node ctrl API

[Nithin Dabilpuram <ndabilpuram@marvell.com>]

On Fri, Apr 10, 2020 at 10:39:44AM +0530, Nithin Dabilpuram wrote:
> On Fri, Apr 10, 2020 at 01:07:17AM +0200, Andrzej Ostruszka wrote:
> > On 4/5/20 10:56 AM, jerinj@marvell.com wrote:
> > > From: Nithin Dabilpuram <ndabilpuram@marvell.com>
> > > 
> > > Add ctrl api to setup ethdev_rx and ethdev_tx node.
> > > This ctrl api clones 'N' number of ethdev_rx and ethdev_tx
> > > nodes with specific (port, queue) pairs updated in their context.
> > > All the ethdev ports and queues are setup before this api
> > > is called.
> > > 
> > > Signed-off-by: Nithin Dabilpuram <ndabilpuram@marvell.com>
> > > Signed-off-by: Pavan Nikhilesh <pbhagavatula@marvell.com>
> > > Signed-off-by: Kiran Kumar K <kirankumark@marvell.com>
> > > ---
> > [...]
> > > +struct rte_node_mbuf_priv1 {
> > > +	union {
> > > +		/* IP4 rewrite */
> > > +		struct {
> > > +			uint16_t nh;
> > > +			uint16_t ttl;
> > > +			uint32_t cksum;
> > > +		};
> > > +
> > > +		uint64_t u;
> > > +	};
> > > +};
> > > +
> > > +/**
> > > + * @warning
> > > + * @b EXPERIMENTAL: this API may change without prior notice
> > > + *
> > > + * Node mbuf private data to store crypto operation.
> > > + */
> > > +struct rte_node_mbuf_priv2 {
> > > +	union {
> > > +		/* Sym crypto */
> > > +		struct {
> > > +			struct rte_crypto_op op;
> > > +		};
> > > +	};
> > > +} __rte_cache_aligned;
> > 
> > Why such definition?
> 
> For communication b/w nodes, we need some per mbuf private space.
> We defined it into two halfs for performance reasons as
> #1 rte_node_mbuf_priv1(8 bytes) mapped to mbuf->udata64
> #2 rte_node_mbuf_priv2(RTE_CACHE_LINE_SIZE bytes) mapped to mbuf private area.
> 
> #1 is smaller area and will not have a cache miss when accessed as mbuf
> is already in cache.
> #2 is larger area and probably good enough for many use cases like ipsec, crypto 
> etc, and there will be an extra cost of cache miss to access it.
> 
> Atleast in OCTEONTX2, we are able to see 27% performance drop, if use single
> private area #2 for everything instead.
> 
> Since pkt_mbuf pool are created by application, we these structures are defined
> here have a check in ctrl api if the pkt_mbuf pool meets the mbuf private area
> size requirement.

Just wanted to update that I'll also rename this structure and 
related api's to not start with "rte_" in next version
as they are not visible outside this librte_node library.


> 
> > 
> > With regards
> > Andrzej Ostruszka