[RFC] ethdev: introduce protocol type based header split

[Zhang, Qi Z]

> -----Original Message-----
> From: Stephen Hemminger <stephen at networkplumber.org>
> Sent: Friday, March 4, 2022 12:16 AM
> To: Ding, Xuan <xuan.ding at intel.com>
> Cc: thomas at monjalon.net; Yigit, Ferruh <ferruh.yigit at intel.com>;
> andrew.rybchenko at oktetlabs.ru; dev at dpdk.org; viacheslavo at nvidia.com;
> Zhang, Qi Z <qi.z.zhang at intel.com>; Yu, Ping <ping.yu at intel.com>; Wang,
> YuanX <yuanx.wang at intel.com>
> Subject: Re: [RFC] ethdev: introduce protocol type based header split
> 
> On Thu,  3 Mar 2022 06:01:36 +0000
> xuan.ding at intel.com wrote:
> 
> > diff --git a/lib/ethdev/rte_ethdev.h b/lib/ethdev/rte_ethdev.h index
> > c2d1f9a972..6743648c22 100644
> > --- a/lib/ethdev/rte_ethdev.h
> > +++ b/lib/ethdev/rte_ethdev.h
> > @@ -1202,7 +1202,8 @@ struct rte_eth_rxseg_split {
> >  	struct rte_mempool *mp; /**< Memory pool to allocate segment from.
> */
> >  	uint16_t length; /**< Segment data length, configures split point. */
> >  	uint16_t offset; /**< Data offset from beginning of mbuf data buffer. */
> > -	uint32_t reserved; /**< Reserved field. */
> > +	uint16_t proto;
> > +	uint16_t reserved; /**< Reserved field. */
> >  };
> 
> This feature suffers from a common bad design pattern.
> You can't just start using reserved fields unless the previous versions enforced
> that the field was a particular value (usually zero).

Yes, agree, that's a mistake there is no document for the reserved field in the previous release, and usually, it should be zero, 
And I think one of the typical purposes of the reserved field is to make life easy for new feature adding without breaking ABI.
So, should we just take the risk as I guess it might not be a big deal in real cases? 

Thanks
Qi



> 
> There is no guarantee that application will initialize these reserved fields and
> now using them risks breaking the API/ABI. It looks like
> 
> rte_eth_rx_queue_check_split(const struct rte_eth_rxseg_split *rx_seg,
> 
> Would have had to check in previous release.
> 
> This probably has to wait until 22.11 next API release.