[PATCH] net/pcap: add snaplen argument

[PATCH] net/pcap: add snaplen argument

[Tianli Lai]

snaplen argument would set the length of each packet
that will save to pcap file.

Signed-off-by: Tianli Lai <laitianli@tom.com>
---
 doc/guides/nics/pcap_ring.rst  | 16 +++++++++-
 drivers/net/pcap/pcap_ethdev.c | 58 +++++++++++++++++++++++++---------
 2 files changed, 58 insertions(+), 16 deletions(-)

diff --git a/doc/guides/nics/pcap_ring.rst b/doc/guides/nics/pcap_ring.rst
index acb1f00e30..8fb309212f 100644
--- a/doc/guides/nics/pcap_ring.rst
+++ b/doc/guides/nics/pcap_ring.rst
@@ -94,6 +94,13 @@ The different stream types are:
 
         iface=eth0
 
+*   snaplen: Defines the length of one packet that will save pcap file.
+    the length of each packets can be snap by set this value.
+    this value between 64 and 65535.
+
+        snaplen=<packet len>
+
+
 Runtime Config Options
 ^^^^^^^^^^^^^^^^^^^^^^
 
@@ -132,6 +139,13 @@ Runtime Config Options
 
 In this case, one dummy rx queue is created for each tx queue argument passed
 
+ - Receive packets on Tx and set the length of packet, for example::
+
+    --vdev 'net_pcap0,tx_pcap=file_tx.pcap,snaplen=100'
+
+In this case, one dummy rx queue is created for each tx queue argument passed and the length of each packet would not over 100 byte.
+
+
 Examples of Usage
 ^^^^^^^^^^^^^^^^^
 
@@ -140,7 +154,7 @@ Read packets from one pcap file and write them to another:
 .. code-block:: console
 
     ./<build_dir>/app/dpdk-testpmd -l 0-3 -n 4 \
-        --vdev 'net_pcap0,rx_pcap=file_rx.pcap,tx_pcap=file_tx.pcap' \
+        --vdev 'net_pcap0,rx_pcap=file_rx.pcap,tx_pcap=file_tx.pcap,snaplen=100' \
         -- --port-topology=chained
 
 Read packets from a network interface and write them to a pcap file:
diff --git a/drivers/net/pcap/pcap_ethdev.c b/drivers/net/pcap/pcap_ethdev.c
index ec29fd6bc5..6e111518e7 100644
--- a/drivers/net/pcap/pcap_ethdev.c
+++ b/drivers/net/pcap/pcap_ethdev.c
@@ -33,10 +33,12 @@
 #define ETH_PCAP_IFACE_ARG    "iface"
 #define ETH_PCAP_PHY_MAC_ARG  "phy_mac"
 #define ETH_PCAP_INFINITE_RX_ARG  "infinite_rx"
+#define ETH_PCAP_SNAPLEN_ARG  "snaplen"
 
 #define ETH_PCAP_ARG_MAXLEN	64
 
 #define RTE_PMD_PCAP_MAX_QUEUES 16
+#define RTE_PCAP_MIN_SNAPLEN    64
 
 static char errbuf[PCAP_ERRBUF_SIZE];
 static struct timespec start_time;
@@ -93,6 +95,7 @@ struct pmd_internals {
 	int single_iface;
 	int phy_mac;
 	unsigned int infinite_rx;
+	int snaplen;
 };
 
 struct pmd_process_private {
@@ -121,6 +124,14 @@ struct pmd_devargs_all {
 	unsigned int is_rx_pcap;
 	unsigned int is_rx_iface;
 	unsigned int infinite_rx;
+	int snaplen;
+};
+struct pmd_devargs_all devargs_all = {
+	.single_iface = 0,
+	.is_tx_pcap = 0,
+	.is_tx_iface = 0,
+	.infinite_rx = 0,
+	.snaplen = RTE_ETH_PCAP_SNAPSHOT_LEN,
 };
 
 static const char *valid_arguments[] = {
@@ -132,6 +143,7 @@ static const char *valid_arguments[] = {
 	ETH_PCAP_IFACE_ARG,
 	ETH_PCAP_PHY_MAC_ARG,
 	ETH_PCAP_INFINITE_RX_ARG,
+	ETH_PCAP_SNAPLEN_ARG,
 	NULL
 };
 
@@ -384,8 +396,10 @@ eth_pcap_tx_dumper(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts)
 	pcap_dumper_t *dumper;
 	unsigned char temp_data[RTE_ETH_PCAP_SNAPLEN];
 	size_t len, caplen;
-
-	pp = rte_eth_devices[dumper_q->port_id].process_private;
+	struct pmd_internals *internals;
+	struct rte_eth_dev *dev = &rte_eth_devices[dumper_q->port_id];
+	internals = dev->data->dev_private;
+	pp = dev->process_private;
 	dumper = pp->tx_dumper[dumper_q->queue_id];
 
 	if (dumper == NULL || nb_pkts == 0)
@@ -402,6 +416,8 @@ eth_pcap_tx_dumper(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts)
 		}
 
 		calculate_timestamp(&header.ts);
+		if ((typeof(internals->snaplen))caplen > internals->snaplen)
+			caplen = internals->snaplen;
 		header.len = len;
 		header.caplen = caplen;
 		/* rte_pktmbuf_read() returns a pointer to the data directly
@@ -536,7 +552,7 @@ open_single_iface(const char *iface, pcap_t **pcap)
 }
 
 static int
-open_single_tx_pcap(const char *pcap_filename, pcap_dumper_t **dumper)
+open_single_tx_pcap(const char *pcap_filename, pcap_dumper_t **dumper, int snaplen)
 {
 	pcap_t *tx_pcap;
 
@@ -546,7 +562,7 @@ open_single_tx_pcap(const char *pcap_filename, pcap_dumper_t **dumper)
 	 * pcap holder.
 	 */
 	tx_pcap = pcap_open_dead_with_tstamp_precision(DLT_EN10MB,
-			RTE_ETH_PCAP_SNAPSHOT_LEN, PCAP_TSTAMP_PRECISION_NANO);
+			snaplen, PCAP_TSTAMP_PRECISION_NANO);
 	if (tx_pcap == NULL) {
 		PMD_LOG(ERR, "Couldn't create dead pcap");
 		return -1;
@@ -627,7 +643,7 @@ eth_dev_start(struct rte_eth_dev *dev)
 		if (!pp->tx_dumper[i] &&
 				strcmp(tx->type, ETH_PCAP_TX_PCAP_ARG) == 0) {
 			if (open_single_tx_pcap(tx->name,
-				&pp->tx_dumper[i]) < 0)
+				&pp->tx_dumper[i], internals->snaplen) < 0)
 				return -1;
 		} else if (!pp->tx_pcap[i] &&
 				strcmp(tx->type, ETH_PCAP_TX_IFACE_ARG) == 0) {
@@ -1055,7 +1071,7 @@ open_tx_pcap(const char *key, const char *value, void *extra_args)
 	struct pmd_devargs *dumpers = extra_args;
 	pcap_dumper_t *dumper;
 
-	if (open_single_tx_pcap(pcap_filename, &dumper) < 0)
+	if (open_single_tx_pcap(pcap_filename, &dumper, devargs_all.snaplen) < 0)
 		return -1;
 
 	if (add_queue(dumpers, pcap_filename, key, NULL, dumper) < 0) {
@@ -1066,6 +1082,15 @@ open_tx_pcap(const char *key, const char *value, void *extra_args)
 	return 0;
 }
 
+static int
+parse_uint_value(const char *key __rte_unused, const char *value, void *extra_args)
+{
+	char *end;
+	int *val = extra_args;
+	*val = strtoul(value, &end, 10);
+	return 0;
+}
+
 /*
  * Opens an interface for reading and writing
  */
@@ -1325,10 +1350,10 @@ eth_from_pcaps(struct rte_vdev_device *vdev,
 	int ret;
 
 	ret = eth_from_pcaps_common(vdev, devargs_all, &internals, &eth_dev);
-
 	if (ret < 0)
 		return ret;
 
+	internals->snaplen = devargs_all->snaplen;
 	/* store weather we are using a single interface for rx/tx or not */
 	internals->single_iface = single_iface;
 
@@ -1404,13 +1429,6 @@ pmd_pcap_probe(struct rte_vdev_device *dev)
 	struct pmd_internals *internal;
 	int ret = 0;
 
-	struct pmd_devargs_all devargs_all = {
-		.single_iface = 0,
-		.is_tx_pcap = 0,
-		.is_tx_iface = 0,
-		.infinite_rx = 0,
-	};
-
 	name = rte_vdev_device_name(dev);
 	PMD_LOG(INFO, "Initializing pmd_pcap for %s", name);
 
@@ -1444,6 +1462,16 @@ pmd_pcap_probe(struct rte_vdev_device *dev)
 			return -1;
 	}
 
+	if (rte_kvargs_count(kvlist, ETH_PCAP_SNAPLEN_ARG) == 1) {
+		ret = rte_kvargs_process(kvlist, ETH_PCAP_SNAPLEN_ARG,
+				&parse_uint_value, &devargs_all.snaplen);
+		if (ret < 0)
+			goto free_kvlist;
+		if (devargs_all.snaplen < RTE_PCAP_MIN_SNAPLEN ||
+			devargs_all.snaplen >= RTE_ETH_PCAP_SNAPSHOT_LEN)
+			devargs_all.snaplen = RTE_PCAP_MIN_SNAPLEN;
+	}
+
 	/*
 	 * If iface argument is passed we open the NICs and use them for
 	 * reading / writing
@@ -1593,7 +1621,7 @@ pmd_pcap_probe(struct rte_vdev_device *dev)
 			pp->tx_dumper[i] = dumpers.queue[i].dumper;
 			pp->tx_pcap[i] = dumpers.queue[i].pcap;
 		}
-
+		internal->snaplen = devargs_all.snaplen;
 		eth_dev->process_private = pp;
 		eth_dev->rx_pkt_burst = eth_pcap_rx;
 		if (devargs_all.is_tx_pcap)
-- 
2.27.0

Re: [PATCH] net/pcap: add snaplen argument

[Ferruh Yigit]

On 3/13/2022 11:26 AM, Tianli Lai wrote:
> snaplen argument would set the length of each packet
> that will save to pcap file.
> 
> Signed-off-by: Tianli Lai <laitianli@tom.com>
> ---
>   doc/guides/nics/pcap_ring.rst  | 16 +++++++++-
>   drivers/net/pcap/pcap_ethdev.c | 58 +++++++++++++++++++++++++---------
>   2 files changed, 58 insertions(+), 16 deletions(-)
> 
> diff --git a/doc/guides/nics/pcap_ring.rst b/doc/guides/nics/pcap_ring.rst
> index acb1f00e30..8fb309212f 100644
> --- a/doc/guides/nics/pcap_ring.rst
> +++ b/doc/guides/nics/pcap_ring.rst
> @@ -94,6 +94,13 @@ The different stream types are:
>   
>           iface=eth0
>   
> +*   snaplen: Defines the length of one packet that will save pcap file.
> +    the length of each packets can be snap by set this value.
> +    this value between 64 and 65535.
> +
> +        snaplen=<packet len>
> +
> +

Hi Tianli,

This section is "Device Streams", it documents main Rx/Tx streams,
'snaplen' can be a detail for this section, what do you think
to drop it form here, it is already documented below?

>   Runtime Config Options
>   ^^^^^^^^^^^^^^^^^^^^^^
>   
> @@ -132,6 +139,13 @@ Runtime Config Options
>   
>   In this case, one dummy rx queue is created for each tx queue argument passed
>   
> + - Receive packets on Tx and set the length of packet, for example::
> +

What about focusing on the argument description, something like:

- `snaplen`: Set the length of the saved packet size, for example::

> +    --vdev 'net_pcap0,tx_pcap=file_tx.pcap,snaplen=100'
> +
> +In this case, one dummy rx queue is created for each tx queue argument passed and the length of each packet would not over 100 byte.
> +

Don't focus on other parameters (like dummy Rx), please only explain 
what 'snaplen' does.

> +
>   Examples of Usage
>   ^^^^^^^^^^^^^^^^^
>   
> @@ -140,7 +154,7 @@ Read packets from one pcap file and write them to another:
>   .. code-block:: console
>   
>       ./<build_dir>/app/dpdk-testpmd -l 0-3 -n 4 \
> -        --vdev 'net_pcap0,rx_pcap=file_rx.pcap,tx_pcap=file_tx.pcap' \
> +        --vdev 'net_pcap0,rx_pcap=file_rx.pcap,tx_pcap=file_tx.pcap,snaplen=100' \
>           -- --port-topology=chained
>   

Can you add a new sample with 'snaplen', instead of attaching to 
existing one?

>   Read packets from a network interface and write them to a pcap file:
> diff --git a/drivers/net/pcap/pcap_ethdev.c b/drivers/net/pcap/pcap_ethdev.c
> index ec29fd6bc5..6e111518e7 100644
> --- a/drivers/net/pcap/pcap_ethdev.c
> +++ b/drivers/net/pcap/pcap_ethdev.c
> @@ -33,10 +33,12 @@
>   #define ETH_PCAP_IFACE_ARG    "iface"
>   #define ETH_PCAP_PHY_MAC_ARG  "phy_mac"
>   #define ETH_PCAP_INFINITE_RX_ARG  "infinite_rx"
> +#define ETH_PCAP_SNAPLEN_ARG  "snaplen"
>   
>   #define ETH_PCAP_ARG_MAXLEN	64
>   
>   #define RTE_PMD_PCAP_MAX_QUEUES 16
> +#define RTE_PCAP_MIN_SNAPLEN    64
>   
>   static char errbuf[PCAP_ERRBUF_SIZE];
>   static struct timespec start_time;
> @@ -93,6 +95,7 @@ struct pmd_internals {
>   	int single_iface;
>   	int phy_mac;
>   	unsigned int infinite_rx;
> +	int snaplen;
>   };
>   
>   struct pmd_process_private {
> @@ -121,6 +124,14 @@ struct pmd_devargs_all {
>   	unsigned int is_rx_pcap;
>   	unsigned int is_rx_iface;
>   	unsigned int infinite_rx;
> +	int snaplen;
> +};
> +struct pmd_devargs_all devargs_all = {
> +	.single_iface = 0,
> +	.is_tx_pcap = 0,
> +	.is_tx_iface = 0,
> +	.infinite_rx = 0,
> +	.snaplen = RTE_ETH_PCAP_SNAPSHOT_LEN,
>   };
>

'devargs_all' is to store user provided devargs per port,
making it a global variable is wrong, it causes problems where there are 
multiple ports, please leave it where it is.


>   static const char *valid_arguments[] = {
> @@ -132,6 +143,7 @@ static const char *valid_arguments[] = {
>   	ETH_PCAP_IFACE_ARG,
>   	ETH_PCAP_PHY_MAC_ARG,
>   	ETH_PCAP_INFINITE_RX_ARG,
> +	ETH_PCAP_SNAPLEN_ARG,
>   	NULL
>   };
>   
> @@ -384,8 +396,10 @@ eth_pcap_tx_dumper(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts)
>   	pcap_dumper_t *dumper;
>   	unsigned char temp_data[RTE_ETH_PCAP_SNAPLEN];
>   	size_t len, caplen;
> -
> -	pp = rte_eth_devices[dumper_q->port_id].process_private;
> +	struct pmd_internals *internals;
> +	struct rte_eth_dev *dev = &rte_eth_devices[dumper_q->port_id];
> +	internals = dev->data->dev_private;
> +	pp = dev->process_private;
>   	dumper = pp->tx_dumper[dumper_q->queue_id];
>   
>   	if (dumper == NULL || nb_pkts == 0)
> @@ -402,6 +416,8 @@ eth_pcap_tx_dumper(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts)
>   		}
>   
>   		calculate_timestamp(&header.ts);
> +		if ((typeof(internals->snaplen))caplen > internals->snaplen)
> +			caplen = internals->snaplen;

When opening pcap interface 'snaplen' is already provided,
what happens if 'caplen' is not set correctly here, won't pcap
library limit the packets to 'snaplen' already?

>   		header.len = len;
>   		header.caplen = caplen;
>   		/* rte_pktmbuf_read() returns a pointer to the data directly
> @@ -536,7 +552,7 @@ open_single_iface(const char *iface, pcap_t **pcap)
>   }
>   
>   static int
> -open_single_tx_pcap(const char *pcap_filename, pcap_dumper_t **dumper)
> +open_single_tx_pcap(const char *pcap_filename, pcap_dumper_t **dumper, int snaplen)
>   {
>   	pcap_t *tx_pcap;
>   
> @@ -546,7 +562,7 @@ open_single_tx_pcap(const char *pcap_filename, pcap_dumper_t **dumper)
>   	 * pcap holder.
>   	 */
>   	tx_pcap = pcap_open_dead_with_tstamp_precision(DLT_EN10MB,
> -			RTE_ETH_PCAP_SNAPSHOT_LEN, PCAP_TSTAMP_PRECISION_NANO);
> +			snaplen, PCAP_TSTAMP_PRECISION_NANO);
>   	if (tx_pcap == NULL) {
>   		PMD_LOG(ERR, "Couldn't create dead pcap");
>   		return -1;
> @@ -627,7 +643,7 @@ eth_dev_start(struct rte_eth_dev *dev)
>   		if (!pp->tx_dumper[i] &&
>   				strcmp(tx->type, ETH_PCAP_TX_PCAP_ARG) == 0) {
>   			if (open_single_tx_pcap(tx->name,
> -				&pp->tx_dumper[i]) < 0)
> +				&pp->tx_dumper[i], internals->snaplen) < 0)
>   				return -1;
>   		} else if (!pp->tx_pcap[i] &&
>   				strcmp(tx->type, ETH_PCAP_TX_IFACE_ARG) == 0) {
> @@ -1055,7 +1071,7 @@ open_tx_pcap(const char *key, const char *value, void *extra_args)
>   	struct pmd_devargs *dumpers = extra_args;
>   	pcap_dumper_t *dumper;
>   
> -	if (open_single_tx_pcap(pcap_filename, &dumper) < 0)
> +	if (open_single_tx_pcap(pcap_filename, &dumper, devargs_all.snaplen) < 0)

Here 'devargs_all' is used directly since patch makes it global, but 
when it is not global anymore, perhaps you can consider to add 'snaplen'
to "struct pmd_devargs" and get it to this function via it.

>   		return -1;
>   
>   	if (add_queue(dumpers, pcap_filename, key, NULL, dumper) < 0) {
> @@ -1066,6 +1082,15 @@ open_tx_pcap(const char *key, const char *value, void *extra_args)
>   	return 0;
>   }
>   
> +static int
> +parse_uint_value(const char *key __rte_unused, const char *value, void *extra_args)
> +{
> +	char *end;
> +	int *val = extra_args;
> +	*val = strtoul(value, &end, 10);

Should check 'strtoul' return value and return error accordingly.

> +	return 0;
> +}
> +
>   /*
>    * Opens an interface for reading and writing
>    */
> @@ -1325,10 +1350,10 @@ eth_from_pcaps(struct rte_vdev_device *vdev,
>   	int ret;
>   
>   	ret = eth_from_pcaps_common(vdev, devargs_all, &internals, &eth_dev);
> -

unrelated change

>   	if (ret < 0)
>   		return ret;
>   
> +	internals->snaplen = devargs_all->snaplen; >   	/* store weather we are using a single interface for rx/tx or not */
>   	internals->single_iface = single_iface;
>   
> @@ -1404,13 +1429,6 @@ pmd_pcap_probe(struct rte_vdev_device *dev)
>   	struct pmd_internals *internal;
>   	int ret = 0;
>   
> -	struct pmd_devargs_all devargs_all = {
> -		.single_iface = 0,
> -		.is_tx_pcap = 0,
> -		.is_tx_iface = 0,
> -		.infinite_rx = 0,
> -	};
> -
>   	name = rte_vdev_device_name(dev);
>   	PMD_LOG(INFO, "Initializing pmd_pcap for %s", name);
>   
> @@ -1444,6 +1462,16 @@ pmd_pcap_probe(struct rte_vdev_device *dev)
>   			return -1;
>   	}
>   
> +	if (rte_kvargs_count(kvlist, ETH_PCAP_SNAPLEN_ARG) == 1) {
> +		ret = rte_kvargs_process(kvlist, ETH_PCAP_SNAPLEN_ARG,
> +				&parse_uint_value, &devargs_all.snaplen);
> +		if (ret < 0)
> +			goto free_kvlist;

Can you print an error log here?

> +		if (devargs_all.snaplen < RTE_PCAP_MIN_SNAPLEN ||
> +			devargs_all.snaplen >= RTE_ETH_PCAP_SNAPSHOT_LEN)
> +			devargs_all.snaplen = RTE_PCAP_MIN_SNAPLEN;

I think better to return an error here, with a log.

> +	}

Can you please move parsing 'ETH_PCAP_SNAPLEN_ARG' argument below in the 
function, where Tx related parameters parsed. After 
'devargs_all.is_tx_pcap' checks.

And can you please define (and document in above documentation), when 
'snaplen' argument is valid?
Like can I use it with only 'iface=' argument? Or does it requires 
'tx_pcap' to work? If so this should be checked in the code.

> +
>   	/*
>   	 * If iface argument is passed we open the NICs and use them for
>   	 * reading / writing
> @@ -1593,7 +1621,7 @@ pmd_pcap_probe(struct rte_vdev_device *dev)
>   			pp->tx_dumper[i] = dumpers.queue[i].dumper;
>   			pp->tx_pcap[i] = dumpers.queue[i].pcap;
>   		}
> -
> +		internal->snaplen = devargs_all.snaplen;

This is not required, 'internals' is shared between primary and 
secondary, if primary has this parameter, secondary can use it
already.

>   		eth_dev->process_private = pp;
>   		eth_dev->rx_pkt_burst = eth_pcap_rx;
>   		if (devargs_all.is_tx_pcap)

[PATCH] net/pcap: add snaplen argument

[Tianli Lai]

snaplen argument would set the length of each packet
that will save to pcap file.

Signed-off-by: Tianli Lai <laitianli@tom.com>
---
 doc/guides/nics/pcap_ring.rst  | 16 ++++++-
 drivers/net/pcap/pcap_ethdev.c | 79 ++++++++++++++++------------------
 2 files changed, 52 insertions(+), 43 deletions(-)

diff --git a/doc/guides/nics/pcap_ring.rst b/doc/guides/nics/pcap_ring.rst
index acb1f00e30..8fb309212f 100644
--- a/doc/guides/nics/pcap_ring.rst
+++ b/doc/guides/nics/pcap_ring.rst
@@ -94,6 +94,13 @@ The different stream types are:
 
         iface=eth0
 
+*   snaplen: Defines the length of one packet that will save pcap file.
+    the length of each packets can be snap by set this value.
+    this value between 64 and 65535.
+
+        snaplen=<packet len>
+
+
 Runtime Config Options
 ^^^^^^^^^^^^^^^^^^^^^^
 
@@ -132,6 +139,13 @@ Runtime Config Options
 
 In this case, one dummy rx queue is created for each tx queue argument passed
 
+ - Receive packets on Tx and set the length of packet, for example::
+
+    --vdev 'net_pcap0,tx_pcap=file_tx.pcap,snaplen=100'
+
+In this case, one dummy rx queue is created for each tx queue argument passed and the length of each packet would not over 100 byte.
+
+
 Examples of Usage
 ^^^^^^^^^^^^^^^^^
 
@@ -140,7 +154,7 @@ Read packets from one pcap file and write them to another:
 .. code-block:: console
 
     ./<build_dir>/app/dpdk-testpmd -l 0-3 -n 4 \
-        --vdev 'net_pcap0,rx_pcap=file_rx.pcap,tx_pcap=file_tx.pcap' \
+        --vdev 'net_pcap0,rx_pcap=file_rx.pcap,tx_pcap=file_tx.pcap,snaplen=100' \
         -- --port-topology=chained
 
 Read packets from a network interface and write them to a pcap file:
diff --git a/drivers/net/pcap/pcap_ethdev.c b/drivers/net/pcap/pcap_ethdev.c
index 8aea6d66ee..6e111518e7 100644
--- a/drivers/net/pcap/pcap_ethdev.c
+++ b/drivers/net/pcap/pcap_ethdev.c
@@ -95,13 +95,13 @@ struct pmd_internals {
 	int single_iface;
 	int phy_mac;
 	unsigned int infinite_rx;
+	int snaplen;
 };
 
 struct pmd_process_private {
 	pcap_t *rx_pcap[RTE_PMD_PCAP_MAX_QUEUES];
 	pcap_t *tx_pcap[RTE_PMD_PCAP_MAX_QUEUES];
 	pcap_dumper_t *tx_dumper[RTE_PMD_PCAP_MAX_QUEUES];
-	int snaplen;
 };
 
 struct pmd_devargs {
@@ -113,7 +113,6 @@ struct pmd_devargs {
 		const char *type;
 	} queue[RTE_PMD_PCAP_MAX_QUEUES];
 	int phy_mac;
-	int snaplen;
 };
 
 struct pmd_devargs_all {
@@ -125,6 +124,14 @@ struct pmd_devargs_all {
 	unsigned int is_rx_pcap;
 	unsigned int is_rx_iface;
 	unsigned int infinite_rx;
+	int snaplen;
+};
+struct pmd_devargs_all devargs_all = {
+	.single_iface = 0,
+	.is_tx_pcap = 0,
+	.is_tx_iface = 0,
+	.infinite_rx = 0,
+	.snaplen = RTE_ETH_PCAP_SNAPSHOT_LEN,
 };
 
 static const char *valid_arguments[] = {
@@ -389,8 +396,10 @@ eth_pcap_tx_dumper(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts)
 	pcap_dumper_t *dumper;
 	unsigned char temp_data[RTE_ETH_PCAP_SNAPLEN];
 	size_t len, caplen;
-
-	pp = rte_eth_devices[dumper_q->port_id].process_private;
+	struct pmd_internals *internals;
+	struct rte_eth_dev *dev = &rte_eth_devices[dumper_q->port_id];
+	internals = dev->data->dev_private;
+	pp = dev->process_private;
 	dumper = pp->tx_dumper[dumper_q->queue_id];
 
 	if (dumper == NULL || nb_pkts == 0)
@@ -407,14 +416,10 @@ eth_pcap_tx_dumper(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts)
 		}
 
 		calculate_timestamp(&header.ts);
+		if ((typeof(internals->snaplen))caplen > internals->snaplen)
+			caplen = internals->snaplen;
 		header.len = len;
 		header.caplen = caplen;
-		if (pp->snaplen >= RTE_PCAP_MIN_SNAPLEN) {
-			if ((typeof(pp->snaplen))header.caplen > pp->snaplen) {
-				header.caplen = pp->snaplen;
-				caplen = pp->snaplen;
-			}
-		}
 		/* rte_pktmbuf_read() returns a pointer to the data directly
 		 * in the mbuf (when the mbuf is contiguous) or, otherwise,
 		 * a pointer to temp_data after copying into it.
@@ -523,11 +528,8 @@ eth_pcap_tx(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts)
  * pcap_open_live wrapper function
  */
 static inline int
-open_iface_live(const char *iface, pcap_t **pcap, int snaplen) {
-	int caplen = RTE_ETH_PCAP_SNAPLEN;
-	if (snaplen >= RTE_PCAP_MIN_SNAPLEN)
-		caplen = snaplen;
-	*pcap = pcap_open_live(iface, caplen,
+open_iface_live(const char *iface, pcap_t **pcap) {
+	*pcap = pcap_open_live(iface, RTE_ETH_PCAP_SNAPLEN,
 			RTE_ETH_PCAP_PROMISC, RTE_ETH_PCAP_TIMEOUT, errbuf);
 
 	if (*pcap == NULL) {
@@ -539,9 +541,9 @@ open_iface_live(const char *iface, pcap_t **pcap, int snaplen) {
 }
 
 static int
-open_single_iface(const char *iface, pcap_t **pcap, int snaplen)
+open_single_iface(const char *iface, pcap_t **pcap)
 {
-	if (open_iface_live(iface, pcap, snaplen) < 0) {
+	if (open_iface_live(iface, pcap) < 0) {
 		PMD_LOG(ERR, "Couldn't open interface %s", iface);
 		return -1;
 	}
@@ -553,16 +555,14 @@ static int
 open_single_tx_pcap(const char *pcap_filename, pcap_dumper_t **dumper, int snaplen)
 {
 	pcap_t *tx_pcap;
-	int caplen = RTE_ETH_PCAP_SNAPSHOT_LEN;
-	if (snaplen >= RTE_PCAP_MIN_SNAPLEN)
-		caplen = snaplen;
+
 	/*
 	 * We need to create a dummy empty pcap_t to use it
 	 * with pcap_dump_open(). We create big enough an Ethernet
 	 * pcap holder.
 	 */
 	tx_pcap = pcap_open_dead_with_tstamp_precision(DLT_EN10MB,
-			caplen, PCAP_TSTAMP_PRECISION_NANO);
+			snaplen, PCAP_TSTAMP_PRECISION_NANO);
 	if (tx_pcap == NULL) {
 		PMD_LOG(ERR, "Couldn't create dead pcap");
 		return -1;
@@ -628,7 +628,7 @@ eth_dev_start(struct rte_eth_dev *dev)
 
 		if (!pp->tx_pcap[0] &&
 			strcmp(tx->type, ETH_PCAP_IFACE_ARG) == 0) {
-			if (open_single_iface(tx->name, &pp->tx_pcap[0], pp->snaplen) < 0)
+			if (open_single_iface(tx->name, &pp->tx_pcap[0]) < 0)
 				return -1;
 			pp->rx_pcap[0] = pp->tx_pcap[0];
 		}
@@ -643,11 +643,11 @@ eth_dev_start(struct rte_eth_dev *dev)
 		if (!pp->tx_dumper[i] &&
 				strcmp(tx->type, ETH_PCAP_TX_PCAP_ARG) == 0) {
 			if (open_single_tx_pcap(tx->name,
-				&pp->tx_dumper[i], pp->snaplen) < 0)
+				&pp->tx_dumper[i], internals->snaplen) < 0)
 				return -1;
 		} else if (!pp->tx_pcap[i] &&
 				strcmp(tx->type, ETH_PCAP_TX_IFACE_ARG) == 0) {
-			if (open_single_iface(tx->name, &pp->tx_pcap[i], pp->snaplen) < 0)
+			if (open_single_iface(tx->name, &pp->tx_pcap[i]) < 0)
 				return -1;
 		}
 	}
@@ -663,7 +663,7 @@ eth_dev_start(struct rte_eth_dev *dev)
 			if (open_single_rx_pcap(rx->name, &pp->rx_pcap[i]) < 0)
 				return -1;
 		} else if (strcmp(rx->type, ETH_PCAP_RX_IFACE_ARG) == 0) {
-			if (open_single_iface(rx->name, &pp->rx_pcap[i], pp->snaplen) < 0)
+			if (open_single_iface(rx->name, &pp->rx_pcap[i]) < 0)
 				return -1;
 		}
 	}
@@ -1071,7 +1071,7 @@ open_tx_pcap(const char *key, const char *value, void *extra_args)
 	struct pmd_devargs *dumpers = extra_args;
 	pcap_dumper_t *dumper;
 
-	if (open_single_tx_pcap(pcap_filename, &dumper, dumpers->snaplen) < 0)
+	if (open_single_tx_pcap(pcap_filename, &dumper, devargs_all.snaplen) < 0)
 		return -1;
 
 	if (add_queue(dumpers, pcap_filename, key, NULL, dumper) < 0) {
@@ -1083,9 +1083,8 @@ open_tx_pcap(const char *key, const char *value, void *extra_args)
 }
 
 static int
-parse_uint_value(const char *key, const char *value, void *extra_args)
+parse_uint_value(const char *key __rte_unused, const char *value, void *extra_args)
 {
-	(void)key;
 	char *end;
 	int *val = extra_args;
 	*val = strtoul(value, &end, 10);
@@ -1102,7 +1101,7 @@ open_rx_tx_iface(const char *key, const char *value, void *extra_args)
 	struct pmd_devargs *tx = extra_args;
 	pcap_t *pcap = NULL;
 
-	if (open_single_iface(iface, &pcap, tx->snaplen) < 0)
+	if (open_single_iface(iface, &pcap) < 0)
 		return -1;
 
 	tx->queue[0].pcap = pcap;
@@ -1134,7 +1133,7 @@ open_iface(const char *key, const char *value, void *extra_args)
 	struct pmd_devargs *pmd = extra_args;
 	pcap_t *pcap = NULL;
 
-	if (open_single_iface(iface, &pcap, pmd->snaplen) < 0)
+	if (open_single_iface(iface, &pcap) < 0)
 		return -1;
 	if (add_queue(pmd, iface, key, pcap, NULL) < 0) {
 		pcap_close(pcap);
@@ -1351,10 +1350,10 @@ eth_from_pcaps(struct rte_vdev_device *vdev,
 	int ret;
 
 	ret = eth_from_pcaps_common(vdev, devargs_all, &internals, &eth_dev);
-
 	if (ret < 0)
 		return ret;
 
+	internals->snaplen = devargs_all->snaplen;
 	/* store weather we are using a single interface for rx/tx or not */
 	internals->single_iface = single_iface;
 
@@ -1429,14 +1428,6 @@ pmd_pcap_probe(struct rte_vdev_device *dev)
 	struct rte_eth_dev *eth_dev =  NULL;
 	struct pmd_internals *internal;
 	int ret = 0;
-	int snaplen = 0;
-
-	struct pmd_devargs_all devargs_all = {
-		.single_iface = 0,
-		.is_tx_pcap = 0,
-		.is_tx_iface = 0,
-		.infinite_rx = 0,
-	};
 
 	name = rte_vdev_device_name(dev);
 	PMD_LOG(INFO, "Initializing pmd_pcap for %s", name);
@@ -1473,8 +1464,12 @@ pmd_pcap_probe(struct rte_vdev_device *dev)
 
 	if (rte_kvargs_count(kvlist, ETH_PCAP_SNAPLEN_ARG) == 1) {
 		ret = rte_kvargs_process(kvlist, ETH_PCAP_SNAPLEN_ARG,
-				&parse_uint_value, &snaplen);
-		dumpers.snaplen = snaplen;
+				&parse_uint_value, &devargs_all.snaplen);
+		if (ret < 0)
+			goto free_kvlist;
+		if (devargs_all.snaplen < RTE_PCAP_MIN_SNAPLEN ||
+			devargs_all.snaplen >= RTE_ETH_PCAP_SNAPSHOT_LEN)
+			devargs_all.snaplen = RTE_PCAP_MIN_SNAPLEN;
 	}
 
 	/*
@@ -1626,7 +1621,7 @@ pmd_pcap_probe(struct rte_vdev_device *dev)
 			pp->tx_dumper[i] = dumpers.queue[i].dumper;
 			pp->tx_pcap[i] = dumpers.queue[i].pcap;
 		}
-		pp->snaplen = snaplen;
+		internal->snaplen = devargs_all.snaplen;
 		eth_dev->process_private = pp;
 		eth_dev->rx_pkt_burst = eth_pcap_rx;
 		if (devargs_all.is_tx_pcap)
-- 
2.27.0