Thank you for addressing the comments I applied the incremental version and backported it until 2.7.
Thank you, Alin. -----Original Message----- From: Wilson Peng <pweis...@vmware.com> Sent: Thursday, September 30, 2021 8:02 AM 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, 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&data=04%7C01%7Cpweisong%40vmware.com%7Ca65191ed9abd47b21a8608d9835cddfe%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637685257162775283%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=oonWmImrDVicJ%2BBdGaYvmNDiUsueMXwOOjSn9MuFmBE%3D&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&data=04%7C01%7Cpweisong%40vmware.com%7Ca65191ed9abd47b21a8608d9835cddfe%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637685257162775283%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=29gFWHNVRPTixEkYAQeD6HevknOxi6%2BYerYFAUM3FTI%3D&reserved=0 _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev