Comment inline below.

From: Raslan Darawsheh [mailto:rasland@nvidia.com]
Sent: Monday, 24 March 2025 08.35

>> From: Raslan Darawsheh [mailto:rasland@nvidia.com]
>> Sent: Sunday, 23 March 2025 13.28
>>
>> Updated the `get_ethertype_by_ptype` function to correctly parse VLAN
>> headers.
>>
>> Fixes: 76730c7b9b5a ("app/testpmd: use packet type parsing API")
>> Cc: haijie1@huawei.com
>>
>>> Signed-off-by: Raslan Darawsheh <rasland@nvidia.com>
>> ---
>>  app/test-pmd/csumonly.c | 8 +++++---
>>  1 file changed, 5 insertions(+), 3 deletions(-)
>>
>> diff --git a/app/test-pmd/csumonly.c b/app/test-pmd/csumonly.c
>> index 5b906eaa53..302cc4cc66 100644
>> --- a/app/test-pmd/csumonly.c
>> +++ b/app/test-pmd/csumonly.c
>> @@ -468,6 +468,7 @@ get_ethertype_by_ptype(struct rte_ether_hdr
>> *eth_hdr, uint32_t ptype)
>>  {
>>        struct rte_vlan_hdr *vlan_hdr;
>>        uint16_t ethertype;
>> +     uint32_t i = 0;
>>
>>        switch (ptype) {
>>        case RTE_PTYPE_L3_IPV4:
>> @@ -486,10 +487,11 @@ get_ethertype_by_ptype(struct rte_ether_hdr
>> *eth_hdr, uint32_t ptype)
>>                return _htons(RTE_ETHER_TYPE_IPV6);
>>        default:
>>                ethertype = eth_hdr->ether_type;
>> -             while (eth_hdr->ether_type == _htons(RTE_ETHER_TYPE_VLAN)
>> ||
>> -                     eth_hdr->ether_type == _htons(RTE_ETHER_TYPE_QINQ)) {
>> +             while (ethertype == _htons(RTE_ETHER_TYPE_VLAN) ||
>> +                     ethertype == _htons(RTE_ETHER_TYPE_QINQ)) {
>>                        vlan_hdr = (struct rte_vlan_hdr *)
>> -                             ((char *)eth_hdr + sizeof(*eth_hdr));
>> +                             ((char *)eth_hdr + sizeof(*eth_hdr) +
>> +                             (i * sizeof(struct rte_vlan_hdr)));

>Doesn't work; "i" is not incremented, and remains 0.
>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.
Yes you are correct I had a (i++ *) but must have slipped while rebasing will handle your way in v2

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.

                 Yes sure will handle,

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.

              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.

>>                        ethertype = vlan_hdr->eth_proto;
>>                }
>>                return ethertype;
>> --
>> 2.39.5 (Apple Git-154)

Kindest regards

Raslan Darawsheh