DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH 0/3] Multiseg fixes for pcap pmd
@ 2019-07-24 11:54 David Marchand
  2019-07-24 11:54 ` [dpdk-dev] [PATCH 1/3] net/pcap: fix Rx with small buffers David Marchand
                   ` (4 more replies)
  0 siblings, 5 replies; 23+ messages in thread
From: David Marchand @ 2019-07-24 11:54 UTC (permalink / raw)
  To: dev; +Cc: ferruh.yigit

Here are some fixes caught while looking at oerrors statistics for this
driver.
The second patch can be seen as a revert or a followup of [1].

1: https://git.dpdk.org/dpdk/commit/?id=49a0a2ffd5db

-- 
David Marchand

David Marchand (3):
  net/pcap: fix Rx with small buffers
  net/pcap: fix transmit return count in error conditions
  net/pcap: fix concurrent multiseg packet transmits

 drivers/net/pcap/rte_eth_pcap.c | 103 +++++++++++++++-------------------------
 1 file changed, 37 insertions(+), 66 deletions(-)

-- 
1.8.3.1


^ permalink raw reply	[flat|nested] 23+ messages in thread

* [dpdk-dev] [PATCH 1/3] net/pcap: fix Rx with small buffers
  2019-07-24 11:54 [dpdk-dev] [PATCH 0/3] Multiseg fixes for pcap pmd David Marchand
@ 2019-07-24 11:54 ` David Marchand
  2019-07-24 18:28   ` Ferruh Yigit
  2019-07-24 11:54 ` [dpdk-dev] [PATCH 2/3] net/pcap: fix transmit return count in error conditions David Marchand
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 23+ messages in thread
From: David Marchand @ 2019-07-24 11:54 UTC (permalink / raw)
  To: dev; +Cc: ferruh.yigit, stable

If the pkt pool contains only buffers smaller than the default headroom,
then the driver will compute an invalid buffer size (negative value cast
to an uint16_t).
Rely on the mbuf api to check how much space is available in the mbuf.

Fixes: 6eb0ae218a98 ("pcap: fix mbuf allocation")
Cc: stable@dpdk.org

Signed-off-by: David Marchand <david.marchand@redhat.com>
---
 drivers/net/pcap/rte_eth_pcap.c | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/drivers/net/pcap/rte_eth_pcap.c b/drivers/net/pcap/rte_eth_pcap.c
index 322c18f..470867d 100644
--- a/drivers/net/pcap/rte_eth_pcap.c
+++ b/drivers/net/pcap/rte_eth_pcap.c
@@ -242,7 +242,6 @@ struct pmd_devargs_all {
 	struct rte_mbuf *mbuf;
 	struct pcap_rx_queue *pcap_q = queue;
 	uint16_t num_rx = 0;
-	uint16_t buf_size;
 	uint32_t rx_bytes = 0;
 	pcap_t *pcap;
 
@@ -265,11 +264,7 @@ struct pmd_devargs_all {
 		if (unlikely(mbuf == NULL))
 			break;
 
-		/* Now get the space available for data in the mbuf */
-		buf_size = rte_pktmbuf_data_room_size(pcap_q->mb_pool) -
-				RTE_PKTMBUF_HEADROOM;
-
-		if (header.caplen <= buf_size) {
+		if (header.caplen <= rte_pktmbuf_tailroom(mbuf)) {
 			/* pcap packet will fit in the mbuf, can copy it */
 			rte_memcpy(rte_pktmbuf_mtod(mbuf, void *), packet,
 					header.caplen);
-- 
1.8.3.1


^ permalink raw reply	[flat|nested] 23+ messages in thread

* [dpdk-dev] [PATCH 2/3] net/pcap: fix transmit return count in error conditions
  2019-07-24 11:54 [dpdk-dev] [PATCH 0/3] Multiseg fixes for pcap pmd David Marchand
  2019-07-24 11:54 ` [dpdk-dev] [PATCH 1/3] net/pcap: fix Rx with small buffers David Marchand
@ 2019-07-24 11:54 ` David Marchand
  2019-07-24 18:36   ` Ferruh Yigit
  2019-07-24 11:54 ` [dpdk-dev] [PATCH 3/3] net/pcap: fix concurrent multiseg packet transmits David Marchand
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 23+ messages in thread
From: David Marchand @ 2019-07-24 11:54 UTC (permalink / raw)
  To: dev; +Cc: ferruh.yigit, stable

When a packet cannot be transmitted, the driver is supposed to free this
packet and report it as handled.
This is to prevent the application from retrying to send the same packet
and ending up in a liveloop since the driver will never manage to send
it.

Fixes: 49a0a2ffd5db ("net/pcap: fix possible mbuf double freeing")
Fixes: 6db141c91e1f ("pcap: support jumbo frames")
CC: stable@dpdk.org

Signed-off-by: David Marchand <david.marchand@redhat.com>
---
 drivers/net/pcap/rte_eth_pcap.c | 18 ++++++++++--------
 1 file changed, 10 insertions(+), 8 deletions(-)

diff --git a/drivers/net/pcap/rte_eth_pcap.c b/drivers/net/pcap/rte_eth_pcap.c
index 470867d..5e5aab7 100644
--- a/drivers/net/pcap/rte_eth_pcap.c
+++ b/drivers/net/pcap/rte_eth_pcap.c
@@ -354,7 +354,8 @@ struct pmd_devargs_all {
 					mbuf->pkt_len,
 					RTE_ETHER_MAX_JUMBO_FRAME_LEN);
 
-				break;
+				rte_pktmbuf_free(mbuf);
+				continue;
 			}
 		}
 
@@ -373,7 +374,7 @@ struct pmd_devargs_all {
 	dumper_q->tx_stat.bytes += tx_bytes;
 	dumper_q->tx_stat.err_pkts += nb_pkts - num_tx;
 
-	return num_tx;
+	return nb_pkts;
 }
 
 /*
@@ -439,14 +440,15 @@ struct pmd_devargs_all {
 					mbuf->pkt_len,
 					RTE_ETHER_MAX_JUMBO_FRAME_LEN);
 
-				break;
+				rte_pktmbuf_free(mbuf);
+				continue;
 			}
 		}
 
-		if (unlikely(ret != 0))
-			break;
-		num_tx++;
-		tx_bytes += mbuf->pkt_len;
+		if (ret == 0) {
+			num_tx++;
+			tx_bytes += mbuf->pkt_len;
+		}
 		rte_pktmbuf_free(mbuf);
 	}
 
@@ -454,7 +456,7 @@ struct pmd_devargs_all {
 	tx_queue->tx_stat.bytes += tx_bytes;
 	tx_queue->tx_stat.err_pkts += nb_pkts - num_tx;
 
-	return num_tx;
+	return nb_pkts;
 }
 
 /*
-- 
1.8.3.1


^ permalink raw reply	[flat|nested] 23+ messages in thread

* [dpdk-dev] [PATCH 3/3] net/pcap: fix concurrent multiseg packet transmits
  2019-07-24 11:54 [dpdk-dev] [PATCH 0/3] Multiseg fixes for pcap pmd David Marchand
  2019-07-24 11:54 ` [dpdk-dev] [PATCH 1/3] net/pcap: fix Rx with small buffers David Marchand
  2019-07-24 11:54 ` [dpdk-dev] [PATCH 2/3] net/pcap: fix transmit return count in error conditions David Marchand
@ 2019-07-24 11:54 ` David Marchand
  2019-07-24 18:43   ` Ferruh Yigit
  2019-07-25  8:18   ` [dpdk-dev] [dpdk-stable] " David Marchand
  2019-07-25 12:04 ` [dpdk-dev] [PATCH v2 0/3] Multiseg fixes for pcap pmd David Marchand
  2019-07-25 19:24 ` [dpdk-dev] [PATCH v3 0/3] Multiseg fixes for pcap pmd David Marchand
  4 siblings, 2 replies; 23+ messages in thread
From: David Marchand @ 2019-07-24 11:54 UTC (permalink / raw)
  To: dev; +Cc: ferruh.yigit, stable

Two cores can send multi segment packets on two different pcap ports.
Because of this, we can't have one single buffer to linearize packets.

Use rte_pktmbuf_read() to copy the packet into a buffer on the stack
and remove eth_pcap_gather_data().

Fixes: 6db141c91e1f ("pcap: support jumbo frames")
Cc: stable@dpdk.org

Signed-off-by: David Marchand <david.marchand@redhat.com>
---
 drivers/net/pcap/rte_eth_pcap.c | 90 +++++++++++++++--------------------------
 1 file changed, 32 insertions(+), 58 deletions(-)

diff --git a/drivers/net/pcap/rte_eth_pcap.c b/drivers/net/pcap/rte_eth_pcap.c
index 5e5aab7..7398b1b 100644
--- a/drivers/net/pcap/rte_eth_pcap.c
+++ b/drivers/net/pcap/rte_eth_pcap.c
@@ -46,7 +46,6 @@
 #define RTE_PMD_PCAP_MAX_QUEUES 16
 
 static char errbuf[PCAP_ERRBUF_SIZE];
-static unsigned char tx_pcap_data[RTE_ETH_PCAP_SNAPLEN];
 static struct timeval start_time;
 static uint64_t start_cycles;
 static uint64_t hz;
@@ -180,21 +179,6 @@ struct pmd_devargs_all {
 	return mbuf->nb_segs;
 }
 
-/* Copy data from mbuf chain to a buffer suitable for writing to a PCAP file. */
-static void
-eth_pcap_gather_data(unsigned char *data, struct rte_mbuf *mbuf)
-{
-	uint16_t data_len = 0;
-
-	while (mbuf) {
-		rte_memcpy(data + data_len, rte_pktmbuf_mtod(mbuf, void *),
-			mbuf->data_len);
-
-		data_len += mbuf->data_len;
-		mbuf = mbuf->next;
-	}
-}
-
 static uint16_t
 eth_pcap_rx_infinite(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts)
 {
@@ -325,6 +309,9 @@ struct pmd_devargs_all {
 	uint32_t tx_bytes = 0;
 	struct pcap_pkthdr header;
 	pcap_dumper_t *dumper;
+	unsigned char _data[RTE_ETH_PCAP_SNAPLEN];
+	const unsigned char *data;
+	size_t len;
 
 	pp = rte_eth_devices[dumper_q->port_id].process_private;
 	dumper = pp->tx_dumper[dumper_q->queue_id];
@@ -336,31 +323,25 @@ struct pmd_devargs_all {
 	 * dumper */
 	for (i = 0; i < nb_pkts; i++) {
 		mbuf = bufs[i];
-		calculate_timestamp(&header.ts);
-		header.len = mbuf->pkt_len;
-		header.caplen = header.len;
-
-		if (likely(mbuf->nb_segs == 1)) {
-			pcap_dump((u_char *)dumper, &header,
-				  rte_pktmbuf_mtod(mbuf, void*));
+		len = rte_pktmbuf_pkt_len(mbuf);
+		if (likely(rte_pktmbuf_is_contiguous(mbuf))) {
+			data = rte_pktmbuf_mtod(mbuf, unsigned char *);
+		} else if (len <= sizeof(_data)) {
+			data = rte_pktmbuf_read(mbuf, 0, len, _data);
 		} else {
-			if (mbuf->pkt_len <= RTE_ETHER_MAX_JUMBO_FRAME_LEN) {
-				eth_pcap_gather_data(tx_pcap_data, mbuf);
-				pcap_dump((u_char *)dumper, &header,
-					  tx_pcap_data);
-			} else {
-				PMD_LOG(ERR,
-					"Dropping PCAP packet. Size (%d) > max jumbo size (%d).",
-					mbuf->pkt_len,
-					RTE_ETHER_MAX_JUMBO_FRAME_LEN);
-
-				rte_pktmbuf_free(mbuf);
-				continue;
-			}
+			PMD_LOG(ERR, "Dropping PCAP packet. Size (%zd) > max size (%zd).",
+				len, sizeof(_data));
+			rte_pktmbuf_free(mbuf);
+			continue;
 		}
 
+		calculate_timestamp(&header.ts);
+		header.len = len;
+		header.caplen = header.len;
+		pcap_dump((u_char *)dumper, &header, data);
+
 		num_tx++;
-		tx_bytes += mbuf->pkt_len;
+		tx_bytes += len;
 		rte_pktmbuf_free(mbuf);
 	}
 
@@ -408,13 +389,15 @@ struct pmd_devargs_all {
 eth_pcap_tx(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts)
 {
 	unsigned int i;
-	int ret;
 	struct rte_mbuf *mbuf;
 	struct pmd_process_private *pp;
 	struct pcap_tx_queue *tx_queue = queue;
 	uint16_t num_tx = 0;
 	uint32_t tx_bytes = 0;
 	pcap_t *pcap;
+	unsigned char _data[RTE_ETH_PCAP_SNAPLEN];
+	const unsigned char *data;
+	size_t len;
 
 	pp = rte_eth_devices[tx_queue->port_id].process_private;
 	pcap = pp->tx_pcap[tx_queue->queue_id];
@@ -424,30 +407,21 @@ struct pmd_devargs_all {
 
 	for (i = 0; i < nb_pkts; i++) {
 		mbuf = bufs[i];
-
-		if (likely(mbuf->nb_segs == 1)) {
-			ret = pcap_sendpacket(pcap,
-					rte_pktmbuf_mtod(mbuf, u_char *),
-					mbuf->pkt_len);
+		len = rte_pktmbuf_pkt_len(mbuf);
+		if (likely(rte_pktmbuf_is_contiguous(mbuf))) {
+			data = rte_pktmbuf_mtod(mbuf, unsigned char *);
+		} else if (len <= sizeof(_data)) {
+			data = rte_pktmbuf_read(mbuf, 0, len, _data);
 		} else {
-			if (mbuf->pkt_len <= RTE_ETHER_MAX_JUMBO_FRAME_LEN) {
-				eth_pcap_gather_data(tx_pcap_data, mbuf);
-				ret = pcap_sendpacket(pcap,
-						tx_pcap_data, mbuf->pkt_len);
-			} else {
-				PMD_LOG(ERR,
-					"Dropping PCAP packet. Size (%d) > max jumbo size (%d).",
-					mbuf->pkt_len,
-					RTE_ETHER_MAX_JUMBO_FRAME_LEN);
-
-				rte_pktmbuf_free(mbuf);
-				continue;
-			}
+			PMD_LOG(ERR, "Dropping PCAP packet. Size (%zd) > max size (%zd).",
+				len, sizeof(_data));
+			rte_pktmbuf_free(mbuf);
+			continue;
 		}
 
-		if (ret == 0) {
+		if (pcap_sendpacket(pcap, data, len) == 0) {
 			num_tx++;
-			tx_bytes += mbuf->pkt_len;
+			tx_bytes += len;
 		}
 		rte_pktmbuf_free(mbuf);
 	}
-- 
1.8.3.1


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [dpdk-dev] [PATCH 1/3] net/pcap: fix Rx with small buffers
  2019-07-24 11:54 ` [dpdk-dev] [PATCH 1/3] net/pcap: fix Rx with small buffers David Marchand
@ 2019-07-24 18:28   ` Ferruh Yigit
  0 siblings, 0 replies; 23+ messages in thread
From: Ferruh Yigit @ 2019-07-24 18:28 UTC (permalink / raw)
  To: David Marchand, dev; +Cc: stable

On 7/24/2019 12:54 PM, David Marchand wrote:
> If the pkt pool contains only buffers smaller than the default headroom,
> then the driver will compute an invalid buffer size (negative value cast
> to an uint16_t).
> Rely on the mbuf api to check how much space is available in the mbuf.
> 
> Fixes: 6eb0ae218a98 ("pcap: fix mbuf allocation")
> Cc: stable@dpdk.org
> 
> Signed-off-by: David Marchand <david.marchand@redhat.com>

Acked-by: Ferruh Yigit <ferruh.yigit@intel.com>

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [dpdk-dev] [PATCH 2/3] net/pcap: fix transmit return count in error conditions
  2019-07-24 11:54 ` [dpdk-dev] [PATCH 2/3] net/pcap: fix transmit return count in error conditions David Marchand
@ 2019-07-24 18:36   ` Ferruh Yigit
  2019-07-25  7:40     ` David Marchand
  0 siblings, 1 reply; 23+ messages in thread
From: Ferruh Yigit @ 2019-07-24 18:36 UTC (permalink / raw)
  To: David Marchand, dev; +Cc: stable, A.McLoughlin

On 7/24/2019 12:54 PM, David Marchand wrote:
> When a packet cannot be transmitted, the driver is supposed to free this
> packet and report it as handled.
> This is to prevent the application from retrying to send the same packet
> and ending up in a liveloop since the driver will never manage to send
> it.
> 
> Fixes: 49a0a2ffd5db ("net/pcap: fix possible mbuf double freeing")
> Fixes: 6db141c91e1f ("pcap: support jumbo frames")
> CC: stable@dpdk.org
> 
> Signed-off-by: David Marchand <david.marchand@redhat.com>
> ---
>  drivers/net/pcap/rte_eth_pcap.c | 18 ++++++++++--------
>  1 file changed, 10 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/net/pcap/rte_eth_pcap.c b/drivers/net/pcap/rte_eth_pcap.c
> index 470867d..5e5aab7 100644
> --- a/drivers/net/pcap/rte_eth_pcap.c
> +++ b/drivers/net/pcap/rte_eth_pcap.c
> @@ -354,7 +354,8 @@ struct pmd_devargs_all {
>  					mbuf->pkt_len,
>  					RTE_ETHER_MAX_JUMBO_FRAME_LEN);
>  
> -				break;
> +				rte_pktmbuf_free(mbuf);
> +				continue;

+1
Very recently 'rte_pktmbuf_free()' was moved because it wasn't compatible with
return value, but this looks better, to free the mbuf and record it as error.

>  			}
>  		}
>  
> @@ -373,7 +374,7 @@ struct pmd_devargs_all {
>  	dumper_q->tx_stat.bytes += tx_bytes;
>  	dumper_q->tx_stat.err_pkts += nb_pkts - num_tx;
>  
> -	return num_tx;
> +	return nb_pkts;
>  }
>  
>  /*
> @@ -439,14 +440,15 @@ struct pmd_devargs_all {
>  					mbuf->pkt_len,
>  					RTE_ETHER_MAX_JUMBO_FRAME_LEN);
>  
> -				break;
> +				rte_pktmbuf_free(mbuf);
> +				continue;
>  			}
>  		}
>  
> -		if (unlikely(ret != 0))
> -			break;
> -		num_tx++;
> -		tx_bytes += mbuf->pkt_len;
> +		if (ret == 0) {
> +			num_tx++;
> +			tx_bytes += mbuf->pkt_len;
> +		}

I don't know this part, this is in 'eth_pcap_tx()' which writes packets to the
interfaces.

if 'pcap_sendpacket()' fails this doesn't mean packet can't be sent and may
cause a liveloop. Why not keep the existing behavior and let application to decide?

>  		rte_pktmbuf_free(mbuf);
>  	}
>  
> @@ -454,7 +456,7 @@ struct pmd_devargs_all {
>  	tx_queue->tx_stat.bytes += tx_bytes;
>  	tx_queue->tx_stat.err_pkts += nb_pkts - num_tx;
>  
> -	return num_tx;
> +	return nb_pkts;
>  }
>  
>  /*
> 


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [dpdk-dev] [PATCH 3/3] net/pcap: fix concurrent multiseg packet transmits
  2019-07-24 11:54 ` [dpdk-dev] [PATCH 3/3] net/pcap: fix concurrent multiseg packet transmits David Marchand
@ 2019-07-24 18:43   ` Ferruh Yigit
  2019-07-25  8:18   ` [dpdk-dev] [dpdk-stable] " David Marchand
  1 sibling, 0 replies; 23+ messages in thread
From: Ferruh Yigit @ 2019-07-24 18:43 UTC (permalink / raw)
  To: David Marchand, dev; +Cc: stable

On 7/24/2019 12:54 PM, David Marchand wrote:
> Two cores can send multi segment packets on two different pcap ports.
> Because of this, we can't have one single buffer to linearize packets.

+1, you are right.

> 
> Use rte_pktmbuf_read() to copy the packet into a buffer on the stack
> and remove eth_pcap_gather_data().

+1

> 
> Fixes: 6db141c91e1f ("pcap: support jumbo frames")
> Cc: stable@dpdk.org
> 
> Signed-off-by: David Marchand <david.marchand@redhat.com>

Acked-by: Ferruh Yigit <ferruh.yigit@intel.com>

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [dpdk-dev] [PATCH 2/3] net/pcap: fix transmit return count in error conditions
  2019-07-24 18:36   ` Ferruh Yigit
@ 2019-07-25  7:40     ` David Marchand
  2019-07-25 11:01       ` Ferruh Yigit
  0 siblings, 1 reply; 23+ messages in thread
From: David Marchand @ 2019-07-25  7:40 UTC (permalink / raw)
  To: Ferruh Yigit; +Cc: dev, dpdk stable, A.McLoughlin

On Wed, Jul 24, 2019 at 8:36 PM Ferruh Yigit <ferruh.yigit@intel.com> wrote:
>
> On 7/24/2019 12:54 PM, David Marchand wrote:
> > When a packet cannot be transmitted, the driver is supposed to free this
> > packet and report it as handled.
> > This is to prevent the application from retrying to send the same packet
> > and ending up in a liveloop since the driver will never manage to send
> > it.
> >
> > Fixes: 49a0a2ffd5db ("net/pcap: fix possible mbuf double freeing")
> > Fixes: 6db141c91e1f ("pcap: support jumbo frames")
> > CC: stable@dpdk.org
> >
> > Signed-off-by: David Marchand <david.marchand@redhat.com>
> > ---
> >  drivers/net/pcap/rte_eth_pcap.c | 18 ++++++++++--------
> >  1 file changed, 10 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/net/pcap/rte_eth_pcap.c b/drivers/net/pcap/rte_eth_pcap.c
> > index 470867d..5e5aab7 100644
> > --- a/drivers/net/pcap/rte_eth_pcap.c
> > +++ b/drivers/net/pcap/rte_eth_pcap.c
> > @@ -354,7 +354,8 @@ struct pmd_devargs_all {
> >                                       mbuf->pkt_len,
> >                                       RTE_ETHER_MAX_JUMBO_FRAME_LEN);
> >
> > -                             break;
> > +                             rte_pktmbuf_free(mbuf);
> > +                             continue;
>
> +1
> Very recently 'rte_pktmbuf_free()' was moved because it wasn't compatible with
> return value, but this looks better, to free the mbuf and record it as error.
>
> >                       }
> >               }
> >
> > @@ -373,7 +374,7 @@ struct pmd_devargs_all {
> >       dumper_q->tx_stat.bytes += tx_bytes;
> >       dumper_q->tx_stat.err_pkts += nb_pkts - num_tx;
> >
> > -     return num_tx;
> > +     return nb_pkts;
> >  }
> >
> >  /*
> > @@ -439,14 +440,15 @@ struct pmd_devargs_all {
> >                                       mbuf->pkt_len,
> >                                       RTE_ETHER_MAX_JUMBO_FRAME_LEN);
> >
> > -                             break;
> > +                             rte_pktmbuf_free(mbuf);
> > +                             continue;
> >                       }
> >               }
> >
> > -             if (unlikely(ret != 0))
> > -                     break;
> > -             num_tx++;
> > -             tx_bytes += mbuf->pkt_len;
> > +             if (ret == 0) {
> > +                     num_tx++;
> > +                     tx_bytes += mbuf->pkt_len;
> > +             }
>
> I don't know this part, this is in 'eth_pcap_tx()' which writes packets to the
> interfaces.
>
> if 'pcap_sendpacket()' fails this doesn't mean packet can't be sent and may
> cause a liveloop. Why not keep the existing behavior and let application to decide?

The manual is not clear to me.
Do we really have temporary situations where retries are fine? and if
so, can we differentiate them from things like incorrect permissions
etc...

If we can't, dropping is safer.

-- 
David Marchand

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [dpdk-dev] [dpdk-stable] [PATCH 3/3] net/pcap: fix concurrent multiseg packet transmits
  2019-07-24 11:54 ` [dpdk-dev] [PATCH 3/3] net/pcap: fix concurrent multiseg packet transmits David Marchand
  2019-07-24 18:43   ` Ferruh Yigit
@ 2019-07-25  8:18   ` David Marchand
  2019-07-25 11:07     ` Ferruh Yigit
  1 sibling, 1 reply; 23+ messages in thread
From: David Marchand @ 2019-07-25  8:18 UTC (permalink / raw)
  To: Yigit, Ferruh; +Cc: dpdk stable, dev

On Wed, Jul 24, 2019 at 1:55 PM David Marchand
<david.marchand@redhat.com> wrote:
>
> Two cores can send multi segment packets on two different pcap ports.
> Because of this, we can't have one single buffer to linearize packets.
>
> Use rte_pktmbuf_read() to copy the packet into a buffer on the stack
> and remove eth_pcap_gather_data().
>
> Fixes: 6db141c91e1f ("pcap: support jumbo frames")
> Cc: stable@dpdk.org
>
> Signed-off-by: David Marchand <david.marchand@redhat.com>
> ---
>  drivers/net/pcap/rte_eth_pcap.c | 90 +++++++++++++++--------------------------
>  1 file changed, 32 insertions(+), 58 deletions(-)
>
> diff --git a/drivers/net/pcap/rte_eth_pcap.c b/drivers/net/pcap/rte_eth_pcap.c
> index 5e5aab7..7398b1b 100644
> --- a/drivers/net/pcap/rte_eth_pcap.c
> +++ b/drivers/net/pcap/rte_eth_pcap.c

[snip]

> @@ -336,31 +323,25 @@ struct pmd_devargs_all {
>          * dumper */
>         for (i = 0; i < nb_pkts; i++) {
>                 mbuf = bufs[i];
> -               calculate_timestamp(&header.ts);
> -               header.len = mbuf->pkt_len;
> -               header.caplen = header.len;
> -
> -               if (likely(mbuf->nb_segs == 1)) {
> -                       pcap_dump((u_char *)dumper, &header,
> -                                 rte_pktmbuf_mtod(mbuf, void*));
> +               len = rte_pktmbuf_pkt_len(mbuf);
> +               if (likely(rte_pktmbuf_is_contiguous(mbuf))) {
> +                       data = rte_pktmbuf_mtod(mbuf, unsigned char *);
> +               } else if (len <= sizeof(_data)) {
> +                       data = rte_pktmbuf_read(mbuf, 0, len, _data);

We can actually skip the check on contiguous data, since
rte_pktmbuf_read returns a pointer to the mbuf data without copying.
WDYT ?


--
David Marchand

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [dpdk-dev] [PATCH 2/3] net/pcap: fix transmit return count in error conditions
  2019-07-25  7:40     ` David Marchand
@ 2019-07-25 11:01       ` Ferruh Yigit
  0 siblings, 0 replies; 23+ messages in thread
From: Ferruh Yigit @ 2019-07-25 11:01 UTC (permalink / raw)
  To: David Marchand; +Cc: dev, dpdk stable, A.McLoughlin

On 7/25/2019 8:40 AM, David Marchand wrote:
> On Wed, Jul 24, 2019 at 8:36 PM Ferruh Yigit <ferruh.yigit@intel.com> wrote:
>>
>> On 7/24/2019 12:54 PM, David Marchand wrote:
>>> When a packet cannot be transmitted, the driver is supposed to free this
>>> packet and report it as handled.
>>> This is to prevent the application from retrying to send the same packet
>>> and ending up in a liveloop since the driver will never manage to send
>>> it.
>>>
>>> Fixes: 49a0a2ffd5db ("net/pcap: fix possible mbuf double freeing")
>>> Fixes: 6db141c91e1f ("pcap: support jumbo frames")
>>> CC: stable@dpdk.org
>>>
>>> Signed-off-by: David Marchand <david.marchand@redhat.com>
>>> ---
>>>  drivers/net/pcap/rte_eth_pcap.c | 18 ++++++++++--------
>>>  1 file changed, 10 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/drivers/net/pcap/rte_eth_pcap.c b/drivers/net/pcap/rte_eth_pcap.c
>>> index 470867d..5e5aab7 100644
>>> --- a/drivers/net/pcap/rte_eth_pcap.c
>>> +++ b/drivers/net/pcap/rte_eth_pcap.c
>>> @@ -354,7 +354,8 @@ struct pmd_devargs_all {
>>>                                       mbuf->pkt_len,
>>>                                       RTE_ETHER_MAX_JUMBO_FRAME_LEN);
>>>
>>> -                             break;
>>> +                             rte_pktmbuf_free(mbuf);
>>> +                             continue;
>>
>> +1
>> Very recently 'rte_pktmbuf_free()' was moved because it wasn't compatible with
>> return value, but this looks better, to free the mbuf and record it as error.
>>
>>>                       }
>>>               }
>>>
>>> @@ -373,7 +374,7 @@ struct pmd_devargs_all {
>>>       dumper_q->tx_stat.bytes += tx_bytes;
>>>       dumper_q->tx_stat.err_pkts += nb_pkts - num_tx;
>>>
>>> -     return num_tx;
>>> +     return nb_pkts;
>>>  }
>>>
>>>  /*
>>> @@ -439,14 +440,15 @@ struct pmd_devargs_all {
>>>                                       mbuf->pkt_len,
>>>                                       RTE_ETHER_MAX_JUMBO_FRAME_LEN);
>>>
>>> -                             break;
>>> +                             rte_pktmbuf_free(mbuf);
>>> +                             continue;
>>>                       }
>>>               }
>>>
>>> -             if (unlikely(ret != 0))
>>> -                     break;
>>> -             num_tx++;
>>> -             tx_bytes += mbuf->pkt_len;
>>> +             if (ret == 0) {
>>> +                     num_tx++;
>>> +                     tx_bytes += mbuf->pkt_len;
>>> +             }
>>
>> I don't know this part, this is in 'eth_pcap_tx()' which writes packets to the
>> interfaces.
>>
>> if 'pcap_sendpacket()' fails this doesn't mean packet can't be sent and may
>> cause a liveloop. Why not keep the existing behavior and let application to decide?
> 
> The manual is not clear to me.
> Do we really have temporary situations where retries are fine?

Same here, I am not that clear, I assume these situations exists, specially
taking into account nobody complaining about existing implementation.

> and if
> so, can we differentiate them from things like incorrect permissions
> etc...
> 
> If we can't, dropping is safer.
> 

May not differentiate them, because all we are getting is the fail from API
without reason.

But overall instead of driving freeing a packet, it feels better to leave this
decision to application unless driver needs to free it.

Other changes in the patchset is fixing defects in driver, only this part
changes behavior without a clear defect, I am leaning to keep old behavior.

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [dpdk-dev] [dpdk-stable] [PATCH 3/3] net/pcap: fix concurrent multiseg packet transmits
  2019-07-25  8:18   ` [dpdk-dev] [dpdk-stable] " David Marchand
@ 2019-07-25 11:07     ` Ferruh Yigit
  0 siblings, 0 replies; 23+ messages in thread
From: Ferruh Yigit @ 2019-07-25 11:07 UTC (permalink / raw)
  To: David Marchand; +Cc: dpdk stable, dev

On 7/25/2019 9:18 AM, David Marchand wrote:
> On Wed, Jul 24, 2019 at 1:55 PM David Marchand
> <david.marchand@redhat.com> wrote:
>>
>> Two cores can send multi segment packets on two different pcap ports.
>> Because of this, we can't have one single buffer to linearize packets.
>>
>> Use rte_pktmbuf_read() to copy the packet into a buffer on the stack
>> and remove eth_pcap_gather_data().
>>
>> Fixes: 6db141c91e1f ("pcap: support jumbo frames")
>> Cc: stable@dpdk.org
>>
>> Signed-off-by: David Marchand <david.marchand@redhat.com>
>> ---
>>  drivers/net/pcap/rte_eth_pcap.c | 90 +++++++++++++++--------------------------
>>  1 file changed, 32 insertions(+), 58 deletions(-)
>>
>> diff --git a/drivers/net/pcap/rte_eth_pcap.c b/drivers/net/pcap/rte_eth_pcap.c
>> index 5e5aab7..7398b1b 100644
>> --- a/drivers/net/pcap/rte_eth_pcap.c
>> +++ b/drivers/net/pcap/rte_eth_pcap.c
> 
> [snip]
> 
>> @@ -336,31 +323,25 @@ struct pmd_devargs_all {
>>          * dumper */
>>         for (i = 0; i < nb_pkts; i++) {
>>                 mbuf = bufs[i];
>> -               calculate_timestamp(&header.ts);
>> -               header.len = mbuf->pkt_len;
>> -               header.caplen = header.len;
>> -
>> -               if (likely(mbuf->nb_segs == 1)) {
>> -                       pcap_dump((u_char *)dumper, &header,
>> -                                 rte_pktmbuf_mtod(mbuf, void*));
>> +               len = rte_pktmbuf_pkt_len(mbuf);
>> +               if (likely(rte_pktmbuf_is_contiguous(mbuf))) {
>> +                       data = rte_pktmbuf_mtod(mbuf, unsigned char *);
>> +               } else if (len <= sizeof(_data)) {
>> +                       data = rte_pktmbuf_read(mbuf, 0, len, _data);
> 
> We can actually skip the check on contiguous data, since
> rte_pktmbuf_read returns a pointer to the mbuf data without copying.
> WDYT ?

Right, +1 to skip 'rte_pktmbuf_is_contiguous()' only it can be good to comment
'rte_pktmbuf_read()' usage to say it covers multi-segment too.

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [dpdk-dev] [PATCH v2 0/3] Multiseg fixes for pcap pmd
  2019-07-24 11:54 [dpdk-dev] [PATCH 0/3] Multiseg fixes for pcap pmd David Marchand
                   ` (2 preceding siblings ...)
  2019-07-24 11:54 ` [dpdk-dev] [PATCH 3/3] net/pcap: fix concurrent multiseg packet transmits David Marchand
@ 2019-07-25 12:04 ` David Marchand
  2019-07-25 12:04   ` [dpdk-dev] [PATCH v2 1/3] net/pcap: fix Rx with small buffers David Marchand
                     ` (2 more replies)
  2019-07-25 19:24 ` [dpdk-dev] [PATCH v3 0/3] Multiseg fixes for pcap pmd David Marchand
  4 siblings, 3 replies; 23+ messages in thread
From: David Marchand @ 2019-07-25 12:04 UTC (permalink / raw)
  To: dev; +Cc: ferruh.yigit

Here are some fixes caught while looking at oerrors statistics for this
driver.
The second patch can be seen as a revert or a followup of [1].

1: https://git.dpdk.org/dpdk/commit/?id=49a0a2ffd5db

Changelog since v1:
- comments from Ferruh on patch2,
- little simplification in patch3,

-- 
David Marchand

David Marchand (3):
  net/pcap: fix Rx with small buffers
  net/pcap: fix transmit return count in error conditions
  net/pcap: fix concurrent multiseg packet transmits

 drivers/net/pcap/rte_eth_pcap.c | 104 ++++++++++++++++------------------------
 1 file changed, 40 insertions(+), 64 deletions(-)

-- 
1.8.3.1


^ permalink raw reply	[flat|nested] 23+ messages in thread

* [dpdk-dev] [PATCH v2 1/3] net/pcap: fix Rx with small buffers
  2019-07-25 12:04 ` [dpdk-dev] [PATCH v2 0/3] Multiseg fixes for pcap pmd David Marchand
@ 2019-07-25 12:04   ` David Marchand
  2019-07-25 12:04   ` [dpdk-dev] [PATCH v2 2/3] net/pcap: fix transmit return count in error conditions David Marchand
  2019-07-25 12:04   ` [dpdk-dev] [PATCH v2 3/3] net/pcap: fix concurrent multiseg packet transmits David Marchand
  2 siblings, 0 replies; 23+ messages in thread
From: David Marchand @ 2019-07-25 12:04 UTC (permalink / raw)
  To: dev; +Cc: ferruh.yigit, stable

If the pkt pool contains only buffers smaller than the default headroom,
then the driver will compute an invalid buffer size (negative value cast
to an uint16_t).
Rely on the mbuf api to check how much space is available in the mbuf.

Fixes: 6eb0ae218a98 ("pcap: fix mbuf allocation")
Cc: stable@dpdk.org

Signed-off-by: David Marchand <david.marchand@redhat.com>
Acked-by: Ferruh Yigit <ferruh.yigit@intel.com>
---
 drivers/net/pcap/rte_eth_pcap.c | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/drivers/net/pcap/rte_eth_pcap.c b/drivers/net/pcap/rte_eth_pcap.c
index 322c18f..470867d 100644
--- a/drivers/net/pcap/rte_eth_pcap.c
+++ b/drivers/net/pcap/rte_eth_pcap.c
@@ -242,7 +242,6 @@ eth_pcap_rx(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts)
 	struct rte_mbuf *mbuf;
 	struct pcap_rx_queue *pcap_q = queue;
 	uint16_t num_rx = 0;
-	uint16_t buf_size;
 	uint32_t rx_bytes = 0;
 	pcap_t *pcap;
 
@@ -265,11 +264,7 @@ eth_pcap_rx(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts)
 		if (unlikely(mbuf == NULL))
 			break;
 
-		/* Now get the space available for data in the mbuf */
-		buf_size = rte_pktmbuf_data_room_size(pcap_q->mb_pool) -
-				RTE_PKTMBUF_HEADROOM;
-
-		if (header.caplen <= buf_size) {
+		if (header.caplen <= rte_pktmbuf_tailroom(mbuf)) {
 			/* pcap packet will fit in the mbuf, can copy it */
 			rte_memcpy(rte_pktmbuf_mtod(mbuf, void *), packet,
 					header.caplen);
-- 
1.8.3.1


^ permalink raw reply	[flat|nested] 23+ messages in thread

* [dpdk-dev] [PATCH v2 2/3] net/pcap: fix transmit return count in error conditions
  2019-07-25 12:04 ` [dpdk-dev] [PATCH v2 0/3] Multiseg fixes for pcap pmd David Marchand
  2019-07-25 12:04   ` [dpdk-dev] [PATCH v2 1/3] net/pcap: fix Rx with small buffers David Marchand
@ 2019-07-25 12:04   ` David Marchand
  2019-07-25 14:43     ` Ferruh Yigit
  2019-07-25 12:04   ` [dpdk-dev] [PATCH v2 3/3] net/pcap: fix concurrent multiseg packet transmits David Marchand
  2 siblings, 1 reply; 23+ messages in thread
From: David Marchand @ 2019-07-25 12:04 UTC (permalink / raw)
  To: dev; +Cc: ferruh.yigit, stable

When a packet cannot be transmitted, the driver is supposed to free this
packet and report it as handled.
This is to prevent the application from retrying to send the same packet
and ending up in a liveloop since the driver will never manage to send
it.

Fixes: 49a0a2ffd5db ("net/pcap: fix possible mbuf double freeing")
Fixes: 6db141c91e1f ("pcap: support jumbo frames")
CC: stable@dpdk.org

Signed-off-by: David Marchand <david.marchand@redhat.com>
---
Changelog since v1:
- restored the original behavior if pcap_sendpacket returns an error

---
 drivers/net/pcap/rte_eth_pcap.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/drivers/net/pcap/rte_eth_pcap.c b/drivers/net/pcap/rte_eth_pcap.c
index 470867d..bfc0756 100644
--- a/drivers/net/pcap/rte_eth_pcap.c
+++ b/drivers/net/pcap/rte_eth_pcap.c
@@ -354,7 +354,8 @@ eth_pcap_tx_dumper(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts)
 					mbuf->pkt_len,
 					RTE_ETHER_MAX_JUMBO_FRAME_LEN);
 
-				break;
+				rte_pktmbuf_free(mbuf);
+				continue;
 			}
 		}
 
@@ -373,7 +374,7 @@ eth_pcap_tx_dumper(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts)
 	dumper_q->tx_stat.bytes += tx_bytes;
 	dumper_q->tx_stat.err_pkts += nb_pkts - num_tx;
 
-	return num_tx;
+	return nb_pkts;
 }
 
 /*
@@ -439,7 +440,8 @@ eth_pcap_tx(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts)
 					mbuf->pkt_len,
 					RTE_ETHER_MAX_JUMBO_FRAME_LEN);
 
-				break;
+				rte_pktmbuf_free(mbuf);
+				continue;
 			}
 		}
 
@@ -452,9 +454,9 @@ eth_pcap_tx(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts)
 
 	tx_queue->tx_stat.pkts += num_tx;
 	tx_queue->tx_stat.bytes += tx_bytes;
-	tx_queue->tx_stat.err_pkts += nb_pkts - num_tx;
+	tx_queue->tx_stat.err_pkts += i - num_tx;
 
-	return num_tx;
+	return i;
 }
 
 /*
-- 
1.8.3.1


^ permalink raw reply	[flat|nested] 23+ messages in thread

* [dpdk-dev] [PATCH v2 3/3] net/pcap: fix concurrent multiseg packet transmits
  2019-07-25 12:04 ` [dpdk-dev] [PATCH v2 0/3] Multiseg fixes for pcap pmd David Marchand
  2019-07-25 12:04   ` [dpdk-dev] [PATCH v2 1/3] net/pcap: fix Rx with small buffers David Marchand
  2019-07-25 12:04   ` [dpdk-dev] [PATCH v2 2/3] net/pcap: fix transmit return count in error conditions David Marchand
@ 2019-07-25 12:04   ` David Marchand
  2019-07-25 12:05     ` [dpdk-dev] [dpdk-stable] " David Marchand
  2019-07-25 14:47     ` [dpdk-dev] " Ferruh Yigit
  2 siblings, 2 replies; 23+ messages in thread
From: David Marchand @ 2019-07-25 12:04 UTC (permalink / raw)
  To: dev; +Cc: ferruh.yigit, stable

Two cores can send multi segment packets on two different pcap ports.
Because of this, we can't have one single buffer to linearize packets.

Use rte_pktmbuf_read() to copy the packet into a buffer on the stack
and remove eth_pcap_gather_data() when necessary (if the mbuf is
contiguous, rte_pktmbuf_read() just points at the buffer address).

With this change, we won't support mono segment mbuf larger than 16k.

Fixes: 6db141c91e1f ("pcap: support jumbo frames")
Cc: stable@dpdk.org

Signed-off-by: David Marchand <david.marchand@redhat.com>
---
Changelog since v1:
- rely on rte_pktmbuf_read to handle both mono and multi segments cases

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

diff --git a/drivers/net/pcap/rte_eth_pcap.c b/drivers/net/pcap/rte_eth_pcap.c
index bfc0756..7a5fb6a 100644
--- a/drivers/net/pcap/rte_eth_pcap.c
+++ b/drivers/net/pcap/rte_eth_pcap.c
@@ -46,7 +46,6 @@
 #define RTE_PMD_PCAP_MAX_QUEUES 16
 
 static char errbuf[PCAP_ERRBUF_SIZE];
-static unsigned char tx_pcap_data[RTE_ETH_PCAP_SNAPLEN];
 static struct timeval start_time;
 static uint64_t start_cycles;
 static uint64_t hz;
@@ -180,21 +179,6 @@ eth_pcap_rx_jumbo(struct rte_mempool *mb_pool, struct rte_mbuf *mbuf,
 	return mbuf->nb_segs;
 }
 
-/* Copy data from mbuf chain to a buffer suitable for writing to a PCAP file. */
-static void
-eth_pcap_gather_data(unsigned char *data, struct rte_mbuf *mbuf)
-{
-	uint16_t data_len = 0;
-
-	while (mbuf) {
-		rte_memcpy(data + data_len, rte_pktmbuf_mtod(mbuf, void *),
-			mbuf->data_len);
-
-		data_len += mbuf->data_len;
-		mbuf = mbuf->next;
-	}
-}
-
 static uint16_t
 eth_pcap_rx_infinite(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts)
 {
@@ -325,6 +309,8 @@ eth_pcap_tx_dumper(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts)
 	uint32_t tx_bytes = 0;
 	struct pcap_pkthdr header;
 	pcap_dumper_t *dumper;
+	unsigned char temp_data[RTE_ETH_PCAP_SNAPLEN];
+	size_t len;
 
 	pp = rte_eth_devices[dumper_q->port_id].process_private;
 	dumper = pp->tx_dumper[dumper_q->queue_id];
@@ -336,31 +322,27 @@ eth_pcap_tx_dumper(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts)
 	 * dumper */
 	for (i = 0; i < nb_pkts; i++) {
 		mbuf = bufs[i];
+		len = rte_pktmbuf_pkt_len(mbuf);
+		if (unlikely(!rte_pktmbuf_is_contiguous(mbuf) &&
+				len > sizeof(temp_data))) {
+			PMD_LOG(ERR, "Dropping multi segment PCAP packet. Size (%zd) > max size (%zd).",
+				len, sizeof(temp_data));
+			rte_pktmbuf_free(mbuf);
+			continue;
+		}
+
 		calculate_timestamp(&header.ts);
-		header.len = mbuf->pkt_len;
+		header.len = len;
 		header.caplen = header.len;
-
-		if (likely(mbuf->nb_segs == 1)) {
-			pcap_dump((u_char *)dumper, &header,
-				  rte_pktmbuf_mtod(mbuf, void*));
-		} else {
-			if (mbuf->pkt_len <= RTE_ETHER_MAX_JUMBO_FRAME_LEN) {
-				eth_pcap_gather_data(tx_pcap_data, mbuf);
-				pcap_dump((u_char *)dumper, &header,
-					  tx_pcap_data);
-			} else {
-				PMD_LOG(ERR,
-					"Dropping PCAP packet. Size (%d) > max jumbo size (%d).",
-					mbuf->pkt_len,
-					RTE_ETHER_MAX_JUMBO_FRAME_LEN);
-
-				rte_pktmbuf_free(mbuf);
-				continue;
-			}
-		}
+		/* 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.
+		 */
+		pcap_dump((u_char *)dumper, &header,
+			  rte_pktmbuf_read(mbuf, 0, len, temp_data));
 
 		num_tx++;
-		tx_bytes += mbuf->pkt_len;
+		tx_bytes += len;
 		rte_pktmbuf_free(mbuf);
 	}
 
@@ -415,6 +397,8 @@ eth_pcap_tx(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts)
 	uint16_t num_tx = 0;
 	uint32_t tx_bytes = 0;
 	pcap_t *pcap;
+	unsigned char temp_data[RTE_ETH_PCAP_SNAPLEN];
+	size_t len;
 
 	pp = rte_eth_devices[tx_queue->port_id].process_private;
 	pcap = pp->tx_pcap[tx_queue->queue_id];
@@ -424,31 +408,26 @@ eth_pcap_tx(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts)
 
 	for (i = 0; i < nb_pkts; i++) {
 		mbuf = bufs[i];
-
-		if (likely(mbuf->nb_segs == 1)) {
-			ret = pcap_sendpacket(pcap,
-					rte_pktmbuf_mtod(mbuf, u_char *),
-					mbuf->pkt_len);
-		} else {
-			if (mbuf->pkt_len <= RTE_ETHER_MAX_JUMBO_FRAME_LEN) {
-				eth_pcap_gather_data(tx_pcap_data, mbuf);
-				ret = pcap_sendpacket(pcap,
-						tx_pcap_data, mbuf->pkt_len);
-			} else {
-				PMD_LOG(ERR,
-					"Dropping PCAP packet. Size (%d) > max jumbo size (%d).",
-					mbuf->pkt_len,
-					RTE_ETHER_MAX_JUMBO_FRAME_LEN);
-
-				rte_pktmbuf_free(mbuf);
-				continue;
-			}
+		len = rte_pktmbuf_pkt_len(mbuf);
+		if (unlikely(!rte_pktmbuf_is_contiguous(mbuf) &&
+				len > sizeof(temp_data))) {
+			PMD_LOG(ERR, "Dropping multi segment PCAP packet. Size (%zd) > max size (%zd).",
+				len, sizeof(temp_data));
+			rte_pktmbuf_free(mbuf);
+			continue;
 		}
 
+		/* 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.
+		 */
+		ret = pcap_sendpacket(pcap,
+				      rte_pktmbuf_read(mbuf, 0, len, temp_data),
+				      len);
 		if (unlikely(ret != 0))
 			break;
 		num_tx++;
-		tx_bytes += mbuf->pkt_len;
+		tx_bytes += len;
 		rte_pktmbuf_free(mbuf);
 	}
 
-- 
1.8.3.1


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [dpdk-dev] [dpdk-stable] [PATCH v2 3/3] net/pcap: fix concurrent multiseg packet transmits
  2019-07-25 12:04   ` [dpdk-dev] [PATCH v2 3/3] net/pcap: fix concurrent multiseg packet transmits David Marchand
@ 2019-07-25 12:05     ` David Marchand
  2019-07-25 14:47     ` [dpdk-dev] " Ferruh Yigit
  1 sibling, 0 replies; 23+ messages in thread
From: David Marchand @ 2019-07-25 12:05 UTC (permalink / raw)
  To: dev; +Cc: Yigit, Ferruh, dpdk stable

On Thu, Jul 25, 2019 at 2:05 PM David Marchand
<david.marchand@redhat.com> wrote:
>
> Two cores can send multi segment packets on two different pcap ports.
> Because of this, we can't have one single buffer to linearize packets.
>
> Use rte_pktmbuf_read() to copy the packet into a buffer on the stack
> and remove eth_pcap_gather_data() when necessary (if the mbuf is
> contiguous, rte_pktmbuf_read() just points at the buffer address).
>
> With this change, we won't support mono segment mbuf larger than 16k.

Argh, should have removed this wrong comment...


-- 
David Marchand

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [dpdk-dev] [PATCH v2 2/3] net/pcap: fix transmit return count in error conditions
  2019-07-25 12:04   ` [dpdk-dev] [PATCH v2 2/3] net/pcap: fix transmit return count in error conditions David Marchand
@ 2019-07-25 14:43     ` Ferruh Yigit
  0 siblings, 0 replies; 23+ messages in thread
From: Ferruh Yigit @ 2019-07-25 14:43 UTC (permalink / raw)
  To: David Marchand, dev; +Cc: stable

On 7/25/2019 1:04 PM, David Marchand wrote:
> When a packet cannot be transmitted, the driver is supposed to free this
> packet and report it as handled.
> This is to prevent the application from retrying to send the same packet
> and ending up in a liveloop since the driver will never manage to send
> it.
> 
> Fixes: 49a0a2ffd5db ("net/pcap: fix possible mbuf double freeing")
> Fixes: 6db141c91e1f ("pcap: support jumbo frames")
> CC: stable@dpdk.org
> 
> Signed-off-by: David Marchand <david.marchand@redhat.com>

Acked-by: Ferruh Yigit <ferruh.yigit@intel.com>

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [dpdk-dev] [PATCH v2 3/3] net/pcap: fix concurrent multiseg packet transmits
  2019-07-25 12:04   ` [dpdk-dev] [PATCH v2 3/3] net/pcap: fix concurrent multiseg packet transmits David Marchand
  2019-07-25 12:05     ` [dpdk-dev] [dpdk-stable] " David Marchand
@ 2019-07-25 14:47     ` Ferruh Yigit
  1 sibling, 0 replies; 23+ messages in thread
From: Ferruh Yigit @ 2019-07-25 14:47 UTC (permalink / raw)
  To: David Marchand, dev; +Cc: stable

On 7/25/2019 1:04 PM, David Marchand wrote:
> Two cores can send multi segment packets on two different pcap ports.
> Because of this, we can't have one single buffer to linearize packets.
> 
> Use rte_pktmbuf_read() to copy the packet into a buffer on the stack
> and remove eth_pcap_gather_data() when necessary (if the mbuf is
> contiguous, rte_pktmbuf_read() just points at the buffer address).
> 
> With this change, we won't support mono segment mbuf larger than 16k.
> 
> Fixes: 6db141c91e1f ("pcap: support jumbo frames")
> Cc: stable@dpdk.org
> 
> Signed-off-by: David Marchand <david.marchand@redhat.com>

Thanks David, lgtm, only a few minor syntax issues, please check below.

<...>

> @@ -336,31 +322,27 @@ eth_pcap_tx_dumper(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts)
>  	 * dumper */
>  	for (i = 0; i < nb_pkts; i++) {
>  		mbuf = bufs[i];
> +		len = rte_pktmbuf_pkt_len(mbuf);
> +		if (unlikely(!rte_pktmbuf_is_contiguous(mbuf) &&
> +				len > sizeof(temp_data))) {
> +			PMD_LOG(ERR, "Dropping multi segment PCAP packet. Size (%zd) > max size (%zd).",

Can we break the line to reduce the line length:
PMD_LOG(ERR,
	"Dropping multi segment PCAP packet. Size (%zd) > max size (%zd).",

> +				len, sizeof(temp_data));
> +			rte_pktmbuf_free(mbuf);
> +			continue;
> +		}
> +
>  		calculate_timestamp(&header.ts);
> -		header.len = mbuf->pkt_len;
> +		header.len = len;
>  		header.caplen = header.len;
> -
> -		if (likely(mbuf->nb_segs == 1)) {
> -			pcap_dump((u_char *)dumper, &header,
> -				  rte_pktmbuf_mtod(mbuf, void*));

DPDK coding convention requires a tab, instead of aligning to the parenthesis.

<...>

> +		/* 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.
> +		 */
> +		pcap_dump((u_char *)dumper, &header,
> +			  rte_pktmbuf_read(mbuf, 0, len, temp_data));

Same here, not need to align to the parenthesis.

<...>

> +		len = rte_pktmbuf_pkt_len(mbuf);
> +		if (unlikely(!rte_pktmbuf_is_contiguous(mbuf) &&
> +				len > sizeof(temp_data))) {
> +			PMD_LOG(ERR, "Dropping multi segment PCAP packet. Size (%zd) > max size (%zd).",
> +				len, sizeof(temp_data));

ditto

> +			rte_pktmbuf_free(mbuf);
> +			continue;
>  		}
>  
> +		/* 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.
> +		 */
> +		ret = pcap_sendpacket(pcap,
> +				      rte_pktmbuf_read(mbuf, 0, len, temp_data),
> +				      len);

ditto


^ permalink raw reply	[flat|nested] 23+ messages in thread

* [dpdk-dev] [PATCH v3 0/3] Multiseg fixes for pcap pmd
  2019-07-24 11:54 [dpdk-dev] [PATCH 0/3] Multiseg fixes for pcap pmd David Marchand
                   ` (3 preceding siblings ...)
  2019-07-25 12:04 ` [dpdk-dev] [PATCH v2 0/3] Multiseg fixes for pcap pmd David Marchand
@ 2019-07-25 19:24 ` David Marchand
  2019-07-25 19:24   ` [dpdk-dev] [PATCH v3 1/3] net/pcap: fix Rx with small buffers David Marchand
                     ` (3 more replies)
  4 siblings, 4 replies; 23+ messages in thread
From: David Marchand @ 2019-07-25 19:24 UTC (permalink / raw)
  To: dev; +Cc: ferruh.yigit

Here are some fixes caught while looking at oerrors statistics for this
driver.
The second patch can be seen as a revert or a followup of [1].

1: https://git.dpdk.org/dpdk/commit/?id=49a0a2ffd5db

Changelog since v2:
- coding style fixes in patch3,
- added acks,

Changelog since v1:
- comments from Ferruh on patch2,
- little simplification in patch3,

-- 
David Marchand

David Marchand (3):
  net/pcap: fix Rx with small buffers
  net/pcap: fix transmit return count in error conditions
  net/pcap: fix concurrent multiseg packet transmits

 drivers/net/pcap/rte_eth_pcap.c | 106 ++++++++++++++++------------------------
 1 file changed, 42 insertions(+), 64 deletions(-)

-- 
1.8.3.1


^ permalink raw reply	[flat|nested] 23+ messages in thread

* [dpdk-dev] [PATCH v3 1/3] net/pcap: fix Rx with small buffers
  2019-07-25 19:24 ` [dpdk-dev] [PATCH v3 0/3] Multiseg fixes for pcap pmd David Marchand
@ 2019-07-25 19:24   ` David Marchand
  2019-07-25 19:24   ` [dpdk-dev] [PATCH v3 2/3] net/pcap: fix transmit return count in error conditions David Marchand
                     ` (2 subsequent siblings)
  3 siblings, 0 replies; 23+ messages in thread
From: David Marchand @ 2019-07-25 19:24 UTC (permalink / raw)
  To: dev; +Cc: ferruh.yigit, stable

If the pkt pool contains only buffers smaller than the default headroom,
then the driver will compute an invalid buffer size (negative value cast
to an uint16_t).
Rely on the mbuf api to check how much space is available in the mbuf.

Fixes: 6eb0ae218a98 ("pcap: fix mbuf allocation")
Cc: stable@dpdk.org

Signed-off-by: David Marchand <david.marchand@redhat.com>
Acked-by: Ferruh Yigit <ferruh.yigit@intel.com>
---
 drivers/net/pcap/rte_eth_pcap.c | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/drivers/net/pcap/rte_eth_pcap.c b/drivers/net/pcap/rte_eth_pcap.c
index 322c18f..470867d 100644
--- a/drivers/net/pcap/rte_eth_pcap.c
+++ b/drivers/net/pcap/rte_eth_pcap.c
@@ -242,7 +242,6 @@ eth_pcap_rx(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts)
 	struct rte_mbuf *mbuf;
 	struct pcap_rx_queue *pcap_q = queue;
 	uint16_t num_rx = 0;
-	uint16_t buf_size;
 	uint32_t rx_bytes = 0;
 	pcap_t *pcap;
 
@@ -265,11 +264,7 @@ eth_pcap_rx(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts)
 		if (unlikely(mbuf == NULL))
 			break;
 
-		/* Now get the space available for data in the mbuf */
-		buf_size = rte_pktmbuf_data_room_size(pcap_q->mb_pool) -
-				RTE_PKTMBUF_HEADROOM;
-
-		if (header.caplen <= buf_size) {
+		if (header.caplen <= rte_pktmbuf_tailroom(mbuf)) {
 			/* pcap packet will fit in the mbuf, can copy it */
 			rte_memcpy(rte_pktmbuf_mtod(mbuf, void *), packet,
 					header.caplen);
-- 
1.8.3.1


^ permalink raw reply	[flat|nested] 23+ messages in thread

* [dpdk-dev] [PATCH v3 2/3] net/pcap: fix transmit return count in error conditions
  2019-07-25 19:24 ` [dpdk-dev] [PATCH v3 0/3] Multiseg fixes for pcap pmd David Marchand
  2019-07-25 19:24   ` [dpdk-dev] [PATCH v3 1/3] net/pcap: fix Rx with small buffers David Marchand
@ 2019-07-25 19:24   ` David Marchand
  2019-07-25 19:24   ` [dpdk-dev] [PATCH v3 3/3] net/pcap: fix concurrent multiseg packet transmits David Marchand
  2019-07-25 22:36   ` [dpdk-dev] [PATCH v3 0/3] Multiseg fixes for pcap pmd Ferruh Yigit
  3 siblings, 0 replies; 23+ messages in thread
From: David Marchand @ 2019-07-25 19:24 UTC (permalink / raw)
  To: dev; +Cc: ferruh.yigit, stable

When a packet cannot be transmitted, the driver is supposed to free this
packet and report it as handled.
This is to prevent the application from retrying to send the same packet
and ending up in a liveloop since the driver will never manage to send
it.

Fixes: 49a0a2ffd5db ("net/pcap: fix possible mbuf double freeing")
Fixes: 6db141c91e1f ("pcap: support jumbo frames")
CC: stable@dpdk.org

Signed-off-by: David Marchand <david.marchand@redhat.com>
Acked-by: Ferruh Yigit <ferruh.yigit@intel.com>
---
Changelog since v1:
- restored the original behavior if pcap_sendpacket returns an error

---
 drivers/net/pcap/rte_eth_pcap.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/drivers/net/pcap/rte_eth_pcap.c b/drivers/net/pcap/rte_eth_pcap.c
index 470867d..bfc0756 100644
--- a/drivers/net/pcap/rte_eth_pcap.c
+++ b/drivers/net/pcap/rte_eth_pcap.c
@@ -354,7 +354,8 @@ eth_pcap_tx_dumper(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts)
 					mbuf->pkt_len,
 					RTE_ETHER_MAX_JUMBO_FRAME_LEN);
 
-				break;
+				rte_pktmbuf_free(mbuf);
+				continue;
 			}
 		}
 
@@ -373,7 +374,7 @@ eth_pcap_tx_dumper(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts)
 	dumper_q->tx_stat.bytes += tx_bytes;
 	dumper_q->tx_stat.err_pkts += nb_pkts - num_tx;
 
-	return num_tx;
+	return nb_pkts;
 }
 
 /*
@@ -439,7 +440,8 @@ eth_pcap_tx(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts)
 					mbuf->pkt_len,
 					RTE_ETHER_MAX_JUMBO_FRAME_LEN);
 
-				break;
+				rte_pktmbuf_free(mbuf);
+				continue;
 			}
 		}
 
@@ -452,9 +454,9 @@ eth_pcap_tx(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts)
 
 	tx_queue->tx_stat.pkts += num_tx;
 	tx_queue->tx_stat.bytes += tx_bytes;
-	tx_queue->tx_stat.err_pkts += nb_pkts - num_tx;
+	tx_queue->tx_stat.err_pkts += i - num_tx;
 
-	return num_tx;
+	return i;
 }
 
 /*
-- 
1.8.3.1


^ permalink raw reply	[flat|nested] 23+ messages in thread

* [dpdk-dev] [PATCH v3 3/3] net/pcap: fix concurrent multiseg packet transmits
  2019-07-25 19:24 ` [dpdk-dev] [PATCH v3 0/3] Multiseg fixes for pcap pmd David Marchand
  2019-07-25 19:24   ` [dpdk-dev] [PATCH v3 1/3] net/pcap: fix Rx with small buffers David Marchand
  2019-07-25 19:24   ` [dpdk-dev] [PATCH v3 2/3] net/pcap: fix transmit return count in error conditions David Marchand
@ 2019-07-25 19:24   ` David Marchand
  2019-07-25 22:36   ` [dpdk-dev] [PATCH v3 0/3] Multiseg fixes for pcap pmd Ferruh Yigit
  3 siblings, 0 replies; 23+ messages in thread
From: David Marchand @ 2019-07-25 19:24 UTC (permalink / raw)
  To: dev; +Cc: ferruh.yigit, stable

Two cores can send multi segment packets on two different pcap ports.
Because of this, we can't have one single buffer to linearize packets.

Use rte_pktmbuf_read() to copy the packet into a buffer on the stack
and remove eth_pcap_gather_data() when necessary (if the mbuf is
contiguous, rte_pktmbuf_read() just points at the buffer address).

Fixes: 6db141c91e1f ("pcap: support jumbo frames")
Cc: stable@dpdk.org

Signed-off-by: David Marchand <david.marchand@redhat.com>
Acked-by: Ferruh Yigit <ferruh.yigit@intel.com>
---
Changelog since v2:
- fixed commit log
- fixed coding style

Changelog since v1:
- rely on rte_pktmbuf_read to handle both mono and multi segments cases

---
 drivers/net/pcap/rte_eth_pcap.c | 95 +++++++++++++++++------------------------
 1 file changed, 38 insertions(+), 57 deletions(-)

diff --git a/drivers/net/pcap/rte_eth_pcap.c b/drivers/net/pcap/rte_eth_pcap.c
index bfc0756..da03b97 100644
--- a/drivers/net/pcap/rte_eth_pcap.c
+++ b/drivers/net/pcap/rte_eth_pcap.c
@@ -46,7 +46,6 @@
 #define RTE_PMD_PCAP_MAX_QUEUES 16
 
 static char errbuf[PCAP_ERRBUF_SIZE];
-static unsigned char tx_pcap_data[RTE_ETH_PCAP_SNAPLEN];
 static struct timeval start_time;
 static uint64_t start_cycles;
 static uint64_t hz;
@@ -180,21 +179,6 @@ eth_pcap_rx_jumbo(struct rte_mempool *mb_pool, struct rte_mbuf *mbuf,
 	return mbuf->nb_segs;
 }
 
-/* Copy data from mbuf chain to a buffer suitable for writing to a PCAP file. */
-static void
-eth_pcap_gather_data(unsigned char *data, struct rte_mbuf *mbuf)
-{
-	uint16_t data_len = 0;
-
-	while (mbuf) {
-		rte_memcpy(data + data_len, rte_pktmbuf_mtod(mbuf, void *),
-			mbuf->data_len);
-
-		data_len += mbuf->data_len;
-		mbuf = mbuf->next;
-	}
-}
-
 static uint16_t
 eth_pcap_rx_infinite(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts)
 {
@@ -325,6 +309,8 @@ eth_pcap_tx_dumper(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts)
 	uint32_t tx_bytes = 0;
 	struct pcap_pkthdr header;
 	pcap_dumper_t *dumper;
+	unsigned char temp_data[RTE_ETH_PCAP_SNAPLEN];
+	size_t len;
 
 	pp = rte_eth_devices[dumper_q->port_id].process_private;
 	dumper = pp->tx_dumper[dumper_q->queue_id];
@@ -336,31 +322,28 @@ eth_pcap_tx_dumper(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts)
 	 * dumper */
 	for (i = 0; i < nb_pkts; i++) {
 		mbuf = bufs[i];
+		len = rte_pktmbuf_pkt_len(mbuf);
+		if (unlikely(!rte_pktmbuf_is_contiguous(mbuf) &&
+				len > sizeof(temp_data))) {
+			PMD_LOG(ERR,
+				"Dropping multi segment PCAP packet. Size (%zd) > max size (%zd).",
+				len, sizeof(temp_data));
+			rte_pktmbuf_free(mbuf);
+			continue;
+		}
+
 		calculate_timestamp(&header.ts);
-		header.len = mbuf->pkt_len;
+		header.len = len;
 		header.caplen = header.len;
-
-		if (likely(mbuf->nb_segs == 1)) {
-			pcap_dump((u_char *)dumper, &header,
-				  rte_pktmbuf_mtod(mbuf, void*));
-		} else {
-			if (mbuf->pkt_len <= RTE_ETHER_MAX_JUMBO_FRAME_LEN) {
-				eth_pcap_gather_data(tx_pcap_data, mbuf);
-				pcap_dump((u_char *)dumper, &header,
-					  tx_pcap_data);
-			} else {
-				PMD_LOG(ERR,
-					"Dropping PCAP packet. Size (%d) > max jumbo size (%d).",
-					mbuf->pkt_len,
-					RTE_ETHER_MAX_JUMBO_FRAME_LEN);
-
-				rte_pktmbuf_free(mbuf);
-				continue;
-			}
-		}
+		/* 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.
+		 */
+		pcap_dump((u_char *)dumper, &header,
+				rte_pktmbuf_read(mbuf, 0, len, temp_data));
 
 		num_tx++;
-		tx_bytes += mbuf->pkt_len;
+		tx_bytes += len;
 		rte_pktmbuf_free(mbuf);
 	}
 
@@ -415,6 +398,8 @@ eth_pcap_tx(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts)
 	uint16_t num_tx = 0;
 	uint32_t tx_bytes = 0;
 	pcap_t *pcap;
+	unsigned char temp_data[RTE_ETH_PCAP_SNAPLEN];
+	size_t len;
 
 	pp = rte_eth_devices[tx_queue->port_id].process_private;
 	pcap = pp->tx_pcap[tx_queue->queue_id];
@@ -424,31 +409,27 @@ eth_pcap_tx(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts)
 
 	for (i = 0; i < nb_pkts; i++) {
 		mbuf = bufs[i];
-
-		if (likely(mbuf->nb_segs == 1)) {
-			ret = pcap_sendpacket(pcap,
-					rte_pktmbuf_mtod(mbuf, u_char *),
-					mbuf->pkt_len);
-		} else {
-			if (mbuf->pkt_len <= RTE_ETHER_MAX_JUMBO_FRAME_LEN) {
-				eth_pcap_gather_data(tx_pcap_data, mbuf);
-				ret = pcap_sendpacket(pcap,
-						tx_pcap_data, mbuf->pkt_len);
-			} else {
-				PMD_LOG(ERR,
-					"Dropping PCAP packet. Size (%d) > max jumbo size (%d).",
-					mbuf->pkt_len,
-					RTE_ETHER_MAX_JUMBO_FRAME_LEN);
-
-				rte_pktmbuf_free(mbuf);
-				continue;
-			}
+		len = rte_pktmbuf_pkt_len(mbuf);
+		if (unlikely(!rte_pktmbuf_is_contiguous(mbuf) &&
+				len > sizeof(temp_data))) {
+			PMD_LOG(ERR,
+				"Dropping multi segment PCAP packet. Size (%zd) > max size (%zd).",
+				len, sizeof(temp_data));
+			rte_pktmbuf_free(mbuf);
+			continue;
 		}
 
+		/* 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.
+		 */
+		ret = pcap_sendpacket(pcap,
+				rte_pktmbuf_read(mbuf, 0, len, temp_data),
+				len);
 		if (unlikely(ret != 0))
 			break;
 		num_tx++;
-		tx_bytes += mbuf->pkt_len;
+		tx_bytes += len;
 		rte_pktmbuf_free(mbuf);
 	}
 
-- 
1.8.3.1


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [dpdk-dev] [PATCH v3 0/3] Multiseg fixes for pcap pmd
  2019-07-25 19:24 ` [dpdk-dev] [PATCH v3 0/3] Multiseg fixes for pcap pmd David Marchand
                     ` (2 preceding siblings ...)
  2019-07-25 19:24   ` [dpdk-dev] [PATCH v3 3/3] net/pcap: fix concurrent multiseg packet transmits David Marchand
@ 2019-07-25 22:36   ` Ferruh Yigit
  3 siblings, 0 replies; 23+ messages in thread
From: Ferruh Yigit @ 2019-07-25 22:36 UTC (permalink / raw)
  To: David Marchand, dev

On 7/25/2019 8:24 PM, David Marchand wrote:
> Here are some fixes caught while looking at oerrors statistics for this
> driver.
> The second patch can be seen as a revert or a followup of [1].
> 
> 1: https://git.dpdk.org/dpdk/commit/?id=49a0a2ffd5db
> 
> Changelog since v2:
> - coding style fixes in patch3,
> - added acks,
> 
> Changelog since v1:
> - comments from Ferruh on patch2,
> - little simplification in patch3,
> 

Series applied to dpdk-next-net/master, thanks.

^ permalink raw reply	[flat|nested] 23+ messages in thread

end of thread, other threads:[~2019-07-25 22:36 UTC | newest]

Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-07-24 11:54 [dpdk-dev] [PATCH 0/3] Multiseg fixes for pcap pmd David Marchand
2019-07-24 11:54 ` [dpdk-dev] [PATCH 1/3] net/pcap: fix Rx with small buffers David Marchand
2019-07-24 18:28   ` Ferruh Yigit
2019-07-24 11:54 ` [dpdk-dev] [PATCH 2/3] net/pcap: fix transmit return count in error conditions David Marchand
2019-07-24 18:36   ` Ferruh Yigit
2019-07-25  7:40     ` David Marchand
2019-07-25 11:01       ` Ferruh Yigit
2019-07-24 11:54 ` [dpdk-dev] [PATCH 3/3] net/pcap: fix concurrent multiseg packet transmits David Marchand
2019-07-24 18:43   ` Ferruh Yigit
2019-07-25  8:18   ` [dpdk-dev] [dpdk-stable] " David Marchand
2019-07-25 11:07     ` Ferruh Yigit
2019-07-25 12:04 ` [dpdk-dev] [PATCH v2 0/3] Multiseg fixes for pcap pmd David Marchand
2019-07-25 12:04   ` [dpdk-dev] [PATCH v2 1/3] net/pcap: fix Rx with small buffers David Marchand
2019-07-25 12:04   ` [dpdk-dev] [PATCH v2 2/3] net/pcap: fix transmit return count in error conditions David Marchand
2019-07-25 14:43     ` Ferruh Yigit
2019-07-25 12:04   ` [dpdk-dev] [PATCH v2 3/3] net/pcap: fix concurrent multiseg packet transmits David Marchand
2019-07-25 12:05     ` [dpdk-dev] [dpdk-stable] " David Marchand
2019-07-25 14:47     ` [dpdk-dev] " Ferruh Yigit
2019-07-25 19:24 ` [dpdk-dev] [PATCH v3 0/3] Multiseg fixes for pcap pmd David Marchand
2019-07-25 19:24   ` [dpdk-dev] [PATCH v3 1/3] net/pcap: fix Rx with small buffers David Marchand
2019-07-25 19:24   ` [dpdk-dev] [PATCH v3 2/3] net/pcap: fix transmit return count in error conditions David Marchand
2019-07-25 19:24   ` [dpdk-dev] [PATCH v3 3/3] net/pcap: fix concurrent multiseg packet transmits David Marchand
2019-07-25 22:36   ` [dpdk-dev] [PATCH v3 0/3] Multiseg fixes for pcap pmd Ferruh Yigit

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).