[dpdk-dev] [PATCH v2 06/13] mbuf: add functions to get the name of an ol_flag

[Zhang, Helin]

HI Konstantin

> -----Original Message-----
> From: Ananyev, Konstantin
> Sent: Tuesday, November 25, 2014 6:38 PM
> To: 'Olivier MATZ'; 'dev at dpdk.org'
> Cc: 'jigsaw at 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 at dpdk.org
> > Cc: jigsaw at 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 at 6wind.com]
> > > Sent: Tuesday, November 18, 2014 9:30 AM
> > > To: Ananyev, Konstantin; dev at dpdk.org
> > > Cc: jigsaw at 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