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

[Wilson Peng]

Alin,

Thanks for your comments, I have sent out the updated patch, please help check.
https://patchwork.ozlabs.org/project/openvswitch/patch/20210930045626.9250-1-pweis...@vmware.com/

Regards
Wilson

在 2021/9/29 23:21，“Alin-Gabriel Serdean”<aserd...@ovn.org> 写入:

    Indeed. Thanks for pointing that out.

    To be honest that looks like a bug.
    We should return an error if the it is not a valid VLAN frame.

    -----Original Message-----
    From: Wilson Peng <pweis...@vmware.com> 
    Sent: Wednesday, September 29, 2021 5:52 PM
    To: Alin-Gabriel Serdean <aserd...@ovn.org>; ovs-dev@openvswitch.org
    Subject: Re: [ovs-dev] [PATCH] datapath-windows:adjust Offset when 
processing packet in POP_VLAN action

    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%7Ca65191ed9abd47b21a8608d9835cddfe%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637685257162775283%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=oonWmImrDVicJ%2BBdGaYvmNDiUsueMXwOOjSn9MuFmBE%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%7Ca65191ed9abd47b21a8608d9835cddfe%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637685257162775283%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=29gFWHNVRPTixEkYAQeD6HevknOxi6%2BYerYFAUM3FTI%3D&amp;reserved=0



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