From mboxrd@z Thu Jan  1 00:00:00 1970
From: Olivier MATZ <olivier.matz-pdR9zngts4EAvxtiuMwx3w@public.gmane.org>
Subject: Re: [PATCH 13/20] testpmd: support gre tunnels in csum
	fwd engine
Date: Mon, 02 Feb 2015 14:30:58 +0100
Message-ID: <54CF7C12.8040603@6wind.com>
References: <1421883395-27235-1-git-send-email-olivier.matz@6wind.com>
 <1422623775-8050-1-git-send-email-olivier.matz@6wind.com>
 <1422623775-8050-14-git-send-email-olivier.matz@6wind.com>
 <1ED644BD7E0A5F4091CF203DAFB8E4CC01DC9728@SHSMSX101.ccr.corp.intel.com>
 <54CF73B7.1020104@6wind.com>
 <1ED644BD7E0A5F4091CF203DAFB8E4CC01DCAA13@SHSMSX101.ccr.corp.intel.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=iso-2022-jp
Content-Transfer-Encoding: 7bit
Cc: "dev-VfR2kkLFssw@public.gmane.org" <dev-VfR2kkLFssw@public.gmane.org>
To: "Liu, Jijiang" <jijiang.liu-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Return-path: <dev-bounces-VfR2kkLFssw@public.gmane.org>
In-Reply-To: <1ED644BD7E0A5F4091CF203DAFB8E4CC01DCAA13-0J0gbvR4kThpB2pF5aRoyrfspsVTdybXVpNB7YpNyf8@public.gmane.org>
List-Id: patches and discussions about DPDK <dev.dpdk.org>
List-Unsubscribe: <http://dpdk.org/ml/options/dev>,
 <mailto:dev-request-VfR2kkLFssw@public.gmane.org?subject=unsubscribe>
List-Archive: <http://dpdk.org/ml/archives/dev/>
List-Post: <mailto:dev-VfR2kkLFssw@public.gmane.org>
List-Help: <mailto:dev-request-VfR2kkLFssw@public.gmane.org?subject=help>
List-Subscribe: <http://dpdk.org/ml/listinfo/dev>,
 <mailto:dev-request-VfR2kkLFssw@public.gmane.org?subject=subscribe>
Errors-To: dev-bounces-VfR2kkLFssw@public.gmane.org
Sender: "dev" <dev-bounces-VfR2kkLFssw@public.gmane.org>

Hi Jijiang,

On 02/02/2015 02:16 PM, Liu, Jijiang wrote:
>>> For NVGRE:
>>> I will do some change here.
>>> eth_hdr = (struct ether_hdr *)((char *)gre_hdr +
>>> 		sizeof(struct nvgre_hdr)); // replace  simple_gre_hdr with
>> nvgre_hdr.
>>>
>>
>> I think replacing gre_hdr by nvgre_hdr here would break the gre.
>> If we want to support NVGRE, I think we should check the value of gre_hdr-
>>> flags to determine the length of the gre header.
>>
>> I'm not a NVGRE expert, but from what I've seen NVGRE is just a GRE header
>> with the key flag present.
> 
>  NVGRE frame format in section 3.2
> http://datatracker.ietf.org/doc/draft-sridharan-virtualization-nvgre/?include_text=1
> 
> When gre_hdr->proto = htons (0x6558),  which means this is NVGRE protocol( MAC over GRE), the key flag must be present, so gre_hdr->flags != 0.
> 
> Anyway,  the following check should be removed when we support NVGRE TX checksum offload later, or it will failed.
> 
> 	/* if flags != 0; it's not supported */
>               if (gre_hdr->flags != 0)
> 		return;

Yes, what I'm saying is that we need to replace it by something like:

	gre_len = sizeof(*gre);

	/* replaces "if (gre->flags != 0)" */
	if ((gre->flags & UNSUPPORTED_FLAGS) != 0)
		return;

	if (gre->flags & KEY)
		gre_len += 4;


If the key flag is mandatory in key flag is mandatory in case of
Ethernet over GRE, this should be checked after.

Regards,
Olivier