[dpdk-dev] [PATCH v2] net/pcap: rx_iface_in stream type support

[dpdk-dev] [PATCH v2] net/pcap: rx_iface_in stream type support

[ido goshen]

Support rx of in direction packets only
Useful for apps that also tx to eth_pcap ports in order not to see them
echoed back in as rx when out direction is also captured

Signed-off-by: ido goshen <ido@cgstowernetworks.com>
---
v2: clean checkpatch warning

 drivers/net/pcap/rte_eth_pcap.c | 36 ++++++++++++++++++++++++++++++++----
 1 file changed, 32 insertions(+), 4 deletions(-)

diff --git a/drivers/net/pcap/rte_eth_pcap.c b/drivers/net/pcap/rte_eth_pcap.c
index 6bd4a7d..132f469 100644
--- a/drivers/net/pcap/rte_eth_pcap.c
+++ b/drivers/net/pcap/rte_eth_pcap.c
@@ -26,6 +26,7 @@
 #define ETH_PCAP_RX_PCAP_ARG  "rx_pcap"
 #define ETH_PCAP_TX_PCAP_ARG  "tx_pcap"
 #define ETH_PCAP_RX_IFACE_ARG "rx_iface"
+#define ETH_PCAP_RX_IFACE_IN_ARG "rx_iface_in"
 #define ETH_PCAP_TX_IFACE_ARG "tx_iface"
 #define ETH_PCAP_IFACE_ARG    "iface"
 
@@ -83,6 +84,7 @@ struct pmd_devargs {
 	ETH_PCAP_RX_PCAP_ARG,
 	ETH_PCAP_TX_PCAP_ARG,
 	ETH_PCAP_RX_IFACE_ARG,
+	ETH_PCAP_RX_IFACE_IN_ARG,
 	ETH_PCAP_TX_IFACE_ARG,
 	ETH_PCAP_IFACE_ARG,
 	NULL
@@ -726,6 +728,22 @@ struct pmd_devargs {
 	return 0;
 }
 
+static inline int
+set_iface_direction(const char *iface, const char *key, pcap_t *pcap)
+{
+	if (strcmp(key, ETH_PCAP_RX_IFACE_IN_ARG) == 0) {
+		if (pcap_setdirection(pcap, PCAP_D_IN) < 0) {
+			PMD_LOG(ERR,
+				"Setting %s pcap direction IN failed - %s\n",
+				 iface,
+				 pcap_geterr(pcap));
+			return -1;
+		}
+		PMD_LOG(INFO, "Setting %s pcap direction IN\n", iface);
+	}
+	return 0;
+}
+
 /*
  * Opens a NIC for reading packets from it
  */
@@ -740,11 +758,12 @@ struct pmd_devargs {
 	for (i = 0; i < rx->num_of_queue; i++) {
 		if (open_single_iface(iface, &pcap) < 0)
 			return -1;
+		if (set_iface_direction(iface, key, pcap) < 0)
+			return -1;
 		rx->queue[i].pcap = pcap;
 		rx->queue[i].name = iface;
 		rx->queue[i].type = key;
 	}
-
 	return 0;
 }
 
@@ -963,17 +982,25 @@ struct pmd_devargs {
 		is_rx_pcap = 1;
 	else
 		pcaps.num_of_queue = rte_kvargs_count(kvlist,
-				ETH_PCAP_RX_IFACE_ARG);
+				ETH_PCAP_RX_IFACE_ARG) +
+				rte_kvargs_count(kvlist,
+						ETH_PCAP_RX_IFACE_IN_ARG);
 
 	if (pcaps.num_of_queue > RTE_PMD_PCAP_MAX_QUEUES)
 		pcaps.num_of_queue = RTE_PMD_PCAP_MAX_QUEUES;
 
-	if (is_rx_pcap)
+	if (is_rx_pcap) {
 		ret = rte_kvargs_process(kvlist, ETH_PCAP_RX_PCAP_ARG,
 				&open_rx_pcap, &pcaps);
-	else
+	} else {
 		ret = rte_kvargs_process(kvlist, ETH_PCAP_RX_IFACE_ARG,
 				&open_rx_iface, &pcaps);
+		if (ret == 0)
+			ret = rte_kvargs_process(kvlist,
+					ETH_PCAP_RX_IFACE_IN_ARG,
+					&open_rx_iface,
+					&pcaps);
+	}
 
 	if (ret < 0)
 		goto free_kvlist;
@@ -1046,6 +1073,7 @@ struct pmd_devargs {
 	ETH_PCAP_RX_PCAP_ARG "=<string> "
 	ETH_PCAP_TX_PCAP_ARG "=<string> "
 	ETH_PCAP_RX_IFACE_ARG "=<ifc> "
+	ETH_PCAP_RX_IFACE_IN_ARG "=<ifc> "
 	ETH_PCAP_TX_IFACE_ARG "=<ifc> "
 	ETH_PCAP_IFACE_ARG "=<ifc>");
 
-- 
1.9.1

Re: [dpdk-dev] [PATCH v2] net/pcap: rx_iface_in stream type support

[Ferruh Yigit]

On 6/5/2018 10:39 AM, ido goshen wrote:
> Support rx of in direction packets only
> Useful for apps that also tx to eth_pcap ports in order not to see them
> echoed back in as rx when out direction is also captured

Can you please explain the problem a little more?
If you are using rx_iface and tx_iface arguments you should have different pcap
handlers for Rx and Tx and you shouldn't be getting Tx packets in Rx handler.

> 
> Signed-off-by: ido goshen <ido@cgstowernetworks.com>
> ---
> v2: clean checkpatch warning
> 
>  drivers/net/pcap/rte_eth_pcap.c | 36 ++++++++++++++++++++++++++++++++----

doc/guides/nics/pcap_ring.rst also needs to be updated with this new arg, but
please wait to clarify the above comment, to figure out if this patch really
necessary, before updating that document.

>  1 file changed, 32 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/pcap/rte_eth_pcap.c b/drivers/net/pcap/rte_eth_pcap.c
> index 6bd4a7d..132f469 100644
> --- a/drivers/net/pcap/rte_eth_pcap.c
> +++ b/drivers/net/pcap/rte_eth_pcap.c
> @@ -26,6 +26,7 @@
>  #define ETH_PCAP_RX_PCAP_ARG  "rx_pcap"
>  #define ETH_PCAP_TX_PCAP_ARG  "tx_pcap"
>  #define ETH_PCAP_RX_IFACE_ARG "rx_iface"
> +#define ETH_PCAP_RX_IFACE_IN_ARG "rx_iface_in"
>  #define ETH_PCAP_TX_IFACE_ARG "tx_iface"
>  #define ETH_PCAP_IFACE_ARG    "iface"
>  
> @@ -83,6 +84,7 @@ struct pmd_devargs {
>  	ETH_PCAP_RX_PCAP_ARG,
>  	ETH_PCAP_TX_PCAP_ARG,
>  	ETH_PCAP_RX_IFACE_ARG,
> +	ETH_PCAP_RX_IFACE_IN_ARG,
>  	ETH_PCAP_TX_IFACE_ARG,
>  	ETH_PCAP_IFACE_ARG,
>  	NULL
> @@ -726,6 +728,22 @@ struct pmd_devargs {
>  	return 0;
>  }
>  
> +static inline int
> +set_iface_direction(const char *iface, const char *key, pcap_t *pcap)
> +{
> +	if (strcmp(key, ETH_PCAP_RX_IFACE_IN_ARG) == 0) {

set_iface_direction() can be more generic function, instead of checking the key
here, it can be done the functions calls this.

Also can be possible to make even more generic, this function only sets
direction as PCAP_D_IN, you can get in/out as input to function.

> +		if (pcap_setdirection(pcap, PCAP_D_IN) < 0) {
> +			PMD_LOG(ERR,
> +				"Setting %s pcap direction IN failed - %s\n",
> +				 iface,
> +				 pcap_geterr(pcap));
> +			return -1;
> +		}
> +		PMD_LOG(INFO, "Setting %s pcap direction IN\n", iface);
> +	}
> +	return 0;
> +}
> +
>  /*
>   * Opens a NIC for reading packets from it
>   */
> @@ -740,11 +758,12 @@ struct pmd_devargs {
>  	for (i = 0; i < rx->num_of_queue; i++) {
>  		if (open_single_iface(iface, &pcap) < 0)
>  			return -1;
> +		if (set_iface_direction(iface, key, pcap) < 0)
> +			return -1;
>  		rx->queue[i].pcap = pcap;
>  		rx->queue[i].name = iface;
>  		rx->queue[i].type = key;
>  	}
> -
>  	return 0;
>  }
>  
> @@ -963,17 +982,25 @@ struct pmd_devargs {
>  		is_rx_pcap = 1;
>  	else
>  		pcaps.num_of_queue = rte_kvargs_count(kvlist,
> -				ETH_PCAP_RX_IFACE_ARG);
> +				ETH_PCAP_RX_IFACE_ARG) +
> +				rte_kvargs_count(kvlist,
> +						ETH_PCAP_RX_IFACE_IN_ARG);
>  
>  	if (pcaps.num_of_queue > RTE_PMD_PCAP_MAX_QUEUES)
>  		pcaps.num_of_queue = RTE_PMD_PCAP_MAX_QUEUES;
>  
> -	if (is_rx_pcap)
> +	if (is_rx_pcap) {
>  		ret = rte_kvargs_process(kvlist, ETH_PCAP_RX_PCAP_ARG,
>  				&open_rx_pcap, &pcaps);
> -	else
> +	} else {
>  		ret = rte_kvargs_process(kvlist, ETH_PCAP_RX_IFACE_ARG,
>  				&open_rx_iface, &pcaps);
> +		if (ret == 0)
> +			ret = rte_kvargs_process(kvlist,
> +					ETH_PCAP_RX_IFACE_IN_ARG,
> +					&open_rx_iface,
> +					&pcaps);

I guess there is an assumption here ETH_PCAP_RX_IFACE_ARG and
ETH_PCAP_RX_IFACE_IN_ARG are mutually exclusive. What if user uses both?
Anything to protect against this, or any information to prevent this usage?

> +	}
>  
>  	if (ret < 0)
>  		goto free_kvlist;
> @@ -1046,6 +1073,7 @@ struct pmd_devargs {
>  	ETH_PCAP_RX_PCAP_ARG "=<string> "
>  	ETH_PCAP_TX_PCAP_ARG "=<string> "
>  	ETH_PCAP_RX_IFACE_ARG "=<ifc> "
> +	ETH_PCAP_RX_IFACE_IN_ARG "=<ifc> "
>  	ETH_PCAP_TX_IFACE_ARG "=<ifc> "
>  	ETH_PCAP_IFACE_ARG "=<ifc>");
>  
> 

Re: [dpdk-dev] [PATCH v2] net/pcap: rx_iface_in stream type support

[Ido Goshen]

The problem is if a dpdk app uses the same iface(s) both as rx_iface and tx_iface then it will receive back the packets it sends.
If my app sends a packet to portid=X with rte_eth_tx_burst() then I wouldn't expect to receive it back by rte_eth_rx_burst() for that same portid=X  (assuming of course there's no external loopback)
This is coming from the default nature of pcap that like a sniffer captures both incoming and outgoing direction.
The patch provides an option to limit pcap rx_iface to get only incoming traffic which is more like a real (non-pcap) dpdk device.

for example:
when using existing *rx_iface*
	l2fwd -c 3 -n1 --no-huge --vdev=eth_pcap0,rx_iface=eth1,tx_iface=eth1 --vdev=eth_pcap1,rx_iface=dummy0,tx_iface=dummy0  -- -p 3 -T 1 
sending only 1 single packet into eth1 will end in an infinite loop - 
the packet that is being transmitted by l2fwd to tx_iface=dummy0 is being recaptured again by the rx_iface=dummy0 (as it captures outgoing packets too) 
then l2fwd it to tx_iface=eth0 and it is received back on it by the rx_iface=eth0 and so on...
	Port statistics ====================================
	Statistics for port 0 ------------------------------
	Packets sent:                     4658
	Packets received:                 4658
	Packets dropped:                     0
	Statistics for port 1 ------------------------------
	Packets sent:                     4658
	Packets received:                 4658
	Packets dropped:                     0
	Aggregate statistics ===============================
	Total packets sent:               9316
	Total packets received:           9316
	Total packets dropped:               0
	====================================================

while if using the suggested *rx_iface_in*
	l2fwd -c 3 -n1 --no-huge --vdev=eth_pcap0,rx_iface_in=eth1,tx_iface=eth1 --vdev=eth_pcap1,rx_iface_in=dummy0,tx_iface=dummy0  -- -p 3 -T 1
The packet will be transmitted once and not received again (as I'd expect)
	Port statistics ====================================
	Statistics for port 0 ------------------------------
	Packets sent:                        0
	Packets received:                    1
	Packets dropped:                     0
	Statistics for port 1 ------------------------------
	Packets sent:                        1
	Packets received:                    0
	Packets dropped:                     0
	Aggregate statistics ===============================
	Total packets sent:                  1
	Total packets received:              1
	Total packets dropped:               0
	====================================================

-----Original Message-----
From: Ferruh Yigit <ferruh.yigit@intel.com> 
Sent: Tuesday, June 5, 2018 4:27 PM
To: Ido Goshen <Ido@cgstowernetworks.com>
Cc: dev@dpdk.org
Subject: Re: [PATCH v2] net/pcap: rx_iface_in stream type support

On 6/5/2018 10:39 AM, ido goshen wrote:
> Support rx of in direction packets only Useful for apps that also tx 
> to eth_pcap ports in order not to see them echoed back in as rx when 
> out direction is also captured

Can you please explain the problem a little more?
If you are using rx_iface and tx_iface arguments you should have different pcap handlers for Rx and Tx and you shouldn't be getting Tx packets in Rx handler.

> 
> Signed-off-by: ido goshen <ido@cgstowernetworks.com>
> ---
> v2: clean checkpatch warning
> 
>  drivers/net/pcap/rte_eth_pcap.c | 36 
> ++++++++++++++++++++++++++++++++----

doc/guides/nics/pcap_ring.rst also needs to be updated with this new arg, but please wait to clarify the above comment, to figure out if this patch really necessary, before updating that document.

>  1 file changed, 32 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/pcap/rte_eth_pcap.c 
> b/drivers/net/pcap/rte_eth_pcap.c index 6bd4a7d..132f469 100644
> --- a/drivers/net/pcap/rte_eth_pcap.c
> +++ b/drivers/net/pcap/rte_eth_pcap.c
> @@ -26,6 +26,7 @@
>  #define ETH_PCAP_RX_PCAP_ARG  "rx_pcap"
>  #define ETH_PCAP_TX_PCAP_ARG  "tx_pcap"
>  #define ETH_PCAP_RX_IFACE_ARG "rx_iface"
> +#define ETH_PCAP_RX_IFACE_IN_ARG "rx_iface_in"
>  #define ETH_PCAP_TX_IFACE_ARG "tx_iface"
>  #define ETH_PCAP_IFACE_ARG    "iface"
>  
> @@ -83,6 +84,7 @@ struct pmd_devargs {
>  	ETH_PCAP_RX_PCAP_ARG,
>  	ETH_PCAP_TX_PCAP_ARG,
>  	ETH_PCAP_RX_IFACE_ARG,
> +	ETH_PCAP_RX_IFACE_IN_ARG,
>  	ETH_PCAP_TX_IFACE_ARG,
>  	ETH_PCAP_IFACE_ARG,
>  	NULL
> @@ -726,6 +728,22 @@ struct pmd_devargs {
>  	return 0;
>  }
>  
> +static inline int
> +set_iface_direction(const char *iface, const char *key, pcap_t *pcap) 
> +{
> +	if (strcmp(key, ETH_PCAP_RX_IFACE_IN_ARG) == 0) {

set_iface_direction() can be more generic function, instead of checking the key here, it can be done the functions calls this.

Also can be possible to make even more generic, this function only sets direction as PCAP_D_IN, you can get in/out as input to function.

> +		if (pcap_setdirection(pcap, PCAP_D_IN) < 0) {
> +			PMD_LOG(ERR,
> +				"Setting %s pcap direction IN failed - %s\n",
> +				 iface,
> +				 pcap_geterr(pcap));
> +			return -1;
> +		}
> +		PMD_LOG(INFO, "Setting %s pcap direction IN\n", iface);
> +	}
> +	return 0;
> +}
> +
>  /*
>   * Opens a NIC for reading packets from it
>   */
> @@ -740,11 +758,12 @@ struct pmd_devargs {
>  	for (i = 0; i < rx->num_of_queue; i++) {
>  		if (open_single_iface(iface, &pcap) < 0)
>  			return -1;
> +		if (set_iface_direction(iface, key, pcap) < 0)
> +			return -1;
>  		rx->queue[i].pcap = pcap;
>  		rx->queue[i].name = iface;
>  		rx->queue[i].type = key;
>  	}
> -
>  	return 0;
>  }
>  
> @@ -963,17 +982,25 @@ struct pmd_devargs {
>  		is_rx_pcap = 1;
>  	else
>  		pcaps.num_of_queue = rte_kvargs_count(kvlist,
> -				ETH_PCAP_RX_IFACE_ARG);
> +				ETH_PCAP_RX_IFACE_ARG) +
> +				rte_kvargs_count(kvlist,
> +						ETH_PCAP_RX_IFACE_IN_ARG);
>  
>  	if (pcaps.num_of_queue > RTE_PMD_PCAP_MAX_QUEUES)
>  		pcaps.num_of_queue = RTE_PMD_PCAP_MAX_QUEUES;
>  
> -	if (is_rx_pcap)
> +	if (is_rx_pcap) {
>  		ret = rte_kvargs_process(kvlist, ETH_PCAP_RX_PCAP_ARG,
>  				&open_rx_pcap, &pcaps);
> -	else
> +	} else {
>  		ret = rte_kvargs_process(kvlist, ETH_PCAP_RX_IFACE_ARG,
>  				&open_rx_iface, &pcaps);
> +		if (ret == 0)
> +			ret = rte_kvargs_process(kvlist,
> +					ETH_PCAP_RX_IFACE_IN_ARG,
> +					&open_rx_iface,
> +					&pcaps);

I guess there is an assumption here ETH_PCAP_RX_IFACE_ARG and ETH_PCAP_RX_IFACE_IN_ARG are mutually exclusive. What if user uses both?
Anything to protect against this, or any information to prevent this usage?

> +	}
>  
>  	if (ret < 0)
>  		goto free_kvlist;
> @@ -1046,6 +1073,7 @@ struct pmd_devargs {
>  	ETH_PCAP_RX_PCAP_ARG "=<string> "
>  	ETH_PCAP_TX_PCAP_ARG "=<string> "
>  	ETH_PCAP_RX_IFACE_ARG "=<ifc> "
> +	ETH_PCAP_RX_IFACE_IN_ARG "=<ifc> "
>  	ETH_PCAP_TX_IFACE_ARG "=<ifc> "
>  	ETH_PCAP_IFACE_ARG "=<ifc>");
>  
> 

Re: [dpdk-dev] [PATCH v2] net/pcap: rx_iface_in stream type support

[Ferruh Yigit]

On 6/5/2018 6:10 PM, Ido Goshen wrote:
> The problem is if a dpdk app uses the same iface(s) both as rx_iface and tx_iface then it will receive back the packets it sends.
> If my app sends a packet to portid=X with rte_eth_tx_burst() then I wouldn't expect to receive it back by rte_eth_rx_burst() for that same portid=X  (assuming of course there's no external loopback)
> This is coming from the default nature of pcap that like a sniffer captures both incoming and outgoing direction.
> The patch provides an option to limit pcap rx_iface to get only incoming traffic which is more like a real (non-pcap) dpdk device.
> 
> for example:
> when using existing *rx_iface*
> 	l2fwd -c 3 -n1 --no-huge --vdev=eth_pcap0,rx_iface=eth1,tx_iface=eth1 --vdev=eth_pcap1,rx_iface=dummy0,tx_iface=dummy0  -- -p 3 -T 1 
> sending only 1 single packet into eth1 will end in an infinite loop - 

If you are using same interface for both Rx & Tx, why not using "iface=xxx"
argument, can you please test with following:

l2fwd -c 3 -n1 --no-huge --vdev=eth_pcap0,iface=eth1
--vdev=eth_pcap1,iface=dummy0 -- -p 3 -T 1


I can't reproduce the issue with above command.

Thanks,
ferruh

Re: [dpdk-dev] [PATCH v2] net/pcap: rx_iface_in stream type support

[Ido Goshen]

I use "rx_iface","tx_iface" (and not just "iface") in order to have multiple TX queues
I just gave a simplified setting with 1 queue
My app  does a full mesh between the ports (not fixed pairs like l2fwd) so all the forwarding lcores can tx to the same port simultaneously
and as DPDK docs say:
"Multiple logical cores should never share receive or transmit queues for interfaces since this would require global locks and hinder performance."
For example if I have 3 ports handled by 3 cores it'll be 
	myapp -c 7 -n1 --no-huge \
	--vdev=eth_pcap0,rx_iface=eth0,tx_iface=eth0,tx_iface=eth0,tx_iface=eth0 \
	--vdev=eth_pcap0,rx_iface=eth1,tx_iface=eth1,tx_iface=eth1,tx_iface=eth1 \
	--vdev=eth_pcap0,rx_iface=eth2,tx_iface=eth2,tx_iface=eth2,tx_iface=eth2 \
	-- -p 7
Is there another way to achieve multiple queues in pcap vdev?

I do see that using "iface" behaves differently - I'll try to investigate why
And still even when using "iface" I also see packets that are transmitted out of eth1 (e.g. tcpreplay -i eth1 packets.pcap) 
and not only packets that are received (e.g. ping from far end to eth0 ip)


-----Original Message-----
From: Ferruh Yigit <ferruh.yigit@intel.com> 
Sent: Wednesday, June 13, 2018 1:57 PM
To: Ido Goshen <Ido@cgstowernetworks.com>
Cc: dev@dpdk.org
Subject: Re: [PATCH v2] net/pcap: rx_iface_in stream type support

On 6/5/2018 6:10 PM, Ido Goshen wrote:
> The problem is if a dpdk app uses the same iface(s) both as rx_iface and tx_iface then it will receive back the packets it sends.
> If my app sends a packet to portid=X with rte_eth_tx_burst() then I 
> wouldn't expect to receive it back by rte_eth_rx_burst() for that same portid=X  (assuming of course there's no external loopback) This is coming from the default nature of pcap that like a sniffer captures both incoming and outgoing direction.
> The patch provides an option to limit pcap rx_iface to get only incoming traffic which is more like a real (non-pcap) dpdk device.
> 
> for example:
> when using existing *rx_iface*
> 	l2fwd -c 3 -n1 --no-huge --vdev=eth_pcap0,rx_iface=eth1,tx_iface=eth1 
> --vdev=eth_pcap1,rx_iface=dummy0,tx_iface=dummy0  -- -p 3 -T 1 sending 
> only 1 single packet into eth1 will end in an infinite loop -

If you are using same interface for both Rx & Tx, why not using "iface=xxx"
argument, can you please test with following:

l2fwd -c 3 -n1 --no-huge --vdev=eth_pcap0,iface=eth1
--vdev=eth_pcap1,iface=dummy0 -- -p 3 -T 1


I can't reproduce the issue with above command.

Thanks,
ferruh

Re: [dpdk-dev] [PATCH v2] net/pcap: rx_iface_in stream type support

[Ferruh Yigit]

On 6/14/2018 6:14 PM, Ido Goshen wrote:
> I use "rx_iface","tx_iface" (and not just "iface") in order to have multiple TX queues
> I just gave a simplified setting with 1 queue
> My app  does a full mesh between the ports (not fixed pairs like l2fwd) so all the forwarding lcores can tx to the same port simultaneously
> and as DPDK docs say:
> "Multiple logical cores should never share receive or transmit queues for interfaces since this would require global locks and hinder performance."
> For example if I have 3 ports handled by 3 cores it'll be 
> 	myapp -c 7 -n1 --no-huge \
> 	--vdev=eth_pcap0,rx_iface=eth0,tx_iface=eth0,tx_iface=eth0,tx_iface=eth0 \
> 	--vdev=eth_pcap0,rx_iface=eth1,tx_iface=eth1,tx_iface=eth1,tx_iface=eth1 \
> 	--vdev=eth_pcap0,rx_iface=eth2,tx_iface=eth2,tx_iface=eth2,tx_iface=eth2 \
> 	-- -p 7
> Is there another way to achieve multiple queues in pcap vdev?

If you want to use multiple core you need multiple queues, as you said, and
above is the way to create multiple queues for pcap.

Currently "iface" argument only supports single interface in a hardcoded way,
but technically it should be possible to update it to support multiple queue.

So if "iface" arguments works for you, it can be better to add multi queue
support to "iface" instead of introducing a new device argument.

> 
> I do see that using "iface" behaves differently - I'll try to investigate why

pcap_open_live() is called for both arguments, for "rx_iface/tx_iface" pair it
has been called twice one for each. Not sure if pcap library returns same
handler or two different handlers for this case since iface name is same.
For "iface" argument pcap_open_live() called once, so we have single handler for
both Rx & Tx. This may be difference.

> And still even when using "iface" I also see packets that are transmitted out of eth1 (e.g. tcpreplay -i eth1 packets.pcap) 
> and not only packets that are received (e.g. ping from far end to eth0 ip)

This is interesting, I have tried with external packet generator, "iface" was
working as expected for me.

> 
> 
> -----Original Message-----
> From: Ferruh Yigit <ferruh.yigit@intel.com> 
> Sent: Wednesday, June 13, 2018 1:57 PM
> To: Ido Goshen <Ido@cgstowernetworks.com>
> Cc: dev@dpdk.org
> Subject: Re: [PATCH v2] net/pcap: rx_iface_in stream type support
> 
> On 6/5/2018 6:10 PM, Ido Goshen wrote:
>> The problem is if a dpdk app uses the same iface(s) both as rx_iface and tx_iface then it will receive back the packets it sends.
>> If my app sends a packet to portid=X with rte_eth_tx_burst() then I 
>> wouldn't expect to receive it back by rte_eth_rx_burst() for that same portid=X  (assuming of course there's no external loopback) This is coming from the default nature of pcap that like a sniffer captures both incoming and outgoing direction.
>> The patch provides an option to limit pcap rx_iface to get only incoming traffic which is more like a real (non-pcap) dpdk device.
>>
>> for example:
>> when using existing *rx_iface*
>> 	l2fwd -c 3 -n1 --no-huge --vdev=eth_pcap0,rx_iface=eth1,tx_iface=eth1 
>> --vdev=eth_pcap1,rx_iface=dummy0,tx_iface=dummy0  -- -p 3 -T 1 sending 
>> only 1 single packet into eth1 will end in an infinite loop -
> 
> If you are using same interface for both Rx & Tx, why not using "iface=xxx"
> argument, can you please test with following:
> 
> l2fwd -c 3 -n1 --no-huge --vdev=eth_pcap0,iface=eth1
> --vdev=eth_pcap1,iface=dummy0 -- -p 3 -T 1
> 
> 
> I can't reproduce the issue with above command.
> 
> Thanks,
> ferruh
> 

Re: [dpdk-dev] [PATCH v2] net/pcap: rx_iface_in stream type support

[Ido Goshen]

I think we are starting to mix two things
One is how to configure pcap eth dev with multiple queues and I totally agree it would have been nicer to just say something like "max_tx_queues =N" instead of needing to write "tx_iface" N times, but as it was already supported in that way (for any reason?) I wasn't trying to enhance or change it.
The other issue is pcap direction API, which I was trying to expose to users of dpdk pcap device.
Refer to https://www.tcpdump.org/manpages/pcap_setdirection.3pcap.txt or man tcpdump for -P/--direction in|out|inout option, 
Actually I think a more realistic emulation of a physical device (non-virtual) would be to capture only the incoming direction (set PCAP_D_IN), again the existing behavior is very useful too and I didn't try to change or eliminate it but just add additional stream type option

-----Original Message-----
From: Ferruh Yigit <ferruh.yigit@intel.com> 
Sent: Thursday, June 14, 2018 9:09 PM
To: Ido Goshen <Ido@cgstowernetworks.com>
Cc: dev@dpdk.org
Subject: Re: [PATCH v2] net/pcap: rx_iface_in stream type support

On 6/14/2018 6:14 PM, Ido Goshen wrote:
> I use "rx_iface","tx_iface" (and not just "iface") in order to have 
> multiple TX queues I just gave a simplified setting with 1 queue My 
> app  does a full mesh between the ports (not fixed pairs like l2fwd) 
> so all the forwarding lcores can tx to the same port simultaneously and as DPDK docs say:
> "Multiple logical cores should never share receive or transmit queues for interfaces since this would require global locks and hinder performance."
> For example if I have 3 ports handled by 3 cores it'll be 
> 	myapp -c 7 -n1 --no-huge \
> 	--vdev=eth_pcap0,rx_iface=eth0,tx_iface=eth0,tx_iface=eth0,tx_iface=eth0 \
> 	--vdev=eth_pcap0,rx_iface=eth1,tx_iface=eth1,tx_iface=eth1,tx_iface=eth1 \
> 	--vdev=eth_pcap0,rx_iface=eth2,tx_iface=eth2,tx_iface=eth2,tx_iface=eth2 \
> 	-- -p 7
> Is there another way to achieve multiple queues in pcap vdev?

If you want to use multiple core you need multiple queues, as you said, and above is the way to create multiple queues for pcap.

Currently "iface" argument only supports single interface in a hardcoded way, but technically it should be possible to update it to support multiple queue.

So if "iface" arguments works for you, it can be better to add multi queue support to "iface" instead of introducing a new device argument.

> 
> I do see that using "iface" behaves differently - I'll try to 
> investigate why

pcap_open_live() is called for both arguments, for "rx_iface/tx_iface" pair it has been called twice one for each. Not sure if pcap library returns same handler or two different handlers for this case since iface name is same.
For "iface" argument pcap_open_live() called once, so we have single handler for both Rx & Tx. This may be difference.

> And still even when using "iface" I also see packets that are 
> transmitted out of eth1 (e.g. tcpreplay -i eth1 packets.pcap) and not 
> only packets that are received (e.g. ping from far end to eth0 ip)

This is interesting, I have tried with external packet generator, "iface" was working as expected for me.

> 
> 
> -----Original Message-----
> From: Ferruh Yigit <ferruh.yigit@intel.com>
> Sent: Wednesday, June 13, 2018 1:57 PM
> To: Ido Goshen <Ido@cgstowernetworks.com>
> Cc: dev@dpdk.org
> Subject: Re: [PATCH v2] net/pcap: rx_iface_in stream type support
> 
> On 6/5/2018 6:10 PM, Ido Goshen wrote:
>> The problem is if a dpdk app uses the same iface(s) both as rx_iface and tx_iface then it will receive back the packets it sends.
>> If my app sends a packet to portid=X with rte_eth_tx_burst() then I 
>> wouldn't expect to receive it back by rte_eth_rx_burst() for that same portid=X  (assuming of course there's no external loopback) This is coming from the default nature of pcap that like a sniffer captures both incoming and outgoing direction.
>> The patch provides an option to limit pcap rx_iface to get only incoming traffic which is more like a real (non-pcap) dpdk device.
>>
>> for example:
>> when using existing *rx_iface*
>> 	l2fwd -c 3 -n1 --no-huge 
>> --vdev=eth_pcap0,rx_iface=eth1,tx_iface=eth1
>> --vdev=eth_pcap1,rx_iface=dummy0,tx_iface=dummy0  -- -p 3 -T 1 
>> sending only 1 single packet into eth1 will end in an infinite loop -
> 
> If you are using same interface for both Rx & Tx, why not using "iface=xxx"
> argument, can you please test with following:
> 
> l2fwd -c 3 -n1 --no-huge --vdev=eth_pcap0,iface=eth1
> --vdev=eth_pcap1,iface=dummy0 -- -p 3 -T 1
> 
> 
> I can't reproduce the issue with above command.
> 
> Thanks,
> ferruh
> 

Re: [dpdk-dev] [PATCH v2] net/pcap: rx_iface_in stream type support

[Ferruh Yigit]

On 6/14/2018 9:44 PM, Ido Goshen wrote:
> I think we are starting to mix two things
> One is how to configure pcap eth dev with multiple queues and I totally agree it would have been nicer to just say something like "max_tx_queues =N" instead of needing to write "tx_iface" N times, but as it was already supported in that way (for any reason?) I wasn't trying to enhance or change it.
> The other issue is pcap direction API, which I was trying to expose to users of dpdk pcap device.

Hi Ido,

Assuming "iface" argument solves the direction issue, I am suggestion adding
multiqueue support to "iface" argument as a solution to your problem.

I am not suggesting using new arguments like "max_tx_queues =N", "iface" can be
used same as how rx/tx_ifcase used now, provide it multiple times.

> Refer to https://www.tcpdump.org/manpages/pcap_setdirection.3pcap.txt or man tcpdump for -P/--direction in|out|inout option, 
> Actually I think a more realistic emulation of a physical device (non-virtual) would be to capture only the incoming direction (set PCAP_D_IN), again the existing behavior is very useful too and I didn't try to change or eliminate it but just add additional stream type option

I am not sure exiting behavior is intentional, which is capturing sent packages
in Rx pcap handler to same interface.
Are you aware of any use case of existing behavior? Perhaps it can be option to
set PCAP_D_IN by default for rx_iface argument.

> 
> -----Original Message-----
> From: Ferruh Yigit <ferruh.yigit@intel.com> 
> Sent: Thursday, June 14, 2018 9:09 PM
> To: Ido Goshen <Ido@cgstowernetworks.com>
> Cc: dev@dpdk.org
> Subject: Re: [PATCH v2] net/pcap: rx_iface_in stream type support
> 
> On 6/14/2018 6:14 PM, Ido Goshen wrote:
>> I use "rx_iface","tx_iface" (and not just "iface") in order to have 
>> multiple TX queues I just gave a simplified setting with 1 queue My 
>> app  does a full mesh between the ports (not fixed pairs like l2fwd) 
>> so all the forwarding lcores can tx to the same port simultaneously and as DPDK docs say:
>> "Multiple logical cores should never share receive or transmit queues for interfaces since this would require global locks and hinder performance."
>> For example if I have 3 ports handled by 3 cores it'll be 
>> 	myapp -c 7 -n1 --no-huge \
>> 	--vdev=eth_pcap0,rx_iface=eth0,tx_iface=eth0,tx_iface=eth0,tx_iface=eth0 \
>> 	--vdev=eth_pcap0,rx_iface=eth1,tx_iface=eth1,tx_iface=eth1,tx_iface=eth1 \
>> 	--vdev=eth_pcap0,rx_iface=eth2,tx_iface=eth2,tx_iface=eth2,tx_iface=eth2 \
>> 	-- -p 7
>> Is there another way to achieve multiple queues in pcap vdev?
> 
> If you want to use multiple core you need multiple queues, as you said, and above is the way to create multiple queues for pcap.
> 
> Currently "iface" argument only supports single interface in a hardcoded way, but technically it should be possible to update it to support multiple queue.
> 
> So if "iface" arguments works for you, it can be better to add multi queue support to "iface" instead of introducing a new device argument.
> 
>>
>> I do see that using "iface" behaves differently - I'll try to 
>> investigate why
> 
> pcap_open_live() is called for both arguments, for "rx_iface/tx_iface" pair it has been called twice one for each. Not sure if pcap library returns same handler or two different handlers for this case since iface name is same.
> For "iface" argument pcap_open_live() called once, so we have single handler for both Rx & Tx. This may be difference.
> 
>> And still even when using "iface" I also see packets that are 
>> transmitted out of eth1 (e.g. tcpreplay -i eth1 packets.pcap) and not 
>> only packets that are received (e.g. ping from far end to eth0 ip)
> 
> This is interesting, I have tried with external packet generator, "iface" was working as expected for me.
> 
>>
>>
>> -----Original Message-----
>> From: Ferruh Yigit <ferruh.yigit@intel.com>
>> Sent: Wednesday, June 13, 2018 1:57 PM
>> To: Ido Goshen <Ido@cgstowernetworks.com>
>> Cc: dev@dpdk.org
>> Subject: Re: [PATCH v2] net/pcap: rx_iface_in stream type support
>>
>> On 6/5/2018 6:10 PM, Ido Goshen wrote:
>>> The problem is if a dpdk app uses the same iface(s) both as rx_iface and tx_iface then it will receive back the packets it sends.
>>> If my app sends a packet to portid=X with rte_eth_tx_burst() then I 
>>> wouldn't expect to receive it back by rte_eth_rx_burst() for that same portid=X  (assuming of course there's no external loopback) This is coming from the default nature of pcap that like a sniffer captures both incoming and outgoing direction.
>>> The patch provides an option to limit pcap rx_iface to get only incoming traffic which is more like a real (non-pcap) dpdk device.
>>>
>>> for example:
>>> when using existing *rx_iface*
>>> 	l2fwd -c 3 -n1 --no-huge 
>>> --vdev=eth_pcap0,rx_iface=eth1,tx_iface=eth1
>>> --vdev=eth_pcap1,rx_iface=dummy0,tx_iface=dummy0  -- -p 3 -T 1 
>>> sending only 1 single packet into eth1 will end in an infinite loop -
>>
>> If you are using same interface for both Rx & Tx, why not using "iface=xxx"
>> argument, can you please test with following:
>>
>> l2fwd -c 3 -n1 --no-huge --vdev=eth_pcap0,iface=eth1
>> --vdev=eth_pcap1,iface=dummy0 -- -p 3 -T 1
>>
>>
>> I can't reproduce the issue with above command.
>>
>> Thanks,
>> ferruh
>>
> 

Re: [dpdk-dev] [PATCH v2] net/pcap: rx_iface_in stream type support

[Ido Goshen]

Is pcap_sendpacket() to the same pcap_t handle thread-safe? I couldn't find clear answer so I'd rather assume not.
If it's not thread-safe then supporting multiple "iface"'s will require multiple pcap_open_live()'s and we are back in the same place.

>> I am not sure exiting behavior is intentional, which is capturing sent packages in Rx pcap handler to same interface.
>> Are you aware of any use case of existing behavior? Perhaps it can be option to set PCAP_D_IN by default for rx_iface argument.
Even if unintentional I find it very useful for testing, as this way it's very easy to send traffic to the app by tcpreplay on the same host the app is running on. 
Using tcpreplay is in the out direction that will not be captured if PCAP_D_IN is set. 
If PCAP_D_IN is the only option then it will require external device (or some networking trick) to send packets to the app.
So, I'd say it is good for testing but less good for real functionality 

-----Original Message-----
From: Ferruh Yigit <ferruh.yigit@intel.com> 
Sent: Friday, June 15, 2018 3:53 PM
To: Ido Goshen <Ido@cgstowernetworks.com>
Cc: dev@dpdk.org
Subject: Re: [PATCH v2] net/pcap: rx_iface_in stream type support

On 6/14/2018 9:44 PM, Ido Goshen wrote:
> I think we are starting to mix two things One is how to configure pcap 
> eth dev with multiple queues and I totally agree it would have been nicer to just say something like "max_tx_queues =N" instead of needing to write "tx_iface" N times, but as it was already supported in that way (for any reason?) I wasn't trying to enhance or change it.
> The other issue is pcap direction API, which I was trying to expose to users of dpdk pcap device.

Hi Ido,

Assuming "iface" argument solves the direction issue, I am suggestion adding multiqueue support to "iface" argument as a solution to your problem.

I am not suggesting using new arguments like "max_tx_queues =N", "iface" can be used same as how rx/tx_ifcase used now, provide it multiple times.

> Refer to https://www.tcpdump.org/manpages/pcap_setdirection.3pcap.txt 
> or man tcpdump for -P/--direction in|out|inout option, Actually I 
> think a more realistic emulation of a physical device (non-virtual) 
> would be to capture only the incoming direction (set PCAP_D_IN), again 
> the existing behavior is very useful too and I didn't try to change or 
> eliminate it but just add additional stream type option

I am not sure exiting behavior is intentional, which is capturing sent packages in Rx pcap handler to same interface.
Are you aware of any use case of existing behavior? Perhaps it can be option to set PCAP_D_IN by default for rx_iface argument.

> 
> -----Original Message-----
> From: Ferruh Yigit <ferruh.yigit@intel.com>
> Sent: Thursday, June 14, 2018 9:09 PM
> To: Ido Goshen <Ido@cgstowernetworks.com>
> Cc: dev@dpdk.org
> Subject: Re: [PATCH v2] net/pcap: rx_iface_in stream type support
> 
> On 6/14/2018 6:14 PM, Ido Goshen wrote:
>> I use "rx_iface","tx_iface" (and not just "iface") in order to have 
>> multiple TX queues I just gave a simplified setting with 1 queue My 
>> app  does a full mesh between the ports (not fixed pairs like l2fwd) 
>> so all the forwarding lcores can tx to the same port simultaneously and as DPDK docs say:
>> "Multiple logical cores should never share receive or transmit queues for interfaces since this would require global locks and hinder performance."
>> For example if I have 3 ports handled by 3 cores it'll be 
>> 	myapp -c 7 -n1 --no-huge \
>> 	--vdev=eth_pcap0,rx_iface=eth0,tx_iface=eth0,tx_iface=eth0,tx_iface=eth0 \
>> 	--vdev=eth_pcap0,rx_iface=eth1,tx_iface=eth1,tx_iface=eth1,tx_iface=eth1 \
>> 	--vdev=eth_pcap0,rx_iface=eth2,tx_iface=eth2,tx_iface=eth2,tx_iface=eth2 \
>> 	-- -p 7
>> Is there another way to achieve multiple queues in pcap vdev?
> 
> If you want to use multiple core you need multiple queues, as you said, and above is the way to create multiple queues for pcap.
> 
> Currently "iface" argument only supports single interface in a hardcoded way, but technically it should be possible to update it to support multiple queue.
> 
> So if "iface" arguments works for you, it can be better to add multi queue support to "iface" instead of introducing a new device argument.
> 
>>
>> I do see that using "iface" behaves differently - I'll try to 
>> investigate why
> 
> pcap_open_live() is called for both arguments, for "rx_iface/tx_iface" pair it has been called twice one for each. Not sure if pcap library returns same handler or two different handlers for this case since iface name is same.
> For "iface" argument pcap_open_live() called once, so we have single handler for both Rx & Tx. This may be difference.
> 
>> And still even when using "iface" I also see packets that are 
>> transmitted out of eth1 (e.g. tcpreplay -i eth1 packets.pcap) and not 
>> only packets that are received (e.g. ping from far end to eth0 ip)
> 
> This is interesting, I have tried with external packet generator, "iface" was working as expected for me.
> 
>>
>>
>> -----Original Message-----
>> From: Ferruh Yigit <ferruh.yigit@intel.com>
>> Sent: Wednesday, June 13, 2018 1:57 PM
>> To: Ido Goshen <Ido@cgstowernetworks.com>
>> Cc: dev@dpdk.org
>> Subject: Re: [PATCH v2] net/pcap: rx_iface_in stream type support
>>
>> On 6/5/2018 6:10 PM, Ido Goshen wrote:
>>> The problem is if a dpdk app uses the same iface(s) both as rx_iface and tx_iface then it will receive back the packets it sends.
>>> If my app sends a packet to portid=X with rte_eth_tx_burst() then I 
>>> wouldn't expect to receive it back by rte_eth_rx_burst() for that same portid=X  (assuming of course there's no external loopback) This is coming from the default nature of pcap that like a sniffer captures both incoming and outgoing direction.
>>> The patch provides an option to limit pcap rx_iface to get only incoming traffic which is more like a real (non-pcap) dpdk device.
>>>
>>> for example:
>>> when using existing *rx_iface*
>>> 	l2fwd -c 3 -n1 --no-huge
>>> --vdev=eth_pcap0,rx_iface=eth1,tx_iface=eth1
>>> --vdev=eth_pcap1,rx_iface=dummy0,tx_iface=dummy0  -- -p 3 -T 1 
>>> sending only 1 single packet into eth1 will end in an infinite loop 
>>> -
>>
>> If you are using same interface for both Rx & Tx, why not using "iface=xxx"
>> argument, can you please test with following:
>>
>> l2fwd -c 3 -n1 --no-huge --vdev=eth_pcap0,iface=eth1
>> --vdev=eth_pcap1,iface=dummy0 -- -p 3 -T 1
>>
>>
>> I can't reproduce the issue with above command.
>>
>> Thanks,
>> ferruh
>>
> 

Re: [dpdk-dev] [PATCH v2] net/pcap: rx_iface_in stream type support

[Ferruh Yigit]

On 6/16/2018 10:27 AM, Ido Goshen wrote:
> Is pcap_sendpacket() to the same pcap_t handle thread-safe? I couldn't find clear answer so I'd rather assume not.
> If it's not thread-safe then supporting multiple "iface"'s will require multiple pcap_open_live()'s and we are back in the same place.

I am not suggesting extra multi thread safety.

Currently in "iface" path, following is hardcoded:
	pcaps.num_of_queue = 1;
	dumpers.num_of_queue = 1;

It can be possible to update that path to support multiple queue while using
"iface" devargs.

> 
>>> I am not sure exiting behavior is intentional, which is capturing sent packages in Rx pcap handler to same interface.
>>> Are you aware of any use case of existing behavior? Perhaps it can be option to set PCAP_D_IN by default for rx_iface argument.
> Even if unintentional I find it very useful for testing, as this way it's very easy to send traffic to the app by tcpreplay on the same host the app is running on. 
> Using tcpreplay is in the out direction that will not be captured if PCAP_D_IN is set. 
> If PCAP_D_IN is the only option then it will require external device (or some networking trick) to send packets to the app.
> So, I'd say it is good for testing but less good for real functionality 

OK to keep it if there is a usecase.

> 
> -----Original Message-----
> From: Ferruh Yigit <ferruh.yigit@intel.com> 
> Sent: Friday, June 15, 2018 3:53 PM
> To: Ido Goshen <Ido@cgstowernetworks.com>
> Cc: dev@dpdk.org
> Subject: Re: [PATCH v2] net/pcap: rx_iface_in stream type support
> 
> On 6/14/2018 9:44 PM, Ido Goshen wrote:
>> I think we are starting to mix two things One is how to configure pcap 
>> eth dev with multiple queues and I totally agree it would have been nicer to just say something like "max_tx_queues =N" instead of needing to write "tx_iface" N times, but as it was already supported in that way (for any reason?) I wasn't trying to enhance or change it.
>> The other issue is pcap direction API, which I was trying to expose to users of dpdk pcap device.
> 
> Hi Ido,
> 
> Assuming "iface" argument solves the direction issue, I am suggestion adding multiqueue support to "iface" argument as a solution to your problem.
> 
> I am not suggesting using new arguments like "max_tx_queues =N", "iface" can be used same as how rx/tx_ifcase used now, provide it multiple times.
> 
>> Refer to https://www.tcpdump.org/manpages/pcap_setdirection.3pcap.txt 
>> or man tcpdump for -P/--direction in|out|inout option, Actually I 
>> think a more realistic emulation of a physical device (non-virtual) 
>> would be to capture only the incoming direction (set PCAP_D_IN), again 
>> the existing behavior is very useful too and I didn't try to change or 
>> eliminate it but just add additional stream type option
> 
> I am not sure exiting behavior is intentional, which is capturing sent packages in Rx pcap handler to same interface.
> Are you aware of any use case of existing behavior? Perhaps it can be option to set PCAP_D_IN by default for rx_iface argument.
> 
>>
>> -----Original Message-----
>> From: Ferruh Yigit <ferruh.yigit@intel.com>
>> Sent: Thursday, June 14, 2018 9:09 PM
>> To: Ido Goshen <Ido@cgstowernetworks.com>
>> Cc: dev@dpdk.org
>> Subject: Re: [PATCH v2] net/pcap: rx_iface_in stream type support
>>
>> On 6/14/2018 6:14 PM, Ido Goshen wrote:
>>> I use "rx_iface","tx_iface" (and not just "iface") in order to have 
>>> multiple TX queues I just gave a simplified setting with 1 queue My 
>>> app  does a full mesh between the ports (not fixed pairs like l2fwd) 
>>> so all the forwarding lcores can tx to the same port simultaneously and as DPDK docs say:
>>> "Multiple logical cores should never share receive or transmit queues for interfaces since this would require global locks and hinder performance."
>>> For example if I have 3 ports handled by 3 cores it'll be 
>>> 	myapp -c 7 -n1 --no-huge \
>>> 	--vdev=eth_pcap0,rx_iface=eth0,tx_iface=eth0,tx_iface=eth0,tx_iface=eth0 \
>>> 	--vdev=eth_pcap0,rx_iface=eth1,tx_iface=eth1,tx_iface=eth1,tx_iface=eth1 \
>>> 	--vdev=eth_pcap0,rx_iface=eth2,tx_iface=eth2,tx_iface=eth2,tx_iface=eth2 \
>>> 	-- -p 7
>>> Is there another way to achieve multiple queues in pcap vdev?
>>
>> If you want to use multiple core you need multiple queues, as you said, and above is the way to create multiple queues for pcap.
>>
>> Currently "iface" argument only supports single interface in a hardcoded way, but technically it should be possible to update it to support multiple queue.
>>
>> So if "iface" arguments works for you, it can be better to add multi queue support to "iface" instead of introducing a new device argument.
>>
>>>
>>> I do see that using "iface" behaves differently - I'll try to 
>>> investigate why
>>
>> pcap_open_live() is called for both arguments, for "rx_iface/tx_iface" pair it has been called twice one for each. Not sure if pcap library returns same handler or two different handlers for this case since iface name is same.
>> For "iface" argument pcap_open_live() called once, so we have single handler for both Rx & Tx. This may be difference.
>>
>>> And still even when using "iface" I also see packets that are 
>>> transmitted out of eth1 (e.g. tcpreplay -i eth1 packets.pcap) and not 
>>> only packets that are received (e.g. ping from far end to eth0 ip)
>>
>> This is interesting, I have tried with external packet generator, "iface" was working as expected for me.
>>
>>>
>>>
>>> -----Original Message-----
>>> From: Ferruh Yigit <ferruh.yigit@intel.com>
>>> Sent: Wednesday, June 13, 2018 1:57 PM
>>> To: Ido Goshen <Ido@cgstowernetworks.com>
>>> Cc: dev@dpdk.org
>>> Subject: Re: [PATCH v2] net/pcap: rx_iface_in stream type support
>>>
>>> On 6/5/2018 6:10 PM, Ido Goshen wrote:
>>>> The problem is if a dpdk app uses the same iface(s) both as rx_iface and tx_iface then it will receive back the packets it sends.
>>>> If my app sends a packet to portid=X with rte_eth_tx_burst() then I 
>>>> wouldn't expect to receive it back by rte_eth_rx_burst() for that same portid=X  (assuming of course there's no external loopback) This is coming from the default nature of pcap that like a sniffer captures both incoming and outgoing direction.
>>>> The patch provides an option to limit pcap rx_iface to get only incoming traffic which is more like a real (non-pcap) dpdk device.
>>>>
>>>> for example:
>>>> when using existing *rx_iface*
>>>> 	l2fwd -c 3 -n1 --no-huge
>>>> --vdev=eth_pcap0,rx_iface=eth1,tx_iface=eth1
>>>> --vdev=eth_pcap1,rx_iface=dummy0,tx_iface=dummy0  -- -p 3 -T 1 
>>>> sending only 1 single packet into eth1 will end in an infinite loop 
>>>> -
>>>
>>> If you are using same interface for both Rx & Tx, why not using "iface=xxx"
>>> argument, can you please test with following:
>>>
>>> l2fwd -c 3 -n1 --no-huge --vdev=eth_pcap0,iface=eth1
>>> --vdev=eth_pcap1,iface=dummy0 -- -p 3 -T 1
>>>
>>>
>>> I can't reproduce the issue with above command.
>>>
>>> Thanks,
>>> ferruh
>>>
>>
> 

Re: [dpdk-dev] [PATCH v2] net/pcap: rx_iface_in stream type support

[Ido Goshen]

I'm really not sure that just setting the pcaps/dumpers.num_of_queue w/o actually creating multiple queues is good enough.
If one uses N queues there are good changes he is using N cores.
To be consistent with DPDK behavior it should be safe to concurrently tx to different queues.
If pcap_sendpacket to the same pcap_t handle is not thread safe then it will require to pcap_open_live() for each queue
If using multiple pcap_open_live()  then it will cause the rx out direction problem again


-----Original Message-----
From: Ferruh Yigit <ferruh.yigit@intel.com> 
Sent: Monday, June 18, 2018 11:13 AM
To: Ido Goshen <Ido@cgstowernetworks.com>
Cc: dev@dpdk.org
Subject: Re: [PATCH v2] net/pcap: rx_iface_in stream type support

On 6/16/2018 10:27 AM, Ido Goshen wrote:
> Is pcap_sendpacket() to the same pcap_t handle thread-safe? I couldn't find clear answer so I'd rather assume not.
> If it's not thread-safe then supporting multiple "iface"'s will require multiple pcap_open_live()'s and we are back in the same place.

I am not suggesting extra multi thread safety.

Currently in "iface" path, following is hardcoded:
	pcaps.num_of_queue = 1;
	dumpers.num_of_queue = 1;

It can be possible to update that path to support multiple queue while using "iface" devargs.

> 
>>> I am not sure exiting behavior is intentional, which is capturing sent packages in Rx pcap handler to same interface.
>>> Are you aware of any use case of existing behavior? Perhaps it can be option to set PCAP_D_IN by default for rx_iface argument.
> Even if unintentional I find it very useful for testing, as this way it's very easy to send traffic to the app by tcpreplay on the same host the app is running on. 
> Using tcpreplay is in the out direction that will not be captured if PCAP_D_IN is set. 
> If PCAP_D_IN is the only option then it will require external device (or some networking trick) to send packets to the app.
> So, I'd say it is good for testing but less good for real 
> functionality

OK to keep it if there is a usecase.

> 
> -----Original Message-----
> From: Ferruh Yigit <ferruh.yigit@intel.com>
> Sent: Friday, June 15, 2018 3:53 PM
> To: Ido Goshen <Ido@cgstowernetworks.com>
> Cc: dev@dpdk.org
> Subject: Re: [PATCH v2] net/pcap: rx_iface_in stream type support
> 
> On 6/14/2018 9:44 PM, Ido Goshen wrote:
>> I think we are starting to mix two things One is how to configure 
>> pcap eth dev with multiple queues and I totally agree it would have been nicer to just say something like "max_tx_queues =N" instead of needing to write "tx_iface" N times, but as it was already supported in that way (for any reason?) I wasn't trying to enhance or change it.
>> The other issue is pcap direction API, which I was trying to expose to users of dpdk pcap device.
> 
> Hi Ido,
> 
> Assuming "iface" argument solves the direction issue, I am suggestion adding multiqueue support to "iface" argument as a solution to your problem.
> 
> I am not suggesting using new arguments like "max_tx_queues =N", "iface" can be used same as how rx/tx_ifcase used now, provide it multiple times.
> 
>> Refer to https://www.tcpdump.org/manpages/pcap_setdirection.3pcap.txt
>> or man tcpdump for -P/--direction in|out|inout option, Actually I 
>> think a more realistic emulation of a physical device (non-virtual) 
>> would be to capture only the incoming direction (set PCAP_D_IN), 
>> again the existing behavior is very useful too and I didn't try to 
>> change or eliminate it but just add additional stream type option
> 
> I am not sure exiting behavior is intentional, which is capturing sent packages in Rx pcap handler to same interface.
> Are you aware of any use case of existing behavior? Perhaps it can be option to set PCAP_D_IN by default for rx_iface argument.
> 
>>
>> -----Original Message-----
>> From: Ferruh Yigit <ferruh.yigit@intel.com>
>> Sent: Thursday, June 14, 2018 9:09 PM
>> To: Ido Goshen <Ido@cgstowernetworks.com>
>> Cc: dev@dpdk.org
>> Subject: Re: [PATCH v2] net/pcap: rx_iface_in stream type support
>>
>> On 6/14/2018 6:14 PM, Ido Goshen wrote:
>>> I use "rx_iface","tx_iface" (and not just "iface") in order to have 
>>> multiple TX queues I just gave a simplified setting with 1 queue My 
>>> app  does a full mesh between the ports (not fixed pairs like l2fwd) 
>>> so all the forwarding lcores can tx to the same port simultaneously and as DPDK docs say:
>>> "Multiple logical cores should never share receive or transmit queues for interfaces since this would require global locks and hinder performance."
>>> For example if I have 3 ports handled by 3 cores it'll be 
>>> 	myapp -c 7 -n1 --no-huge \
>>> 	--vdev=eth_pcap0,rx_iface=eth0,tx_iface=eth0,tx_iface=eth0,tx_iface=eth0 \
>>> 	--vdev=eth_pcap0,rx_iface=eth1,tx_iface=eth1,tx_iface=eth1,tx_iface=eth1 \
>>> 	--vdev=eth_pcap0,rx_iface=eth2,tx_iface=eth2,tx_iface=eth2,tx_iface=eth2 \
>>> 	-- -p 7
>>> Is there another way to achieve multiple queues in pcap vdev?
>>
>> If you want to use multiple core you need multiple queues, as you said, and above is the way to create multiple queues for pcap.
>>
>> Currently "iface" argument only supports single interface in a hardcoded way, but technically it should be possible to update it to support multiple queue.
>>
>> So if "iface" arguments works for you, it can be better to add multi queue support to "iface" instead of introducing a new device argument.
>>
>>>
>>> I do see that using "iface" behaves differently - I'll try to 
>>> investigate why
>>
>> pcap_open_live() is called for both arguments, for "rx_iface/tx_iface" pair it has been called twice one for each. Not sure if pcap library returns same handler or two different handlers for this case since iface name is same.
>> For "iface" argument pcap_open_live() called once, so we have single handler for both Rx & Tx. This may be difference.
>>
>>> And still even when using "iface" I also see packets that are 
>>> transmitted out of eth1 (e.g. tcpreplay -i eth1 packets.pcap) and 
>>> not only packets that are received (e.g. ping from far end to eth0 
>>> ip)
>>
>> This is interesting, I have tried with external packet generator, "iface" was working as expected for me.
>>
>>>
>>>
>>> -----Original Message-----
>>> From: Ferruh Yigit <ferruh.yigit@intel.com>
>>> Sent: Wednesday, June 13, 2018 1:57 PM
>>> To: Ido Goshen <Ido@cgstowernetworks.com>
>>> Cc: dev@dpdk.org
>>> Subject: Re: [PATCH v2] net/pcap: rx_iface_in stream type support
>>>
>>> On 6/5/2018 6:10 PM, Ido Goshen wrote:
>>>> The problem is if a dpdk app uses the same iface(s) both as rx_iface and tx_iface then it will receive back the packets it sends.
>>>> If my app sends a packet to portid=X with rte_eth_tx_burst() then I 
>>>> wouldn't expect to receive it back by rte_eth_rx_burst() for that same portid=X  (assuming of course there's no external loopback) This is coming from the default nature of pcap that like a sniffer captures both incoming and outgoing direction.
>>>> The patch provides an option to limit pcap rx_iface to get only incoming traffic which is more like a real (non-pcap) dpdk device.
>>>>
>>>> for example:
>>>> when using existing *rx_iface*
>>>> 	l2fwd -c 3 -n1 --no-huge
>>>> --vdev=eth_pcap0,rx_iface=eth1,tx_iface=eth1
>>>> --vdev=eth_pcap1,rx_iface=dummy0,tx_iface=dummy0  -- -p 3 -T 1 
>>>> sending only 1 single packet into eth1 will end in an infinite loop
>>>> -
>>>
>>> If you are using same interface for both Rx & Tx, why not using "iface=xxx"
>>> argument, can you please test with following:
>>>
>>> l2fwd -c 3 -n1 --no-huge --vdev=eth_pcap0,iface=eth1
>>> --vdev=eth_pcap1,iface=dummy0 -- -p 3 -T 1
>>>
>>>
>>> I can't reproduce the issue with above command.
>>>
>>> Thanks,
>>> ferruh
>>>
>>
> 

Re: [dpdk-dev] [PATCH v2] net/pcap: rx_iface_in stream type support

[Ferruh Yigit]

On 6/18/2018 10:49 AM, Ido Goshen wrote:
> I'm really not sure that just setting the pcaps/dumpers.num_of_queue w/o actually creating multiple queues is good enough.
> If one uses N queues there are good changes he is using N cores.
> To be consistent with DPDK behavior it should be safe to concurrently tx to different queues.
> If pcap_sendpacket to the same pcap_t handle is not thread safe then it will require to pcap_open_live() for each queue
> If using multiple pcap_open_live()  then it will cause the rx out direction problem again

Please do not reply top post, this thread become very hard to follow.

I am not suggesting just increase num_of_queue, simply what I was suggesting it
add capability to create multiple queues by using "iface" devarg, that is all.

This should solve your problem because with "iface" devarg I don't get Tx
packets back with my Rx handler. This has a downside that you create queues as
pairs, you can't get 4Rx 1Tx queues etc..

So I agree with your suggestion, but there was some comments to your initial
patch [1], can you please address them and send a new version?

[1]
https://mails.dpdk.org/archives/dev/2018-June/103476.html

Thanks.

> 
> -----Original Message-----
> From: Ferruh Yigit <ferruh.yigit@intel.com> 
> Sent: Monday, June 18, 2018 11:13 AM
> To: Ido Goshen <Ido@cgstowernetworks.com>
> Cc: dev@dpdk.org
> Subject: Re: [PATCH v2] net/pcap: rx_iface_in stream type support
> 
> On 6/16/2018 10:27 AM, Ido Goshen wrote:
>> Is pcap_sendpacket() to the same pcap_t handle thread-safe? I couldn't find clear answer so I'd rather assume not.
>> If it's not thread-safe then supporting multiple "iface"'s will require multiple pcap_open_live()'s and we are back in the same place.
> 
> I am not suggesting extra multi thread safety.
> 
> Currently in "iface" path, following is hardcoded:
> 	pcaps.num_of_queue = 1;
> 	dumpers.num_of_queue = 1;
> 
> It can be possible to update that path to support multiple queue while using "iface" devargs.
> 
>>
>>>> I am not sure exiting behavior is intentional, which is capturing sent packages in Rx pcap handler to same interface.
>>>> Are you aware of any use case of existing behavior? Perhaps it can be option to set PCAP_D_IN by default for rx_iface argument.
>> Even if unintentional I find it very useful for testing, as this way it's very easy to send traffic to the app by tcpreplay on the same host the app is running on. 
>> Using tcpreplay is in the out direction that will not be captured if PCAP_D_IN is set. 
>> If PCAP_D_IN is the only option then it will require external device (or some networking trick) to send packets to the app.
>> So, I'd say it is good for testing but less good for real 
>> functionality
> 
> OK to keep it if there is a usecase.
> 
>>
>> -----Original Message-----
>> From: Ferruh Yigit <ferruh.yigit@intel.com>
>> Sent: Friday, June 15, 2018 3:53 PM
>> To: Ido Goshen <Ido@cgstowernetworks.com>
>> Cc: dev@dpdk.org
>> Subject: Re: [PATCH v2] net/pcap: rx_iface_in stream type support
>>
>> On 6/14/2018 9:44 PM, Ido Goshen wrote:
>>> I think we are starting to mix two things One is how to configure 
>>> pcap eth dev with multiple queues and I totally agree it would have been nicer to just say something like "max_tx_queues =N" instead of needing to write "tx_iface" N times, but as it was already supported in that way (for any reason?) I wasn't trying to enhance or change it.
>>> The other issue is pcap direction API, which I was trying to expose to users of dpdk pcap device.
>>
>> Hi Ido,
>>
>> Assuming "iface" argument solves the direction issue, I am suggestion adding multiqueue support to "iface" argument as a solution to your problem.
>>
>> I am not suggesting using new arguments like "max_tx_queues =N", "iface" can be used same as how rx/tx_ifcase used now, provide it multiple times.
>>
>>> Refer to https://www.tcpdump.org/manpages/pcap_setdirection.3pcap.txt
>>> or man tcpdump for -P/--direction in|out|inout option, Actually I 
>>> think a more realistic emulation of a physical device (non-virtual) 
>>> would be to capture only the incoming direction (set PCAP_D_IN), 
>>> again the existing behavior is very useful too and I didn't try to 
>>> change or eliminate it but just add additional stream type option
>>
>> I am not sure exiting behavior is intentional, which is capturing sent packages in Rx pcap handler to same interface.
>> Are you aware of any use case of existing behavior? Perhaps it can be option to set PCAP_D_IN by default for rx_iface argument.
>>
>>>
>>> -----Original Message-----
>>> From: Ferruh Yigit <ferruh.yigit@intel.com>
>>> Sent: Thursday, June 14, 2018 9:09 PM
>>> To: Ido Goshen <Ido@cgstowernetworks.com>
>>> Cc: dev@dpdk.org
>>> Subject: Re: [PATCH v2] net/pcap: rx_iface_in stream type support
>>>
>>> On 6/14/2018 6:14 PM, Ido Goshen wrote:
>>>> I use "rx_iface","tx_iface" (and not just "iface") in order to have 
>>>> multiple TX queues I just gave a simplified setting with 1 queue My 
>>>> app  does a full mesh between the ports (not fixed pairs like l2fwd) 
>>>> so all the forwarding lcores can tx to the same port simultaneously and as DPDK docs say:
>>>> "Multiple logical cores should never share receive or transmit queues for interfaces since this would require global locks and hinder performance."
>>>> For example if I have 3 ports handled by 3 cores it'll be 
>>>> 	myapp -c 7 -n1 --no-huge \
>>>> 	--vdev=eth_pcap0,rx_iface=eth0,tx_iface=eth0,tx_iface=eth0,tx_iface=eth0 \
>>>> 	--vdev=eth_pcap0,rx_iface=eth1,tx_iface=eth1,tx_iface=eth1,tx_iface=eth1 \
>>>> 	--vdev=eth_pcap0,rx_iface=eth2,tx_iface=eth2,tx_iface=eth2,tx_iface=eth2 \
>>>> 	-- -p 7
>>>> Is there another way to achieve multiple queues in pcap vdev?
>>>
>>> If you want to use multiple core you need multiple queues, as you said, and above is the way to create multiple queues for pcap.
>>>
>>> Currently "iface" argument only supports single interface in a hardcoded way, but technically it should be possible to update it to support multiple queue.
>>>
>>> So if "iface" arguments works for you, it can be better to add multi queue support to "iface" instead of introducing a new device argument.
>>>
>>>>
>>>> I do see that using "iface" behaves differently - I'll try to 
>>>> investigate why
>>>
>>> pcap_open_live() is called for both arguments, for "rx_iface/tx_iface" pair it has been called twice one for each. Not sure if pcap library returns same handler or two different handlers for this case since iface name is same.
>>> For "iface" argument pcap_open_live() called once, so we have single handler for both Rx & Tx. This may be difference.
>>>
>>>> And still even when using "iface" I also see packets that are 
>>>> transmitted out of eth1 (e.g. tcpreplay -i eth1 packets.pcap) and 
>>>> not only packets that are received (e.g. ping from far end to eth0 
>>>> ip)
>>>
>>> This is interesting, I have tried with external packet generator, "iface" was working as expected for me.
>>>
>>>>
>>>>
>>>> -----Original Message-----
>>>> From: Ferruh Yigit <ferruh.yigit@intel.com>
>>>> Sent: Wednesday, June 13, 2018 1:57 PM
>>>> To: Ido Goshen <Ido@cgstowernetworks.com>
>>>> Cc: dev@dpdk.org
>>>> Subject: Re: [PATCH v2] net/pcap: rx_iface_in stream type support
>>>>
>>>> On 6/5/2018 6:10 PM, Ido Goshen wrote:
>>>>> The problem is if a dpdk app uses the same iface(s) both as rx_iface and tx_iface then it will receive back the packets it sends.
>>>>> If my app sends a packet to portid=X with rte_eth_tx_burst() then I 
>>>>> wouldn't expect to receive it back by rte_eth_rx_burst() for that same portid=X  (assuming of course there's no external loopback) This is coming from the default nature of pcap that like a sniffer captures both incoming and outgoing direction.
>>>>> The patch provides an option to limit pcap rx_iface to get only incoming traffic which is more like a real (non-pcap) dpdk device.
>>>>>
>>>>> for example:
>>>>> when using existing *rx_iface*
>>>>> 	l2fwd -c 3 -n1 --no-huge
>>>>> --vdev=eth_pcap0,rx_iface=eth1,tx_iface=eth1
>>>>> --vdev=eth_pcap1,rx_iface=dummy0,tx_iface=dummy0  -- -p 3 -T 1 
>>>>> sending only 1 single packet into eth1 will end in an infinite loop
>>>>> -
>>>>
>>>> If you are using same interface for both Rx & Tx, why not using "iface=xxx"
>>>> argument, can you please test with following:
>>>>
>>>> l2fwd -c 3 -n1 --no-huge --vdev=eth_pcap0,iface=eth1
>>>> --vdev=eth_pcap1,iface=dummy0 -- -p 3 -T 1
>>>>
>>>>
>>>> I can't reproduce the issue with above command.
>>>>
>>>> Thanks,
>>>> ferruh
>>>>
>>>
>>
>