buglwIP - A Lightweight TCP/IP stack - Bugs: bug #21049, Allow reuse of pbufs after return...

 
 

You are not allowed to post comments on this tracker with your current authentication level.

bug #21049: Allow reuse of pbufs after return from APIs

Submitter:  Jonathan Larmour <jifl>
Submitted:  Wed 12 Sep 2007 01:31:57 PM UTC
   
 
Category:  pbufs Severity:  2 - Minor
Item Group:  Faulty Behaviour Status:  Wont Fix
Privacy:  Public Assigned to:  jifl
Open/Closed:  Closed Planned Release:  None
lwIP version:  None

Tue 25 Mar 2008 01:48:39 PM UTC, comment #4: 

If we aren't going to permit user reuse of pbufs, as per your most recent comment in task #6735, then I agree this problem disappears and this task can be closed.

Closing now, but someone can reopen if they feel it should be.

Jonathan Larmour <jifl>
Group Member
Tue 25 Mar 2008 01:33:06 PM UTC, comment #3: 

I'm of the opinion that the code needed to do this will be too much of a burden to justify the feature.  Unless anyone who really wants this feature can persuade me otherwise, I think we should close this.

Kieran Mansley <kieranm>
Group Member
Wed 12 Sep 2007 02:14:03 PM UTC, comment #2: 

The problem is that most people won't be interested in reusing pbufs. The socket layer won't for example. So we shouldn't add overhead for a usually unnecssary operation.

A user-created pre-filled pbuf could have been changed in many ways. The payload pointer could have been changed arbitrarily. The only way you could guarantee reuse of the contents is to copy the pbuf and payload every time no matter what. That's too painful.

I think if you really want to reuse pbuf data, you should be using a PBUF_REF. Otherwise we should draw the line.

Jonathan Larmour <jifl>
Group Member
Wed 12 Sep 2007 01:56:22 PM UTC, comment #1: 

As nice as this seems, it has a slightly different goal than the "udp_send" thread in lwip-users. As I can see it (and this is also what I would expect the API to behave), Julian expected the pbuf to come back like it was given to udp_send().

The solution proposed here would let you re-fill the pbuf, but not really re-use it (e.g. change a pre-filled pbuf...).

Of course, restoring them to what they were before has the dowside of making the code bigger, even if you don't need that.

Simon Goldschmidt <goldsimon>
Group administrator
Wed 12 Sep 2007 01:31:57 PM UTC, original submission:  

From discussion of task #6735, it was decided that it should be possible to reuse pbufs after return from API calls.

At present this is not guaranteed, and task #6735 would need to be completed to deal with that, as queuing a pbuf would cause problems if the API user could then modify the pbuf. But there are other issues as discussed in the "udp_send" thread on the lwip-users mailing list of 2007-09-11. In there it was observed that udp_send uses pbuf_header on a pbuf passed to it. As a result, when the pbuf is returned, the payload pointer has changed.

If we are to allow reuse of pbufs, then there must be a mechanism to fix this. Instead of fixing it every time in udp_send, which seemed to be a bit of a faff for something uncommon, I proposed a potential API extension which users would be required to call before reusing a pbuf:

int pbuf_reuse(struct pbuf *p, pbuf_layer l)
{
  u16_t offset = 0;
  switch (l) {
  case PBUF_TRANSPORT:
   [... same treatment of offset as pbuf_alloc() ... ]
  }
  switch (p->type) {
  case PBUF_POOL:
  case PBUF_RAM:
    p->payload = LWIP_MEM_ALIGN((void )((u8_t )p + (SIZEOF_STRUCT_PBUF + offset)));
    break;
  case PBUF_ROM:
  case PBUF_REF:
    [do something?]
    break;
}


The only problem here is that pbuf_header allows use of a negative header offset for PBUF_ROM and PBUF_REF. If that is done there is no way to restore the old value. In fact that's a problem anyway whether we went for adding this new API call, or some other mechanism of restoring the pbuf. However although the stack uses negative header offset when adjusting incoming pbufs, I don't know for definite if it uses negative header offsets for pbufs for transmission. I haven't found any examples yet. Or maybe it's better just to return an error if this is called for PBUF_ROM or PBUF_REF?

I suspect Simon would like this approach because it gives a particularly good place to check that pbuf->ref is 1 before reuse. On the other hand, it's an API change. Kieran decided it should wait until after 1.3.0.

There may be other examples, especially with the raw API, where pbufs are not returned in the same state they were passed in.


Jonathan Larmour <jifl>
Group Member

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

No files currently attached

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by kieranm (Posted a comment)
  • -email is unavailable- added by goldsimon (Posted a comment)
  • -email is unavailable- added by jifl (Submitted the item)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

     

    Follow 3 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2008-03-25 jifl StatusNone Wont Fix
        Assigned toNone jifl
        Open/ClosedOpen Closed

    Back to the top

    Powered by Savane 3.13-02a9.
    Corresponding source code