>
>>@@ -504,9 +517,14 @@ CalculateChecksumNB(const PNET_BUFFER nb,
>>+ /*
>>+ * To this point we do not have VXLAN offloading.
>>+ * Apply defined checksums
>>+ */
>
>Sai: Correct me if am wrong, this computes checksum for the inner
>packet if there is no segmentation.
>Alin: You are correct.
>Sai: In case the inner packet is segmented via LSO, shouldn't there be
>additional checksum calculations for each inner-segment?
>Alin: it is computed for each inner segment OvsTcpSegmentNBL->
>FixSegmentHeader which computes tcp checksums for each NB created by
>NdisAllocateFragmentNetBufferList.
> Looking again over the code I think we could change it to
>compute IP checksum only once and not for each segment. I will change
>this in v2.
Sai: OvsTcpSegmentNBL gets called prior to this code. Are you going to reorder
the code to achieve this?
Alin: I will change FixSegmentHeader to use a predefined value for ip checksums
instead of computing it for every net buffer.
Maybe I misunderstood your commentary, could you please detail your thoughts.
>Sai: If this is just for non-lso packet, then you can do the following
>prior to creating *newNbl to avoid having to compute bufferStart.
>Alin: I do not want to compute the checksum if the copy failed
>
>>+ /* Compute IP checksum only if the NIC does not support
>>offloading */
>
>Sai: I don't think this check is appropriate.
>csumInfo is still pointing at the inner packet. The ipHdr is for outer
>packet.
>This just checks if the underlying VM had offloaded the
>IpHeaderChecksum calculation.
>
>>+ if (!csumInfo.Transmit.IpHeaderChecksum) {
>>+ ipHdr->check = 0;
>>+ ipHdr->check = IPChecksum((UINT8 *)ipHdr, sizeof *ipHdr, 0);
>>+ }
>>
>> /* UDP header */
>> udpHdr = (UDPHdr *)((PCHAR)ipHdr + sizeof *ipHdr);
>>--
>
>Alin: I am going on the following assumption:
>If you have LSO or IP/TCP/UDP checksums enabled in your VM it means
>that the NIC on which the switch is created has them enabled.
>The check above means that the VM did not have IP checksum enabled,
>which means that the physical one did not had it enabled also, telling
>us to compute it.
>If the VM had it enabled, means the physical one had it thus meaning we
>should not compute it.
>It is a rather hard assumption but I will leave it for more discussion
>if we should leave it or not.
Sai: I think you can skip this check and instead offload the task to NDIS.
I did something similar in my patch for enabling checksum in STT.
I was able to get pings to work even when the offloads were disabled on the
Hyper-V Host, but enabled in the underlying VM.
Alin: I will look over your patch tonight. It isn't a question if it works or
not. The idea is that I am not fond of lying to the user of the VM that we
support capability which doesn't exist. It is a virtual
http://www.hackerthreads.org/Topic-10367 :).
_______________________________________________
dev mailing list
[email protected]
http://openvswitch.org/mailman/listinfo/dev
