Page MenuHomeFreeBSD

Set MAC to broadcast when pinging 255.255.255.255 as noted in RFC919
AcceptedPublic

Authored by antranigv_freebsd.am on Jan 16 2021, 10:12 PM.

Details

Reviewers
kp
melifaro
tuexen
bz
Group Reviewers
network
Summary

According to RFC919, when pinging 255.255.255.255 the MAC address should be set to broadcast (ffff ffff ffff).
Relates to Bug 252596 submitted by jcaplan

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint OK
Unit
No Unit Test Coverage
Build Status
Buildable 36283
Build 33172: arc lint + arc unit

Event Timeline

antranigv_freebsd.am retitled this revision from Set MAC to broadcast when pinging 255.255.255.255 to Set MAC to broadcast when pinging 255.255.255.255 as noted in RFC919.Jan 16 2021, 10:19 PM
antranigv_freebsd.am edited the summary of this revision. (Show Details)
antranigv_freebsd.am added a reviewer: network.
This revision is now accepted and ready to land.Jan 16 2021, 10:47 PM

We already have IP_SENDONES flag for ip_output, that should serve for this purpose. Maybe is it not safe enough to allow this feature for all protocols?

In D28200#630843, @ae wrote:

We already have IP_SENDONES flag for ip_output, that should serve for this purpose. Maybe is it not safe enough to allow this feature for all protocols?

I think that serves a different purpose. IP_SENDONES allows users to decide the packet should be broadcast, regardless of destination address. This change fixes an oversight, where we don't mark a packet as being broadcast (and thus send it to ethernet ff:ff:ff:ff:ff:ff) if it's sent to 255.255.255.255.

@kp should we merge this or it's stale? :) Just reminding.

So, I've been looking at committing this, and I'm still convinced that the previous behaviour is indeed incorrect, but I'm no longer sure this is the correct fix.
We should probably call in_broadcast() to run this check, rather than duplicate the logic here.

In D28200#722356, @kp wrote:

So, I've been looking at committing this, and I'm still convinced that the previous behaviour is indeed incorrect, but I'm no longer sure this is the correct fix.
We should probably call in_broadcast() to run this check, rather than duplicate the logic here.

Yes, please use in_broadcast() to do the check.

sys/netinet/ip_output.c
506

INADDR_ANY looks very wrong here, this would make 0.0.0.0 a broadcast address. Please see D31861 as we just fixed that in the general case.