> From: Zhang, Qi Z [mailto:qi.z.zh...@intel.com] > Sent: Friday, 4 March 2022 10.58 > > > From: Stephen Hemminger <step...@networkplumber.org> > > Sent: Friday, March 4, 2022 12:16 AM > > > > On Thu, 3 Mar 2022 06:01:36 +0000 > > xuan.d...@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? >
In this specific case, I think it can be done with very low risk in real cases. Assuming that splitting based on fixed length and protocol header parsing is mutually exclusive, the PMDs can simply ignore the "proto" field (and log a warning about it) if the length field is non-zero. This will provide backwards compatibility with applications not zeroing out the 32 bit "reserved" field. > 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.
