From: Thomas Monjalon <thomas.monjalon@6wind.com>
To: "Dumitrescu, Cristian" <cristian.dumitrescu@intel.com>
Cc: dev@dpdk.org
Subject: Re: [dpdk-dev] [PATCH v5 01/13] port: added structures for port stats and config option
Date: Tue, 23 Jun 2015 16:54:31 +0200 [thread overview]
Message-ID: <2132356.WZTODM8iGX@xps13> (raw)
In-Reply-To: <3EB4FA525960D640B5BDFFD6A3D891263238FA63@IRSMSX108.ger.corp.intel.com>
2015-06-23 14:30, Dumitrescu, Cristian:
> From: dev [mailto:dev-bounces@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.
next prev parent reply other threads:[~2015-06-23 14:55 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-06-19 9:41 [dpdk-dev] [PATCH v5 00/13] port: added port statistics Maciej Gajdzica
2015-06-19 9:41 ` [dpdk-dev] [PATCH v5 01/13] port: added structures for port stats and config option Maciej Gajdzica
2015-06-23 10:26 ` Dumitrescu, Cristian
2015-06-23 13:55 ` Thomas Monjalon
2015-06-23 14:30 ` Dumitrescu, Cristian
2015-06-23 14:54 ` Thomas Monjalon [this message]
2015-06-23 15:21 ` Dumitrescu, Cristian
2015-06-23 15:16 ` Neil Horman
2015-06-19 9:41 ` [dpdk-dev] [PATCH v5 02/13] port: added port_ethdev_reader stats Maciej Gajdzica
2015-06-19 9:41 ` [dpdk-dev] [PATCH v5 03/13] port: added port_ethdev_writer stats Maciej Gajdzica
2015-06-19 9:41 ` [dpdk-dev] [PATCH v5 04/13] port: added port_ethdev_writer_nodrop stats Maciej Gajdzica
2015-06-19 9:41 ` [dpdk-dev] [PATCH v5 05/13] port: added port_frag stats Maciej Gajdzica
2015-06-19 9:41 ` [dpdk-dev] [PATCH v5 06/13] port: added port_ras stats Maciej Gajdzica
2015-06-19 9:41 ` [dpdk-dev] [PATCH v5 07/13] port: added port_ring_reader stats Maciej Gajdzica
2015-06-19 9:41 ` [dpdk-dev] [PATCH v5 08/13] port: added port_ring_writer stats Maciej Gajdzica
2015-06-19 9:41 ` [dpdk-dev] [PATCH v5 09/13] port: added port_ring_writer_nodrop stats Maciej Gajdzica
2015-06-19 9:41 ` [dpdk-dev] [PATCH v5 10/13] port: added port_sched_reader stats Maciej Gajdzica
2015-06-19 9:41 ` [dpdk-dev] [PATCH v5 11/13] port: added port_sched_writer stats Maciej Gajdzica
2015-06-19 9:41 ` [dpdk-dev] [PATCH v5 12/13] port: added port_source stats Maciej Gajdzica
2015-06-19 9:41 ` [dpdk-dev] [PATCH v5 13/13] port: added port_sink stats Maciej Gajdzica
2015-06-23 10:31 ` [dpdk-dev] [PATCH v5 00/13] port: added port statistics Dumitrescu, Cristian
2015-06-23 21:05 ` Thomas Monjalon
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=2132356.WZTODM8iGX@xps13 \
--to=thomas.monjalon@6wind.com \
--cc=cristian.dumitrescu@intel.com \
--cc=dev@dpdk.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).