Page MenuHomeFreeBSD

make ifconfig print ipv4 netmasks in dotted-quad notation, rather than hex
ClosedPublic

Authored by allanjude on Jun 18 2015, 4:40 PM.

Diff Detail

Repository
rS FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

allanjude retitled this revision from to make ifconfig print ipv4 netmasks in dotted-quad notation, rather than hex.
allanjude updated this object.
allanjude edited the test plan for this revision. (Show Details)
allanjude added reviewers: eadler, gnn, emaste.

I hate to say this but right now we'd probably want this to be a command line switch. I bet lots of scripts depend on the current functionality. I'm not saying "no" just saying "caution."

In D2856#55303, @gnn wrote:

I hate to say this but right now we'd probably want this to be a command line switch. I bet lots of scripts depend on the current functionality. I'm not saying "no" just saying "caution."

I understand that reaction, although I wonder what scripts actually do that. The only script that I know of that does use it, is bsdinstall, and it will happily accept anything, and stuff it in rc.conf, so as long as rc.conf accepts it, we're fine there. It will actually solve an issue, where you type in 255.255.255.255.248 and it comes out as 0xfffffff8 in rc.conf if you have to revisit the menu in bsdinstall.

The target for this change is 11.0-RELEASE, which seems might be enough time to allow for the switch. Although it might make sense to hide this behind a flag and get it into 10.2-RELEASE, with a note in the release notes that dotted-quad will become the default in 11?

Updated the patch to hide the behaviour behind a new switch

Also added a switch for CIDR notation

allanjude edited edge metadata.

Fix whitespace

Isn't this the 4.4 behavior that we changed for a reason? What was that reason and why does the installer's quirks justify such a fundamental change?

In D2856#72201, @imp wrote:

Isn't this the 4.4 behavior that we changed for a reason? What was that reason and why does the installer's quirks justify such a fundamental change?

The latest version of the patch does not change the default behaviour, it just offers the two additional formats as an option now.

I think the more human readable subnet mask makes more sense for a default, but I don't feel all that strongly about it.

The installer quirk is just a minor POLA thing. I type in 255.255.255.0, and if I have to restart the install process (because of the lack of error recovery in the installer), my previously entered subnet mask is returned in hex.

Looking at the ifconfig in 4.4 lite, it looks like the netmask has always been printed as a hex number and I've always found that to be less than ideal. Having options for how the subnet is displayed is a good suggestion. I like the CIDR notion for example.

In D2856#72311, @marcel wrote:

Looking at the ifconfig in 4.4 lite, it looks like the netmask has always been printed as a hex number and I've always found that to be less than ideal. Having options for how the subnet is displayed is a good suggestion. I like the CIDR notion for example.

Yes. The original impetus for this was Sean Chittenden remarking at BSDCan that the hex output confused the linux sysadmins at his company during their FreeBSD training.

I too prefer the CIDR notation, although there may be a better way to calculate it, I am still very much a newcomer to C.

marcel edited edge metadata.

Overall: I like the change. You may want to get a second thumbs up though.

sbin/ifconfig/ifconfig.8
39–40 ↗(On Diff #8321)

I fear I just fell victim to the over-engineering trap, but I have a comment -- treat it as inspirational more than a request for change:

I can't shake of this feeling that -c and -s are specific to IP (v4 and/or v6) and that different protocol families may want to change the output in some form or shape as well and that this therefore may create a flurry of "one-off" options. What if we have a single option, say -f, for format that takes an argument. The argument is a protocol-specific specifier for how to print addresses. For IP this could be "-f cidr" and "-f dotted" as the moral equivalent of "-c" and "-s" (resp).

Since we print addresses for different protocols at the same time, maybe even add a protocol prefix, like inet:dotted and allow -f to give us a list of specifiers so that we can tweak the output of multiple families at the same time.

Examples of different ways one may want to print an address:
ethernet: use dashes instead of colons between bytes, print as 3 groups of hex numbers with dots.
IPv6: don't suffix the scope, don't abbreviate, resolve to hostname, hex only, etc
IPv4: resolve to hostname, hex only, CIDR, dotted, etc

Again: this probably goes too far, but I feel is more future-proof.

This revision is now accepted and ready to land.Aug 29 2015, 5:55 PM
sbin/ifconfig/ifconfig.8
39–40 ↗(On Diff #8321)

This would likely make sense, and avoid using up all of the letters of the alphabet on one-off flags

This would also solve the problem that, in this patch, the -s and -c flags only apply to 'inet', and not inet6 or anything else.

I will have to look at how the options parsing framework works, as currently you register a flag like -c from inside af_inet.c so I am not sure how to make a -f apply to multiple modules, nor deal with the fact that some modes apply to multiple address families.

I might play with this a bit more then

allanjude edited edge metadata.

Start of the patch to fulfill marcel@'s vision

accepted -f paramters:
subnet:cidr (subnet mask in CIDR notation)
subnet:dotted (subnet mask in dotted quad notation)
ether:dash (ethernet MAC address separated by dashes instead of colons)

This revision now requires review to proceed.Aug 30 2015, 1:11 AM

Very much like the idea in general. The next request you will get after this is a way to set a system-wide default to either CIDR or dotted quad (though presumably folks can just use shell aliases)

sbin/ifconfig/ifconfig.c
353 ↗(On Diff #8343)

I would actually call this "inet" instead of "subnet" as Marcel suggested since it is IPv4 specific. I realize it's odd because it only controls the subnet/netmask display, but I think it is more consistent with -f in the long run (e.g. having "inet6" formats and the "ether" format).

In terms of scripts, the primary fear I would have is out-of-tree scripts that various sysadmins have to parse ifconfig output (they do exist) rather than bsdinstall. Otherwise I would be happy to use something else for the default as the hex is clumsy.

@jhb A global switch sounds like a great idea! How would something like that work though? login.conf setting or sysctl or?

In D2856#126654, @brd wrote:

@jhb A global switch sounds like a great idea! How would something like that work though? login.conf setting or sysctl or?

A sysctl would be odd. A new config file would be a bit clumsy. One option would be simple define default aliases in /etc/profile or /etc/csh.cshrc.

However, another route would be to use an environment variable. Other tools like ls(1) do this. For example, you could support an 'IFCONFIG_FORMATS' environment variable that was parsed before the command line arguments as if it was a -f argument (so you could still use -f to override it). I think if we go that route we should provide explicit names for the "old" formats as well (e.g. "inet:hex" and "ether:colon"). You could then set IFCONFIG_FORMATS="inet:cidr" or the like in shell rc files (e.g. /etc/profile) but also via login.conf.

In D2856#126695, @jhb wrote:
In D2856#126654, @brd wrote:

@jhb A global switch sounds like a great idea! How would something like that work though? login.conf setting or sysctl or?

A sysctl would be odd. A new config file would be a bit clumsy. One option would be simple define default aliases in /etc/profile or /etc/csh.cshrc.

However, another route would be to use an environment variable. Other tools like ls(1) do this. For example, you could support an 'IFCONFIG_FORMATS' environment variable that was parsed before the command line arguments as if it was a -f argument (so you could still use -f to override it). I think if we go that route we should provide explicit names for the "old" formats as well (e.g. "inet:hex" and "ether:colon"). You could then set IFCONFIG_FORMATS="inet:cidr" or the like in shell rc files (e.g. /etc/profile) but also via login.conf.

I like this approach. Will post diff in the next few days

In D2856#126695, @jhb wrote:
In D2856#126654, @brd wrote:

@jhb A global switch sounds like a great idea! How would something like that work though? login.conf setting or sysctl or?

A sysctl would be odd. A new config file would be a bit clumsy. One option would be simple define default aliases in /etc/profile or /etc/csh.cshrc.

However, another route would be to use an environment variable. Other tools like ls(1) do this. For example, you could support an 'IFCONFIG_FORMATS' environment variable that was parsed before the command line arguments as if it was a -f argument (so you could still use -f to override it). I think if we go that route we should provide explicit names for the "old" formats as well (e.g. "inet:hex" and "ether:colon"). You could then set IFCONFIG_FORMATS="inet:cidr" or the like in shell rc files (e.g. /etc/profile) but also via login.conf.

Thanks for the explanation!

Sorry for the tangent everyone!

Updated with feedback from jhb

gnn edited edge metadata.
This revision is now accepted and ready to land.May 31 2016, 4:47 PM
This revision was automatically updated to reflect the committed changes.
hrs added inline comments.
head/sbin/ifconfig/af_inet.c
56

Please replace MAXHOSTNAMELEN * 2 + 1 with NI_MAXHOST. It can allso be found in other places but MAXHOSTNAMELEN is not for a domain name.

head/sbin/ifconfig/af_inet6.c
67

Is there any practical use case of f_addr == "full" or f_scope?

I think we should conform to RFC 5952 about the leading zeros even if the fixed-width representation may look useful. And I cannot understand where f_scope == "none" is useful. A scope id is a part of a link-local scope address.

head/sbin/ifconfig/af_link.c
70

Please use strchr(3).

allanjude added inline comments.
head/sbin/ifconfig/af_inet6.c
67

I admit I was just looking to see what else I could do, to make something more robust, rather than just -c and -s for CIDR and subnet.

Removed.

This revision is now accepted and ready to land.Jun 1 2016, 12:45 AM
allanjude edited edge metadata.

Address feedback from hrs@

This revision now requires review to proceed.Jun 1 2016, 12:45 AM
allanjude edited edge metadata.

Update man page to match partial reversion

sbin/ifconfig/af_link.c
69 ↗(On Diff #17172)

Looking at this again, it might be simpler to always use 'ether_format':

ether_format = ether_ntoa((struct ether_addr *)LLADDR(sdl));
if (f_ether != NULL && strcmp(f_ether, "dash") == 0) {
    for (format_char = strchr(ether_format, ':'); format_char != NULL;
        format_char = strchr(format_char, ':'))
        *format_char = '-';
}
printf("\tether %s\n", ether_format);
gnn edited edge metadata.
This revision is now accepted and ready to land.Jun 2 2016, 3:04 AM
This revision was automatically updated to reflect the committed changes.