From: Panu Matilainen <pmatilai@redhat.com>
To: "Dumitrescu, Cristian" <cristian.dumitrescu@intel.com>,
Thomas Monjalon <thomas.monjalon@6wind.com>,
"Zhang, Roy Fan" <roy.fan.zhang@intel.com>
Cc: "dev@dpdk.org" <dev@dpdk.org>
Subject: Re: [dpdk-dev] [PATCH v2 1/4] lib/librte_port: add PCAP file support to source port
Date: Tue, 8 Mar 2016 11:06:04 +0200 [thread overview]
Message-ID: <56DE95FC.3000201@redhat.com> (raw)
In-Reply-To: <3EB4FA525960D640B5BDFFD6A3D8912647968D3E@IRSMSX108.ger.corp.intel.com>
On 03/08/2016 10:36 AM, Dumitrescu, Cristian wrote:
>
>
>> -----Original Message-----
>> From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
>> Sent: Monday, March 7, 2016 11:18 AM
>> To: Zhang, Roy Fan <roy.fan.zhang@intel.com>
>> Cc: dev@dpdk.org; Panu Matilainen <pmatilai@redhat.com>; Dumitrescu,
>> Cristian <cristian.dumitrescu@intel.com>
>> Subject: Re: [dpdk-dev] [PATCH v2 1/4] lib/librte_port: add PCAP file support
>> to source port
>>
>> 2016-02-17 11:11, Fan Zhang:
>>> --- a/lib/librte_port/rte_port_source_sink.h
>>> +++ b/lib/librte_port/rte_port_source_sink.h
>>> @@ -53,6 +53,13 @@ extern "C" {
>>> struct rte_port_source_params {
>>> /** Pre-initialized buffer pool */
>>> struct rte_mempool *mempool;
>>> + /** The full path of the pcap file to read packets from */
>>> + char *file_name;
>>> + /** The number of bytes to be read from each packet in the
>>> + * pcap file. If this value is 0, the whole packet is read;
>>> + * if it is bigger than packet size, the generated packets
>>> + * will contain the whole packet */
>>> + uint32_t n_bytes_per_pkt;
>>> };
>>
>> If this struct is used in a table, changing its size will break the ABI.
>
> This structure is already present in the API of the source port in file librte_port/rte_port_source_sink.h, this patch is simply adding two new fields at the end of it. I think we accepted adding parameters at the end of the API parameter structures in other parts of DPDK without considering them ABI breakages?
>
> Per Panu's previous comment, this structure is indeed used within an array of unions in the ip_pipeline application, but (1) it is very unlikely a "regular" user application will use it this same way; and (2) somebody using the ip_pipeline application will upgrade both the library and the application at the same time.
>
> If you guys still think this is breaking the ABI, please let us know asap and we'll go with your suggestion.
That it breaks ip_pipeline means it quite obviously is an ABI break.
Adding elements to end of struct might be borderline okay in some
limited cases but in general, who's to say a struct is not embedded in
somebody elses struct in some other program, or in an array? There's no
room for such guessing if ABI compatibility is to mean anything at all.
Explicitly breaking the ABI is not a bad thing, lying about it is.
- Panu -
>
next prev parent reply other threads:[~2016-03-08 9:06 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-02-17 11:11 [dpdk-dev] [PATCH v2 0/4] Add PCAP support to source and sink port Fan Zhang
2016-02-17 11:11 ` [dpdk-dev] [PATCH v2 1/4] lib/librte_port: add PCAP file support to source port Fan Zhang
2016-03-07 11:17 ` Thomas Monjalon
2016-03-08 8:36 ` Dumitrescu, Cristian
2016-03-08 9:06 ` Panu Matilainen [this message]
2016-03-08 10:14 ` Thomas Monjalon
2016-02-17 11:11 ` [dpdk-dev] [PATCH v2 2/4] example/ip_pipeline: add PCAP file support Fan Zhang
2016-02-17 11:11 ` [dpdk-dev] [PATCH v2 3/4] lib/librte_port: add packet dumping to PCAP file support in sink port Fan Zhang
2016-02-17 11:11 ` [dpdk-dev] [PATCH v2 4/4] examples/ip_pipeline: add packets dumping to PCAP file support Fan Zhang
2016-03-09 16:07 ` [dpdk-dev] [PATCH v3 0/4] Add PCAP support to source and sink port Fan Zhang
2016-03-09 16:07 ` [dpdk-dev] [PATCH v3 1/4] lib/librte_port: add PCAP file support to source port Fan Zhang
2016-03-09 16:07 ` [dpdk-dev] [PATCH v3 2/4] example/ip_pipeline: add PCAP file support Fan Zhang
2016-03-11 11:10 ` Thomas Monjalon
2016-03-25 12:00 ` Zhang, Roy Fan
2016-03-09 16:07 ` [dpdk-dev] [PATCH v3 3/4] lib/librte_port: add packet dumping to PCAP file support in sink port Fan Zhang
2016-03-09 16:07 ` [dpdk-dev] [PATCH v3 4/4] examples/ip_pipeline: add packets dumping to PCAP file support Fan Zhang
2016-03-11 17:08 ` [dpdk-dev] [PATCH v4 0/4] Add PCAP support to source and sink port Fan Zhang
2016-03-11 17:08 ` [dpdk-dev] [PATCH v4 1/4] lib/librte_port: add PCAP file support to source port Fan Zhang
2016-03-11 17:08 ` [dpdk-dev] [PATCH v4 2/4] example/ip_pipeline: add PCAP file support Fan Zhang
2016-03-11 17:08 ` [dpdk-dev] [PATCH v4 3/4] lib/librte_port: add packet dumping to PCAP file support in sink port Fan Zhang
2016-03-11 17:08 ` [dpdk-dev] [PATCH v4 4/4] examples/ip_pipeline: add packets dumping to PCAP file support Fan Zhang
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=56DE95FC.3000201@redhat.com \
--to=pmatilai@redhat.com \
--cc=cristian.dumitrescu@intel.com \
--cc=dev@dpdk.org \
--cc=roy.fan.zhang@intel.com \
--cc=thomas.monjalon@6wind.com \
/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).