Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support ARP for 802 networks. #1166

Closed
wants to merge 1 commit into from
Closed

Conversation

dennypage
Copy link
Contributor

Enable ARP support for hardware type 6, 802 networks, which includes 802.3 Ethernet, 802.4 Token Bus and 802.5 token Ring.

802 network types are processed the same as the original 10Mb Ethernet (hardware type 1).

References:

…Token Ring).

Signed-off-by: Denny Page <dennypage@me.com>
Copy link
Contributor

@gmshake gmshake left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The FDDI and Token Ring support have been removed via 69f0fec, 4204224 and eec0241 . I believe this change is against the FreeBSD's approach. CC @brooksdavis .

@dennypage
Copy link
Contributor Author

This PR does not re-add anything specific to Token Ring or FDDI. This is adding support for generic 802 ARP packets.

All the 802 networks, of which 802.3 Ethernet is one, use the same ARP hardware type (6) and interchangeable MAC address scheme. Since they all use the same values, there is no way to say "hardware type 6 is valid for the 802.3 variant only" and technically it would work for any of them. That said, since Token Ring 802.5 drivers have been removed from FreeBSD there would be no way to receive ARP packets from 802.5 Token Ring to begin with.

With specific regard to Ethernet, while hardware type 1 (original Ethernet) is the most commonly used to be sure, some IP implementations use hardware type 6 (Ethernet 802.3). Receipt of these 802.3 ARP packets currently triggers an error from the kernel "arp: packet with unknown hardware format 0x06 received on XXXX." This PR is targeted at addressing that issue.

If the reference to "tb/tr" in the comment in if_arp.h is bothersome, it can be removed.

@bsdimp
Copy link
Member

bsdimp commented Apr 19, 2024

@glebius can you comment on this?

@gmshake
Copy link
Contributor

gmshake commented Apr 22, 2024

This PR does not re-add anything specific to Token Ring or FDDI. This is adding support for generic 802 ARP packets.

All the 802 networks, of which 802.3 Ethernet is one, use the same ARP hardware type (6) and interchangeable MAC address scheme. Since they all use the same values, there is no way to say "hardware type 6 is valid for the 802.3 variant only" and technically it would work for any of them. That said, since Token Ring 802.5 drivers have been removed from FreeBSD there would be no way to receive ARP packets from 802.5 Token Ring to begin with.

With specific regard to Ethernet, while hardware type 1 (original Ethernet) is the most commonly used to be sure, some IP implementations use hardware type 6 (Ethernet 802.3). Receipt of these 802.3 ARP packets currently triggers an error from the kernel "arp: packet with unknown hardware format 0x06 received on XXXX." This PR is targeted at addressing that issue.

I'm get even confused by the above statement. How is that possible ?
Currently only ethernet (ii), firewire and infiniband consume NETISR_ARP / arpintr(), so I'd expect this change will introduce unreachable code only. PS, FreeBSD's kernel do not have implementation for 802.3 Ethernet IIUC. Correct me if wrong.

Do you have any issues with that "arp: packet with unknown hardware format 0x06 received on XXXX.", or are you planning to support 802.3 Ethernet ?

If the reference to "tb/tr" in the comment in if_arp.h is bothersome, it can be removed.

@dennypage
Copy link
Contributor Author

Do you have any issues with that "arp: packet with unknown hardware format 0x06 received on XXXX."

Yes, I see this in real life, as do others. Execute this search:

google "arp: packet with unknown hardware format 0x06 received on"

There are a number of posts with occurrences.

In my case the issue comes from IoT devices (electrical switches) on startup. I cannot speak to what devices cause the issues for others.

@gmshake
Copy link
Contributor

gmshake commented Apr 23, 2024

Do you have any issues with that "arp: packet with unknown hardware format 0x06 received on XXXX."

Yes, I see this in real life, as do others. Execute this search:

google "arp: packet with unknown hardware format 0x06 received on"

There are a number of posts with occurrences.

In my case the issue comes from IoT devices (electrical switches) on startup. I cannot speak to what devices cause the issues for others.

I think that is a X problem, but this change is for Y and is not complete.

I'd recommend firstly do packet sniff on wire to trace the root cause of X, i.e. "arp: packet with unknown hardware format 0x06 received on" . And since you know those packets (unknown hardware format 0x06) are from IoT devices on startup, you may want to have contact with the vendor to ask for why.

@emaste
Copy link
Member

emaste commented Apr 23, 2024

introduce unreachable code only

The code is definitely reachable as logs indicate, so presumably this means the ARP packet is carried in an ETHERTYPE_IP Ethernet frame, but is for 802.2. It'd be interesting to see a packet capture of one of these.

@dennypage
Copy link
Contributor Author

And since you know those packets (unknown hardware format 0x06) are from IoT devices on startup, you may want to have contact with the vendor to ask for why.

Couple of things here:

  • There is nothing wrong with the vendor sending 802.2 packets in the network. This is fully allowed by the 802.X standards.
  • Contacting an IoT hardware vendor and suggesting that they redo how their devices operate is simply not practical. Particularly when what they are currently doing is defined/allowed by the standards.

As far as I am able to discern, there is no downside to this change. This is a very simple fix to address a simple standards compliance issue. May I ask why there is so much reluctance?

@emaste
Copy link
Member

emaste commented Apr 23, 2024

Merged in fcdf9a1, thanks for the contribution! It turns out this was accidentally removed in 0437c8e.

@emaste emaste closed this Apr 23, 2024
freebsd-git pushed a commit that referenced this pull request Apr 23, 2024
This is used by 802.3 Ethernet.  (Also be used by 802.4 Token Bus and
802.5 Token Ring, but we don't support those.)

This was accidentally removed along with FDDI support in commit
0437c8e, presumably because comments implied it was used only by
FDDI or Token Ring.

Fixes: 0437c8e ("Remove support for FDDI networks.")
Reviewed-by: emaste
Signed-off-by: Denny Page <dennypage@me.com>
Pull-request: #1166
@dennypage
Copy link
Contributor Author

@emaste Thank you kindly. FWIW, I didn't realized that it had been previously removed. Apologies for not catching that.

@dennypage dennypage deleted the arp_802 branch April 23, 2024 18:58
@emaste
Copy link
Member

emaste commented Apr 23, 2024

@dennypage no worries, I didn't realize it had been removed either. It wasn't until I started looking at arpintr() history in other BSDs, and then found the commit in FreeBSD that removed it.

@gmshake
Copy link
Contributor

gmshake commented Apr 24, 2024

introduce unreachable code only

The code is definitely reachable as logs indicate, so presumably this means the ARP packet is carried in an ETHERTYPE_IP Ethernet frame, but is for 802.2. It'd be interesting to see a packet capture of one of these.

I'm sorry for "unreachable code only".
I meant, for the current logic of processing ethernet frames ( logic from ether_demux() ) , all 802.2 frame will be discarded ( let's forget netgraph / lacp right now ). So when normal packet is coming in, i.e., ARP packet encapsulated in Ethernet II frame, this new code becomes unreachable.

I do not think it is valid to have such ARP packets ( encapsulate hardware type IEEE 802 ARP in Ethernet II frame ), and this change is going to hide the real problems ( that is the X problem, invalid encapsulation ).

@glebius
Copy link
Member

glebius commented Apr 24, 2024

I second Zhenlei opinion here. Do we have any evidence that such packets from IoT devices are actually meaningful, and if we start processing them we will enable a meaningful communication that previously failed? IMHO, the change was checked in prematurely.

@gmshake
Copy link
Contributor

gmshake commented Apr 24, 2024

And since you know those packets (unknown hardware format 0x06) are from IoT devices on startup, you may want to have contact with the vendor to ask for why.

Couple of things here:

  • There is nothing wrong with the vendor sending 802.2 packets in the network. This is fully allowed by the 802.X standards.

Yes, you're right. But, the FreeBSD ( from stable/12 to CURRENT ) 's net stack will drop those packets. PS: IIRC Linux does the same, unless the module llc is loaded. See the Kconfig option CONFIG_LLC . And it seems Linux do not support IP in 802.2 frame from the mailing list https://lkml.iu.edu/hypermail/linux/kernel/1107.3/01421.html.
So why should it ( ARP packets in 802 frame ) be supported ?

  • Contacting an IoT hardware vendor and suggesting that they redo how their devices operate is simply not practical. Particularly when what they are currently doing is defined/allowed by the standards.

I do not think so. The hardware vendor may do right or not ( although unlikely ). Sometime ( maybe ) that is on purpose. So the vendor knows precisely what happens. And the packet sniff is then important.

As far as I am able to discern, there is no downside to this change. This is a very simple fix to address a simple standards compliance issue. May I ask why there is so much reluctance?

Yes the fix is simple. No it towards the wrong direction.

@dennypage
Copy link
Contributor Author

dennypage commented Apr 24, 2024

But, the FreeBSD ( from stable/12 to CURRENT ) 's net stack will drop those packets. PS: IIRC Linux does the same, unless the module llc is loaded. See the Kconfig option CONFIG_LLC . And it seems Linux do not support IP in 802.2 frame from the mailing list https://lkml.iu.edu/hypermail/linux/kernel/1107.3/01421.html.
So why should it ( ARP packets in 802 frame ) be supported ?

FreeBSD's net stack does not drop these packets. An 802 ethernet IP frame and an ARP packet with an 802 type are not the same thing.

FWIW, searching old mailing lists isn't the easiest way to find out how Linux operates. The actual code can be found here. Linux does handle the IEEE 802 ARP type, and treats ETHER, IEEE802 and FDDI ARP types identically as prescribed. It's been this way since at least Linux 2.6, released in 2004, which is the oldest available on git.kernel.org.

freebsd-git pushed a commit that referenced this pull request Apr 27, 2024
This is used by 802.3 Ethernet.  (Also be used by 802.4 Token Bus and
802.5 Token Ring, but we don't support those.)

This was accidentally removed along with FDDI support in commit
0437c8e, presumably because comments implied it was used only by
FDDI or Token Ring.

Fixes: 0437c8e ("Remove support for FDDI networks.")
Reviewed-by: emaste
Signed-off-by: Denny Page <dennypage@me.com>
Pull-request: #1166
(cherry picked from commit fcdf9a1)
freebsd-git pushed a commit that referenced this pull request Apr 27, 2024
This is used by 802.3 Ethernet.  (Also be used by 802.4 Token Bus and
802.5 Token Ring, but we don't support those.)

This was accidentally removed along with FDDI support in commit
0437c8e, presumably because comments implied it was used only by
FDDI or Token Ring.

Fixes: 0437c8e ("Remove support for FDDI networks.")
Reviewed-by: emaste
Signed-off-by: Denny Page <dennypage@me.com>
Pull-request: #1166
(cherry picked from commit fcdf9a1)
(cherry picked from commit d776dd5)
fichtner pushed a commit to opnsense/src that referenced this pull request May 2, 2024
This is used by 802.3 Ethernet.  (Also be used by 802.4 Token Bus and
802.5 Token Ring, but we don't support those.)

This was accidentally removed along with FDDI support in commit
0437c8e, presumably because comments implied it was used only by
FDDI or Token Ring.

Fixes: 0437c8e ("Remove support for FDDI networks.")
Reviewed-by: emaste
Signed-off-by: Denny Page <dennypage@me.com>
Pull-request: freebsd/freebsd-src#1166
(cherry picked from commit fcdf9a19893b9b5beb7a21407de507f0ae4c500b)
(cherry picked from commit d776dd5)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
5 participants