Spot this while testing function of PCP. Reported here in case someone else encounter the same issue. Steps to repeat: 1. Create vlan interface with vlanpcp 2. Send ICMP requests with custom PCP 3. Verify the vlan interface's vlanpcp, it should not be altered Snip to simplify the repeat: ``` # ifconfig epair create epair0a # ifconfig epair0a.10 create vlanpcp 7 inet 192.0.2.1/24 # jail -ic vnet persist 1 # ifconfig epair0b vnet 1 # jexec 1 ifconfig epair0b up # jexec 1 ifconfig epair0b.10 create inet 192.0.2.2/24 # ping -C1 -c1 -t1 -q 192.0.2.2 # ifconfig epair0a.10 | grep vlanpcp vlan: 10 vlanproto: 802.1q vlanpcp: 1 parent interface: epair0a ```
WIP fix: https://reviews.freebsd.org/D39505
A commit in branch main references this bug: URL: https://cgit.FreeBSD.org/src/commit/?id=838c8c47860a0203130bd558d507a0565a2eba8f commit 838c8c47860a0203130bd558d507a0565a2eba8f Author: Zhenlei Huang <zlei@FreeBSD.org> AuthorDate: 2023-08-23 09:48:12 +0000 Commit: Zhenlei Huang <zlei@FreeBSD.org> CommitDate: 2023-08-23 09:53:48 +0000 net: Do not overwrite if_vlan's PCP In commit c7cffd65c5d8 the function ether_8021q_frame() was slightly refactored to use pointer of struct ether_8021q_tag as parameter qtag to include the new option proto. It is wrong to write to qtag->pcp as it will effectively change the memory that qtag points to. Unfortunately the transmit routine of if_vlan parses pointer of the member ifv_qtag of its softc which stores vlan interface's PCP internally, when transmitting mbufs that contains PCP the vlan interface's PCP will get overwritten. Fix by operating on a local copy of qtag->pcp. Also mark 'struct ether_8021q_tag' as const so that compilers can pick up such kind of bug. PR: 273304 Reviewed by: kp Fixes: c7cffd65c5d85 Add support for stacked VLANs (IEEE 802.1ad, AKA Q-in-Q) MFC after: 3 days Differential Revision: https://reviews.freebsd.org/D39505 sys/net/ethernet.h | 2 +- sys/net/if_ethersubr.c | 7 ++++--- 2 files changed, 5 insertions(+), 4 deletions(-)
A commit in branch stable/13 references this bug: URL: https://cgit.FreeBSD.org/src/commit/?id=96c4b704ec169044aa98867965e046a4728bcba1 commit 96c4b704ec169044aa98867965e046a4728bcba1 Author: Zhenlei Huang <zlei@FreeBSD.org> AuthorDate: 2023-08-23 09:48:12 +0000 Commit: Zhenlei Huang <zlei@FreeBSD.org> CommitDate: 2023-08-30 10:09:25 +0000 net: Do not overwrite if_vlan's PCP In commit c7cffd65c5d8 the function ether_8021q_frame() was slightly refactored to use pointer of struct ether_8021q_tag as parameter qtag to include the new option proto. It is wrong to write to qtag->pcp as it will effectively change the memory that qtag points to. Unfortunately the transmit routine of if_vlan parses pointer of the member ifv_qtag of its softc which stores vlan interface's PCP internally, when transmitting mbufs that contains PCP the vlan interface's PCP will get overwritten. Fix by operating on a local copy of qtag->pcp. Also mark 'struct ether_8021q_tag' as const so that compilers can pick up such kind of bug. PR: 273304 Reviewed by: kp Fixes: c7cffd65c5d85 Add support for stacked VLANs (IEEE 802.1ad, AKA Q-in-Q) MFC after: 3 days Differential Revision: https://reviews.freebsd.org/D39505 (cherry picked from commit 838c8c47860a0203130bd558d507a0565a2eba8f) sys/net/ethernet.h | 2 +- sys/net/if_ethersubr.c | 7 ++++--- 2 files changed, 5 insertions(+), 4 deletions(-)