Hi Phil, On Thu, Aug 27, 2020 at 04:23:19PM +0200, Phil Sutter wrote: > Hi Pablo, > > On Wed, Aug 26, 2020 at 05:32:19PM +0200, Pablo Neira Ayuso wrote: > [...] > > There several possibilities, just a few that can be explored: [...] > > Note that userspace requires no changes to support batching mode: > > libmnl's mnl_cb_run() keeps iterating over the buffer that was > > received until all netlink messages are consumed. > > > > The quick patch is incomplete, I just want to prove the saving in > > terms of memory. I'll give it another spin and submit this for > > review. > > Highly appreciated. Put me in Cc and I'll stress-test it a bit. Let's give a try to this patch: https://patchwork.ozlabs.org/project/netfilter-devel/patch/20200827172842.24478-1-pablo@xxxxxxxxxxxxx/ > > * Probably recover the cookie idea: firewalld specifies a cookie > > that identifies the rule from userspace, so there is no need for > > NLM_F_ECHO. Eric mentioned he would like to delete rules by cookie, > > this should be possible by looking up for the handle from the > > cookie to delete rules. The cookie is stored in NFTA_RULE_USERDATA > > so this is only meaningful to userspace. No kernel changes are > > required (this is supported for a bit of time already). > > > > Note: The cookie could be duplicated. Since the cookie is allocated > > by userspace, it's up to userspace to ensure uniqueness. In case > > cookies are duplicated, if you delete a rule by cookie then > > > > Rule deletion by cookie requires dumping the whole ruleset though > > (slow). However, I think firewalld keeps a rule cache, so it uses > > the rule handle to delete the rule instead (no need to dump the > > cache then). Therefore, the cookie is only used to fetch the rule > > handle. > > > > With the rule cookie in place, firewalld can send the batch, then > > make a NLM_F_DUMP request after sending the batch to retrieve the > > handle from the cookie instead of using NLM_F_ECHO. The batch send + > > NLM_F_DUMP barely consumes 16KBytes per recvmsg() call. The send + > > dump is not atomic though. > > > > Because NLM_F_ECHO is only used to get back the rule handle, right? > > The discussion that led to NLM_F_ECHO was to support atomic rule handle > retrieval. A user-defined "pseudo-handle" obviously can't uphold that > promise and therefore won't be a full replacement. > > Apart from that, JSON echo output in it's current form is useful for > scripts to retrieve the handle. We've had that discussion already, I > pointed out they could just do 'input = output' and know > 'input["nftables"][5]["add"]["rule"]["expr"][0]["match"]["right"]' is > still present and has the expected value. > > I recently found out that firewalld (e.g.) doesn't even do that, but > instead manually iterates over the list of commands it got back and > extracts handle values. > > Doing that without NLM_F_ECHO but with cookies instead means to add > some rules, then list ruleset and search for one's cookies. > > That aside, if nftables doesn't support cookies beyond keeping them in > place, why not just use a custom comment instead? That's even > backwards-compatible. Something else to improve netlink socket receiver usage: Userspace might also set on NLM_F_ECHO for rules only, so the kernel only consumes the netlink receive socket buffer for these reports. Although not sure how to expose this yet through the library in a nice way... Probably it should be possible to extend netlink to filter out attributes that do not need to be reported back to userspace via NLM_F_ECHO... The main gain is to amortize skbuff cost, ie. increasing the batching factor, but going over NLMSG_GOODSIZE is tricky. We have to update userspace (provide larger buffer) and revisit if this is possible without breaking backward compatibility. BTW, if NLM_F_ECHO results in ENOSPC, the ruleset has been applied, it's just that the socket buffer got full, a fallback to NLM_F_DUMP is probably an option in that case? But I understand the goal is to not hit ENOSPC. Let me know if my patch helps mitigate the problem, thanks.
