Re: [dpdk-dev] [PATCH 03/13] mbuf: add packet_type field

["Zhang, Helin" <helin.zhang@intel.com>]

> -----Original Message-----
> From: Olivier MATZ [mailto:olivier.matz@6wind.com]
> Sent: Tuesday, September 9, 2014 4:03 PM
> To: Zhang, Helin; Yerden Zhumabekov; Richardson, Bruce; dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH 03/13] mbuf: add packet_type field
> 
> Hello,
> 
> On 09/09/2014 05:59 AM, Zhang, Helin wrote:
> > It is a common field which i40e PMD will use it to store the 'packet
> > type ID'. i40e hardware can recognize more than a hundred of packet
> > types of received packets, this is quite useful for upper layer stack
> > or application. So this field is quite useful and will be filled by PMD.
> > In ixgbe/igb, it has less than 10 packet types which are marked in
> > offload flags. From now on, it would be better to have new field here
> > to put the hardware offloaded packet type in and it could be used for future
> NICs.
> >
> >>
> >> I'm not saying this field is useless. But even if it's useful for
> >> some applications like yours, it does not mean that it should go in the
> generic mbuf structure.
> >>
> >> Also, for a new field, we should define who is in charge of filling it.
> >> Is is the driver? Does it mean that all drivers have to be modified
> >> to fill it? Or is it just a placeholder for applications? In this
> >> case, shouldn't we use application-specific metadata? In the other
> >> direction (TX), we would also need to define if this field must be
> >> filled by the application before transmitting a mbuf to a driver.
> > Yes, PMD will fill it. I40e PMD will be the first one, ixgbe/igb can
> > be kept as it is, or modified to be consistent. It is used for RX side
> > only, and for TX side, it can be investigated to see if it can be used
> > also. I think some new features in development can think of that.
> > Anyway, it is a quite useful field for i40e and future generation of NICs.
> 
> To me, having the support in a hardware for that feature is not a sufficient
> reason for adding this field. There are many hardware features that will never
> be integrated in dpdk.

At least this field is quite important for i40e.
e.g. packet type=43 means that hardware recognize it as a VXLAN packet. To avoid checking what type of packet by software, hardware can offload that, and fill the packet type ID in that field.
It cannot be put in ol_flags anymore, as it has more than 100 packet type can be recognized by hardware. Without it, vxlan feature cannot be implemented at all. 

> 
> This first version of the patch:
> 
> - just adds a field that is not used by any code, so it is useless.
>    At least testpmd or an application example should show how to
>    use it.

It will be used at least in vxlan feature which is in development. Without it, vxlan cannot be completed. So this is a very important field i40e and future NICs.

>
> - does not describe what enhancement is provided by adding the
>    field (performance? in this case, numbers + use case would help
>    to convince people).

I40e hardware can recognize received packets as different packet types, and there are about 256 packet types can be recognized by i40e hardware. It is not a enhancement, it is the key for at least i40e features.

> 
> - does not describe what can be the content of the field. Is it
>    a protocol number?
> 

The packet type is a offload feature of hardware, the value of it can mean one type of packet recognized by the i40e hardware. E.g.

| Packet type | Description            |
| 0         | Reserved              |
| 1         | MAC, PAY2            |
| 2         | MAC, TimeSync, PAY2    |
...
| 43        | MAC, IPV4, GRENAT, PAY3 |

> - does not explain if all drivers must fill this field. If yes,
>    the patch has to update all drivers. If not, something must be
>    done to mark the packet field as unknown by default.
> 
I40e needs it.
igb/ixgbe can be changed to support it, but not mandatory, as ol_flags can represent it.
Actually ixgbe and igb has packet type also, but the number of those types is less than 10, so it can be put in ol_flags. For i40e and future NICs, the number of that could be 256 or more, ol_flags does not have that many bits for it. The best idea is to fill the packet type ID directly into a field.

> Regards,
> Olivier

Regards,
Helin