That's what I have figured. I am fixing a bug here where the we check for less
length then we actually copied, thus potentially resulting in a copy failure if
inner packet is empty (wrong error code will be returned because it bypasses
the length check, which is not strict enough). I hesitated to change the copy
length because I am not 100% that the extra copy size is not needed anywhere.
Since you confirmed it's not used, I might as well just shrink the copy size to
be equal to the headroom.
_______________________________________
From: Nithin Raju
Sent: Thursday, May 19, 2016 5:01 PM
To: Yin Lin; [email protected]
Subject: Re: [ovs-dev] [PATCH 2/3] [PATCH v1] Move UDP checksum computation to
Offload.c
Thanks for the patch.
>@@ -414,7 +372,7 @@ OvsDecapVxlan(POVS_SWITCH_CONTEXT switchContext,
> IPHdr *ipHdr;
> UDPHdr *udpHdr;
> VXLANHdr *vxlanHdr;
>- UINT32 tunnelSize = 0, packetLength = 0;
>+ UINT32 tunnelSize, packetLength, copySize;
> PUINT8 bufferStart;
> NDIS_STATUS status;
>
>@@ -422,7 +380,8 @@ OvsDecapVxlan(POVS_SWITCH_CONTEXT switchContext,
> curNb = NET_BUFFER_LIST_FIRST_NB(curNbl);
> packetLength = NET_BUFFER_DATA_LENGTH(curNb);
> tunnelSize = OvsGetVxlanTunHdrSize();
>- if (packetLength <= tunnelSize) {
>+ copySize = tunnelSize + OVS_DEFAULT_COPY_SIZE;
>+ if (packetLength < copySize) {
> return NDIS_STATUS_INVALID_LENGTH;
> }
>
>@@ -430,7 +389,7 @@ OvsDecapVxlan(POVS_SWITCH_CONTEXT switchContext,
> * Create a copy of the NBL so that we have all the headers in one
>MDL.
> */
> *newNbl = OvsPartialCopyNBL(switchContext, curNbl,
>- tunnelSize + OVS_DEFAULT_COPY_SIZE, 0,
>+ copySize, 0,
> TRUE /*copy NBL info */);
>
> if (*newNbl == NULL) {
We don¹t need to use tunnelSize + OVS_DEFAULT_COPY_SIZE in the partial
copy. Just ŒtunnelSize¹ is sufficient. But, not a big deal.
Acked-by: Nithin Raju <[email protected]>
_______________________________________________
dev mailing list
[email protected]
http://openvswitch.org/mailman/listinfo/dev
