Re: [dpdk-dev] [PATCH RFC] mbuf: Adjust TX flags to start at bit 32

["Wiles, Roger Keith" <keith.wiles@windriver.com>]

Hi Bruce,

I like the idea of the split, which should make it easier to do the testing of those bits.
One comment below.

On Sep 30, 2014, at 10:33 AM, Bruce Richardson <bruce.richardson@intel.com> wrote:

> On Tue, Sep 30, 2014 at 04:26:02PM +0100, Bruce Richardson wrote:
>> This patch takes the existing TX flags defined for the mbuf and shifts
>> each uniquely defined one left so that additional RX flags can be
>> defined without having RX and TX flags mixed together.
>> 
>> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
>> ---
> 
> This is just an RFC patch for now, as I'm looking for input to make sure 
> this is done right. Couple of opens, if people have input:
> * is a 32/32 split for RX/TX flags appropriate? Are we likely to have about 
>  equal numbers of each?
> * Doing a grep for the TX flag use, it seems the defines are commonly used, 
>  but if anyone is aware of anywhere where the code depends on the flags  
> having a particular value, please let me know.
> 
> If I have time, I also hope to look at doing rework on the testpmd flag 
> handling based off Olivier's previous patches, but since that is not 
> affecting the public ABI, I consider it a bit lower priority.
> 
> Thanks,
> /Bruce
> 
>> lib/librte_mbuf/rte_mbuf.h | 26 +++++++++++++-------------
>> 1 file changed, 13 insertions(+), 13 deletions(-)
>> 
>> diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h
>> index 1c6e115..c9fc4ec 100644
>> --- a/lib/librte_mbuf/rte_mbuf.h
>> +++ b/lib/librte_mbuf/rte_mbuf.h
>> @@ -86,26 +86,26 @@ extern "C" {
>> #define PKT_RX_IEEE1588_PTP  0x0200 /**< RX IEEE1588 L2 Ethernet PT Packet. */
>> #define PKT_RX_IEEE1588_TMST 0x0400 /**< RX IEEE1588 L2/L4 timestamped packet.*/
>> 
>> -#define PKT_TX_VLAN_PKT      0x0800 /**< TX packet is a 802.1q VLAN packet. */
>> -#define PKT_TX_IP_CKSUM      0x1000 /**< IP cksum of TX pkt. computed by NIC. */
>> -#define PKT_TX_IPV4_CSUM     0x1000 /**< Alias of PKT_TX_IP_CKSUM. */
>> -#define PKT_TX_IPV4          PKT_RX_IPV4_HDR /**< IPv4 with no IP checksum offload. */
>> -#define PKT_TX_IPV6          PKT_RX_IPV6_HDR /**< IPv6 packet */
>> +#define PKT_TX_VLAN_PKT  (0x0001 << 32) /**< TX packet is a 802.1q VLAN packet. */
>> +#define PKT_TX_IP_CKSUM  (0x0002 << 32) /**< IP cksum of TX pkt. computed by NIC. */

One little nit in the patch is does (0x0001 << 32) need to be (0x0001ULL << 32)? I have not tested it and just a thought.

Thanks
++Keith
>> +#define PKT_TX_IPV4_CSUM PKT_TX_IP_CKSUM /**< Alias of PKT_TX_IP_CKSUM. */
>> +#define PKT_TX_IPV4      PKT_RX_IPV4_HDR /**< IPv4 with no IP checksum offload. */
>> +#define PKT_TX_IPV6      PKT_RX_IPV6_HDR /**< IPv6 packet */
>> 
>> /*
>> - * Bit 14~13 used for L4 packet type with checksum enabled.
>> + * Bit 35~34 used for L4 packet type with checksum enabled.
>>  *     00: Reserved
>>  *     01: TCP checksum
>>  *     10: SCTP checksum
>>  *     11: UDP checksum
>>  */
>> -#define PKT_TX_L4_MASK       0x6000 /**< Mask bits for L4 checksum offload request. */
>> -#define PKT_TX_L4_NO_CKSUM   0x0000 /**< Disable L4 cksum of TX pkt. */
>> -#define PKT_TX_TCP_CKSUM     0x2000 /**< TCP cksum of TX pkt. computed by NIC. */
>> -#define PKT_TX_SCTP_CKSUM    0x4000 /**< SCTP cksum of TX pkt. computed by NIC. */
>> -#define PKT_TX_UDP_CKSUM     0x6000 /**< UDP cksum of TX pkt. computed by NIC. */
>> -/* Bit 15 */
>> -#define PKT_TX_IEEE1588_TMST 0x8000 /**< TX IEEE1588 packet to timestamp. */
>> +#define PKT_TX_L4_NO_CKSUM   (0x0000 << 32) /**< Disable L4 cksum of TX pkt. */
>> +#define PKT_TX_TCP_CKSUM     (0x0004 << 32) /**< TCP cksum of TX pkt. computed by NIC. */
>> +#define PKT_TX_SCTP_CKSUM    (0x0008 << 32) /**< SCTP cksum of TX pkt. computed by NIC. */
>> +#define PKT_TX_UDP_CKSUM     (0x000C << 32) /**< UDP cksum of TX pkt. computed by NIC. */
>> +#define PKT_TX_L4_MASK       (0x000C << 32) /**< Mask for L4 cksum offload request. */
>> +/* Bit 36 */
>> +#define PKT_TX_IEEE1588_TMST (0x0010 << 32) /**< TX IEEE1588 packet to timestamp. */
>> 
>> /* Use final bit of flags to indicate a control mbuf */
>> #define CTRL_MBUF_FLAG       (1ULL << 63)
>> -- 
>> 1.9.3
>> 

Keith Wiles, Principal Technologist with CTO office, Wind River mobile 972-213-5533