Re: [PATCH libnetfilter_queue 0/3] pktbuff API updates

[Pablo Neira Ayuso <pablo@netfilter.org>]

On Thu, Apr 30, 2020 at 05:54:14AM +1000, Duncan Roe wrote:
> On Wed, Apr 29, 2020 at 09:00:20PM +0200, Pablo Neira Ayuso wrote:
> > On Wed, Apr 29, 2020 at 11:28:40PM +1000, Duncan Roe wrote:
> > > On Wed, Apr 29, 2020 at 12:55:20AM +0200, Pablo Neira Ayuso wrote:
> > > > On Wed, Apr 29, 2020 at 07:14:52AM +1000, Duncan Roe wrote:
> > > > > On Tue, Apr 28, 2020 at 12:34:07PM +0200, Pablo Neira Ayuso wrote:
> > > > [...]
> > > I think we should not be usurping the data pointer of mnl_cb_run().
> > > I can see people wanting to use it to pass a pointer to e.g. some
> > > kind of database that callbacks need to access. There's no
> > > performance gain to recycling the buffer: the CB doesn't need to
> > > call pktb_head_size() on every invocation, that can be done once by
> > > main() e.g.
> > >
> > >  static size_t sizeof_head;
> > >  ...
> > >  int main(int argc, char *argv[])
> > >  {
> > >  ...
> > >          sizeof_head = pktb_head_size(); /* Avoid multiple calls in CB */
> > >  ...
> > >  static int queue_cb(const struct nlmsghdr *nlh, void *data)
> > >  {
> > >          char head[sizeof_head];
> >
> > You might also declare the pre-allocated pkt_buff as a global if you
> > don't want to use the data pointer in mnl_cb_run().
> 
> I'm uneasy about this. We're writing a library here. We shouldn't be dictating
> to the user that he must declare globals. "static" won't do in a multi-threaded
> program, but you could use "thread local" (malloc'd under the covers, (tiny)
> performance hit c/w stack (which is always thread local)).
> 
> "The road to bloat is paved with tiny performance hits" [1]

In nf_queue, the way to go is to set the _GSO flag on the client and
then set on NFT_QUEUE_FLAG_CPU_FANOUT from the nft queue rule. If you
need multiple processes, then users pre-allocate one pkt_buff per
process.