DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH RFC] mbuf: Adjust TX flags to start at bit 32
@ 2014-09-30 15:26 Bruce Richardson
  2014-09-30 15:33 ` Bruce Richardson
  2014-09-30 17:06 ` Neil Horman
  0 siblings, 2 replies; 7+ messages in thread
From: Bruce Richardson @ 2014-09-30 15:26 UTC (permalink / raw)
  To: dev

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>
---
 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. */
+#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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [dpdk-dev] [PATCH RFC] mbuf: Adjust TX flags to start at bit 32
  2014-09-30 15:26 [dpdk-dev] [PATCH RFC] mbuf: Adjust TX flags to start at bit 32 Bruce Richardson
@ 2014-09-30 15:33 ` Bruce Richardson
  2014-09-30 16:00   ` Wiles, Roger Keith
  2014-09-30 17:06 ` Neil Horman
  1 sibling, 1 reply; 7+ messages in thread
From: Bruce Richardson @ 2014-09-30 15:33 UTC (permalink / raw)
  To: dev

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. */
> +#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
> 

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [dpdk-dev] [PATCH RFC] mbuf: Adjust TX flags to start at bit 32
  2014-09-30 15:33 ` Bruce Richardson
@ 2014-09-30 16:00   ` Wiles, Roger Keith
  2014-09-30 16:03     ` Bruce Richardson
  0 siblings, 1 reply; 7+ messages in thread
From: Wiles, Roger Keith @ 2014-09-30 16:00 UTC (permalink / raw)
  To: RICHARDSON, BRUCE; +Cc: dev

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [dpdk-dev] [PATCH RFC] mbuf: Adjust TX flags to start at bit 32
  2014-09-30 16:00   ` Wiles, Roger Keith
@ 2014-09-30 16:03     ` Bruce Richardson
  0 siblings, 0 replies; 7+ messages in thread
From: Bruce Richardson @ 2014-09-30 16:03 UTC (permalink / raw)
  To: Wiles, Roger Keith; +Cc: dev

On Tue, Sep 30, 2014 at 05:00:04PM +0100, Wiles, Roger Keith wrote:
> 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.

Yes, indeed it does, good catch!

/Bruce

> 
> 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
> 

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [dpdk-dev] [PATCH RFC] mbuf: Adjust TX flags to start at bit 32
  2014-09-30 15:26 [dpdk-dev] [PATCH RFC] mbuf: Adjust TX flags to start at bit 32 Bruce Richardson
  2014-09-30 15:33 ` Bruce Richardson
@ 2014-09-30 17:06 ` Neil Horman
  2014-10-01  8:47   ` Bruce Richardson
  1 sibling, 1 reply; 7+ messages in thread
From: Neil Horman @ 2014-09-30 17:06 UTC (permalink / raw)
  To: Bruce Richardson; +Cc: dev

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>
> ---
>  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. */
> +#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
> 
> 

I'm not opposed to the patch at all, but I would like to point out that this is
the sort of change that breaks ABI very easily (which is fine right now given
the mbuf changes already staged for the release, but still something to be aware
of).  As such, are there advantages to this patch (other than the niceness of
human readability)?

If we're going to reshuffle these flags now, it might be nice to start tx flags
at the most significant bit and count back, and start rx flags at the least
significant bit and count up.  That would ensure that we don't reserve flags for
a direction without need.

Best
Neil

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [dpdk-dev] [PATCH RFC] mbuf: Adjust TX flags to start at bit 32
  2014-09-30 17:06 ` Neil Horman
@ 2014-10-01  8:47   ` Bruce Richardson
  2014-10-01 11:14     ` Neil Horman
  0 siblings, 1 reply; 7+ messages in thread
From: Bruce Richardson @ 2014-10-01  8:47 UTC (permalink / raw)
  To: Neil Horman; +Cc: dev

On Tue, Sep 30, 2014 at 01:06:32PM -0400, Neil Horman 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>
> > ---
> >  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. */
> > +#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
> > 
> > 
> 
> I'm not opposed to the patch at all, but I would like to point out that this is
> the sort of change that breaks ABI very easily (which is fine right now given
> the mbuf changes already staged for the release, but still something to be aware
> of).  As such, are there advantages to this patch (other than the niceness of
> human readability)?
> 
> If we're going to reshuffle these flags now, it might be nice to start tx flags
> at the most significant bit and count back, and start rx flags at the least
> significant bit and count up.  That would ensure that we don't reserve flags for
> a direction without need.
> 
> Best
> Neil
>
Good idea, though currently the most significant bit is being used as a flag 
to indicate a control mbuf. My thinking is that we may potentially need 
other flags that are not just for RX or TX, and to have those at the end.  
However, given that that is likely to be the exception case, perhaps we 
could reserve the last byte for non-RX/TX flags and then implement the 
scheme you suggest. What do you think?

/Bruce 

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [dpdk-dev] [PATCH RFC] mbuf: Adjust TX flags to start at bit 32
  2014-10-01  8:47   ` Bruce Richardson
@ 2014-10-01 11:14     ` Neil Horman
  0 siblings, 0 replies; 7+ messages in thread
From: Neil Horman @ 2014-10-01 11:14 UTC (permalink / raw)
  To: Bruce Richardson; +Cc: dev

On Wed, Oct 01, 2014 at 09:47:21AM +0100, Bruce Richardson wrote:
> On Tue, Sep 30, 2014 at 01:06:32PM -0400, Neil Horman 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>
> > > ---
> > >  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. */
> > > +#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
> > > 
> > > 
> > 
> > I'm not opposed to the patch at all, but I would like to point out that this is
> > the sort of change that breaks ABI very easily (which is fine right now given
> > the mbuf changes already staged for the release, but still something to be aware
> > of).  As such, are there advantages to this patch (other than the niceness of
> > human readability)?
> > 
> > If we're going to reshuffle these flags now, it might be nice to start tx flags
> > at the most significant bit and count back, and start rx flags at the least
> > significant bit and count up.  That would ensure that we don't reserve flags for
> > a direction without need.
> > 
> > Best
> > Neil
> >
> Good idea, though currently the most significant bit is being used as a flag 
> to indicate a control mbuf. My thinking is that we may potentially need 
> other flags that are not just for RX or TX, and to have those at the end.  
> However, given that that is likely to be the exception case, perhaps we 
> could reserve the last byte for non-RX/TX flags and then implement the 
> scheme you suggest. What do you think?
> 
Sure, seems reasonable
Neil

> /Bruce 
> 

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2014-10-01 11:08 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-09-30 15:26 [dpdk-dev] [PATCH RFC] mbuf: Adjust TX flags to start at bit 32 Bruce Richardson
2014-09-30 15:33 ` Bruce Richardson
2014-09-30 16:00   ` Wiles, Roger Keith
2014-09-30 16:03     ` Bruce Richardson
2014-09-30 17:06 ` Neil Horman
2014-10-01  8:47   ` Bruce Richardson
2014-10-01 11:14     ` Neil Horman

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).