WAN DHCP6 VLAN Priority tagging

[franco]

Ok here is the rub-in:

The vlan_mtag_pcp sysctl is only used twice.

https://github.com/opnsense/src/blob/master/sys/net/if_vlan.c#L1105

This is the first usage spot that checks wether a PCP was stored, also via pf. It does not allocate.

https://github.com/opnsense/src/blob/master/sys/net/if_vlan.c#L1211

And this second spot tries to preserve the PCP from a packet where the actual allocations take place.

Circling back to the description, it mentions the allocations:

https://github.com/opnsense/src/blob/master/sys/net/if_vlan.c#L160

So the value protects agains performance degradation for allocations and frees, but in the end that only pertains to the second spot which causes these allocations. Even more so, if pf made allocations, the free happens anyway, so the performance is already lost.

My preference would be to kill the first conditional so that pf modifications work always, which won't make 18.1-RC1, but there is still time for 18.1-RC2 or 18.1. That adds a list traversal overhead, but I don't see how this patch wouldn't be accepted by FreeBSD unless they want to go the other route and protect "set prio" in pf with that same sysctl (I can't see how).

This would eliminate the need for fiddling with the sysctl at all.

What do you think?


Cheers,
Franco

[marjohn56]

I would go with whatever you think is best.

As I am adding the GUI for this option, maybe the solution is to set it to one ONLY IF this type of situation occurs, there will be a flag that will be set in the config, that could be read and the value set to 1 then.

[franco]

The problematic bit is that we're generating a side-effect while unclogging "set prio" with the sysctl:

"While uncommon, it is possible that we will find a 802.1q packet encapsulated inside another packet that also had an 802.1q header.  For example, ethernet tunneled over IPSEC arriving over ethernet.  In that case, we replace the existing 802.1q PCP m_tag value."

So options are:

o removing the sysctl check in the reading spot, or
o adding a different sysctl for the reading spot, or
o making the current sysctl aware of "read-only" + "read-write" PCP mode.

Unfortunately, none of these can be fully worked around in user space. :)

Will try to get more feedback from FreeBSD committers and then commit something appropriate next week.


Cheers,
Franco

[marjohn56]

I'm sure Orange France knew of all the problems they would create when they decided on this daft idea!

[franco]

You can probably hear them laughing right now. :)

Still, fun times chasing this down.

[marjohn56]

There will be a lot of Orange France users happy when this is finally sorted. Still need the dhclient on FreeBSD sorted and dhcp6c too.

Perhaps we could create a package for them to replace the existing clients with the modified versions just for them.

[nivek1612]

Great work tracking this down

[franco]

So the sysctl usage is made obsolete here:

https://github.com/opnsense/src/commit/dabc3cf4ef

Then I added...

https://github.com/opnsense/src/commit/5f9b4916

And now I'm working on simplifying the option 77 patch:

https://github.com/opnsense/src/commit/c24697cb

I'm not sure if the latter compiles at the moment, but except for the BPF magic that needs review it should work if it builds. :)

[franco]

Small build issue, otherwise ready for testing...

https://github.com/opnsense/src/commit/f6b3e14

It's the dhclient_77 branch:

# opnsense-code src
# git checkout dhclient_77
# cd /usr/src/sbin/dhclient
# make
# make install


Cheers,
Franco

[nivek1612]

Excellent thanks Franco

Will be tomorrow before I can test (family party tonight)

[nivek1612]

Is the plan to compile a dhcp6c as well or will that need to go upstream

[marjohn56]

# opnsense-code src
# git checkout dhclient_77
# cd /usr/src/sbin/dhclient
# make
# make install


Cheers,
Franco

Should the be:

# cd /usr
# opnsense-code src
#cd /usr/src
# git checkout dhclient_77
# cd /usr/src/sbin/dhclient
# make
# make install

The build issue I assume is with Kyuafile, not important?

[nivek1612]

Im getting

fatal: Not a git repository (or any of the parent directories) .git

sure its me but I don't know what Im doing wrong

[franco]

Oh, sorry, /usr/src exists in stock install. Use this the first time:

# opnsense-code -f src

[franco]

Our next issue is here...

https://github.com/opnsense/src/commit/c24697cb44ee5ac9963ba1a7e767ae590e1ea97f#diff-4832d06a0637c8fc27150af6b8f05b19R100

The change now requests the packet for *every* packet reaching the filter instead of checking because it's returning in this first step.