2015-06-23 14:30, Dumitrescu, Cristian:
> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Thomas Monjalon
> > 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 simply adding a field at the end of the API structure. This structure
> is instantiated per each port type and its role is very similar to a driver
> ops structure, for example:
>
> in file "rte_port_ethdev.h": extern struct rte_port_out_ops
> rte_port_ethdev_writer_ops;
> in file "rte_port_ring.h": extern struct rte_port_out_ops
> rte_port_ring_writer_nodrop_ops;
>
> Typically, instances of these structures are only referenced through pointers
> by application code (and other libraries, like librte_pipeline), so code that
> is not aware of this last field in the structure will still continue to work.
>
> The only case I see possible when code will break is if somebody would create
> an array of such structures, but I think this is not a realistic scenario.
> Instances of this structure are infrequent: once per port type in
> librte_port, and new instances are only created when the user wants to create
> new port type. Basically, instances of this structure are created in
> isolation and not in bulk (arrays).
Why wouldn't it be a problem even for single instance?
If the application allocates one with old sizeof and the lib is trying to write
in the new field, there can be a problem, no?
Maybe Neil has an opinion?
> Due to this, I do not see this as breaking the API. I think this is the most
> it could be done to minimize the effect on the ABI will still adding new
> functionality. Please let me know what you think.
>
> >
> > > 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?
>
> Shall we submit a new patch revision to fix the alignment of the comments?
Yes using spaces for alignment would be better.
