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

[Olivier MATZ <olivier.matz@6wind.com>]

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