<META HTTP-EQUIV="Content-Type" CONTENT="text/html; charset=iso-8859-1"> <html xmlns:v="urn:schemas-microsoft-com:vml" xmlns:o="urn:schemas-microsoft-com:office:office" xmlns:w="urn:schemas-microsoft-com:office:word" xmlns:m="http://schemas.microsoft.com/office/2004/12/omml" xmlns="http://www.w3.org/TR/REC-html40"><head><meta name=Generator content="Microsoft Word 12 (filtered medium)"><style></style></head><body lang=EN-US link=blue vlink=purple style='word-wrap:break-word'><div class=WordSection1><p class=MsoNormal><a name="_MailEndCompose"><span style='font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D'>Answer inline below.<o:p></o:p></span></a></p><p class=MsoNormal><span style='font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D'><o:p> </o:p></span></p><div style='border:none;border-left:solid blue 1.5pt;padding:0cm 0cm 0cm 4.0pt'><p class=MsoNormal><b><span style='font-family:"Tahoma","sans-serif"'>From:</span></b><span style='font-family:"Tahoma","sans-serif"'> Raslan Darawsheh [mailto:rasland@nvidia.com] <br><b>Sent:</b> Monday, 24 March 2025 09.32<br><br></span></p><div id=mail-editor-reference-message-container><div><div><div><div><p class=MsoNormal><span style='font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D'>Comment inline below.</span><o:p></o:p></p></div><p class=MsoNormal style='margin-left:36.0pt'><span style='font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D'> </span><o:p></o:p></p><div style='border:none;border-left:solid blue 1.5pt;padding:0cm 0cm 0cm 4.0pt'><div><div style='border:none;border-top:solid #B5C4DF 1.0pt;padding:3.0pt 0cm 0cm 0cm'><p class=MsoNormal style='mso-margin-top-alt:0cm;margin-right:0cm;margin-bottom:12.0pt;margin-left:36.0pt'><b><span style='font-family:"Tahoma","sans-serif"'>From:</span></b><span style='font-family:"Tahoma","sans-serif"'> Raslan Darawsheh [mailto:rasland@nvidia.com] <br><b>Sent:</b> Monday, 24 March 2025 08.35</span><o:p></o:p></p></div></div><p class=MsoNormal style='margin-left:36.0pt'> <o:p></o:p></p><div id=mail-editor-reference-message-container><div><div><div><p class=MsoNormal style='mso-margin-top-alt:0cm;margin-right:0cm;margin-bottom:12.0pt;margin-left:72.0pt'><span style='font-size:11.0pt'>>> From: Raslan Darawsheh [<a href="mailto:rasland@nvidia.com">mailto:rasland@nvidia.com</a>]<br>>> Sent: Sunday, 23 March 2025 13.28<br>>> <br>>> Updated the `get_ethertype_by_ptype` function to correctly parse VLAN<br>>> headers.<br>>> <br>>> Fixes: 76730c7b9b5a ("app/testpmd: use packet type parsing API")<br>>> Cc: haijie1@huawei.com<br>>> <br>>>> Signed-off-by: Raslan Darawsheh <rasland@nvidia.com><br>>> ---<br>>> app/test-pmd/csumonly.c | 8 +++++---<br>>> 1 file changed, 5 insertions(+), 3 deletions(-)<br>>> <br>>> diff --git a/app/test-pmd/csumonly.c b/app/test-pmd/csumonly.c<br>>> index 5b906eaa53..302cc4cc66 100644<br>>> --- a/app/test-pmd/csumonly.c<br>>> +++ b/app/test-pmd/csumonly.c<br>>> @@ -468,6 +468,7 @@ get_ethertype_by_ptype(struct rte_ether_hdr<br>>> *eth_hdr, uint32_t ptype)<br>>> {<br>>> struct rte_vlan_hdr *vlan_hdr;<br>>> uint16_t ethertype;<br>>> + uint32_t i = 0;<br>>> <br>>> switch (ptype) {<br>>> case RTE_PTYPE_L3_IPV4:<br>>> @@ -486,10 +487,11 @@ get_ethertype_by_ptype(struct rte_ether_hdr<br>>> *eth_hdr, uint32_t ptype)<br>>> return _htons(RTE_ETHER_TYPE_IPV6);<br>>> default:<br>>> ethertype = eth_hdr->ether_type;<br>>> - while (eth_hdr->ether_type == _htons(RTE_ETHER_TYPE_VLAN)<br>>> ||<br>>> - eth_hdr->ether_type == _htons(RTE_ETHER_TYPE_QINQ)) {<br>>> + while (ethertype == _htons(RTE_ETHER_TYPE_VLAN) ||<br>>> + ethertype == _htons(RTE_ETHER_TYPE_QINQ)) {<br>>> vlan_hdr = (struct rte_vlan_hdr *)<br>>> - ((char *)eth_hdr + sizeof(*eth_hdr));<br>>> + ((char *)eth_hdr + sizeof(*eth_hdr) +<br>>> + (i * sizeof(struct rte_vlan_hdr)));<br><br>>Doesn't work; "i" is not incremented, and remains 0.<br>>Instead do this: Initialize (struct rte_vlan_hdr *)vlan_hdr=RTE_PTR_ADD(eth_hdr, offsetof(rte_ether_hdr, ether_type)) before the loop, and move vlan_hdr+=RTE_VLAN_HLEN inside the loop.<br>Yes you are correct I had a (i++ *) but must have slipped while rebasing will handle your way in v2 </span><o:p></o:p></p><p class=MsoNormal style='mso-margin-top-alt:0cm;margin-right:0cm;margin-bottom:12.0pt;margin-left:36.0pt'><span style='font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D'>MB: Noticed a bug in my suggestion: Due to the type of vlan_hdr, moving it should be vlan_hdr++, not vlan_hdr+=RTE_VLAN_HLEN.</span><span style='font-size:11.0pt;font-family:"Calibri","sans-serif"'><o:p></o:p></span></p><p class=MsoNormal style='margin-bottom:12.0pt'><span style='font-size:11.0pt'> Yes sure will handle, <o:p></o:p></span></p><p class=MsoNormal style='mso-margin-top-alt:0cm;margin-right:0cm;margin-bottom:12.0pt;margin-left:36.0pt'><span style='font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D'>MB: And you might want to impose a maximum boundary to the loop, as suggested by Stephen. E.g. break out of the loop if vlan_hdr>RTE_PTR_ADD(eth_hdr, offsetof(rte_ether_hdr, ether_type)+4*sizeof(struct rte_vlan_hdr)). Supporting up to 4 stacked VLAN tags should suffice.</span><span style='font-size:11.0pt;font-family:"Calibri","sans-serif"'><o:p></o:p></span></p><p class=MsoNormal style='margin-bottom:12.0pt'><span style='font-size:11.0pt'> I’m not sure this should be added to this patch, as I tend to believe it’s less critical to handle for this release, and also, this might cause inconsistent results, think of the case if we send five stacked vlan tags, then we’ll return ethertype as VLAN as we skipped the last one.<o:p></o:p></span></p><p class=MsoNormal style='margin-bottom:12.0pt'><span style='font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D'>MB:<o:p></o:p></span></p><p class=MsoNormal style='margin-bottom:12.0pt'><span style='font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D'>It should go in this patch, to prevent code analysis tools from warning about unbounded loop causing potential buffer overrun – the bug described by Stephen.<o:p></o:p></span></p><p class=MsoNormal style='margin-bottom:12.0pt'><span style='font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D'>IMO, a stack of 4 VLAN tags should suffice, but you can make it 8 to give it some extra margin. More than 8 VLAN tags is never going to happen with real traffic.<o:p></o:p></span></p><p class=MsoNormal style='margin-bottom:12.0pt'><span style='font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D'>In reality, most NICs can only strip 1 or 2 VLAN tags, and would return ethertype as VLAN. But this can be ignored for this patch.<o:p></o:p></span></p><p class=MsoNormal style='mso-margin-top-alt:0cm;margin-right:0cm;margin-bottom:12.0pt;margin-left:72.0pt'><span style='font-size:11.0pt'><br>>> ethertype = vlan_hdr->eth_proto;<br>>> }<br>>> return ethertype;<br>>> --<br>>> 2.39.5 (Apple Git-154)</span><o:p></o:p></p><p class=MsoNormal style='mso-margin-top-alt:0cm;margin-right:0cm;margin-bottom:12.0pt;margin-left:36.0pt'><span style='font-size:11.0pt'> </span><o:p></o:p></p><p class=MsoNormal style='mso-margin-top-alt:0cm;margin-right:0cm;margin-bottom:12.0pt;margin-left:36.0pt'><span style='font-size:11.0pt'>Kindest regards</span><o:p></o:p></p><p class=MsoNormal style='mso-margin-top-alt:0cm;margin-right:0cm;margin-bottom:12.0pt;margin-left:36.0pt'><span style='font-size:11.0pt'>Raslan Darawsheh</span><o:p></o:p></p></div></div></div></div></div></div></div></div></div></div></div></body></html>