net: Close all connections when interface goes down #98127
Conversation
jukkar added In progress 
subsys/net/ip/net_context.c Outdated
| /* Decrement refcount on user app's behalf */ | ||
| net_context_unref(&contexts[i]); |
There was a problem hiding this comment.
But I'm not sure if we should dereference net context as well? On the application side there's still an open socket that needs to be closed, when we close the socket the context would be unreffed by the socket code again I think?
Probably better just to have the net stack to report an error on the socket, and let the application close it. The question is, whether we should do it only for the sockets with active connections (i.e. TCP only, which should be handled by net_tcp_put() call here already), or really all sockets (we'd need some extra handling for UDP sockets then). IMHO, TCP only is fine, I don't think there's a need to close connection-less sockets?
jukkar force-pushed the fix/close-sockets-on-iface-down branch from 693c666 to 25da33c Compare 
|
include/zephyr/net/net_context.h Outdated
| * @brief Close and unref all network context bound to an network interface. | ||
| * | ||
| * @details This releases all the contexts that are bound to a specific | ||
| * network interface. It is not possible to send or receive data via those | ||
| * contexts after this call. |
There was a problem hiding this comment.
This description seems no longer valid. Maybe the function needs a rename as well given it's now just shutting down TCP connections on given interface.
subsys/net/ip/net_context.c Outdated
| contexts[i].connect_cb = NULL; | ||
| contexts[i].recv_cb = NULL; | ||
| contexts[i].send_cb = NULL; |
There was a problem hiding this comment.
We shouldn't clear the callbacks I think, TCP still needs connect_cb/recv_cb (depending on the internal state it's in) to notify the socket layer about connection being closed? If we clear the callback now, the application won't get an error from the socket.
There was a problem hiding this comment.
Indeed, good point, I will remove those lines.
If the network interface goes down, then we do not have any time to tear down TCP connection gracefully because there is no more a connection to send data to. In this case the TCP connection is forcefully closed without going into FIN_WAIT_1 state. Signed-off-by: Jukka Rissanen <jukka.rissanen@nordicsemi.no>
jukkar force-pushed the fix/close-sockets-on-iface-down branch from 25da33c to 360a716 Compare
|
rlubos left a comment
rlubos left a comment There was a problem hiding this comment.
One final nit, looks good otherwise.
subsys/net/ip/tcp.c Outdated
| if (IS_ENABLED(CONFIG_NET_OFFLOAD) && net_if_is_ip_offloaded(iface)) { | ||
| context->flags &= ~NET_CONTEXT_IN_USE; | ||
| (void)net_offload_put(iface, context); | ||
| k_mutex_unlock(&context->lock); | ||
| return; | ||
| } |
There was a problem hiding this comment.
I don't think we need this anymore, native TCP stack would never be used by offloaded interfaces.
There was a problem hiding this comment.
Yep, very true, will remove.
jukkar force-pushed the fix/close-sockets-on-iface-down branch from 360a716 to ada0276 Compare 
If the network interface goes down, close all TCP connections that are bound to that interface. The reasoning here is that we need a way to remove IP address from the interface and it might not be possible if there is a TCP socket that is bound to that address. If the interface comes back on, we might get the same IP address in which case the socket closure was not really needed but it is not possible to know in advance. Signed-off-by: Jukka Rissanen <jukka.rissanen@nordicsemi.no>
jukkar force-pushed the fix/close-sockets-on-iface-down branch from ada0276 to 23f49eb Compare
|
|
jukkar removed the In progress 
4 participants
If the network interface goes down, close all TCP connections that are bound to that interface. The reasoning here is that we need a way to remove IP address from the interface and it might not be possible if there is a connection that is bound to that address. If the interface comes back on, we might get the same IP address in which case the socket closure was not really needed but it is not possible to know in advance.