Allow route change requests to not specify the gateway.
ClosedPublic
Actions

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

Details

Reviewers

eugen_grosbein.net
rgrimes

Group Reviewers

manpages

Commits

rS329743: Allow route change requests to not specify the gateway.

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 Passed

Unit

No Test Coverage

Build Status

Buildable 14917
Build 15029: arc lint + arc unit

Event Timeline

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

Herald added a subscriber: ae. · View Herald TranscriptFeb 10 2018, 1:52 AM

Harbormaster completed remote builds in B14917: Diff 39126.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.

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.

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

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

Harbormaster completed remote builds in B15083: Diff 39386.Feb 16 2018, 4:42 PM

Clarify manpage update as per rgrimes' suggestion

Harbormaster completed remote builds in B15090: Diff 39396.Feb 16 2018, 6:32 PM

rstone added inline comments.Feb 16 2018, 6:34 PM

sbin/route/route.8
136 ↗	(On Diff #39396)	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 ↗	(On Diff #39396)	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 ↗	(On Diff #39396)	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.