Message ID | 1417532767-1309-3-git-send-email-jijiang.liu@intel.com (mailing list archive) |
---|---|
State | Accepted, archived |
Headers |
Return-Path: <dev-bounces@dpdk.org> X-Original-To: patchwork@dpdk.org Delivered-To: patchwork@dpdk.org Received: from [92.243.14.124] (localhost [IPv6:::1]) by dpdk.org (Postfix) with ESMTP id 20D827F6D; Tue, 2 Dec 2014 16:06:56 +0100 (CET) Received: from mga14.intel.com (mga14.intel.com [192.55.52.115]) by dpdk.org (Postfix) with ESMTP id D0A877F51 for <dev@dpdk.org>; Tue, 2 Dec 2014 16:06:52 +0100 (CET) Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by fmsmga103.fm.intel.com with ESMTP; 02 Dec 2014 06:58:55 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.07,501,1413270000"; d="scan'208";a="631418037" Received: from shvmail01.sh.intel.com ([10.239.29.42]) by fmsmga001.fm.intel.com with ESMTP; 02 Dec 2014 07:06:17 -0800 Received: from shecgisg004.sh.intel.com (shecgisg004.sh.intel.com [10.239.29.89]) by shvmail01.sh.intel.com with ESMTP id sB2F6G54007324; Tue, 2 Dec 2014 23:06:16 +0800 Received: from shecgisg004.sh.intel.com (localhost [127.0.0.1]) by shecgisg004.sh.intel.com (8.13.6/8.13.6/SuSE Linux 0.8) with ESMTP id sB2F6DL7001358; Tue, 2 Dec 2014 23:06:16 +0800 Received: (from jijiangl@localhost) by shecgisg004.sh.intel.com (8.13.6/8.13.6/Submit) id sB2F6DVQ001354; Tue, 2 Dec 2014 23:06:13 +0800 From: Jijiang Liu <jijiang.liu@intel.com> To: dev@dpdk.org Date: Tue, 2 Dec 2014 23:06:06 +0800 Message-Id: <1417532767-1309-3-git-send-email-jijiang.liu@intel.com> X-Mailer: git-send-email 1.7.12.2 In-Reply-To: <1417532767-1309-1-git-send-email-jijiang.liu@intel.com> References: <1417532767-1309-1-git-send-email-jijiang.liu@intel.com> Subject: [dpdk-dev] [PATCH v5 2/3] mbuf:add three TX ol_flags and repalce PKT_TX_VXLAN_CKSUM X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK <dev.dpdk.org> List-Unsubscribe: <http://dpdk.org/ml/options/dev>, <mailto:dev-request@dpdk.org?subject=unsubscribe> List-Archive: <http://dpdk.org/ml/archives/dev/> List-Post: <mailto:dev@dpdk.org> List-Help: <mailto:dev-request@dpdk.org?subject=help> List-Subscribe: <http://dpdk.org/ml/listinfo/dev>, <mailto:dev-request@dpdk.org?subject=subscribe> Errors-To: dev-bounces@dpdk.org Sender: "dev" <dev-bounces@dpdk.org> |
Commit Message
Jijiang Liu
Dec. 2, 2014, 3:06 p.m. UTC
Replace PKT_TX_VXLAN_CKSUM with PKT_TX_UDP_TUNNEL_PKT in order to indicate a packet is an UDP tunneling packet, and introduce 3 TX offload flags for outer IP TX checksum, which are PKT_TX_OUTER_IP_CKSUM, PKT_TX_OUTER_IPV4 and PKT_TX_OUTER_IPV6 respectively;Rework csum forward engine and i40e PMD due to these changes.
Signed-off-by: Jijiang Liu <jijiang.liu@intel.com>
---
app/test-pmd/csumonly.c | 9 +++++++--
lib/librte_mbuf/rte_mbuf.c | 7 ++++++-
lib/librte_mbuf/rte_mbuf.h | 11 ++++++++++-
lib/librte_pmd_i40e/i40e_rxtx.c | 6 +++---
4 files changed, 26 insertions(+), 7 deletions(-)
Comments
Hi Jijiang, On 12/02/2014 04:06 PM, Jijiang Liu wrote: > Replace PKT_TX_VXLAN_CKSUM with PKT_TX_UDP_TUNNEL_PKT in order to indicate a packet is an UDP tunneling packet, and introduce 3 TX offload flags for outer IP TX checksum, which are PKT_TX_OUTER_IP_CKSUM, PKT_TX_OUTER_IPV4 and PKT_TX_OUTER_IPV6 respectively;Rework csum forward engine and i40e PMD due to these changes. > > Signed-off-by: Jijiang Liu <jijiang.liu@intel.com> > --- > app/test-pmd/csumonly.c | 9 +++++++-- > lib/librte_mbuf/rte_mbuf.c | 7 ++++++- > lib/librte_mbuf/rte_mbuf.h | 11 ++++++++++- > lib/librte_pmd_i40e/i40e_rxtx.c | 6 +++--- > 4 files changed, 26 insertions(+), 7 deletions(-) > > diff --git a/app/test-pmd/csumonly.c b/app/test-pmd/csumonly.c > index d8c080a..9094967 100644 > --- a/app/test-pmd/csumonly.c > +++ b/app/test-pmd/csumonly.c > @@ -257,7 +257,7 @@ process_outer_cksums(void *outer_l3_hdr, uint16_t outer_ethertype, > uint64_t ol_flags = 0; > > if (testpmd_ol_flags & TESTPMD_TX_OFFLOAD_VXLAN_CKSUM) > - ol_flags |= PKT_TX_VXLAN_CKSUM; > + ol_flags |= PKT_TX_UDP_TUNNEL_PKT; > > if (outer_ethertype == _htons(ETHER_TYPE_IPv4)) { > ipv4_hdr->hdr_checksum = 0; > @@ -470,7 +470,12 @@ pkt_burst_checksum_forward(struct fwd_stream *fs) > { PKT_TX_UDP_CKSUM, PKT_TX_L4_MASK }, > { PKT_TX_TCP_CKSUM, PKT_TX_L4_MASK }, > { PKT_TX_SCTP_CKSUM, PKT_TX_L4_MASK }, > - { PKT_TX_VXLAN_CKSUM, PKT_TX_VXLAN_CKSUM }, > + { PKT_TX_UDP_TUNNEL_PKT, PKT_TX_UDP_TUNNEL_PKT }, > + { PKT_TX_IPV4, PKT_TX_IPV4 }, > + { PKT_TX_IPV6, PKT_TX_IPV6 }, > + { PKT_TX_OUTER_IP_CKSUM, PKT_TX_OUTER_IP_CKSUM }, > + { PKT_TX_OUTER_IPV4, PKT_TX_OUTER_IPV4 }, > + { PKT_TX_OUTER_IPV6, PKT_TX_OUTER_IPV6 }, > { PKT_TX_TCP_SEG, PKT_TX_TCP_SEG }, I still think having a flag IPV4 + another flag IP_CHECKSUM is not appropriate. I though Konstantin agreed on other flags, but I may have misunderstood: http://dpdk.org/ml/archives/dev/2014-November/009070.html Olivier
Hi Oliver, > -----Original Message----- > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Olivier MATZ > Sent: Wednesday, December 03, 2014 11:41 AM > To: Liu, Jijiang; dev@dpdk.org > Subject: Re: [dpdk-dev] [PATCH v5 2/3] mbuf:add three TX ol_flags and repalce PKT_TX_VXLAN_CKSUM > > Hi Jijiang, > > On 12/02/2014 04:06 PM, Jijiang Liu wrote: > > Replace PKT_TX_VXLAN_CKSUM with PKT_TX_UDP_TUNNEL_PKT in order to indicate a packet is an UDP tunneling packet, and > introduce 3 TX offload flags for outer IP TX checksum, which are PKT_TX_OUTER_IP_CKSUM, PKT_TX_OUTER_IPV4 and > PKT_TX_OUTER_IPV6 respectively;Rework csum forward engine and i40e PMD due to these changes. > > > > Signed-off-by: Jijiang Liu <jijiang.liu@intel.com> > > --- > > app/test-pmd/csumonly.c | 9 +++++++-- > > lib/librte_mbuf/rte_mbuf.c | 7 ++++++- > > lib/librte_mbuf/rte_mbuf.h | 11 ++++++++++- > > lib/librte_pmd_i40e/i40e_rxtx.c | 6 +++--- > > 4 files changed, 26 insertions(+), 7 deletions(-) > > > > diff --git a/app/test-pmd/csumonly.c b/app/test-pmd/csumonly.c > > index d8c080a..9094967 100644 > > --- a/app/test-pmd/csumonly.c > > +++ b/app/test-pmd/csumonly.c > > @@ -257,7 +257,7 @@ process_outer_cksums(void *outer_l3_hdr, uint16_t outer_ethertype, > > uint64_t ol_flags = 0; > > > > if (testpmd_ol_flags & TESTPMD_TX_OFFLOAD_VXLAN_CKSUM) > > - ol_flags |= PKT_TX_VXLAN_CKSUM; > > + ol_flags |= PKT_TX_UDP_TUNNEL_PKT; > > > > if (outer_ethertype == _htons(ETHER_TYPE_IPv4)) { > > ipv4_hdr->hdr_checksum = 0; > > @@ -470,7 +470,12 @@ pkt_burst_checksum_forward(struct fwd_stream *fs) > > { PKT_TX_UDP_CKSUM, PKT_TX_L4_MASK }, > > { PKT_TX_TCP_CKSUM, PKT_TX_L4_MASK }, > > { PKT_TX_SCTP_CKSUM, PKT_TX_L4_MASK }, > > - { PKT_TX_VXLAN_CKSUM, PKT_TX_VXLAN_CKSUM }, > > + { PKT_TX_UDP_TUNNEL_PKT, PKT_TX_UDP_TUNNEL_PKT }, > > + { PKT_TX_IPV4, PKT_TX_IPV4 }, > > + { PKT_TX_IPV6, PKT_TX_IPV6 }, > > + { PKT_TX_OUTER_IP_CKSUM, PKT_TX_OUTER_IP_CKSUM }, > > + { PKT_TX_OUTER_IPV4, PKT_TX_OUTER_IPV4 }, > > + { PKT_TX_OUTER_IPV6, PKT_TX_OUTER_IPV6 }, > > { PKT_TX_TCP_SEG, PKT_TX_TCP_SEG }, > > I still think having a flag IPV4 + another flag IP_CHECKSUM is not > appropriate. Sorry, didn't get you here. Are you talking about our discussion should PKT_TX_IP_CKSUM and PKT_TX_IPV4 be mutually exclusive or not? > I though Konstantin agreed on other flags, but I may > have misunderstood: > > http://dpdk.org/ml/archives/dev/2014-November/009070.html In that mail, I was talking about my suggestion to make PKT_TX_IP_CKSUM, PKT_TX_IPV4 and PKT_TX_IPV6 to occupy 2 bits. Something like: #define PKT_TX_IP_CKSUM (1 << X) #define PKT_TX_IPV6 (2 << X) #define PKT_TX_IPV4 (3 << X) "Even better, if we can squeeze these 3 flags into 2 bits. Would save us 2 bits, plus might be handy, as in the PMD you can do: switch (ol_flags & TX_L3_MASK) { case TX_IPV4: ... break; case TX_IPV6: ... break; case TX_IP_CKSUM: ... break; }" As you pointed out, it will break backward compatibility. I agreed with that and self-NACKed it. > > > Olivier
Hi Konstantin, On 12/03/2014 01:59 PM, Ananyev, Konstantin wrote: >> I still think having a flag IPV4 + another flag IP_CHECKSUM is not >> appropriate. > > Sorry, didn't get you here. > Are you talking about our discussion should PKT_TX_IP_CKSUM and PKT_TX_IPV4 be mutually exclusive or not? Yes >> I though Konstantin agreed on other flags, but I may >> have misunderstood: >> >> http://dpdk.org/ml/archives/dev/2014-November/009070.html > > In that mail, I was talking about my suggestion to make PKT_TX_IP_CKSUM, PKT_TX_IPV4 and PKT_TX_IPV6 to occupy 2 bits. > Something like: > #define PKT_TX_IP_CKSUM (1 << X) > #define PKT_TX_IPV6 (2 << X) > #define PKT_TX_IPV4 (3 << X) > > "Even better, if we can squeeze these 3 flags into 2 bits. > Would save us 2 bits, plus might be handy, as in the PMD you can do: > > switch (ol_flags & TX_L3_MASK) { > case TX_IPV4: > ... > break; > case TX_IPV6: > ... > break; > case TX_IP_CKSUM: > ... > break; > }" > > As you pointed out, it will break backward compatibility. > I agreed with that and self-NACKed it. ok, so we are back between: 1/ (Jijiang's patch) PKT_TX_IP_CKSUM /* packet is IPv4, and we want hw cksum */ PKT_TX_IPV6 /* packet is IPv6 */ PKT_TX_IPV4 /* packet is IPv4, and we don't want hw cksum */ with PKT_TX_IP_CKSUM and PKT_TX_IPV4 exclusive and 2/ PKT_TX_IP_CKSUM /* we want hw IP cksum */ PKT_TX_IPV6 /* packet is IPv6 */ PKT_TX_IPV4 /* packet is IPv4 */ with PKT_TX_IP_CKSUM implies PKT_TX_IPV4 Solution 2/ looks better from a user point of view. Anyone else has an opinion? Regards, Olivier
Hi Olivier, > -----Original Message----- > From: Olivier MATZ [mailto:olivier.matz@6wind.com] > Sent: Wednesday, December 3, 2014 10:42 PM > To: Ananyev, Konstantin; Liu, Jijiang; dev@dpdk.org > Subject: Re: [dpdk-dev] [PATCH v5 2/3] mbuf:add three TX ol_flags and repalce > PKT_TX_VXLAN_CKSUM > > Hi Konstantin, > > On 12/03/2014 01:59 PM, Ananyev, Konstantin wrote: > >> I still think having a flag IPV4 + another flag IP_CHECKSUM is not > >> appropriate. > > > > Sorry, didn't get you here. > > Are you talking about our discussion should PKT_TX_IP_CKSUM and > PKT_TX_IPV4 be mutually exclusive or not? > > Yes > > >> I though Konstantin agreed on other flags, but I may have > >> misunderstood: > >> > >> http://dpdk.org/ml/archives/dev/2014-November/009070.html > > > > In that mail, I was talking about my suggestion to make PKT_TX_IP_CKSUM, > PKT_TX_IPV4 and PKT_TX_IPV6 to occupy 2 bits. > > Something like: > > #define PKT_TX_IP_CKSUM (1 << X) > > #define PKT_TX_IPV6 (2 << X) > > #define PKT_TX_IPV4 (3 << X) > > > > "Even better, if we can squeeze these 3 flags into 2 bits. > > Would save us 2 bits, plus might be handy, as in the PMD you can do: > > > > switch (ol_flags & TX_L3_MASK) { > > case TX_IPV4: > > ... > > break; > > case TX_IPV6: > > ... > > break; > > case TX_IP_CKSUM: > > ... > > break; > > }" > > > > As you pointed out, it will break backward compatibility. > > I agreed with that and self-NACKed it. > > ok, so we are back between: > > 1/ (Jijiang's patch) > PKT_TX_IP_CKSUM /* packet is IPv4, and we want hw cksum */ > PKT_TX_IPV6 /* packet is IPv6 */ > PKT_TX_IPV4 /* packet is IPv4, and we don't want hw cksum */ > > with PKT_TX_IP_CKSUM and PKT_TX_IPV4 exclusive > > and > > 2/ > PKT_TX_IP_CKSUM /* we want hw IP cksum */ > PKT_TX_IPV6 /* packet is IPv6 */ > PKT_TX_IPV4 /* packet is IPv4 */ > > with PKT_TX_IP_CKSUM implies PKT_TX_IPV4 > > > Solution 2/ looks better from a user point of view. Anyone else has an opinion? Let's think about these IPv4/6 flags in terms of checksum and IP version/type, 1. For IPv6 IP checksum is meaningful only for IPv4, so we define 'PKT_TX_IPV6 /* packet is IPv6 */' to tell driver/HW that this is IPV6 packet, here we don't talk about the checksum for IPv6 as it is meaningless. Right? PKT_TX_IPV6 /* packet is IPv6 */ ------ IP type: v6; HW checksum: meaningless 2. For IPv4, My patch: PKT_TX_IP_CKSUM /* packet is IPv4, and we want hw cksum */--------------------------IP type: v4; HW checksum: Yes PKT_TX_IPV4 /* packet is IPv4, and we don't want hw cksum */ ----------------------- IP type: v4; HW checksum: No You want: PKT_TX_IP_CKSUM /* we want hw IP cksum */-------------------------- IP type: v4; HW checksum: Yes PKT_TX_IPV4 /* packet is IPv4*/ ------------------------ IP type: v4; HW checksum: yes or no? driver/HW don't know, just know this is packet with IPv4 header. HW checksum: meaningless?? > Regards, > Olivier
> -----Original Message----- > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Olivier MATZ > Sent: Wednesday, December 3, 2014 10:42 PM > To: Ananyev, Konstantin; Liu, Jijiang; dev@dpdk.org > Subject: Re: [dpdk-dev] [PATCH v5 2/3] mbuf:add three TX ol_flags and repalce > PKT_TX_VXLAN_CKSUM > > Hi Konstantin, > > On 12/03/2014 01:59 PM, Ananyev, Konstantin wrote: > >> I still think having a flag IPV4 + another flag IP_CHECKSUM is not > >> appropriate. > > > > Sorry, didn't get you here. > > Are you talking about our discussion should PKT_TX_IP_CKSUM and > PKT_TX_IPV4 be mutually exclusive or not? > > Yes > > >> I though Konstantin agreed on other flags, but I may have > >> misunderstood: > >> > >> http://dpdk.org/ml/archives/dev/2014-November/009070.html > > > > In that mail, I was talking about my suggestion to make PKT_TX_IP_CKSUM, > PKT_TX_IPV4 and PKT_TX_IPV6 to occupy 2 bits. > > Something like: > > #define PKT_TX_IP_CKSUM (1 << X) > > #define PKT_TX_IPV6 (2 << X) > > #define PKT_TX_IPV4 (3 << X) > > > > "Even better, if we can squeeze these 3 flags into 2 bits. > > Would save us 2 bits, plus might be handy, as in the PMD you can do: > > > > switch (ol_flags & TX_L3_MASK) { > > case TX_IPV4: > > ... > > break; > > case TX_IPV6: > > ... > > break; > > case TX_IP_CKSUM: > > ... > > break; > > }" > > > > As you pointed out, it will break backward compatibility. > > I agreed with that and self-NACKed it. > > ok, so we are back between: > > 1/ (Jijiang's patch) > PKT_TX_IP_CKSUM /* packet is IPv4, and we want hw cksum */ > PKT_TX_IPV6 /* packet is IPv6 */ > PKT_TX_IPV4 /* packet is IPv4, and we don't want hw cksum */ > > with PKT_TX_IP_CKSUM and PKT_TX_IPV4 exclusive > > and > > 2/ > PKT_TX_IP_CKSUM /* we want hw IP cksum */ > PKT_TX_IPV6 /* packet is IPv6 */ > PKT_TX_IPV4 /* packet is IPv4 */ There is another bit flag named 'PKT_TX_IPV4_CSUM' which uses the same bit of 'PKT_TX_IP_CSUM'. It is for identifying if ipv4 hardware checksum offload is needed or not. It seems that we do not need 'PKT_TX_IPV6_CSUM'. 'PKT_TX_IPV4' and 'PKT_TX_IPV6' just indicates its packet type, and I guess other features should not be contained in it, according to its name. So here I got the option 3: PKT_TX_IPV4_CKSUM /* we want hw IPv4 cksum */ PKT_TX_IPV6 /* packet is IPv6 */ PKT_TX_IPV4 /* packet is IPv4 */ > > with PKT_TX_IP_CKSUM implies PKT_TX_IPV4 > > > Solution 2/ looks better from a user point of view. Anyone else has an opinion? > > Regards, > Olivier Regards, Helin
Hi Helin, > -----Original Message----- > From: Zhang, Helin > Sent: Thursday, December 4, 2014 2:52 PM > To: Olivier MATZ; Ananyev, Konstantin; Liu, Jijiang; dev@dpdk.org > Subject: RE: [dpdk-dev] [PATCH v5 2/3] mbuf:add three TX ol_flags and repalce > PKT_TX_VXLAN_CKSUM > > > > > -----Original Message----- > > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Olivier MATZ > > Sent: Wednesday, December 3, 2014 10:42 PM > > To: Ananyev, Konstantin; Liu, Jijiang; dev@dpdk.org > > Subject: Re: [dpdk-dev] [PATCH v5 2/3] mbuf:add three TX ol_flags and > > repalce PKT_TX_VXLAN_CKSUM > > > > Hi Konstantin, > > > > On 12/03/2014 01:59 PM, Ananyev, Konstantin wrote: > > >> I still think having a flag IPV4 + another flag IP_CHECKSUM is not > > >> appropriate. > > > > > > Sorry, didn't get you here. > > > Are you talking about our discussion should PKT_TX_IP_CKSUM and > > PKT_TX_IPV4 be mutually exclusive or not? > > > > Yes > > > > >> I though Konstantin agreed on other flags, but I may have > > >> misunderstood: > > >> > > >> http://dpdk.org/ml/archives/dev/2014-November/009070.html > > > > > > In that mail, I was talking about my suggestion to make > > > PKT_TX_IP_CKSUM, > > PKT_TX_IPV4 and PKT_TX_IPV6 to occupy 2 bits. > > > Something like: > > > #define PKT_TX_IP_CKSUM (1 << X) > > > #define PKT_TX_IPV6 (2 << X) > > > #define PKT_TX_IPV4 (3 << X) > > > > > > "Even better, if we can squeeze these 3 flags into 2 bits. > > > Would save us 2 bits, plus might be handy, as in the PMD you can do: > > > > > > switch (ol_flags & TX_L3_MASK) { > > > case TX_IPV4: > > > ... > > > break; > > > case TX_IPV6: > > > ... > > > break; > > > case TX_IP_CKSUM: > > > ... > > > break; > > > }" > > > > > > As you pointed out, it will break backward compatibility. > > > I agreed with that and self-NACKed it. > > > > ok, so we are back between: > > > > 1/ (Jijiang's patch) > > PKT_TX_IP_CKSUM /* packet is IPv4, and we want hw cksum */ > > PKT_TX_IPV6 /* packet is IPv6 */ > > PKT_TX_IPV4 /* packet is IPv4, and we don't want hw cksum */ > > > > with PKT_TX_IP_CKSUM and PKT_TX_IPV4 exclusive > > > > and > > > > 2/ > > PKT_TX_IP_CKSUM /* we want hw IP cksum */ > > PKT_TX_IPV6 /* packet is IPv6 */ > > PKT_TX_IPV4 /* packet is IPv4 */ > There is another bit flag named 'PKT_TX_IPV4_CSUM' which uses the same bit of > 'PKT_TX_IP_CSUM'. It is for identifying if ipv4 hardware checksum offload is > needed or not. > It seems that we do not need 'PKT_TX_IPV6_CSUM'. > 'PKT_TX_IPV4' and 'PKT_TX_IPV6' just indicates its packet type, and I guess other > features should not be contained in it, according to its name. > > So here I got the option 3: > PKT_TX_IPV4_CKSUM /* we want hw IPv4 cksum */ > PKT_TX_IPV6 /* packet is IPv6 */ > PKT_TX_IPV4 /* packet is IPv4 */ In TX side, if just tell driver/HW this is a IPV4 packet , and don't tell driver/HW whether TX checksum or other offload is required or not , the flag is senseless, and hardware will not do any offload. The flag is not used in igb/ixgbe codes, which is just used in i40e codes, and set this bit(IPv4 packet with no IP checksum offload) in i40e driver. ... } else if (ol_flags & PKT_TX_IPV4) { *td_cmd |= I40E_TX_DESC_CMD_IIPT_IPV4; ... > > > > with PKT_TX_IP_CKSUM implies PKT_TX_IPV4 > > > > > > Solution 2/ looks better from a user point of view. Anyone else has an opinion? > > > > Regards, > > Olivier > > Regards, > Helin
Hi Helin, > -----Original Message----- > From: Zhang, Helin > Sent: Thursday, December 04, 2014 6:52 AM > To: Olivier MATZ; Ananyev, Konstantin; Liu, Jijiang; dev@dpdk.org > Subject: RE: [dpdk-dev] [PATCH v5 2/3] mbuf:add three TX ol_flags and repalce PKT_TX_VXLAN_CKSUM > > > > > -----Original Message----- > > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Olivier MATZ > > Sent: Wednesday, December 3, 2014 10:42 PM > > To: Ananyev, Konstantin; Liu, Jijiang; dev@dpdk.org > > Subject: Re: [dpdk-dev] [PATCH v5 2/3] mbuf:add three TX ol_flags and repalce > > PKT_TX_VXLAN_CKSUM > > > > Hi Konstantin, > > > > On 12/03/2014 01:59 PM, Ananyev, Konstantin wrote: > > >> I still think having a flag IPV4 + another flag IP_CHECKSUM is not > > >> appropriate. > > > > > > Sorry, didn't get you here. > > > Are you talking about our discussion should PKT_TX_IP_CKSUM and > > PKT_TX_IPV4 be mutually exclusive or not? > > > > Yes > > > > >> I though Konstantin agreed on other flags, but I may have > > >> misunderstood: > > >> > > >> http://dpdk.org/ml/archives/dev/2014-November/009070.html > > > > > > In that mail, I was talking about my suggestion to make PKT_TX_IP_CKSUM, > > PKT_TX_IPV4 and PKT_TX_IPV6 to occupy 2 bits. > > > Something like: > > > #define PKT_TX_IP_CKSUM (1 << X) > > > #define PKT_TX_IPV6 (2 << X) > > > #define PKT_TX_IPV4 (3 << X) > > > > > > "Even better, if we can squeeze these 3 flags into 2 bits. > > > Would save us 2 bits, plus might be handy, as in the PMD you can do: > > > > > > switch (ol_flags & TX_L3_MASK) { > > > case TX_IPV4: > > > ... > > > break; > > > case TX_IPV6: > > > ... > > > break; > > > case TX_IP_CKSUM: > > > ... > > > break; > > > }" > > > > > > As you pointed out, it will break backward compatibility. > > > I agreed with that and self-NACKed it. > > > > ok, so we are back between: > > > > 1/ (Jijiang's patch) > > PKT_TX_IP_CKSUM /* packet is IPv4, and we want hw cksum */ > > PKT_TX_IPV6 /* packet is IPv6 */ > > PKT_TX_IPV4 /* packet is IPv4, and we don't want hw cksum */ > > > > with PKT_TX_IP_CKSUM and PKT_TX_IPV4 exclusive > > > > and > > > > 2/ > > PKT_TX_IP_CKSUM /* we want hw IP cksum */ > > PKT_TX_IPV6 /* packet is IPv6 */ > > PKT_TX_IPV4 /* packet is IPv4 */ > There is another bit flag named 'PKT_TX_IPV4_CSUM' which uses the > same bit of 'PKT_TX_IP_CSUM'. It is for identifying if ipv4 hardware > checksum offload is needed or not. Yes, 'PKT_TX_IPV4_CSUM is an alias to PKT_TX_IP_CKSUM and we are going to remove it. > It seems that we do not need 'PKT_TX_IPV6_CSUM'. No one even planned it. > 'PKT_TX_IPV4' and 'PKT_TX_IPV6' just indicates its packet type, and I guess > other features should not be contained in it, according to its name. > > So here I got the option 3: > PKT_TX_IPV4_CKSUM /* we want hw IPv4 cksum */ > PKT_TX_IPV6 /* packet is IPv6 */ > PKT_TX_IPV4 /* packet is IPv4 */ Hmm, and how this is different from what we have now in the Jijiang's patch? Except that you renamed PKT_TX_IP_CKSUM to PKT_TX_IPV4_CKSUM? Konstantin > > > > > with PKT_TX_IP_CKSUM implies PKT_TX_IPV4 > > > > > > Solution 2/ looks better from a user point of view. Anyone else has an opinion? > > > > Regards, > > Olivier > > Regards, > Helin
> -----Original Message----- > From: Liu, Jijiang > Sent: Thursday, December 04, 2014 2:08 AM > To: Olivier MATZ > Cc: Ananyev, Konstantin; dev@dpdk.org > Subject: RE: [dpdk-dev] [PATCH v5 2/3] mbuf:add three TX ol_flags and repalce PKT_TX_VXLAN_CKSUM > > Hi Olivier, > > > > -----Original Message----- > > From: Olivier MATZ [mailto:olivier.matz@6wind.com] > > Sent: Wednesday, December 3, 2014 10:42 PM > > To: Ananyev, Konstantin; Liu, Jijiang; dev@dpdk.org > > Subject: Re: [dpdk-dev] [PATCH v5 2/3] mbuf:add three TX ol_flags and repalce > > PKT_TX_VXLAN_CKSUM > > > > Hi Konstantin, > > > > On 12/03/2014 01:59 PM, Ananyev, Konstantin wrote: > > >> I still think having a flag IPV4 + another flag IP_CHECKSUM is not > > >> appropriate. > > > > > > Sorry, didn't get you here. > > > Are you talking about our discussion should PKT_TX_IP_CKSUM and > > PKT_TX_IPV4 be mutually exclusive or not? > > > > Yes > > > > >> I though Konstantin agreed on other flags, but I may have > > >> misunderstood: > > >> > > >> http://dpdk.org/ml/archives/dev/2014-November/009070.html > > > > > > In that mail, I was talking about my suggestion to make PKT_TX_IP_CKSUM, > > PKT_TX_IPV4 and PKT_TX_IPV6 to occupy 2 bits. > > > Something like: > > > #define PKT_TX_IP_CKSUM (1 << X) > > > #define PKT_TX_IPV6 (2 << X) > > > #define PKT_TX_IPV4 (3 << X) > > > > > > "Even better, if we can squeeze these 3 flags into 2 bits. > > > Would save us 2 bits, plus might be handy, as in the PMD you can do: > > > > > > switch (ol_flags & TX_L3_MASK) { > > > case TX_IPV4: > > > ... > > > break; > > > case TX_IPV6: > > > ... > > > break; > > > case TX_IP_CKSUM: > > > ... > > > break; > > > }" > > > > > > As you pointed out, it will break backward compatibility. > > > I agreed with that and self-NACKed it. > > > > ok, so we are back between: > > > > 1/ (Jijiang's patch) > > PKT_TX_IP_CKSUM /* packet is IPv4, and we want hw cksum */ > > PKT_TX_IPV6 /* packet is IPv6 */ > > PKT_TX_IPV4 /* packet is IPv4, and we don't want hw cksum */ > > > > with PKT_TX_IP_CKSUM and PKT_TX_IPV4 exclusive > > > > and > > > > 2/ > > PKT_TX_IP_CKSUM /* we want hw IP cksum */ > > PKT_TX_IPV6 /* packet is IPv6 */ > > PKT_TX_IPV4 /* packet is IPv4 */ > > > > with PKT_TX_IP_CKSUM implies PKT_TX_IPV4 > > > > > > Solution 2/ looks better from a user point of view. Anyone else has an opinion? > > Let's think about these IPv4/6 flags in terms of checksum and IP version/type, > > 1. For IPv6 > IP checksum is meaningful only for IPv4, so we define 'PKT_TX_IPV6 /* packet is IPv6 */' to tell driver/HW that this is IPV6 packet, > here we don't talk about the checksum for IPv6 as it is meaningless. Right? > > PKT_TX_IPV6 /* packet is IPv6 */ ------ IP type: v6; HW checksum: meaningless > > 2. For IPv4, > My patch: > > PKT_TX_IP_CKSUM /* packet is IPv4, and we want hw cksum */--------------------------IP type: v4; HW checksum: Yes > PKT_TX_IPV4 /* packet is IPv4, and we don't want hw cksum */ ----------------------- IP type: v4; HW checksum: No > > You want: > PKT_TX_IP_CKSUM /* we want hw IP cksum */-------------------------- IP type: v4; HW checksum: Yes > PKT_TX_IPV4 /* packet is IPv4*/ ------------------------ IP type: v4; HW checksum: yes or no? > driver/HW don't know, just know this is packet with IPv4 header. > HW checksum: meaningless?? Yep, that's why I also don't like that suggestion: PKT_TX_IPV4 itself doesn't contain all information. PMD will have to check PKT_TX_IP_CKSUM anyway. Konstantin > > > Regards, > > Olivier
Hi, 2014-12-04 10:23, Ananyev, Konstantin: > From: Liu, Jijiang > > From: Olivier MATZ [mailto:olivier.matz@6wind.com] > > > On 12/03/2014 01:59 PM, Ananyev, Konstantin wrote: > > > >> I still think having a flag IPV4 + another flag IP_CHECKSUM is not > > > >> appropriate. > > > > > > > > Sorry, didn't get you here. > > > > Are you talking about our discussion should PKT_TX_IP_CKSUM and > > > > PKT_TX_IPV4 be mutually exclusive or not? > > > > > > Yes > > > > > > >> I though Konstantin agreed on other flags, but I may have > > > >> misunderstood: > > > >> > > > >> http://dpdk.org/ml/archives/dev/2014-November/009070.html > > > > > > > > In that mail, I was talking about my suggestion to make PKT_TX_IP_CKSUM, > > > PKT_TX_IPV4 and PKT_TX_IPV6 to occupy 2 bits. > > > > Something like: > > > > #define PKT_TX_IP_CKSUM (1 << X) > > > > #define PKT_TX_IPV6 (2 << X) > > > > #define PKT_TX_IPV4 (3 << X) > > > > > > > > "Even better, if we can squeeze these 3 flags into 2 bits. > > > > Would save us 2 bits, plus might be handy, as in the PMD you can do: > > > > > > > > switch (ol_flags & TX_L3_MASK) { > > > > case TX_IPV4: > > > > ... > > > > break; > > > > case TX_IPV6: > > > > ... > > > > break; > > > > case TX_IP_CKSUM: > > > > ... > > > > break; > > > > }" > > > > > > > > As you pointed out, it will break backward compatibility. > > > > I agreed with that and self-NACKed it. > > > > > > ok, so we are back between: > > > > > > 1/ (Jijiang's patch) > > > PKT_TX_IP_CKSUM /* packet is IPv4, and we want hw cksum */ > > > PKT_TX_IPV6 /* packet is IPv6 */ > > > PKT_TX_IPV4 /* packet is IPv4, and we don't want hw cksum */ > > > > > > with PKT_TX_IP_CKSUM and PKT_TX_IPV4 exclusive > > > > > > and > > > > > > 2/ > > > PKT_TX_IP_CKSUM /* we want hw IP cksum */ > > > PKT_TX_IPV6 /* packet is IPv6 */ > > > PKT_TX_IPV4 /* packet is IPv4 */ > > > > > > with PKT_TX_IP_CKSUM implies PKT_TX_IPV4 > > > > > > > > > Solution 2/ looks better from a user point of view. Anyone else has an opinion? > > > > Let's think about these IPv4/6 flags in terms of checksum and IP version/type, > > > > 1. For IPv6 > > IP checksum is meaningful only for IPv4, so we define 'PKT_TX_IPV6 /* packet is IPv6 */' to tell driver/HW that this is IPV6 packet, > > here we don't talk about the checksum for IPv6 as it is meaningless. Right? > > > > PKT_TX_IPV6 /* packet is IPv6 */ ------ IP type: v6; HW checksum: meaningless > > > > 2. For IPv4, > > My patch: > > > > PKT_TX_IP_CKSUM /* packet is IPv4, and we want hw cksum */--------------------------IP type: v4; HW checksum: Yes > > PKT_TX_IPV4 /* packet is IPv4, and we don't want hw cksum */ ----------------------- IP type: v4; HW checksum: No > > > > You want: > > PKT_TX_IP_CKSUM /* we want hw IP cksum */-------------------------- IP type: v4; HW checksum: Yes > > PKT_TX_IPV4 /* packet is IPv4*/ ------------------------ IP type: v4; HW checksum: yes or no? > > driver/HW don't know, just know this is packet with IPv4 header. > > HW checksum: meaningless?? > > Yep, that's why I also don't like that suggestion: PKT_TX_IPV4 itself doesn't contain all information. > PMD will have to check PKT_TX_IP_CKSUM anyway. I prefer solution 2 because a flag should bring only 1 information. It's simply saner and could fit to more situations in the future.
Hi Thomas, > -----Original Message----- > From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com] > Sent: Thursday, December 04, 2014 10:45 AM > To: Ananyev, Konstantin; Olivier MATZ > Cc: dev@dpdk.org; Liu, Jijiang > Subject: Re: [dpdk-dev] [PATCH v5 2/3] mbuf:add three TX ol_flags and repalce PKT_TX_VXLAN_CKSUM > > Hi, > > 2014-12-04 10:23, Ananyev, Konstantin: > > From: Liu, Jijiang > > > From: Olivier MATZ [mailto:olivier.matz@6wind.com] > > > > On 12/03/2014 01:59 PM, Ananyev, Konstantin wrote: > > > > >> I still think having a flag IPV4 + another flag IP_CHECKSUM is not > > > > >> appropriate. > > > > > > > > > > Sorry, didn't get you here. > > > > > Are you talking about our discussion should PKT_TX_IP_CKSUM and > > > > > PKT_TX_IPV4 be mutually exclusive or not? > > > > > > > > Yes > > > > > > > > >> I though Konstantin agreed on other flags, but I may have > > > > >> misunderstood: > > > > >> > > > > >> http://dpdk.org/ml/archives/dev/2014-November/009070.html > > > > > > > > > > In that mail, I was talking about my suggestion to make PKT_TX_IP_CKSUM, > > > > PKT_TX_IPV4 and PKT_TX_IPV6 to occupy 2 bits. > > > > > Something like: > > > > > #define PKT_TX_IP_CKSUM (1 << X) > > > > > #define PKT_TX_IPV6 (2 << X) > > > > > #define PKT_TX_IPV4 (3 << X) > > > > > > > > > > "Even better, if we can squeeze these 3 flags into 2 bits. > > > > > Would save us 2 bits, plus might be handy, as in the PMD you can do: > > > > > > > > > > switch (ol_flags & TX_L3_MASK) { > > > > > case TX_IPV4: > > > > > ... > > > > > break; > > > > > case TX_IPV6: > > > > > ... > > > > > break; > > > > > case TX_IP_CKSUM: > > > > > ... > > > > > break; > > > > > }" > > > > > > > > > > As you pointed out, it will break backward compatibility. > > > > > I agreed with that and self-NACKed it. > > > > > > > > ok, so we are back between: > > > > > > > > 1/ (Jijiang's patch) > > > > PKT_TX_IP_CKSUM /* packet is IPv4, and we want hw cksum */ > > > > PKT_TX_IPV6 /* packet is IPv6 */ > > > > PKT_TX_IPV4 /* packet is IPv4, and we don't want hw cksum */ > > > > > > > > with PKT_TX_IP_CKSUM and PKT_TX_IPV4 exclusive > > > > > > > > and > > > > > > > > 2/ > > > > PKT_TX_IP_CKSUM /* we want hw IP cksum */ > > > > PKT_TX_IPV6 /* packet is IPv6 */ > > > > PKT_TX_IPV4 /* packet is IPv4 */ > > > > > > > > with PKT_TX_IP_CKSUM implies PKT_TX_IPV4 > > > > > > > > > > > > Solution 2/ looks better from a user point of view. Anyone else has an opinion? > > > > > > Let's think about these IPv4/6 flags in terms of checksum and IP version/type, > > > > > > 1. For IPv6 > > > IP checksum is meaningful only for IPv4, so we define 'PKT_TX_IPV6 /* packet is IPv6 */' to tell driver/HW that this is IPV6 > packet, > > > here we don't talk about the checksum for IPv6 as it is meaningless. Right? > > > > > > PKT_TX_IPV6 /* packet is IPv6 */ ------ IP type: v6; HW checksum: meaningless > > > > > > 2. For IPv4, > > > My patch: > > > > > > PKT_TX_IP_CKSUM /* packet is IPv4, and we want hw cksum */--------------------------IP type: v4; HW checksum: Yes > > > PKT_TX_IPV4 /* packet is IPv4, and we don't want hw cksum */ ----------------------- IP type: v4; HW checksum: No > > > > > > You want: > > > PKT_TX_IP_CKSUM /* we want hw IP cksum */-------------------------- IP type: v4; HW checksum: Yes > > > PKT_TX_IPV4 /* packet is IPv4*/ ------------------------ IP type: v4; HW checksum: yes or no? > > > driver/HW don't know, just know this is packet with IPv4 header. > > > HW checksum: meaningless?? > > > > Yep, that's why I also don't like that suggestion: PKT_TX_IPV4 itself doesn't contain all information. > > PMD will have to check PKT_TX_IP_CKSUM anyway. > > I prefer solution 2 because a flag should bring only 1 information. Why is that? For example in mbuf we already have a flag that brings 2 things: PKT_TX_IP_CKSUM /* packet is IPv4, and we want hw cksum */ If it would be possible to compress 10 meanings into 1 bit, I would happily do that. Unfortunately, it is rarely possible :) > It's simply saner and could fit to more situations in the future. Could you give an example of such situation? I personally couldn't come up with the case where #2 would have any real advantage. Konstantin > > -- > Thomas
Hi, On 12/04/2014 11:19 AM, Ananyev, Konstantin wrote: >>> 1/ (Jijiang's patch) >>> PKT_TX_IP_CKSUM /* packet is IPv4, and we want hw cksum */ >>> PKT_TX_IPV6 /* packet is IPv6 */ >>> PKT_TX_IPV4 /* packet is IPv4, and we don't want hw cksum */ >>> >>> with PKT_TX_IP_CKSUM and PKT_TX_IPV4 exclusive >>> >>> and >>> >>> 2/ >>> PKT_TX_IP_CKSUM /* we want hw IP cksum */ >>> PKT_TX_IPV6 /* packet is IPv6 */ >>> PKT_TX_IPV4 /* packet is IPv4 */ >> There is another bit flag named 'PKT_TX_IPV4_CSUM' which uses the >> same bit of 'PKT_TX_IP_CSUM'. It is for identifying if ipv4 hardware >> checksum offload is needed or not. > > Yes, 'PKT_TX_IPV4_CSUM is an alias to PKT_TX_IP_CKSUM and we are going to remove it. > >> It seems that we do not need 'PKT_TX_IPV6_CSUM'. > > No one even planned it. > >> 'PKT_TX_IPV4' and 'PKT_TX_IPV6' just indicates its packet type, and I guess >> other features should not be contained in it, according to its name. >> >> So here I got the option 3: >> PKT_TX_IPV4_CKSUM /* we want hw IPv4 cksum */ >> PKT_TX_IPV6 /* packet is IPv6 */ >> PKT_TX_IPV4 /* packet is IPv4 */ > > Hmm, and how this is different from what we have now in the Jijiang's patch? > Except that you renamed PKT_TX_IP_CKSUM to PKT_TX_IPV4_CKSUM? I think it's more like solution 2 with a renaming. And it is more coherent to always have "IPV4" on all flag names. Regards, Olivier
Hi, On 12/04/2014 12:03 PM, Ananyev, Konstantin wrote: >>>>> 1/ (Jijiang's patch) >>>>> PKT_TX_IP_CKSUM /* packet is IPv4, and we want hw cksum */ >>>>> PKT_TX_IPV6 /* packet is IPv6 */ >>>>> PKT_TX_IPV4 /* packet is IPv4, and we don't want hw cksum */ >>>>> >>>>> with PKT_TX_IP_CKSUM and PKT_TX_IPV4 exclusive >>>>> >>>>> and >>>>> >>>>> 2/ >>>>> PKT_TX_IP_CKSUM /* we want hw IP cksum */ >>>>> PKT_TX_IPV6 /* packet is IPv6 */ >>>>> PKT_TX_IPV4 /* packet is IPv4 */ >>>>> >>>>> with PKT_TX_IP_CKSUM implies PKT_TX_IPV4 >>>>> >>>>> >>>>> Solution 2/ looks better from a user point of view. Anyone else has an opinion? >>>> >>>> Let's think about these IPv4/6 flags in terms of checksum and IP version/type, >>>> >>>> 1. For IPv6 >>>> IP checksum is meaningful only for IPv4, so we define 'PKT_TX_IPV6 /* packet is IPv6 */' to tell driver/HW that this is IPV6 >> packet, >>>> here we don't talk about the checksum for IPv6 as it is meaningless. Right? >>>> >>>> PKT_TX_IPV6 /* packet is IPv6 */ ------ IP type: v6; HW checksum: meaningless >>>> >>>> 2. For IPv4, >>>> My patch: >>>> >>>> PKT_TX_IP_CKSUM /* packet is IPv4, and we want hw cksum */--------------------------IP type: v4; HW checksum: Yes >>>> PKT_TX_IPV4 /* packet is IPv4, and we don't want hw cksum */ ----------------------- IP type: v4; HW checksum: No >>>> >>>> You want: >>>> PKT_TX_IP_CKSUM /* we want hw IP cksum */-------------------------- IP type: v4; HW checksum: Yes >>>> PKT_TX_IPV4 /* packet is IPv4*/ ------------------------ IP type: v4; HW checksum: yes or no? >>>> driver/HW don't know, just know this is packet with IPv4 header. >>>> HW checksum: meaningless?? >>> >>> Yep, that's why I also don't like that suggestion: PKT_TX_IPV4 itself doesn't contain all information. >>> PMD will have to check PKT_TX_IP_CKSUM anyway. >> >> I prefer solution 2 because a flag should bring only 1 information. > > Why is that? For example in mbuf we already have a flag that brings 2 things: > PKT_TX_IP_CKSUM /* packet is IPv4, and we want hw cksum */ For the user, it's clearer to have one information in a flag. If you just look at the name of the flag, the natural meaning is 2/, else we would need to rename them in: PKT_TX_IPV4_CKSUM PKT_TX_IPV4_NO_CKSUM > If it would be possible to compress 10 meanings into 1 bit, I would happily do that. > Unfortunately, it is rarely possible :) > >> It's simply saner and could fit to more situations in the future. > > Could you give an example of such situation? > I personally couldn't come up with the case where #2 would have any real advantage. in solution 2/, PKT_TX_IP_CKSUM implies PKT_TX_IPV4 so checking PKT_TX_IP_CKSUM is still enough in drivers. In the driver, it is also simpler. With solution 1/: /* check if we need ipcsum */ if (flags & PKT_TX_IP_CKSUM) /* check if packet is ipv4, may be needed to set a hw field */ if (flags & (PKT_TX_IP_CKSUM|PKT_TX_IPV4)) With solution 2/ /* check if we need ipcsum */ if (flags & PKT_TX_IP_CKSUM) /* check if packet is ipv4, may be needed to set a hw field */ if (flags & PKT_TX_IPV4) I agree it can looks like a detail, but I really think it's important to have the most logical and straightforward api for mbuf, as it's the core of DPDK. Regards, Olivier
> -----Original Message----- > From: Olivier MATZ [mailto:olivier.matz@6wind.com] > Sent: Thursday, December 04, 2014 1:48 PM > To: Ananyev, Konstantin; Zhang, Helin; Liu, Jijiang; dev@dpdk.org > Subject: Re: [dpdk-dev] [PATCH v5 2/3] mbuf:add three TX ol_flags and repalce PKT_TX_VXLAN_CKSUM > > Hi, > > On 12/04/2014 11:19 AM, Ananyev, Konstantin wrote: > >>> 1/ (Jijiang's patch) > >>> PKT_TX_IP_CKSUM /* packet is IPv4, and we want hw cksum */ > >>> PKT_TX_IPV6 /* packet is IPv6 */ > >>> PKT_TX_IPV4 /* packet is IPv4, and we don't want hw cksum */ > >>> > >>> with PKT_TX_IP_CKSUM and PKT_TX_IPV4 exclusive > >>> > >>> and > >>> > >>> 2/ > >>> PKT_TX_IP_CKSUM /* we want hw IP cksum */ > >>> PKT_TX_IPV6 /* packet is IPv6 */ > >>> PKT_TX_IPV4 /* packet is IPv4 */ > >> There is another bit flag named 'PKT_TX_IPV4_CSUM' which uses the > >> same bit of 'PKT_TX_IP_CSUM'. It is for identifying if ipv4 hardware > >> checksum offload is needed or not. > > > > Yes, 'PKT_TX_IPV4_CSUM is an alias to PKT_TX_IP_CKSUM and we are going to remove it. > > > >> It seems that we do not need 'PKT_TX_IPV6_CSUM'. > > > > No one even planned it. > > > >> 'PKT_TX_IPV4' and 'PKT_TX_IPV6' just indicates its packet type, and I guess > >> other features should not be contained in it, according to its name. > >> > >> So here I got the option 3: > >> PKT_TX_IPV4_CKSUM /* we want hw IPv4 cksum */ > >> PKT_TX_IPV6 /* packet is IPv6 */ > >> PKT_TX_IPV4 /* packet is IPv4 */ > > > > Hmm, and how this is different from what we have now in the Jijiang's patch? > > Except that you renamed PKT_TX_IP_CKSUM to PKT_TX_IPV4_CKSUM? > > I think it's more like solution 2 with a renaming. And it is more > coherent to always have "IPV4" on all flag names. About renaming PKT_TX_IP_CKSUM to PKT_TX_IPV4_CKSUM: I don't mind. But that means we would have to update all the drivers and apps that using PKT_TX_IP_CKSUM. Again there are customers that using that flag in their apps right now. They would have to change it too. For me - too much hassle for such small nit. Konstantin > > Regards, > Olivier
> -----Original Message----- > From: Olivier MATZ [mailto:olivier.matz@6wind.com] > Sent: Thursday, December 04, 2014 1:51 PM > To: Ananyev, Konstantin; Thomas Monjalon > Cc: dev@dpdk.org; Liu, Jijiang > Subject: Re: [dpdk-dev] [PATCH v5 2/3] mbuf:add three TX ol_flags and repalce PKT_TX_VXLAN_CKSUM > > Hi, > > On 12/04/2014 12:03 PM, Ananyev, Konstantin wrote: > >>>>> 1/ (Jijiang's patch) > >>>>> PKT_TX_IP_CKSUM /* packet is IPv4, and we want hw cksum */ > >>>>> PKT_TX_IPV6 /* packet is IPv6 */ > >>>>> PKT_TX_IPV4 /* packet is IPv4, and we don't want hw cksum */ > >>>>> > >>>>> with PKT_TX_IP_CKSUM and PKT_TX_IPV4 exclusive > >>>>> > >>>>> and > >>>>> > >>>>> 2/ > >>>>> PKT_TX_IP_CKSUM /* we want hw IP cksum */ > >>>>> PKT_TX_IPV6 /* packet is IPv6 */ > >>>>> PKT_TX_IPV4 /* packet is IPv4 */ > >>>>> > >>>>> with PKT_TX_IP_CKSUM implies PKT_TX_IPV4 > >>>>> > >>>>> > >>>>> Solution 2/ looks better from a user point of view. Anyone else has an opinion? > >>>> > >>>> Let's think about these IPv4/6 flags in terms of checksum and IP version/type, > >>>> > >>>> 1. For IPv6 > >>>> IP checksum is meaningful only for IPv4, so we define 'PKT_TX_IPV6 /* packet is IPv6 */' to tell driver/HW that this is IPV6 > >> packet, > >>>> here we don't talk about the checksum for IPv6 as it is meaningless. Right? > >>>> > >>>> PKT_TX_IPV6 /* packet is IPv6 */ ------ IP type: v6; HW checksum: meaningless > >>>> > >>>> 2. For IPv4, > >>>> My patch: > >>>> > >>>> PKT_TX_IP_CKSUM /* packet is IPv4, and we want hw cksum */--------------------------IP type: v4; HW checksum: Yes > >>>> PKT_TX_IPV4 /* packet is IPv4, and we don't want hw cksum */ ----------------------- IP type: v4; HW checksum: No > >>>> > >>>> You want: > >>>> PKT_TX_IP_CKSUM /* we want hw IP cksum */-------------------------- IP type: v4; HW checksum: Yes > >>>> PKT_TX_IPV4 /* packet is IPv4*/ ------------------------ IP type: v4; HW checksum: yes or no? > >>>> driver/HW don't know, just know this is packet with IPv4 header. > >>>> HW checksum: meaningless?? > >>> > >>> Yep, that's why I also don't like that suggestion: PKT_TX_IPV4 itself doesn't contain all information. > >>> PMD will have to check PKT_TX_IP_CKSUM anyway. > >> > >> I prefer solution 2 because a flag should bring only 1 information. > > > > Why is that? For example in mbuf we already have a flag that brings 2 things: > > PKT_TX_IP_CKSUM /* packet is IPv4, and we want hw cksum */ > > For the user, it's clearer to have one information in a flag. > If you just look at the name of the flag, the natural meaning is 2/, > else we would need to rename them in: > PKT_TX_IPV4_CKSUM > PKT_TX_IPV4_NO_CKSUM > > > If it would be possible to compress 10 meanings into 1 bit, I would happily do that. > > Unfortunately, it is rarely possible :) > > > >> It's simply saner and could fit to more situations in the future. > > > > Could you give an example of such situation? > > I personally couldn't come up with the case where #2 would have any real advantage. > > in solution 2/, PKT_TX_IP_CKSUM implies PKT_TX_IPV4 so checking > PKT_TX_IP_CKSUM is still enough in drivers. Both 1 and 2 seems backward compatible. > > In the driver, it is also simpler. With solution 1/: > > /* check if we need ipcsum */ > if (flags & PKT_TX_IP_CKSUM) > > /* check if packet is ipv4, may be needed to set a hw field */ > if (flags & (PKT_TX_IP_CKSUM|PKT_TX_IPV4)) Do you really mean 1 here? When all 3 flags are mutually exclusive? If so, it doesn't look right. For 1 both (PKT_TX_IP_CKSUM|PKT_TX_IPV4) should never be up. > > > With solution 2/ > > /* check if we need ipcsum */ > if (flags & PKT_TX_IP_CKSUM) > > /* check if packet is ipv4, may be needed to set a hw field */ > if (flags & PKT_TX_IPV4) The thing is that it wouldn't be possible with FVL driver - it has to setup mutually exclusive fields for these 2 cases: PKT_TX_IP_CKSUM - ipv4 with HW checksum PKT_TX_IPV4 - ipv4 without HW checksum So with #2, driver has either: if (flags & PKT_TX_IP_CKSUM) {...} else if (flags & PKT_TX_IPV4) {...} And always keep condition for PKT_TX_IP_CKSUM first. Or do: if (flags & PKT_TX_IPV4) {...} if (flags & PKT_TX_IP_CKSUM) {...} and in that case always keep condition for PKT_TX_IP_CKSUM last, so it always overwrite PKT_TX_IPV4 settings. Basically with #2 PKT_TX_IPV4 is not enough to make a decision, even if it is set, we'll have to check for PKT_TX_IP_CKSUM anyway. While with 1 we can put them in any order, both: If (flags & PKT_TX_IP_CKSUM) {...} else if (flags & PKT_TX_IPV4) {...} And If (flags & PKT_TX_IPV4) {...} else if (flags & PKT_TX_IP_CKSUM) {...} Will work. Konstantin > > > I agree it can looks like a detail, but I really think it's important > to have the most logical and straightforward api for mbuf, as it's > the core of DPDK. > > Regards, > Olivier
> -----Original Message----- > From: Ananyev, Konstantin > Sent: Thursday, December 4, 2014 6:20 PM > To: Zhang, Helin; Olivier MATZ; Liu, Jijiang; dev@dpdk.org > Subject: RE: [dpdk-dev] [PATCH v5 2/3] mbuf:add three TX ol_flags and repalce > PKT_TX_VXLAN_CKSUM > > Hi Helin, > > > -----Original Message----- > > From: Zhang, Helin > > Sent: Thursday, December 04, 2014 6:52 AM > > To: Olivier MATZ; Ananyev, Konstantin; Liu, Jijiang; dev@dpdk.org > > Subject: RE: [dpdk-dev] [PATCH v5 2/3] mbuf:add three TX ol_flags and > > repalce PKT_TX_VXLAN_CKSUM > > > > > > > > > -----Original Message----- > > > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Olivier MATZ > > > Sent: Wednesday, December 3, 2014 10:42 PM > > > To: Ananyev, Konstantin; Liu, Jijiang; dev@dpdk.org > > > Subject: Re: [dpdk-dev] [PATCH v5 2/3] mbuf:add three TX ol_flags > > > and repalce PKT_TX_VXLAN_CKSUM > > > > > > Hi Konstantin, > > > > > > On 12/03/2014 01:59 PM, Ananyev, Konstantin wrote: > > > >> I still think having a flag IPV4 + another flag IP_CHECKSUM is > > > >> not appropriate. > > > > > > > > Sorry, didn't get you here. > > > > Are you talking about our discussion should PKT_TX_IP_CKSUM and > > > PKT_TX_IPV4 be mutually exclusive or not? > > > > > > Yes > > > > > > >> I though Konstantin agreed on other flags, but I may have > > > >> misunderstood: > > > >> > > > >> http://dpdk.org/ml/archives/dev/2014-November/009070.html > > > > > > > > In that mail, I was talking about my suggestion to make > > > > PKT_TX_IP_CKSUM, > > > PKT_TX_IPV4 and PKT_TX_IPV6 to occupy 2 bits. > > > > Something like: > > > > #define PKT_TX_IP_CKSUM (1 << X) > > > > #define PKT_TX_IPV6 (2 << X) > > > > #define PKT_TX_IPV4 (3 << X) > > > > > > > > "Even better, if we can squeeze these 3 flags into 2 bits. > > > > Would save us 2 bits, plus might be handy, as in the PMD you can do: > > > > > > > > switch (ol_flags & TX_L3_MASK) { > > > > case TX_IPV4: > > > > ... > > > > break; > > > > case TX_IPV6: > > > > ... > > > > break; > > > > case TX_IP_CKSUM: > > > > ... > > > > break; > > > > }" > > > > > > > > As you pointed out, it will break backward compatibility. > > > > I agreed with that and self-NACKed it. > > > > > > ok, so we are back between: > > > > > > 1/ (Jijiang's patch) > > > PKT_TX_IP_CKSUM /* packet is IPv4, and we want hw cksum */ > > > PKT_TX_IPV6 /* packet is IPv6 */ > > > PKT_TX_IPV4 /* packet is IPv4, and we don't want hw cksum */ > > > > > > with PKT_TX_IP_CKSUM and PKT_TX_IPV4 exclusive > > > > > > and > > > > > > 2/ > > > PKT_TX_IP_CKSUM /* we want hw IP cksum */ > > > PKT_TX_IPV6 /* packet is IPv6 */ > > > PKT_TX_IPV4 /* packet is IPv4 */ > > There is another bit flag named 'PKT_TX_IPV4_CSUM' which uses the same > > bit of 'PKT_TX_IP_CSUM'. It is for identifying if ipv4 hardware > > checksum offload is needed or not. > > Yes, 'PKT_TX_IPV4_CSUM is an alias to PKT_TX_IP_CKSUM and we are going to > remove it. > > > It seems that we do not need 'PKT_TX_IPV6_CSUM'. > > No one even planned it. > > > 'PKT_TX_IPV4' and 'PKT_TX_IPV6' just indicates its packet type, and I > > guess other features should not be contained in it, according to its name. > > > > So here I got the option 3: > > PKT_TX_IPV4_CKSUM /* we want hw IPv4 cksum */ > > PKT_TX_IPV6 /* packet is IPv6 */ > > PKT_TX_IPV4 /* packet is IPv4 */ > > Hmm, and how this is different from what we have now in the Jijiang's patch? > Except that you renamed PKT_TX_IP_CKSUM to PKT_TX_IPV4_CKSUM? The comments are different, PKT_TX_IPV4 has different meanings. Regards, Helin > > Konstantin > > > > > > > > > with PKT_TX_IP_CKSUM implies PKT_TX_IPV4 > > > > > > > > > Solution 2/ looks better from a user point of view. Anyone else has an > opinion? > > > > > > Regards, > > > Olivier > > > > Regards, > > Helin
> -----Original Message----- > From: Ananyev, Konstantin > Sent: Friday, December 5, 2014 6:56 AM > To: Olivier MATZ; Thomas Monjalon > Cc: dev@dpdk.org; Liu, Jijiang > Subject: RE: [dpdk-dev] [PATCH v5 2/3] mbuf:add three TX ol_flags and repalce > PKT_TX_VXLAN_CKSUM > > > > > -----Original Message----- > > From: Olivier MATZ [mailto:olivier.matz@6wind.com] > > Sent: Thursday, December 04, 2014 1:51 PM > > To: Ananyev, Konstantin; Thomas Monjalon > > Cc: dev@dpdk.org; Liu, Jijiang > > Subject: Re: [dpdk-dev] [PATCH v5 2/3] mbuf:add three TX ol_flags and > > repalce PKT_TX_VXLAN_CKSUM > > > > Hi, > > > > On 12/04/2014 12:03 PM, Ananyev, Konstantin wrote: > > >>>>> 1/ (Jijiang's patch) > > >>>>> PKT_TX_IP_CKSUM /* packet is IPv4, and we want hw cksum */ > > >>>>> PKT_TX_IPV6 /* packet is IPv6 */ > > >>>>> PKT_TX_IPV4 /* packet is IPv4, and we don't want hw cksum */ > > >>>>> > > >>>>> with PKT_TX_IP_CKSUM and PKT_TX_IPV4 exclusive > > >>>>> > > >>>>> and > > >>>>> > > >>>>> 2/ > > >>>>> PKT_TX_IP_CKSUM /* we want hw IP cksum */ > > >>>>> PKT_TX_IPV6 /* packet is IPv6 */ > > >>>>> PKT_TX_IPV4 /* packet is IPv4 */ > > >>>>> > > >>>>> with PKT_TX_IP_CKSUM implies PKT_TX_IPV4 > > >>>>> > > >>>>> > > >>>>> Solution 2/ looks better from a user point of view. Anyone else has an > opinion? > > >>>> > > >>>> Let's think about these IPv4/6 flags in terms of checksum and IP > > >>>> version/type, > > >>>> > > >>>> 1. For IPv6 > > >>>> IP checksum is meaningful only for IPv4, so we define 'PKT_TX_IPV6 /* > packet is IPv6 */' to tell driver/HW that this is IPV6 > > >> packet, > > >>>> here we don't talk about the checksum for IPv6 as it is meaningless. > Right? > > >>>> > > >>>> PKT_TX_IPV6 /* packet is IPv6 */ ------ IP type: v6; HW checksum: > meaningless > > >>>> > > >>>> 2. For IPv4, > > >>>> My patch: > > >>>> > > >>>> PKT_TX_IP_CKSUM /* packet is IPv4, and we want hw cksum */----------- > ---------------IP type: v4; HW checksum: Yes > > >>>> PKT_TX_IPV4 /* packet is IPv4, and we don't want hw cksum */ -------- > --------------- IP type: v4; HW checksum: No > > >>>> > > >>>> You want: > > >>>> PKT_TX_IP_CKSUM /* we want hw IP cksum */-------------------------- IP > type: v4; HW checksum: Yes > > >>>> PKT_TX_IPV4 /* packet is IPv4*/ ------------------------ IP type: v4; HW > checksum: yes or no? > > >>>> driver/HW don't > know, just know this is packet with IPv4 header. > > >>>> HW checksum: > meaningless?? > > >>> > > >>> Yep, that's why I also don't like that suggestion: PKT_TX_IPV4 itself doesn't > contain all information. > > >>> PMD will have to check PKT_TX_IP_CKSUM anyway. > > >> > > >> I prefer solution 2 because a flag should bring only 1 information. > > > > > > Why is that? For example in mbuf we already have a flag that brings 2 things: > > > PKT_TX_IP_CKSUM /* packet is IPv4, and we want hw cksum */ > > > > For the user, it's clearer to have one information in a flag. > > If you just look at the name of the flag, the natural meaning is 2/, > > else we would need to rename them in: > > PKT_TX_IPV4_CKSUM > > PKT_TX_IPV4_NO_CKSUM > > > > > If it would be possible to compress 10 meanings into 1 bit, I would happily do > that. > > > Unfortunately, it is rarely possible :) > > > > > >> It's simply saner and could fit to more situations in the future. > > > > > > Could you give an example of such situation? > > > I personally couldn't come up with the case where #2 would have any real > advantage. > > > > in solution 2/, PKT_TX_IP_CKSUM implies PKT_TX_IPV4 so checking > > PKT_TX_IP_CKSUM is still enough in drivers. > > Both 1 and 2 seems backward compatible. > > > > > In the driver, it is also simpler. With solution 1/: > > > > /* check if we need ipcsum */ > > if (flags & PKT_TX_IP_CKSUM) > > > > /* check if packet is ipv4, may be needed to set a hw field */ if > > (flags & (PKT_TX_IP_CKSUM|PKT_TX_IPV4)) > > Do you really mean 1 here? When all 3 flags are mutually exclusive? > If so, it doesn't look right. For 1 both (PKT_TX_IP_CKSUM|PKT_TX_IPV4) should > never be up. > > > > > > > With solution 2/ > > > > /* check if we need ipcsum */ > > if (flags & PKT_TX_IP_CKSUM) > > > > /* check if packet is ipv4, may be needed to set a hw field */ if > > (flags & PKT_TX_IPV4) > > The thing is that it wouldn't be possible with FVL driver - it has to setup mutually > exclusive fields for these 2 cases: > PKT_TX_IP_CKSUM - ipv4 with HW checksum > PKT_TX_IPV4 - ipv4 without HW checksum > > So with #2, driver has either: > if (flags & PKT_TX_IP_CKSUM) {...} else if (flags & PKT_TX_IPV4) {...} And always > keep condition for PKT_TX_IP_CKSUM first. > Or do: > if (flags & PKT_TX_IPV4) {...} if (flags & PKT_TX_IP_CKSUM) {...} and in that case > always keep condition for PKT_TX_IP_CKSUM last, so it always overwrite > PKT_TX_IPV4 settings. > > Basically with #2 PKT_TX_IPV4 is not enough to make a decision, even if it is set, > we'll have to check for PKT_TX_IP_CKSUM anyway. > > While with 1 we can put them in any order, both: > If (flags & PKT_TX_IP_CKSUM) {...} else if (flags & PKT_TX_IPV4) {...} And If (flags > & PKT_TX_IPV4) {...} else if (flags & PKT_TX_IP_CKSUM) {...} Will work. > > Konstantin Yes. I agree. PKT_TX_IPV4 /* packet is IPv4 */ This flag don't have too much offload meaning for TX side, because we can't use this information to set transmit descriptor ( or set offload register ) precisely , so it is not a real offload flag. PKT_TX_IPV4 /* packet is IPv4, and we don't want hw cksum */ It is a offload flag, we can use this flag to set transmit descriptor precisely. Yes, the comments are different, the PKT_TX_IPV4 has different meanings, but we have to consider which comment will affect if offload work or not. BTW, we pay too much time on this topic... > > > > > > I agree it can looks like a detail, but I really think it's important > > to have the most logical and straightforward api for mbuf, as it's the > > core of DPDK. > > > > Regards, > > Olivier
Hi, On 12/02/2014 04:06 PM, Jijiang Liu wrote: > Replace PKT_TX_VXLAN_CKSUM with PKT_TX_UDP_TUNNEL_PKT in order to indicate a packet is an UDP tunneling packet, and introduce 3 TX offload flags for outer IP TX checksum, which are PKT_TX_OUTER_IP_CKSUM, PKT_TX_OUTER_IPV4 and PKT_TX_OUTER_IPV6 respectively;Rework csum forward engine and i40e PMD due to these changes. > > Signed-off-by: Jijiang Liu <jijiang.liu@intel.com> > --- > app/test-pmd/csumonly.c | 9 +++++++-- > lib/librte_mbuf/rte_mbuf.c | 7 ++++++- > lib/librte_mbuf/rte_mbuf.h | 11 ++++++++++- > lib/librte_pmd_i40e/i40e_rxtx.c | 6 +++--- > 4 files changed, 26 insertions(+), 7 deletions(-) > As we need to conclude on this: Acked-by: Olivier Matz <olivier.matz@6wind.com> Just few minor comments below: > diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h > index cbadf8e..6eb898f 100644 > --- a/lib/librte_mbuf/rte_mbuf.h > +++ b/lib/librte_mbuf/rte_mbuf.h > @@ -118,7 +118,7 @@ extern "C" { > */ > #define PKT_TX_TCP_SEG (1ULL << 49) > > -#define PKT_TX_VXLAN_CKSUM (1ULL << 50) /**< TX checksum of VXLAN computed by NIC */ > +#define PKT_TX_UDP_TUNNEL_PKT (1ULL << 50) /**< TX packet is an UDP tunneling packet */ > #define PKT_TX_IEEE1588_TMST (1ULL << 51) /**< TX IEEE1588 packet to timestamp. */ We could replace the comment by: "Tell the NIC it's a UDP tunneled packet. It must be specified when using hw outer checksum offload (PKT_TX_OUTER_IP_CKSUM)" > > /** > @@ -149,6 +149,15 @@ extern "C" { > > #define PKT_TX_VLAN_PKT (1ULL << 57) /**< TX packet is a 802.1q VLAN packet. */ > > +/** Outer IP checksum of TX packet, computed by NIC for tunneling packet */ > +#define PKT_TX_OUTER_IP_CKSUM (1ULL << 58) Maybe add: "The tunnel type must also be specified, ex: PKT_TX_UDP_TUNNEL_PKT" (altought I don't understand why the hw need this info) Regards, Olivier
diff --git a/app/test-pmd/csumonly.c b/app/test-pmd/csumonly.c index d8c080a..9094967 100644 --- a/app/test-pmd/csumonly.c +++ b/app/test-pmd/csumonly.c @@ -257,7 +257,7 @@ process_outer_cksums(void *outer_l3_hdr, uint16_t outer_ethertype, uint64_t ol_flags = 0; if (testpmd_ol_flags & TESTPMD_TX_OFFLOAD_VXLAN_CKSUM) - ol_flags |= PKT_TX_VXLAN_CKSUM; + ol_flags |= PKT_TX_UDP_TUNNEL_PKT; if (outer_ethertype == _htons(ETHER_TYPE_IPv4)) { ipv4_hdr->hdr_checksum = 0; @@ -470,7 +470,12 @@ pkt_burst_checksum_forward(struct fwd_stream *fs) { PKT_TX_UDP_CKSUM, PKT_TX_L4_MASK }, { PKT_TX_TCP_CKSUM, PKT_TX_L4_MASK }, { PKT_TX_SCTP_CKSUM, PKT_TX_L4_MASK }, - { PKT_TX_VXLAN_CKSUM, PKT_TX_VXLAN_CKSUM }, + { PKT_TX_UDP_TUNNEL_PKT, PKT_TX_UDP_TUNNEL_PKT }, + { PKT_TX_IPV4, PKT_TX_IPV4 }, + { PKT_TX_IPV6, PKT_TX_IPV6 }, + { PKT_TX_OUTER_IP_CKSUM, PKT_TX_OUTER_IP_CKSUM }, + { PKT_TX_OUTER_IPV4, PKT_TX_OUTER_IPV4 }, + { PKT_TX_OUTER_IPV6, PKT_TX_OUTER_IPV6 }, { PKT_TX_TCP_SEG, PKT_TX_TCP_SEG }, }; unsigned j; diff --git a/lib/librte_mbuf/rte_mbuf.c b/lib/librte_mbuf/rte_mbuf.c index 87c2963..1b14e02 100644 --- a/lib/librte_mbuf/rte_mbuf.c +++ b/lib/librte_mbuf/rte_mbuf.c @@ -240,8 +240,13 @@ const char *rte_get_tx_ol_flag_name(uint64_t mask) case PKT_TX_SCTP_CKSUM: return "PKT_TX_SCTP_CKSUM"; case PKT_TX_UDP_CKSUM: return "PKT_TX_UDP_CKSUM"; case PKT_TX_IEEE1588_TMST: return "PKT_TX_IEEE1588_TMST"; - case PKT_TX_VXLAN_CKSUM: return "PKT_TX_VXLAN_CKSUM"; + case PKT_TX_UDP_TUNNEL_PKT: return "PKT_TX_UDP_TUNNEL_PKT"; case PKT_TX_TCP_SEG: return "PKT_TX_TCP_SEG"; + case PKT_TX_IPV4: return "PKT_TX_IPV4"; + case PKT_TX_IPV6: return "PKT_TX_IPV6"; + case PKT_TX_OUTER_IP_CKSUM: return "PKT_TX_OUTER_IP_CKSUM"; + case PKT_TX_OUTER_IPV4: return "PKT_TX_OUTER_IPV4"; + case PKT_TX_OUTER_IPV6: return "PKT_TX_OUTER_IPV6"; default: return NULL; } } diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h index cbadf8e..6eb898f 100644 --- a/lib/librte_mbuf/rte_mbuf.h +++ b/lib/librte_mbuf/rte_mbuf.h @@ -118,7 +118,7 @@ extern "C" { */ #define PKT_TX_TCP_SEG (1ULL << 49) -#define PKT_TX_VXLAN_CKSUM (1ULL << 50) /**< TX checksum of VXLAN computed by NIC */ +#define PKT_TX_UDP_TUNNEL_PKT (1ULL << 50) /**< TX packet is an UDP tunneling packet */ #define PKT_TX_IEEE1588_TMST (1ULL << 51) /**< TX IEEE1588 packet to timestamp. */ /** @@ -149,6 +149,15 @@ extern "C" { #define PKT_TX_VLAN_PKT (1ULL << 57) /**< TX packet is a 802.1q VLAN packet. */ +/** Outer IP checksum of TX packet, computed by NIC for tunneling packet */ +#define PKT_TX_OUTER_IP_CKSUM (1ULL << 58) + +/** Packet is outer IPv4 without requiring IP checksum offload for tunneling packet. */ +#define PKT_TX_OUTER_IPV4 (1ULL << 59) + +/** Tell the NIC it's an outer IPv6 packet for tunneling packet */ +#define PKT_TX_OUTER_IPV6 (1ULL << 60) + /* Use final bit of flags to indicate a control mbuf */ #define CTRL_MBUF_FLAG (1ULL << 63) /**< Mbuf contains control data */ diff --git a/lib/librte_pmd_i40e/i40e_rxtx.c b/lib/librte_pmd_i40e/i40e_rxtx.c index 2d2ef04..078e973 100644 --- a/lib/librte_pmd_i40e/i40e_rxtx.c +++ b/lib/librte_pmd_i40e/i40e_rxtx.c @@ -478,7 +478,7 @@ i40e_txd_enable_checksum(uint64_t ol_flags, } /* VXLAN packet TX checksum offload */ - if (unlikely(ol_flags & PKT_TX_VXLAN_CKSUM)) { + if (unlikely(ol_flags & PKT_TX_UDP_TUNNEL_PKT)) { uint8_t l4tun_len; l4tun_len = ETHER_VXLAN_HLEN + inner_l2_len; @@ -1158,8 +1158,8 @@ i40e_calc_context_desc(uint64_t flags) { uint64_t mask = 0ULL; - if (flags | PKT_TX_VXLAN_CKSUM) - mask |= PKT_TX_VXLAN_CKSUM; + if (flags | PKT_TX_UDP_TUNNEL_PKT) + mask |= PKT_TX_UDP_TUNNEL_PKT; #ifdef RTE_LIBRTE_IEEE1588 mask |= PKT_TX_IEEE1588_TMST;