[dpdk-dev,v2,1/4] lib/librte_port: add PCAP file support to source port
Commit Message
Originally, source ports in librte_port is an input port used as packet
generator. Similar to Linux kernel /dev/zero character device, it
generates null packets. This patch adds optional PCAP file support to
source port: instead of sending NULL packets, the source port generates
packets copied from a PCAP file. To increase the performance, the packets
in the file are loaded to memory initially, and copied to mbufs in circular
manner. Users can enable or disable this feature by setting
CONFIG_RTE_PORT_PCAP compiler option "y" or "n".
Signed-off-by: Fan Zhang <roy.fan.zhang@intel.com>
Acked-by: Cristian Dumitrescu <cristian.dumitrescu@intel.com>
---
config/common_bsdapp | 1 +
config/common_linuxapp | 1 +
lib/librte_port/Makefile | 4 +
lib/librte_port/rte_port_source_sink.c | 226 ++++++++++++++++++++++++++++++++-
lib/librte_port/rte_port_source_sink.h | 7 +
mk/rte.app.mk | 1 +
6 files changed, 239 insertions(+), 1 deletion(-)
Comments
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.
More generally, are you sure of the benefits of exposing a configuration
structure in the API?
[...]
> --- 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.
> -----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.
> 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.
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 -
>
2016-03-08 08:36, Dumitrescu, Cristian:
> From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> > 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?
It depends of the struct but generally it is considered an ABI break.
> 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.
Yes it is.
> > 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?
There are 2 ways of passing parameters: struct or individual params.
By using some functions with individual params, you can avoid ABI break
(see librte_compat).
In this case you would have a function pcap_config(). Just an idea.
@@ -459,6 +459,7 @@ CONFIG_RTE_LIBRTE_REORDER=y
#
CONFIG_RTE_LIBRTE_PORT=y
CONFIG_RTE_PORT_STATS_COLLECT=n
+CONFIG_RTE_PORT_PCAP=n
#
# Compile librte_table
@@ -476,6 +476,7 @@ CONFIG_RTE_LIBRTE_REORDER=y
#
CONFIG_RTE_LIBRTE_PORT=y
CONFIG_RTE_PORT_STATS_COLLECT=n
+CONFIG_RTE_PORT_PCAP=n
#
# Compile librte_table
@@ -36,6 +36,10 @@ include $(RTE_SDK)/mk/rte.vars.mk
#
LIB = librte_port.a
+ifeq ($(CONFIG_RTE_PORT_PCAP),y)
+LDLIBS += -lpcap
+endif
+
CFLAGS += -O3
CFLAGS += $(WERROR_FLAGS)
@@ -36,6 +36,11 @@
#include <rte_mbuf.h>
#include <rte_mempool.h>
#include <rte_malloc.h>
+#include <rte_memcpy.h>
+
+#ifdef RTE_PORT_PCAP
+#include <pcap.h>
+#endif
#include "rte_port_source_sink.h"
@@ -60,14 +65,175 @@ struct rte_port_source {
struct rte_port_in_stats stats;
struct rte_mempool *mempool;
+
+ /* PCAP buffers and indexes */
+ uint8_t **pkts;
+ uint8_t *pkt_buff;
+ uint32_t *pkt_len;
+ uint32_t n_pkts;
+ uint32_t pkt_index;
};
+#ifdef RTE_PORT_PCAP
+
+/**
+ * Load PCAP file, allocate and copy packets in the file to memory
+ *
+ * @param p
+ * Parameters for source port
+ * @param port
+ * Handle to source port
+ * @param socket_id
+ * Socket id where the memory is created
+ * @return
+ * 0 on SUCCESS
+ * error code otherwise
+ */
+static int
+pcap_source_load(struct rte_port_source_params *p,
+ struct rte_port_source *port,
+ int socket_id)
+{
+ uint32_t status = 0;
+ uint32_t n_pkts = 0;
+ uint32_t i;
+ uint32_t *pkt_len_aligns = NULL;
+ size_t total_buff_len = 0;
+ pcap_t *pcap_handle;
+ char pcap_errbuf[PCAP_ERRBUF_SIZE];
+ uint32_t max_len;
+ struct pcap_pkthdr pcap_hdr;
+ const uint8_t *pkt;
+ uint8_t *buff = NULL;
+ uint32_t pktmbuf_maxlen = (uint32_t)
+ (rte_pktmbuf_data_room_size(port->mempool) -
+ RTE_PKTMBUF_HEADROOM);
+
+ if (p->file_name == NULL)
+ return 0;
+
+ if (p->n_bytes_per_pkt == 0)
+ max_len = pktmbuf_maxlen;
+ else
+ max_len = RTE_MIN(p->n_bytes_per_pkt, pktmbuf_maxlen);
+
+ /* first time open, get packet number */
+ pcap_handle = pcap_open_offline(p->file_name, pcap_errbuf);
+ if (pcap_handle == NULL) {
+ status = -ENOENT;
+ goto error_exit;
+ }
+
+ while ((pkt = pcap_next(pcap_handle, &pcap_hdr)) != NULL)
+ n_pkts++;
+
+ pcap_close(pcap_handle);
+
+ port->pkt_len = rte_zmalloc_socket("PCAP",
+ (sizeof(*port->pkt_len) * n_pkts), 0, socket_id);
+ if (port->pkt_len == NULL) {
+ status = -ENOMEM;
+ goto error_exit;
+ }
+
+ pkt_len_aligns = rte_malloc("PCAP",
+ (sizeof(*pkt_len_aligns) * n_pkts), 0);
+ if (pkt_len_aligns == NULL) {
+ status = -ENOMEM;
+ goto error_exit;
+ }
+
+ port->pkts = rte_zmalloc_socket("PCAP",
+ (sizeof(*port->pkts) * n_pkts), 0, socket_id);
+ if (port->pkts == NULL) {
+ status = -ENOMEM;
+ goto error_exit;
+ }
+
+ /* open 2nd time, get pkt_len */
+ pcap_handle = pcap_open_offline(p->file_name, pcap_errbuf);
+ if (pcap_handle == NULL) {
+ status = -ENOENT;
+ goto error_exit;
+ }
+
+ for (i = 0; i < n_pkts; i++) {
+ pkt = pcap_next(pcap_handle, &pcap_hdr);
+ port->pkt_len[i] = RTE_MIN(max_len, pcap_hdr.len);
+ pkt_len_aligns[i] = RTE_CACHE_LINE_ROUNDUP(
+ port->pkt_len[i]);
+ total_buff_len += pkt_len_aligns[i];
+ }
+
+ pcap_close(pcap_handle);
+
+ /* allocate a big trunk of data for pcap file load */
+ buff = rte_zmalloc_socket("PCAP",
+ total_buff_len, 0, socket_id);
+ if (buff == NULL) {
+ status = -ENOMEM;
+ goto error_exit;
+ }
+
+ port->pkt_buff = buff;
+
+ /* open file one last time to copy the pkt content */
+ pcap_handle = pcap_open_offline(p->file_name, pcap_errbuf);
+ if (pcap_handle == NULL) {
+ status = -ENOENT;
+ goto error_exit;
+ }
+
+ for (i = 0; i < n_pkts; i++) {
+ pkt = pcap_next(pcap_handle, &pcap_hdr);
+ rte_memcpy(buff, pkt, port->pkt_len[i]);
+ port->pkts[i] = buff;
+ buff += pkt_len_aligns[i];
+ }
+
+ pcap_close(pcap_handle);
+
+ port->n_pkts = n_pkts;
+
+ rte_free(pkt_len_aligns);
+
+ return 0;
+
+error_exit:
+ if (pkt_len_aligns)
+ rte_free(pkt_len_aligns);
+ if (port->pkt_len)
+ rte_free(port->pkt_len);
+ if (port->pkts)
+ rte_free(port->pkts);
+ if (port->pkt_buff)
+ rte_free(port->pkt_buff);
+
+ return status;
+}
+
+#else
+static int
+pcap_source_load(__rte_unused struct rte_port_source_params *p,
+ struct rte_port_source *port,
+ __rte_unused int socket_id)
+{
+ port->pkt_buff = NULL;
+ port->pkt_len = NULL;
+ port->pkts = NULL;
+ port->pkt_index = 0;
+
+ return -ENOTSUP;
+}
+#endif
+
static void *
rte_port_source_create(void *params, int socket_id)
{
struct rte_port_source_params *p =
(struct rte_port_source_params *) params;
struct rte_port_source *port;
+ int status;
/* Check input arguments*/
if ((p == NULL) || (p->mempool == NULL)) {
@@ -79,23 +245,65 @@ rte_port_source_create(void *params, int socket_id)
port = rte_zmalloc_socket("PORT", sizeof(*port),
RTE_CACHE_LINE_SIZE, socket_id);
if (port == NULL) {
- RTE_LOG(ERR, PORT, "%s: Failed to allocate port\n", __func__);
+ RTE_LOG(ERR, PORT, "%s: Failed to allocate port\n",
+ __func__);
return NULL;
}
/* Initialization */
port->mempool = (struct rte_mempool *) p->mempool;
+ /* pcap file load and initialization */
+ status = pcap_source_load(p, port, socket_id);
+ if (status == 0) {
+ if (port->pkt_buff != NULL) {
+ RTE_LOG(INFO, PORT, "Successfully load pcap file "
+ "'%s' with %u pkts\n",
+ p->file_name, port->n_pkts);
+ }
+ } else if (status != -ENOTSUP) {
+ /* ENOTSUP is not treated as error */
+ switch (status) {
+ case -ENOENT:
+ RTE_LOG(ERR, PORT, "%s: Failed to open pcap file "
+ "'%s' for reading\n",
+ __func__, p->file_name);
+ break;
+ case -ENOMEM:
+ RTE_LOG(ERR, PORT, "%s: Not enough memory\n",
+ __func__);
+ break;
+ default:
+ RTE_LOG(ERR, PORT, "%s: Failed to enable PCAP "
+ "support for unknown reason\n",
+ __func__);
+ break;
+ }
+
+ rte_free(port);
+ port = NULL;
+ }
+
return port;
}
static int
rte_port_source_free(void *port)
{
+ struct rte_port_source *p =
+ (struct rte_port_source *)port;
+
/* Check input parameters */
if (port == NULL)
return 0;
+ if (p->pkt_len)
+ rte_free(p->pkt_len);
+ if (p->pkts)
+ rte_free(p->pkts);
+ if (p->pkt_buff)
+ rte_free(p->pkt_buff);
+
rte_free(port);
return 0;
@@ -115,6 +323,22 @@ rte_port_source_rx(void *port, struct rte_mbuf **pkts, uint32_t n_pkts)
rte_pktmbuf_reset(pkts[i]);
}
+ if (p->pkt_buff != NULL) {
+ for (i = 0; i < n_pkts; i++) {
+ uint8_t *pkt_data = rte_pktmbuf_mtod(pkts[i],
+ uint8_t *);
+
+ rte_memcpy(pkt_data, p->pkts[p->pkt_index],
+ p->pkt_len[p->pkt_index]);
+ pkts[i]->data_len = p->pkt_len[p->pkt_index];
+ pkts[i]->pkt_len = pkts[i]->data_len;
+
+ p->pkt_index++;
+ if (p->pkt_index >= p->n_pkts)
+ p->pkt_index = 0;
+ }
+ }
+
RTE_PORT_SOURCE_STATS_PKTS_IN_ADD(p, n_pkts);
return n_pkts;
@@ -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;
};
/** source port operations */
@@ -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
endif # CONFIG_RTE_BUILD_COMBINE_LIBS or not CONFIG_RTE_BUILD_SHARED_LIBS
_LDLIBS-y += --start-group