On Fri, 23 Aug 2019 17:11:38 -0700
John Fastabend <john.fastab...@gmail.com> wrote:
> An excerpt from netlink(7) man page,
>
> In multipart messages (multiple nlmsghdr headers with associated payload
> in one byte stream) the first and all following headers have the
> NLM_F_MULTI flag set, except for the last header which has the type
> NLMSG_DONE.
>
> but, after (ee28906) there is a missing NLM_F_MULTI flag in the middle of a
> FIB dump.
In your case (see below), it can be zero or more, depending on how many
exception routes you have.
> The result is user space applications following above man page
> excerpt may get confused and may stop parsing msg believing something went
> wrong.
Worse yet, also RFC 3459 says:
[...] For multipart
messages, the first and all following headers have the NLM_F_MULTI
Netlink header flag set, except for the last header which has the
Netlink header type NLMSG_DONE.
But iproute2 doesn't check for this, so the selftests I added didn't
notice. Thanks for fixing this!
> In the golang netlink lib [0] the library logic stops parsing believing the
> message is not a multipart message. Found this running Cilium[1] against
> net-next while adding a feature to auto-detect routes. I noticed with
> multiple route tables we no longer could detect the default routes on net
> tree kernels because the library logic was not returning them.
However, note that if strict netlink checking is requested (I think the
library should be updated), and RTM_F_CLONED is not set (which should
be the case if you are just looking for "regular" routes), you won't
hit this.
> Fix this by handling the fib_dump_info_fnhe() case the same way the
> fib_dump_info() handles it by passing the flags argument through the
> call chain and adding a flags argument to rt_fill_info().
>
> Tested with Cilium stack and auto-detection of routes works again. Also
> annotated libs to dump netlink msgs and inspected NLM_F_MULTI and
> NLMSG_DONE flags look correct after this.
>
> Note: In inet_rtm_getroute() pass rt_fill_info() '0' for flags the same
> as is done for fib_dump_info() so this looks correct to me.
Yes, that's correct, because if the buffer is too small for a single
route dumped by a single rt_fill_info() call, we'll just fail, so that
will never be a multipart message.
> [0] https://github.com/vishvananda/netlink/
> [1] https://github.com/cilium/
>
> Fixes: ee28906fd7a14 ("ipv4: Dump route exceptions if requested")
> Signed-off-by: John Fastabend <john.fastab...@gmail.com>
Reviewed-by: Stefano Brivio <sbri...@redhat.com>
--
Stefano
