[dpdk-dev] [PATCH v2 1/4] lib/librte_port: add PCAP file support to source port

[Dumitrescu, Cristian]

> -----Original Message-----
> From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com]
> Sent: Monday, March 7, 2016 11:18 AM
> To: Zhang, Roy Fan <roy.fan.zhang at intel.com>
> Cc: dev at dpdk.org; Panu Matilainen <pmatilai at redhat.com>; Dumitrescu,
> Cristian <cristian.dumitrescu at 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.

> More generally, are you sure of the benefits of exposing a configuration
> structure in the API?

This is not an internal (implementation side) structure, it is the external (API side) structure with the parameters required from the user for creating this object, I am not sure I understand your comment?


> 
> [...]
> > --- a/mk/rte.app.mk
> > +++ b/mk/rte.app.mk
> > @@ -111,6 +111,7 @@ _LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_XENVIRT)
> += -lxenstore
> >  _LDLIBS-$(CONFIG_RTE_LIBRTE_MPIPE_PMD)      += -lgxio
> >  # QAT PMD has a dependency on libcrypto (from openssl) for calculating
> HMAC precomputes
> >  _LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_QAT)        += -lcrypto
> > +_LDLIBS-$(CONFIG_RTE_PORT_PCAP)                        += -lpcap
> 
> Please move this line upper before PMD_PCAP.