Page MenuHomeFreeBSD

Remove classful masks from interface addition code.
Needs RevisionPublic

Authored by melifaro on Feb 13 2021, 10:52 AM.

Details

Reviewers
glebius
Group Reviewers
network
Summary

Classful networks approach was deprecated in favour of CIDR quite a while ago.

However, we still have some parts of it in the kernel. If one wants to add an interface address/alias and adds it without a mask, instead of adding "host" address, kernel adds a mask based on classful addressing:

10:52 [1] m@devel2 s ifconfig vtnet0 alias 10.11.0.2
10:52 [1] m@devel2 s ifconfig vtnet0
vtnet0: flags=8863<UP,BROADCAST,RUNNING,SIMPLEX,MULTICAST> metric 0 mtu 1500
	options=4c07bb<RXCSUM,TXCSUM,VLAN_MTU,VLAN_HWTAGGING,JUMBO_MTU,VLAN_HWCSUM,TSO4,TSO6,LRO,VLAN_HWTSO,LINKSTATE,TXCSUM_IPV6>
	ether 52:54:00:14:e3:19
	inet 10.0.0.8 netmask 0xffffff00 broadcast 10.0.0.255
	inet 10.11.0.2 netmask 0xff000000 broadcast 10.255.255.255
                               ^^^^^^^^^^^

Retire this behaviour and assume /32 mask in such cases.

Test Plan
11:26 [0] m@devel2 s ifconfig vtnet0 alias 10.11.0.2
11:27 [0] m@devel2 s ifconfig vtnet0 inet
vtnet0: flags=8863<UP,BROADCAST,RUNNING,SIMPLEX,MULTICAST> metric 0 mtu 1500
	options=4c07bb<RXCSUM,TXCSUM,VLAN_MTU,VLAN_HWTAGGING,JUMBO_MTU,VLAN_HWCSUM,TSO4,TSO6,LRO,VLAN_HWTSO,LINKSTATE,TXCSUM_IPV6>
	inet 10.0.0.8 netmask 0xffffff00 broadcast 10.0.0.255
	inet 10.11.0.2 netmask 0xffffffff broadcast 10.11.0.2

Diff Detail

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

Event Timeline

melifaro added a reviewer: network.
melifaro edited the summary of this revision. (Show Details)

Should be mentioned in the releasenotes, when it comes to that, since this is a change in behavior exposed to the user.

Should be mentioned in the releasenotes, when it comes to that, since this is a change in behavior exposed to the user.

Sure!

I would like to see a survey of what some other OS's due in this situation. I have participated in discussions elsewhere on this issue and just blindly making these host only, IMHO, is probably going to silently cause a lot of failures on finger memory. Infact it is probalby safer to assume a /24 than a /32. BUTT I think the preferable solution here may be to return an error if the netmask is not specified, thus doing nothing rather than guessing and doing something wrong.

Infact it is probalby safer to assume a /24 than a /32.

No, please. It would assume, that the user is using the system in a home lan only.

BUTT I think the preferable solution here may be to return an error if the netmask is not specified, thus doing nothing rather than guessing and doing something wrong.

I'd like to have a warning: "Host mask missing, assuming /32".

Infact it is probalby safer to assume a /24 than a /32.

No, please. It would assume, that the user is using the system in a home lan only.

/24 is not an assumption of home lab, it is the longest of the 3 prefixes that would of been picked before, and infact is probably most often the correct mask to be used.

BUTT I think the preferable solution here may be to return an error if the netmask is not specified, thus doing nothing rather than guessing and doing something wrong.

I'd like to have a warning: "Host mask missing, assuming /32".

If this code path is Only for -alias I would accept a /32 and a warning as a possible solution.

glebius requested changes to this revision.Feb 26 2021, 4:56 PM
glebius added a subscriber: glebius.

IMHO, a good sysadmin should always specify mask. And good program should require mask. However, there was historical behavior to guess mask based on classes. Thus, my personal preference list is the following:

  1. Keep the historical behavior.
  2. Return error if mask is empty.
  3. Change to a different behavior.

Don't consider my decline as a blocker. If majority thinks differently, I'm not blocking this change.

This revision now requires changes to proceed.Feb 26 2021, 4:56 PM
  1. Keep the historical behavior.

The key of this review is to remove this historical technical depth for very good reasons.
Primary reason: Classfull is almost always the wrong choice nowadays.

  1. Return error if mask is empty.

That would be the technically correct solution.

  1. Change to a different behavior.

Assigning a host mask (/32 /128) is the most secure default for a missing netmask.
Assuming that the missing netmask is a hint to a technically illiterate admin, would suggest a /24 (/64) as a typical default.

I'd vote for default host mask and a visible warning, that the mask is missing.