buglwIP - A Lightweight TCP/IP stack - Bugs: bug #3168, Handling UDP/TCP PCBs upon netif...

 
 

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

bug #3168: Handling UDP/TCP PCBs upon netif address change.

Submitter:  Leon Woestenberg <likewise>
Submitted:  Wed 09 Apr 2003 03:21:01 PM UTC
   
 
Category:  TCP Severity:  3,5
Item Group:  Faulty Behaviour Status:  Fixed
Privacy:  Public Assigned to:  goldsimon
Open/Closed:  Closed Planned Release:  None
lwIP version:  None

Jump to the original submission

Mon 21 May 2007 05:04:23 PM UTC, comment #19: 

OK, since noone seems to care, I'm closing this now!
A little summary:

While (at least) linux does not inform a socket that its local address has changed (if != INADDR_ANY), we do for TCP sockets, but not for UDP sockets. Personally, I think this is OK as long as we document it (where???).

Simon Goldschmidt <goldsimon>
Group administrator
Wed 16 May 2007 06:53:49 PM UTC, comment #18: 

OK, I've checked in file #12764.

What remains is that udp and tcp behave differently. Personally, I think we can leave it that way, document it and close this bug.

Simon Goldschmidt <goldsimon>
Group administrator
Mon 14 May 2007 08:48:23 PM UTC, comment #17: 

Funny. Maybe someone was too concerned about this, after all...

Since we already have the TCP code on netif address change, I suggest we leave it. The UDP address check is still needed. -> in progress...

Simon Goldschmidt <goldsimon>
Group administrator
Mon 14 May 2007 07:48:09 PM UTC, comment #16: 

Re TCP: the same. Only the bind() would fail.

Jonathan Larmour <jifl>
Group Member
Mon 14 May 2007 05:37:10 PM UTC, comment #15: 


>After trying on Linux, even when the locally bound address goes away, recv() will remain blocked and not report an error. Even if I do a non-blocking recv(), still no error is returned, and netstat shows the listening socket remains bound to the (now gone) address.


Hearing that, I'll check in my change that udp_send() checks the local address. That way we make sure we don't send packets from the old address.

On the receive side, the application has to work around that (like it has to do on linux). We are already supplying the functions for that (netif->callback, maybe we should call that on IP address change also?) or the app must use a receive timeout (or not bind to a specific address ;-)

I think we would be done with that. Only out of interest: what happens on linux with a tcp socket that is in a blocking recv call?

Simon Goldschmidt <goldsimon>
Group administrator
Mon 14 May 2007 02:31:45 PM UTC, comment #14: 

After trying on Linux, even when the locally bound address goes away, recv() will remain blocked and not report an error. Even if I do a non-blocking recv(), still no error is returned, and netstat shows the listening socket remains bound to the (now gone) address.

I guess the idea is that it's up to something else to deal with that, if it needs dealing with. Most sockets are (usually implicitly) bound to 0.0.0.0 of course, and in that case clearly one interface going away should not cause the socket to get an error. Perhaps this could be considered just the logical extension of that.

So I guess this could go back to the original report. Why is someone trying to bind to an address other than IP_ADDR_ANY then?

Jonathan Larmour <jifl>
Group Member
Sun 13 May 2007 03:05:55 PM UTC, comment #13: 


>The receive side, however, sits there and waits since there is no new packet delivered.


Knowing which error code to return, it would be the easiest to call udp_pcb->recv() with a pbuf = NULL to indicate an error, which could set conn->err appropriately. That way, a new recv would return that error. Handling a blocking recv while changing the ip_addr would still remain, though.

Simon Goldschmidt <goldsimon>
Group administrator
Sun 13 May 2007 02:48:01 PM UTC, comment #12: 


>From experimentation on Linux, trying it out with a UDP socket, send() gets EINVAL if the local IP address that had been bound gets changed.


We can get that by including a check in udp_send() that checks if the local_ip of the udp pcb (if not is_any) is the same as the ip_addr of the netif used to send the packet (if not, we return ERR_VAL).

If we then send to the socket/netconn, we get an error ERR_VAL into conn->err (do_send() has to be modified to catch more errors, see task #6880) that is translated to EINVAL.

See attached patch for this.

The receive side, however, sits there and waits since there is no new packet delivered. Jonathan, can you check on linux what happens calling recv() on such a socket? Or what happens if, during a blocking recv() call, the ip address gets changed?

(file #12764)

Simon Goldschmidt <goldsimon>
Group administrator
Fri 27 Apr 2007 11:55:53 AM UTC, comment #11: 

Systems are allowed to generate additional error codes beyond what  the standard lists for each function (as per http://www.opengroup.org/onlinepubs/007908799/xsh/errors.html).

The implication is that there isn't a laid down SUS standard for what to do here. EBADF definitely wouldn't be right though - that's what you get when you try to perform an operation on an invalid or closed file descriptor/socket. The socket isn't closed though - you still need to call close() on it (in UNIX systems) to free up the resources. For lwIP the principle would be the same - it still would need to be closed by the application (not by the stack itself), and until it is, it returns an error.

Jonathan Larmour <jifl>
Group Member
Fri 27 Apr 2007 11:32:04 AM UTC, comment #10: 


>From experimentation on Linux, trying it out with a UDP socket, send() gets EINVAL if the local IP address that had been bound gets changed.


That seems odd. Opengroup documentation does not mention EINVAL for send(), Linux documentation says invalid argument, OpenBSD says 'flags' argument is invalid. Shouldn't it return EBADF instead, if the UDP pcb is really closed?

Simon Goldschmidt <goldsimon>
Group administrator
Thu 26 Apr 2007 08:23:43 PM UTC, comment #9: 

Re comment 8, I think we should follow what other stacks do. From memory, with them, if they are unbound, i.e. local IP of 0.0.0.0, then they determine the local IP only at the point of transmission, based on routing. Otherwise if they are bound to a specific local IP addr, they get an error. From experimentation on Linux, trying it out with a UDP socket, send() gets EINVAL if the local IP address that had been bound gets changed.

Jonathan Larmour <jifl>
Group Member
Thu 26 Apr 2007 07:42:15 PM UTC, comment #8: 

Hm, thinking some more about this: can't we simply set the local_ip address of the UDP pcbs to the new address? I mean, maybe they are 'connected' somehow, but since UDP is a connectionless protocol, just setting the new address should be enough.

What still remains are RAW pcbs. They have a local_ip parameter which is used on sending, so must be updated on netif_ip_addr change also.

Simon Goldschmidt <goldsimon>
Group administrator
Thu 19 Apr 2007 11:23:30 AM UTC, comment #7: 

Since for TCP pcbs, a call to pcb->recv() with a pbuf p=NULL means the connection is closed, I propose to do it that way for UDP pcbs on netif address change.

I'll attach a patch as a first draft.

Note that the comment in netif_set_ipaddr() can be removed and the documentation (doc/rawapi.txt) must be updated (a notification mail to lwip-users would be good as well).

Last, the core stack and the ports in contrib should be updated to test p against null in all udp_pcv->recv() callbacks.

(file #12527)

Simon Goldschmidt <goldsimon>
Group administrator
Thu 05 Apr 2007 08:47:25 AM UTC, comment #6: 

The new error callback might have further uses, but I'd be happy with a change to the receive callback to indicate connection closed as well.  If we try and keep the UDP semantics as similar to the TCP semantics as possible I think that would be good.

Kieran Mansley <kieranm>
Group Member
Wed 04 Apr 2007 07:28:01 PM UTC, comment #5: 

I thought about it: another (simpler) solution would be to add an additional parameter to the receive callback to tell it the connection has closed. Or we define calling it with p = 0 means 'closed' or something.

The additional parameter means one more stack save and restore for most processors, while p = 0 can mean nullpointer-dereferencing (and not everyone catches them...).

I just don't like the idea of adding an extra callback that is only called for one single purpose (change of ip address).

What do you think?

Simon Goldschmidt <goldsimon>
Group administrator
Tue 06 Mar 2007 12:05:42 PM UTC, comment #4: 

Yes, at first glance it looks like we need to implement an error callback in the udp_pcb similar to the tcp one.  At the moment only receive callbacks are supported on UDP.

By adding that we could then close the defunct UDP connection, call the error callback, and be done.

Kieran Mansley <kieranm>
Group Member
Tue 06 Mar 2007 11:55:52 AM UTC, comment #3: 

Searching through the code, I found out that calling pcb->recv with pbuf argument = 0 is not a good idea, since at least recv_udp() in api_msg.c directly dereferences the p pointer without a check.

To fix this bug, we would have to implement closing a UDP connection from the stack side, which is AFAIK, not yet implemented, is it?

Simon Goldschmidt <goldsimon>
Group administrator
Tue 06 Mar 2007 10:23:57 AM UTC, comment #2: 

OK, I think we have to do something with all connections listed in 'udp_pcbs' (defined in udp.c) that are connected (e.g. have a remote port or remote ip set).

The only question is: what do we do? I would disconnect them but I don't know how to inform the connection of this state change...
Is calling pcb->recv(arg, pcb, 0, 0, 0) right?

Simon Goldschmidt <goldsimon>
Group administrator
Mon 26 Feb 2007 03:06:17 PM UTC, comment #1: 

Sounds like only UDP remains to be fixed.  Should be straightforward.  Any volunteers?

Kieran Mansley <kieranm>
Group Member
Wed 09 Apr 2003 03:21:01 PM UTC, original submission:  

Stale PCBs hang around upon netif address day.

http://mail.gnu.org/archive/html/lwip-users/2003-03/msg00118.html

A first approach of handling the TCP pcbs is in CVS.

Leon Woestenberg <likewise>
Group Member

 

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

Attached Files
file #12527:  netif_handle_udp.patch added by goldsimon (3KiB - 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 jifl (Posted a comment)
  • -email is unavailable- added by goldsimon (Posted a comment)
  • -email is unavailable- added by kieranm (Posted a comment)
  •  

    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
    2007-05-21 goldsimon StatusIn Progress Fixed
        Open/ClosedOpen Closed
    2007-05-13 goldsimon Attached File- Added udp_check_local_ip_on_send.patch, #12764
    2007-04-19 goldsimon StatusNone In Progress
    2007-04-19 goldsimon Attached File- Added netif_handle_udp.patch, #12527
    2007-03-06 kieranm Assigned tolikewise goldsimon

    Back to the top

    Powered by Savane 3.13-d3ae.
    Corresponding source code