Page MenuHomeFreeBSD

Allow route change requests to not specify the gateway.
ClosedPublic

Authored by rstone on Feb 10 2018, 1:52 AM.

Details

Summary

Only require a gateway to be specified on a route add request. On
a route change request that does not specify the gateway, the
gateway will remain the same. This allows changing other route
parameters without having to re-specifying the gateway, like in
"route change 10.0.0.0/8 -mtu 9000".

MFC after: 2 weeks
Sponsored by: Dell EMC Isilon

Diff Detail

Lint
Lint OK
Unit
No Unit Test Coverage
Build Status
Buildable 15083
Build 15176: arc lint + arc unit

Event Timeline

rstone created this revision.Feb 10 2018, 1:52 AM

Do you really need to mix whitespace change with the fix (last chunk)?

This revision is now accepted and ready to land.Feb 10 2018, 11:49 AM

This is missing the associated man page change that changes gateway to [gateway] on the change syntax,
Though perhaps BSD did allow this in the past, the man page never said that the gateway was optional as far as I can recall.

karels added a subscriber: karels.Feb 10 2018, 7:30 PM

I agree with the change as well; the regression seems to have happened when the code was refactored and the "add" and "change" code became joined.

ae added a comment.Feb 13 2018, 9:53 AM

How it supposed to work for RADIX_MPATH case? I know RADIX_MPATH is currently broken, but anyway...

rstone updated this revision to Diff 39386.Feb 16 2018, 4:42 PM

Fix the manpage to document that the gateway parameter is
optional for most route subcommands.

This revision now requires review to proceed.Feb 16 2018, 4:42 PM
rstone updated this revision to Diff 39396.Feb 16 2018, 6:32 PM

Clarify manpage update as per rgrimes' suggestion

rstone added inline comments.Feb 16 2018, 6:34 PM
sbin/route/route.8
136

BTW, this is wrong. The syntax implies that "flush" is an argument accepted by the -n flag (you can check out "man route" to see how this is rendered and that it is clearly wrong). I have opened a separate review that fixes this: https://reviews.freebsd.org/D14401

rgrimes accepted this revision.Feb 16 2018, 7:13 PM
rgrimes added inline comments.
sbin/route/route.8
136

The whole man page could use some reworking. Including expanding out the various correct forms in the SYNOPSIS section and removing the scattering of syntax info.

Has anyone run this page through, um, igor from ports?

This revision is now accepted and ready to land.Feb 16 2018, 7:13 PM
rstone added inline comments.Feb 16 2018, 7:53 PM
sbin/route/route.8
136

I did run it through igor. It complains about "aggregatable", believing that it's not a real word (Google seems to disagree). Other than that the file is clean.

In D14291#300646, @ae wrote:

How it supposed to work for RADIX_MPATH case? I know RADIX_MPATH is currently broken, but anyway...

In my opinion, in any case where it's ambiguous we should fail and require the user to unambiguously specify the route.

This revision was automatically updated to reflect the committed changes.