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%7C97a24c5e1c5642cde59d08d98350cbed%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637685205333861058%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=J4d0vndzCmjpdacwp5KBVntxzNsWepJkSxqvEPRTbCQ%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%7C97a24c5e1c5642cde59d08d98350cbed%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637685205333861058%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=cYplthGDj6Y3JKU33UWegukyXDh7ZehNv6wXVqfWD6s%3D&reserved=0 _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev