[dpdk-dev] [PATCH v2 1/9] ethdev: introduce Rx buffer split

[Thomas Monjalon]

12/10/2020 11:40, Slava Ovsiienko:
> From: Thomas Monjalon <thomas at monjalon.net>
> > > int
> > > rte_eth_rx_queue_setup_ex(uint16_t port_id, uint16_t rx_queue_id,
> > >                           uint16_t nb_rx_desc, unsigned int socket_id,
> > >                           const struct rte_eth_rxconf *rx_conf,
> > > 		          const struct rte_eth_rxseg *rx_seg,
> > >                           uint16_t n_seg)
> > 
> > An alternative name for this function:
> > 	rte_eth_rxseg_queue_setup
> M-m-m... Routine name follows patter object_verb:
> rx_queue is an object, setup is an action.
> rxseg_queue is not an object.
> What about "rte_eth_rx_queue_setup_seg"?

rte_eth_rxseg is the name of the struct,
so it looks natural to me to keep it as prefix (object name).

[...]
> > > The new offload flag DEV_RX_OFFLOAD_BUFFER_SPLIT in device
> > 
> > The name should start with RTE_ prefix.
> 
> It is an existing pattern for DEV_RX_OFFLOAD_xxxx, no RTE_ for the case.

It is a wrong pattern which must be fixed.
Please start fresh with the right prefix for new ones.
Thinking twice, it should be:
	RTE_ETH_RX_OFFLOAD_BUFFER_SPLIT

[...]
> > > Also, the proposed segment description might be used to specify Rx
> > > packet split for some other features. For example, provide the way to
> > > specify the extra memory pool for the Header Split feature of some
> > > Intel PMD.
> > 
> > I don't understand what you are referring in this last paragraph.
> > I think explanation above is enough to demonstrate the flexibility.
> > 
> Just noted the segment description is common thing and could be
> promoted to be used in some other features. 

I think it is not needed. And giving Intel as an example is arbitrary.