On Fri, Dec 6, 2024 at 3:49 AM Dmitry Safonov 0x7f454c46@gmail.com wrote:
Hi Jakub, Eric,
On Thu, 5 Dec 2024 at 09:09, Eric Dumazet edumazet@google.com wrote:
On Thu, Dec 5, 2024 at 2:13 AM Jakub Kicinski kuba@kernel.org wrote:
Hi Eric!
This was posted while you were away -- any thoughts or recommendation on how to address the required nl message size changing? Or other problems pointed out by Dmitry? My suggestion in the subthread is to re-dump with a fixed, large buffer on EMSGSIZE, but that's not super clean..
Hi Jakub
inet_diag_dump_one_icsk() could retry, doubling the size until the ~32768 byte limit is reached ?
Also, we could make sure inet_sk_attr_size() returns at least NLMSG_DEFAULT_SIZE, there is no point trying to save memory for a single skb in inet_diag_dump_one_icsk().
Starting from NLMSG_DEFAULT_SIZE sounds like a really sane idea! :-)
There is a consensus for this one, I will cook a patch with this part only.
[..]
@@ -585,8 +589,14 @@ int inet_diag_dump_one_icsk(struct inet_hashinfo *hashinfo,
err = sk_diag_fill(sk, rep, cb, req, 0, net_admin); if (err < 0) {
WARN_ON(err == -EMSGSIZE); nlmsg_free(rep);
if (err == -EMSGSIZE) {attr_size <<= 1;if (attr_size + NLMSG_HDRLEN <=SKB_WITH_OVERHEAD(32768)) {
cond_resched();goto retry;}} goto out; } err = nlmsg_unicast(net->diag_nlsk, rep, NETLINK_CB(in_skb).portid);To my personal taste on larger than 327 md5 keys scale, I'd prefer to see "dump may be inconsistent, retry if you need consistency" than -EMSGSIZE fail, yet userspace potentially may use the errno as a "retry" signal.
I do not yet understand this point. I will let you send a patch for further discussion.
Thanks.