[dpdk-dev] [PATCH v3 0/3] enhance TX checksum command and csum forwarding engine
Hi Olivier,
> -Original Message-
> From: Olivier MATZ [mailto:olivier.matz at 6wind.com]
> Sent: Tuesday, January 27, 2015 8:34 AM
> To: Ananyev, Konstantin; Liu, Jijiang; Zhang, Helin
> Cc: dev at dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v3 0/3] enhance TX checksum command and csum
> forwarding engine
>
> Hi Konstantin,
>
> On 01/26/2015 03:15 PM, Ananyev, Konstantin wrote:
> >>>> Another thing - IPIP seems to work ok by HW.
> >>>> There is something wrong on our (PMD/test-pmd) side.
> >>>> I think at least we have to remove the following check:
> >>>> if (!l2_len) {
> >>>> PMD_DRV_LOG(DEBUG, "L2 length set to 0");
> >>>> return;
> >>>> }
> >>>> in i40e_txd_enable_checksum().
> >>>
> >>> Yes, for IPIP, the check should be removed.
> >>
> >> Yes, I think these lines should be removed for 2 reasons:
> >> - it may be the cause of ipip tunnel not working
> >> - we shouldn't do these kind of tests in dataplane. I think we have to
> >>suppose that the data passed to the PMD is valid.
> >>
> >> I'll redo the test with ipip tomorrow with this fix and let you
> >> know the result. If it works, I'll add this in the next version
> >> of the patch.
> >
> > While you are on this, can I suggest you'll add debug logging for TCD and
> > TDD we are writing to the TX ring?
> > Something like that:
> >
> > + PMD_TX_LOG(DEBUG, "mbuf: %p, TCD[%u]:\n"
> > + "tunneling_params: %#x;\n"
> > + "l2tag2: %#hx;\n"
> > + "rsvd: %#hx;\n"
> > + "type_cmd_tso_mss: %#lx;\n",
> > + tx_pkt, tx_id,
> > + ctx_txd->tunneling_params,
> > + ctx_txd->l2tag2,
> > + ctx_txd->rsvd,
> > + ctx_txd->type_cmd_tso_mss);
> >
> > And same for TDD.
> > It helped me a lot to figure out what is going on, when I did my tests.
> > Probably would be useful for other people too.
>
> Sure, I'll add this.
>
> Also, just to let you know that I tested the ipip case without the
> "if (l2_len) return" and "if (l3_len) return", and it is working.
That's great.
Thanks
Konstantin
>
> Regards,
> Olivier
[dpdk-dev] [PATCH v3 0/3] enhance TX checksum command and csum forwarding engine
Hi Konstantin,
On 01/26/2015 03:15 PM, Ananyev, Konstantin wrote:
Another thing - IPIP seems to work ok by HW.
There is something wrong on our (PMD/test-pmd) side.
I think at least we have to remove the following check:
if (!l2_len) {
PMD_DRV_LOG(DEBUG, "L2 length set to 0");
return;
}
in i40e_txd_enable_checksum().
>>>
>>> Yes, for IPIP, the check should be removed.
>>
>> Yes, I think these lines should be removed for 2 reasons:
>> - it may be the cause of ipip tunnel not working
>> - we shouldn't do these kind of tests in dataplane. I think we have to
>>suppose that the data passed to the PMD is valid.
>>
>> I'll redo the test with ipip tomorrow with this fix and let you
>> know the result. If it works, I'll add this in the next version
>> of the patch.
>
> While you are on this, can I suggest you'll add debug logging for TCD and TDD
> we are writing to the TX ring?
> Something like that:
>
> + PMD_TX_LOG(DEBUG, "mbuf: %p, TCD[%u]:\n"
> + "tunneling_params: %#x;\n"
> + "l2tag2: %#hx;\n"
> + "rsvd: %#hx;\n"
> + "type_cmd_tso_mss: %#lx;\n",
> + tx_pkt, tx_id,
> + ctx_txd->tunneling_params,
> + ctx_txd->l2tag2,
> + ctx_txd->rsvd,
> + ctx_txd->type_cmd_tso_mss);
>
> And same for TDD.
> It helped me a lot to figure out what is going on, when I did my tests.
> Probably would be useful for other people too.
Sure, I'll add this.
Also, just to let you know that I tested the ipip case without the
"if (l2_len) return" and "if (l3_len) return", and it is working.
Regards,
Olivier
[dpdk-dev] [PATCH v3 0/3] enhance TX checksum command and csum forwarding engine
Hi,
On 01/26/2015 07:02 AM, Liu, Jijiang wrote:
>> I tried to repeat Olivier test-cases on my box.
>> Though, I didn't use test-pmd cusmonly and i40ePMD logic, but filled TCD and
>> TDD mostly from hardcoded values.
>> That's what I got:
>>
>> 4 input packets:
>> a) ETHER/IPv4/UDP/VXLAN/ETHER/IPV4/TCP
>> b) ETHER/IPv4/GRE/ETHER/IPV4/TCP
>> c) ETHER/IPv4/GRE/IPV4/TCP
>> d) ETHER/IPv4/IPV4/TCP
>>
>> 1/ L4TUNT==1(I40E_TXD_CTX_UDP_TUNNELING):
>> a),b): all checksums ok
>> c),d): not transmitted by HW.
>>
>> 2/ L4TUNT==2(I40E_TXD_CTX_GRE_TUNNELING):
>> a) b),c): all checksums ok
>> d): not transmitted by HW.
>>
>> 3/ L4TUNT==0(UNKNOWN):
>> a),b),c),d): all checksums ok
>>
>> So yes, it seems that L4TUNT==0 works perfectly ok for all cases, as long as
>> L4TUNLEN and other TCD values are setup properly.
>> Which makes me think, that probably we can do what you suggested: just use
>> L4TUNT=0 for all cases.
>> Though as Jijiang said, we waiting for confirmation from FVL guys, that
>> there are
>> no hidden implications with that approach.
>
> Yes, the L4TUNT=0 is ok for all cases.
Great! Thanks for testing on your side too.
> But we still need to get confirmation from FVL guys, probably there are some
> issues in HW/FW.
> I and Helin will confirm this with FVL guys ASAP.
OK, thank you.
>> Another thing - IPIP seems to work ok by HW.
>> There is something wrong on our (PMD/test-pmd) side.
>> I think at least we have to remove the following check:
>> if (!l2_len) {
>> PMD_DRV_LOG(DEBUG, "L2 length set to 0");
>> return;
>> }
>> in i40e_txd_enable_checksum().
>
> Yes, for IPIP, the check should be removed.
Yes, I think these lines should be removed for 2 reasons:
- it may be the cause of ipip tunnel not working
- we shouldn't do these kind of tests in dataplane. I think we have to
suppose that the data passed to the PMD is valid.
I'll redo the test with ipip tomorrow with this fix and let you
know the result. If it works, I'll add this in the next version
of the patch.
Regards,
Olivier
[dpdk-dev] [PATCH v3 0/3] enhance TX checksum command and csum forwarding engine
Hi Olivier,
> -Original Message-
> From: Olivier MATZ [mailto:olivier.matz at 6wind.com]
> Sent: Monday, January 26, 2015 2:07 PM
> To: Liu, Jijiang; Ananyev, Konstantin; Zhang, Helin
> Cc: dev at dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v3 0/3] enhance TX checksum command and csum
> forwarding engine
>
> Hi,
>
> On 01/26/2015 07:02 AM, Liu, Jijiang wrote:
> >> I tried to repeat Olivier test-cases on my box.
> >> Though, I didn't use test-pmd cusmonly and i40ePMD logic, but filled TCD
> >> and
> >> TDD mostly from hardcoded values.
> >> That's what I got:
> >>
> >> 4 input packets:
> >> a) ETHER/IPv4/UDP/VXLAN/ETHER/IPV4/TCP
> >> b) ETHER/IPv4/GRE/ETHER/IPV4/TCP
> >> c) ETHER/IPv4/GRE/IPV4/TCP
> >> d) ETHER/IPv4/IPV4/TCP
> >>
> >> 1/ L4TUNT==1(I40E_TXD_CTX_UDP_TUNNELING):
> >> a),b): all checksums ok
> >> c),d): not transmitted by HW.
> >>
> >> 2/ L4TUNT==2(I40E_TXD_CTX_GRE_TUNNELING):
> >> a) b),c): all checksums ok
> >> d): not transmitted by HW.
> >>
> >> 3/ L4TUNT==0(UNKNOWN):
> >> a),b),c),d): all checksums ok
> >>
> >> So yes, it seems that L4TUNT==0 works perfectly ok for all cases, as long
> >> as
> >> L4TUNLEN and other TCD values are setup properly.
> >> Which makes me think, that probably we can do what you suggested: just use
> >> L4TUNT=0 for all cases.
> >> Though as Jijiang said, we waiting for confirmation from FVL guys, that
> >> there are
> >> no hidden implications with that approach.
> >
> > Yes, the L4TUNT=0 is ok for all cases.
>
> Great! Thanks for testing on your side too.
>
> > But we still need to get confirmation from FVL guys, probably there are
> > some issues in HW/FW.
> > I and Helin will confirm this with FVL guys ASAP.
>
> OK, thank you.
>
> >> Another thing - IPIP seems to work ok by HW.
> >> There is something wrong on our (PMD/test-pmd) side.
> >> I think at least we have to remove the following check:
> >> if (!l2_len) {
> >> PMD_DRV_LOG(DEBUG, "L2 length set to 0");
> >> return;
> >> }
> >> in i40e_txd_enable_checksum().
> >
> > Yes, for IPIP, the check should be removed.
>
> Yes, I think these lines should be removed for 2 reasons:
> - it may be the cause of ipip tunnel not working
> - we shouldn't do these kind of tests in dataplane. I think we have to
> suppose that the data passed to the PMD is valid.
>
> I'll redo the test with ipip tomorrow with this fix and let you
> know the result. If it works, I'll add this in the next version
> of the patch.
While you are on this, can I suggest you'll add debug logging for TCD and TDD
we are writing to the TX ring?
Something like that:
+ PMD_TX_LOG(DEBUG, "mbuf: %p, TCD[%u]:\n"
+ "tunneling_params: %#x;\n"
+ "l2tag2: %#hx;\n"
+ "rsvd: %#hx;\n"
+ "type_cmd_tso_mss: %#lx;\n",
+ tx_pkt, tx_id,
+ ctx_txd->tunneling_params,
+ ctx_txd->l2tag2,
+ ctx_txd->rsvd,
+ ctx_txd->type_cmd_tso_mss);
And same for TDD.
It helped me a lot to figure out what is going on, when I did my tests.
Probably would be useful for other people too.
Konstantin
>
> Regards,
> Olivier
[dpdk-dev] [PATCH v3 0/3] enhance TX checksum command and csum forwarding engine
Hi,
> -Original Message-
> From: Ananyev, Konstantin
> Sent: Monday, January 26, 2015 12:14 PM
> To: Olivier MATZ; Liu, Jijiang
> Cc: dev at dpdk.org
> Subject: RE: [dpdk-dev] [PATCH v3 0/3] enhance TX checksum command and
> csum forwarding engine
>
> Hi lads,
>
> > -Original Message-
> > From: Olivier MATZ [mailto:olivier.matz at 6wind.com]
> > Sent: Wednesday, January 21, 2015 3:25 PM
> > To: Liu, Jijiang; Ananyev, Konstantin
> > Cc: dev at dpdk.org
> > Subject: Re: [dpdk-dev] [PATCH v3 0/3] enhance TX checksum command and
> > csum forwarding engine
> >
> > Hi,
> >
> > On 01/21/2015 04:12 AM, Liu, Jijiang wrote:
> > >>>>> Ok, and why it should be our problem?
> > >>>>> We have a lot of things done in a different manner then
> > >>>>> linux/freebsd kernel drivers, Why now it became a problem?
> > >>>>
> > >>>> If linux doesn't need an equivalent flag for doing the same
> > >>>> thing, it probably means we don't need it either.
> > >>>
> > >>> Probably yes Or probably not.
> > >>> Why do we need to guess what was the intention of guys who wrote
> > >>> that
> > >> part of linux driver?
> > >>
> > >> Because the dpdk looks very similar to that part of linux driver.
> > >
> > > A guy from Intel who have already confirmed that the NVGRE is not
> supported yet in Linux kernel.
> > >
> > > He said "So far as I know it is not yet supported and I have no
> > > information on
> when it will be."
> >
> > I added the support of Ether over GRE, IP over GRE and IP over IP
> > tunnels in csumonly to do the test. I ask the csum forward engine to
> > calculate inner IP+TCP checksums, and outer IP (case 6 in [1]).
> > Here are the results:
> >
> > 1/ When I use I40E_TXD_CTX_UDP_TUNNELING:
> > - vxlan: all checksums ok
> > - eth over gre: all checksums ok
> > - ip over gre: not transmitted by hw
> > - ip over ip: all checksums wrong (set to 0 by hw)
> >
> > 2/ When I use I40E_TXD_CTX_GRE_TUNNELING:
> > - vxlan: checksums ok
> > - eth over gre: all checksums ok
> > - ip over gre: all checksums ok
> > - ip over ip: all checksums wrong (set to 0 by hw)
> >
> > 3/ When I use 00b:
> > - vxlan: all checksums ok
> > - eth over gre: all checksums ok
> > - ip over gre: all checksums ok
> > - ip over ip: checksums wrong (set to 0 by hw)
> >
> > All the ip over ip tests do not work yet for an unknown reason.
> > There is maybe something wrong in my app or in the driver (although
> > the registers looks consistent with the datasheet).
> >
> > I think we could use 3/ for all tunnels, because the ipip case is
> > supposed to work according to the datasheet, and all other cases work
> > too.
> >
> > It would allow to remove the UDP_TUNNELING flag from mbuf API.
> >
> > I will send a RFC patch that provides the API change and this new
> > feature in csum forward engine, with full tests on ixgbe and i40e and
> > explanations for all changes.
> >
> > Regards,
> > Olivier
> >
> > [1] http://dpdk.org/ml/archives/dev/2015-January/011127.html
>
> I tried to repeat Olivier test-cases on my box.
> Though, I didn't use test-pmd cusmonly and i40ePMD logic, but filled TCD and
> TDD mostly from hardcoded values.
> That's what I got:
>
> 4 input packets:
> a) ETHER/IPv4/UDP/VXLAN/ETHER/IPV4/TCP
> b) ETHER/IPv4/GRE/ETHER/IPV4/TCP
> c) ETHER/IPv4/GRE/IPV4/TCP
> d) ETHER/IPv4/IPV4/TCP
>
> 1/ L4TUNT==1(I40E_TXD_CTX_UDP_TUNNELING):
> a),b): all checksums ok
> c),d): not transmitted by HW.
>
> 2/ L4TUNT==2(I40E_TXD_CTX_GRE_TUNNELING):
> a) b),c): all checksums ok
> d): not transmitted by HW.
>
> 3/ L4TUNT==0(UNKNOWN):
> a),b),c),d): all checksums ok
>
> So yes, it seems that L4TUNT==0 works perfectly ok for all cases, as long as
> L4TUNLEN and other TCD values are setup properly.
> Which makes me think, that probably we can do what you suggested: just use
> L4TUNT=0 for all cases.
> Though as Jijiang said, we waiting for confirmation from FVL guys, that there
> are
> no hidden implications with that approach.
Yes, the L4TUNT=0 is ok for all cases.
But we still need to get confirmation from FVL guys, probably there are some
issues in HW/FW.
I and Helin will confirm this with FVL guys ASAP.
> Another thing - IPIP seems to work ok by HW.
> There is something wrong on our (PMD/test-pmd) side.
> I think at least we have to remove the following check:
> if (!l2_len) {
> PMD_DRV_LOG(DEBUG, "L2 length set to 0");
> return;
> }
> in i40e_txd_enable_checksum().
Yes, for IPIP, the check should be removed.
> Konstantin
[dpdk-dev] [PATCH v3 0/3] enhance TX checksum command and csum forwarding engine
Hi lads,
> -Original Message-
> From: Olivier MATZ [mailto:olivier.matz at 6wind.com]
> Sent: Wednesday, January 21, 2015 3:25 PM
> To: Liu, Jijiang; Ananyev, Konstantin
> Cc: dev at dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v3 0/3] enhance TX checksum command and csum
> forwarding engine
>
> Hi,
>
> On 01/21/2015 04:12 AM, Liu, Jijiang wrote:
> >>>>> Ok, and why it should be our problem?
> >>>>> We have a lot of things done in a different manner then
> >>>>> linux/freebsd kernel drivers, Why now it became a problem?
> >>>>
> >>>> If linux doesn't need an equivalent flag for doing the same thing, it
> >>>> probably means we don't need it either.
> >>>
> >>> Probably yes Or probably not.
> >>> Why do we need to guess what was the intention of guys who wrote that
> >> part of linux driver?
> >>
> >> Because the dpdk looks very similar to that part of linux driver.
> >
> > A guy from Intel who have already confirmed that the NVGRE is not
> > supported yet in Linux kernel.
> >
> > He said "So far as I know it is not yet supported and I have no information
> > on when it will be."
>
> I added the support of Ether over GRE, IP over GRE and IP over IP
> tunnels in csumonly to do the test. I ask the csum forward engine
> to calculate inner IP+TCP checksums, and outer IP (case 6 in [1]).
> Here are the results:
>
> 1/ When I use I40E_TXD_CTX_UDP_TUNNELING:
> - vxlan: all checksums ok
> - eth over gre: all checksums ok
> - ip over gre: not transmitted by hw
> - ip over ip: all checksums wrong (set to 0 by hw)
>
> 2/ When I use I40E_TXD_CTX_GRE_TUNNELING:
> - vxlan: checksums ok
> - eth over gre: all checksums ok
> - ip over gre: all checksums ok
> - ip over ip: all checksums wrong (set to 0 by hw)
>
> 3/ When I use 00b:
> - vxlan: all checksums ok
> - eth over gre: all checksums ok
> - ip over gre: all checksums ok
> - ip over ip: checksums wrong (set to 0 by hw)
>
> All the ip over ip tests do not work yet for an unknown reason.
> There is maybe something wrong in my app or in the driver
> (although the registers looks consistent with the datasheet).
>
> I think we could use 3/ for all tunnels, because the ipip case
> is supposed to work according to the datasheet, and all other cases
> work too.
>
> It would allow to remove the UDP_TUNNELING flag from mbuf API.
>
> I will send a RFC patch that provides the API change and this new
> feature in csum forward engine, with full tests on ixgbe and i40e
> and explanations for all changes.
>
> Regards,
> Olivier
>
> [1] http://dpdk.org/ml/archives/dev/2015-January/011127.html
I tried to repeat Olivier test-cases on my box.
Though, I didn't use test-pmd cusmonly and i40ePMD logic, but filled TCD and
TDD mostly from hardcoded values.
That's what I got:
4 input packets:
a) ETHER/IPv4/UDP/VXLAN/ETHER/IPV4/TCP
b) ETHER/IPv4/GRE/ETHER/IPV4/TCP
c) ETHER/IPv4/GRE/IPV4/TCP
d) ETHER/IPv4/IPV4/TCP
1/ L4TUNT==1(I40E_TXD_CTX_UDP_TUNNELING):
a),b): all checksums ok
c),d): not transmitted by HW.
2/ L4TUNT==2(I40E_TXD_CTX_GRE_TUNNELING):
a) b),c): all checksums ok
d): not transmitted by HW.
3/ L4TUNT==0(UNKNOWN):
a),b),c),d): all checksums ok
So yes, it seems that L4TUNT==0 works perfectly ok for all cases, as long as
L4TUNLEN and other TCD values are setup properly.
Which makes me think, that probably we can do what you suggested: just use
L4TUNT=0 for all cases.
Though as Jijiang said, we waiting for confirmation from FVL guys, that there
are no hidden implications with that approach.
Another thing - IPIP seems to work ok by HW.
There is something wrong on our (PMD/test-pmd) side.
I think at least we have to remove the following check:
if (!l2_len) {
PMD_DRV_LOG(DEBUG, "L2 length set to 0");
return;
}
in i40e_txd_enable_checksum().
Konstantin
[dpdk-dev] [PATCH v3 0/3] enhance TX checksum command and csum forwarding engine
Hi, > -Original Message- > From: Stephen Hemminger [mailto:stephen at networkplumber.org] > Sent: Thursday, January 22, 2015 3:45 AM > To: Liu, Jijiang > Cc: Olivier MATZ; Ananyev, Konstantin; dev at dpdk.org > Subject: Re: [dpdk-dev] [PATCH v3 0/3] enhance TX checksum command and > csum forwarding engine > > On Wed, 21 Jan 2015 03:12:35 + > "Liu, Jijiang" wrote: > > > > Because the dpdk looks very similar to that part of linux driver. > > > > A guy from Intel who have already confirmed that the NVGRE is not > supported yet in Linux kernel. > > > > He said "So far as I know it is not yet supported and I have no information > on when it will be." > > The existing GRETAP support is sufficient to support NVGRE. No new work is > needed. Sorry, I meant i40e NVGRE feature.
[dpdk-dev] [PATCH v3 0/3] enhance TX checksum command and csum forwarding engine
Hi Konstantin, On 01/21/2015 05:28 PM, Ananyev, Konstantin wrote: >> I added the support of Ether over GRE, IP over GRE and IP over IP >> tunnels in csumonly to do the test. I ask the csum forward engine >> to calculate inner IP+TCP checksums, and outer IP (case 6 in [1]). >> Here are the results: >> >> 1/ When I use I40E_TXD_CTX_UDP_TUNNELING: >> - vxlan: all checksums ok >> - eth over gre: all checksums ok >> - ip over gre: not transmitted by hw >> - ip over ip: all checksums wrong (set to 0 by hw) >> >> 2/ When I use I40E_TXD_CTX_GRE_TUNNELING: >> - vxlan: checksums ok >> - eth over gre: all checksums ok >> - ip over gre: all checksums ok >> - ip over ip: all checksums wrong (set to 0 by hw) >> >> 3/ When I use 00b: >> - vxlan: all checksums ok >> - eth over gre: all checksums ok >> - ip over gre: all checksums ok >> - ip over ip: checksums wrong (set to 0 by hw) > > Wow, so there is absolutely no difference in results for L4TUNT=2(GRE) and > L4TUNT=0, right? > And IP over IP doesn't work at all? Right. I probably missed something in i40e driver. The application seems to fill the mbuf properly. > I suppose you set L4TUNLEN as described in spec for each case, right? I think so. > That looks really weird to me and as I can see completely contradicts with > what spec. > I suppose we'll need to reproduce all that tests on our HW too. > Could you send to us a patch with your changes, so we can try same thing? > Or just a dump of TDD and TCD values for each case. Sure, I'm going to send all my code and tests in a RFC patchset in a few minutes. By the way, I'm off tomorrow, I won't be able to answer. Regards, Olivier
[dpdk-dev] [PATCH v3 0/3] enhance TX checksum command and csum forwarding engine
Hi Olivier, > -Original Message- > From: Olivier MATZ [mailto:olivier.matz at 6wind.com] > Sent: Wednesday, January 21, 2015 3:25 PM > To: Liu, Jijiang; Ananyev, Konstantin > Cc: dev at dpdk.org > Subject: Re: [dpdk-dev] [PATCH v3 0/3] enhance TX checksum command and csum > forwarding engine > > Hi, > > On 01/21/2015 04:12 AM, Liu, Jijiang wrote: > >>>>> Ok, and why it should be our problem? > >>>>> We have a lot of things done in a different manner then > >>>>> linux/freebsd kernel drivers, Why now it became a problem? > >>>> > >>>> If linux doesn't need an equivalent flag for doing the same thing, it > >>>> probably means we don't need it either. > >>> > >>> Probably yes Or probably not. > >>> Why do we need to guess what was the intention of guys who wrote that > >> part of linux driver? > >> > >> Because the dpdk looks very similar to that part of linux driver. > > > > A guy from Intel who have already confirmed that the NVGRE is not > > supported yet in Linux kernel. > > > > He said "So far as I know it is not yet supported and I have no information > > on when it will be." > > I added the support of Ether over GRE, IP over GRE and IP over IP > tunnels in csumonly to do the test. I ask the csum forward engine > to calculate inner IP+TCP checksums, and outer IP (case 6 in [1]). > Here are the results: > > 1/ When I use I40E_TXD_CTX_UDP_TUNNELING: > - vxlan: all checksums ok > - eth over gre: all checksums ok > - ip over gre: not transmitted by hw > - ip over ip: all checksums wrong (set to 0 by hw) > > 2/ When I use I40E_TXD_CTX_GRE_TUNNELING: > - vxlan: checksums ok > - eth over gre: all checksums ok > - ip over gre: all checksums ok > - ip over ip: all checksums wrong (set to 0 by hw) > > 3/ When I use 00b: > - vxlan: all checksums ok > - eth over gre: all checksums ok > - ip over gre: all checksums ok > - ip over ip: checksums wrong (set to 0 by hw) Wow, so there is absolutely no difference in results for L4TUNT=2(GRE) and L4TUNT=0, right? And IP over IP doesn't work at all? I suppose you set L4TUNLEN as described in spec for each case, right? That looks really weird to me and as I can see completely contradicts with what spec. I suppose we'll need to reproduce all that tests on our HW too. Could you send to us a patch with your changes, so we can try same thing? Or just a dump of TDD and TCD values for each case. Konstantin > > All the ip over ip tests do not work yet for an unknown reason. > There is maybe something wrong in my app or in the driver > (although the registers looks consistent with the datasheet). > > I think we could use 3/ for all tunnels, because the ipip case > is supposed to work according to the datasheet, and all other cases > work too. > > It would allow to remove the UDP_TUNNELING flag from mbuf API. > > I will send a RFC patch that provides the API change and this new > feature in csum forward engine, with full tests on ixgbe and i40e > and explanations for all changes. > > Regards, > Olivier > > [1] http://dpdk.org/ml/archives/dev/2015-January/011127.html
[dpdk-dev] [PATCH v3 0/3] enhance TX checksum command and csum forwarding engine
Hi, On 01/21/2015 04:12 AM, Liu, Jijiang wrote: > Ok, and why it should be our problem? > We have a lot of things done in a different manner then > linux/freebsd kernel drivers, Why now it became a problem? If linux doesn't need an equivalent flag for doing the same thing, it probably means we don't need it either. >>> >>> Probably yes Or probably not. >>> Why do we need to guess what was the intention of guys who wrote that >> part of linux driver? >> >> Because the dpdk looks very similar to that part of linux driver. > > A guy from Intel who have already confirmed that the NVGRE is not supported > yet in Linux kernel. > > He said "So far as I know it is not yet supported and I have no information > on when it will be." I added the support of Ether over GRE, IP over GRE and IP over IP tunnels in csumonly to do the test. I ask the csum forward engine to calculate inner IP+TCP checksums, and outer IP (case 6 in [1]). Here are the results: 1/ When I use I40E_TXD_CTX_UDP_TUNNELING: - vxlan: all checksums ok - eth over gre: all checksums ok - ip over gre: not transmitted by hw - ip over ip: all checksums wrong (set to 0 by hw) 2/ When I use I40E_TXD_CTX_GRE_TUNNELING: - vxlan: checksums ok - eth over gre: all checksums ok - ip over gre: all checksums ok - ip over ip: all checksums wrong (set to 0 by hw) 3/ When I use 00b: - vxlan: all checksums ok - eth over gre: all checksums ok - ip over gre: all checksums ok - ip over ip: checksums wrong (set to 0 by hw) All the ip over ip tests do not work yet for an unknown reason. There is maybe something wrong in my app or in the driver (although the registers looks consistent with the datasheet). I think we could use 3/ for all tunnels, because the ipip case is supposed to work according to the datasheet, and all other cases work too. It would allow to remove the UDP_TUNNELING flag from mbuf API. I will send a RFC patch that provides the API change and this new feature in csum forward engine, with full tests on ixgbe and i40e and explanations for all changes. Regards, Olivier [1] http://dpdk.org/ml/archives/dev/2015-January/011127.html
[dpdk-dev] [PATCH v3 0/3] enhance TX checksum command and csum forwarding engine
> -Original Message- > From: Olivier MATZ [mailto:olivier.matz at 6wind.com] > Sent: Wednesday, January 21, 2015 9:11 AM > To: Liu, Jijiang > Cc: Ananyev, Konstantin; dev at dpdk.org > Subject: Re: [dpdk-dev] [PATCH v3 0/3] enhance TX checksum command and csum > forwarding engine > > Hi Jijiang, > > On 01/21/2015 09:01 AM, Liu, Jijiang wrote: > >>> I still don't understand why you are so eager to 'forbid' it. > >>> Yes we support it for FVL, but no one forces you to use it. > >> > >> Well, how would you describe this 2 ways of doing the same thing in the > >> offload API? Would you talk about the i40e registers? It's not because i40e > >> has 2 ways to do the same operation that the DPDK should do the same. > >> > >> How will you explain to a user how to choose between these 2 cases? > > > > Talk about B method in > > http://dpdk.org/ml/archives/dev/2014-December/009213.html again. > > > > DPDK Never supports a NIC that can recognize tunneling packet for TX side > > before 1.8, right? > > When you say "recognize tunnel", if you mean offlading checksum of > tunnel headers, I agree. > > If you mean recognizing a tunnel packet in rx, I also agree > it's new to dpdk-1.8, but I think it's unrelated to what we > are talking about, which is tx checksum. A DPDK application > is able to generate tunnel packets by itself and offload the > checksums to the NIC. > > > So when we need to support TX checksum offload for tunneling packet, and > > we have to choose B.2. > > I don't see why we should choose either B.1 or B.2 (I guess you want > to say B.1 here, right?). > > The m->lX_len are not filled in rx today. If one day they are, it won't > prevent the application to configure the lX_len fields and offload > flags according to the API. > > > After introducing i40e(FVL), FVL is able to recognize tunneling packet and > > support outer IP, or inner IP or outer IP and inner IP TX > checksum for tunneling packet. > > And you agree on "outer and inner at the same time", why do you object > > "only inner"? > > > > Actually, B.2 method is a software workaround using L2 length when NIC > > can't recognize tunneling packet. > > When NIC is able to recognize tunneling packet, I think you shouldn't take > > B.2 as a standard to 'forbid' other method. > > Again, I'm not sure there is a link between "recognizing tunneling > packets" and tx checksum offload of tunnels. I think what Jijiang doesn't talk about RX here. What he is trying to say that with B.2 (case 4) we hide from HW the fact that the packet is tunnelling. We just using the fact, that to calculate cksum for inner packet only, HW doesn't need to know is it a tunnelling packet or not. All it needs is a proper values of l2_len and l3_len. > > Regards, > Olivier
[dpdk-dev] [PATCH v3 0/3] enhance TX checksum command and csum forwarding engine
Hi Jijiang, On 01/21/2015 09:01 AM, Liu, Jijiang wrote: >>> I still don't understand why you are so eager to 'forbid' it. >>> Yes we support it for FVL, but no one forces you to use it. >> >> Well, how would you describe this 2 ways of doing the same thing in the >> offload API? Would you talk about the i40e registers? It's not because i40e >> has 2 ways to do the same operation that the DPDK should do the same. >> >> How will you explain to a user how to choose between these 2 cases? > > Talk about B method in > http://dpdk.org/ml/archives/dev/2014-December/009213.html again. > > DPDK Never supports a NIC that can recognize tunneling packet for TX side > before 1.8, right? When you say "recognize tunnel", if you mean offlading checksum of tunnel headers, I agree. If you mean recognizing a tunnel packet in rx, I also agree it's new to dpdk-1.8, but I think it's unrelated to what we are talking about, which is tx checksum. A DPDK application is able to generate tunnel packets by itself and offload the checksums to the NIC. > So when we need to support TX checksum offload for tunneling packet, and we > have to choose B.2. I don't see why we should choose either B.1 or B.2 (I guess you want to say B.1 here, right?). The m->lX_len are not filled in rx today. If one day they are, it won't prevent the application to configure the lX_len fields and offload flags according to the API. > After introducing i40e(FVL), FVL is able to recognize tunneling packet and > support outer IP, or inner IP or outer IP and inner IP TX checksum for > tunneling packet. > And you agree on "outer and inner at the same time", why do you object "only > inner"? > > Actually, B.2 method is a software workaround using L2 length when NIC can't > recognize tunneling packet. > When NIC is able to recognize tunneling packet, I think you shouldn't take > B.2 as a standard to 'forbid' other method. Again, I'm not sure there is a link between "recognizing tunneling packets" and tx checksum offload of tunnels. Regards, Olivier
[dpdk-dev] [PATCH v3 0/3] enhance TX checksum command and csum forwarding engine
Hi, > -Original Message- > From: Olivier MATZ [mailto:olivier.matz at 6wind.com] > Sent: Wednesday, January 21, 2015 2:16 AM > To: Ananyev, Konstantin; Liu, Jijiang > Cc: dev at dpdk.org > Subject: Re: [dpdk-dev] [PATCH v3 0/3] enhance TX checksum command and > csum forwarding engine > > Hi Konstantin, > > > >> Case 4) and case 9) would fill the hardware registers exactly the same. > > > > No, they wouldn't. > > Please read corresponding section of FVL spec and i40e_rxtx.c For case > > 4) we only need to setup TDD (TX data descriptor) with the following > values: > > IIPT, IPLEN, L4T, L4LEN > > For case 9) we need to setup both TDD and TCD (TX context descriptor) > with the following values: > > TDD: IIPT, IPLEN, L4T, L4LEN > > TCD: EIPT, EIPLEN, L4TUNT, L4TUNLEN > > > >> To me, it's just an API question. > > > > No, it is not. > > > > I still don't understand why you are so eager to 'forbid' it. > > Yes we support it for FVL, but no one forces you to use it. > > Well, how would you describe this 2 ways of doing the same thing in the > offload API? Would you talk about the i40e registers? It's not because i40e > has 2 ways to do the same operation that the DPDK should do the same. > > How will you explain to a user how to choose between these 2 cases? Talk about B method in http://dpdk.org/ml/archives/dev/2014-December/009213.html again. DPDK Never supports a NIC that can recognize tunneling packet for TX side before 1.8, right? So when we need to support TX checksum offload for tunneling packet, and we have to choose B.2. After introducing i40e(FVL), FVL is able to recognize tunneling packet and support outer IP, or inner IP or outer IP and inner IP TX checksum for tunneling packet. And you agree on "outer and inner at the same time", why do you object "only inner"? Actually, B.2 method is a software workaround using L2 length when NIC can't recognize tunneling packet. When NIC is able to recognize tunneling packet, I think you shouldn't take B.2 as a standard to 'forbid' other method. > Having to support these 2 different cases for the same thing will complexify > all future drivers that will not work the same way than i40e. > > >>> As one of possible use-cases: HW VLAN tags insertion for both inner and > outer packets. > >>> FVL can do that, though as I know our PMD doesn't implement it yet. > >>> For that, we'll need to specify at least: > >>> outer_l2_len, outer_l3_len, l2_len. > >>> While PKT_TX_OUTER_* might stay cleared. > >> > >> If a VLAN flag has to be inserted in outer header, a new flag > >> PKT_TX_OUTER_INSERT_VLAN would be added. So my specification > would > >> still be correct: > >> > >> The driver should look at mb->outer_lX_len only if a > >> PKT_TX_OUTER_* flag is present. > >> > > > > Introducing PKT_TX_OUTER_INSERT_VLAN is ok. > > Though I still think we'll need TX_*_TUNNEL flags and no need to 'forbid' > case 9). > > BTW, as I can see linux i40e driver for tunnelling packets uses case 9), not > case 4), right? > > I need to check this. > > Regards, > Olivier
[dpdk-dev] [PATCH v3 0/3] enhance TX checksum command and csum forwarding engine
Hi, > -Original Message- > From: Olivier MATZ [mailto:olivier.matz at 6wind.com] > Sent: Wednesday, January 21, 2015 2:16 AM > To: Ananyev, Konstantin; Liu, Jijiang > Cc: dev at dpdk.org > Subject: Re: [dpdk-dev] [PATCH v3 0/3] enhance TX checksum command and > csum forwarding engine > > > > >>> Ok, and why it should be our problem? > >>> We have a lot of things done in a different manner then > >>> linux/freebsd kernel drivers, Why now it became a problem? > >> > >> If linux doesn't need an equivalent flag for doing the same thing, it > >> probably means we don't need it either. > > > > Probably yes Or probably not. > > Why do we need to guess what was the intention of guys who wrote that > part of linux driver? > > Because the dpdk looks very similar to that part of linux driver. A guy from Intel who have already confirmed that the NVGRE is not supported yet in Linux kernel. He said "So far as I know it is not yet supported and I have no information on when it will be." > > BTW, the macro for GRE is here: > > find lib/librte_pmd_i40e/i40e -type f | xargs grep TUN | grep TXD > > lib/librte_pmd_i40e/i40e/i40e_type.h:#define > > I40E_TXD_CTX_UDP_TUNNELING (0x1ULL << > I40E_TXD_CTX_QW0_NATT_SHIFT) > > lib/librte_pmd_i40e/i40e/i40e_type.h:#define > > I40E_TXD_CTX_GRE_TUNNELING (0x2ULL << > I40E_TXD_CTX_QW0_NATT_SHIFT) > > > > Though it not used (yet?) by some reason. > > > >> > > Regards, > Olivier
[dpdk-dev] [PATCH v3 0/3] enhance TX checksum command and csum forwarding engine
Hi Konstantin, On 01/20/2015 06:23 PM, Ananyev, Konstantin wrote: >> Sure, it does not make a big difference in terms of code. But >> in terms of API, the naming of the flag is coherent to what it is >> used for. And it's easier to find a simple definition, like: >> >>* Packet is IPv4. This flag must be set when using any offload feature >>* (TSO, L3 or L4 checksum) to tell the NIC that the packet is an IPv4 >>* packet. > > Ok, and what's wrong with: > "Packet is IPv4. This flag must be set when using any offload feature > (TSO, L3 or L4 checksum) to tell the NIC that the packet is an IPv4 > packet and no HW offload for IPv4 header checksum calculation is required" > ? I honestly find the first one simpler. Again, I understand both are possible, but I think choosing the most trivial one is the right way for an API. >>> Ok, and why it should be our problem? >>> We have a lot of things done in a different manner then linux/freebsd >>> kernel drivers, >>> Why now it became a problem? >> >> If linux doesn't need an equivalent flag for doing the same thing, >> it probably means we don't need it either. > > Probably yes Or probably not. > Why do we need to guess what was the intention of guys who wrote that part of > linux driver? Because the dpdk looks very similar to that part of linux driver. > BTW, the macro for GRE is here: > find lib/librte_pmd_i40e/i40e -type f | xargs grep TUN | grep TXD > lib/librte_pmd_i40e/i40e/i40e_type.h:#define I40E_TXD_CTX_UDP_TUNNELING > (0x1ULL << I40E_TXD_CTX_QW0_NATT_SHIFT) > lib/librte_pmd_i40e/i40e/i40e_type.h:#define I40E_TXD_CTX_GRE_TUNNELING > (0x2ULL << I40E_TXD_CTX_QW0_NATT_SHIFT) > > Though it not used (yet?) by some reason. > >> >> In a performance-oriented software like dpdk, having a flag that we >> don't know what the hardware does with, that is not needed in other >> drivers of the same harware, that makes the API harder to understand >> could be a problem. > > Here is a HW spec, that says what values have to be setup for L4TUNT. > Yes, I am not sure why they need to distinguish between VXLAN/GRE tunnelling. > Though, I suppose that wouldn't eliminate the requirement. > But for same, there is no good explanation why FVL HW need to know that it is > IPv4 or IPv6 packet, > in the case when only L4 checksum offload is required (IIPT field). > Niantic, as I remember, is able to work ok without that requirement. > Though, we still have to set it up. > >> Another argument: if we can remove this flag, it would make the >> testpmd commands reworkd proposed by Jijiang much more easy to >> understand: only a new "csum parse-tunnel on|off" would be required, >> and it can be explained in a few words. > > Well, from my point - testpmd commands that Jijiang proposed are perfectly > clear and understandable. > Another thing, as I remember, our primary concern should be public API, no > testpmd. OK let's talk about testpmd later. >> We should avoid the need to specify the tunnel type in the OUTER >> checksum API if we can, else it would limit us to specific >> supported protocols. > > From the FVL spec it is required by HW, it is not what we introducing on our > own. > Spec stays explicitly that L4TUNT (L4 tunneling type) has to be setup for > tunnelling packets. > Again from the spec, there are 3 different values it can take. > If you have an idea how to pass that information to PMD without using flags, > sure we can consider it. > >> >> I think the following cases should be *forbidden by the API*: >> >> case 9) calculate checksum of in_ip and in_tcp (was case B.1 in [1]) >> >> mb->outer_l2_len = len(out_eth) >> mb->outer_l3_len = len(out_ip) >> mb->l2_len = len(out_udp + vxlan + in_eth) >> mb->l3_len = len(out_ip) >> mb->ol_flags |= PKT_TX_IPV4 | PKT_TX_UDP_TUNNEL_PKT | \ >>PKT_TX_IP_CSUM | PKT_TX_UDP_CKSUM; >> set out_ip checksum to 0 in the packet >> set out_udp checksum to pseudo header using rte_ipv4_phdr_cksum() >> >> If we remove the flag PKT_TX_UDP_TUNNEL_PKT, this cannot be >> supported, but there is no reason to support it as there is >> already one way to do the same. >> >> I think the driver should not even look at mb->outer_l2_len >> and mb->outer_l3_len if no flag PKT_TX_OUTER_* is set. > > Why it should be forbidden? > I admit it might be a bit slower than case 4), > but I think absolutely legal way to setup HW offloads for inner L3/L4. > As I said we need a PKT_TX_UDP_TUNNEL_PKT anyway, so I suppose > PKT_TX_*_TUNNEL_PKT should be an indication is it a tunnel packet or not. > PKT_TX_OUTER_* flags indicate does outer cksum offload is required or not. I don't understand. The result in terms of hardware is exactly the same than case 4). Why should we have 2 different ways for doing the same thing? >>> >>> If HW supports that capability,
[dpdk-dev] [PATCH v3 0/3] enhance TX checksum command and csum forwarding engine
Hi Olivier, > -Original Message- > From: Olivier MATZ [mailto:olivier.matz at 6wind.com] > Sent: Tuesday, January 20, 2015 12:39 PM > To: Ananyev, Konstantin; Liu, Jijiang > Cc: dev at dpdk.org > Subject: Re: [dpdk-dev] [PATCH v3 0/3] enhance TX checksum command and csum > forwarding engine > > Hi, > > On 01/20/2015 02:12 AM, Ananyev, Konstantin wrote: > >>>> I think a good definition would > >>>> be: > >>>> > >>>> Packet is IPv4. This flag must be set when using any offload > >>>> feature (TSO, L3 or L4 checksum) to tell the NIC that the packet > >>>> is an IPv4 packet. > >>>> > >>>> That's why I added PKT_TX_IPV4 in the examples. > >>> > >>> I suppose we discussed it several times: both ways are possible. > >>> From PMD perspective - treating PKT_TX_IPV4 and PKT_TX_IP_CSUM > >>> As mutually exclusive seems a bit more plausible. > >>> From the upper layer - my understanding, that it is doesn't really > >>> matter. > >>> I thought we had an agreement about it in 1.8, no? > >> > >> Indeed, this was already discussed, but there was a lot of pressure > >> for 1.8.0 to push something, even not perfect. The fog around comments > >> shows that the API was not very clearly defined for 1.8.0. If you read > >> the comments of the API, it is impossible to understand when the > >> PKT_TX_IPV4 or PKT_TX_IP_CSUM flags must be set. I would even say > >> more: the only place where the comments bring a valuable information > >> (L4 checksum and TSO) describe the case where PKT_TX_IPV4 and > >> PKT_TX_IP_CSUM are not exclusive... > >> > >> So I will fix that in my coming patch series. Just for information, > >> I'm pretty sure that having PKT_TX_IPV4 and PKT_TX_IP_CSUM as not > >> exclusive flag would not require any change anywhere in the PMDs (even > >> in i40e). > > > > Right now - no. > > Though as I said from PMD perspective having them exclusive is a bit > > preferable. > > Again, I don't see any big difference from upper layer code. > > Sure, it does not make a big difference in terms of code. But > in terms of API, the naming of the flag is coherent to what it is > used for. And it's easier to find a simple definition, like: > > * Packet is IPv4. This flag must be set when using any offload feature > * (TSO, L3 or L4 checksum) to tell the NIC that the packet is an IPv4 > * packet. Ok, and what's wrong with: "Packet is IPv4. This flag must be set when using any offload feature (TSO, L3 or L4 checksum) to tell the NIC that the packet is an IPv4 packet and no HW offload for IPv4 header checksum calculation is required" ? > > >> On the contrary, making them exclusive would require to > >> change the ixgbe TSO code because we check. > > > > Hmm, so you are saying there is a bug somewhere in ixbe_rxtx.c? > > What particular place you are talking about? > > Sorry, I spoke too fast. In TSO code, we check PKT_TX_IP_CKSUM (and not > PKT_TX_IPV4 as I thought), so it would work for both methods without > patching the code. > > In this case, it means that both approach would not require to > modify the code. Ok. > > >>>> *Problem 3*: without using the word "fortville", it is difficult > >>>> to understand the goal of the flag PKT_TX_UDP_TUNNEL_PKT. Indeed, > >>>> once PKT_TX_OUTER_IPV4/6 is set, it looks obvious that it's a > >>>> tunnel packet. I suggest to remove the PKT_TX_UDP_TUNNEL_PKT > >>>> flag. In linux, the driver doesn't care about the tunnel type, > >>>> it always set I40E_TXD_CTX_UDP_TUNNELING for all encapsulations [6]. > >>> > >>> It might be obvious that it is a tunnel packet from PKT_TX_OUTER_* is set, > >>> but it is not obvious what type of tunnelling it would be. > >>> FVL HW supports HW TX offloads for different type of tunnelling and > >>> requires that SW provide information about tunnelling type. > >>> From i40e datasheet: > >>> L4TUNT L4 Tunneling Type (Teredo / GRE header / VXLAN header) indication: > >>> 00b - No UDP / GRE tunneling (field must be set to zero if EIPT equals to > >>> zero) > >>> 01b - UDP tunneling header (any UDP tunneling, VXLAN and Geneve). > >>> 10b - GRE tunneling header > >>> As we do plan to support other than UDP tunnelling types, I suppose we'll > >>> need to keep &
[dpdk-dev] [PATCH v3 0/3] enhance TX checksum command and csum forwarding engine
2015-01-20 13:39, Olivier MATZ: > On 01/20/2015 02:12 AM, Ananyev, Konstantin wrote: > >> So I will fix that in my coming patch series. Just for information, > >> I'm pretty sure that having PKT_TX_IPV4 and PKT_TX_IP_CSUM as not > >> exclusive flag would not require any change anywhere in the PMDs (even > >> in i40e). > > > > Right now - no. > > Though as I said from PMD perspective having them exclusive is a bit > > preferable. > > Again, I don't see any big difference from upper layer code. > > Sure, it does not make a big difference in terms of code. But > in terms of API, the naming of the flag is coherent to what it is > used for. And it's easier to find a simple definition, like: > > * Packet is IPv4. This flag must be set when using any offload feature > * (TSO, L3 or L4 checksum) to tell the NIC that the packet is an IPv4 > * packet. +1 It's clearer to set PKT_TX_IPV4 in all offload cases of IPv4 packets, and add PKT_TX_IP_CSUM when checksum offload is required. Simply simpler ;) -- Thomas
[dpdk-dev] [PATCH v3 0/3] enhance TX checksum command and csum forwarding engine
Hi, On 01/20/2015 02:12 AM, Ananyev, Konstantin wrote: I think a good definition would be: Packet is IPv4. This flag must be set when using any offload feature (TSO, L3 or L4 checksum) to tell the NIC that the packet is an IPv4 packet. That's why I added PKT_TX_IPV4 in the examples. >>> >>> I suppose we discussed it several times: both ways are possible. >>> From PMD perspective - treating PKT_TX_IPV4 and PKT_TX_IP_CSUM >>> As mutually exclusive seems a bit more plausible. >>> From the upper layer - my understanding, that it is doesn't really matter. >>> I thought we had an agreement about it in 1.8, no? >> >> Indeed, this was already discussed, but there was a lot of pressure >> for 1.8.0 to push something, even not perfect. The fog around comments >> shows that the API was not very clearly defined for 1.8.0. If you read >> the comments of the API, it is impossible to understand when the >> PKT_TX_IPV4 or PKT_TX_IP_CSUM flags must be set. I would even say >> more: the only place where the comments bring a valuable information >> (L4 checksum and TSO) describe the case where PKT_TX_IPV4 and >> PKT_TX_IP_CSUM are not exclusive... >> >> So I will fix that in my coming patch series. Just for information, >> I'm pretty sure that having PKT_TX_IPV4 and PKT_TX_IP_CSUM as not >> exclusive flag would not require any change anywhere in the PMDs (even >> in i40e). > > Right now - no. > Though as I said from PMD perspective having them exclusive is a bit > preferable. > Again, I don't see any big difference from upper layer code. Sure, it does not make a big difference in terms of code. But in terms of API, the naming of the flag is coherent to what it is used for. And it's easier to find a simple definition, like: * Packet is IPv4. This flag must be set when using any offload feature * (TSO, L3 or L4 checksum) to tell the NIC that the packet is an IPv4 * packet. >> On the contrary, making them exclusive would require to >> change the ixgbe TSO code because we check. > > Hmm, so you are saying there is a bug somewhere in ixbe_rxtx.c? > What particular place you are talking about? Sorry, I spoke too fast. In TSO code, we check PKT_TX_IP_CKSUM (and not PKT_TX_IPV4 as I thought), so it would work for both methods without patching the code. In this case, it means that both approach would not require to modify the code. *Problem 3*: without using the word "fortville", it is difficult to understand the goal of the flag PKT_TX_UDP_TUNNEL_PKT. Indeed, once PKT_TX_OUTER_IPV4/6 is set, it looks obvious that it's a tunnel packet. I suggest to remove the PKT_TX_UDP_TUNNEL_PKT flag. In linux, the driver doesn't care about the tunnel type, it always set I40E_TXD_CTX_UDP_TUNNELING for all encapsulations [6]. >>> >>> It might be obvious that it is a tunnel packet from PKT_TX_OUTER_* is set, >>> but it is not obvious what type of tunnelling it would be. >>> FVL HW supports HW TX offloads for different type of tunnelling and >>> requires that SW provide information about tunnelling type. >>> From i40e datasheet: >>> L4TUNT L4 Tunneling Type (Teredo / GRE header / VXLAN header) indication: >>> 00b - No UDP / GRE tunneling (field must be set to zero if EIPT equals to >>> zero) >>> 01b - UDP tunneling header (any UDP tunneling, VXLAN and Geneve). >>> 10b - GRE tunneling header >>> As we do plan to support other than UDP tunnelling types, I suppose we'll >>> need to keep >>> PKT_TX_UDP_TUNNEL_PKT flag. >> >> As I've said: in linux, the driver doesn't care about the tunnel type, >> it always set I40E_TXD_CTX_UDP_TUNNELING for all encapsulations. > > Ok, and why it should be our problem? > We have a lot of things done in a different manner then linux/freebsd kernel > drivers, > Why now it became a problem? If linux doesn't need an equivalent flag for doing the same thing, it probably means we don't need it either. In a performance-oriented software like dpdk, having a flag that we don't know what the hardware does with, that is not needed in other drivers of the same harware, that makes the API harder to understand could be a problem. Another argument: if we can remove this flag, it would make the testpmd commands reworkd proposed by Jijiang much more easy to understand: only a new "csum parse-tunnel on|off" would be required, and it can be explained in a few words. I'll try to do some tests on a fortville NIC if I can find one. I'm curious to see if we can transmit any encapsulation packet (ip in ip, ip in gre, eth in gre, eth in vxlan, or even a proprietary tunnel). We should avoid the need to specify the tunnel type in the OUTER checksum API if we can, else it would limit us to specific supported protocols. I think the following cases should be *forbidden by the API*: case 9) calculate checksum of in_ip and in_tcp (was case B.1 in [1]) mb->outer_l2_len = len(out_eth)
[dpdk-dev] [PATCH v3 0/3] enhance TX checksum command and csum forwarding engine
> -Original Message- > From: Olivier MATZ [mailto:olivier.matz at 6wind.com] > Sent: Monday, January 19, 2015 2:39 PM > To: Ananyev, Konstantin; Liu, Jijiang > Cc: dev at dpdk.org > Subject: Re: [dpdk-dev] [PATCH v3 0/3] enhance TX checksum command and csum > forwarding engine > > Hi Konstantin, > > On 01/19/2015 02:04 PM, Ananyev, Konstantin wrote: > >> case 2) calculate checksum of out_ip and out_udp > >> > >>mb->l2_len = len(out_eth) > >>mb->l3_len = len(out_ip) > >>mb->ol_flags |= PKT_TX_IPV4 | PKT_TX_IP_CSUM | PKT_TX_UDP_CKSUM > >>set out_ip checksum to 0 in the packet > >>set out_udp checksum to pseudo header using rte_ipv4_phdr_cksum() > >> > >>supported on hardware advertising DEV_TX_OFFLOAD_IPV4_CKSUM and > >>DEV_TX_OFFLOAD_UDP_CKSUM > >> > >>*Problem 1*: The comment above PKT_TX_IPV4 says "Packet is IPv4 > >>without requiring IP checksum offload" [2], and the help of L4 > >>checksum and TSO says that it is required to set the PKT_TX_IPV4 > >>flag [3]. This is not coherent. > > > > So what is the problem? > > Comments in rte_mbuf.h are not coherent? > > No there're not coherent Ok, if the problem is just comments - let's fix it. > > >>We are back on the debate about the meaning of PKT_TX_IPV4 vs > >>PKT_TX_IP_CSUM from [4]. This incoherency in comments are introduced > >>by patch [5]. The question is "when an application should set > >>this flag? for any IP packet that does not require IP checksum?". > > > > Yes, if it is an IPv4 packet and application required TX offload for L4 > > checksum or TSO, > > but doesn't want HW offload ofr IPV4 checksum calculation. > > > >>This would break many applications. > > > > Which ones? > > As I know, so far nothing is broken. > > The problem today is that it's not obvious for a developper to > know when an application should set the PKT_TX_IPV4 flag. From the > comments, we could think that an application has to set it for any > transmitted IP packet, even for packets that do not require tx > offload. Asking to do this in the API would break many applications. > > The comment should at least say that this flag is *only* required > when asking for L4 checksum. As TSO implies IP checksum, it means the > PKT_TX_IPV4 should not be set, but PKT_TX_IP_CSUM instead. Ok, so the problem is in comments again? If so, sure let's update them to make things clear. > > >> I think a good definition would > >>be: > >> > >>Packet is IPv4. This flag must be set when using any offload > >>feature (TSO, L3 or L4 checksum) to tell the NIC that the packet > >>is an IPv4 packet. > >> > >>That's why I added PKT_TX_IPV4 in the examples. > > > > I suppose we discussed it several times: both ways are possible. > > From PMD perspective - treating PKT_TX_IPV4 and PKT_TX_IP_CSUM > > As mutually exclusive seems a bit more plausible. > > From the upper layer - my understanding, that it is doesn't really matter. > > I thought we had an agreement about it in 1.8, no? > > Indeed, this was already discussed, but there was a lot of pressure > for 1.8.0 to push something, even not perfect. The fog around comments > shows that the API was not very clearly defined for 1.8.0. If you read > the comments of the API, it is impossible to understand when the > PKT_TX_IPV4 or PKT_TX_IP_CSUM flags must be set. I would even say > more: the only place where the comments bring a valuable information > (L4 checksum and TSO) describe the case where PKT_TX_IPV4 and > PKT_TX_IP_CSUM are not exclusive... > > So I will fix that in my coming patch series. Just for information, > I'm pretty sure that having PKT_TX_IPV4 and PKT_TX_IP_CSUM as not > exclusive flag would not require any change anywhere in the PMDs (even > in i40e). Right now - no. Though as I said from PMD perspective having them exclusive is a bit preferable. Again, I don't see any big difference from upper layer code. > On the contrary, making them exclusive would require to > change the ixgbe TSO code because we check PKT_TX_IPV4. Hmm, so you are saying there is a bug somewhere in ixbe_rxtx.c? What particular place you are talking about? > > >>*Problem 3*: without using the word "fortville", it is difficult > >>to understand the goal of the flag PKT_TX_UDP_TUNNEL_PKT. Indeed, > >>once PKT_TX_OUTER_IPV4/6 is set, it looks obvious that it's a > >>tunnel packet. I suggest
[dpdk-dev] [PATCH v3 0/3] enhance TX checksum command and csum forwarding engine
Hi Konstantin, On 01/19/2015 02:04 PM, Ananyev, Konstantin wrote: >> case 2) calculate checksum of out_ip and out_udp >> >>mb->l2_len = len(out_eth) >>mb->l3_len = len(out_ip) >>mb->ol_flags |= PKT_TX_IPV4 | PKT_TX_IP_CSUM | PKT_TX_UDP_CKSUM >>set out_ip checksum to 0 in the packet >>set out_udp checksum to pseudo header using rte_ipv4_phdr_cksum() >> >>supported on hardware advertising DEV_TX_OFFLOAD_IPV4_CKSUM and >>DEV_TX_OFFLOAD_UDP_CKSUM >> >>*Problem 1*: The comment above PKT_TX_IPV4 says "Packet is IPv4 >>without requiring IP checksum offload" [2], and the help of L4 >>checksum and TSO says that it is required to set the PKT_TX_IPV4 >>flag [3]. This is not coherent. > > So what is the problem? > Comments in rte_mbuf.h are not coherent? No there're not coherent >>We are back on the debate about the meaning of PKT_TX_IPV4 vs >>PKT_TX_IP_CSUM from [4]. This incoherency in comments are introduced >>by patch [5]. The question is "when an application should set >>this flag? for any IP packet that does not require IP checksum?". > > Yes, if it is an IPv4 packet and application required TX offload for L4 > checksum or TSO, > but doesn't want HW offload ofr IPV4 checksum calculation. > >>This would break many applications. > > Which ones? > As I know, so far nothing is broken. The problem today is that it's not obvious for a developper to know when an application should set the PKT_TX_IPV4 flag. From the comments, we could think that an application has to set it for any transmitted IP packet, even for packets that do not require tx offload. Asking to do this in the API would break many applications. The comment should at least say that this flag is *only* required when asking for L4 checksum. As TSO implies IP checksum, it means the PKT_TX_IPV4 should not be set, but PKT_TX_IP_CSUM instead. >> I think a good definition would >>be: >> >>Packet is IPv4. This flag must be set when using any offload >>feature (TSO, L3 or L4 checksum) to tell the NIC that the packet >>is an IPv4 packet. >> >>That's why I added PKT_TX_IPV4 in the examples. > > I suppose we discussed it several times: both ways are possible. > From PMD perspective - treating PKT_TX_IPV4 and PKT_TX_IP_CSUM > As mutually exclusive seems a bit more plausible. > From the upper layer - my understanding, that it is doesn't really matter. > I thought we had an agreement about it in 1.8, no? Indeed, this was already discussed, but there was a lot of pressure for 1.8.0 to push something, even not perfect. The fog around comments shows that the API was not very clearly defined for 1.8.0. If you read the comments of the API, it is impossible to understand when the PKT_TX_IPV4 or PKT_TX_IP_CSUM flags must be set. I would even say more: the only place where the comments bring a valuable information (L4 checksum and TSO) describe the case where PKT_TX_IPV4 and PKT_TX_IP_CSUM are not exclusive... So I will fix that in my coming patch series. Just for information, I'm pretty sure that having PKT_TX_IPV4 and PKT_TX_IP_CSUM as not exclusive flag would not require any change anywhere in the PMDs (even in i40e). On the contrary, making them exclusive would require to change the ixgbe TSO code because we check PKT_TX_IPV4. >>*Problem 3*: without using the word "fortville", it is difficult >>to understand the goal of the flag PKT_TX_UDP_TUNNEL_PKT. Indeed, >>once PKT_TX_OUTER_IPV4/6 is set, it looks obvious that it's a >>tunnel packet. I suggest to remove the PKT_TX_UDP_TUNNEL_PKT >>flag. In linux, the driver doesn't care about the tunnel type, >>it always set I40E_TXD_CTX_UDP_TUNNELING for all encapsulations [6]. > > It might be obvious that it is a tunnel packet from PKT_TX_OUTER_* is set, > but it is not obvious what type of tunnelling it would be. > FVL HW supports HW TX offloads for different type of tunnelling and > requires that SW provide information about tunnelling type. > From i40e datasheet: > L4TUNT L4 Tunneling Type (Teredo / GRE header / VXLAN header) indication: > 00b - No UDP / GRE tunneling (field must be set to zero if EIPT equals to > zero) > 01b - UDP tunneling header (any UDP tunneling, VXLAN and Geneve). > 10b - GRE tunneling header > As we do plan to support other than UDP tunnelling types, I suppose we'll > need to keep > PKT_TX_UDP_TUNNEL_PKT flag. As I've said: in linux, the driver doesn't care about the tunnel type, it always set I40E_TXD_CTX_UDP_TUNNELING for all encapsulations. However I suppose that linux driver is able to process the hw outer checksum even for other encapsulations (gre, ipip). And, does it mean that ipip tunnels are not supported by i40e? I can't believe it. If it's the case... how an application on top of DPDK can know which tunnel types are supported by the underlying port? >From what I've read, what the datasheet does not explain is: "what is done differently for this packet between
[dpdk-dev] [PATCH v3 0/3] enhance TX checksum command and csum forwarding engine
Hi Olivier, > -Original Message- > From: Olivier MATZ [mailto:olivier.matz at 6wind.com] > Sent: Friday, January 16, 2015 5:28 PM > To: Ananyev, Konstantin; Liu, Jijiang > Cc: dev at dpdk.org > Subject: Re: [dpdk-dev] [PATCH v3 0/3] enhance TX checksum command and csum > forwarding engine > > Hi Konstantin, Hi Jijiang, > > On 01/15/2015 02:31 PM, Ananyev, Konstantin wrote: > > To be honest, there are so many mails around that subject, so I am already > > lost :) > > Oliver, as I understand you are not happy with the test-pmd commands Frank > > is proposing. > > Both syntax and semantics. > > Is that correct? > > If so, could you suggest something from your side? > > That would allow to configure test-pmd to behave in any of 4 possible ways > > we discussed previously: > > http://dpdk.org/ml/archives/dev/2014-December/009213.html > > I first wanted to send a mail to describe the current problem with > testpmd command line and the 2 solutions (Jijiang's and mine). > But, first, I think we need to fully clarify the checksum offload > API through examples as it will help to implement testpmd and do > the documentation. They are based on Jijiang's previous mail [1]. > > I will submit a patchset fixing the problems described below in > the coming days. If we agree on it, I'll submit another one for testpmd. > > Let's use the following packet for all the examples below: > out_eth / out_ip / out_udp / vxlan / in_eth / in_ip / in_tcp > > > The following cases are supposed to work on niantic and fortville: > > case 1) calculate checksum of out_ip (was case A in [1]) > >mb->l2_len = len(out_eth) >mb->l3_len = len(out_ip) >mb->ol_flags |= PKT_TX_IPV4 | PKT_TX_IP_CSUM >set out_ip checksum to 0 in the packet > >supported on hardware advertising DEV_TX_OFFLOAD_IPV4_CKSUM > > case 2) calculate checksum of out_ip and out_udp > >mb->l2_len = len(out_eth) >mb->l3_len = len(out_ip) >mb->ol_flags |= PKT_TX_IPV4 | PKT_TX_IP_CSUM | PKT_TX_UDP_CKSUM >set out_ip checksum to 0 in the packet >set out_udp checksum to pseudo header using rte_ipv4_phdr_cksum() > >supported on hardware advertising DEV_TX_OFFLOAD_IPV4_CKSUM and >DEV_TX_OFFLOAD_UDP_CKSUM > >*Problem 1*: The comment above PKT_TX_IPV4 says "Packet is IPv4 >without requiring IP checksum offload" [2], and the help of L4 >checksum and TSO says that it is required to set the PKT_TX_IPV4 >flag [3]. This is not coherent. So what is the problem? Comments in rte_mbuf.h are not coherent? > >We are back on the debate about the meaning of PKT_TX_IPV4 vs >PKT_TX_IP_CSUM from [4]. This incoherency in comments are introduced >by patch [5]. The question is "when an application should set >this flag? for any IP packet that does not require IP checksum?". Yes, if it is an IPv4 packet and application required TX offload for L4 checksum or TSO, but doesn't want HW offload ofr IPV4 checksum calculation. >This would break many applications. Which ones? As I know, so far nothing is broken. > I think a good definition would >be: > >Packet is IPv4. This flag must be set when using any offload >feature (TSO, L3 or L4 checksum) to tell the NIC that the packet >is an IPv4 packet. > >That's why I added PKT_TX_IPV4 in the examples. I suppose we discussed it several times: both ways are possible. >From PMD perspective - treating PKT_TX_IPV4 and PKT_TX_IP_CSUM As mutually exclusive seems a bit more plausible. >From the upper layer - my understanding, that it is doesn't really matter. I thought we had an agreement about it in 1.8, no? > > case 3) calculate checksum of in_ip > >mb->l2_len = len(out_eth + out_ip + out_udp + vxlan + in_eth) >mb->l3_len = len(in_ip) >mb->ol_flags |= PKT_TX_IPV4 | PKT_TX_IP_CSUM >set in_ip checksum to 0 in the packet > >This is similar to case 1), but l2_len is different. > >supported on hardware advertising DEV_TX_OFFLOAD_IPV4_CKSUM > >Note that it can only work if outer L4 checksum is 0. > > case 4) calculate checksum of in_ip and in_tcp (was case B.2 in [1]) > >mb->l2_len = len(out_eth + out_ip + out_udp + vxlan + in_eth) >mb->l3_len = len(in_ip) >mb->ol_flags |= PKT_TX_IPV4 | PKT_TX_IP_CSUM | PKT_TX_TCP_CKSUM >set in_ip checksum to 0 in the packet >set in_tcp checksum to pseudo header using rte_ipv4_phdr_cksum() > >This is similar to case 2), but l2_len is different. > >supported on hardware advertising DEV_TX_OFFLOAD_IPV4_CKSUM and >DEV_TX_OFFLOAD_TCP_CKSUM >
[dpdk-dev] [PATCH v3 0/3] enhance TX checksum command and csum forwarding engine
Hi Konstantin, Hi Jijiang, On 01/15/2015 02:31 PM, Ananyev, Konstantin wrote: > To be honest, there are so many mails around that subject, so I am already > lost :) > Oliver, as I understand you are not happy with the test-pmd commands Frank is > proposing. > Both syntax and semantics. > Is that correct? > If so, could you suggest something from your side? > That would allow to configure test-pmd to behave in any of 4 possible ways we > discussed previously: > http://dpdk.org/ml/archives/dev/2014-December/009213.html I first wanted to send a mail to describe the current problem with testpmd command line and the 2 solutions (Jijiang's and mine). But, first, I think we need to fully clarify the checksum offload API through examples as it will help to implement testpmd and do the documentation. They are based on Jijiang's previous mail [1]. I will submit a patchset fixing the problems described below in the coming days. If we agree on it, I'll submit another one for testpmd. Let's use the following packet for all the examples below: out_eth / out_ip / out_udp / vxlan / in_eth / in_ip / in_tcp The following cases are supposed to work on niantic and fortville: case 1) calculate checksum of out_ip (was case A in [1]) mb->l2_len = len(out_eth) mb->l3_len = len(out_ip) mb->ol_flags |= PKT_TX_IPV4 | PKT_TX_IP_CSUM set out_ip checksum to 0 in the packet supported on hardware advertising DEV_TX_OFFLOAD_IPV4_CKSUM case 2) calculate checksum of out_ip and out_udp mb->l2_len = len(out_eth) mb->l3_len = len(out_ip) mb->ol_flags |= PKT_TX_IPV4 | PKT_TX_IP_CSUM | PKT_TX_UDP_CKSUM set out_ip checksum to 0 in the packet set out_udp checksum to pseudo header using rte_ipv4_phdr_cksum() supported on hardware advertising DEV_TX_OFFLOAD_IPV4_CKSUM and DEV_TX_OFFLOAD_UDP_CKSUM *Problem 1*: The comment above PKT_TX_IPV4 says "Packet is IPv4 without requiring IP checksum offload" [2], and the help of L4 checksum and TSO says that it is required to set the PKT_TX_IPV4 flag [3]. This is not coherent. We are back on the debate about the meaning of PKT_TX_IPV4 vs PKT_TX_IP_CSUM from [4]. This incoherency in comments are introduced by patch [5]. The question is "when an application should set this flag? for any IP packet that does not require IP checksum?". This would break many applications. I think a good definition would be: Packet is IPv4. This flag must be set when using any offload feature (TSO, L3 or L4 checksum) to tell the NIC that the packet is an IPv4 packet. That's why I added PKT_TX_IPV4 in the examples. case 3) calculate checksum of in_ip mb->l2_len = len(out_eth + out_ip + out_udp + vxlan + in_eth) mb->l3_len = len(in_ip) mb->ol_flags |= PKT_TX_IPV4 | PKT_TX_IP_CSUM set in_ip checksum to 0 in the packet This is similar to case 1), but l2_len is different. supported on hardware advertising DEV_TX_OFFLOAD_IPV4_CKSUM Note that it can only work if outer L4 checksum is 0. case 4) calculate checksum of in_ip and in_tcp (was case B.2 in [1]) mb->l2_len = len(out_eth + out_ip + out_udp + vxlan + in_eth) mb->l3_len = len(in_ip) mb->ol_flags |= PKT_TX_IPV4 | PKT_TX_IP_CSUM | PKT_TX_TCP_CKSUM set in_ip checksum to 0 in the packet set in_tcp checksum to pseudo header using rte_ipv4_phdr_cksum() This is similar to case 2), but l2_len is different. supported on hardware advertising DEV_TX_OFFLOAD_IPV4_CKSUM and DEV_TX_OFFLOAD_TCP_CKSUM Note that it can only work if outer L4 checksum is 0. case 5) segment inner TCP mb->l2_len = len(out_eth + out_ip + out_udp + vxlan + in_eth) mb->l3_len = len(in_ip) mb->l4_len = len(in_tcp) mb->ol_flags |= PKT_TX_IPV4 | PKT_TX_IP_CKSUM | PKT_TX_TCP_CKSUM | PKT_TX_TCP_SEG; set in_ip checksum to 0 in the packet set in_tcp checksum to pseudo header without including the IP payload length using rte_ipv4_phdr_cksum() supported on hardware advertising DEV_TX_OFFLOAD_TCP_TSO. Note that it can only work if outer L4 checksum is 0. Problem 1 is also visible here. The following cases are supposed to *work on fortville*: case 6) calculate checksum of out_ip, in_ip, in_tcp (was case C in [1]) mb->outer_l2_len = len(out_eth) mb->outer_l3_len = len(out_ip) mb->l2_len = len(out_udp + vxlan + in_eth) mb->l3_len = len(in_ip) mb->ol_flags |= PKT_TX_OUTER_IPV4 | PKT_TX_OUTER_IP_CKSUM | \ PKT_TX_IP_CKSUM | PKT_TX_TCP_CKSUM; set out_ip checksum to 0 in the packet set in_ip checksum to 0 in the packet set in_tcp checksum to pseudo header using rte_ipv4_phdr_cksum() supported on hardware advertising DEV_TX_OFFLOAD_IPV4_CKSUM, DEV_TX_OFFLOAD_UDP_CKSUM and DEV_TX_OFFLOAD_OUTER_IPV4_CKSUM *Problem 2*: it is not written in the API comments that out_ip checksum field should be set to 0 by the application. They should be enhanced. *Problem 3*: without using the word "fortville",
[dpdk-dev] [PATCH v3 0/3] enhance TX checksum command and csum forwarding engine
Hi lads,
> -Original Message-
> From: Liu, Jijiang
> Sent: Wednesday, January 14, 2015 3:01 AM
> To: Olivier MATZ
> Cc: dev at dpdk.org; Ananyev, Konstantin
> Subject: RE: [dpdk-dev] [PATCH v3 0/3] enhance TX checksum command and csum
> forwarding engine
>
> Hi Olivier,
>
> > -Original Message-
> > From: Olivier MATZ [mailto:olivier.matz at 6wind.com]
> > Sent: Tuesday, January 13, 2015 5:56 PM
> > To: Liu, Jijiang
> > Cc: dev at dpdk.org; Ananyev, Konstantin
> > Subject: Re: [dpdk-dev] [PATCH v3 0/3] enhance TX checksum command and
> > csum forwarding engine
> >
> > Hi Jijiang,
> >
> > On 01/13/2015 04:04 AM, Liu, Jijiang wrote:
> > > the following two commands are.
> > >
> > > 1. tx_checksum set sw-tunnel-mode on/off
> > >
> > > 2. tx_checksum set hw-tunnel-mode on/off
> > >
> > > For command 1, If the sw-tunnel-mode is set/clear, which will
> > > set/clear a testpmd flag that is used in the process of analyzing
> > > incoming packet., the pseudo-codes are list below,
> > >
> > > If (sw-tunnel-mode)
> > >
> > > Csum fwd engine will analyze if incoming packet is a tunneling packet.
> > > tunnel = 1;
> > > else
> > > Csum fwd engine will not analyze if incoming packet is a
> > > tunneling
> > packet, and treat all the incoming packets as non-tunneling packets.
> > > It is used for A.
> >
> > What about "recognize-tunnel" instead of "sw-tunnel-mode"?
> > Or "parse-tunnel"?
>
> Ok, "parse-tunnel" or "parse-tunnel-pkt" is better.
> Thanks.
>
>
> > To me, using "sw-" or "hw-" prefix is confusing because in any case the
> > checksums
> > can be calculated in software or hardware depending on "tx_checksum set
> > outer-
> > ip hw|sw".
> >
> > Moreover, this command has an impact on receive side, but the name is still
> > "tx_checksum". Maybe this is also confusing.
> Ok, how about this?
>
> set checksum parse-tunnel-pkt on|off (port-id)
>
> > > For command 2, If the hw-tunnel-mode is set/clear, which will
> > > set/clear a testpmd flag that is used in the process of how to handle
> > > tunneling packet, the pseudo-codes are list below,
> > >
> > > if (tunnel == 1) { // this is a tunneling packet
> > > If (hw-tunnel-mode)
> > >ol_flags |= PKT_TX_UDP_TUNNEL_PKT;
> > >
> > > Csum fwd engine set PKT_TX_UDP_TUNNEL_PKT offload flag, which
> > means to tell HW treat the transmit packet as a tunneling packet to do
> > checksum
> > offload.
> > > It is used for B.1
> > > Else
> > >Csum fwd engine doesn't set PKT_TX_UDP_TUNNEL_PKT
> > > offload
> > flag, which means tell HW to treat the packet as ordinary (non-tunnelled)
> > packet.
> > > It is used for B.2
> > > }
> >
> > What about:
> >tx_checksum set tunnel-method normal|outer
> > It would select if we use lX_len or outer_lX_len. Is it what you mean?
>
> tx_checksum set tunnel-method normal|outer
>
> Let me explain that what differences of TX checksum mechanism between
> ixgbe(82599) and i40e(40G NIC) are.
>
> For 82599, there is only one register that is used for L3 checksum offload.
> So for tunneling packet, hardware is unable to recognize the
> packet is tunneling packet and the register cannot be worked for both outer
> L3 checksum offload and inner L3 checksum offload at the
> same time, just for outer or inner.
>
> For i40e(40G NIC), there are two registers that are user for L3 TX checksum
> offload, so for tunneling packet, the outer and inner L3
> checksum offload can be done by hardware at the same time, but a
> prerequisite is that we must tell
> Hardware the packet is a tunneling packet by setting a register
> (PKT_TX_UDP_TUNNEL_PKT offload flag is used to indicate to set this
> register.)
>
> As for other NIC, I think its working mechanism should be same as the i40e if
> it can recognize tunneling packet.
>
> So my idea:
> tx_checksum set tunnel-method tunnel-pkt on|off
>
> or
> tx_checksum set tunnel-pkt on|off
>
> What do you think?
>
>
> > And this only makes sense when we use hw checksum right?
> yes
>
> >
> > >> And will it be possible to support future hardware that will be able
> >
[dpdk-dev] [PATCH v3 0/3] enhance TX checksum command and csum forwarding engine
Hi Olivier,
> -Original Message-
> From: Olivier MATZ [mailto:olivier.matz at 6wind.com]
> Sent: Tuesday, January 13, 2015 5:56 PM
> To: Liu, Jijiang
> Cc: dev at dpdk.org; Ananyev, Konstantin
> Subject: Re: [dpdk-dev] [PATCH v3 0/3] enhance TX checksum command and
> csum forwarding engine
>
> Hi Jijiang,
>
> On 01/13/2015 04:04 AM, Liu, Jijiang wrote:
> > the following two commands are.
> >
> > 1. tx_checksum set sw-tunnel-mode on/off
> >
> > 2. tx_checksum set hw-tunnel-mode on/off
> >
> > For command 1, If the sw-tunnel-mode is set/clear, which will
> > set/clear a testpmd flag that is used in the process of analyzing
> > incoming packet., the pseudo-codes are list below,
> >
> > If (sw-tunnel-mode)
> >
> > Csum fwd engine will analyze if incoming packet is a tunneling packet.
> > tunnel = 1;
> > else
> > Csum fwd engine will not analyze if incoming packet is a
> > tunneling
> packet, and treat all the incoming packets as non-tunneling packets.
> > It is used for A.
>
> What about "recognize-tunnel" instead of "sw-tunnel-mode"?
> Or "parse-tunnel"?
Ok, "parse-tunnel" or "parse-tunnel-pkt" is better.
Thanks.
> To me, using "sw-" or "hw-" prefix is confusing because in any case the
> checksums
> can be calculated in software or hardware depending on "tx_checksum set outer-
> ip hw|sw".
>
> Moreover, this command has an impact on receive side, but the name is still
> "tx_checksum". Maybe this is also confusing.
Ok, how about this?
set checksum parse-tunnel-pkt on|off (port-id)
> > For command 2, If the hw-tunnel-mode is set/clear, which will
> > set/clear a testpmd flag that is used in the process of how to handle
> > tunneling packet, the pseudo-codes are list below,
> >
> > if (tunnel == 1) { // this is a tunneling packet
> > If (hw-tunnel-mode)
> >ol_flags |= PKT_TX_UDP_TUNNEL_PKT;
> >
> >Csum fwd engine set PKT_TX_UDP_TUNNEL_PKT offload flag, which
> means to tell HW treat the transmit packet as a tunneling packet to do
> checksum
> offload.
> >It is used for B.1
> > Else
> >Csum fwd engine doesn't set PKT_TX_UDP_TUNNEL_PKT
> > offload
> flag, which means tell HW to treat the packet as ordinary (non-tunnelled)
> packet.
> > It is used for B.2
> > }
>
> What about:
>tx_checksum set tunnel-method normal|outer
> It would select if we use lX_len or outer_lX_len. Is it what you mean?
tx_checksum set tunnel-method normal|outer
Let me explain that what differences of TX checksum mechanism between
ixgbe(82599) and i40e(40G NIC) are.
For 82599, there is only one register that is used for L3 checksum offload. So
for tunneling packet, hardware is unable to recognize the packet is tunneling
packet and the register cannot be worked for both outer L3 checksum offload
and inner L3 checksum offload at the same time, just for outer or inner.
For i40e(40G NIC), there are two registers that are user for L3 TX checksum
offload, so for tunneling packet, the outer and inner L3 checksum offload can
be done by hardware at the same time, but a prerequisite is that we must tell
Hardware the packet is a tunneling packet by setting a register
(PKT_TX_UDP_TUNNEL_PKT offload flag is used to indicate to set this register.)
As for other NIC, I think its working mechanism should be same as the i40e if
it can recognize tunneling packet.
So my idea:
tx_checksum set tunnel-method tunnel-pkt on|off
or
tx_checksum set tunnel-pkt on|off
What do you think?
> And this only makes sense when we use hw checksum right?
yes
>
> >> And will it be possible to support future hardware that will be able
> >> to compute both outer l3, outer l4, l3 and l4 checksums?
Currently, if outer l4 will be supported in the future, and we can add
outer-udp/tcp option into following command.
Tx_checksum set outer-ip|ip|sctp|udp|tcp.
> > Yes.
> > Currently, i40e support outer l3, outer l4, l3 and l4 checksums offload at
> > the
> same time.
Sorry, my bad.
I40e just support outer l3, l3 and l4.
Fortville can offload the following L3 and L4 integrity checks: IPv4 header(s)
checksum for "simple" and tunneled packets, Inner TCP or UDP checksum and SCTP
CRC integrity. Tunneling UDP headers and GRE header are not offloaded while
Fortville leaves their checksum field as is. If a checksum is required,
software should provide it as well as the inner checksum value(s) that are
required for the outer checksum.
>
> &g
[dpdk-dev] [PATCH v3 0/3] enhance TX checksum command and csum forwarding engine
Hi Jijiang,
On 01/13/2015 04:04 AM, Liu, Jijiang wrote:
> the following two commands are.
>
> 1. tx_checksum set sw-tunnel-mode on/off
>
> 2. tx_checksum set hw-tunnel-mode on/off
>
> For command 1, If the sw-tunnel-mode is set/clear, which will set/clear a
> testpmd flag that is used in the process of analyzing incoming packet., the
> pseudo-codes are list below,
>
> If (sw-tunnel-mode)
>
> Csum fwd engine will analyze if incoming packet is a tunneling packet.
> tunnel = 1;
> else
> Csum fwd engine will not analyze if incoming packet is a
> tunneling packet, and treat all the incoming packets as non-tunneling packets.
> It is used for A.
What about "recognize-tunnel" instead of "sw-tunnel-mode"?
Or "parse-tunnel"?
To me, using "sw-" or "hw-" prefix is confusing because in any case
the checksums can be calculated in software or hardware depending on
"tx_checksum set outer-ip hw|sw".
Moreover, this command has an impact on receive side, but the name
is still "tx_checksum". Maybe this is also confusing.
> For command 2, If the hw-tunnel-mode is set/clear, which will set/clear a
> testpmd flag that is used in the process of how to handle tunneling packet,
> the pseudo-codes are list below,
>
> if (tunnel == 1) { // this is a tunneling packet
> If (hw-tunnel-mode)
>ol_flags |= PKT_TX_UDP_TUNNEL_PKT;
>
> Csum fwd engine set PKT_TX_UDP_TUNNEL_PKT offload flag, which
> means to tell HW treat the transmit packet as a tunneling packet to do
> checksum offload.
> It is used for B.1
> Else
>Csum fwd engine doesn't set PKT_TX_UDP_TUNNEL_PKT
> offload flag, which means tell HW to treat the packet as ordinary
> (non-tunnelled) packet.
> It is used for B.2
> }
What about:
tx_checksum set tunnel-method normal|outer
It would select if we use lX_len or outer_lX_len. Is it what you mean?
And this only makes sense when we use hw checksum right?
>> And will it be possible to support future hardware that will be able to
>> compute
>> both outer l3, outer l4, l3 and l4 checksums?
>
> Yes.
> Currently, i40e support outer l3, outer l4, l3 and l4 checksums offload at
> the same time.
I probably missed something here: we only have PKT_TX_OUTER_IP_CKSUM
but there is no PKT_TX_OUTER_UDP_CKSUM. Is outer UDP checksum supported
> test case C:
> tx_checksum set tunnel-mode hw
> tx_checksum set outer-ip hw
> tx_checksum set ip hw
> tx_checksum set tcp hw
>
> Of course, outer udp is not listed here for VXLAN.
I don't understand why. Could you detail it?
>> I have another idea, please let me know if you find it clearer or not.
>> The commands format would be:
>>
>> tx_checksum ...
>>
>> [...]
>>
>> What do you think?
>
> Thanks for your proposal.
> It is clear for me.
>
> But there are two questions for me.
>
> As I know, in current command line framework, the option in command line is
> exact match, so you probably have to add duplicated codes when you want to
> support a new packet types.
I don't think it's really a problem. The cmdline library supports
string list, so can have the following 3 commands definitions:
1. tx_checksum
ip-udp|ip-tcp|ip-sctp|vxlan-ip-udp|vxlan-ip-tcp|vxlan-ip-sctp l3
off|sw|hw l4 off|sw|hw
2. tx_checksum ip-other|vxlan-ip-other l3 off|sw|hw
3. tx_checksum vxlan outer-l3 off|sw|hw outer-l4 off|sw|hw
Maybe 1 and 2 could be splitted in non-vxlan and vxlan. But only
the structure should be redefined to have a different help string,
not the callback function.
> Other question:
>
> Currently, the following testpmd flag is for per port, not for per packet
> type, when they are set, which will affect whole port, not just for packet
> type or format, if you add option in cmdline, which means you
> have to other changes.
>
> /** Offload IP checksum in csum forward engine */
> #define TESTPMD_TX_OFFLOAD_IP_CKSUM 0x0001
> /** Offload UDP checksum in csum forward engine */
> #define TESTPMD_TX_OFFLOAD_UDP_CKSUM 0x0002
> /** Offload TCP checksum in csum forward engine */
> #define TESTPMD_TX_OFFLOAD_TCP_CKSUM 0x0004
> /** Offload SCTP checksum in csum forward engine */
> #define TESTPMD_TX_OFFLOAD_SCTP_CKSUM0x0008
> /** Offload VxLAN checksum in csum forward engine */
> #define TESTPMD_TX_OFFLOAD_VXLAN_CKSUM 0x0010
We can add a portid in each command.
> Of course, it is welcome if you can send this patch set with this idea for
> community review.
Let's first agree on the user API :)
Regards,
Olivier
[dpdk-dev] [PATCH v3 0/3] enhance TX checksum command and csum forwarding engine
Hi Jijiang, Please find some comments below. On 01/12/2015 04:41 AM, Liu, Jijiang wrote: > There are some examples for the different packet types: > > 1. For L2 Packet types: > MAC, ARP > MAC, PAY2 > ... > They are forwarded without beeing modified no matter if these above commands > are set. ok > 2. For Non Tunneled IPv4/6 packet > MAC, IPV4, UDP, PAY4 > MAC, IPV6, UDP, PAY4 > ... > Ipv4: > tx_checksum set ip hw > tx_checksum set udp hw > > IPv6: > tx_checksum set udp hw > > They are forwarded with TX checksum offload if these above commands are set. Two questions here: - today, we also have the "sw" argument that allows to calculate the checksum in software. Do you plan to keep this behavior? - today, the csumonly forward engine modifies the IP addresses to validate that it is able to recalculate the checksum. Do you plan to keep this behavior? I'm not opposed to remove it if it makes the code more complex. > 3. For Tunneled IPv4/6 packet > > See the above test cases: > Test case A > test case B.1 > test case B.2 > test case C > > They are forwarded with TX checksum offload if these above commands are set. > >> I think that the test-pmd command API should define a behavior for the csum >> forward engine for any packet. What do you think? > > Agree. > > Let me explain the checksum offload behavior of different packet type below, > > 1. For L2 Packet types: > Checksum offload behavior definition: > tx_checksum set sw-tunnel-mode on : NONE > tx_checksum set hw-tunnel-mode on: NONE > tx_checksum set outer-ip|ip|tcp|udp|sctp hw: NONE > > 2. For Non Tunneled IPv4/6 packet > > Checksum offload behavior definition: > > tx_checksum set sw-tunnel-mode on :NONE > tx_checksum set hw-tunnel-mode on: NONE > tx_checksum set outer-ip|ip|tcp|udp|sctp hw: ip|tcp|udp|sctp options > are VALID > > 3. For Tunneled IPv4/6 packet > Checksum offload behavior definition: > > tx_checksum set sw-tunnel-mode on :VALID > tx_checksum set hw-tunnel-mode on: VALID > tx_checksum set outer-ip|ip|tcp|udp|sctp hw: VALID > > It is very welcome if you have better solution that is able to cover all the > case in the http://dpdk.org/ml/archives/dev/2014-December/009213.html and > all packet types in csum fwd engine. Thank you for your efforts to explain your proposition. I still have some difficulties to understand the naming "sw-tunnel" and "hw-tunnel". >From the user point of view "sw" means "software" and "hw" means "hardware". I think it's difficult to understand how both can be on at the same time. Maybe it's just a naming problem? Also, is it still possible to compute the checksum in software? And will it be possible to support future hardware that will be able to compute both outer l3, outer l4, l3 and l4 checksums? I have another idea, please let me know if you find it clearer or not. The commands format would be: tx_checksum ... List of commands: # select behavior for non tunnel packets tx_checksum ip-udp l3 off|sw|hw l4 off|sw|hw tx_checksum ip-tcp l3 off|sw|hw l4 off|sw|hw tx_checksum ip-sctp l3 off|sw|hw l4 off|sw|hw tx_checksum ip-other l3 off|sw|hw # select behavior for vxlan packets tx_checksum vxlan outer-l3 off|sw|hw outer-l4 off|sw|hw tx_checksum vxlan-ip-udp l3 off|sw|hw l4 off|sw|hw tx_checksum vxlan-ip-tcp l3 off|sw|hw l4 off|sw|hw tx_checksum vxlan-ip-sctp l3 off|sw|hw l4 off|sw|hw tx_checksum vxlan-ip-other l3 off|sw|hw Examples: 1. calculate l3 and l4 checksum of ip/udp packets in hw, and ip/tcp packets in sw. Do nothing for the other packet types # assume all is off by default tx_checksum ip-udp l3 hw l4 hw tx_checksum ip-tcp l3 sw l4 sw 2. calculate outer checksums of tunnel packets (your case A.) # assume all is off by default tx_checksum vxlan outer-l3 hw outer-l4 hw 3. calculate inner checksums of tunnel packets (your case B.1) # assume all is off by default tx_checksum vxlan-ip-udp l3 hw l4 hw tx_checksum vxlan-ip-tcp l3 hw l4 hw tx_checksum vxlan-ip-sctp l3 hw l4 hw 4. calculate all checksums of tunnel packets (your case C) # assume all is off by default tx_checksum vxlan outer-l3 hw outer-l4 hw tx_checksum vxlan-ip-udp l3 hw l4 hw tx_checksum vxlan-ip-tcp l3 hw l4 hw tx_checksum vxlan-ip-sctp l3 hw l4 hw Advantages: - clearer from use point of view: the user knows what is done for each packet type - software checksum is supported for comparison with hw - the syntax already supports future hw that can do outer l3, outer l4, l3 and l4 at the same time. - we can add future tunnel packets in the same model: tx_checksum gre outer-l3 off|sw|hw tx_checksum gre-ip-udp l3 off|sw|hw l4 off|sw|hw Cons: - with the definition above, we cannot do B.2. But if we really want it, we could change the commands: tx_checksum xxx l3 off|sw|hw|outer-hw l4 off|sw|hw|outer-hw "outer-hw" means: use the outer mbuf flags to
[dpdk-dev] [PATCH v3 0/3] enhance TX checksum command and csum forwarding engine
Hi, Thank you Jijiang for taking the time to get back on this. On 01/08/2015 11:54 AM, Ananyev, Konstantin wrote: >> And we are able to test all of cases in >> http://dpdk.org/ml/archives/dev/2014-December/009213.html >> >> Test case A: >> >> tx_checksum set sw-tunnel-mode off >> tx_checksum set hw-tunnel-mode off >> tx_checksum set ip hw >> >> test case B.1: >> >> tx_checksum set sw-tunnel-mode on >> tx_checksum set hw-tunnel-mode on >> tx_checksum set ip hw >> tx_checksum set tcp hw >> >> test case B.2: >> >> tx_checksum set sw-tunnel-mode on >> tx_checksum set hw-tunnel-mode off >> tx_checksum set ip hw >> >> test case C: >> >> tx_checksum set sw-tunnel-mode on >> tx_checksum set hw-tunnel-mode on >> tx_checksum set outer-ip hw >> tx_checksum set ip hw >> tx_checksum set tcp hw There is something I don't understand. A forward engine takes any packet in input and output. Packets can be of any kind (eth/arp, eth/ip/tcp, eth/vlan/ip/udp/vxlan/eth/ip/tcp, ...) Today, the behavior of csum forward engine is defined for any kind of packet. See the description and the table in http://dpdk.org/ml/archives/dev/2014-December/009886.html All packets that are not "Ether[/vlan]/(IP|IP6)/(UDP|TCP|SCTP)" or "Ether[/vlan]/(IP|IP6)/UDP/VxLAN/Ether[/vlan]/(IP|IP6)/UDP|TCP|SCTP" are forwarded without beeing modified. In my understanding, the use-case you are describing correspond to specific packet types. So a configuration would work only for one packet format only. Is it correct? I think that the test-pmd command API should define a behavior for the csum forward engine for any packet. What do you think? Regards, Olivier
[dpdk-dev] [PATCH v3 0/3] enhance TX checksum command and csum forwarding engine
Hi Frank, > -Original Message- > From: Liu, Jijiang > Sent: Thursday, January 08, 2015 8:52 AM > To: Ananyev, Konstantin; 'Olivier MATZ' > Cc: dev at dpdk.org > Subject: RE: [dpdk-dev] [PATCH v3 0/3] enhance TX checksum command and csum > forwarding engine > > Hi, > > > -Original Message- > > From: Ananyev, Konstantin > > Sent: Wednesday, January 7, 2015 8:07 PM > > To: Liu, Jijiang; 'Olivier MATZ' > > Cc: dev at dpdk.org > > Subject: RE: [dpdk-dev] [PATCH v3 0/3] enhance TX checksum command and > > csum forwarding engine > > > > > > > > > -Original Message- > > > From: Liu, Jijiang > > > Sent: Wednesday, January 07, 2015 11:39 AM > > > To: Ananyev, Konstantin; 'Olivier MATZ' > > > Cc: dev at dpdk.org > > > Subject: RE: [dpdk-dev] [PATCH v3 0/3] enhance TX checksum command and > > > csum forwarding engine > > > > > > Hi Konstantin, > > > > > > > -Original Message- > > > > From: Ananyev, Konstantin > > > > Sent: Wednesday, January 7, 2015 5:59 PM > > > > To: Liu, Jijiang; 'Olivier MATZ' > > > > Cc: dev at dpdk.org > > > > Subject: RE: [dpdk-dev] [PATCH v3 0/3] enhance TX checksum command > > > > and csum forwarding engine > > > > > > > > Hi Frank, > > > > > > > > > -Original Message- > > > > > From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Liu, Jijiang > > > > > Sent: Wednesday, January 07, 2015 2:04 AM > > > > > To: 'Olivier MATZ' > > > > > Cc: dev at dpdk.org > > > > > Subject: Re: [dpdk-dev] [PATCH v3 0/3] enhance TX checksum command > > > > > and csum forwarding engine > > > > > > > > > > Hi Olivier, > > > > > > > > > > > -Original Message- > > > > > > From: Olivier MATZ [mailto:olivier.matz at 6wind.com] > > > > > > Sent: Saturday, December 13, 2014 12:33 AM > > > > > > To: Liu, Jijiang > > > > > > Cc: dev at dpdk.org > > > > > > Subject: Re: [dpdk-dev] [PATCH v3 0/3] enhance TX checksum > > > > > > command and csum forwarding engine > > > > > > > > > > > > Hello, > > > > > > > > > > > > On 12/12/2014 04:48 AM, Liu, Jijiang wrote: > > > > > > > The 'hw/sw' option is used to set/clear the flag of enabling > > > > > > > TX tunneling packet > > > > > > checksum hardware offload in testpmd application. > > > > > > > > > > > > This is not clear at all. > > > > > > In your command, there is (hw|sw|none). > > > > > > Are you talking about inner or outer? > > > > > > Is this command useful for any kind of packet? > > > > > > How does it combine with "tx_checksum set outer-ip (hw|sw)"? > > > > > > > > > > > > > > > > I rethink these TX checksum commands in this patch set and agree > > > > > with you that we should make some changes for having clear meaning for > > them. > > > > > > > > > > There are 3 commands in patch set as follows, 1. tx_checksum set > > > > > tunnel (hw|sw|none) (port-id) > > > > > > > > > > Now I also think the command 1 may confuse user, they probably > > > > > don't understand why we need 'hw' or 'sw' option and when to use > > > > > the two option, so I will replace the command with 'tx_checksum > > > > > set hw-tunnel-mode > > > > (on|off) (port-id)' command. > > > > > > > > I am a bit confused here, could you explain what would be a > > > > behaviour for 'on' and 'off'? > > > > Konstantin > > > > > > I have explained the behaviour for 'on' and'off' below, > > > > > > The command 'tx_checksum set hw-tunnel-mode (on|off) (port-id)' is > > > used to set/clear TESTPMD_TX_OFFLOAD_TUNNEL_CKSUM flag. > > > > > > Actually, the PKT_TX_UDP_TUNNEL_PKT offload flag will be set if the > > > testpmd flag is set, which means to tell HW treat that transmit > > > packet as a tunneling packet to do checksum offload When 'on' is set, > > > which is > > able to meet Method B.1 and method C. > > > > > > When 'off' is set, the TESTPMD_TX_OFFLOAD_TUNNEL_CKSUM is not need
[dpdk-dev] [PATCH v3 0/3] enhance TX checksum command and csum forwarding engine
Hi, > -Original Message- > From: Ananyev, Konstantin > Sent: Wednesday, January 7, 2015 8:07 PM > To: Liu, Jijiang; 'Olivier MATZ' > Cc: dev at dpdk.org > Subject: RE: [dpdk-dev] [PATCH v3 0/3] enhance TX checksum command and > csum forwarding engine > > > > > -Original Message- > > From: Liu, Jijiang > > Sent: Wednesday, January 07, 2015 11:39 AM > > To: Ananyev, Konstantin; 'Olivier MATZ' > > Cc: dev at dpdk.org > > Subject: RE: [dpdk-dev] [PATCH v3 0/3] enhance TX checksum command and > > csum forwarding engine > > > > Hi Konstantin, > > > > > -Original Message- > > > From: Ananyev, Konstantin > > > Sent: Wednesday, January 7, 2015 5:59 PM > > > To: Liu, Jijiang; 'Olivier MATZ' > > > Cc: dev at dpdk.org > > > Subject: RE: [dpdk-dev] [PATCH v3 0/3] enhance TX checksum command > > > and csum forwarding engine > > > > > > Hi Frank, > > > > > > > -Original Message- > > > > From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Liu, Jijiang > > > > Sent: Wednesday, January 07, 2015 2:04 AM > > > > To: 'Olivier MATZ' > > > > Cc: dev at dpdk.org > > > > Subject: Re: [dpdk-dev] [PATCH v3 0/3] enhance TX checksum command > > > > and csum forwarding engine > > > > > > > > Hi Olivier, > > > > > > > > > -Original Message- > > > > > From: Olivier MATZ [mailto:olivier.matz at 6wind.com] > > > > > Sent: Saturday, December 13, 2014 12:33 AM > > > > > To: Liu, Jijiang > > > > > Cc: dev at dpdk.org > > > > > Subject: Re: [dpdk-dev] [PATCH v3 0/3] enhance TX checksum > > > > > command and csum forwarding engine > > > > > > > > > > Hello, > > > > > > > > > > On 12/12/2014 04:48 AM, Liu, Jijiang wrote: > > > > > > The 'hw/sw' option is used to set/clear the flag of enabling > > > > > > TX tunneling packet > > > > > checksum hardware offload in testpmd application. > > > > > > > > > > This is not clear at all. > > > > > In your command, there is (hw|sw|none). > > > > > Are you talking about inner or outer? > > > > > Is this command useful for any kind of packet? > > > > > How does it combine with "tx_checksum set outer-ip (hw|sw)"? > > > > > > > > > > > > > I rethink these TX checksum commands in this patch set and agree > > > > with you that we should make some changes for having clear meaning for > them. > > > > > > > > There are 3 commands in patch set as follows, 1. tx_checksum set > > > > tunnel (hw|sw|none) (port-id) > > > > > > > > Now I also think the command 1 may confuse user, they probably > > > > don't understand why we need 'hw' or 'sw' option and when to use > > > > the two option, so I will replace the command with 'tx_checksum > > > > set hw-tunnel-mode > > > (on|off) (port-id)' command. > > > > > > I am a bit confused here, could you explain what would be a > > > behaviour for 'on' and 'off'? > > > Konstantin > > > > I have explained the behaviour for 'on' and'off' below, > > > > The command 'tx_checksum set hw-tunnel-mode (on|off) (port-id)' is > > used to set/clear TESTPMD_TX_OFFLOAD_TUNNEL_CKSUM flag. > > > > Actually, the PKT_TX_UDP_TUNNEL_PKT offload flag will be set if the > > testpmd flag is set, which means to tell HW treat that transmit > > packet as a tunneling packet to do checksum offload When 'on' is set, > > which is > able to meet Method B.1 and method C. > > > > When 'off' is set, the TESTPMD_TX_OFFLOAD_TUNNEL_CKSUM is not needed > > to set, so the PKT_TX_UDP_TUNNEL_PKT offload flag is not needed to > > set, then HW treat that transmit packet as a non-tunneling packet. It is > > able to > meet Method B.2. > > > > Is the explanation not clear? > > Ok, and how I can set method A (testpmd treat all packets as non-tunnelling > and > never look beyond outer L4 header) then? > Konstantin > > > > As to case A, I think it is not mandatory to cover it in csum fwd > > > > engine for tunneling packet. If you think the case A is essential, and it must be covered in csum fwd, then we can add a command: tx_checksum set sw-tunnel-mode (on|off) (port-id) if the 'off' is set , csum fwd engine don't
[dpdk-dev] [PATCH v3 0/3] enhance TX checksum command and csum forwarding engine
On 12/10/2014 9:04 AM, Jijiang Liu wrote: > In the current codes, the "tx_checksum set (ip|udp|tcp|sctp|vxlan) (hw|sw) > (port-id)" command is not easy to understand and extend, so the patch set > enhances the tx_checksum command and reworks csum forwarding engine due to > the change of tx_checksum command. > The main changes of the tx_checksum command are listed below, > > 1> add "tx_checksum set tunnel (hw|sw|none) (port-id)" command > > The command is used to set/clear tunnel flag that is used to tell the NIC > that the packetg is a tunneing packet and application want hardware TX > checksum offload for outer layer, or inner layer, or both. > > The 'none' option means that user treat tunneling packet as ordinary packet > when using the csum forward engine. > for example, let say we have a tunnel packet: > eth_hdr_out/ipv4_hdr_out/udp_hdr_out/vxlan_hdr/ehtr_hdr_in/ipv4_hdr_in/tcp_hdr_in. > one of several scenarios: > > 1) User requests HW offload for ipv4_hdr_out checksum, and doesn't care is > it a tunnelled packet or not. So he sets: > > tx_checksum set tunnel none 0 > > tx_checksum set ip hw 0 Hi Jijiang, I have one question, you know lots of command need port-id field like here, why we do not put port-id just after the command? like below: tx_checksum (port-id) set tunnel (hw|sw|none) Then for users, if we do not care whether it is a tunneling packet, we just ignore the field after port-id. tx_checksum (port-id) For code it maybe simpler to praise command, and better for user. What all I mean is, we can put the required parameters just close the command and put the optional parameters(or can be optional) at the end of the command line. (Command) (required parameter) (optional parameters) Thus, it would be a better user experience. But just personal idea. Thanks, Michael > > So for such case we should set tx_tunnel to 'none'. > > 2> add "tx_checksum set outer-ip (hw|sw) (port-id)" command > > The command is used to set/clear outer IP flag that is used to tell the NIC > that application want hardware offload for outer layer. > > 3> remove the 'vxlan' option from the "tx_checksum set > (ip|udp|tcp|sctp|vxlan) (hw|sw) (port-id)" command > > The command is used to set IP, UDP, TCP and SCTP TX checksum flag. In the > case of tunneling packet, the IP, UDP, TCP and SCTP flags always concern > inner layer. > > Moreover, replace the TESTPMD_TX_OFFLOAD_VXLAN_CKSUM flag with > TESTPMD_TX_OFFLOAD_TUNNEL_CKSUM flag and add the > TESTPMD_TX_OFFLOAD_OUTER_IP_CKSUM and TESTPMD_TX_OFFLOAD_NON_TUNNEL_CKSUM > flag in test-pmd application. > > v2 change: > redefine the 'none' behaviour for "tx_checksum set tunnel (hw|sw|none) > (port-id)" command. > v3 change: > typo correction in cmdline help > > Jijiang Liu (3): > add outer IP offload capability in librte_ether. > add outer IP checksum capability in i40e PMD > testpmd command lines of the tx_checksum and csum forwarding rework > > app/test-pmd/cmdline.c| 201 > +++-- > app/test-pmd/csumonly.c | 35 --- > app/test-pmd/testpmd.h|6 +- > lib/librte_ether/rte_ethdev.h |1 + > lib/librte_pmd_i40e/i40e_ethdev.c |3 +- > 5 files changed, 218 insertions(+), 28 deletions(-) >
[dpdk-dev] [PATCH v3 0/3] enhance TX checksum command and csum forwarding engine
> -Original Message- > From: Liu, Jijiang > Sent: Wednesday, January 07, 2015 11:39 AM > To: Ananyev, Konstantin; 'Olivier MATZ' > Cc: dev at dpdk.org > Subject: RE: [dpdk-dev] [PATCH v3 0/3] enhance TX checksum command and csum > forwarding engine > > Hi Konstantin, > > > -Original Message- > > From: Ananyev, Konstantin > > Sent: Wednesday, January 7, 2015 5:59 PM > > To: Liu, Jijiang; 'Olivier MATZ' > > Cc: dev at dpdk.org > > Subject: RE: [dpdk-dev] [PATCH v3 0/3] enhance TX checksum command and > > csum forwarding engine > > > > Hi Frank, > > > > > -Original Message- > > > From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Liu, Jijiang > > > Sent: Wednesday, January 07, 2015 2:04 AM > > > To: 'Olivier MATZ' > > > Cc: dev at dpdk.org > > > Subject: Re: [dpdk-dev] [PATCH v3 0/3] enhance TX checksum command and > > > csum forwarding engine > > > > > > Hi Olivier, > > > > > > > -Original Message----- > > > > From: Olivier MATZ [mailto:olivier.matz at 6wind.com] > > > > Sent: Saturday, December 13, 2014 12:33 AM > > > > To: Liu, Jijiang > > > > Cc: dev at dpdk.org > > > > Subject: Re: [dpdk-dev] [PATCH v3 0/3] enhance TX checksum command > > > > and csum forwarding engine > > > > > > > > Hello, > > > > > > > > On 12/12/2014 04:48 AM, Liu, Jijiang wrote: > > > > > The 'hw/sw' option is used to set/clear the flag of enabling TX > > > > > tunneling packet > > > > checksum hardware offload in testpmd application. > > > > > > > > This is not clear at all. > > > > In your command, there is (hw|sw|none). > > > > Are you talking about inner or outer? > > > > Is this command useful for any kind of packet? > > > > How does it combine with "tx_checksum set outer-ip (hw|sw)"? > > > > > > > > > > I rethink these TX checksum commands in this patch set and agree with > > > you that we should make some changes for having clear meaning for them. > > > > > > There are 3 commands in patch set as follows, 1. tx_checksum set > > > tunnel (hw|sw|none) (port-id) > > > > > > Now I also think the command 1 may confuse user, they probably don't > > > understand why we need 'hw' or 'sw' option and when to use the two > > > option, so I will replace the command with 'tx_checksum set hw-tunnel-mode > > (on|off) (port-id)' command. > > > > I am a bit confused here, could you explain what would be a behaviour for > > 'on' and > > 'off'? > > Konstantin > > I have explained the behaviour for 'on' and'off' below, > > The command 'tx_checksum set hw-tunnel-mode (on|off) (port-id)' is > used to set/clear TESTPMD_TX_OFFLOAD_TUNNEL_CKSUM flag. > > Actually, the PKT_TX_UDP_TUNNEL_PKT offload flag will be set if the > testpmd flag is set, which means to tell HW treat that transmit packet as a > tunneling packet to do checksum offload > When 'on' is set, which is able to meet Method B.1 and method C. > > When 'off' is set, the TESTPMD_TX_OFFLOAD_TUNNEL_CKSUM is not needed > to set, so the PKT_TX_UDP_TUNNEL_PKT offload flag is not needed to set, then > HW treat that transmit packet as a non-tunneling > packet. It is able to meet Method B.2. > > Is the explanation not clear? Ok, and how I can set method A (testpmd treat all packets as non-tunnelling and never look beyond outer L4 header) then? Konstantin > > > > > > > > > 2. tx_checksum set outer-ip (hw|sw) (port-id) 3. tx_checksum set > > > (ip|udp|tcp|sctp) (hw|sw) (port-id) > > > > > > The command 2 will be merged into command 3, the new command is ' > > > tx_checksum set (outer-ip|ip|udp|tcp|sctp) (hw|sw) (port- id)'. > > > > > > These most of the cases in > > > http://dpdk.org/ml/archives/dev/2014-December/009213.html will be > > > covered by using the two commands > > > > > > The command 'tx_checksum set hw-tunnel-mode (on|off) (port-id)' is > > > used to set/clear TESTPMD_TX_OFFLOAD_TUNNEL_CKSUM flag. > > > Actually, the PKT_TX_UDP_TUNNEL_PKT offload flag will be set if the > > > testpmd flag is set, which tell driver/HW treat that transmit packet as a > > tunneling packet. > > > > > > When 'on' is set, which is able to meet Method B.1 and method C. > > > > > > When 'off' is set, the TESTPMD_TX_OFFLOAD_TUNNEL_CKSUM is not needed > > > to set, so the PKT_TX_UDP_TUNNEL_PKT offload flag is not needed to set, > > > then > > HW treat that transmit packet as a non-tunneling packet. It is able to meet > > Method B.2. > > > > > > As to case A, I think it is not mandatory to cover it in csum fwd engine > > > for > > tunneling packet. > > > > > > Is the above description clear for you? > > > > > > > Regards, > > > > Olivier
[dpdk-dev] [PATCH v3 0/3] enhance TX checksum command and csum forwarding engine
Hi Konstantin, > -Original Message- > From: Ananyev, Konstantin > Sent: Wednesday, January 7, 2015 5:59 PM > To: Liu, Jijiang; 'Olivier MATZ' > Cc: dev at dpdk.org > Subject: RE: [dpdk-dev] [PATCH v3 0/3] enhance TX checksum command and > csum forwarding engine > > Hi Frank, > > > -Original Message- > > From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Liu, Jijiang > > Sent: Wednesday, January 07, 2015 2:04 AM > > To: 'Olivier MATZ' > > Cc: dev at dpdk.org > > Subject: Re: [dpdk-dev] [PATCH v3 0/3] enhance TX checksum command and > > csum forwarding engine > > > > Hi Olivier, > > > > > -Original Message- > > > From: Olivier MATZ [mailto:olivier.matz at 6wind.com] > > > Sent: Saturday, December 13, 2014 12:33 AM > > > To: Liu, Jijiang > > > Cc: dev at dpdk.org > > > Subject: Re: [dpdk-dev] [PATCH v3 0/3] enhance TX checksum command > > > and csum forwarding engine > > > > > > Hello, > > > > > > On 12/12/2014 04:48 AM, Liu, Jijiang wrote: > > > > The 'hw/sw' option is used to set/clear the flag of enabling TX > > > > tunneling packet > > > checksum hardware offload in testpmd application. > > > > > > This is not clear at all. > > > In your command, there is (hw|sw|none). > > > Are you talking about inner or outer? > > > Is this command useful for any kind of packet? > > > How does it combine with "tx_checksum set outer-ip (hw|sw)"? > > > > > > > I rethink these TX checksum commands in this patch set and agree with > > you that we should make some changes for having clear meaning for them. > > > > There are 3 commands in patch set as follows, 1. tx_checksum set > > tunnel (hw|sw|none) (port-id) > > > > Now I also think the command 1 may confuse user, they probably don't > > understand why we need 'hw' or 'sw' option and when to use the two > > option, so I will replace the command with 'tx_checksum set hw-tunnel-mode > (on|off) (port-id)' command. > > I am a bit confused here, could you explain what would be a behaviour for > 'on' and > 'off'? > Konstantin I have explained the behaviour for 'on' and'off' below, The command 'tx_checksum set hw-tunnel-mode (on|off) (port-id)' is used to set/clear TESTPMD_TX_OFFLOAD_TUNNEL_CKSUM flag. Actually, the PKT_TX_UDP_TUNNEL_PKT offload flag will be set if the testpmd flag is set, which means to tell HW treat that transmit packet as a tunneling packet to do checksum offload When 'on' is set, which is able to meet Method B.1 and method C. When 'off' is set, the TESTPMD_TX_OFFLOAD_TUNNEL_CKSUM is not needed to set, so the PKT_TX_UDP_TUNNEL_PKT offload flag is not needed to set, then HW treat that transmit packet as a non-tunneling packet. It is able to meet Method B.2. Is the explanation not clear? > > > > > 2. tx_checksum set outer-ip (hw|sw) (port-id) 3. tx_checksum set > > (ip|udp|tcp|sctp) (hw|sw) (port-id) > > > > The command 2 will be merged into command 3, the new command is ' > > tx_checksum set (outer-ip|ip|udp|tcp|sctp) (hw|sw) (port- id)'. > > > > These most of the cases in > > http://dpdk.org/ml/archives/dev/2014-December/009213.html will be > > covered by using the two commands > > > > The command 'tx_checksum set hw-tunnel-mode (on|off) (port-id)' is > > used to set/clear TESTPMD_TX_OFFLOAD_TUNNEL_CKSUM flag. > > Actually, the PKT_TX_UDP_TUNNEL_PKT offload flag will be set if the > > testpmd flag is set, which tell driver/HW treat that transmit packet as a > tunneling packet. > > > > When 'on' is set, which is able to meet Method B.1 and method C. > > > > When 'off' is set, the TESTPMD_TX_OFFLOAD_TUNNEL_CKSUM is not needed > > to set, so the PKT_TX_UDP_TUNNEL_PKT offload flag is not needed to set, > > then > HW treat that transmit packet as a non-tunneling packet. It is able to meet > Method B.2. > > > > As to case A, I think it is not mandatory to cover it in csum fwd engine for > tunneling packet. > > > > Is the above description clear for you? > > > > > Regards, > > > Olivier
[dpdk-dev] [PATCH v3 0/3] enhance TX checksum command and csum forwarding engine
Hi Frank, > -Original Message- > From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Liu, Jijiang > Sent: Wednesday, January 07, 2015 2:04 AM > To: 'Olivier MATZ' > Cc: dev at dpdk.org > Subject: Re: [dpdk-dev] [PATCH v3 0/3] enhance TX checksum command and csum > forwarding engine > > Hi Olivier, > > > -Original Message- > > From: Olivier MATZ [mailto:olivier.matz at 6wind.com] > > Sent: Saturday, December 13, 2014 12:33 AM > > To: Liu, Jijiang > > Cc: dev at dpdk.org > > Subject: Re: [dpdk-dev] [PATCH v3 0/3] enhance TX checksum command and > > csum forwarding engine > > > > Hello, > > > > On 12/12/2014 04:48 AM, Liu, Jijiang wrote: > > > The 'hw/sw' option is used to set/clear the flag of enabling TX > > > tunneling packet > > checksum hardware offload in testpmd application. > > > > This is not clear at all. > > In your command, there is (hw|sw|none). > > Are you talking about inner or outer? > > Is this command useful for any kind of packet? > > How does it combine with "tx_checksum set outer-ip (hw|sw)"? > > > > I rethink these TX checksum commands in this patch set and agree with you > that we should make some changes for having clear > meaning for them. > > There are 3 commands in patch set as follows, > 1. tx_checksum set tunnel (hw|sw|none) (port-id) > > Now I also think the command 1 may confuse user, they probably don't > understand why we need 'hw' or 'sw' option and when to > use the two option, > so I will replace the command with 'tx_checksum set hw-tunnel-mode (on|off) > (port-id)' command. I am a bit confused here, could you explain what would be a behaviour for 'on' and 'off'? Konstantin > > 2. tx_checksum set outer-ip (hw|sw) (port-id) > 3. tx_checksum set (ip|udp|tcp|sctp) (hw|sw) (port-id) > > The command 2 will be merged into command 3, the new command is ' tx_checksum > set (outer-ip|ip|udp|tcp|sctp) (hw|sw) (port- > id)'. > > These most of the cases in > http://dpdk.org/ml/archives/dev/2014-December/009213.html will be covered by > using the two > commands > > The command 'tx_checksum set hw-tunnel-mode (on|off) (port-id)' is used to > set/clear TESTPMD_TX_OFFLOAD_TUNNEL_CKSUM > flag. > Actually, the PKT_TX_UDP_TUNNEL_PKT offload flag will be set if the testpmd > flag is set, which tell driver/HW treat that transmit > packet as a tunneling packet. > > When 'on' is set, which is able to meet Method B.1 and method C. > > When 'off' is set, the TESTPMD_TX_OFFLOAD_TUNNEL_CKSUM is not needed to set, > so the PKT_TX_UDP_TUNNEL_PKT offload flag is > not needed to set, then HW treat that transmit packet as a non-tunneling > packet. It is able to meet Method B.2. > > As to case A, I think it is not mandatory to cover it in csum fwd engine for > tunneling packet. > > Is the above description clear for you? > > > Regards, > > Olivier
[dpdk-dev] [PATCH v3 0/3] enhance TX checksum command and csum forwarding engine
Hi Olivier, > -Original Message- > From: Olivier MATZ [mailto:olivier.matz at 6wind.com] > Sent: Saturday, December 13, 2014 12:33 AM > To: Liu, Jijiang > Cc: dev at dpdk.org > Subject: Re: [dpdk-dev] [PATCH v3 0/3] enhance TX checksum command and > csum forwarding engine > > Hello, > > On 12/12/2014 04:48 AM, Liu, Jijiang wrote: > > The 'hw/sw' option is used to set/clear the flag of enabling TX tunneling > > packet > checksum hardware offload in testpmd application. > > This is not clear at all. > In your command, there is (hw|sw|none). > Are you talking about inner or outer? > Is this command useful for any kind of packet? > How does it combine with "tx_checksum set outer-ip (hw|sw)"? > I rethink these TX checksum commands in this patch set and agree with you that we should make some changes for having clear meaning for them. There are 3 commands in patch set as follows, 1. tx_checksum set tunnel (hw|sw|none) (port-id) Now I also think the command 1 may confuse user, they probably don't understand why we need 'hw' or 'sw' option and when to use the two option, so I will replace the command with 'tx_checksum set hw-tunnel-mode (on|off) (port-id)' command. 2. tx_checksum set outer-ip (hw|sw) (port-id) 3. tx_checksum set (ip|udp|tcp|sctp) (hw|sw) (port-id) The command 2 will be merged into command 3, the new command is ' tx_checksum set (outer-ip|ip|udp|tcp|sctp) (hw|sw) (port-id)'. These most of the cases in http://dpdk.org/ml/archives/dev/2014-December/009213.html will be covered by using the two commands The command 'tx_checksum set hw-tunnel-mode (on|off) (port-id)' is used to set/clear TESTPMD_TX_OFFLOAD_TUNNEL_CKSUM flag. Actually, the PKT_TX_UDP_TUNNEL_PKT offload flag will be set if the testpmd flag is set, which tell driver/HW treat that transmit packet as a tunneling packet. When 'on' is set, which is able to meet Method B.1 and method C. When 'off' is set, the TESTPMD_TX_OFFLOAD_TUNNEL_CKSUM is not needed to set, so the PKT_TX_UDP_TUNNEL_PKT offload flag is not needed to set, then HW treat that transmit packet as a non-tunneling packet. It is able to meet Method B.2. As to case A, I think it is not mandatory to cover it in csum fwd engine for tunneling packet. Is the above description clear for you? > Regards, > Olivier
[dpdk-dev] [PATCH v3 0/3] enhance TX checksum command and csum forwarding engine
Hello, On 12/12/2014 04:48 AM, Liu, Jijiang wrote: > The 'hw/sw' option is used to set/clear the flag of enabling TX tunneling > packet checksum hardware offload in testpmd application. This is not clear at all. In your command, there is (hw|sw|none). Are you talking about inner or outer? Is this command useful for any kind of packet? How does it combine with "tx_checksum set outer-ip (hw|sw)"? >> You are mixing scenario descriptions with what you do in your patchset: >> 1/ is a scenario >> 2/ and 3/ are descriptions of added/removed commands > > No. > Please note the symbols for command descriptions and scenario descriptions. > > The command descriptions with ">" symbol. > 1> add "tx_checksum set tunnel (hw|sw|none) (port-id)" command > 2> add "tx_checksum set outer-ip (hw|sw) (port-id)" command > 3> (ip|udp|tcp|sctp|vxlan) (hw|sw) (port-id)" command > > The scenario descriptions with ")" symbol. > 1) User requests HW offload for ipv4_hdr_out checksum, and doesn't care is > it a tunneled packet or not. So he sets: I read again your cover letter. You enumerate *one* scenario. An enumeration starts with 2 items. Then you mix 1> and 1) numbers, which does not make things readable. >> Another thing: you don't describe what you want to be able to do: >> >> 1/ packet type 1: compute L3 and/or L4 checksum using lX_len 2/ packet type >> 2: >> compute inner L3 and/or L4 checksum using lX_len 3/ packet type 2: compute >> outer L3 and/or L4 checksum using lX_len 4/ packet type 2: compute inner L3 >> and/or L4 checksum using lX_len >> and outer L3 and/or L4 checksum using outer_lX_len > > These details have already covered in > http://dpdk.org/ml/archives/dev/2014-December/009213.html, > if the patch set is applied, and we aslo have to update the some documents. First, this link was not referenced in the cover letter or patchset. I think it would help to first try to explain what problem is fixed, what is the need, and why. I think in this case it should not require lines and it could be done in a simple way. Indeed, yesterday I spent more than an hour to review your patches and read your descriptions. After that, I still don't understand how the commands impact the behavior of the csumonly application. The possible reasons are: 1) I am too dumb to understand. In this case, it would be better for you and the community to find another reviewer. 2) Your descriptions are not clear. In this case, you need to think about how to reword them so they can be understood, or even maybe think about rework your commands if they cannot be explained with simple words. Note that 1) and 2) are not exclusive ;) >> why not having the 2 following commands: >> > > I have talked about why we need the current 3 commands in another mail loop, > let me explain it here again. The community does not have this private thread. And, that's right, I remember this thread: in it, I asked 3 times some precisions about the commands without any clear answer. > First. We still think we need some command to enable/disable tunneling > support in testpmd, that's why the command 1 is needed. What does enable/disable tunneling support mean? > 1. tx_checksum set tunnel (hw|sw|none) (port-id) command > > 2. tx_cksum set (outer-ip) (hw|sw) (port_id) > > 3. tx_cksum set (ip|l4) (hw|sw) (port_id) > > Secondly, in most of cases, user application use non-tunneling packet, so > he just care how to use 3, don't need to care 1 and 2, don't you think it > becomes simpler? > If we mix tunneling packet command and non-tunneling packet together, and the > commands will become more complicated and not easy to understand. Really no, it is not simpler. But if you are able to explain it in few words what is done by csumonly, maybe I can change my mind. >> tx_checksum set >> (ip|udp|tcp|sctp|outer-ip|outer-udp|outer-tcp|outer-sctp) > > As far as I know, so far, there is no a type of tunneling packet with > outer-tcp and outer-sctp. For TCP, there is STT, which is used in storage. For SCTP, it could probably be removed. >>select if we use hw or sw calculation for each header type >> >> tx_checksum tunnel (inner|outer|both) >> >>when a tunnel packet is received in csum only, control wether >>we want to process inner, outer or both headers > > This command can't meet/match our previous discussions and current > implementation. In terms of 'inner' option, which can't meet the two > following cases. > > B) User is aware that it is a tunneled packet and requests HW offload for > ipv4_hdr_in and tcp_hdr_in *only*. > He doesn't care about outer IP checksum offload. > In that case, for FVL he has 2 choices: >1. Treat that packet as a 'proper' tunnelled packet, and fill all the > fields: > mb->l2_len = udp_hdr_len + vxlan_hdr_len + eth_hdr_in; > mb->l3_len = ipv4_hdr_in; > mb->outer_l2_len = eth_hdr_out; > mb->outer_l3_len = ipv4_hdr_out; > mb->ol_flags |=
[dpdk-dev] [PATCH v3 0/3] enhance TX checksum command and csum forwarding engine
Hi Olivier, Thanks for your comments. > -Original Message- > From: Olivier MATZ [mailto:olivier.matz at 6wind.com] > Sent: Thursday, December 11, 2014 6:18 PM > To: Liu, Jijiang; dev at dpdk.org > Subject: Re: [dpdk-dev] [PATCH v3 0/3] enhance TX checksum command and > csum forwarding engine > > Hi Jijiang, > > Sorry for the late review, I was very busy these last days. Please find my > comments below. > > On 12/10/2014 02:03 AM, Jijiang Liu wrote: > > In the current codes, the "tx_checksum set (ip|udp|tcp|sctp|vxlan) (hw|sw) > (port-id)" command is not easy to understand and extend, so the patch set > enhances the tx_checksum command and reworks csum forwarding engine due > to the change of tx_checksum command. > > The main changes of the tx_checksum command are listed below, > > > > 1> add "tx_checksum set tunnel (hw|sw|none) (port-id)" command > > > > The command is used to set/clear tunnel flag that is used to tell the NIC > > that the > packetg is a tunneing packet and application want hardware TX checksum offload > for outer layer, or inner layer, or both. > > packetg -> packet > tunneing -> tunneling > > I don't understand the description: this flag cannot be set to tell the NIC > that it's a > tunnel packet because it's a configuration flag. > Whatever the value of this configuration option, the packets can be either > tunnel > or non-tunnel packets. The real question is, what is the behavior for each > packet > type for each value for this option. Ok, Will replace the above the description with the following: The 'hw/sw' option is used to set/clear the flag of enabling TX tunneling packet checksum hardware offload in testpmd application. > > The 'none' option means that user treat tunneling packet as ordinary packet > when using the csum forward engine. > > for example, let say we have a tunnel packet: > eth_hdr_out/ipv4_hdr_out/udp_hdr_out/vxlan_hdr/ehtr_hdr_in/ipv4_hdr_in/tcp > _hdr_in. one of several scenarios: > > > > 1) User requests HW offload for ipv4_hdr_out checksum, and doesn't care is > > it > a tunnelled packet or not. So he sets: > > tunnelled -> tunneled > > > > > tx_checksum set tunnel none 0 > > > > tx_checksum set ip hw 0 > > > > So for such case we should set tx_tunnel to 'none'. > > > > 2> add "tx_checksum set outer-ip (hw|sw) (port-id)" command > > > > The command is used to set/clear outer IP flag that is used to tell the NIC > > that > application want hardware offload for outer layer. > > > > 3> remove the 'vxlan' option from the "tx_checksum set > > 3> (ip|udp|tcp|sctp|vxlan) (hw|sw) (port-id)" command > > > > The command is used to set IP, UDP, TCP and SCTP TX checksum flag. In the > > case > of tunneling packet, the IP, UDP, TCP and SCTP flags always concern inner > layer. > > > > Moreover, replace the TESTPMD_TX_OFFLOAD_VXLAN_CKSUM flag with > TESTPMD_TX_OFFLOAD_TUNNEL_CKSUM flag and add the > TESTPMD_TX_OFFLOAD_OUTER_IP_CKSUM and > TESTPMD_TX_OFFLOAD_NON_TUNNEL_CKSUM flag in test-pmd application. > > You are mixing scenario descriptions with what you do in your patchset: > 1/ is a scenario > 2/ and 3/ are descriptions of added/removed commands No. Please note the symbols for command descriptions and scenario descriptions. The command descriptions with ">" symbol. 1> add "tx_checksum set tunnel (hw|sw|none) (port-id)" command 2> add "tx_checksum set outer-ip (hw|sw) (port-id)" command 3> (ip|udp|tcp|sctp|vxlan) (hw|sw) (port-id)" command The scenario descriptions with ")" symbol. 1) User requests HW offload for ipv4_hdr_out checksum, and doesn't care is it a tunneled packet or not. So he sets: > Let's first sumarize what was the behavior before this patch. This is the > description in csumonly code. > > Receive a burst of packets, and for each packet: > - parse packet, and try to recognize a supported packet type (1) > - if it's not a supported packet type, don't touch the packet, else: > - modify the IPs in inner headers and in outer headers if any > - reprocess the checksum of all supported layers. This is done in SW > or HW, depending on testpmd command line configuration > - if TSO is enabled in testpmd command line, also flag the mbuf for TCP > segmentation offload (this implies HW TCP checksum) Then transmit packets > on > the output port. > > (1) Supported packets are: >Ether / (vlan) / IP|IP6 / UDP|TCP|SCTP . >Ether / (vlan) / outer IP|IP6 / outer UDP / VxLAN / Ether / IP|IP6 / >
[dpdk-dev] [PATCH v3 0/3] enhance TX checksum command and csum forwarding engine
Hi Jijiang, Sorry for the late review, I was very busy these last days. Please find my comments below. On 12/10/2014 02:03 AM, Jijiang Liu wrote: > In the current codes, the "tx_checksum set (ip|udp|tcp|sctp|vxlan) (hw|sw) > (port-id)" command is not easy to understand and extend, so the patch set > enhances the tx_checksum command and reworks csum forwarding engine due to > the change of tx_checksum command. > The main changes of the tx_checksum command are listed below, > > 1> add "tx_checksum set tunnel (hw|sw|none) (port-id)" command > > The command is used to set/clear tunnel flag that is used to tell the NIC > that the packetg is a tunneing packet and application want hardware TX > checksum offload for outer layer, or inner layer, or both. packetg -> packet tunneing -> tunneling I don't understand the description: this flag cannot be set to tell the NIC that it's a tunnel packet because it's a configuration flag. Whatever the value of this configuration option, the packets can be either tunnel or non-tunnel packets. The real question is, what is the behavior for each packet type for each value for this option. > The 'none' option means that user treat tunneling packet as ordinary packet > when using the csum forward engine. > for example, let say we have a tunnel packet: > eth_hdr_out/ipv4_hdr_out/udp_hdr_out/vxlan_hdr/ehtr_hdr_in/ipv4_hdr_in/tcp_hdr_in. > one of several scenarios: > > 1) User requests HW offload for ipv4_hdr_out checksum, and doesn't care is > it a tunnelled packet or not. So he sets: tunnelled -> tunneled > > tx_checksum set tunnel none 0 > > tx_checksum set ip hw 0 > > So for such case we should set tx_tunnel to 'none'. > > 2> add "tx_checksum set outer-ip (hw|sw) (port-id)" command > > The command is used to set/clear outer IP flag that is used to tell the NIC > that application want hardware offload for outer layer. > > 3> remove the 'vxlan' option from the "tx_checksum set > (ip|udp|tcp|sctp|vxlan) (hw|sw) (port-id)" command > > The command is used to set IP, UDP, TCP and SCTP TX checksum flag. In the > case of tunneling packet, the IP, UDP, TCP and SCTP flags always concern > inner layer. > > Moreover, replace the TESTPMD_TX_OFFLOAD_VXLAN_CKSUM flag with > TESTPMD_TX_OFFLOAD_TUNNEL_CKSUM flag and add the > TESTPMD_TX_OFFLOAD_OUTER_IP_CKSUM and TESTPMD_TX_OFFLOAD_NON_TUNNEL_CKSUM > flag in test-pmd application. You are mixing scenario descriptions with what you do in your patchset: 1/ is a scenario 2/ and 3/ are descriptions of added/removed commands Let's first sumarize what was the behavior before this patch. This is the description in csumonly code. Receive a burst of packets, and for each packet: - parse packet, and try to recognize a supported packet type (1) - if it's not a supported packet type, don't touch the packet, else: - modify the IPs in inner headers and in outer headers if any - reprocess the checksum of all supported layers. This is done in SW or HW, depending on testpmd command line configuration - if TSO is enabled in testpmd command line, also flag the mbuf for TCP segmentation offload (this implies HW TCP checksum) Then transmit packets on the output port. (1) Supported packets are: Ether / (vlan) / IP|IP6 / UDP|TCP|SCTP . Ether / (vlan) / outer IP|IP6 / outer UDP / VxLAN / Ether / IP|IP6 / UDP|TCP|SCTP The testpmd command line for this forward engine sets the flags TESTPMD_TX_OFFLOAD_* in ports[tx_port].tx_ol_flags. They control wether a checksum must be calculated in software or in hardware. The IP, UDP, TCP and SCTP flags always concern the inner layer. The VxLAN flag concerns the outer IP (if packet is recognized as a vxlan packet). From this description, it is easy to deduct this table: Packet type 1: Ether / (vlan) / IP|IP6 / UDP|TCP|SCTP . Packet type 2 Ether / (vlan) / outer IP|IP6 / outer UDP / VxLAN / Ether / IP|IP6 / UDP|TCP|SCTP +---+---+---+ |test-pmd config \ packets |Packet type 1 |Packet type 2 | +---+---+---+ |whatever the flags |IP addresses |inner and outer IP | | |incremented|addresses | | | |incremented| +---+---+---+ |IP = sw|IP cksum is sw |inner IP cksum is | | | |sw | +---+---+---+ |IP = hw|IP cksum is hw |inner IP cksum is | | |(using lX_len) |hw (using lX_len) | +---+---+---+ |UDP or TCP or SCTP = sw|L4 cksum is sw |inner L4 cksum is | | | |sw |
[dpdk-dev] [PATCH v3 0/3] enhance TX checksum command and csum forwarding engine
In the current codes, the "tx_checksum set (ip|udp|tcp|sctp|vxlan) (hw|sw) (port-id)" command is not easy to understand and extend, so the patch set enhances the tx_checksum command and reworks csum forwarding engine due to the change of tx_checksum command. The main changes of the tx_checksum command are listed below, 1> add "tx_checksum set tunnel (hw|sw|none) (port-id)" command The command is used to set/clear tunnel flag that is used to tell the NIC that the packetg is a tunneing packet and application want hardware TX checksum offload for outer layer, or inner layer, or both. The 'none' option means that user treat tunneling packet as ordinary packet when using the csum forward engine. for example, let say we have a tunnel packet: eth_hdr_out/ipv4_hdr_out/udp_hdr_out/vxlan_hdr/ehtr_hdr_in/ipv4_hdr_in/tcp_hdr_in. one of several scenarios: 1) User requests HW offload for ipv4_hdr_out checksum, and doesn't care is it a tunnelled packet or not. So he sets: tx_checksum set tunnel none 0 tx_checksum set ip hw 0 So for such case we should set tx_tunnel to 'none'. 2> add "tx_checksum set outer-ip (hw|sw) (port-id)" command The command is used to set/clear outer IP flag that is used to tell the NIC that application want hardware offload for outer layer. 3> remove the 'vxlan' option from the "tx_checksum set (ip|udp|tcp|sctp|vxlan) (hw|sw) (port-id)" command The command is used to set IP, UDP, TCP and SCTP TX checksum flag. In the case of tunneling packet, the IP, UDP, TCP and SCTP flags always concern inner layer. Moreover, replace the TESTPMD_TX_OFFLOAD_VXLAN_CKSUM flag with TESTPMD_TX_OFFLOAD_TUNNEL_CKSUM flag and add the TESTPMD_TX_OFFLOAD_OUTER_IP_CKSUM and TESTPMD_TX_OFFLOAD_NON_TUNNEL_CKSUM flag in test-pmd application. v2 change: redefine the 'none' behaviour for "tx_checksum set tunnel (hw|sw|none) (port-id)" command. v3 change: typo correction in cmdline help Jijiang Liu (3): add outer IP offload capability in librte_ether. add outer IP checksum capability in i40e PMD testpmd command lines of the tx_checksum and csum forwarding rework app/test-pmd/cmdline.c| 201 +++-- app/test-pmd/csumonly.c | 35 --- app/test-pmd/testpmd.h|6 +- lib/librte_ether/rte_ethdev.h |1 + lib/librte_pmd_i40e/i40e_ethdev.c |3 +- 5 files changed, 218 insertions(+), 28 deletions(-) -- 1.7.7.6
