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. 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. - does not describe what enhancement is provided by adding the field (performance? in this case, numbers + use case would help to convince people). - does not describe what can be the content of the field. Is it a protocol number? - 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. Regards, Olivier