[ovs-dev] [PATCH v3] datapath-windows: Add GRE checksum

[Alin Serdean]

Thanks for the review answer inlined.

Thanks,
Alin.

> -----Mesaj original-----
> De la: Nithin Raju [mailto:nithin at vmware.com]
> Trimis: Wednesday, June 8, 2016 10:10 PM
> Către: Alin Serdean <aserdean at cloudbasesolutions.com>;
> dev at openvswitch.org
> Subiect: Re: [ovs-dev] [PATCH v3] datapath-windows: Add GRE checksum
> 
> 
> >+    /* Get a contignuous buffer for the maxmimum length of a GRE
> >+ header
> >*/
> >+    bufferStart = NdisGetDataBuffer(curNb, OVS_MAX_GRE_LGTH, NULL, 1,
> >+ 0);
> 
> Sorry we have to go back and forth on this. Like I mentioned in the previous
> email, the idea is to get a contiguous chunk of memory so we can walk all the
> header until the GRE header. It is a good idea to use
> NdisGetDataBuffer() instead of copying the NBL. But, we won¹t avoid the
> copied NBL anyway since decap has to happen on the copied NBL.
[Alin Gabriel Serdean: ] Agreed. I only wanted a check to make sure the whole ETH, IP, GRE was contiguous. 
> 
> In any case, NdisGetDataBuffer() has some pitfalls:
> "If the requested data in the buffer is contiguous, this return value is a
> pointer to a location that NDIS provides. If the data is not contiguous, NDIS
> uses the Storage parameter as follows:
> 
> * If the Storage parameter is non-NULL, NDIS copies the data to the buffer at
> Storage. This return value is the pointer passed to the Storage parameter.
> * If the Storage parameter is NULL, this return value is NULL.²
> 
> So, if the first MDL does not fit the headers until the GRE headers, we need
> to pass explicit memory to NdisGetDataBuffer() in argument #3 in order for
> NDIS to copy it over to a contiguous chunk of memory.
[Alin Gabriel Serdean: ] We have to take into consideration that NDIS 6.3 supports NVGRE and in NDIS 6.4 GRE (https://support.microsoft.com/en-us/kb/3022776 ).
Packets can be split in multiple Mdl's, if the miniport supports it, as early as at the beginning of the upper-layer-protocol (https://msdn.microsoft.com/en-us/library/windows/hardware/ff571065(v=vs.85).aspx) and if the split header is not greater than the maximum header size which is configurable. In the case of VXLAN / STT we can be in the case above, because of the UDP/TCP like packet, but for GRE things differ since there are cards which support offloading:
https://msdn.microsoft.com/en-us/library/windows/hardware/dn605832(v=vs.85).aspx
"The protocol driver will guarantee that all the prepended encapsulation headers (ETH, IP, GRE) will be physically contiguous and will be in the first MDL of the packet."
I do not think we will be in a situation in which a GRE header will not be continuous.
If you consider we should still have a fragmented header problem, I will allocate a buffer and execute another NdisGetDataBuffer call if the first one failed.
> 
> Thanks,
> -- Nithin