Re: [PATCH v5 2/3] mbuf:add three TX ol_flags and repalce PKT_TX_VXLAN_CKSUM

[Olivier MATZ <olivier.matz-pdR9zngts4EAvxtiuMwx3w@public.gmane.org>]

Hi,

On 12/02/2014 04:06 PM, Jijiang Liu wrote:
> Replace PKT_TX_VXLAN_CKSUM with PKT_TX_UDP_TUNNEL_PKT in order to indicate a packet is an UDP tunneling packet, and introduce 3 TX offload flags for outer IP TX checksum, which are PKT_TX_OUTER_IP_CKSUM, PKT_TX_OUTER_IPV4 and PKT_TX_OUTER_IPV6 respectively;Rework csum forward engine and i40e PMD due to these changes.
> 
> Signed-off-by: Jijiang Liu <jijiang.liu-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> ---
>  app/test-pmd/csumonly.c         |    9 +++++++--
>  lib/librte_mbuf/rte_mbuf.c      |    7 ++++++-
>  lib/librte_mbuf/rte_mbuf.h      |   11 ++++++++++-
>  lib/librte_pmd_i40e/i40e_rxtx.c |    6 +++---
>  4 files changed, 26 insertions(+), 7 deletions(-)
> 

As we need to conclude on this:
Acked-by: Olivier Matz <olivier.matz-pdR9zngts4EAvxtiuMwx3w@public.gmane.org>

Just few minor comments below:

> diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h
> index cbadf8e..6eb898f 100644
> --- a/lib/librte_mbuf/rte_mbuf.h
> +++ b/lib/librte_mbuf/rte_mbuf.h
> @@ -118,7 +118,7 @@ extern "C" {
>   */
>  #define PKT_TX_TCP_SEG       (1ULL << 49)
>  
> -#define PKT_TX_VXLAN_CKSUM   (1ULL << 50) /**< TX checksum of VXLAN computed by NIC */
> +#define PKT_TX_UDP_TUNNEL_PKT (1ULL << 50) /**< TX packet is an UDP tunneling packet */
>  #define PKT_TX_IEEE1588_TMST (1ULL << 51) /**< TX IEEE1588 packet to timestamp. */

We could replace the comment by:

"Tell the NIC it's a UDP tunneled packet. It must be specified when
 using hw outer checksum offload (PKT_TX_OUTER_IP_CKSUM)"

>  
>  /**
> @@ -149,6 +149,15 @@ extern "C" {
>  
>  #define PKT_TX_VLAN_PKT      (1ULL << 57) /**< TX packet is a 802.1q VLAN packet. */
>  
> +/** Outer IP checksum of TX packet, computed by NIC for tunneling packet */
> +#define PKT_TX_OUTER_IP_CKSUM   (1ULL << 58)

Maybe add:
"The tunnel type must also be specified, ex: PKT_TX_UDP_TUNNEL_PKT"

(altought I don't understand why the hw need this info)


Regards,
Olivier