buglwIP - A Lightweight TCP/IP stack - Bugs: bug #50694, TX exist more pbufs after enable...

 
 

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

bug #50694: TX exist more pbufs after enable LWIP_NETIF_TX_SINGLE_PBUF

Submitter:  Robin <back1127_mtk>
Submitted:  Sat 01 Apr 2017 08:35:28 AM UTC
   
 
Category:  TCP Severity:  3 - Normal
Item Group:  None Status:  Fixed
Privacy:  Public Assigned to:  goldsimon
Open/Closed:  Closed Planned Release:  None
lwIP version:  2.0.2

Jump to the original submission

Tue 25 Apr 2017 09:23:58 AM UTC, comment #8: 

Fixed by not executing phase 2 for LWIP_NETIF_TX_SINGLE_PBUF==1.

http://git.savannah.nongnu.org/cgit/lwip.git/commit/?id=dd811bca0689ed41f1cf6a0725f2b858a4a7a5a5

Simon Goldschmidt <goldsimon>
Group administrator
Tue 25 Apr 2017 08:12:14 AM UTC, comment #7: 

Ah, well I haven't tested that setting, sorry.

Forget the code in comment #5, pbuf_coalesce cannot be used here because 'p' must not be freed (the ref belongs to the caller).

I've added 'pbuf_alloc_copy' for that and clarified LWIP_NETIF_TX_SINGLE_PBUF int opt.h:

if (p->next != NULL) {
  struct pbuf *q = pbuf_alloc_copy(PBUF_RAW, PBUF_RAM, p);
  if (q == NULL) {
    return ERR_MEM;
  }
  /* ATTENTION: do NOT free the old 'p' as the ref belongs to the caller! */
  p = q;
}


We still could fix this bug here by not chaining pbufs to segments. I'd be happy to get a patch for that :-)

Simon Goldschmidt <goldsimon>
Group administrator
Tue 25 Apr 2017 01:23:19 AM UTC, comment #6: 

For the patch in comment #4:
seg->oversize_left is only available if TCP_OVERSIZE_DBGCHECK, so I got build failure if !TCP_OVERSIZE_DBGCHECK.

Axel Lin <axellin>
Group Member
Mon 24 Apr 2017 07:07:45 PM UTC, comment #5: 

To ensure linkoutput gets only packets in one piece, you can always add this to your driver:

if (p->next != NULL) {
  struct pbuf *q = pbuf_coalesce(p, PBUF_RAW_TX);
  if (q == p) {
    /* memory error */
    return ERR_MEM;
  }
  pbuf_free(p);
  p = q;
}
/* go on sending single-pbuf */

This handles future error cases, too.

Simon Goldschmidt <goldsimon>
Group administrator
Mon 24 Apr 2017 07:02:02 PM UTC, comment #4: 

Could you try the attached patch? It removes "seg->oversize_left = 0;" on output and should fix "pcb->unsent_oversize" on rexmit.

(file #40487)

Simon Goldschmidt <goldsimon>
Group administrator
Mon 24 Apr 2017 12:34:46 PM UTC, comment #3: 

I can confirm Robin's description on comment #2 because I can also run into this case:
Even with LWIP_NETIF_TX_SINGLE_PBUF set, it still has chance run into Phase 2: Chain a new pbuf to the end of pcb->unsent.
(My TCP_OVERSIZE is set to TCP_MSS)

Axel Lin <axellin>
Group Member
Fri 07 Apr 2017 01:03:54 AM UTC, comment #2: 

yes, i set TCP_OVERSIZE to TCP_MSS. if you set it as 0, it will check fail when lwip init.
Under this case, When last unsent have some space, and re-transmission it. the unsent_oversize will reset as 0. After that, The App send data again will enter concat_p allocate buffer. Then, it can't ensure TX single pbuf.

Robin <back1127_mtk>
Wed 05 Apr 2017 07:24:44 PM UTC, comment #1: 

For me, this works. Have you set TCP_OVERSIZE to TCP_MSS? In that case, a segment should always allow adding a full segment of data.

Simon Goldschmidt <goldsimon>
Group administrator
Sat 01 Apr 2017 08:35:28 AM UTC, original submission:  

if project enable LWIP_NETIF_TX_SINGLE_PBUF
After app call tcp_write
if ((pos < len) && (space > 0) && (last_unsent->len > 0))
it will create new concat_p
if concat_p is not NULL,it will concatenate last_unsent
So it exist more pbuf。

Robin <back1127_mtk>

 

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

Attached Files
file #40487:  bug50694_test.diff added by goldsimon (841B - application/octet-stream)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by axellin (Posted a comment)
  • -email is unavailable- added by goldsimon (Posted a comment)
  • -email is unavailable- added by back1127_mtk (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 6 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2017-04-25 goldsimon StatusNone Fixed
        Assigned toNone goldsimon
        Open/ClosedOpen Closed
    2017-04-25 goldsimon StatusWorks For Me None
    2017-04-24 goldsimon Attached File- Added bug50694_test.diff, #40487
    2017-04-05 goldsimon StatusNone Works For Me

    Back to the top

    Powered by Savane 3.13-3230.
    Corresponding source code