buglwIP - A Lightweight TCP/IP stack - Bugs: bug #64965, udp_sendto() doesn't remove any...

 
 

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

bug #64965: udp_sendto() doesn't remove any added headers if the send fails

Submitter:  Shawn Silverman <ssilverman>
Submitted:  Sat 02 Dec 2023 08:15:29 PM UTC
   
 
Category:  UDP Severity:  3 - Normal
Item Group:  Faulty Behaviour Status:  Wont Fix
Privacy:  Public Assigned to:  goldsimon
Open/Closed:  Closed Planned Release:  None
lwIP version:  git head

Tue 09 Jan 2024 08:31:38 PM UTC, comment #1: 

I'd count this as "intended behaviour". While this isn't too cool to be a feature, it actually makes the stack smaller because you don't need all the "revert on error" code. It's been like that for ages now...

Simon Goldschmidt <goldsimon>
Group administrator
Sat 02 Dec 2023 08:15:29 PM UTC, original submission:  

I recently encountered this when I was retrying a UDP send. I was getting ERR_BUF errors then realized any added headers down the chain (UDP->IP->etc) weren't removed on failure.

For my specific use case, I do retries until not ERR_WOULDBLOCK because my low-level driver returns that if there are no TX buffers yet available.

My fix looks like this (see the "udp_sendto() may have added a header" part):

  // Note: Use PBUF_RAM for TX
  struct pbuf *p = pbuf_alloc(PBUF_TRANSPORT, len, PBUF_RAM);
  if (p == nullptr) {
    return false;
  }

  pbuf_take(p, data, len);
  err_t err;

  // Repeat until not ERR_WOULDBLOCK because the low-level driver returns that
  // if there are no internal TX buffers available
  do {
    err = udp_sendto(pcb_, p, ipaddr, port);
    if (err != ERR_WOULDBLOCK) {
      break;
    }

    // udp_sendto() may have added a header
    if (p->tot_len > len) {
      pbuf_remove_header(p, p->tot_len - len);
    }
  } while (true);


It feels like a failure should revert the pbuf, unless this is intended behaviour? In my view, a failure should never cause inputs to be modified, or at least restore the state if they are.

That also leads to the question: should it be assumed that any pbufs passed to lwIP functions, even for successful calls, could be modified?

Shawn Silverman <ssilverman>

 

(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 goldsimon (Posted a comment)
  • -email is unavailable- added by ssilverman (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
    2024-01-09 goldsimon StatusNone Wont Fix
        Assigned toNone goldsimon
        Open/ClosedOpen Closed

    Back to the top

    Powered by Savane 3.13-04b1.
    Corresponding source code