Commit 5b4b62a1 authored by Jakub Kicinski's avatar Jakub Kicinski Committed by David S. Miller
Browse files

rtnetlink: make the "split" NLM_DONE handling generic 



Jaroslav reports Dell's OMSA Systems Management Data Engine
expects NLM_DONE in a separate recvmsg(), both for rtnl_dump_ifinfo()
and inet_dump_ifaddr(). We already added a similar fix previously in
commit 460b0d33 ("inet: bring NLM_DONE out to a separate recv() again")

Instead of modifying all the dump handlers, and making them look
different than modern for_each_netdev_dump()-based dump handlers -
put the workaround in rtnetlink code. This will also help us move
the custom rtnl-locking from af_netlink in the future (in net-next).

Note that this change is not touching rtnl_dump_all(). rtnl_dump_all()
is different kettle of fish and a potential problem. We now mix families
in a single recvmsg(), but NLM_DONE is not coalesced.

Tested:

  ./cli.py --dbg-small-recv 4096 --spec netlink/specs/rt_addr.yaml \
           --dump getaddr --json '{"ifa-family": 2}'

  ./cli.py --dbg-small-recv 4096 --spec netlink/specs/rt_route.yaml \
           --dump getroute --json '{"rtm-family": 2}'

  ./cli.py --dbg-small-recv 4096 --spec netlink/specs/rt_link.yaml \
           --dump getlink

Fixes: 3e41af90 ("rtnetlink: use xarray iterator to implement rtnl_dump_ifinfo()")
Fixes: cdb2f80f ("inet: use xa_array iterator to implement inet_dump_ifaddr()")
Reported-by: default avatarJaroslav Pulchart <jaroslav.pulchart@gooddata.com>
Link: https://lore.kernel.org/all/CAK8fFZ7MKoFSEzMBDAOjoUt+vTZRRQgLDNXEOfdCCXSoXXKE0g@mail.gmail.com


Signed-off-by: default avatarJakub Kicinski <kuba@kernel.org>
Reviewed-by: default avatarEric Dumazet <edumazet@google.com>
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
parent e137596e

include/net/rtnetlink.h

+1 −0
Original line number Diff line number Diff line
@@ -13,6 +13,7 @@ enum rtnl_link_flags { 
	RTNL_FLAG_DOIT_UNLOCKED		= BIT(0),

	RTNL_FLAG_BULK_DEL_SUPPORTED	= BIT(1),

	RTNL_FLAG_DUMP_UNLOCKED		= BIT(2),

	RTNL_FLAG_DUMP_SPLIT_NLM_DONE	= BIT(3),	/* legacy behavior */

};



enum rtnl_kinds {

net/core/rtnetlink.c

+42 −2
Original line number Diff line number Diff line
@@ -6484,6 +6484,46 @@ static int rtnl_mdb_del(struct sk_buff *skb, struct nlmsghdr *nlh, 


/* Process one rtnetlink message. */



static int rtnl_dumpit(struct sk_buff *skb, struct netlink_callback *cb)

{

	rtnl_dumpit_func dumpit = cb->data;

	int err;



	/* Previous iteration have already finished, avoid calling->dumpit()

	 * again, it may not expect to be called after it reached the end.

	 */

	if (!dumpit)

		return 0;



	err = dumpit(skb, cb);



	/* Old dump handlers used to send NLM_DONE as in a separate recvmsg().

	 * Some applications which parse netlink manually depend on this.

	 */

	if (cb->flags & RTNL_FLAG_DUMP_SPLIT_NLM_DONE) {

		if (err < 0 && err != -EMSGSIZE)

			return err;

		if (!err)

			cb->data = NULL;



		return skb->len;

	}

	return err;

}



static int rtnetlink_dump_start(struct sock *ssk, struct sk_buff *skb,

				const struct nlmsghdr *nlh,

				struct netlink_dump_control *control)

{

	if (control->flags & RTNL_FLAG_DUMP_SPLIT_NLM_DONE) {

		WARN_ON(control->data);

		control->data = control->dump;

		control->dump = rtnl_dumpit;

	}



	return netlink_dump_start(ssk, skb, nlh, control);

}



static int rtnetlink_rcv_msg(struct sk_buff *skb, struct nlmsghdr *nlh,

			     struct netlink_ext_ack *extack)

{
@@ -6548,7 +6588,7 @@ static int rtnetlink_rcv_msg(struct sk_buff *skb, struct nlmsghdr *nlh, 
				.module		= owner,

				.flags		= flags,

			};

			err = netlink_dump_start(rtnl, skb, nlh, &c);

			err = rtnetlink_dump_start(rtnl, skb, nlh, &c);

			/* netlink_dump_start() will keep a reference on

			 * module if dump is still in progress.

			 */
@@ -6694,7 +6734,7 @@ void __init rtnetlink_init(void) 
	register_netdevice_notifier(&rtnetlink_dev_notifier);



	rtnl_register(PF_UNSPEC, RTM_GETLINK, rtnl_getlink,

		      rtnl_dump_ifinfo, 0);

		      rtnl_dump_ifinfo, RTNL_FLAG_DUMP_SPLIT_NLM_DONE);

	rtnl_register(PF_UNSPEC, RTM_SETLINK, rtnl_setlink, NULL, 0);

	rtnl_register(PF_UNSPEC, RTM_NEWLINK, rtnl_newlink, NULL, 0);

	rtnl_register(PF_UNSPEC, RTM_DELLINK, rtnl_dellink, NULL, 0);

net/ipv4/devinet.c

+1 −1
Original line number Diff line number Diff line
@@ -2805,7 +2805,7 @@ void __init devinet_init(void) 
	rtnl_register(PF_INET, RTM_NEWADDR, inet_rtm_newaddr, NULL, 0);

	rtnl_register(PF_INET, RTM_DELADDR, inet_rtm_deladdr, NULL, 0);

	rtnl_register(PF_INET, RTM_GETADDR, NULL, inet_dump_ifaddr,

		      RTNL_FLAG_DUMP_UNLOCKED);

		      RTNL_FLAG_DUMP_UNLOCKED | RTNL_FLAG_DUMP_SPLIT_NLM_DONE);

	rtnl_register(PF_INET, RTM_GETNETCONF, inet_netconf_get_devconf,

		      inet_netconf_dump_devconf,

		      RTNL_FLAG_DOIT_UNLOCKED | RTNL_FLAG_DUMP_UNLOCKED);

net/ipv4/fib_frontend.c

+1 −6
Original line number Diff line number Diff line
@@ -1050,11 +1050,6 @@ static int inet_dump_fib(struct sk_buff *skb, struct netlink_callback *cb) 
			e++;

		}

	}



	/* Don't let NLM_DONE coalesce into a message, even if it could.

	 * Some user space expects NLM_DONE in a separate recv().

	 */

	err = skb->len;

out:



	cb->args[1] = e;
@@ -1665,5 +1660,5 @@ void __init ip_fib_init(void) 
	rtnl_register(PF_INET, RTM_NEWROUTE, inet_rtm_newroute, NULL, 0);

	rtnl_register(PF_INET, RTM_DELROUTE, inet_rtm_delroute, NULL, 0);

	rtnl_register(PF_INET, RTM_GETROUTE, NULL, inet_dump_fib,

		      RTNL_FLAG_DUMP_UNLOCKED);

		      RTNL_FLAG_DUMP_UNLOCKED | RTNL_FLAG_DUMP_SPLIT_NLM_DONE);

}