> >>@@ -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 dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev