Re: [ovs-dev] [PATCH] datapath-windows:adjust Offset when processing packet in POP_VLAN action

[Wilson Peng]

Alin,

Original code diff is to avoid the case below(it will return 
NDIS_STATUS_SUCCESS before 
RtlMoveMemory in function OvsPopFieldInPacketBuf).

Regards
Wilson

In function OvsPopFieldInPacketBuf(..)
 if (!bufferData) {
        EthHdr *ethHdr = (EthHdr *)bufferStart;
        /* If the frame is not VLAN make it a no op */
        if (ethHdr->Type != ETH_TYPE_802_1PQ_NBO) {
            return NDIS_STATUS_SUCCESS;        ---->  may exit here.
        }
    }

  RtlMoveMemory(bufferStart + shiftLength, bufferStart, shiftOffset);
  NdisAdvanceNetBufferDataStart(curNb, shiftLength, FALSE, NULL);

在 2021/9/29 21:55，“dev 代表 Alin-Gabriel 
Serdean”<ovs-dev-boun...@openvswitch.org 代表 aserd...@ovn.org> 写入:

    From: aserd...@ovn.org

    Thank you for raising awareness about this issue. You are right the
    offsets are not being updated when a pop vlan action is hit, they are
    updated only on pop_mpls.
    
https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fopenvswitch%2Fovs%2Fblob%2Fmaster%2Fdatapath-windows%2Fovsext%2FActions.c%23L1173-L1174&amp;data=04%7C01%7Cpweisong%40vmware.com%7C97a24c5e1c5642cde59d08d98350cbed%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637685205333861058%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=J4d0vndzCmjpdacwp5KBVntxzNsWepJkSxqvEPRTbCQ%3D&amp;reserved=0
    Can you please update the offsets in the corresponding pop vlan
    function. I.e.:
    ---
     datapath-windows/ovsext/Actions.c | 12 +++++++++++-
     1 file changed, 11 insertions(+), 1 deletion(-)

    diff --git a/datapath-windows/ovsext/Actions.c 
b/datapath-windows/ovsext/Actions.c
    index e130c2f96..34aa5c5e4 100644
    --- a/datapath-windows/ovsext/Actions.c
    +++ b/datapath-windows/ovsext/Actions.c
    @@ -1141,11 +1141,21 @@ OvsPopVlanInPktBuf(OvsForwardingContext *ovsFwdCtx)
          * Declare a dummy vlanTag structure since we need to compute the size
          * of shiftLength. The NDIS one is a unionized structure.
          */
    +    NDIS_STATUS status;
    +    OVS_PACKET_HDR_INFO* layers = &ovsFwdCtx->layers;
         NDIS_PACKET_8021Q_INFO vlanTag = {0};
         UINT32 shiftLength = sizeof(vlanTag.TagHeader);
         UINT32 shiftOffset = sizeof(DL_EUI48) + sizeof(DL_EUI48);

    -    return OvsPopFieldInPacketBuf(ovsFwdCtx, shiftOffset, shiftLength, 
NULL);
    +    status = OvsPopFieldInPacketBuf(ovsFwdCtx, shiftOffset, shiftLength,
    +                                    NULL);
    +
    +    if (status == NDIS_STATUS_SUCCESS) {
    +        layers->l3Offset -= (UINT16) shiftLength;
    +        layers->l4Offset -= (UINT16) shiftLength;
    +    }
    +
    +    return status;
     }


    -- 
    2.32.0

    _______________________________________________
    dev mailing list
    d...@openvswitch.org
    
https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fmail.openvswitch.org%2Fmailman%2Flistinfo%2Fovs-dev&amp;data=04%7C01%7Cpweisong%40vmware.com%7C97a24c5e1c5642cde59d08d98350cbed%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637685205333861058%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=cYplthGDj6Y3JKU33UWegukyXDh7ZehNv6wXVqfWD6s%3D&amp;reserved=0

_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev