Message ID | 1415984609-2484-7-git-send-email-olivier.matz@6wind.com (mailing list archive) |
---|---|
State | Superseded, archived |
Headers |
Return-Path: <dev-bounces@dpdk.org> X-Original-To: patchwork@dpdk.org Delivered-To: patchwork@dpdk.org Received: from [92.243.14.124] (localhost [IPv6:::1]) by dpdk.org (Postfix) with ESMTP id 562837FCA; Fri, 14 Nov 2014 17:54:07 +0100 (CET) Received: from mail-wi0-f181.google.com (mail-wi0-f181.google.com [209.85.212.181]) by dpdk.org (Postfix) with ESMTP id CDA497F89 for <dev@dpdk.org>; Fri, 14 Nov 2014 17:53:51 +0100 (CET) Received: by mail-wi0-f181.google.com with SMTP id n3so3306225wiv.14 for <dev@dpdk.org>; Fri, 14 Nov 2014 09:03:55 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references; bh=2yxNXxt6V3TwHun3IpLdpCbw/BX0ohzFaJxkGq4L60k=; b=WC96+wPofoRUIlLVtsapkKMR3NKLDsUHf9sbEz4Aa7LDIzxlExNXVmOX7C4kw9Qmrn e1PiIZCSgtSXq7OkhhLr1EqBIPiq8n4wiF+utTY0zWz8LM0GJtuevO0EtW92TtRNNK9B CsbjCajV+dEbKuh5yaJMci0N61ac25LOeaYH2NtznNd1Q7l+ynNVK3N3YAHt0EVHLRMr /SP28dnNve4IaI0QBFQSU5LKPPcenbxcT9jgn9kOXIrb+tXs78cBvrFh1Am7fk85Qgd9 TtRjQ2530l/SEAiyOrB+RLXoz8nXat+fKvxLmkU2NpUdbAe6Di3LMwIrtC4KHHpeRLmv y23w== X-Gm-Message-State: ALoCoQn8VbIJosQmgESbsGnt8V4c7BfvqGqCxWAhgHZz6pz08obiG1h8x7nXni9/i3mr9VjbTOeU X-Received: by 10.195.13.14 with SMTP id eu14mr15172628wjd.31.1415984635704; Fri, 14 Nov 2014 09:03:55 -0800 (PST) Received: from glumotte.dev.6wind.com (guy78-3-82-239-227-177.fbx.proxad.net. [82.239.227.177]) by mx.google.com with ESMTPSA id cu9sm40352554wjb.0.2014.11.14.09.03.54 for <multiple recipients> (version=TLSv1.2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Fri, 14 Nov 2014 09:03:55 -0800 (PST) From: Olivier Matz <olivier.matz@6wind.com> To: dev@dpdk.org Date: Fri, 14 Nov 2014 18:03:22 +0100 Message-Id: <1415984609-2484-7-git-send-email-olivier.matz@6wind.com> X-Mailer: git-send-email 2.1.0 In-Reply-To: <1415984609-2484-1-git-send-email-olivier.matz@6wind.com> References: <1415635166-1364-1-git-send-email-olivier.matz@6wind.com> <1415984609-2484-1-git-send-email-olivier.matz@6wind.com> Cc: jigsaw@gmail.com Subject: [dpdk-dev] [PATCH v2 06/13] mbuf: add functions to get the name of an ol_flag X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK <dev.dpdk.org> List-Unsubscribe: <http://dpdk.org/ml/options/dev>, <mailto:dev-request@dpdk.org?subject=unsubscribe> List-Archive: <http://dpdk.org/ml/archives/dev/> List-Post: <mailto:dev@dpdk.org> List-Help: <mailto:dev-request@dpdk.org?subject=help> List-Subscribe: <http://dpdk.org/ml/listinfo/dev>, <mailto:dev-request@dpdk.org?subject=subscribe> Errors-To: dev-bounces@dpdk.org Sender: "dev" <dev-bounces@dpdk.org> |
Commit Message
Olivier Matz
Nov. 14, 2014, 5:03 p.m. UTC
In test-pmd (rxonly.c), the code is able to dump the list of ol_flags.
The issue is that the list of flags in the application has to be
synchronized with the flags defined in rte_mbuf.h.
This patch introduces 2 new functions rte_get_rx_ol_flag_name()
and rte_get_tx_ol_flag_name() that returns the name of a flag from
its mask. It also fixes rxonly.c to use this new functions and to
display the proper flags.
Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
---
app/test-pmd/rxonly.c | 36 ++++++++++--------------------------
lib/librte_mbuf/rte_mbuf.c | 45 +++++++++++++++++++++++++++++++++++++++++++++
lib/librte_mbuf/rte_mbuf.h | 22 ++++++++++++++++++++++
3 files changed, 77 insertions(+), 26 deletions(-)
Comments
On Fri, Nov 14, 2014 at 06:03:22PM +0100, Olivier Matz wrote: > In test-pmd (rxonly.c), the code is able to dump the list of ol_flags. > The issue is that the list of flags in the application has to be > synchronized with the flags defined in rte_mbuf.h. > > This patch introduces 2 new functions rte_get_rx_ol_flag_name() > and rte_get_tx_ol_flag_name() that returns the name of a flag from > its mask. It also fixes rxonly.c to use this new functions and to > display the proper flags. > > Signed-off-by: Olivier Matz <olivier.matz@6wind.com> > --- > app/test-pmd/rxonly.c | 36 ++++++++++-------------------------- > lib/librte_mbuf/rte_mbuf.c | 45 +++++++++++++++++++++++++++++++++++++++++++++ > lib/librte_mbuf/rte_mbuf.h | 22 ++++++++++++++++++++++ > 3 files changed, 77 insertions(+), 26 deletions(-) > > diff --git a/app/test-pmd/rxonly.c b/app/test-pmd/rxonly.c > index 9ad1df6..51a530a 100644 > --- a/app/test-pmd/rxonly.c > +++ b/app/test-pmd/rxonly.c > @@ -71,26 +71,6 @@ > > #include "testpmd.h" > > -#define MAX_PKT_RX_FLAGS 13 > -static const char *pkt_rx_flag_names[MAX_PKT_RX_FLAGS] = { > - "VLAN_PKT", > - "RSS_HASH", > - "PKT_RX_FDIR", > - "IP_CKSUM", > - "IP_CKSUM_BAD", > - > - "IPV4_HDR", > - "IPV4_HDR_EXT", > - "IPV6_HDR", > - "IPV6_HDR_EXT", > - > - "IEEE1588_PTP", > - "IEEE1588_TMST", > - > - "TUNNEL_IPV4_HDR", > - "TUNNEL_IPV6_HDR", > -}; > - > static inline void > print_ether_addr(const char *what, struct ether_addr *eth_addr) > { > @@ -214,12 +194,16 @@ pkt_burst_receive(struct fwd_stream *fs) > printf(" - Receive queue=0x%x", (unsigned) fs->rx_queue); > printf("\n"); > if (ol_flags != 0) { > - int rxf; > - > - for (rxf = 0; rxf < MAX_PKT_RX_FLAGS; rxf++) { > - if (ol_flags & (1 << rxf)) > - printf(" PKT_RX_%s\n", > - pkt_rx_flag_names[rxf]); > + unsigned rxf; > + const char *name; > + > + for (rxf = 0; rxf < sizeof(mb->ol_flags) * 8; rxf++) { > + if ((ol_flags & (1ULL << rxf)) == 0) > + continue; > + name = rte_get_rx_ol_flag_name(1ULL << rxf); > + if (name == NULL) > + continue; > + printf(" %s\n", name); > } > } > rte_pktmbuf_free(mb); > diff --git a/lib/librte_mbuf/rte_mbuf.c b/lib/librte_mbuf/rte_mbuf.c > index 52e7574..5cd9137 100644 > --- a/lib/librte_mbuf/rte_mbuf.c > +++ b/lib/librte_mbuf/rte_mbuf.c > @@ -196,3 +196,48 @@ rte_pktmbuf_dump(FILE *f, const struct rte_mbuf *m, unsigned dump_len) > nb_segs --; > } > } > + > +/* > + * Get the name of a RX offload flag > + */ > +const char *rte_get_rx_ol_flag_name(uint64_t mask) > +{ > + switch (mask) { > + case PKT_RX_VLAN_PKT: return "PKT_RX_VLAN_PKT"; > + case PKT_RX_RSS_HASH: return "PKT_RX_RSS_HASH"; > + case PKT_RX_FDIR: return "PKT_RX_FDIR"; > + 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_IPV4_HDR: return "PKT_RX_IPV4_HDR"; > + case PKT_RX_IPV4_HDR_EXT: return "PKT_RX_IPV4_HDR_EXT"; > + case PKT_RX_IPV6_HDR: return "PKT_RX_IPV6_HDR"; > + case PKT_RX_IPV6_HDR_EXT: return "PKT_RX_IPV6_HDR_EXT"; > + case PKT_RX_IEEE1588_PTP: return "PKT_RX_IEEE1588_PTP"; > + case PKT_RX_IEEE1588_TMST: return "PKT_RX_IEEE1588_TMST"; > + case PKT_RX_TUNNEL_IPV4_HDR: return "PKT_RX_TUNNEL_IPV4_HDR"; > + case PKT_RX_TUNNEL_IPV6_HDR: return "PKT_RX_TUNNEL_IPV6_HDR"; > + default: return NULL; > + } > +} > + > +/* > + * Get the name of a TX offload flag > + */ > +const char *rte_get_tx_ol_flag_name(uint64_t mask) > +{ > + switch (mask) { > + case PKT_TX_VLAN_PKT: return "PKT_TX_VLAN_PKT"; > + case PKT_TX_IP_CKSUM: return "PKT_TX_IP_CKSUM"; > + case PKT_TX_TCP_CKSUM: return "PKT_TX_TCP_CKSUM"; > + case PKT_TX_SCTP_CKSUM: return "PKT_TX_SCTP_CKSUM"; > + case PKT_TX_UDP_CKSUM: return "PKT_TX_UDP_CKSUM"; > + case PKT_TX_IEEE1588_TMST: return "PKT_TX_IEEE1588_TMST"; > + case PKT_TX_VXLAN_CKSUM: return "PKT_TX_VXLAN_CKSUM"; > + default: return NULL; > + } > +} > diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h > index 68fb988..e76617f 100644 > --- a/lib/librte_mbuf/rte_mbuf.h > +++ b/lib/librte_mbuf/rte_mbuf.h > @@ -129,6 +129,28 @@ extern "C" { > /* Use final bit of flags to indicate a control mbuf */ > #define CTRL_MBUF_FLAG (1ULL << 63) /**< Mbuf contains control data */ > I think this patch should perhaps also add to a comment at the top of the list of flags that any new flags added should also be added to the appropriate function in rte_mbuf.c. Having the comment in rte_mbuf.h where people would add the flags should help remind people to keep the flag lists in sync. /Bruce > +/** > + * Get the name of a RX offload flag > + * > + * @param mask > + * The mask describing the flag. > + * @return > + * The name of this flag, or NULL if it's not a valid RX flag. > + */ > +const char *rte_get_rx_ol_flag_name(uint64_t mask); > + > +/** > + * Get the name of a TX offload flag > + * > + * @param mask > + * The mask describing the flag. Usually only one bit must be set. > + * Several bits can be given if they belong to the same mask. > + * Ex: PKT_TX_L4_MASK. > + * @return > + * The name of this flag, or NULL if it's not a valid TX flag. > + */ > +const char *rte_get_tx_ol_flag_name(uint64_t mask); > + > /* define a set of marker types that can be used to refer to set points in the > * mbuf */ > typedef void *MARKER[0]; /**< generic marker for a point in a structure */ > -- > 2.1.0 >
Hi Bruce, On 11/17/2014 11:39 AM, Bruce Richardson wrote: >> +/* >> + * Get the name of a TX offload flag >> + */ >> +const char *rte_get_tx_ol_flag_name(uint64_t mask) >> +{ >> + switch (mask) { >> + case PKT_TX_VLAN_PKT: return "PKT_TX_VLAN_PKT"; >> + case PKT_TX_IP_CKSUM: return "PKT_TX_IP_CKSUM"; >> + case PKT_TX_TCP_CKSUM: return "PKT_TX_TCP_CKSUM"; >> + case PKT_TX_SCTP_CKSUM: return "PKT_TX_SCTP_CKSUM"; >> + case PKT_TX_UDP_CKSUM: return "PKT_TX_UDP_CKSUM"; >> + case PKT_TX_IEEE1588_TMST: return "PKT_TX_IEEE1588_TMST"; >> + case PKT_TX_VXLAN_CKSUM: return "PKT_TX_VXLAN_CKSUM"; >> + default: return NULL; >> + } >> +} >> diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h >> index 68fb988..e76617f 100644 >> --- a/lib/librte_mbuf/rte_mbuf.h >> +++ b/lib/librte_mbuf/rte_mbuf.h >> @@ -129,6 +129,28 @@ extern "C" { >> /* Use final bit of flags to indicate a control mbuf */ >> #define CTRL_MBUF_FLAG (1ULL << 63) /**< Mbuf contains control data */ >> > > I think this patch should perhaps also add to a comment at the top of the list > of flags that any new flags added should also be added to the appropriate > function in rte_mbuf.c. Having the comment in rte_mbuf.h where people would add the flags > should help remind people to keep the flag lists in sync. Good idea, I'll add it. Regards, Olivier
Hi Oliver, > -----Original Message----- > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Olivier Matz > Sent: Friday, November 14, 2014 5:03 PM > To: dev@dpdk.org > Cc: jigsaw@gmail.com > Subject: [dpdk-dev] [PATCH v2 06/13] mbuf: add functions to get the name of an ol_flag > > In test-pmd (rxonly.c), the code is able to dump the list of ol_flags. > The issue is that the list of flags in the application has to be > synchronized with the flags defined in rte_mbuf.h. > > This patch introduces 2 new functions rte_get_rx_ol_flag_name() > and rte_get_tx_ol_flag_name() that returns the name of a flag from > its mask. It also fixes rxonly.c to use this new functions and to > display the proper flags. > > Signed-off-by: Olivier Matz <olivier.matz@6wind.com> > --- > app/test-pmd/rxonly.c | 36 ++++++++++-------------------------- > lib/librte_mbuf/rte_mbuf.c | 45 +++++++++++++++++++++++++++++++++++++++++++++ > lib/librte_mbuf/rte_mbuf.h | 22 ++++++++++++++++++++++ > 3 files changed, 77 insertions(+), 26 deletions(-) > > diff --git a/app/test-pmd/rxonly.c b/app/test-pmd/rxonly.c > index 9ad1df6..51a530a 100644 > --- a/app/test-pmd/rxonly.c > +++ b/app/test-pmd/rxonly.c > @@ -71,26 +71,6 @@ > > #include "testpmd.h" > > -#define MAX_PKT_RX_FLAGS 13 > -static const char *pkt_rx_flag_names[MAX_PKT_RX_FLAGS] = { > - "VLAN_PKT", > - "RSS_HASH", > - "PKT_RX_FDIR", > - "IP_CKSUM", > - "IP_CKSUM_BAD", > - > - "IPV4_HDR", > - "IPV4_HDR_EXT", > - "IPV6_HDR", > - "IPV6_HDR_EXT", > - > - "IEEE1588_PTP", > - "IEEE1588_TMST", > - > - "TUNNEL_IPV4_HDR", > - "TUNNEL_IPV6_HDR", > -}; > - > static inline void > print_ether_addr(const char *what, struct ether_addr *eth_addr) > { > @@ -214,12 +194,16 @@ pkt_burst_receive(struct fwd_stream *fs) > printf(" - Receive queue=0x%x", (unsigned) fs->rx_queue); > printf("\n"); > if (ol_flags != 0) { > - int rxf; > - > - for (rxf = 0; rxf < MAX_PKT_RX_FLAGS; rxf++) { > - if (ol_flags & (1 << rxf)) > - printf(" PKT_RX_%s\n", > - pkt_rx_flag_names[rxf]); > + unsigned rxf; > + const char *name; > + > + for (rxf = 0; rxf < sizeof(mb->ol_flags) * 8; rxf++) { > + if ((ol_flags & (1ULL << rxf)) == 0) > + continue; > + name = rte_get_rx_ol_flag_name(1ULL << rxf); > + if (name == NULL) > + continue; > + printf(" %s\n", name); > } > } > rte_pktmbuf_free(mb); > diff --git a/lib/librte_mbuf/rte_mbuf.c b/lib/librte_mbuf/rte_mbuf.c > index 52e7574..5cd9137 100644 > --- a/lib/librte_mbuf/rte_mbuf.c > +++ b/lib/librte_mbuf/rte_mbuf.c > @@ -196,3 +196,48 @@ rte_pktmbuf_dump(FILE *f, const struct rte_mbuf *m, unsigned dump_len) > nb_segs --; > } > } > + > +/* > + * Get the name of a RX offload flag > + */ > +const char *rte_get_rx_ol_flag_name(uint64_t mask) > +{ > + switch (mask) { > + case PKT_RX_VLAN_PKT: return "PKT_RX_VLAN_PKT"; > + case PKT_RX_RSS_HASH: return "PKT_RX_RSS_HASH"; > + case PKT_RX_FDIR: return "PKT_RX_FDIR"; > + 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"; */ Didn't spot it before, wonder why do you need these 5 commented out lines? In fact, why do we need these flags if they all equal to zero right now? I know these flags were not introduced by that patch, in fact as I can see it was a temporary measure, as old ol_flags were just 16 bits long: http://dpdk.org/ml/archives/dev/2014-June/003308.html So wonder should now these flags either get proper values or be removed? Konstantin > + case PKT_RX_IPV4_HDR: return "PKT_RX_IPV4_HDR"; > + case PKT_RX_IPV4_HDR_EXT: return "PKT_RX_IPV4_HDR_EXT"; > + case PKT_RX_IPV6_HDR: return "PKT_RX_IPV6_HDR"; > + case PKT_RX_IPV6_HDR_EXT: return "PKT_RX_IPV6_HDR_EXT"; > + case PKT_RX_IEEE1588_PTP: return "PKT_RX_IEEE1588_PTP"; > + case PKT_RX_IEEE1588_TMST: return "PKT_RX_IEEE1588_TMST"; > + case PKT_RX_TUNNEL_IPV4_HDR: return "PKT_RX_TUNNEL_IPV4_HDR"; > + case PKT_RX_TUNNEL_IPV6_HDR: return "PKT_RX_TUNNEL_IPV6_HDR"; > + default: return NULL; > + } > +} > + > +/* > + * Get the name of a TX offload flag > + */ > +const char *rte_get_tx_ol_flag_name(uint64_t mask) > +{ > + switch (mask) { > + case PKT_TX_VLAN_PKT: return "PKT_TX_VLAN_PKT"; > + case PKT_TX_IP_CKSUM: return "PKT_TX_IP_CKSUM"; > + case PKT_TX_TCP_CKSUM: return "PKT_TX_TCP_CKSUM"; > + case PKT_TX_SCTP_CKSUM: return "PKT_TX_SCTP_CKSUM"; > + case PKT_TX_UDP_CKSUM: return "PKT_TX_UDP_CKSUM"; > + case PKT_TX_IEEE1588_TMST: return "PKT_TX_IEEE1588_TMST"; > + case PKT_TX_VXLAN_CKSUM: return "PKT_TX_VXLAN_CKSUM"; > + default: return NULL; > + } > +} > diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h > index 68fb988..e76617f 100644 > --- a/lib/librte_mbuf/rte_mbuf.h > +++ b/lib/librte_mbuf/rte_mbuf.h > @@ -129,6 +129,28 @@ extern "C" { > /* Use final bit of flags to indicate a control mbuf */ > #define CTRL_MBUF_FLAG (1ULL << 63) /**< Mbuf contains control data */ > > +/** > + * Get the name of a RX offload flag > + * > + * @param mask > + * The mask describing the flag. > + * @return > + * The name of this flag, or NULL if it's not a valid RX flag. > + */ > +const char *rte_get_rx_ol_flag_name(uint64_t mask); > + > +/** > + * Get the name of a TX offload flag > + * > + * @param mask > + * The mask describing the flag. Usually only one bit must be set. > + * Several bits can be given if they belong to the same mask. > + * Ex: PKT_TX_L4_MASK. > + * @return > + * The name of this flag, or NULL if it's not a valid TX flag. > + */ > +const char *rte_get_tx_ol_flag_name(uint64_t mask); > + > /* define a set of marker types that can be used to refer to set points in the > * mbuf */ > typedef void *MARKER[0]; /**< generic marker for a point in a structure */ > -- > 2.1.0
Hi Konstantin, On 11/17/2014 08:00 PM, Ananyev, Konstantin wrote: >> +/* >> + * Get the name of a RX offload flag >> + */ >> +const char *rte_get_rx_ol_flag_name(uint64_t mask) >> +{ >> + switch (mask) { >> + case PKT_RX_VLAN_PKT: return "PKT_RX_VLAN_PKT"; >> + case PKT_RX_RSS_HASH: return "PKT_RX_RSS_HASH"; >> + case PKT_RX_FDIR: return "PKT_RX_FDIR"; >> + 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"; */ > > Didn't spot it before, wonder why do you need these 5 commented out lines? > In fact, why do we need these flags if they all equal to zero right now? > I know these flags were not introduced by that patch, in fact as I can see it was a temporary measure, > as old ol_flags were just 16 bits long: > http://dpdk.org/ml/archives/dev/2014-June/003308.html > So wonder should now these flags either get proper values or be removed? I would be in favor of removing them, or at least the following ones (I don't understand how they can help the application): - 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. I would have say that a statistics counter in the driver is more appropriate for this case (maybe there is already a counter in the hardware). I have no i40e hardware to test that, so I don't feel very comfortable to modify the i40e driver code to add these stats. Adding Helin in CC list, maybe he has an idea. Regards, Olivier
> -----Original Message----- > From: Olivier MATZ [mailto:olivier.matz@6wind.com] > Sent: Tuesday, November 18, 2014 9:30 AM > To: Ananyev, Konstantin; dev@dpdk.org > Cc: jigsaw@gmail.com; Zhang, Helin > Subject: Re: [dpdk-dev] [PATCH v2 06/13] mbuf: add functions to get the name of an ol_flag > > Hi Konstantin, > > On 11/17/2014 08:00 PM, Ananyev, Konstantin wrote: > >> +/* > >> + * Get the name of a RX offload flag > >> + */ > >> +const char *rte_get_rx_ol_flag_name(uint64_t mask) > >> +{ > >> + switch (mask) { > >> + case PKT_RX_VLAN_PKT: return "PKT_RX_VLAN_PKT"; > >> + case PKT_RX_RSS_HASH: return "PKT_RX_RSS_HASH"; > >> + case PKT_RX_FDIR: return "PKT_RX_FDIR"; > >> + 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"; */ > > > > Didn't spot it before, wonder why do you need these 5 commented out lines? > > In fact, why do we need these flags if they all equal to zero right now? > > I know these flags were not introduced by that patch, in fact as I can see it was a temporary measure, > > as old ol_flags were just 16 bits long: > > http://dpdk.org/ml/archives/dev/2014-June/003308.html > > So wonder should now these flags either get proper values or be removed? > > I would be in favor of removing them, or at least the following ones > (I don't understand how they can help the application): > > - 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. Tend to agree... Or probably collapse these 4 flags into one: flag PKT_RX_ERR or something. Might be still used by someone for debugging purposes. Helin, what do you think? > > I would have say that a statistics counter in the driver is more > appropriate for this case (maybe there is already a counter in the > hardware). > > I have no i40e hardware to test that, so I don't feel very comfortable > to modify the i40e driver code to add these stats. > > Adding Helin in CC list, maybe he has an idea. > > Regards, > Olivier
Hi Helin, > -----Original Message----- > From: Ananyev, Konstantin > Sent: Wednesday, November 19, 2014 11:07 AM > To: Olivier MATZ; dev@dpdk.org > Cc: jigsaw@gmail.com; Zhang, Helin > Subject: RE: [dpdk-dev] [PATCH v2 06/13] mbuf: add functions to get the name of an ol_flag > > > > > -----Original Message----- > > From: Olivier MATZ [mailto:olivier.matz@6wind.com] > > Sent: Tuesday, November 18, 2014 9:30 AM > > To: Ananyev, Konstantin; dev@dpdk.org > > Cc: jigsaw@gmail.com; Zhang, Helin > > Subject: Re: [dpdk-dev] [PATCH v2 06/13] mbuf: add functions to get the name of an ol_flag > > > > Hi Konstantin, > > > > On 11/17/2014 08:00 PM, Ananyev, Konstantin wrote: > > >> +/* > > >> + * Get the name of a RX offload flag > > >> + */ > > >> +const char *rte_get_rx_ol_flag_name(uint64_t mask) > > >> +{ > > >> + switch (mask) { > > >> + case PKT_RX_VLAN_PKT: return "PKT_RX_VLAN_PKT"; > > >> + case PKT_RX_RSS_HASH: return "PKT_RX_RSS_HASH"; > > >> + case PKT_RX_FDIR: return "PKT_RX_FDIR"; > > >> + 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"; */ > > > > > > Didn't spot it before, wonder why do you need these 5 commented out lines? > > > In fact, why do we need these flags if they all equal to zero right now? > > > I know these flags were not introduced by that patch, in fact as I can see it was a temporary measure, > > > as old ol_flags were just 16 bits long: > > > http://dpdk.org/ml/archives/dev/2014-June/003308.html > > > So wonder should now these flags either get proper values or be removed? > > > > I would be in favor of removing them, or at least the following ones > > (I don't understand how they can help the application): > > > > - 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. > > Tend to agree... > Or probably collapse these 4 flags into one: flag PKT_RX_ERR or something. > Might be still used by someone for debugging purposes. > Helin, what do you think? As there is no answer, I suppose you don't care these flags any more. So we can just remove them, right? Konstantin > > > > > I would have say that a statistics counter in the driver is more > > appropriate for this case (maybe there is already a counter in the > > hardware). > > > > I have no i40e hardware to test that, so I don't feel very comfortable > > to modify the i40e driver code to add these stats. > > > > Adding Helin in CC list, maybe he has an idea. > > > > Regards, > > Olivier
HI Konstantin > -----Original Message----- > From: Ananyev, Konstantin > Sent: Tuesday, November 25, 2014 6:38 PM > To: 'Olivier MATZ'; 'dev@dpdk.org' > Cc: 'jigsaw@gmail.com'; Zhang, Helin > Subject: RE: [dpdk-dev] [PATCH v2 06/13] mbuf: add functions to get the name of > an ol_flag > > Hi Helin, > > > -----Original Message----- > > From: Ananyev, Konstantin > > Sent: Wednesday, November 19, 2014 11:07 AM > > To: Olivier MATZ; dev@dpdk.org > > Cc: jigsaw@gmail.com; Zhang, Helin > > Subject: RE: [dpdk-dev] [PATCH v2 06/13] mbuf: add functions to get > > the name of an ol_flag > > > > > > > > > -----Original Message----- > > > From: Olivier MATZ [mailto:olivier.matz@6wind.com] > > > Sent: Tuesday, November 18, 2014 9:30 AM > > > To: Ananyev, Konstantin; dev@dpdk.org > > > Cc: jigsaw@gmail.com; Zhang, Helin > > > Subject: Re: [dpdk-dev] [PATCH v2 06/13] mbuf: add functions to get > > > the name of an ol_flag > > > > > > Hi Konstantin, > > > > > > On 11/17/2014 08:00 PM, Ananyev, Konstantin wrote: > > > >> +/* > > > >> + * Get the name of a RX offload flag */ const char > > > >> +*rte_get_rx_ol_flag_name(uint64_t mask) { > > > >> + switch (mask) { > > > >> + case PKT_RX_VLAN_PKT: return "PKT_RX_VLAN_PKT"; > > > >> + case PKT_RX_RSS_HASH: return "PKT_RX_RSS_HASH"; > > > >> + case PKT_RX_FDIR: return "PKT_RX_FDIR"; > > > >> + 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"; */ > > > > > > > > Didn't spot it before, wonder why do you need these 5 commented out > lines? > > > > In fact, why do we need these flags if they all equal to zero right now? > > > > I know these flags were not introduced by that patch, in fact as I > > > > can see it was a temporary measure, as old ol_flags were just 16 bits long: > > > > http://dpdk.org/ml/archives/dev/2014-June/003308.html > > > > So wonder should now these flags either get proper values or be removed? > > > > > > I would be in favor of removing them, or at least the following ones > > > (I don't understand how they can help the application): > > > > > > - 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. > > > > Tend to agree... > > Or probably collapse these 4 flags into one: flag PKT_RX_ERR or something. > > Might be still used by someone for debugging purposes. > > Helin, what do you think? > > As there is no answer, I suppose you don't care these flags any more. > So we can just remove them, right? Sorry, I think I care it a bit. I have a lot of emails to be dealt with, due to the whole week training. Yes, it was added there before new mbuf defined. Why zero? Because of lack of bits for them. Unfortunately, I forgot to add them with correct values after new mbuf introduced. Thank you so much for spotting it out! The error flags were added according to the errors defined by FVL datasheet. It could be helpful for middle layer software or applications with the specific errors identified. I'd prefer to add the correct values for those flags. What do you think? Thanks and Regards, Helin > > Konstantin > > > > > > > > > I would have say that a statistics counter in the driver is more > > > appropriate for this case (maybe there is already a counter in the > > > hardware). > > > > > > I have no i40e hardware to test that, so I don't feel very > > > comfortable to modify the i40e driver code to add these stats. > > > > > > Adding Helin in CC list, maybe he has an idea. > > > > > > Regards, > > > Olivier
Hi Helin, On 11/25/2014 01:15 PM, Zhang, Helin wrote: >>>> I would be in favor of removing them, or at least the following ones >>>> (I don't understand how they can help the application): >>>> >>>> - 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. >>> >>> Tend to agree... >>> Or probably collapse these 4 flags into one: flag PKT_RX_ERR or something. >>> Might be still used by someone for debugging purposes. >>> Helin, what do you think? >> >> As there is no answer, I suppose you don't care these flags any more. >> So we can just remove them, right? > Sorry, I think I care it a bit. I have a lot of emails to be dealt with, due to the whole week training. > Yes, it was added there before new mbuf defined. Why zero? Because of lack of bits for them. > Unfortunately, I forgot to add them with correct values after new mbuf introduced. > Thank you so much for spotting it out! > > The error flags were added according to the errors defined by FVL datasheet. It could be > helpful for middle layer software or applications with the specific errors identified. I'd prefer > to add the correct values for those flags. What do you think? Could you elaborate about why it could be useful for an application to have this flag in the mbuf? When these flags are set, is the data still present in the mbuf? How can the application use this data if the hardware says "there is an error in the packet"? I think a stats counter would do the job here. Regards, Olivier
Hi Olivier > -----Original Message----- > From: Olivier MATZ [mailto:olivier.matz@6wind.com] > Sent: Tuesday, November 25, 2014 8:38 PM > To: Zhang, Helin; Ananyev, Konstantin; 'dev@dpdk.org' > Cc: 'jigsaw@gmail.com' > Subject: Re: [dpdk-dev] [PATCH v2 06/13] mbuf: add functions to get the name of > an ol_flag > > Hi Helin, > > On 11/25/2014 01:15 PM, Zhang, Helin wrote: > >>>> I would be in favor of removing them, or at least the following > >>>> ones (I don't understand how they can help the application): > >>>> > >>>> - 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. > >>> > >>> Tend to agree... > >>> Or probably collapse these 4 flags into one: flag PKT_RX_ERR or something. > >>> Might be still used by someone for debugging purposes. > >>> Helin, what do you think? > >> > >> As there is no answer, I suppose you don't care these flags any more. > >> So we can just remove them, right? > > Sorry, I think I care it a bit. I have a lot of emails to be dealt with, due to the > whole week training. > > Yes, it was added there before new mbuf defined. Why zero? Because of lack > of bits for them. > > Unfortunately, I forgot to add them with correct values after new mbuf > introduced. > > Thank you so much for spotting it out! > > > > The error flags were added according to the errors defined by FVL > > datasheet. It could be helpful for middle layer software or > > applications with the specific errors identified. I'd prefer to add the correct > values for those flags. What do you think? > > Could you elaborate about why it could be useful for an application to have this > flag in the mbuf? When these flags are set, is the data still present in the mbuf? > How can the application use this data if the hardware says "there is an error in > the packet"? That mbuf has already been filled with data, even error happens. The error flags can be used to indicate if the data is valid or not. Though it may not need too many error flags, but error flags with specific root causes could be helpful for users to know what happens. > > I think a stats counter would do the job here. It already supports statistics collection in i40e. > > Regards, > Olivier Regards, Helin
> -----Original Message----- > From: Zhang, Helin > Sent: Tuesday, November 25, 2014 12:15 PM > To: Ananyev, Konstantin; 'Olivier MATZ'; 'dev@dpdk.org' > Cc: 'jigsaw@gmail.com' > Subject: RE: [dpdk-dev] [PATCH v2 06/13] mbuf: add functions to get the name of an ol_flag > > HI Konstantin > > > -----Original Message----- > > From: Ananyev, Konstantin > > Sent: Tuesday, November 25, 2014 6:38 PM > > To: 'Olivier MATZ'; 'dev@dpdk.org' > > Cc: 'jigsaw@gmail.com'; Zhang, Helin > > Subject: RE: [dpdk-dev] [PATCH v2 06/13] mbuf: add functions to get the name of > > an ol_flag > > > > Hi Helin, > > > > > -----Original Message----- > > > From: Ananyev, Konstantin > > > Sent: Wednesday, November 19, 2014 11:07 AM > > > To: Olivier MATZ; dev@dpdk.org > > > Cc: jigsaw@gmail.com; Zhang, Helin > > > Subject: RE: [dpdk-dev] [PATCH v2 06/13] mbuf: add functions to get > > > the name of an ol_flag > > > > > > > > > > > > > -----Original Message----- > > > > From: Olivier MATZ [mailto:olivier.matz@6wind.com] > > > > Sent: Tuesday, November 18, 2014 9:30 AM > > > > To: Ananyev, Konstantin; dev@dpdk.org > > > > Cc: jigsaw@gmail.com; Zhang, Helin > > > > Subject: Re: [dpdk-dev] [PATCH v2 06/13] mbuf: add functions to get > > > > the name of an ol_flag > > > > > > > > Hi Konstantin, > > > > > > > > On 11/17/2014 08:00 PM, Ananyev, Konstantin wrote: > > > > >> +/* > > > > >> + * Get the name of a RX offload flag */ const char > > > > >> +*rte_get_rx_ol_flag_name(uint64_t mask) { > > > > >> + switch (mask) { > > > > >> + case PKT_RX_VLAN_PKT: return "PKT_RX_VLAN_PKT"; > > > > >> + case PKT_RX_RSS_HASH: return "PKT_RX_RSS_HASH"; > > > > >> + case PKT_RX_FDIR: return "PKT_RX_FDIR"; > > > > >> + 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"; */ > > > > > > > > > > Didn't spot it before, wonder why do you need these 5 commented out > > lines? > > > > > In fact, why do we need these flags if they all equal to zero right now? > > > > > I know these flags were not introduced by that patch, in fact as I > > > > > can see it was a temporary measure, as old ol_flags were just 16 bits long: > > > > > http://dpdk.org/ml/archives/dev/2014-June/003308.html > > > > > So wonder should now these flags either get proper values or be removed? > > > > > > > > I would be in favor of removing them, or at least the following ones > > > > (I don't understand how they can help the application): > > > > > > > > - 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. > > > > > > Tend to agree... > > > Or probably collapse these 4 flags into one: flag PKT_RX_ERR or something. > > > Might be still used by someone for debugging purposes. > > > Helin, what do you think? > > > > As there is no answer, I suppose you don't care these flags any more. > > So we can just remove them, right? > Sorry, I think I care it a bit. I have a lot of emails to be dealt with, due to the whole week training. > Yes, it was added there before new mbuf defined. Why zero? Because of lack of bits for them. > Unfortunately, I forgot to add them with correct values after new mbuf introduced. > Thank you so much for spotting it out! > > The error flags were added according to the errors defined by FVL datasheet. It could be > helpful for middle layer software or applications with the specific errors identified. I'd prefer > to add the correct values for those flags. What do you think? I am ok to have one flag for that something like PKT_RX_HW_ERR (or something). Don't really understand why you need all 4 of them - the packet contains invalid data anyway, so there is not much use of it. For debugging purposes you can just add a debug log for all of them. Something like: if (unlikely(error_bits & ...)) { flags |= PKT_RX_MAC_ERR; PMD_DRV_LOG(DEBUG, ...); return flags; } Konstantin > > Thanks and Regards, > Helin > > > > > Konstantin > > > > > > > > > > > > > I would have say that a statistics counter in the driver is more > > > > appropriate for this case (maybe there is already a counter in the > > > > hardware). > > > > > > > > I have no i40e hardware to test that, so I don't feel very > > > > comfortable to modify the i40e driver code to add these stats. > > > > > > > > Adding Helin in CC list, maybe he has an idea. > > > > > > > > Regards, > > > > Olivier
> -----Original Message----- > From: Ananyev, Konstantin > Sent: Tuesday, November 25, 2014 9:49 PM > To: Zhang, Helin; 'Olivier MATZ'; 'dev@dpdk.org' > Cc: 'jigsaw@gmail.com' > Subject: RE: [dpdk-dev] [PATCH v2 06/13] mbuf: add functions to get the name of > an ol_flag > > > > > -----Original Message----- > > From: Zhang, Helin > > Sent: Tuesday, November 25, 2014 12:15 PM > > To: Ananyev, Konstantin; 'Olivier MATZ'; 'dev@dpdk.org' > > Cc: 'jigsaw@gmail.com' > > Subject: RE: [dpdk-dev] [PATCH v2 06/13] mbuf: add functions to get > > the name of an ol_flag > > > > HI Konstantin > > > > > -----Original Message----- > > > From: Ananyev, Konstantin > > > Sent: Tuesday, November 25, 2014 6:38 PM > > > To: 'Olivier MATZ'; 'dev@dpdk.org' > > > Cc: 'jigsaw@gmail.com'; Zhang, Helin > > > Subject: RE: [dpdk-dev] [PATCH v2 06/13] mbuf: add functions to get > > > the name of an ol_flag > > > > > > Hi Helin, > > > > > > > -----Original Message----- > > > > From: Ananyev, Konstantin > > > > Sent: Wednesday, November 19, 2014 11:07 AM > > > > To: Olivier MATZ; dev@dpdk.org > > > > Cc: jigsaw@gmail.com; Zhang, Helin > > > > Subject: RE: [dpdk-dev] [PATCH v2 06/13] mbuf: add functions to > > > > get the name of an ol_flag > > > > > > > > > > > > > > > > > -----Original Message----- > > > > > From: Olivier MATZ [mailto:olivier.matz@6wind.com] > > > > > Sent: Tuesday, November 18, 2014 9:30 AM > > > > > To: Ananyev, Konstantin; dev@dpdk.org > > > > > Cc: jigsaw@gmail.com; Zhang, Helin > > > > > Subject: Re: [dpdk-dev] [PATCH v2 06/13] mbuf: add functions to > > > > > get the name of an ol_flag > > > > > > > > > > Hi Konstantin, > > > > > > > > > > On 11/17/2014 08:00 PM, Ananyev, Konstantin wrote: > > > > > >> +/* > > > > > >> + * Get the name of a RX offload flag */ const char > > > > > >> +*rte_get_rx_ol_flag_name(uint64_t mask) { > > > > > >> + switch (mask) { > > > > > >> + case PKT_RX_VLAN_PKT: return "PKT_RX_VLAN_PKT"; > > > > > >> + case PKT_RX_RSS_HASH: return "PKT_RX_RSS_HASH"; > > > > > >> + case PKT_RX_FDIR: return "PKT_RX_FDIR"; > > > > > >> + 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"; */ > > > > > > > > > > > > Didn't spot it before, wonder why do you need these 5 > > > > > > commented out > > > lines? > > > > > > In fact, why do we need these flags if they all equal to zero right now? > > > > > > I know these flags were not introduced by that patch, in fact > > > > > > as I can see it was a temporary measure, as old ol_flags were just 16 bits > long: > > > > > > http://dpdk.org/ml/archives/dev/2014-June/003308.html > > > > > > So wonder should now these flags either get proper values or be > removed? > > > > > > > > > > I would be in favor of removing them, or at least the following > > > > > ones (I don't understand how they can help the application): > > > > > > > > > > - 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. > > > > > > > > Tend to agree... > > > > Or probably collapse these 4 flags into one: flag PKT_RX_ERR or something. > > > > Might be still used by someone for debugging purposes. > > > > Helin, what do you think? > > > > > > As there is no answer, I suppose you don't care these flags any more. > > > So we can just remove them, right? > > Sorry, I think I care it a bit. I have a lot of emails to be dealt with, due to the > whole week training. > > Yes, it was added there before new mbuf defined. Why zero? Because of lack > of bits for them. > > Unfortunately, I forgot to add them with correct values after new mbuf > introduced. > > Thank you so much for spotting it out! > > > > The error flags were added according to the errors defined by FVL > > datasheet. It could be helpful for middle layer software or > > applications with the specific errors identified. I'd prefer to add the correct > values for those flags. What do you think? > > > I am ok to have one flag for that something like PKT_RX_HW_ERR (or something). > Don't really understand why you need all 4 of them - the packet contains invalid > data anyway, so there is not much use of it. Yes, I agree with you that one bit might be enough. It seems that we have more than one bits for errors previously. Regards, Helin > For debugging purposes you can just add a debug log for all of them. > Something like: > > if (unlikely(error_bits & ...)) { > flags |= PKT_RX_MAC_ERR; > PMD_DRV_LOG(DEBUG, ...); > return flags; > } > > Konstantin > > > > > Thanks and Regards, > > Helin > > > > > > > > Konstantin > > > > > > > > > > > > > > > > > I would have say that a statistics counter in the driver is more > > > > > appropriate for this case (maybe there is already a counter in > > > > > the hardware). > > > > > > > > > > I have no i40e hardware to test that, so I don't feel very > > > > > comfortable to modify the i40e driver code to add these stats. > > > > > > > > > > Adding Helin in CC list, maybe he has an idea. > > > > > > > > > > Regards, > > > > > Olivier
diff --git a/app/test-pmd/rxonly.c b/app/test-pmd/rxonly.c index 9ad1df6..51a530a 100644 --- a/app/test-pmd/rxonly.c +++ b/app/test-pmd/rxonly.c @@ -71,26 +71,6 @@ #include "testpmd.h" -#define MAX_PKT_RX_FLAGS 13 -static const char *pkt_rx_flag_names[MAX_PKT_RX_FLAGS] = { - "VLAN_PKT", - "RSS_HASH", - "PKT_RX_FDIR", - "IP_CKSUM", - "IP_CKSUM_BAD", - - "IPV4_HDR", - "IPV4_HDR_EXT", - "IPV6_HDR", - "IPV6_HDR_EXT", - - "IEEE1588_PTP", - "IEEE1588_TMST", - - "TUNNEL_IPV4_HDR", - "TUNNEL_IPV6_HDR", -}; - static inline void print_ether_addr(const char *what, struct ether_addr *eth_addr) { @@ -214,12 +194,16 @@ pkt_burst_receive(struct fwd_stream *fs) printf(" - Receive queue=0x%x", (unsigned) fs->rx_queue); printf("\n"); if (ol_flags != 0) { - int rxf; - - for (rxf = 0; rxf < MAX_PKT_RX_FLAGS; rxf++) { - if (ol_flags & (1 << rxf)) - printf(" PKT_RX_%s\n", - pkt_rx_flag_names[rxf]); + unsigned rxf; + const char *name; + + for (rxf = 0; rxf < sizeof(mb->ol_flags) * 8; rxf++) { + if ((ol_flags & (1ULL << rxf)) == 0) + continue; + name = rte_get_rx_ol_flag_name(1ULL << rxf); + if (name == NULL) + continue; + printf(" %s\n", name); } } rte_pktmbuf_free(mb); diff --git a/lib/librte_mbuf/rte_mbuf.c b/lib/librte_mbuf/rte_mbuf.c index 52e7574..5cd9137 100644 --- a/lib/librte_mbuf/rte_mbuf.c +++ b/lib/librte_mbuf/rte_mbuf.c @@ -196,3 +196,48 @@ rte_pktmbuf_dump(FILE *f, const struct rte_mbuf *m, unsigned dump_len) nb_segs --; } } + +/* + * Get the name of a RX offload flag + */ +const char *rte_get_rx_ol_flag_name(uint64_t mask) +{ + switch (mask) { + case PKT_RX_VLAN_PKT: return "PKT_RX_VLAN_PKT"; + case PKT_RX_RSS_HASH: return "PKT_RX_RSS_HASH"; + case PKT_RX_FDIR: return "PKT_RX_FDIR"; + 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_IPV4_HDR: return "PKT_RX_IPV4_HDR"; + case PKT_RX_IPV4_HDR_EXT: return "PKT_RX_IPV4_HDR_EXT"; + case PKT_RX_IPV6_HDR: return "PKT_RX_IPV6_HDR"; + case PKT_RX_IPV6_HDR_EXT: return "PKT_RX_IPV6_HDR_EXT"; + case PKT_RX_IEEE1588_PTP: return "PKT_RX_IEEE1588_PTP"; + case PKT_RX_IEEE1588_TMST: return "PKT_RX_IEEE1588_TMST"; + case PKT_RX_TUNNEL_IPV4_HDR: return "PKT_RX_TUNNEL_IPV4_HDR"; + case PKT_RX_TUNNEL_IPV6_HDR: return "PKT_RX_TUNNEL_IPV6_HDR"; + default: return NULL; + } +} + +/* + * Get the name of a TX offload flag + */ +const char *rte_get_tx_ol_flag_name(uint64_t mask) +{ + switch (mask) { + case PKT_TX_VLAN_PKT: return "PKT_TX_VLAN_PKT"; + case PKT_TX_IP_CKSUM: return "PKT_TX_IP_CKSUM"; + case PKT_TX_TCP_CKSUM: return "PKT_TX_TCP_CKSUM"; + case PKT_TX_SCTP_CKSUM: return "PKT_TX_SCTP_CKSUM"; + case PKT_TX_UDP_CKSUM: return "PKT_TX_UDP_CKSUM"; + case PKT_TX_IEEE1588_TMST: return "PKT_TX_IEEE1588_TMST"; + case PKT_TX_VXLAN_CKSUM: return "PKT_TX_VXLAN_CKSUM"; + default: return NULL; + } +} diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h index 68fb988..e76617f 100644 --- a/lib/librte_mbuf/rte_mbuf.h +++ b/lib/librte_mbuf/rte_mbuf.h @@ -129,6 +129,28 @@ extern "C" { /* Use final bit of flags to indicate a control mbuf */ #define CTRL_MBUF_FLAG (1ULL << 63) /**< Mbuf contains control data */ +/** + * Get the name of a RX offload flag + * + * @param mask + * The mask describing the flag. + * @return + * The name of this flag, or NULL if it's not a valid RX flag. + */ +const char *rte_get_rx_ol_flag_name(uint64_t mask); + +/** + * Get the name of a TX offload flag + * + * @param mask + * The mask describing the flag. Usually only one bit must be set. + * Several bits can be given if they belong to the same mask. + * Ex: PKT_TX_L4_MASK. + * @return + * The name of this flag, or NULL if it's not a valid TX flag. + */ +const char *rte_get_tx_ol_flag_name(uint64_t mask); + /* define a set of marker types that can be used to refer to set points in the * mbuf */ typedef void *MARKER[0]; /**< generic marker for a point in a structure */