DPDK patches and discussions
 help / color / mirror / Atom feed
From: "Liu, Jijiang" <jijiang.liu@intel.com>
To: Olivier MATZ <olivier.matz@6wind.com>
Cc: "dev@dpdk.org" <dev@dpdk.org>
Subject: Re: [dpdk-dev] [PATCH v3 0/3] enhance TX checksum command and csum forwarding engine
Date: Wed, 14 Jan 2015 03:01:16 +0000	[thread overview]
Message-ID: <1ED644BD7E0A5F4091CF203DAFB8E4CC01DB0789@SHSMSX101.ccr.corp.intel.com> (raw)
In-Reply-To: <54B4EB92.40209@6wind.com>

Hi Olivier,

> -----Original Message-----
> From: Olivier MATZ [mailto:olivier.matz@6wind.com]
> Sent: Tuesday, January 13, 2015 5:56 PM
> To: Liu, Jijiang
> Cc: dev@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.

> 
> >> I have another idea, please let me know if you find it clearer or not.
> >> The commands format would be:
> >>
> >> tx_checksum <pkt-type> <field1> <action1> <field2> <action2> ...
> >>
> >> [...]
> >>
> >> 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.


Ok, but I think you probably need to add many string in the list :)

> > 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  <pkt-type> 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_CKSUM        0x0008
> > /** Offload VxLAN checksum in csum forward engine */
> > #define TESTPMD_TX_OFFLOAD_VXLAN_CKSUM       0x0010
> 
> We can add a portid in each command.

Ok, but I think your idea will make the csum fwd engine more complicated.

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

If you don't have more comments and questions on my current solution, I will send new patch set.
Or you can send your patch.
Anyway, our goal is the same.

> 
> Regards,
> Olivier
> 
> 

  reply	other threads:[~2015-01-14  3:02 UTC|newest]

Thread overview: 49+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-12-10  1:03 Jijiang Liu
2014-12-10  1:03 ` [dpdk-dev] [PATCH v3 1/3] librte_ether:add outer IP offload capability flag Jijiang Liu
2014-12-11 10:33   ` Olivier MATZ
2014-12-10  1:03 ` [dpdk-dev] [PATCH v3 2/3] i40e:support outer IPv4 checksum capability Jijiang Liu
2014-12-11 10:34   ` Olivier MATZ
2014-12-10  1:03 ` [dpdk-dev] [PATCH v3 3/3] app/testpmd:change tx_checksum command and csum forwarding engine Jijiang Liu
2014-12-11 10:52   ` Olivier MATZ
2014-12-12  4:06     ` Liu, Jijiang
2014-12-11 10:17 ` [dpdk-dev] [PATCH v3 0/3] enhance TX checksum " Olivier MATZ
2014-12-12  3:48   ` Liu, Jijiang
2014-12-12 16:33     ` Olivier MATZ
2015-01-07  2:03       ` Liu, Jijiang
2015-01-07  9:59         ` Ananyev, Konstantin
2015-01-07 11:39           ` Liu, Jijiang
2015-01-07 12:07             ` Ananyev, Konstantin
2015-01-08  8:51               ` Liu, Jijiang
2015-01-08 10:54                 ` Ananyev, Konstantin
2015-01-09 10:45                   ` Olivier MATZ
2015-01-12  3:41                     ` Liu, Jijiang
2015-01-12 11:43                       ` Olivier MATZ
2015-01-13  3:04                         ` Liu, Jijiang
2015-01-13  9:55                           ` Olivier MATZ
2015-01-14  3:01                             ` Liu, Jijiang [this message]
2015-01-15 13:31                               ` Ananyev, Konstantin
2015-01-16 17:27                                 ` Olivier MATZ
2015-01-19 13:04                                   ` Ananyev, Konstantin
2015-01-19 14:38                                     ` Olivier MATZ
2015-01-20  1:12                                       ` Ananyev, Konstantin
2015-01-20 12:39                                         ` Olivier MATZ
2015-01-20 15:18                                           ` Thomas Monjalon
2015-01-20 17:10                                             ` Stephen Hemminger
2015-01-20 17:23                                           ` Ananyev, Konstantin
2015-01-20 18:15                                             ` Olivier MATZ
2015-01-21  3:12                                               ` Liu, Jijiang
2015-01-21 15:25                                                 ` Olivier MATZ
2015-01-21 16:28                                                   ` Ananyev, Konstantin
2015-01-21 17:13                                                     ` Olivier MATZ
2015-01-26  4:13                                                   ` Ananyev, Konstantin
2015-01-26  6:02                                                     ` Liu, Jijiang
2015-01-26 14:07                                                       ` Olivier MATZ
2015-01-26 14:15                                                         ` Ananyev, Konstantin
2015-01-27  8:34                                                           ` Olivier MATZ
2015-01-27 15:26                                                             ` Ananyev, Konstantin
2015-01-21 19:44                                                 ` Stephen Hemminger
2015-01-22  1:40                                                   ` Liu, Jijiang
2015-01-21  8:01                                               ` Liu, Jijiang
2015-01-21  9:10                                                 ` Olivier MATZ
2015-01-21 11:52                                                   ` Ananyev, Konstantin
2015-01-07 13:06 ` Qiu, Michael

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1ED644BD7E0A5F4091CF203DAFB8E4CC01DB0789@SHSMSX101.ccr.corp.intel.com \
    --to=jijiang.liu@intel.com \
    --cc=dev@dpdk.org \
    --cc=olivier.matz@6wind.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).