Re: [PATCH net-next] rtnetlink: add RTNH_REJECT_MASK

On Tue, Nov 30, 2021 at 11:35:17AM +0300, Alexander Mikhalitsyn wrote:

On Tue, 30 Nov 2021 09:59:25 +0200
Ido Schimmel [off-list ref] wrote:

quoted

Looking at the patch again, what is the motivation to expose
RTNH_REJECT_MASK to user space? iproute2 already knows that it only
makes sense to set RTNH_F_ONLINK. Can't we just do:

Sorry, but that's not fully clear for me, why we should exclude RTNH_F_ONLINK?
I thought that we should exclude RTNH_F_DEAD and RTNH_F_LINKDOWN just because
kernel doesn't allow to set these flags.

I don't think we should exclude RTNH_F_ONLINK. I'm saying that it is the
only flag that it makes sense to send to the kernel in the ancillary
header of RTM_NEWROUTE messages. The rest of the RNTH_F_* flags are
either not used by the kernel or are only meant to be sent from the
kernel to user space. Due to omission, they are mistakenly allowed.

Therefore, I think that the only necessary patch is an iproute2 patch
that makes sure that during save/restore you are clearing all the
RTNH_F_* flags but RTNH_F_ONLINK.

BTW, looking at save_route() in iproute2, I think the patch only clears
these flags from the ancillary header, but not from 'struct rtnexthop'
that is nested in RTA_MULTIPATH for multipath routes. See this blog post
for depiction of the message:
http://codecave.cc/multipath-routing-in-linux-part-1.html

I'd also thought about another approach - "offload" this flags filtering
problems to the kernel side for better iproute dump images compatibility.

Now we dump all routes using netlink message like this
	struct {
		struct nlmsghdr nlh;
		struct rtmsg rtm;
		char buf[128];
	} req = {
		.nlh.nlmsg_len = NLMSG_LENGTH(sizeof(struct rtmsg)),
		.nlh.nlmsg_type = RTM_GETROUTE,
		.nlh.nlmsg_flags = NLM_F_DUMP | NLM_F_REQUEST,
...
	};

But we can introduce some "special" flag like NLM_F_FILTERED_DUMP (or something like that)
	} req = {
		.nlh.nlmsg_len = NLMSG_LENGTH(sizeof(struct rtmsg)),
		.nlh.nlmsg_type = RTM_GETROUTE,
		.nlh.nlmsg_flags = NLM_F_FILTERED_DUMP | NLM_F_REQUEST,
...
	};

The idea here is that the kernel nows better which flags should be omitted from the dump
(<=> which flags is prohibited to set directly from the userspace side).

But that change is more "global". WDYT about this?

I'm ready to implement any of the approaches with your kind advice.

Having the kernel filter RO flags upon RTM_GETROUTE with a new special
flag / attribute would be easiest to implement in iproute2 (especially
if my comment about RTA_MULTIPATH is correct), but it's a quite invasive
change that requires new uAPI.

Personally, I think that if something can be done in user space, then I
would do it in user space instead of adding new uAPI.