From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga03.intel.com (mga03.intel.com [134.134.136.65]) by dpdk.org (Postfix) with ESMTP id 33AE43B5 for ; Wed, 1 Oct 2014 10:40:57 +0200 (CEST) Received: from orsmga001.jf.intel.com ([10.7.209.18]) by orsmga103.jf.intel.com with ESMTP; 01 Oct 2014 01:45:23 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.04,630,1406617200"; d="scan'208";a="581751701" Received: from bricha3-mobl.ger.corp.intel.com (HELO bricha3-mobl.ir.intel.com) ([10.243.20.27]) by orsmga001.jf.intel.com with SMTP; 01 Oct 2014 01:47:22 -0700 Received: by bricha3-mobl.ir.intel.com (sSMTP sendmail emulation); Wed, 01 Oct 2014 09:47:21 +0001 Date: Wed, 1 Oct 2014 09:47:21 +0100 From: Bruce Richardson To: Neil Horman Message-ID: <20141001084721.GD1204@BRICHA3-MOBL> References: <1412090762-18852-1-git-send-email-bruce.richardson@intel.com> <20140930170632.GJ2193@hmsreliant.think-freely.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20140930170632.GJ2193@hmsreliant.think-freely.org> Organization: Intel Shannon Ltd. User-Agent: Mutt/1.5.22 (2013-10-16) Cc: dev@dpdk.org Subject: Re: [dpdk-dev] [PATCH RFC] mbuf: Adjust TX flags to start at bit 32 X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 01 Oct 2014 08:40:57 -0000 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 > > --- > > 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