Re: [dpdk-dev] [PATCH v8 10/10] app/testpmd:test VxLAN Tx checksum offload

["Liu, Jijiang" <jijiang.liu@intel.com>]

> -----Original Message-----
> From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> Sent: Thursday, November 13, 2014 5:10 PM
> To: Liu, Jijiang
> Cc: dev@dpdk.org; Olivier MATZ
> Subject: Re: [dpdk-dev] [PATCH v8 10/10] app/testpmd:test VxLAN Tx checksum
> offload
> 
> 2014-11-13 06:51, Liu, Jijiang:
> > From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> > > 2014-11-11 05:29, Liu, Jijiang:
> > > > From: Olivier MATZ
> > > > > On 11/10/2014 07:03 AM, Liu, Jijiang wrote:
> > > > > > > - if PKT_TX_VXLAN_CKSUM is not set (legacy use case), then the
> > > > > > >    driver use l2_len and l3_len to offload inner IP/UDP/TCP checksums.
> > > > > >
> > > > > > If the flag is not set, and imply that it is not VXLAN packet,
> > > > > > and do TX checksum offload as regular packet.
> > > > > >
> > > > > > > - if PKT_TX_VXLAN_CKSUM is set, then the driver has to use
> > > > > > >    inner_l{23}_len instead of l{23}_len for the same operation.
> > > > > >
> > > > > > Your understanding is not fully correct.
> > > > > > The l{23}_len is still used for TX checksum offload, please
> > > > > > refer to i40e_txd_enable_checksum() implementation.
> > > > >
> > > > > This fields are part of public mbuf API. You cannot say to refer
> > > > > to i40e PMD code to understand how to use it.
> > > > >
> > > > > > > Adding PKT_TX_VXLAN_CKSUM changes the semantic of l2_len and
> > > > > > > l3_len.
> > > > > > > To fix this, I suggest to remove the new fields
> > > > > > > inner_l{23}_len then add outer_l{23}_len instead. Therefore,
> > > > > > > the semantic of l2_len and l3_len would not change, and a
> > > > > > > driver would always use the same field for a specific offload.
> > > > > >
> > > > > > Oh...
> > > > >
> > > > > Does it mean you agree?
> > > >
> > > > I don't agree to change inner_l{23}_len the name.
> > > > The reason is that using the "inner" word means incoming packet is
> > > > tunneling packet or encapsulation packet.
> > > > if we add "outer"{2,3}_len, which will cause confusion when
> > > > processing non- tunneling packet.
> > >
> > > Sorry Jijiang, maybe I don't understand what you are saying, but I
> > > think you missed something. Let me explain the problem.
> > >
> > > For PKT_TX_IP_CKSUM, we must set l{2,3}_len.
> > > When PKT_TX_VXLAN_CKSUM is set, PKT_TX_IP_CKSUM is related to inner
> > > IP, right?
> > > So we must set inner_l{2,3}_len.
> > > It means that PKT_TX_IP_CKSUM requires different fields to be set,
> > > depending of PKT_TX_VXLAN_CKSUM. That's what Olivier calls a semantic
> change.
> > > It's not acceptable for an API.
> >
> > I'd like to explain what PKT_TX_VXLAN_CKSUM means, it is to tell
> > driver should set whole VXLAN packet TX checksum according to  L3/L4 flag
> setting.
> > VXLAN packet IP checksum  not only include inner IP, but also include
> > outer IP, so when PKT_TX_VXLAN_CKSUM is set, the  PKT_TX_IP_CKSUM is not
> only
> > related to inner IP, but also IP.   In other words, we use this one flag to
> > set inner IP and outer IP checksum offload at the same time in driver,
> > because it is not necessary to add other flag to stand for inner IP
> > flag
> 
> You mean that PKT_TX_VXLAN_CKSUM request hardware checksumming of outer
> L3, outer L4, inner L3 and inner L4?
> So maybe the name and comments are not enough clear.

Yes, PKT_TX_VXLAN_CKSUM request hardware checksum of outerL3, outer L4, inner L3 and inner L4.

BTW, for outer UDP Checksum, generally, It SHOULD be transmitted as zero.  When a packet is received with a UDP checksum of zero, it MUST be accepted for decapsulation.

> > L4 flag usage is the same the L3 flag as well.
> 
> What do you mean?

The PKT_TX_TCP_CKSUM, PKT_TX_SCTP_CKSUM,  and PKT_TX_UDP_CKSUM flag are used to  set TX checksum  for outer L4 and inner for tunneling packet.
But for VXLAN, outer L4 protocol must be UDP, so setting PKT_TX_TCP_CKSUM and PKT_TX_SCTP_CKSUM is meaningless for outer L4.


> > > PKT_TX_IP_CKSUM should always be related to l{2,3}_len.
> > > When PKT_TX_VXLAN_CKSUM is set, we should add outer_l{2,3}_len.
> > > Please, correct me if I'm wrong or fix the API.
> >
> > Probably we can refer to struct sk_buff in Linux kernel .
> > Just as a reference!!
> > struct sk_buff {
> > ...
> > *       @inner_protocol: Protocol (encapsulation)
> >  *      @inner_transport_header: Inner transport layer header (encapsulation)
> >  *      @inner_network_header: Network layer header (encapsulation)
> >  *      @inner_mac_header: Link layer header (encapsulation)
> >
> >         __u16                   inner_transport_header;
> >         __u16                   inner_network_header;
> >         __u16                   inner_mac_header;
> >         __u16                   transport_header;
> >         __u16                   network_header;
> >         __u16                   mac_header;
> 
> Yes it's a reference. But some things are made differently in DPDK.
> Is there a flag PKT_TX_VXLAN_CKSUM in Linux?
> 
> I'm not sure what the checksumming API would be.
> But I'm sure the VXLAN API is not enough commented.
> Olivier is improving documentation of the legacy checksum API:
> 	http://dpdk.org/ml/archives/dev/2014-November/007956.html
> I'd like you do the same thing for VXLAN checksum.
Ok , I will improve  document about VXLAN checksum. 
> 
> Thanks
> --
> Thomas