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

[Jonathan Larmour]

Update of task #6735 (project lwip):

        Percent Complete:                      0% => 50%                    

    _______________________________________________________

Follow-up Comment #7:

Hopefully the patch in the attachment is straightforward. It's a lot smaller
than I thought it would be.

The next stage of this task will update the maintained ports and ppp.

Let me know if you think the existence of the PBUF_RAM_NOCOPY type of pbuf
should be controlled by an option. The obvious issue being that the underlying
driver will need to support freeing the pbuf on transmission. But maybe this
just comes under the long list of things people need to change in ports and
drivers for 1.3, and I know Simon is itching to use NOCOPY pbufs in generic
core code as it is, so perhaps we should just make it a requirement for
ethernet drivers.

With this change the notional abstraction for the netif layer output function
is now very clear that it will behave as if it sent the packet immediately
(e.g. like a polled driver). In practice this may mean copying the payload if
it needs to. The upshot being that the payload is allowed to be changed by
higher layers (whether that be the stack or the user) once the netif->output
function returns. The exception of course being any pbuf marked as nocopy
(PBUF_RAM_NOCOPY or PBUF_ROM) which is the user's guarantee that that will not
happen, so the driver can reference the pbuf and its payload and for
PBUF_RAM_NOCOPY,  makes the call to free it.

Fragmentation of a PBUF_RAM_NOCOPY pbuf poses an awkward problem - if we need
to fragment, then we're creating new temporary pbufs and those are what are
handed to the driver, and so the driver can't tell that it needs to free some
original pbuf. So instead all we can do is create PBUF_REFs as before, which
may be copied (despite the original pbuf being marked as not needing to be
copied), and free the original on exit.

I'm not aware of any other "abuses" of pbuf internals like with ip_frag, but
I'll have a look around - let me know if you know any.

Note that I have some trouble with testing here because I can't test current
CVS - my port is still stuck in the 1.1 dark ages, and that's not likely to
change till 1.3 comes out (or at least the code base has stabilised in
preparation for 1.3 - I'm not in a position to play with unstable code and
changing APIs, so will have to wait at least for a pre-1.3 feature freeze). I
will be backporting this change to my 1.1 code base though.

So any comments before I commit?

Simon wrote:
> Wouldn't it be good to add an assertion into 
> sockets.c:lwip_sendto() at the point where the pbuf is freed 
> that there is no other reference to it? (i.e. p->ref == 1) If 
> ref is > 1, the MAC still holds a reference (because the packet
> is still queued) and that might lead to wrong data being sent.
> If such an assert triggers, the user must switch to use 
> PBUF_RAM(_NOCOPY?) instead.

An assert may be a good idea, although I note the problem cannot in fact be
fixed by the user if they're using the sockets API. It is the sockets API
internals which is calling netbuf_ref to make a PBUF_REF, and that's outside
of user control. The model the stack must use now is that if a PBUF_REF needs
to be queued in the driver, then it must be copied by the driver. It's the
only safe way to do it.

Jifl


(file #13528)
    _______________________________________________________

Additional Item Attachment:

File name: lwip.pbuf.nocopy.patch.txt     Size:10 KB


    _______________________________________________________

Reply to this item at:

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

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