Details

Reviewers

bz
krzysztof.galazka_intel.com
erj
philip
kp

Group Reviewers

network
manpages

Commits

rGa2c37aa836c0: ixl: Permit 802.1ad frames to pass though the chip
rG9c40fca6c809: ixl: Permit 802.1ad frames to pass though the chip
rGfa6662b3689e: ixl: Permit 802.1ad frames to pass though the chip

Summary

Intel X710 chips have an internal (on-chip) switch for various SDN purposes.
The internal switch used 0x88a8 to tag frames within the chip itself.
In order to keep the internal switch protected, the chip silently drops all packets with the currently used internal ethertype.

The ixl driver does already contain code to set the internally used ethertype, but this function is not called in any place.

This patch is a quick hack to change the internal switch_tag within the chip. It's only published to move things forward, to help people with run into the same problem (processing 802.1ad, or double tagged frames).

I'm unsure how move this into the normal ixl driver, but do not want to postphone this solution for a real world problem.

Test Plan

# tcpdump -eni ixl0 -c 2

nothing happens, no counters going up to tell about any incoming frame.

# sysctl -d dev.ixl.0.debug.switch_vlans
dev.ixl.0.debug.switch_vlans: HW Switch VLAN Configuration
# sysctl dev.ixl.0.debug.switch_vlans=0x9200
# dmesg | tail -1
ixl0: Setting switch config to switch_tag=9200, first_tag=0000, second_tag=0000
# tcpdump -eni ixl0 -c 2
23:49:16.356591 dc:39:6f:0a:10:78 > a0:23:9f:37:29:3f, ethertype 802.1Q-QinQ (0x88a8), length 78: vlan 2, p 0, ethertype 802.1Q, vlan 140, p 0, ethertype IPv4, 91.137.26.17 > 2.207.28.131: ICMP host 91.137.26.17 unreachable - admin prohibited filter, length 36
23:49:17.129581 00:1d:aa:d8:3d:99 > 00:07:b4:00:78:01, ethertype 802.1Q-QinQ (0x88a8), length 93: vlan 2, p 0, ethertype 802.1Q, vlan 140, p 0, ethertype IPv4, 5.102.161.204.33305 > 224.0.0.251.5353: 27179 PTR (QM)? 192.168.10.3.in-addr.arpa. (43)

Diff Detail

Repository

rS FreeBSD src repository - subversion

Lint

No Lint Coverage

Unit

No Test Coverage

Build Status

Buildable 33821
Build 31038: arc lint + arc unit

Event Timeline

donner created this revision.Mar 24 2020, 10:50 PM

Herald added subscribers: Contributor Reviews (src), ae, imp. · View Herald TranscriptMar 24 2020, 10:50 PM

Harbormaster completed remote builds in B30088: Diff 69842.Mar 24 2020, 10:50 PM

donner added reviewers: network, bz.Mar 24 2020, 10:52 PM

donner edited the test plan for this revision. (Show Details)Mar 24 2020, 10:54 PM

Patch does work with 12-STABLE, too. (removing the NEEDGIANT flag)

erj added reviewers: krzysztof.galazka_intel.com, erj.Mar 25 2020, 11:26 PM

Rebase to r366170.

Harbormaster completed remote builds in B33821: Diff 77540.Sep 25 2020, 7:24 PM

donner mentioned this in D26436: Add support for stacked VLANs (IEEE 802.1ad, AKA Q-in-Q)..Oct 21 2020, 3:22 PM

There are a few other minor style(9) nits, but it's entirely consistent with the rest of the file, so let's not argue about those.

Give Philip a day or two to take a look as well, he likes network drivers, but then I think it can go in.

It may be useful to document this sysctl in the ixl man page as well, if only to give users who run into this problem a fighting chance of figuring out that they can fix it.

sys/dev/ixl/ixl_pf_main.c
3519	`return (status);`.

fixing style(9)
adding documentation for the hack

Herald added a reviewer: manpages. · View Herald TranscriptJan 19 2021, 12:02 AM

Harbormaster completed remote builds in B36326: Diff 82512.Jan 19 2021, 12:02 AM

donner marked an inline comment as done.Jan 19 2021, 12:02 AM

kp added a reviewer: philip.Jan 19 2021, 8:37 AM

I am okay with this in principle, modulo the documentation nitpicking, but I wonder if it wouldn't be better to change the *default* to something more sensible -- i.e., have 802.1ad work out of the box. One of the "local experimentation" types come to mind. Or set it to one of the types assigned to Intel!

http://standards-oui.ieee.org/ethertype/eth.txt

What do other operating systems do?

share/man/man4/ixl.4
235 ↗	(On Diff #82512)	I would suggest wording like "if you need 802.1ad support, set this to another ethertype". Note that 0xffff is not "unused" it's allocated to some entity. Perhaps suggest 0x88b5 or 0x88b6 instead. These are reserved for "local experimentation" in IEEE 802.

brueffer added a subscriber: brueffer.Jan 19 2021, 10:10 AM

brueffer added inline comments.

share/man/man4/ixl.4
232 ↗	(On Diff #82512)	mdoc style: a new sentence should start on a new line.
233 ↗	(On Diff #82512)	This should probably be Defaults to .Dv 0x88a8 , which...
234 ↗	(On Diff #82512)	mdoc style: a new sentence should start on a new line.
235 ↗	(On Diff #82512)	Same as above, .Dv 0xffff

Speaking nitpicking: I would also consistently capitalise Ethertype and VLAN as the IEEE does. :)

In D24179#631321, @philip wrote:

I am okay with this in principle, modulo the documentation nitpicking, but I wonder if it wouldn't be better to change the *default* to something more sensible -- i.e., have 802.1ad work out of the box. One of the "local experimentation" types come to mind. Or set it to one of the types assigned to Intel!

http://standards-oui.ieee.org/ethertype/eth.txt

What do other operating systems do?

Linux is setting it to 0xffff unconditionally.

But I do not want to change this without discussion. The first step is to enable the interface at all. Then somebody can play with it. The internal bridge of the chip is most useful for virtualization, it can represent a vlan (or double tagged vlan) as an "physical" interface suited to be plugged into a VM using IO-VMM.

BTW: Linux is not exposing the interface. Hyper-V seems to use it actively.

donner added inline comments.Jan 19 2021, 12:45 PM

share/man/man4/ixl.4
235 ↗	(On Diff #82512)	Everybody out there seems to set it to 0xffff. Which should be the default in FreeBSD, too (sometimes later)

Fix man page style.

Harbormaster completed remote builds in B36336: Diff 82565.Jan 19 2021, 12:54 PM

donner marked 5 inline comments as done.Jan 19 2021, 12:54 PM

Approved by: kp (mentor)

As you say, we can change the default later. We probably should, but it also makes sense to do that in a separate commit.

This revision is now accepted and ready to land.Jan 19 2021, 1:10 PM

brueffer added inline comments.Jan 19 2021, 1:16 PM

share/man/man4/ixl.4
236 ↗	(On Diff #82565)	Just another minor nitpick, I think we capitalize LAN and VLAN everywhere. Looks good otherwise, thanks! Please bump the document date before you commit.