Re: [PATCH v6 2/2] netfilter: nfnetlink_queue: optimize verdict lookup with hash table

[Florian Westphal <fw@strlen.de>]

Scott Mitchell <scott.k.mitch1@gmail.com> wrote:
> For NFQA_CFG_QUEUE_MAXLEN API translation there are a few challenges:
> 1. Max packet size - If GRO is enabled, the MTU may not be a reliable
> upper bound. Using 2mb would be a conservative approach but also
> overcommit memory in many cases. Since there is no per-byte limit
> today it is likely safest to go with the conservative approach for
> backwards compatibility.
> 2. Per queue limit vs pernet limit - The number of queues and
> NFQA_CFG_QUEUE_MAXLEN are dynamic. How would you derive a pernet
> limit? One approach is "number of queues * queue with the max
> NFQA_CFG_QUEUE_MAXLEN" (which requires some additional state
> tracking).

I don't think a per queue limit was ever a good idea.

Back then network namespaces did not exist and nfqueue needs root
privileges, so misconfiguration was always self-sabotage.

But thats not true anymore.  I think we can keep a per queue limit,
if just to allow userspace to limit some queues more than others.

But to keep memory usage at sane levels we'll need some pernet
limit (pcpu counters?), counting based on skb->truesize.

We could adopt a low limit, say, 32 Mbyte, by default and add
nfnetlink options to increase this. (The default 1024 packet
queue length would use ~2mbyte, assuming 2k pages and no
packet aggregation of any kind).

Maybe we can precharge this to the requesting sockets memcg as well
to also prevent netns from configureing a 1 TB pernet limit.

> For the pernet byte limit API, were you thinking sysctl similar to
> nf_conntrack_max (e.g., /proc/sys/net/netfilter/nfqueue_max_bytes)?

Thats another option,  My first hunch was to extend nfqnl_attr_config
enum, as that api already has to be used to configure the queues from
userland.