[lwip-devel] [task #6735] Provide new pbuf type: PBUF_RAM_NOCOPY

[Jonathan Larmour]

Follow-up Comment #32, task #6735 (project lwip):

EVS Hardware Dpt wrote:
> 
> After reading the whole discussion my conclusion is that you
> want to be sure that each time you call an output function
> (tcp_write, udp_send,...) the buffer is free for re-use ?

That is what was decided, after looking at both sides of the argument.
 
> In our port, we use Dma functionnality, so low_level_output
> makes a pbuf_ref(p), put it on dma chain and return, then the
> TX Isr free this pbuf. If nobody has a ref on it, it's freed,
> else the buffer is still allocated and owned by it's creator.

Although the creator cannot tell, other than polling, whether they can reuse
it or not.
 
> For Udp, why not simply free the buffer directly after sending,
> either the ethernet driver has already sent the buffer and it
> is effectively freed, or the driver has still a reference on
> it and thus the buffer is not freed. Now if you want to send
> another bunch of data, simply ask for a new pBuf.
> 
> The basic schema would be :
> 
> while(MustSendData == TRUE)
> {
>    pbuf = pbuf_alloc()
>    fill_buf_with_data()
>    udp_send(pbuf)
>    pbuf_free(pbuf)
> }

Sure. That was explicitly considered - we could just say that a pbuf can't be
reused. But people were saying that it's sometimes useful to make minor
modifications to an existing buffer, rather than refilling it afresh every
time. (Admittedly personally, I'd have thought a PBUF_REF pointing to an
existing behaviour would be fine for that case, but whatever).
 
> Now for udp, if you don't want to free pbuf everytime, why not
> create a Macro like PBUF_CAN_REUSE that check that ref_count
> is 1, so this buffer is free for reuse. So the schema would
> be:
> 
> pbuf = pbuf_alloc()
> 
> while(MustSendData == TRUE)
> {
>    if(PBUF_CAN_REUSE(pbuf))
>    {
>       fill_buf_with_data()
>       udp_send(pbuf)
>    }
> }

Well, you could do that right now - no special macro required. But polling is
a highly inefficient way to do this, and bad in a multi-threaded system. But I
don't think this half way house makes sense - applications can't stall
depending on whether pbufs can be reused or not.

Or at the very least, there would want to be some sort of new callback to
indicate a pbuf can be reused, e.g. udp_sent().
 
> I like the current state of stack, and I think we must not
> change to something with lot of copies just to make sure that
> we can use it without prior knownledge. In our design we are
> able to get 950Mbps throughput and certainly don't want to go
> below that.

Exactly, that's the point of resolving this. Right now, since it's been
decided that pbufs can be reused, this cannot be done safely so I would guess
the majority of ethernet drivers would always need to copy every packet which
is ridiculous. You're saying we shouldn't change the stack, but arguably any
driver that isn't immediately sending any packet (and blocking till its
complete) is in error. So adding a PBUF_RAM_NOCOPY type is one way to preserve
the API and allow efficient use. I would like to hope that before long most
people would use PBUF_RAM_NOCOPY for pretty much everything and _not_
PBUF_RAM. I'm sure reusing pbufs is something that very few people do, but
apparently that's the way API has to be.

Personally in my driver I am doing what you did - just pbuf_ref and
pbuf_free, but then I'm guaranteeing I don't reuse the buffer (and insisting
my users don't either). But I want lwIP fixed properly because otherwise we
have anomalies like drivers queuing up data in pbufs that had been marked
PBUF_REF, which is an explicit indication from the user that the data is
volatile.

There are other potential ways to solve this while preserving the "you can
change the pbuf and data after sending" API behaviour. One might be to allow
registering a callback for when a pbuf has been pbuf_free()d such that its
reference count drops to 0. However this doesn't map well to higher level
APIs.

Jifl


    _______________________________________________________

Reply to this item at:

  <http://savannah.nongnu.org/task/?6735>

_______________________________________________
  Message sent via/by Savannah
  http://savannah.nongnu.org/