Received-SPF: pass (google.com: best guess record for domain of linux-wireless-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Message-ID: <34a0b0652403edcb630f65a240a30dfddb82950c.camel@sipsolutions.net>
Subject: Re: [RFCv2 2/4] nl80211: Support >4096 byte NEW_WIPHY event nlmsg
From:   Johannes Berg <johannes@sipsolutions.net>
To:     Denis Kenzior <denkenz@gmail.com>, linux-wireless@vger.kernel.org
Date:   Fri, 30 Aug 2019 11:36:35 +0200
In-Reply-To: <20190816192703.12445-2-denkenz@gmail.com> (sfid-20190816_212730_190469_72603E51)
References: <20190816192703.12445-1-denkenz@gmail.com>
         <20190816192703.12445-2-denkenz@gmail.com>
         (sfid-20190816_212730_190469_72603E51)
Content-Type: text/plain; charset="UTF-8"
User-Agent: Evolution 3.30.5 (3.30.5-1.fc29) 
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Sender: linux-wireless-owner@vger.kernel.org
Precedence: bulk

On Fri, 2019-08-16 at 14:27 -0500, Denis Kenzior wrote:
> For historical reasons, NEW_WIPHY messages generated by dumps or
> GET_WIPHY commands were limited to 4096 bytes due to userspace tools
> using limited buffers.

I think now that I've figured out why, it'd be good to note that it
wasn't due to userspace tools, but rather due to the default netlink
dump skb allocation at the time, prior to commit  9063e21fb026
("netlink: autosize skb lengthes").

> Once the sizes NEW_WIPHY messages exceeded these
> sizes, split dumps were introduced.  All any non-legacy data was added
> only to messages using split-dumps (including filtered dumps).
> 
> However, split-dumping has quite a significant overhead.  On cards
> tested, split dumps generated message sizes 1.7-1.8x compared to
> non-split dumps, while still comfortably fitting into an 8k buffer.  The
> kernel now expects userspace to provide 16k buffers by default, and 32k
> buffers are possible.
> 
> Introduce a concept of a large message, so that if the kernel detects
> that userspace has provided a buffer of sufficient size, a non-split
> message could be generated.

So, there's still a wrinkle with this. Larger SKB allocation can fail,
and instead of returning an error to userspace, the kernel will allocate
a smaller SKB instead.

With genetlink, we currently don't even have a way of controlling the
minimum allocation that's always required.

Since we already have basically all of the mechanics, I'd say perhaps a
better concept would be to "split when necessary", aborting if split
isn't supported.

IOW, do something like

... nl80211_send_wiphy(...)
{
[...]

switch (state->split_start) {
[...]
case <N>:
	[...] // put stuff
	state->split_start++;
	state->skb_end = nlmsg_get_pos(skb);
	/* fall through */
case <N+1>:
[...]
}

finish:
	genlmsg_end(msg, hdr);
	return 0;
nla_put_failure:
	if (state->split_start < 9) {
		genlmsg_cancel(msg, hdr);
		return -EMSGSIZE;
	}
	nlmsg_trim(msg, state->skb_end);
	goto finish;
}


That way, we fill each SKB as much as possible, up to 32k if userspace
provided big enough buffers *and* we could allocate the SKB.


Your userspace would still set the split flag, and thus be compatible
with all kinds of options:
 * really old kernel not supporting split
 * older kernel sending many messages
 * kernel after this change packing more into one message
 * even if allocating big SKBs failed

johannes