Hi, I noticed that there is a problem in gateway code when there are more than one service connected simultaneously. It is possible to have more than one default gateways at the same time. Fortunately the service code was correct so only one service was in online state. While fixing that one I noticed problems in other parts of connection.c like VPN route setting etc so this ended up being a bit bigger patchset than I hoped.
First two patches add more debugging support in inet.c and connection.c. These are useful to understand what is going on in the system. Patch #3 fixes the issue when we trigger service state change before all the gateways are set. Normally this does not cause problems but this patch makes it more clearer (and easier to read the logs) if the service online checks etc. are done after we have set all the routes. There is no need to remove the nameservers twice in the same function. This is fixed in patch #4. When the nameservers are removed in __connman_connection_gateway_remove() we should remove only certain type nameservers (IPv4 or IPv6). Without the patch #5, we might remove IPv4 routes by accident if we are only removing IPv6 gateway routes. If default gateway changes, the service will notify it. It is possible that we notify the default changed unnecessarily if the default have not been changed. This is checked in patch #6. The add_gateway() can change the gateway hash so we get the active gateway only after add_gateway(). Otherwise it is possible that active_gateway points to already freed memory, I saw the issue in one valgrind run. This is fixed in patch #7. The VPN host routes are not needed so the patch #8 does not set them any longer. Because there can be more than one active gateway (active means that we have received rtnl new gateway message for it) in the system, we just cannot blindly take some active gateway and set VPN routes based on it. So the find_phy_gateway() will resolve the correct phy ip and set_vpn_routes() will save it. This is done in patch #9. The patch #10 is also fixing multiple active gateway issue. We must check all the active gateways in __connman_connection_update_gateway() so that there will be only one default gateway. The gateway host route is not needed in VPN so remove it in patch #11. The system automagically creates a VPN route to gateway, this is unnecessary so remove it in patch #12. The patch #13 finally fixes the issue of multiple default gateways. It is only possible to get multiple default gateways when connman starts or exists offline mode. In that case multiple bearers will compete each other for the default route and rtnl messages come and go. Depending on timings, it is possible that more than one gateway will go active and have default gateway set. The patch fixes that issue by checking in connection_newgateway() whether there are other gateways that are active or not. All the other active gateways are downgraded (their default gateway is removed) if the ordering permits. It is still possible to have more than one default route if the rtnl new route message is not received. In this case connman cannot activate the gateway yet. I am not sure how this issue could be solved. Cheers, Jukka Jukka Rissanen (13): inet: Fix debugging prints connection: Fix debugging prints connection: Trigger service updates only after setting gateways connection: Remove nameserver routes only once service: Allow removing only certain type nameservers service: Check redundant default changed notify connection: Avoid stale memory access connection: Do not set host routes in VPN case connection: Get correct VPN phy link data connection: Check all active gateways in update connection: Remove obsolete routes for VPN connection: Remove unnecessary VPN gateway route connection: Try to avoid having two default gateways src/connection.c | 429 ++++++++++++++++++++++++++++++++++++++++-------------- src/connman.h | 3 +- src/inet.c | 21 ++- src/service.c | 36 ++++-- 4 files changed, 359 insertions(+), 130 deletions(-) _______________________________________________ connman mailing list [email protected] http://lists.connman.net/listinfo/connman
