Hi Thomas, Yes, would update in next version, thanks for reminding.
Regards, Xueming > -----Original Message----- > From: Thomas Monjalon [mailto:tho...@monjalon.net] > Sent: Wednesday, January 31, 2018 4:21 AM > To: Xueming(Steven) Li <xuemi...@mellanox.com> > Cc: Ananyev, Konstantin <konstantin.anan...@intel.com>; Olivier MATZ > <olivier.m...@6wind.com>; dev@dpdk.org; Wu, Jingjing > <jingjing...@intel.com>; Shahaf Shuler <shah...@mellanox.com>; Yongseok > Koh <ys...@mellanox.com>; Yigit, Ferruh <ferruh.yi...@intel.com> > Subject: Re: [dpdk-dev] [PATCH v2 1/5] ethdev: introduce Tx generic tunnel > offloads > > Xueming, > > We already discussed privately some time ago about this API. > You got some questions from me, from Olivier and from Konstantin. > The next version should answer the questions asked. > > When defining an API, the doxygen documentation must explain precisely > what must be set, when and why. > The documentation must be sufficient to allow users using it, and to allow > PMD writers to implement it. > > I think it cannot be properly reviewed until we clearly understand the API > and the HW requirements/expectations. > > Hope this helps to understand what is expected for integrating such API. > > > 30/01/2018 18:54, Xueming(Steven) Li: > > > > > > > > > > > > > This patch introduce new TX offloads flag for > > > > > > > > > > > > devices that support tunnel agnostic checksum and > TSO offloads. > > > > > > > > > > > > > > > > > > > > > > > > The support from the device is for inner and outer > > > > > > > > > > > > checksums on IPV4/TCP/UDP and TSO for *any packet > > > > > > > > > > > > with the following > > > > > > > format*: > > > > > > > > > > > > > > > > > > > > > > > > < some headers > / [optional IPv4/IPv6] / > > > > > > > > > > > > [optional TCP/UDP] / <some > > > > > > > > > > > > headers> / [optional inner IPv4/IPv6] / [optional > > > > > > > > > > > > headers> TCP/UDP] > > > > > > > > > > > > > > > > > > > > > > > > For example the following packets can use this > feature: > > > > > > > > > > > > > > > > > > > > > > > > 1. eth / ipv4 / udp / VXLAN / ip / tcp 2. eth / > > > > > > > > > > > > ipv4 / GRE / MPLS / > > > > > > > > > > > > ipv4 / udp > > > > > > > > > > > > > > > > > > > > > > So in terms of usage - what is the difference with > > > > > > > > > > > current TSO > > > > > > > types? > > > > > > > > > > > Konstantin > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Traditionally, HW only recognize "known" tunnel type, > > > > > > > > > > do TSO calculation based on L3/L4 headers known to > > > > > > > > > > tunnel type. For example, it must be > > > > > > > > > > L2 header after VXLAN, then L3. While this Generic > > > > > > > > > > offloading provides inner/outer L3/L4 header info(len > > > > > > > > > > and > > > > > > > > > > offset) to HW, and thus tunnel info become less > important. > > > > > > > > > > Please note the MPLS over GRE tunnel in last example > above. > > > > > > > > > > > > > > > > > > Ok, but I wonder when the user would like to do TSO on > > > > > > > > > tunnel packet, for this offload - would he need to do > > > > > > > > > something differently from what he has to do now: > > > > > > > > > raise PKT_TX_TCP_SEG and related flags, raise > > > > > > > > > appropriate > > > > > > > > > PKT_TX_TUNNEL_* flag, fill l2_len, l3_len, > > > > > > > l4_len,tso_segsz,outer_l2_len,outer_l3_len? > > > > > > > > > > > > > > > > > > > > > > > > > Yes, these fields are sufficient except PKT_TX_TUNNEL_*, > > > > > > > > major target of this new feature is to support "unknown" > > > > > > > > tunnel offloading, it > > > > > > > supports "known" > > > > > > > > tunnel type as well. > > > > > > > > > > > > > > Ok, but user would still need to set some flag to indicate > > > > > > > that this is a tunnel packet, and he wants TSO over it, right? > > > > > > > For pre-defined tunnel types it can be one of > > > > > > > PKT_TX_TUNNEL_* (which actually means that user still have > > > > > > > to know tunnel type > > > > > > > anyway?) But for some not defined tunnel type - what it would > be? > > > > > > > Konstantin > > > > > > > > > > > > > > > > > > > As this feature target to TX path, Outer length as tunnel > > > > > > indication, leave it empty if tunnel not defined. > > > > > > > > > > Sorry, I didn't get it. > > > > > We need to let PMD know that it is a tunnel packet, right? > > > > > So we do need to raise PKT_TX_TUNNEL_* flag. > > > > > > > > > > > > > In my current code, mbuf.outer_l2_len is used to test tunnel packet. > > > > Agree a new tunnel flag would be better. > > > > > > > > > > > > > > > > But I think it good to define something like: > > > > > > PKT_TX_TUNNEL_GENERIC = PKT_TX_TUNNEL_MASK > > > > > > > > > > Yes, that's an option, I would probably name it > PKT_TX_TUNNEL_UNKNOWN. > > > > > > > > > > > And a new flag PKT_TX_OUTER_UDP, how do you think? > > > > > > > > > > Not sure why do we need it? > > > > > HW still needs to know outer_l4_type to be able to work correctly? > > > > > > > > For tunnel type like vxlan, if outer UDP present, hw has to update > > > > UDP length field for each TSO packet segment. > > > > > > I understand that, but I thought that HW is smart enough to parse > > > the header and recognize outer L3/L4 type - as it is a 'generic' > > > tunneling (sorry I am not familiar with MLX HW at all). > > > > It might be useful if the outer encapsulation not regular, for example > MPLS. > > > > > From what you saying - that assumption was wrong and user still > > > need to provide some packet-type info at least about outer headers, > right? > > > So what else need to be set? > > > Probably PKT_TX_OUTER_IPV*, might be something else? > > > > Sorry for the confusion, besides optional outer UDP type, still need > > PKT_TX_IPV4/6 and PKT_TX_OUTER_IPV4/6
