net: Check per-flow priority code point for untagged traffic

[zlei]

Summary

Commit 868aabb4708d introduced per-flow priority. There's a fault in the
logic for untagged traffic, it does not check M_VLANTAG which is set in
the mbuf packet header and pass the mbuf to underlaying interface. When
the driver is incapable or disabled to do hardware VLAN tag insertion then
the outbound packets will not include the desired priority.

Some drivers happen to work due to bug mentioned in D39499, so it may not
repeat on those drivers.

The current logic also bypass soft pad processing and checking priority
set by firewall (currently only pf).

Fix by checking flag M_VLANTAG which is set in mbuf packet header and
priority set in mbuf tag MTAG_8021Q / MTAG_8021Q_PCP_OUT.

PR: 273431
Fixes: 868aabb4708d Add IP(V6)_VLAN_PCP to set 802.1 priority per-flow
MFC after: 1 week

Test Plan

Test per-flow priority and priority set by firewall (pf) .

em0 <---> igc0

Capture packets on igc0

# ifconfig igc0 up
# tcpdump -nevi igc0

Setup

# ifconfig em0 -vlanhwtag -pcp
# ifconfig em0 inet 192.0.2.1/24
# arp -s 192.0.2.2 0:1:2:3:4:5

Test per-flow priority

# ping -c1 -C3 192.0.2.2

Test priority set by firewall (pf) .

# echo "pass out proto icmp set prio 5" > /tmp/pf.conf
# kldload pf
# pfctl -f /tmp/pf.conf
# pfctl -e
pf enabled
# sysctl net.link.vlan.mtag_pcp=1
net.link.vlan.mtag_pcp: 0 -> 1
# ping -c1 192.0.2.2

[zlei]

I think that it happens to work because many hardwares have VLAN tag offload, and the driver rely on (m->m_flags & M_VLANTAG) to do VLAN tagging (without checking its capability IFCAP_VLAN_HWTAGGING).

[melifaro]

sys/net/if_ethersubr.c

450–453

476

Do we need both M_VLANTAG and IFT_L2VLAN checks?
It's worth commenting the usecases here.

[zlei]

sys/net/if_ethersubr.c

476

Do we need both M_VLANTAG and IFT_L2VLAN checks?
It's worth commenting the usecases here.

Actually it is perfect valid for users to set per-flow PCP and the transmit interface is not if_vlan.

[rscheff]

So when I added the capability, it happened to work incidenitally one the nic drivers i had tested this on? And this would extend the proper operation to other drivers (or skip the PCP setting, when no vlan tagging capabilities exist?

[zlei]

In D39536#899990, @rscheff wrote:

So when I added the capability, it happened to work incidenitally one the nic drivers i had tested this on?

I think so. I've tested with re(4) and cxgbe(4), both work even I disable vlanhwtag (ifconfig re0 -vlanhwtag).

And this would extend the proper operation to other drivers (or skip the PCP setting, when no vlan tagging capabilities exist?

Yes.
I do not have such devices, but I expect per-flow PCP will fail on if_epair with the recent change c69ae8419734 .

[zlei]

sys/net/if_ethersubr.c

475

We should respect firewall (pf) 's PCP tag. Can @kp comment on this ?

See also mailing list https://lists.freebsd.org/archives/freebsd-net/2023-August/003852.html

[kib]

sys/net/if_ethersubr.c

471

Blank lines are not needed (and style(9) actually prohibits them)

472

[zlei]

Fix errors when compile.

[zlei]

Style(9).

[zlei]

sys/net/if_ethersubr.c

476

Do we need both M_VLANTAG and IFT_L2VLAN checks?
It's worth commenting the usecases here.

I may misread @melifaro 's comment.

IFT_L2VLAN check is still needed, as ether_output_frame() is also called by if_vlan and the ifp->if_type == IFT_L2VLAN. It is introduced in 47c39432b11a (Unbreak VLANs after r337943) by @np.

[kp]

sys/net/if_ethersubr.c

475

pf uses vlan_set_pcp(), so it adds an MTAG_8021Q/MTAG_8021Q_PCP_OUT mbuf tag with the desired PCP.

ether_8021q_frame() looks for that, as does ether_do_pcp(), so I'd expect this to work fine with pf.

[zlei]

sys/net/if_ethersubr.c

475

Tested with pf, looks good so far.

[zlei]

Ping .