Useless while() in pfring_parse_pkt code

[pavel-odintsov]

Hello, folks!

I'm playing with parser code and have question about while loop here:

PF_RING/userland/lib/pfring_utils.c

Line 224 in 57b553b

while (hdr->extended_hdr.parsed_pkt.eth_type == 0x8100 /* 802.1q (VLAN) */ ) {

I know about vlan frames and nested vlan tags (QnQ) but I haven't idea about triple nested vlan tags. And in this case while loop is useless and could be replaced with if condition.

What do you think about it?

I'm asking this because loop and condition variable are not equal in case if performance evaluation and loops much times harder to predict for CPU.

[cardigliano]

Actually in my experience I've never seen more than two tags in a single frame,
but in practice there is no limiting to the number of tags. To be honest I would like
to remove the while but I am not sure it's the best choice.

[cmhungry]

Double tagged frame is ok for extracting IP addresses
It has inner tag and outer tag. If you configured Cisco or Juniper router you could use double tagged interfaces, single tagged or untagged. But not triple tagged etc.

Of course MTU is the key for making as much tags as you wish. But if you run three tags - it means you have transit VLAN with QinQ enabled and your customer runs his own QinQ. In this case you don't need to know what traffic he runs.

(0x8100 as ethertype is the most common, but it should be remembered that 0x8100 is not the only one option)

[pavel-odintsov]

And in case of nested vlan (QnQ) we need separate field (for example: outer_vlan) for storing external vlan number.

[cardigliano]

It makes sense. Pavel if you have a patch, please send a pull request. Please pay attention to backward compatibility (i.e. an outer_vlan_id field is fine, rather than inner_vlan_id, in addition to the current vlan_id)

[vasubabukandimalla]

Hi Pavel,

Could you please help me to find out the patch supporting QinQ in pf_ring.

Thanks,
Vasu.

[pavel-odintsov]

Hello!

I havent tried to implement this code because it's pretty complex.
Different vendors are using different approaches here with different ether
types. Some vendors offer option to change QnQ's ethertype. So you need to
write custom code acording to your network vendor.

On Tuesday, 10 May 2016, vasubabukandimalla notifications@github.com
wrote:

Hi Pavel,

Could you please help me to find out the patch supporting QinQ in pf_ring.

Thanks,
Vasu.

—
You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub
#57 (comment)

Sincerely yours, Pavel Odintsov

[pavel-odintsov]

Hello!

And this issue hit us again :) So, as I mentioned before we still need to add additional code to handle QnQ properly and this code depends on vendor's implementation.

For this purposes, we need to modify your code for each vendor and use custom version for each one. But the library is licensed under GPLv2 terms and it's incompatible with commercial/closed source code. And we could not do it :(

Maybe you could unify this code some way to export such interfaces or offer another license for code in this particular file/function?

Thanks!