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

[Duncan Roe <duncan_roe@optusnet.com.au>]

Hi Pablo,

I sent this email (explanation of how the system works) before I saw your email
asking for that explanation.

Then I replied to that email of yours before I saw this one.

On Wed, Apr 29, 2020 at 09:16:43PM +0200, Pablo Neira Ayuso wrote:
> On Thu, Apr 30, 2020 at 05:10:47AM +1000, Duncan Roe wrote:
> [...]
> > Sorry, I should have explained a bit more how the system would work:
> >
> > struct pkt_buff has 3 new members:
> >
> >         bool    copy_done;
> >         uint32_t extra;
> >         uint8_t *copy_buf;
> >
> > When extra > 0, pktb_alloc2 verifies that buflen is >= len + extra. It then
> > stores extra and copy_buf in pktb, ready for use by pktb_mangle() (all the other
> > manglers call this eventually).
> >
> > So that's how pktb_mangle() doesn't need to allocate a buffer.
>
> Thanks for the explaining. Given this is in userspace, it is easier if

Tiny nit - this could be userspace on an embedded device where memory is really
tight. So perhaps document with "If memory is at a premium, you really only need
len + extra" otherwise a big buf is fine.

> the user allocates the maximum packet length that is possible:
>
>         0xffff + (MNL_SOCKET_BUFFER_SIZE/2);
>
> We can probably expose this to the header so they can pre-allocate a
> buffer that is large enough and, hence, _mangle() is guaranteed to
> have always enough room to add extra bytes.

Yes I saw that expression in examples/nf-queue.c. How about

#define COPY_BUF_SIZE (0xffff + (MNL_SOCKET_BUFFER_SIZE/2))

or what other name would you like?

--- Off-topic

I'm intrigued that you ccan use MNL_SOCKET_BUFFER_SIZE when dimensioning static
variables. The expansion is

#define MNL_SOCKET_BUFFER_SIZE (sysconf(_SC_PAGESIZE) < 8192L ?
sysconf(_SC_PAGESIZE) : 8192L)

and sysconf is an actual function:

unistd.h:622:extern long int sysconf (int __name) __THROW;

If I try to dimension a static variable using pktb_head_size(), the compiler
throws an error.  Why is sysconf() ok but not pktb_head_size()?

Cheers ... Duncan.