| Message ID | 20190517175913.20629-1-dsahern@kernel.org |
|---|---|
| State | Changes Requested |
| Delegated to: | stephen hemminger |
| Headers | show |
| Series | [iproute2] ip route: Set rtm_dst_len to 32 for all ip route get requests | expand |
On Fri, 17 May 2019 10:59:13 -0700 David Ahern <dsahern@kernel.org> wrote: > From: David Ahern <dsahern@gmail.com> > > Jason reported that ip route get with a prefix length is now > failing: > $ 192.168.50.0/24 > RTNETLINK answers: Invalid argument > > iproute2 now uses strict mode and strict mode in the kernel > requires rtm_dst_len to be 32. Non-strict mode ignores the > prefix length, so this allows ip to work without affecting > existing users who add a prefix length to the request. > > Fixes: aea41afcfd6d6 ("ip bridge: Set NETLINK_GET_STRICT_CHK on socket") > Reported-by: Jason A. Donenfeld <Jason@zx2c4.com> > Signed-off-by: David Ahern <dsahern@gmail.com> > --- > ip/iproute.c | 6 +++++- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/ip/iproute.c b/ip/iproute.c > index 2b3dcc5dbd53..d980b86ffd42 100644 > --- a/ip/iproute.c > +++ b/ip/iproute.c > @@ -2035,7 +2035,11 @@ static int iproute_get(int argc, char **argv) > if (addr.bytelen) > addattr_l(&req.n, sizeof(req), > RTA_DST, &addr.data, addr.bytelen); > - req.r.rtm_dst_len = addr.bitlen; > + /* kernel ignores prefix length on 'route get' > + * requests; to allow ip to work with strict mode > + * but not break existing users, just set to 32 > + */ > + req.r.rtm_dst_len = 32; > address_found = true; > } > argc--; argv++; I don't like silently ignoring things. It was wrong before and it is trapped now. Probably better to error out in iproute2 if any prefix is given.
On Fri, 17 May 2019 10:59:13 -0700 David Ahern <dsahern@kernel.org> wrote: > index 2b3dcc5dbd53..d980b86ffd42 100644 > --- a/ip/iproute.c > +++ b/ip/iproute.c > @@ -2035,7 +2035,11 @@ static int iproute_get(int argc, char **argv) > if (addr.bytelen) > addattr_l(&req.n, sizeof(req), > RTA_DST, &addr.data, addr.bytelen); > - req.r.rtm_dst_len = addr.bitlen; > + /* kernel ignores prefix length on 'route get' > + * requests; to allow ip to work with strict mode > + * but not break existing users, just set to 32 > + */ > + req.r.rtm_dst_len = 32; > address_found = true; > } > argc--; argv++; Why not warn user that any prefix length (ie not 32) is ignored, then do what you propose.
On 5/22/19 5:16 PM, Stephen Hemminger wrote: > On Fri, 17 May 2019 10:59:13 -0700 > David Ahern <dsahern@kernel.org> wrote: > >> index 2b3dcc5dbd53..d980b86ffd42 100644 >> --- a/ip/iproute.c >> +++ b/ip/iproute.c >> @@ -2035,7 +2035,11 @@ static int iproute_get(int argc, char **argv) >> if (addr.bytelen) >> addattr_l(&req.n, sizeof(req), >> RTA_DST, &addr.data, addr.bytelen); >> - req.r.rtm_dst_len = addr.bitlen; >> + /* kernel ignores prefix length on 'route get' >> + * requests; to allow ip to work with strict mode >> + * but not break existing users, just set to 32 >> + */ >> + req.r.rtm_dst_len = 32; >> address_found = true; >> } >> argc--; argv++; > > Why not warn user that any prefix length (ie not 32) is ignored, > then do what you propose. > My first version did that. I thought people would complain about a stderr message. At least with failing the 'route get' scripts can be fixed. After more thought even changing the prefix length for users presents a limitation on future changes.
diff --git a/ip/iproute.c b/ip/iproute.c index 2b3dcc5dbd53..d980b86ffd42 100644 --- a/ip/iproute.c +++ b/ip/iproute.c @@ -2035,7 +2035,11 @@ static int iproute_get(int argc, char **argv) if (addr.bytelen) addattr_l(&req.n, sizeof(req), RTA_DST, &addr.data, addr.bytelen); - req.r.rtm_dst_len = addr.bitlen; + /* kernel ignores prefix length on 'route get' + * requests; to allow ip to work with strict mode + * but not break existing users, just set to 32 + */ + req.r.rtm_dst_len = 32; address_found = true; } argc--; argv++;