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

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

Hi Duncan,

On Mon, Apr 27, 2020 at 09:06:14PM +1000, Duncan Roe wrote:
> Hi Pablo,
> 
> On Sun, Apr 26, 2020 at 03:23:53PM +0200, Pablo Neira Ayuso wrote:
> > Hi Duncan,
> >
> > This is another turn / incremental update to the pktbuff API based on
> > your feedback:
> >
> > Patch #1 adds pktb_alloc_head() to allocate the pkt_buff structure.
> > 	 This patch also adds pktb_build_data() to set up the pktbuff
> > 	 data pointer.
> >
> > Patch #2 updates the existing example to use pktb_alloc_head() and
> >          pktb_build_data().
> >
> > Patch #3 adds a few helper functions to set up the pointer to the
> >          network header.
> >
> > Your goal is to avoid the memory allocation and the memcpy() in
> > pktb_alloc(). With this scheme, users pre-allocate the pktbuff object
> > from the configuration step, and then this object is recycled for each
> > packet that is received from the kernel.
> >
> > Would this update fit for your usecase?
> 
> No, sorry. The show-stopper is, no allowance for the "extra" arg, when you might
> want to mangle a packet tobe larger than it was.

I see, maybe pktb_build_data() can be extended to take the "extra"
arg. Or something like this:

 void pktb_build_data(struct pkt_buff *pktb, uint8_t *payload, uint32_t size, uint32_t len)

where size is the total buffer size, and len is the number of bytes
that are in used in the buffer.

> For "extra" support, you need something with the sophistication of pktb_malloc2.
> If extra == 0, pktb_malloc2 optimises by leaving the packet data where it was.

With this patchset, the user is in control of the data buffer memory
area that is attached to the pkt_buff head, so you can just allocate
the as many extra byte as you require.

> Actually pktb_malloc2 doesn't need to make this decision. That can be deferred
> to pktb_mangle, which could do the copy if it has been told to expand a packet
> and the copy has not already been done (new "copy done" flag in the opaque
> struct pkt_buff).

I think it's fine if pktb_mangle() deals with this data buffer
reallocation in case it needs to expand the packet, a extra patch on
top of this should be fine.

> My nfq-based accidentally-written ad blocker would benefit from that deferment -
> I allow extra bytes in case I have to lengthen a domain name, but most of the
> time I'm shortening them.

Thanks for explaining.