DPDK patches and discussions
 help / color / mirror / Atom feed
From: "Zhang, Helin" <helin.zhang@intel.com>
To: "Ananyev, Konstantin" <konstantin.ananyev@intel.com>,
	'Olivier MATZ' <olivier.matz@6wind.com>,
	"'dev@dpdk.org'" <dev@dpdk.org>
Cc: "'jigsaw@gmail.com'" <jigsaw@gmail.com>
Subject: Re: [dpdk-dev] [PATCH v2 06/13] mbuf: add functions to get the name of	an ol_flag
Date: Wed, 26 Nov 2014 00:58:16 +0000	[thread overview]
Message-ID: <F35DEAC7BCE34641BA9FAC6BCA4A12E70A7CB47F@SHSMSX104.ccr.corp.intel.com> (raw)
In-Reply-To: <2601191342CEEE43887BDE71AB977258213B975F@IRSMSX105.ger.corp.intel.com>



> -----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

  reply	other threads:[~2014-11-26  0:48 UTC|newest]

Thread overview: 112+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-11-10 15:59 [dpdk-dev] [PATCH 00/12] add TSO support Olivier Matz
2014-11-10 15:59 ` [dpdk-dev] [PATCH 01/12] igb/ixgbe: fix IP checksum calculation Olivier Matz
2014-11-10 15:59 ` [dpdk-dev] [PATCH 02/12] ixgbe: fix remaining pkt_flags variable size to 64 bits Olivier Matz
2014-11-10 16:59   ` Bruce Richardson
2014-11-10 15:59 ` [dpdk-dev] [PATCH 03/12] mbuf: move vxlan_cksum flag definition at the proper place Olivier Matz
2014-11-10 17:09   ` Bruce Richardson
2014-11-10 15:59 ` [dpdk-dev] [PATCH 04/12] mbuf: add help about TX checksum flags Olivier Matz
2014-11-10 17:10   ` Bruce Richardson
2014-11-10 15:59 ` [dpdk-dev] [PATCH 05/12] mbuf: remove too specific PKT_TX_OFFLOAD_MASK definition Olivier Matz
2014-11-10 17:14   ` Bruce Richardson
2014-11-10 20:59     ` Olivier MATZ
2014-11-10 15:59 ` [dpdk-dev] [PATCH 06/12] mbuf: add functions to get the name of an ol_flag Olivier Matz
2014-11-10 17:29   ` Bruce Richardson
2014-11-10 20:54     ` Olivier MATZ
2014-11-12 17:21     ` Ananyev, Konstantin
2014-11-12 17:44       ` Olivier MATZ
2014-11-10 15:59 ` [dpdk-dev] [PATCH 07/12] mbuf: generic support for TCP segmentation offload Olivier Matz
2014-11-11  3:17   ` Liu, Jijiang
2014-11-12 13:09   ` Ananyev, Konstantin
2014-11-10 15:59 ` [dpdk-dev] [PATCH 08/12] ixgbe: support " Olivier Matz
2014-11-10 15:59 ` [dpdk-dev] [PATCH 09/12] testpmd: fix use of offload flags in testpmd Olivier Matz
2014-11-10 15:59 ` [dpdk-dev] [PATCH 10/12] testpmd: rework csum forward engine Olivier Matz
2014-11-11  8:35   ` Liu, Jijiang
2014-11-11  9:55     ` Olivier MATZ
2014-11-10 15:59 ` [dpdk-dev] [PATCH 11/12] testpmd: support TSO in " Olivier Matz
2014-11-10 15:59 ` [dpdk-dev] [PATCH 12/12] testpmd: add a verbose mode " Olivier Matz
2014-11-11  9:21 ` [dpdk-dev] [PATCH 00/12] add TSO support Olivier MATZ
2014-11-11  9:48   ` Olivier MATZ
2014-11-14 17:03 ` [dpdk-dev] [PATCH v2 00/13] " Olivier Matz
2014-11-14 17:03   ` [dpdk-dev] [PATCH v2 01/13] igb/ixgbe: fix IP checksum calculation Olivier Matz
2014-11-14 17:03   ` [dpdk-dev] [PATCH v2 02/13] ixgbe: fix remaining pkt_flags variable size to 64 bits Olivier Matz
2014-11-17 16:47     ` Walukiewicz, Miroslaw
2014-11-17 17:03       ` Olivier MATZ
2014-11-17 17:40         ` Thomas Monjalon
2014-11-14 17:03   ` [dpdk-dev] [PATCH v2 03/13] mbuf: move vxlan_cksum flag definition at the proper place Olivier Matz
2014-11-17 22:05     ` Thomas Monjalon
2014-11-18 14:10       ` Olivier MATZ
2014-11-14 17:03   ` [dpdk-dev] [PATCH v2 04/13] mbuf: add help about TX checksum flags Olivier Matz
2014-11-14 17:03   ` [dpdk-dev] [PATCH v2 05/13] mbuf: remove too specific PKT_TX_OFFLOAD_MASK definition Olivier Matz
2014-11-17 10:35     ` Bruce Richardson
2014-11-14 17:03   ` [dpdk-dev] [PATCH v2 06/13] mbuf: add functions to get the name of an ol_flag Olivier Matz
2014-11-17 10:39     ` Bruce Richardson
2014-11-17 12:51       ` Olivier MATZ
2014-11-17 19:00     ` Ananyev, Konstantin
2014-11-18  9:29       ` Olivier MATZ
2014-11-19 11:06         ` Ananyev, Konstantin
2014-11-25 10:37           ` Ananyev, Konstantin
2014-11-25 12:15             ` Zhang, Helin
2014-11-25 12:37               ` Olivier MATZ
2014-11-25 13:31                 ` Zhang, Helin
2014-11-25 13:49               ` Ananyev, Konstantin
2014-11-26  0:58                 ` Zhang, Helin [this message]
2014-11-14 17:03   ` [dpdk-dev] [PATCH v2 07/13] testpmd: fix use of offload flags in testpmd Olivier Matz
2014-11-14 17:03   ` [dpdk-dev] [PATCH v2 08/13] testpmd: rework csum forward engine Olivier Matz
2014-11-17  8:11     ` Liu, Jijiang
2014-11-17 13:00       ` Olivier MATZ
2014-11-14 17:03   ` [dpdk-dev] [PATCH v2 09/13] mbuf: introduce new checksum API Olivier Matz
2014-11-17 18:15     ` Ananyev, Konstantin
2014-11-18  9:10       ` Olivier MATZ
2014-11-14 17:03   ` [dpdk-dev] [PATCH v2 10/13] mbuf: generic support for TCP segmentation offload Olivier Matz
2014-11-17 23:33     ` Ananyev, Konstantin
2014-11-14 17:03   ` [dpdk-dev] [PATCH v2 11/13] ixgbe: support " Olivier Matz
2014-11-17 18:26     ` Ananyev, Konstantin
2014-11-18  9:11       ` Olivier MATZ
2014-11-14 17:03   ` [dpdk-dev] [PATCH v2 12/13] testpmd: support TSO in csum forward engine Olivier Matz
2014-11-14 17:03   ` [dpdk-dev] [PATCH v2 13/13] testpmd: add a verbose mode " Olivier Matz
2014-11-20 22:58   ` [dpdk-dev] [PATCH v3 00/13] add TSO support Olivier Matz
2014-11-20 22:58     ` [dpdk-dev] [PATCH v3 01/13] igb/ixgbe: fix IP checksum calculation Olivier Matz
2014-11-20 22:58     ` [dpdk-dev] [PATCH v3 02/13] ixgbe: fix remaining pkt_flags variable size to 64 bits Olivier Matz
2014-11-20 22:58     ` [dpdk-dev] [PATCH v3 03/13] mbuf: reorder tx ol_flags Olivier Matz
2014-11-25 10:22       ` Thomas Monjalon
2014-11-20 22:58     ` [dpdk-dev] [PATCH v3 04/13] mbuf: add help about TX checksum flags Olivier Matz
2014-11-20 22:58     ` [dpdk-dev] [PATCH v3 05/13] mbuf: remove too specific PKT_TX_OFFLOAD_MASK definition Olivier Matz
2014-11-20 22:58     ` [dpdk-dev] [PATCH v3 06/13] mbuf: add functions to get the name of an ol_flag Olivier Matz
2014-11-25 10:23       ` Thomas Monjalon
2014-11-20 22:58     ` [dpdk-dev] [PATCH v3 07/13] testpmd: fix use of offload flags in testpmd Olivier Matz
2014-11-25 11:52       ` Ananyev, Konstantin
2014-11-20 22:58     ` [dpdk-dev] [PATCH v3 08/13] testpmd: rework csum forward engine Olivier Matz
2014-11-26 10:10       ` Ananyev, Konstantin
2014-11-26 11:14         ` Olivier MATZ
2014-11-26 12:25           ` Ananyev, Konstantin
2014-11-26 14:55             ` Olivier MATZ
2014-11-26 16:34               ` Ananyev, Konstantin
2014-11-27  8:34                 ` Liu, Jijiang
2014-11-26 13:59           ` Liu, Jijiang
2014-11-20 22:58     ` [dpdk-dev] [PATCH v3 09/13] mbuf: introduce new checksum API Olivier Matz
2014-11-20 22:58     ` [dpdk-dev] [PATCH v3 10/13] mbuf: generic support for TCP segmentation offload Olivier Matz
2014-11-20 22:58     ` [dpdk-dev] [PATCH v3 11/13] ixgbe: support " Olivier Matz
2014-11-20 22:58     ` [dpdk-dev] [PATCH v3 12/13] testpmd: support TSO in csum forward engine Olivier Matz
2014-11-20 22:58     ` [dpdk-dev] [PATCH v3 13/13] testpmd: add a verbose mode " Olivier Matz
2014-11-26 15:04     ` [dpdk-dev] [PATCH v4 00/13] add TSO support Olivier Matz
2014-11-26 15:04       ` [dpdk-dev] [PATCH v4 01/13] igb/ixgbe: fix IP checksum calculation Olivier Matz
2014-11-26 15:04       ` [dpdk-dev] [PATCH v4 02/13] ixgbe: fix remaining pkt_flags variable size to 64 bits Olivier Matz
2014-11-26 15:04       ` [dpdk-dev] [PATCH v4 03/13] mbuf: reorder tx ol_flags Olivier Matz
2014-11-26 15:04       ` [dpdk-dev] [PATCH v4 04/13] mbuf: add help about TX checksum flags Olivier Matz
2014-11-26 15:04       ` [dpdk-dev] [PATCH v4 05/13] mbuf: remove too specific PKT_TX_OFFLOAD_MASK definition Olivier Matz
2014-11-26 15:04       ` [dpdk-dev] [PATCH v4 06/13] mbuf: add functions to get the name of an ol_flag Olivier Matz
2014-11-26 15:04       ` [dpdk-dev] [PATCH v4 07/13] testpmd: fix use of offload flags in testpmd Olivier Matz
2014-11-26 15:04       ` [dpdk-dev] [PATCH v4 08/13] testpmd: rework csum forward engine Olivier Matz
2014-11-26 20:02         ` Ananyev, Konstantin
2014-11-27  8:26           ` Liu, Jijiang
2014-11-27  9:10           ` Olivier MATZ
2014-11-27 11:02             ` Ananyev, Konstantin
2014-11-28  8:54               ` Liu, Jijiang
2014-11-28  9:54                 ` Olivier MATZ
2014-11-26 15:04       ` [dpdk-dev] [PATCH v4 09/13] mbuf: introduce new checksum API Olivier Matz
2014-11-26 15:04       ` [dpdk-dev] [PATCH v4 10/13] mbuf: generic support for TCP segmentation offload Olivier Matz
2014-11-26 15:04       ` [dpdk-dev] [PATCH v4 11/13] ixgbe: support " Olivier Matz
2014-11-26 15:04       ` [dpdk-dev] [PATCH v4 12/13] testpmd: support TSO in csum forward engine Olivier Matz
2014-11-26 21:23         ` Ananyev, Konstantin
2014-11-26 15:04       ` [dpdk-dev] [PATCH v4 13/13] testpmd: add a verbose mode " Olivier Matz
2014-11-26 18:30       ` [dpdk-dev] [PATCH v4 00/13] add TSO support Thomas Monjalon

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=F35DEAC7BCE34641BA9FAC6BCA4A12E70A7CB47F@SHSMSX104.ccr.corp.intel.com \
    --to=helin.zhang@intel.com \
    --cc=dev@dpdk.org \
    --cc=jigsaw@gmail.com \
    --cc=konstantin.ananyev@intel.com \
    --cc=olivier.matz@6wind.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).