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

[Nithin Dabilpuram <ndabilpuram@marvell.com>]

On Fri, Apr 10, 2020 at 02:52:33PM +0200, Andrzej Ostruszka wrote:
> External Email
> 
> ----------------------------------------------------------------------
> On 4/10/20 7:09 AM, Nithin Dabilpuram wrote:
> > On Fri, Apr 10, 2020 at 01:07:17AM +0200, Andrzej Ostruszka wrote:
> [...]
> >>> +struct rte_node_mbuf_priv2 {
> >>> +	union {
> >>> +		/* Sym crypto */
> >>> +		struct {
> >>> +			struct rte_crypto_op op;
> >>> +		};
> >>> +	};
> >>> +} __rte_cache_aligned;
> >>
> >> Why such definition?
> 
> The question was more on "technicalities" - you have struct with anon
> union with anon struct with a struct.  Why such deep nesting - I guess
> the union is there for the possible future extensions but the next anon
> struct - what is it for?

I think inner struct helps in collecting together a specific node's data like
priv1. For example

struct node_mbuf_priv2 {
	union {
		/* Sym crypto */
		struct {
			struct rte_crypto_op op;
			uint64_t extra_session_info;
		};

		/* Reassembly info */
		struct {
			uint64_t reassembly_info;
			uint64_t pad;
		};
	};

	uint8_t data[64];
} __rte_cache_aligned;


Another thing is given that currently there is no crypto support, I'm removing 
the current content of struct node_mbuf_priv2 and just leaving a pad field
for future expansion purpose.

> 
> > 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.
> 
> Thank you for explanations.
> 
> With regards
> Andrzej Ostruszka