[dpdk-dev] removing mbuf error flags

[dpdk-dev] removing mbuf error flags

[Olivier Matz]

Hi,

In rte_mbuf.h, some rx flags are set to 0 since a long time since
nearly 2 years. It means nobody use them. They were introduced by
the following commit:

  http://dpdk.org/browse/dpdk/commit/?id=c22265f6

As far as I understand, these flags were introduced to let the
application know that a received packet is invalid.

The 2 drivers using them are i40e and enic. But as this flags are 0
today, it means that invalid packets are silently given to the
application.

My opinion is that invalid packets should not be given to the
application and only a statistic counter should be incremented.
No application check these flags today (in examples, or testpmd).

I would like to remove these flags.
Thoughs?

Olivier

Re: [dpdk-dev] removing mbuf error flags

[John Daley (johndale)]

Hi,

> -----Original Message-----
> From: Olivier Matz [mailto:olivier.matz@6wind.com]
> Sent: Friday, April 29, 2016 5:25 AM
> To: dev@dpdk.org; Zhang, Helin <helin.zhang@intel.com>
> Cc: Ananyev, Konstantin <konstantin.ananyev@intel.com>; John Daley
> (johndale) <johndale@cisco.com>
> Subject: removing mbuf error flags
> 
> Hi,
> 
> In rte_mbuf.h, some rx flags are set to 0 since a long time since nearly 2
> years. It means nobody use them. They were introduced by the following
> commit:
> 
>   http://dpdk.org/browse/dpdk/commit/?id=c22265f6
> 
> As far as I understand, these flags were introduced to let the application
> know that a received packet is invalid.
> 
> The 2 drivers using them are i40e and enic. But as this flags are 0 today, it
> means that invalid packets are silently given to the application.
> 
> My opinion is that invalid packets should not be given to the application and
> only a statistic counter should be incremented.
> No application check these flags today (in examples, or testpmd).
> 
> I would like to remove these flags.
> Thoughs?

I agree. Enic needs a little work to increment a counter and update internal indexes correctly. If you are in a hurry, feel free to 's/PKT_RX_MAC_ERR/0/' in enic for now.

-John

Re: [dpdk-dev] removing mbuf error flags

[Don Provan]

>From: Olivier Matz [mailto:olivier.matz@6wind.com] 
>Subject: [dpdk-dev] removing mbuf error flags
>
>My opinion is that invalid packets should not be given to the application and only a statistic counter should be incremented.

The idea of an application that handles bad packets is perfectly valid. Most applications don't want to see them, of course, but, conceptually, some applications would want to ask for bad packets because they are specifically designed to handle various networking problems including those that result in bad packets that the application can look at and report. Furthermore, it makes technical sense for DPDK to support such applications.

Having said that, I have no idea if that's why that field was added, and I don’t myself care if DPDK provides that feature in the future. I just thought I'd put the idea out there in case it makes any difference to you. If it were me, I'd probably decide it isn't hurting anything and not bother to remove it in case some day someone wants to implement that feature in one driver or another.

-don provan
dprovan@bivio.net

Re: [dpdk-dev] removing mbuf error flags

[Jay Rolette]

On Fri, Apr 29, 2016 at 1:16 PM, Don Provan <dprovan@bivio.net> wrote:

> >From: Olivier Matz [mailto:olivier.matz@6wind.com]
> >Subject: [dpdk-dev] removing mbuf error flags
> >
> >My opinion is that invalid packets should not be given to the application
> and only a statistic counter should be incremented.
>
> The idea of an application that handles bad packets is perfectly valid.
> Most applications don't want to see them, of course, but, conceptually,
> some applications would want to ask for bad packets because they are
> specifically designed to handle various networking problems including those
> that result in bad packets that the application can look at and report.
> Furthermore, it makes technical sense for DPDK to support such applications.
>
> Having said that, I have no idea if that's why that field was added, and I
> don’t myself care if DPDK provides that feature in the future. I just
> thought I'd put the idea out there in case it makes any difference to you.
> If it were me, I'd probably decide it isn't hurting anything and not bother
> to remove it in case some day someone wants to implement that feature in
> one driver or another.
>

Yep. Pretty much any networking security product needs to see malformed
packets.

Jay

Re: [dpdk-dev] removing mbuf error flags

[Arnon Warshavsky]

On Fri, Apr 29, 2016 at 9:24 PM, Jay Rolette <rolette@infinite.io> wrote:

> On Fri, Apr 29, 2016 at 1:16 PM, Don Provan <dprovan@bivio.net> wrote:
>
> > >From: Olivier Matz [mailto:olivier.matz@6wind.com]
> > >Subject: [dpdk-dev] removing mbuf error flags
> > >
> > >My opinion is that invalid packets should not be given to the
> application
> > and only a statistic counter should be incremented.
> >
> > The idea of an application that handles bad packets is perfectly valid.
> > Most applications don't want to see them, of course, but, conceptually,
> > some applications would want to ask for bad packets because they are
> > specifically designed to handle various networking problems including
> those
> > that result in bad packets that the application can look at and report.
> > Furthermore, it makes technical sense for DPDK to support such
> applications.
> >
> > Having said that, I have no idea if that's why that field was added, and
> I
> > don’t myself care if DPDK provides that feature in the future. I just
> > thought I'd put the idea out there in case it makes any difference to
> you.
> > If it were me, I'd probably decide it isn't hurting anything and not
> bother
> > to remove it in case some day someone wants to implement that feature in
> > one driver or another.
> >
>
> Yep. Pretty much any networking security product needs to see malformed
> packets.
>
> Jay
>

+1 for letting the application see bad packets and decide what to do with
them.
We had some zero order insertion issues in the past where the ability to
let the application capture malformed/unexpected packets was very helpful.

/Arnon.

Re: [dpdk-dev] removing mbuf error flags

[Olivier MATZ]

Hi,

On 04/29/2016 10:00 PM, Arnon Warshavsky wrote:
> 
> 
> On Fri, Apr 29, 2016 at 9:24 PM, Jay Rolette <rolette@infinite.io
> <mailto:rolette@infinite.io>> wrote:
> 
>     On Fri, Apr 29, 2016 at 1:16 PM, Don Provan <dprovan@bivio.net
>     <mailto:dprovan@bivio.net>> wrote:
> 
>     > >From: Olivier Matz [mailto:olivier.matz@6wind.com <mailto:olivier.matz@6wind.com>]
>     > >Subject: [dpdk-dev] removing mbuf error flags
>     > >
>     > >My opinion is that invalid packets should not be given to the application
>     > and only a statistic counter should be incremented.
>     >
>     > The idea of an application that handles bad packets is perfectly valid.
>     > Most applications don't want to see them, of course, but, conceptually,
>     > some applications would want to ask for bad packets because they are
>     > specifically designed to handle various networking problems including those
>     > that result in bad packets that the application can look at and report.
>     > Furthermore, it makes technical sense for DPDK to support such applications.
>     >
>     > Having said that, I have no idea if that's why that field was added, and I
>     > don’t myself care if DPDK provides that feature in the future. I just
>     > thought I'd put the idea out there in case it makes any difference to you.
>     > If it were me, I'd probably decide it isn't hurting anything and not bother
>     > to remove it in case some day someone wants to implement that feature in
>     > one driver or another.
>     >
> 
>     Yep. Pretty much any networking security product needs to see malformed
>     packets.
> 
>     Jay
> 
> 
> +1 for letting the application see bad packets and decide what to do
> with them.
> We had some zero order insertion issues in the past where the ability to
> let the application capture malformed/unexpected packets was very helpful.

The point is today it's broken, and no application running on top of
DPDK check these flags because they are set to 0. If we decide to
assign a value to these flags, it will break the working applications
because they don't expect to receive invalid packets. Maybe a proper
solution would be to enable these flags on demand in PMD configuration,
and add a feature flag for this feature.

I think we should not keep things half-done too long. It's
confusing and useless as-is.

If some applications really need to see these malformed packets,
the API has to define in which conditions these flags are set and
what is expected in the mbuf data when one of these flags is set.
The only documentation we have now is:

  PKT_RX_OVERSIZE: Num of desc of an RX pkt oversize.
  PKT_RX_HBUF_OVERFLOW: Header buffer overflow.
  PKT_RX_RECIP_ERR: Hardware processing error.
  PKT_RX_MAC_ERR: MAC error.

If it's not better defined, I don't know how an application could
use these flags.

Also, the PMDs should not behave differently by default.

If someone commit on working on this in the comming weeks, I'll be
happy to help, else I still think the current state has to be reverted.


Regards,
Olivier

Re: [dpdk-dev] removing mbuf error flags

[Don Provan]

>From: Olivier MATZ [mailto:olivier.matz@6wind.com] 
>Sent: Friday, April 29, 2016 1:58 PM
>
>The point is today it's broken, and no application running on top of DPDK
>check these flags because they are set to 0. If we decide to assign a value
>to these flags, it will break the working applications because they don't
>expect to receive invalid packets. Maybe a proper solution would be to
>enable these flags on demand in PMD configuration, and add a feature
>flag for this feature.

It's not broken, it just doesn't do anything. Yes, such a feature *has* to be explicitly requested by the application. By default, broken packets should not be delivered.

>I think we should not keep things half-done too long. It's confusing and useless as-is.

Fine with me. I don't see how it's confusing, but, from what you're saying, it is clearly useless. The only reason to keep it would be that if such a feature is added in the future, it could be added without changing the mbuf structure, but I don't know whether that's important.

-don provan
dprovan@bivio.net

[dpdk-dev] [RFC] mbuf: remove unused rx error flags

[Olivier Matz]

Following the discussions from:
http://dpdk.org/ml/archives/dev/2015-July/021721.html
http://dpdk.org/ml/archives/dev/2016-April/038143.html

The value of these flags is 0, making them useless. Today, no example
application checks them on RX, and only few drivers sets them, and
silently give wrong packets to the application, which should not happen.

This patch removes the unused flags from rte_mbuf, and their use in the
drivers. The enic driver is modified to drop bad packets, but i40e and
fm10k are kept as they are today and should be fixed to drop bad packets.

Fixes: c22265f6 ("mbuf: add new packet flags for i40e")
Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
---

Hi,

Here is a draft patch that removes the rx mbuf flags, as discussed.

John, I did not test the patch on the enic driver, so please review it
carefully.

Comments are welcome.

Olivier


 drivers/net/enic/enic_rx.c         | 31 ++++++++++++++-----------------
 drivers/net/fm10k/fm10k_rxtx.c     | 16 ----------------
 drivers/net/fm10k/fm10k_rxtx_vec.c |  2 +-
 drivers/net/i40e/i40e_rxtx.c       | 15 ---------------
 lib/librte_mbuf/rte_mbuf.c         |  4 ----
 lib/librte_mbuf/rte_mbuf.h         |  5 +----
 6 files changed, 16 insertions(+), 57 deletions(-)

diff --git a/drivers/net/enic/enic_rx.c b/drivers/net/enic/enic_rx.c
index b3ad9ea..bad802e 100644
--- a/drivers/net/enic/enic_rx.c
+++ b/drivers/net/enic/enic_rx.c
@@ -144,20 +144,15 @@ enic_cq_rx_desc_n_bytes(struct cq_desc *cqd)
 }
 
 static inline uint8_t
-enic_cq_rx_to_pkt_err_flags(struct cq_desc *cqd, uint64_t *pkt_err_flags_out)
+enic_cq_rx_check_err(struct cq_desc *cqd)
 {
 	struct cq_enet_rq_desc *cqrd = (struct cq_enet_rq_desc *)cqd;
 	uint16_t bwflags;
-	int ret = 0;
-	uint64_t pkt_err_flags = 0;
 
 	bwflags = enic_cq_rx_desc_bwflags(cqrd);
-	if (unlikely(enic_cq_rx_desc_packet_error(bwflags))) {
-		pkt_err_flags = PKT_RX_MAC_ERR;
-		ret = 1;
-	}
-	*pkt_err_flags_out = pkt_err_flags;
-	return ret;
+	if (unlikely(enic_cq_rx_desc_packet_error(bwflags)))
+		return 1;
+	return 0;
 }
 
 /*
@@ -253,7 +248,7 @@ enic_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts,
 	struct enic *enic = vnic_dev_priv(rq->vdev);
 	unsigned int rx_id;
 	struct rte_mbuf *nmb, *rxmb;
-	uint16_t nb_rx = 0;
+	uint16_t nb_rx = 0, nb_err = 0;
 	uint16_t nb_hold;
 	struct vnic_cq *cq;
 	volatile struct cq_desc *cqd_ptr;
@@ -269,7 +264,6 @@ enic_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts,
 		volatile struct rq_enet_desc *rqd_ptr;
 		dma_addr_t dma_addr;
 		struct cq_desc cqd;
-		uint64_t ol_err_flags;
 		uint8_t packet_error;
 
 		/* Check for pkts available */
@@ -293,7 +287,7 @@ enic_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts,
 		}
 
 		/* A packet error means descriptor and data are untrusted */
-		packet_error = enic_cq_rx_to_pkt_err_flags(&cqd, &ol_err_flags);
+		packet_error = enic_cq_rx_check_err(&cqd);
 
 		/* Get the mbuf to return and replace with one just allocated */
 		rxmb = rq->mbuf_ring[rx_id];
@@ -318,6 +312,13 @@ enic_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts,
 		rqd_ptr->address = rte_cpu_to_le_64(dma_addr);
 		rqd_ptr->length_type = cpu_to_le16(nmb->buf_len);
 
+		/* Drop incoming bad packet */
+		if (packet_error) {
+			rte_pktmbuf_free(rxmb);
+			nb_err++;
+			continue;
+		}
+
 		/* Fill in the rest of the mbuf */
 		rxmb->data_off = RTE_PKTMBUF_HEADROOM;
 		rxmb->nb_segs = 1;
@@ -327,10 +328,6 @@ enic_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts,
 			rxmb->pkt_len = enic_cq_rx_desc_n_bytes(&cqd);
 			rxmb->packet_type = enic_cq_rx_flags_to_pkt_type(&cqd);
 			enic_cq_rx_to_pkt_flags(&cqd, rxmb);
-		} else {
-			rxmb->pkt_len = 0;
-			rxmb->packet_type = 0;
-			rxmb->ol_flags = 0;
 		}
 		rxmb->data_len = rxmb->pkt_len;
 
@@ -342,7 +339,7 @@ enic_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts,
 		rx_pkts[nb_rx++] = rxmb;
 	}
 
-	nb_hold += nb_rx;
+	nb_hold += nb_rx + nb_err;
 	cq->to_clean = rx_id;
 
 	if (nb_hold > rq->rx_free_thresh) {
diff --git a/drivers/net/fm10k/fm10k_rxtx.c b/drivers/net/fm10k/fm10k_rxtx.c
index 81ed4e7..dd92a91 100644
--- a/drivers/net/fm10k/fm10k_rxtx.c
+++ b/drivers/net/fm10k/fm10k_rxtx.c
@@ -96,22 +96,6 @@ rx_desc_to_ol_flags(struct rte_mbuf *m, const union fm10k_rx_desc *d)
 
 	if (d->w.pkt_info & FM10K_RXD_RSSTYPE_MASK)
 		m->ol_flags |= PKT_RX_RSS_HASH;
-
-	if (unlikely((d->d.staterr &
-		(FM10K_RXD_STATUS_IPCS | FM10K_RXD_STATUS_IPE)) ==
-		(FM10K_RXD_STATUS_IPCS | FM10K_RXD_STATUS_IPE)))
-		m->ol_flags |= PKT_RX_IP_CKSUM_BAD;
-
-	if (unlikely((d->d.staterr &
-		(FM10K_RXD_STATUS_L4CS | FM10K_RXD_STATUS_L4E)) ==
-		(FM10K_RXD_STATUS_L4CS | FM10K_RXD_STATUS_L4E)))
-		m->ol_flags |= PKT_RX_L4_CKSUM_BAD;
-
-	if (unlikely(d->d.staterr & FM10K_RXD_STATUS_HBO))
-		m->ol_flags |= PKT_RX_HBUF_OVERFLOW;
-
-	if (unlikely(d->d.staterr & FM10K_RXD_STATUS_RXE))
-		m->ol_flags |= PKT_RX_RECIP_ERR;
 }
 
 uint16_t
diff --git a/drivers/net/fm10k/fm10k_rxtx_vec.c b/drivers/net/fm10k/fm10k_rxtx_vec.c
index 2f3ccfe..b368289 100644
--- a/drivers/net/fm10k/fm10k_rxtx_vec.c
+++ b/drivers/net/fm10k/fm10k_rxtx_vec.c
@@ -101,7 +101,7 @@ fm10k_desc_to_olflags_v(__m128i descs[4], struct rte_mbuf **rx_pkts)
 	const __m128i rxe_flag = _mm_set_epi8(0, 0, 0, 0,
 			0, 0, 0, 0,
 			0, 0, 0, 0,
-			0, 0, PKT_RX_RECIP_ERR, 0);
+			0, 0, 0, 0);
 
 	/* map rss type to rss hash flag */
 	const __m128i rss_flags = _mm_set_epi8(0, 0, 0, 0,
diff --git a/drivers/net/i40e/i40e_rxtx.c b/drivers/net/i40e/i40e_rxtx.c
index 4d35d83..76da7d2 100644
--- a/drivers/net/i40e/i40e_rxtx.c
+++ b/drivers/net/i40e/i40e_rxtx.c
@@ -140,27 +140,12 @@ i40e_rxd_error_to_pkt_flags(uint64_t qword)
 #define I40E_RX_ERR_BITS 0x3f
 	if (likely((error_bits & I40E_RX_ERR_BITS) == 0))
 		return flags;
-	/* If RXE bit set, all other status bits are meaningless */
-	if (unlikely(error_bits & (1 << I40E_RX_DESC_ERROR_RXE_SHIFT))) {
-		flags |= PKT_RX_MAC_ERR;
-		return flags;
-	}
-
-	/* If RECIPE bit set, all other status indications should be ignored */
-	if (unlikely(error_bits & (1 << I40E_RX_DESC_ERROR_RECIPE_SHIFT))) {
-		flags |= PKT_RX_RECIP_ERR;
-		return flags;
-	}
-	if (unlikely(error_bits & (1 << I40E_RX_DESC_ERROR_HBO_SHIFT)))
-		flags |= PKT_RX_HBUF_OVERFLOW;
 	if (unlikely(error_bits & (1 << I40E_RX_DESC_ERROR_IPE_SHIFT)))
 		flags |= PKT_RX_IP_CKSUM_BAD;
 	if (unlikely(error_bits & (1 << I40E_RX_DESC_ERROR_L4E_SHIFT)))
 		flags |= PKT_RX_L4_CKSUM_BAD;
 	if (unlikely(error_bits & (1 << I40E_RX_DESC_ERROR_EIPE_SHIFT)))
 		flags |= PKT_RX_EIP_CKSUM_BAD;
-	if (unlikely(error_bits & (1 << I40E_RX_DESC_ERROR_OVERSIZE_SHIFT)))
-		flags |= PKT_RX_OVERSIZE;
 
 	return flags;
 }
diff --git a/lib/librte_mbuf/rte_mbuf.c b/lib/librte_mbuf/rte_mbuf.c
index eec1456..878db89 100644
--- a/lib/librte_mbuf/rte_mbuf.c
+++ b/lib/librte_mbuf/rte_mbuf.c
@@ -254,10 +254,6 @@ const char *rte_get_rx_ol_flag_name(uint64_t mask)
 	case PKT_RX_L4_CKSUM_BAD: return "PKT_RX_L4_CKSUM_BAD";
 	case PKT_RX_IP_CKSUM_BAD: return "PKT_RX_IP_CKSUM_BAD";
 	case PKT_RX_EIP_CKSUM_BAD: return "PKT_RX_EIP_CKSUM_BAD";
-	/* case PKT_RX_OVERSIZE: return "PKT_RX_OVERSIZE"; */
-	/* case PKT_RX_HBUF_OVERFLOW: return "PKT_RX_HBUF_OVERFLOW"; */
-	/* case PKT_RX_RECIP_ERR: return "PKT_RX_RECIP_ERR"; */
-	/* case PKT_RX_MAC_ERR: return "PKT_RX_MAC_ERR"; */
 	case PKT_RX_IEEE1588_PTP: return "PKT_RX_IEEE1588_PTP";
 	case PKT_RX_IEEE1588_TMST: return "PKT_RX_IEEE1588_TMST";
 	default: return NULL;
diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h
index e3ee0b3..3663d17 100644
--- a/lib/librte_mbuf/rte_mbuf.h
+++ b/lib/librte_mbuf/rte_mbuf.h
@@ -85,10 +85,7 @@ extern "C" {
 #define PKT_RX_L4_CKSUM_BAD  (1ULL << 3)  /**< L4 cksum of RX pkt. is not OK. */
 #define PKT_RX_IP_CKSUM_BAD  (1ULL << 4)  /**< IP cksum of RX pkt. is not OK. */
 #define PKT_RX_EIP_CKSUM_BAD (1ULL << 5)  /**< External IP header checksum error. */
-#define PKT_RX_OVERSIZE      (0ULL << 0)  /**< Num of desc of an RX pkt oversize. */
-#define PKT_RX_HBUF_OVERFLOW (0ULL << 0)  /**< Header buffer overflow. */
-#define PKT_RX_RECIP_ERR     (0ULL << 0)  /**< Hardware processing error. */
-#define PKT_RX_MAC_ERR       (0ULL << 0)  /**< MAC error. */
+/* hole, some bits can be reused here  */
 #define PKT_RX_IEEE1588_PTP  (1ULL << 9)  /**< RX IEEE1588 L2 Ethernet PT Packet. */
 #define PKT_RX_IEEE1588_TMST (1ULL << 10) /**< RX IEEE1588 L2/L4 timestamped packet.*/
 #define PKT_RX_FDIR_ID       (1ULL << 13) /**< FD id reported if FDIR match. */
-- 
2.8.0.rc3

Re: [dpdk-dev] [RFC] mbuf: remove unused rx error flags

[John Daley (johndale)]

Hi,

> -----Original Message-----
> From: Olivier Matz [mailto:olivier.matz@6wind.com]
> Sent: Tuesday, May 10, 2016 1:40 AM
> To: dev@dpdk.org
> Cc: konstantin.ananyev@intel.com; John Daley (johndale)
> <johndale@cisco.com>; helin.zhang@intel.com; arnon@qwilt.com;
> rolette@infinite.io
> Subject: [RFC] mbuf: remove unused rx error flags
> 
> Following the discussions from:
> http://dpdk.org/ml/archives/dev/2015-July/021721.html
> http://dpdk.org/ml/archives/dev/2016-April/038143.html
> 
> The value of these flags is 0, making them useless. Today, no example
> application checks them on RX, and only few drivers sets them, and silently
> give wrong packets to the application, which should not happen.
> 
> This patch removes the unused flags from rte_mbuf, and their use in the
> drivers. The enic driver is modified to drop bad packets, but i40e and fm10k
> are kept as they are today and should be fixed to drop bad packets.

Perhaps the change to enic to drop bad packets should be handled as a separate patch since it's not strictly related to not removing the use of the flags?

> 
> Fixes: c22265f6 ("mbuf: add new packet flags for i40e")
> Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
> ---
> 
> Hi,
> 
> Here is a draft patch that removes the rx mbuf flags, as discussed.
> 
> John, I did not test the patch on the enic driver, so please review it carefully.
> 

The patch works for enic, thanks. There are a few minor changes for performance:
 - put an unlikely in the if (packet_error) conditional
- remove the if (!packet_error) conditional since it will always be true.
Let me know if you would prefer I submit the enic patch separately.

> Comments are welcome.
> 
> Olivier
> 
> 
>  drivers/net/enic/enic_rx.c         | 31 ++++++++++++++-----------------
>  drivers/net/fm10k/fm10k_rxtx.c     | 16 ----------------
>  drivers/net/fm10k/fm10k_rxtx_vec.c |  2 +-
>  drivers/net/i40e/i40e_rxtx.c       | 15 ---------------
>  lib/librte_mbuf/rte_mbuf.c         |  4 ----
>  lib/librte_mbuf/rte_mbuf.h         |  5 +----
>  6 files changed, 16 insertions(+), 57 deletions(-)
> 
> diff --git a/drivers/net/enic/enic_rx.c b/drivers/net/enic/enic_rx.c index
> b3ad9ea..bad802e 100644
> --- a/drivers/net/enic/enic_rx.c
> +++ b/drivers/net/enic/enic_rx.c
> @@ -144,20 +144,15 @@ enic_cq_rx_desc_n_bytes(struct cq_desc *cqd)  }
> 
>  static inline uint8_t
> -enic_cq_rx_to_pkt_err_flags(struct cq_desc *cqd, uint64_t
> *pkt_err_flags_out)
> +enic_cq_rx_check_err(struct cq_desc *cqd)
>  {
>  	struct cq_enet_rq_desc *cqrd = (struct cq_enet_rq_desc *)cqd;
>  	uint16_t bwflags;
> -	int ret = 0;
> -	uint64_t pkt_err_flags = 0;
> 
>  	bwflags = enic_cq_rx_desc_bwflags(cqrd);
> -	if (unlikely(enic_cq_rx_desc_packet_error(bwflags))) {
> -		pkt_err_flags = PKT_RX_MAC_ERR;
> -		ret = 1;
> -	}
> -	*pkt_err_flags_out = pkt_err_flags;
> -	return ret;
> +	if (unlikely(enic_cq_rx_desc_packet_error(bwflags)))
> +		return 1;
> +	return 0;
>  }
> 
>  /*
> @@ -253,7 +248,7 @@ enic_recv_pkts(void *rx_queue, struct rte_mbuf
> **rx_pkts,
>  	struct enic *enic = vnic_dev_priv(rq->vdev);
>  	unsigned int rx_id;
>  	struct rte_mbuf *nmb, *rxmb;
> -	uint16_t nb_rx = 0;
> +	uint16_t nb_rx = 0, nb_err = 0;
>  	uint16_t nb_hold;
>  	struct vnic_cq *cq;
>  	volatile struct cq_desc *cqd_ptr;
> @@ -269,7 +264,6 @@ enic_recv_pkts(void *rx_queue, struct rte_mbuf
> **rx_pkts,
>  		volatile struct rq_enet_desc *rqd_ptr;
>  		dma_addr_t dma_addr;
>  		struct cq_desc cqd;
> -		uint64_t ol_err_flags;
>  		uint8_t packet_error;
> 
>  		/* Check for pkts available */
> @@ -293,7 +287,7 @@ enic_recv_pkts(void *rx_queue, struct rte_mbuf
> **rx_pkts,
>  		}
> 
>  		/* A packet error means descriptor and data are untrusted */
> -		packet_error = enic_cq_rx_to_pkt_err_flags(&cqd,
> &ol_err_flags);
> +		packet_error = enic_cq_rx_check_err(&cqd);
> 
>  		/* Get the mbuf to return and replace with one just allocated
> */
>  		rxmb = rq->mbuf_ring[rx_id];
> @@ -318,6 +312,13 @@ enic_recv_pkts(void *rx_queue, struct rte_mbuf
> **rx_pkts,
>  		rqd_ptr->address = rte_cpu_to_le_64(dma_addr);
>  		rqd_ptr->length_type = cpu_to_le16(nmb->buf_len);
> 
> +		/* Drop incoming bad packet */
> +		if (packet_error) {
> +			rte_pktmbuf_free(rxmb);
> +			nb_err++;
> +			continue;
> +		}
> +
>  		/* Fill in the rest of the mbuf */
>  		rxmb->data_off = RTE_PKTMBUF_HEADROOM;
>  		rxmb->nb_segs = 1;
> @@ -327,10 +328,6 @@ enic_recv_pkts(void *rx_queue, struct rte_mbuf
> **rx_pkts,
>  			rxmb->pkt_len = enic_cq_rx_desc_n_bytes(&cqd);
>  			rxmb->packet_type =
> enic_cq_rx_flags_to_pkt_type(&cqd);
>  			enic_cq_rx_to_pkt_flags(&cqd, rxmb);
> -		} else {
> -			rxmb->pkt_len = 0;
> -			rxmb->packet_type = 0;
> -			rxmb->ol_flags = 0;
>  		}
>  		rxmb->data_len = rxmb->pkt_len;
> 
> @@ -342,7 +339,7 @@ enic_recv_pkts(void *rx_queue, struct rte_mbuf
> **rx_pkts,
>  		rx_pkts[nb_rx++] = rxmb;
>  	}
> 
> -	nb_hold += nb_rx;
> +	nb_hold += nb_rx + nb_err;
>  	cq->to_clean = rx_id;
> 
>  	if (nb_hold > rq->rx_free_thresh) {
> diff --git a/drivers/net/fm10k/fm10k_rxtx.c
> b/drivers/net/fm10k/fm10k_rxtx.c index 81ed4e7..dd92a91 100644
> --- a/drivers/net/fm10k/fm10k_rxtx.c
> +++ b/drivers/net/fm10k/fm10k_rxtx.c
> @@ -96,22 +96,6 @@ rx_desc_to_ol_flags(struct rte_mbuf *m, const union
> fm10k_rx_desc *d)
> 
>  	if (d->w.pkt_info & FM10K_RXD_RSSTYPE_MASK)
>  		m->ol_flags |= PKT_RX_RSS_HASH;
> -
> -	if (unlikely((d->d.staterr &
> -		(FM10K_RXD_STATUS_IPCS | FM10K_RXD_STATUS_IPE)) ==
> -		(FM10K_RXD_STATUS_IPCS | FM10K_RXD_STATUS_IPE)))
> -		m->ol_flags |= PKT_RX_IP_CKSUM_BAD;
> -
> -	if (unlikely((d->d.staterr &
> -		(FM10K_RXD_STATUS_L4CS | FM10K_RXD_STATUS_L4E)) ==
> -		(FM10K_RXD_STATUS_L4CS | FM10K_RXD_STATUS_L4E)))
> -		m->ol_flags |= PKT_RX_L4_CKSUM_BAD;
> -
> -	if (unlikely(d->d.staterr & FM10K_RXD_STATUS_HBO))
> -		m->ol_flags |= PKT_RX_HBUF_OVERFLOW;
> -
> -	if (unlikely(d->d.staterr & FM10K_RXD_STATUS_RXE))
> -		m->ol_flags |= PKT_RX_RECIP_ERR;
>  }
> 
>  uint16_t
> diff --git a/drivers/net/fm10k/fm10k_rxtx_vec.c
> b/drivers/net/fm10k/fm10k_rxtx_vec.c
> index 2f3ccfe..b368289 100644
> --- a/drivers/net/fm10k/fm10k_rxtx_vec.c
> +++ b/drivers/net/fm10k/fm10k_rxtx_vec.c
> @@ -101,7 +101,7 @@ fm10k_desc_to_olflags_v(__m128i descs[4], struct
> rte_mbuf **rx_pkts)
>  	const __m128i rxe_flag = _mm_set_epi8(0, 0, 0, 0,
>  			0, 0, 0, 0,
>  			0, 0, 0, 0,
> -			0, 0, PKT_RX_RECIP_ERR, 0);
> +			0, 0, 0, 0);
> 
>  	/* map rss type to rss hash flag */
>  	const __m128i rss_flags = _mm_set_epi8(0, 0, 0, 0, diff --git
> a/drivers/net/i40e/i40e_rxtx.c b/drivers/net/i40e/i40e_rxtx.c index
> 4d35d83..76da7d2 100644
> --- a/drivers/net/i40e/i40e_rxtx.c
> +++ b/drivers/net/i40e/i40e_rxtx.c
> @@ -140,27 +140,12 @@ i40e_rxd_error_to_pkt_flags(uint64_t qword)
> #define I40E_RX_ERR_BITS 0x3f
>  	if (likely((error_bits & I40E_RX_ERR_BITS) == 0))
>  		return flags;
> -	/* If RXE bit set, all other status bits are meaningless */
> -	if (unlikely(error_bits & (1 << I40E_RX_DESC_ERROR_RXE_SHIFT))) {
> -		flags |= PKT_RX_MAC_ERR;
> -		return flags;
> -	}
> -
> -	/* If RECIPE bit set, all other status indications should be ignored */
> -	if (unlikely(error_bits & (1 <<
> I40E_RX_DESC_ERROR_RECIPE_SHIFT))) {
> -		flags |= PKT_RX_RECIP_ERR;
> -		return flags;
> -	}
> -	if (unlikely(error_bits & (1 << I40E_RX_DESC_ERROR_HBO_SHIFT)))
> -		flags |= PKT_RX_HBUF_OVERFLOW;
>  	if (unlikely(error_bits & (1 << I40E_RX_DESC_ERROR_IPE_SHIFT)))
>  		flags |= PKT_RX_IP_CKSUM_BAD;
>  	if (unlikely(error_bits & (1 << I40E_RX_DESC_ERROR_L4E_SHIFT)))
>  		flags |= PKT_RX_L4_CKSUM_BAD;
>  	if (unlikely(error_bits & (1 << I40E_RX_DESC_ERROR_EIPE_SHIFT)))
>  		flags |= PKT_RX_EIP_CKSUM_BAD;
> -	if (unlikely(error_bits & (1 <<
> I40E_RX_DESC_ERROR_OVERSIZE_SHIFT)))
> -		flags |= PKT_RX_OVERSIZE;
> 
>  	return flags;
>  }
> diff --git a/lib/librte_mbuf/rte_mbuf.c b/lib/librte_mbuf/rte_mbuf.c index
> eec1456..878db89 100644
> --- a/lib/librte_mbuf/rte_mbuf.c
> +++ b/lib/librte_mbuf/rte_mbuf.c
> @@ -254,10 +254,6 @@ const char *rte_get_rx_ol_flag_name(uint64_t
> mask)
>  	case PKT_RX_L4_CKSUM_BAD: return "PKT_RX_L4_CKSUM_BAD";
>  	case PKT_RX_IP_CKSUM_BAD: return "PKT_RX_IP_CKSUM_BAD";
>  	case PKT_RX_EIP_CKSUM_BAD: return "PKT_RX_EIP_CKSUM_BAD";
> -	/* case PKT_RX_OVERSIZE: return "PKT_RX_OVERSIZE"; */
> -	/* case PKT_RX_HBUF_OVERFLOW: return
> "PKT_RX_HBUF_OVERFLOW"; */
> -	/* case PKT_RX_RECIP_ERR: return "PKT_RX_RECIP_ERR"; */
> -	/* case PKT_RX_MAC_ERR: return "PKT_RX_MAC_ERR"; */
>  	case PKT_RX_IEEE1588_PTP: return "PKT_RX_IEEE1588_PTP";
>  	case PKT_RX_IEEE1588_TMST: return "PKT_RX_IEEE1588_TMST";
>  	default: return NULL;
> diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h index
> e3ee0b3..3663d17 100644
> --- a/lib/librte_mbuf/rte_mbuf.h
> +++ b/lib/librte_mbuf/rte_mbuf.h
> @@ -85,10 +85,7 @@ extern "C" {
>  #define PKT_RX_L4_CKSUM_BAD  (1ULL << 3)  /**< L4 cksum of RX pkt. is
> not OK. */  #define PKT_RX_IP_CKSUM_BAD  (1ULL << 4)  /**< IP cksum of
> RX pkt. is not OK. */  #define PKT_RX_EIP_CKSUM_BAD (1ULL << 5)  /**<
> External IP header checksum error. */
> -#define PKT_RX_OVERSIZE      (0ULL << 0)  /**< Num of desc of an RX pkt
> oversize. */
> -#define PKT_RX_HBUF_OVERFLOW (0ULL << 0)  /**< Header buffer
> overflow. */
> -#define PKT_RX_RECIP_ERR     (0ULL << 0)  /**< Hardware processing error.
> */
> -#define PKT_RX_MAC_ERR       (0ULL << 0)  /**< MAC error. */
> +/* hole, some bits can be reused here  */
>  #define PKT_RX_IEEE1588_PTP  (1ULL << 9)  /**< RX IEEE1588 L2 Ethernet PT
> Packet. */  #define PKT_RX_IEEE1588_TMST (1ULL << 10) /**< RX IEEE1588
> L2/L4 timestamped packet.*/
>  #define PKT_RX_FDIR_ID       (1ULL << 13) /**< FD id reported if FDIR match.
> */
> --
> 2.8.0.rc3

Re: [dpdk-dev] [RFC] mbuf: remove unused rx error flags

[Olivier MATZ]

Hi,

On 05/12/2016 03:32 AM, John Daley (johndale) wrote:
>> This patch removes the unused flags from rte_mbuf, and their use in
>> the drivers. The enic driver is modified to drop bad packets, but
>> i40e and fm10k are kept as they are today and should be fixed to
>> drop bad packets.
>
> Perhaps the change to enic to drop bad packets should be handled as a
> separate patch since it's not strictly related to not removing the
> use of the flags?

Yes, it's probably better to have it in a separate patch.

>> John, I did not test the patch on the enic driver, so please review
>> it carefully.
>>
>
> The patch works for enic, thanks. There are a few minor changes for
> performance: - put an unlikely in the if (packet_error) conditional -
> remove the if (!packet_error) conditional since it will always be
> true. Let me know if you would prefer I submit the enic patch
> separately.

I'll do it, thanks for reviewing.
I'll wait a bit for other comments before sending a first version of
the patchset.

Regards,
Olivier

[dpdk-dev] [PATCH] mbuf: remove unused Rx error flags

[Olivier Matz]

Following the discussions from:
http://dpdk.org/ml/archives/dev/2015-July/021721.html
http://dpdk.org/ml/archives/dev/2016-April/038143.html

The value of these flags is 0, making them useless. Today, no example
application checks them on Rx, and only few drivers sets them and
silently give wrong packets to the application, which should not happen.

This patch removes the unused flags from rte_mbuf and their use in the
drivers. The i40e and fm10k are kept as they are today and should be
fixed to drop bad packets. The enic driver is managed by its maintainer
in another patch.

Fixes: c22265f6 ("mbuf: add new packet flags for i40e")
Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
---

RFC -> v1:
- remove the enic part of the patch, it is managed separately
  in this patch:
  http://dpdk.org/ml/archives/dev/2016-May/039452.html
  Note that this enic series should be applied before this patch
  to avoid breaking the compilation.
- fix title according to scripts/check-git-log.sh


 drivers/net/fm10k/fm10k_rxtx.c     | 16 ----------------
 drivers/net/fm10k/fm10k_rxtx_vec.c |  2 +-
 drivers/net/i40e/i40e_rxtx.c       | 15 ---------------
 lib/librte_mbuf/rte_mbuf.c         |  4 ----
 lib/librte_mbuf/rte_mbuf.h         |  5 +----
 5 files changed, 2 insertions(+), 40 deletions(-)

diff --git a/drivers/net/fm10k/fm10k_rxtx.c b/drivers/net/fm10k/fm10k_rxtx.c
index 81ed4e7..dd92a91 100644
--- a/drivers/net/fm10k/fm10k_rxtx.c
+++ b/drivers/net/fm10k/fm10k_rxtx.c
@@ -96,22 +96,6 @@ rx_desc_to_ol_flags(struct rte_mbuf *m, const union fm10k_rx_desc *d)
 
 	if (d->w.pkt_info & FM10K_RXD_RSSTYPE_MASK)
 		m->ol_flags |= PKT_RX_RSS_HASH;
-
-	if (unlikely((d->d.staterr &
-		(FM10K_RXD_STATUS_IPCS | FM10K_RXD_STATUS_IPE)) ==
-		(FM10K_RXD_STATUS_IPCS | FM10K_RXD_STATUS_IPE)))
-		m->ol_flags |= PKT_RX_IP_CKSUM_BAD;
-
-	if (unlikely((d->d.staterr &
-		(FM10K_RXD_STATUS_L4CS | FM10K_RXD_STATUS_L4E)) ==
-		(FM10K_RXD_STATUS_L4CS | FM10K_RXD_STATUS_L4E)))
-		m->ol_flags |= PKT_RX_L4_CKSUM_BAD;
-
-	if (unlikely(d->d.staterr & FM10K_RXD_STATUS_HBO))
-		m->ol_flags |= PKT_RX_HBUF_OVERFLOW;
-
-	if (unlikely(d->d.staterr & FM10K_RXD_STATUS_RXE))
-		m->ol_flags |= PKT_RX_RECIP_ERR;
 }
 
 uint16_t
diff --git a/drivers/net/fm10k/fm10k_rxtx_vec.c b/drivers/net/fm10k/fm10k_rxtx_vec.c
index 03e4a5c..d417c96 100644
--- a/drivers/net/fm10k/fm10k_rxtx_vec.c
+++ b/drivers/net/fm10k/fm10k_rxtx_vec.c
@@ -101,7 +101,7 @@ fm10k_desc_to_olflags_v(__m128i descs[4], struct rte_mbuf **rx_pkts)
 	const __m128i rxe_flag = _mm_set_epi8(0, 0, 0, 0,
 			0, 0, 0, 0,
 			0, 0, 0, 0,
-			0, 0, PKT_RX_RECIP_ERR, 0);
+			0, 0, 0, 0);
 
 	/* map rss type to rss hash flag */
 	const __m128i rss_flags = _mm_set_epi8(0, 0, 0, 0,
diff --git a/drivers/net/i40e/i40e_rxtx.c b/drivers/net/i40e/i40e_rxtx.c
index c833aa3..c010770 100644
--- a/drivers/net/i40e/i40e_rxtx.c
+++ b/drivers/net/i40e/i40e_rxtx.c
@@ -140,27 +140,12 @@ i40e_rxd_error_to_pkt_flags(uint64_t qword)
 #define I40E_RX_ERR_BITS 0x3f
 	if (likely((error_bits & I40E_RX_ERR_BITS) == 0))
 		return flags;
-	/* If RXE bit set, all other status bits are meaningless */
-	if (unlikely(error_bits & (1 << I40E_RX_DESC_ERROR_RXE_SHIFT))) {
-		flags |= PKT_RX_MAC_ERR;
-		return flags;
-	}
-
-	/* If RECIPE bit set, all other status indications should be ignored */
-	if (unlikely(error_bits & (1 << I40E_RX_DESC_ERROR_RECIPE_SHIFT))) {
-		flags |= PKT_RX_RECIP_ERR;
-		return flags;
-	}
-	if (unlikely(error_bits & (1 << I40E_RX_DESC_ERROR_HBO_SHIFT)))
-		flags |= PKT_RX_HBUF_OVERFLOW;
 	if (unlikely(error_bits & (1 << I40E_RX_DESC_ERROR_IPE_SHIFT)))
 		flags |= PKT_RX_IP_CKSUM_BAD;
 	if (unlikely(error_bits & (1 << I40E_RX_DESC_ERROR_L4E_SHIFT)))
 		flags |= PKT_RX_L4_CKSUM_BAD;
 	if (unlikely(error_bits & (1 << I40E_RX_DESC_ERROR_EIPE_SHIFT)))
 		flags |= PKT_RX_EIP_CKSUM_BAD;
-	if (unlikely(error_bits & (1 << I40E_RX_DESC_ERROR_OVERSIZE_SHIFT)))
-		flags |= PKT_RX_OVERSIZE;
 
 	return flags;
 }
diff --git a/lib/librte_mbuf/rte_mbuf.c b/lib/librte_mbuf/rte_mbuf.c
index eec1456..878db89 100644
--- a/lib/librte_mbuf/rte_mbuf.c
+++ b/lib/librte_mbuf/rte_mbuf.c
@@ -254,10 +254,6 @@ const char *rte_get_rx_ol_flag_name(uint64_t mask)
 	case PKT_RX_L4_CKSUM_BAD: return "PKT_RX_L4_CKSUM_BAD";
 	case PKT_RX_IP_CKSUM_BAD: return "PKT_RX_IP_CKSUM_BAD";
 	case PKT_RX_EIP_CKSUM_BAD: return "PKT_RX_EIP_CKSUM_BAD";
-	/* case PKT_RX_OVERSIZE: return "PKT_RX_OVERSIZE"; */
-	/* case PKT_RX_HBUF_OVERFLOW: return "PKT_RX_HBUF_OVERFLOW"; */
-	/* case PKT_RX_RECIP_ERR: return "PKT_RX_RECIP_ERR"; */
-	/* case PKT_RX_MAC_ERR: return "PKT_RX_MAC_ERR"; */
 	case PKT_RX_IEEE1588_PTP: return "PKT_RX_IEEE1588_PTP";
 	case PKT_RX_IEEE1588_TMST: return "PKT_RX_IEEE1588_TMST";
 	default: return NULL;
diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h
index 48911a6..3d2c09b 100644
--- a/lib/librte_mbuf/rte_mbuf.h
+++ b/lib/librte_mbuf/rte_mbuf.h
@@ -85,10 +85,7 @@ extern "C" {
 #define PKT_RX_L4_CKSUM_BAD  (1ULL << 3)  /**< L4 cksum of RX pkt. is not OK. */
 #define PKT_RX_IP_CKSUM_BAD  (1ULL << 4)  /**< IP cksum of RX pkt. is not OK. */
 #define PKT_RX_EIP_CKSUM_BAD (1ULL << 5)  /**< External IP header checksum error. */
-#define PKT_RX_OVERSIZE      (0ULL << 0)  /**< Num of desc of an RX pkt oversize. */
-#define PKT_RX_HBUF_OVERFLOW (0ULL << 0)  /**< Header buffer overflow. */
-#define PKT_RX_RECIP_ERR     (0ULL << 0)  /**< Hardware processing error. */
-#define PKT_RX_MAC_ERR       (0ULL << 0)  /**< MAC error. */
+/* hole, some bits can be reused here  */
 #define PKT_RX_IEEE1588_PTP  (1ULL << 9)  /**< RX IEEE1588 L2 Ethernet PT Packet. */
 #define PKT_RX_IEEE1588_TMST (1ULL << 10) /**< RX IEEE1588 L2/L4 timestamped packet.*/
 #define PKT_RX_FDIR_ID       (1ULL << 13) /**< FD id reported if FDIR match. */
-- 
2.8.0.rc3

Re: [dpdk-dev] [PATCH] mbuf: remove unused Rx error flags

[Olivier Matz]

Hi,

On 05/23/2016 09:56 AM, Olivier Matz wrote:
> Following the discussions from:
> http://dpdk.org/ml/archives/dev/2015-July/021721.html
> http://dpdk.org/ml/archives/dev/2016-April/038143.html
> 
> The value of these flags is 0, making them useless. Today, no example
> application checks them on Rx, and only few drivers sets them and
> silently give wrong packets to the application, which should not happen.
> 
> This patch removes the unused flags from rte_mbuf and their use in the
> drivers. The i40e and fm10k are kept as they are today and should be
> fixed to drop bad packets. The enic driver is managed by its maintainer
> in another patch.
> 
> Fixes: c22265f6 ("mbuf: add new packet flags for i40e")
> Signed-off-by: Olivier Matz <olivier.matz@6wind.com>

Any comment from someone?

Re: [dpdk-dev] [PATCH] mbuf: remove unused Rx error flags

[Ananyev, Konstantin]

> -----Original Message-----
> From: Olivier Matz [mailto:olivier.matz@6wind.com]
> Sent: Monday, May 23, 2016 8:56 AM
> To: dev@dpdk.org
> Cc: johndale@cisco.com; Ananyev, Konstantin; Zhang, Helin; arnon@qwilt.com; rolette@infinite.io; Chen, Jing D; Wu, Jingjing
> Subject: [PATCH] mbuf: remove unused Rx error flags
> 
> Following the discussions from:
> http://dpdk.org/ml/archives/dev/2015-July/021721.html
> http://dpdk.org/ml/archives/dev/2016-April/038143.html
> 
> The value of these flags is 0, making them useless. Today, no example
> application checks them on Rx, and only few drivers sets them and
> silently give wrong packets to the application, which should not happen.
> 
> This patch removes the unused flags from rte_mbuf and their use in the
> drivers. The i40e and fm10k are kept as they are today and should be
> fixed to drop bad packets. The enic driver is managed by its maintainer
> in another patch.
> 
> Fixes: c22265f6 ("mbuf: add new packet flags for i40e")
> Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
> ---

Acked-by: Konstantin Ananyev <konstantin.ananyev@intel.com>

Re: [dpdk-dev] [PATCH] mbuf: remove unused Rx error flags

[Bruce Richardson]

On Mon, Jun 13, 2016 at 12:39:33PM +0000, Ananyev, Konstantin wrote:
> 
> 
> > -----Original Message-----
> > From: Olivier Matz [mailto:olivier.matz@6wind.com]
> > Sent: Monday, May 23, 2016 8:56 AM
> > To: dev@dpdk.org
> > Cc: johndale@cisco.com; Ananyev, Konstantin; Zhang, Helin; arnon@qwilt.com; rolette@infinite.io; Chen, Jing D; Wu, Jingjing
> > Subject: [PATCH] mbuf: remove unused Rx error flags
> > 
> > Following the discussions from:
> > http://dpdk.org/ml/archives/dev/2015-July/021721.html
> > http://dpdk.org/ml/archives/dev/2016-April/038143.html
> > 
> > The value of these flags is 0, making them useless. Today, no example
> > application checks them on Rx, and only few drivers sets them and
> > silently give wrong packets to the application, which should not happen.
> > 
> > This patch removes the unused flags from rte_mbuf and their use in the
> > drivers. The i40e and fm10k are kept as they are today and should be
> > fixed to drop bad packets. The enic driver is managed by its maintainer
> > in another patch.
> > 
> > Fixes: c22265f6 ("mbuf: add new packet flags for i40e")
> > Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
> > ---
> 
> Acked-by: Konstantin Ananyev <konstantin.ananyev@intel.com>
> 
Applied to dpdk-next-net/rel_16_07

/Bruce

Re: [dpdk-dev] [PATCH] mbuf: remove unused Rx error flags

[Ananyev, Konstantin]

Hi Olivier,

> > -----Original Message-----
> > From: Olivier Matz [mailto:olivier.matz@6wind.com]
> > Sent: Monday, May 23, 2016 8:56 AM
> > To: dev@dpdk.org
> > Cc: johndale@cisco.com; Ananyev, Konstantin; Zhang, Helin; arnon@qwilt.com; rolette@infinite.io; Chen, Jing D; Wu, Jingjing
> > Subject: [PATCH] mbuf: remove unused Rx error flags
> >
> > Following the discussions from:
> > http://dpdk.org/ml/archives/dev/2015-July/021721.html
> > http://dpdk.org/ml/archives/dev/2016-April/038143.html
> >
> > The value of these flags is 0, making them useless. Today, no example
> > application checks them on Rx, and only few drivers sets them and
> > silently give wrong packets to the application, which should not happen.
> >
> > This patch removes the unused flags from rte_mbuf and their use in the
> > drivers. The i40e and fm10k are kept as they are today and should be
> > fixed to drop bad packets. The enic driver is managed by its maintainer
> > in another patch.
> >
> > Fixes: c22265f6 ("mbuf: add new packet flags for i40e")
> > Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
> > ---
> 
> Acked-by: Konstantin Ananyev <konstantin.ananyev@intel.com>


Just a note, I think you'll need to rebase your patch with latest code.
enic PMD fails to compile.
Please feel free to keep my ack on it.
Konstantin

Re: [dpdk-dev] [PATCH] mbuf: remove unused Rx error flags

[Olivier Matz]

Hi Konstantin,

On 06/13/2016 02:42 PM, Ananyev, Konstantin wrote:
> 
> Hi Olivier,
> 
>>> -----Original Message-----
>>> From: Olivier Matz [mailto:olivier.matz@6wind.com]
>>> Sent: Monday, May 23, 2016 8:56 AM
>>> To: dev@dpdk.org
>>> Cc: johndale@cisco.com; Ananyev, Konstantin; Zhang, Helin; arnon@qwilt.com; rolette@infinite.io; Chen, Jing D; Wu, Jingjing
>>> Subject: [PATCH] mbuf: remove unused Rx error flags
>>>
>>> Following the discussions from:
>>> http://dpdk.org/ml/archives/dev/2015-July/021721.html
>>> http://dpdk.org/ml/archives/dev/2016-April/038143.html
>>>
>>> The value of these flags is 0, making them useless. Today, no example
>>> application checks them on Rx, and only few drivers sets them and
>>> silently give wrong packets to the application, which should not happen.
>>>
>>> This patch removes the unused flags from rte_mbuf and their use in the
>>> drivers. The i40e and fm10k are kept as they are today and should be
>>> fixed to drop bad packets. The enic driver is managed by its maintainer
>>> in another patch.
>>>
>>> Fixes: c22265f6 ("mbuf: add new packet flags for i40e")
>>> Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
>>> ---
>>
>> Acked-by: Konstantin Ananyev <konstantin.ananyev@intel.com>
> 
> 
> Just a note, I think you'll need to rebase your patch with latest code.
> enic PMD fails to compile.

Indeed, this patch should be applied after John's enic series. Latest
one is there:
http://dpdk.org/ml/archives/dev/2016-June/040183.html

Please Bruce/Thomas, check this dependency before applying.

> Please feel free to keep my ack on it.

Thank you for reviewing.

Regards,
Olivier

Re: [dpdk-dev] [PATCH] mbuf: remove unused Rx error flags

[Bruce Richardson]

On Mon, Jun 13, 2016 at 02:49:41PM +0200, Olivier Matz wrote:
> Hi Konstantin,
> 
> On 06/13/2016 02:42 PM, Ananyev, Konstantin wrote:
> > 
> > Hi Olivier,
> > 
> >>> -----Original Message-----
> >>> From: Olivier Matz [mailto:olivier.matz@6wind.com]
> >>> Sent: Monday, May 23, 2016 8:56 AM
> >>> To: dev@dpdk.org
> >>> Cc: johndale@cisco.com; Ananyev, Konstantin; Zhang, Helin; arnon@qwilt.com; rolette@infinite.io; Chen, Jing D; Wu, Jingjing
> >>> Subject: [PATCH] mbuf: remove unused Rx error flags
> >>>
> >>> Following the discussions from:
> >>> http://dpdk.org/ml/archives/dev/2015-July/021721.html
> >>> http://dpdk.org/ml/archives/dev/2016-April/038143.html
> >>>
> >>> The value of these flags is 0, making them useless. Today, no example
> >>> application checks them on Rx, and only few drivers sets them and
> >>> silently give wrong packets to the application, which should not happen.
> >>>
> >>> This patch removes the unused flags from rte_mbuf and their use in the
> >>> drivers. The i40e and fm10k are kept as they are today and should be
> >>> fixed to drop bad packets. The enic driver is managed by its maintainer
> >>> in another patch.
> >>>
> >>> Fixes: c22265f6 ("mbuf: add new packet flags for i40e")
> >>> Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
> >>> ---
> >>
> >> Acked-by: Konstantin Ananyev <konstantin.ananyev@intel.com>
> > 
> > 
> > Just a note, I think you'll need to rebase your patch with latest code.
> > enic PMD fails to compile.
> 
> Indeed, this patch should be applied after John's enic series. Latest
> one is there:
> http://dpdk.org/ml/archives/dev/2016-June/040183.html
> 
> Please Bruce/Thomas, check this dependency before applying.
> 
That patchset is already applied to next-net so this patch applies cleanly and
compiles ok there.
Thomas, you ok for me just to apply this patch to next-net also?

/Bruce

Re: [dpdk-dev] [PATCH] mbuf: remove unused Rx error flags

[Thomas Monjalon]

> > > Just a note, I think you'll need to rebase your patch with latest code.
> > > enic PMD fails to compile.
> > 
> > Indeed, this patch should be applied after John's enic series. Latest
> > one is there:
> > http://dpdk.org/ml/archives/dev/2016-June/040183.html
> > 
> > Please Bruce/Thomas, check this dependency before applying.
> > 
> That patchset is already applied to next-net so this patch applies cleanly and
> compiles ok there.
> Thomas, you ok for me just to apply this patch to next-net also?

No problem