On Fri, May 31, 2019 at 02:11:41PM -0400, Eric Garver wrote:
> On Thu, May 30, 2019 at 12:55:27PM +0200, Pablo Neira Ayuso wrote:
> > Set a receiver buffer size based on the number of commands and the
> > average message size, this is useful for the --echo option in order to
> > avoid ENOBUFS errors.
> >
> > Double the estimated size is used to ensure enough receiver buffer
> > space.
> >
> > Skip buffer receiver logic if estimation is smaller than current buffer.
> >
> > Reported-by: Phil Sutter <phil@xxxxxx>
> > Signed-off-by: Pablo Neira Ayuso <pablo@xxxxxxxxxxxxx>
> > ---
> [..]
> > diff --git a/src/libnftables.c b/src/libnftables.c
> > index 199dbc97b801..a58b8ca9dcf6 100644
> > --- a/src/libnftables.c
> > +++ b/src/libnftables.c
> [..]
> > @@ -308,14 +310,17 @@ int mnl_batch_talk(struct netlink_ctx *ctx, struct list_head *err_list)
> > .tv_sec = 0,
> > .tv_usec = 0
> > };
> > - fd_set readfds;
> > struct iovec iov[iov_len];
> > struct msghdr msg = {};
> > + fd_set readfds;
> > int err = 0;
> >
> > mnl_set_sndbuffer(ctx->nft->nf_sock, ctx->batch);
> >
> > - mnl_nft_batch_to_msg(ctx, &msg, &snl, iov, iov_len);
> > + batch_size = mnl_nft_batch_to_msg(ctx, &msg, &snl, iov, iov_len);
> > + avg_msg_size = div_round_up(batch_size, num_cmds);
> > +
> > + mnl_set_rcvbuffer(ctx->nft->nf_sock, num_cmds * avg_msg_size * 2);
>
> I think this calculation is incorrect.
Yes, see v4 of this patch:
https://patchwork.ozlabs.org/patch/1107737/
> I'm still getting ENOBUFS with Phil's testcase and firewalld's
> testsuite (large json blob). I changed the multiplier from 2 to 6
> and it worked.
I just pushed out the patchset, the last version is using a multiplier
of 4, I modified Phil's testcase to 100000 and it works fine. Please
try the version upstream and let me know.
We can enhance this code by checking for ENOBUFS in _sendmsg(), extend
the buffer size and retry. Then, we also need to update kernel code to
abort the transaction in case NLM_F_ECHO flag is set on and we hit
ENOBUFS.
