Re: [dpdk-dev] [PATCH v5 01/13] port: added structures for port stats and config option

[Neil Horman <nhorman@tuxdriver.com>]

On Tue, Jun 23, 2015 at 03:55:25PM +0200, Thomas Monjalon wrote:
> 2015-06-19 11:41, Maciej Gajdzica:
> >  /** Input port interface defining the input port operation */
> >  struct rte_port_in_ops {
> >  	rte_port_in_op_create f_create; /**< Create */
> >  	rte_port_in_op_free f_free;     /**< Free */
> >  	rte_port_in_op_rx f_rx;         /**< Packet RX (packet burst) */
> > +	rte_port_in_op_stats_read f_stats;	/**< Stats */
> >  };
> 
> Isn't it breaking an ABI?
> 
This is an interesting question.  Strictly speaking, yes it breaks ABI because
we're adding space, and if older applications statically allocate this
structure, it will be smaller than the port library expects, potentially
scribbling over someone elses memory.  That said, I'm not sure this structure is
meant to be accessed outside of the library.  If it isn't, then we can feel
comfortable that no one is going to access the data structure from code that was
compiled out of sync with the defining library.

The implication if thats true of course is that we should make this structure
opaque outside of the library with a structure prototype and move its definition
into the library private namespace, but I'm fine with doing that at a later date
if the intention is not to have applications touch this structure.

Regards
Neil

> >  struct rte_port_out_ops {
> > -	rte_port_out_op_create f_create;   /**< Create */
> > -	rte_port_out_op_free f_free;       /**< Free */
> > -	rte_port_out_op_tx f_tx;           /**< Packet TX (single packet) */
> > -	rte_port_out_op_tx_bulk f_tx_bulk; /**< Packet TX (packet burst) */
> > -	rte_port_out_op_flush f_flush;     /**< Flush */
> > +	rte_port_out_op_create f_create;		/**< Create */
> > +	rte_port_out_op_free f_free;			/**< Free */
> > +	rte_port_out_op_tx f_tx;				/**< Packet TX (single packet) */
> > +	rte_port_out_op_tx_bulk f_tx_bulk;		/**< Packet TX (packet burst) */
> > +	rte_port_out_op_flush f_flush;			/**< Flush */
> 
> What is the goal of this change? Breaking the alignment?
> 
> > +	rte_port_out_op_stats_read f_stats;     /**< Stats */
> 
>