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 8E588C4B6 for ; Fri, 23 Oct 2015 10:26:46 +0200 (CEST) Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by orsmga103.jf.intel.com with ESMTP; 23 Oct 2015 01:26:22 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.20,185,1444719600"; d="scan'208";a="817683708" Received: from fmsmsx108.amr.corp.intel.com ([10.18.124.206]) by fmsmga001.fm.intel.com with ESMTP; 23 Oct 2015 01:26:22 -0700 Received: from fmsmsx152.amr.corp.intel.com (10.18.125.5) by FMSMSX108.amr.corp.intel.com (10.18.124.206) with Microsoft SMTP Server (TLS) id 14.3.248.2; Fri, 23 Oct 2015 01:26:22 -0700 Received: from shsmsx104.ccr.corp.intel.com (10.239.110.15) by FMSMSX152.amr.corp.intel.com (10.18.125.5) with Microsoft SMTP Server (TLS) id 14.3.248.2; Fri, 23 Oct 2015 01:26:22 -0700 Received: from shsmsx101.ccr.corp.intel.com ([169.254.1.96]) by SHSMSX104.ccr.corp.intel.com ([169.254.5.194]) with mapi id 14.03.0248.002; Fri, 23 Oct 2015 16:26:20 +0800 From: "Wang, Xiao W" To: "Zhang, Helin" , "dev@dpdk.org" Thread-Topic: [PATCH v4 1/2] e1000: enable igb TSO support Thread-Index: AQHRC9X0wb6QF5tt40iF04nwI9aVCp530F0AgADsqMA= Date: Fri, 23 Oct 2015 08:26:19 +0000 Message-ID: References: <1445340132-2630-2-git-send-email-xiao.w.wang@intel.com> <1445414127-11260-1-git-send-email-xiao.w.wang@intel.com> <1445414127-11260-2-git-send-email-xiao.w.wang@intel.com> In-Reply-To: Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.239.127.40] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Subject: Re: [dpdk-dev] [PATCH v4 1/2] e1000: enable igb TSO support 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: Fri, 23 Oct 2015 08:26:47 -0000 -----Original Message----- From: Zhang, Helin=20 Sent: Friday, October 23, 2015 10:04 AM To: Wang, Xiao W; dev@dpdk.org Cc: Lu, Wenzhuo; Richardson, Bruce Subject: RE: [PATCH v4 1/2] e1000: enable igb TSO support > -----Original Message----- > From: Wang, Xiao W > Sent: Wednesday, October 21, 2015 3:55 PM > To: dev@dpdk.org > Cc: Lu, Wenzhuo; Richardson, Bruce; Zhang, Helin; Wang, Xiao W > Subject: [PATCH v4 1/2] e1000: enable igb TSO support >=20 > This patch enables igb TSO feature, the feature works on both PF and VF. > The TCP segmentation offload needs to write the offload related=20 > information into the advanced context descriptors, which is similar to ch= ecksum offload. >=20 > Signed-off-by: Wang Xiao W > --- > drivers/net/e1000/igb_ethdev.c | 6 +- > drivers/net/e1000/igb_rxtx.c | 200 > +++++++++++++++++++++++++---------------- > 2 files changed, 127 insertions(+), 79 deletions(-) >=20 > diff --git a/drivers/net/e1000/igb_ethdev.c=20 > b/drivers/net/e1000/igb_ethdev.c index 848ef6e..2e69394 100644 > --- a/drivers/net/e1000/igb_ethdev.c > +++ b/drivers/net/e1000/igb_ethdev.c > @@ -1497,7 +1497,8 @@ eth_igb_infos_get(struct rte_eth_dev *dev,=20 > struct rte_eth_dev_info *dev_info) > DEV_TX_OFFLOAD_IPV4_CKSUM | > DEV_TX_OFFLOAD_UDP_CKSUM | > DEV_TX_OFFLOAD_TCP_CKSUM | > - DEV_TX_OFFLOAD_SCTP_CKSUM; > + DEV_TX_OFFLOAD_SCTP_CKSUM | > + DEV_TX_OFFLOAD_TCP_TSO; >=20 > switch (hw->mac.type) { > case e1000_82575: > @@ -1588,7 +1589,8 @@ eth_igbvf_infos_get(struct rte_eth_dev *dev,=20 > struct rte_eth_dev_info *dev_info) > DEV_TX_OFFLOAD_IPV4_CKSUM | > DEV_TX_OFFLOAD_UDP_CKSUM | > DEV_TX_OFFLOAD_TCP_CKSUM | > - DEV_TX_OFFLOAD_SCTP_CKSUM; > + DEV_TX_OFFLOAD_SCTP_CKSUM | > + DEV_TX_OFFLOAD_TCP_TSO; > switch (hw->mac.type) { > case e1000_vfadapt: > dev_info->max_rx_queues =3D 2; > diff --git a/drivers/net/e1000/igb_rxtx.c=20 > b/drivers/net/e1000/igb_rxtx.c index > 19905fd..2b8a1c8 100644 > --- a/drivers/net/e1000/igb_rxtx.c > +++ b/drivers/net/e1000/igb_rxtx.c > @@ -76,7 +76,8 @@ > #define IGB_TX_OFFLOAD_MASK ( \ > PKT_TX_VLAN_PKT | \ > PKT_TX_IP_CKSUM | \ > - PKT_TX_L4_MASK) > + PKT_TX_L4_MASK | \ > + PKT_TX_TCP_SEG) >=20 > static inline struct rte_mbuf * > rte_rxmbuf_alloc(struct rte_mempool *mp) @@ -146,32 +147,40 @@ enum=20 > igb_advctx_num { }; >=20 > /** Offload features */ > -union igb_vlan_macip { > - uint32_t data; > +union igb_tx_offload { > + uint64_t data; Adding previous definition back to this union is helpful of using the first= 16 bits and second 16 bits? Of cause, the newly added struct should be kept as is. struct { uint16_t l2_l3_len; uint16_t vlan_tci; uint16_t l4_tso_len; uint16_t others; }; In the patch, we need to quote the l3_len and l2_len separately for some ti= mes, for example "tx_offload.l3_len =3D tx_pkt->l3_len;"=20 It's more convenient to define them separately. > struct { > - uint16_t l2_l3_len; /**< 7bit L2 and 9b L3 lengths combined */ > - uint16_t vlan_tci; > - /**< VLAN Tag Control Identifier (CPU order). */ > - } f; > + uint64_t l3_len:9; /**< L3 (IP) Header Length. */ > + uint64_t l2_len:7; /**< L2 (MAC) Header Length. */ > + uint64_t vlan_tci:16; /**< VLAN Tag Control Identifier(CPU order). */ > + uint64_t l4_len:8; /**< L4 (TCP/UDP) Header Length. */ > + uint64_t tso_segsz:16; /**< TCP TSO segment size. */ > + > + /* uint64_t unused:8; */ > + }; > }; >=20 > /* > - * Compare mask for vlan_macip_len.data, > - * should be in sync with igb_vlan_macip.f layout. > + * Compare mask for igb_tx_offload.data, > + * should be in sync with igb_tx_offload layout. > * */ > -#define TX_VLAN_CMP_MASK 0xFFFF0000 /**< VLAN length - > 16-bits. */ > -#define TX_MAC_LEN_CMP_MASK 0x0000FE00 /**< MAC length - 7-bits. > */ > -#define TX_IP_LEN_CMP_MASK 0x000001FF /**< IP length - 9-bits. */ > -/** MAC+IP length. */ > -#define TX_MACIP_LEN_CMP_MASK (TX_MAC_LEN_CMP_MASK | > TX_IP_LEN_CMP_MASK) > +#define TX_MACIP_LEN_CMP_MASK 0x000000000000FFFFULL /**< L2L3 > header mask. */ > +#define TX_VLAN_CMP_MASK 0x00000000FFFF0000ULL /**< Vlan mask. > */ > +#define TX_TCP_LEN_CMP_MASK 0x000000FF00000000ULL /**< TCP > header mask. */ > +#define TX_TSO_MSS_CMP_MASK 0x00FFFF0000000000ULL /**< TSO > segsz mask. */ > +/** Mac + IP + TCP + Mss mask. */ > +#define TX_TSO_CMP_MASK \ > + (TX_MACIP_LEN_CMP_MASK | TX_TCP_LEN_CMP_MASK | > TX_TSO_MSS_CMP_MASK) >=20 > /** > * Strucutre to check if new context need be built > */ > struct igb_advctx_info { > uint64_t flags; /**< ol_flags related to context build. */ > - uint32_t cmp_mask; /**< compare mask for vlan_macip_lens */ > - union igb_vlan_macip vlan_macip_lens; /**< vlan, mac & ip length. */ > + /** tx offload: vlan, tso, l2-l3-l4 lengths. */ > + union igb_tx_offload tx_offload; > + /** compare mask for tx offload. */ > + union igb_tx_offload tx_offload_mask; Would above mask be better/clearer replaced by uint64_t directly? You can think a bit, I am not quite sure if it is really better. Regards, Helin Since we only use 56 of the 64 bits as mask bits, I think using the union w= ill make it easier to understand the structure of the offload_mask. Thanks, Wang Xiao