[dpdk-dev] mbuf: remove unused Rx error flags
Commit Message
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(-)
Comments
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?
> -----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>
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
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
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
> > > 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
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
@@ -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
@@ -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,
@@ -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;
}
@@ -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;
@@ -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. */