> -----Original Message----- > From: Olivier MATZ [mailto:olivier.matz at 6wind.com] > Sent: Tuesday, September 9, 2014 4:03 PM > To: Zhang, Helin; Yerden Zhumabekov; Richardson, Bruce; dev at 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