Page MenuHomeFreeBSD
Differential D14291

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

Authored by rstone on Feb 10 2018, 1:52 AM.
Tags
None
Referenced Files
Unknown Object (File)
Mon, Apr 7, 3:44 AM
Unknown Object (File)
Sun, Mar 30, 1:44 PM
Unknown Object (File)
Tue, Mar 25, 12:11 PM
Unknown Object (File)
Wed, Mar 12, 3:14 PM
Unknown Object (File)
Feb 20 2025, 1:41 AM
Unknown Object (File)
Feb 17 2025, 1:50 AM
Unknown Object (File)
Jan 12 2025, 4:36 AM
Unknown Object (File)
Dec 23 2024, 6:10 PM
View All 74 Files
Subscribers
ae
eugen_grosbein.net
imp
• karels
rgrimes

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 15090
Build 15183: 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
eugen_grosbein.net accepted this revision.Feb 10 2018, 11:49 AM
eugen_grosbein.net added a subscriber: eugen_grosbein.net.

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
rgrimes added a subscriber: rgrimes.Feb 10 2018, 4:29 PM

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

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

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.

rstone added a comment.Feb 16 2018, 7:59 PM
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.

Closed by commit rS329743: Allow route change requests to not specify the gateway. (authored by rstone). · Explain WhyFeb 21 2018, 7:13 PM
This revision was automatically updated to reflect the committed changes.
Herald added a reviewer: manpages. · View Herald TranscriptFeb 21 2018, 7:13 PM
Herald added a subscriber: imp. · View Herald Transcript

Revision Contents
Changeset List

PathSize
sbin/
route/
18 lines
sys/
net/
10 lines

Diff 39396

View Options

sbin/route/route.8

Show All 22 Lines
.\" HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT .\" HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
.\" LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY .\" LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY
.\" OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF .\" OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
.\" SUCH DAMAGE. .\" SUCH DAMAGE.
.\" .\"
.\" @(#)route.8 8.3 (Berkeley) 3/19/94 .\" @(#)route.8 8.3 (Berkeley) 3/19/94
.\" $FreeBSD$ .\" $FreeBSD$
.\" .\"
.Dd November 11, 2014 .Dd February 16, 2018
.Dt ROUTE 8 .Dt ROUTE 8
.Os .Os
.Sh NAME .Sh NAME
.Nm route .Nm route
.Nd manually manipulate the routing tables .Nd manually manipulate the routing tables
.Sh SYNOPSIS .Sh SYNOPSIS
.Nm .Nm
.Op Fl dnqtv .Op Fl dnqtv
▲ Show 20 Lines • Show All 88 Lines • ▼ Show 20 Lines
.Op Fl n .Op Fl n
.Cm monitor Op Fl fib Ar number .Cm monitor Op Fl fib Ar number
.Ed .Ed
.Pp .Pp
The flush command has the syntax: The flush command has the syntax:
.Pp .Pp
.Bd -ragged -offset indent -compact .Bd -ragged -offset indent -compact
.Nm .Nm
.Oo Fl n Cm flush Oc Oo Ar family Oc Op Fl fib Ar number .Oo Fl n Cm flush Oc Oo Ar family Oc Op Fl fib Ar number
rstoneAuthorUnsubmitted
Not Done
Inline Actions

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

rstone: BTW, this is wrong. The syntax implies that "flush" is an argument accepted by the -n flag…
rgrimesUnsubmitted
Not Done
Inline Actions

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?

rgrimes: The whole man page could use some reworking. Including expanding out the various correct…
rstoneAuthorUnsubmitted
Not Done
Inline Actions

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.

rstone: I did run it through igor. It complains about "aggregatable", believing that it's not a real…
.Ed .Ed
.Pp .Pp
If the If the
.Cm flush .Cm flush
command is specified, command is specified,
.Nm .Nm
will ``flush'' the routing tables of all gateway entries. will ``flush'' the routing tables of all gateway entries.
When the address family may is specified by any of the When the address family may is specified by any of the
Show All 14 Lines
.Fl inet6 .Fl inet6
modifiers. modifiers.
When a When a
.Fl fib .Fl fib
option is specified, the operation will be applied to option is specified, the operation will be applied to
the specified FIB the specified FIB
.Pq routing table . .Pq routing table .
.Pp .Pp
The other commands have the following syntax: The add command has the following syntax:
.Pp .Pp
.Bd -ragged -offset indent -compact .Bd -ragged -offset indent -compact
.Nm .Nm
.Op Fl n .Op Fl n
.Ar command .Cm add
.Op Fl net No \&| Fl host .Op Fl net No \&| Fl host
.Ar destination gateway .Ar destination gateway
.Op Ar netmask .Op Ar netmask
.Op Fl fib Ar number
.Ed
.Pp
and the other commands have the following syntax:
.Pp
.Bd -ragged -offset indent -compact
.Nm
.Op Fl n
.Ar command
.Op Fl net No \&| Fl host
.Ar destination
.Op Ar gateway Op Ar netmask
.Op Fl fib Ar number .Op Fl fib Ar number
.Ed .Ed
.Pp .Pp
where where
.Ar destination .Ar destination
is the destination host or network, is the destination host or network,
.Ar gateway .Ar gateway
is the next-hop intermediary via which packets should be routed. is the next-hop intermediary via which packets should be routed.
▲ Show 20 Lines • Show All 323 Lines • Show Last 20 Lines
View Options

sys/net/rtsock.c

Show First 20 Lines • Show All 668 Lines • ▼ Show 20 Lines if (info.rti_info[RTAX_GATEWAY] != NULL &&
} }
} }
switch (rtm->rtm_type) { switch (rtm->rtm_type) {
struct rtentry *saved_nrt; struct rtentry *saved_nrt;
case RTM_ADD: case RTM_ADD:
case RTM_CHANGE: case RTM_CHANGE:
if (rtm->rtm_type == RTM_ADD) {
if (info.rti_info[RTAX_GATEWAY] == NULL) if (info.rti_info[RTAX_GATEWAY] == NULL)
senderr(EINVAL); senderr(EINVAL);
}
saved_nrt = NULL; saved_nrt = NULL;
/* support for new ARP code */ /* support for new ARP code */
if (info.rti_info[RTAX_GATEWAY]->sa_family == AF_LINK && if (info.rti_info[RTAX_GATEWAY] != NULL &&
info.rti_info[RTAX_GATEWAY]->sa_family == AF_LINK &&
(rtm->rtm_flags & RTF_LLDATA) != 0) { (rtm->rtm_flags & RTF_LLDATA) != 0) {
error = lla_rt_output(rtm, &info); error = lla_rt_output(rtm, &info);
#ifdef INET6 #ifdef INET6
if (error == 0) if (error == 0)
rti_need_deembed = (V_deembed_scopeid) ? 1 : 0; rti_need_deembed = (V_deembed_scopeid) ? 1 : 0;
#endif #endif
break; break;
} }
error = rtrequest1_fib(rtm->rtm_type, &info, &saved_nrt, error = rtrequest1_fib(rtm->rtm_type, &info, &saved_nrt,
fibnum); fibnum);
if (error == 0 && saved_nrt != NULL) { if (error == 0 && saved_nrt != NULL) {
#ifdef INET6 #ifdef INET6
rti_need_deembed = (V_deembed_scopeid) ? 1 : 0; rti_need_deembed = (V_deembed_scopeid) ? 1 : 0;
#endif #endif
RT_LOCK(saved_nrt); RT_LOCK(saved_nrt);
rtm->rtm_index = saved_nrt->rt_ifp->if_index; rtm->rtm_index = saved_nrt->rt_ifp->if_index;
▲ Show 20 Lines • Show All 1,269 Lines • Show Last 20 Lines