Send connman mailing list submissions to
        [email protected]

To subscribe or unsubscribe via the World Wide Web, visit
        https://lists.01.org/mailman/listinfo/connman
or, via email, send a message with subject or body 'help' to
        [email protected]

You can reach the person managing the list at
        [email protected]

When replying, please edit your Subject line so it is more specific
than "Re: Contents of connman digest..."


Today's Topics:

   1. Re: rtnl sockets breaks with ENOBUFS after endless
      NEWROUTE/DELROUTE (Daniel Wagner)
   2. RE: rtnl sockets breaks with ENOBUFS after endless
      NEWROUTE/DELROUTE (Cunningham, Joel)


----------------------------------------------------------------------

Message: 1
Date: Fri, 15 Jun 2018 20:30:47 +0200
From: Daniel Wagner <[email protected]>
To: "Cunningham, Joel" <[email protected]>
Cc: "[email protected]" <[email protected]>
Subject: Re: rtnl sockets breaks with ENOBUFS after endless
        NEWROUTE/DELROUTE
Message-ID: <[email protected]>
Content-Type: text/plain; charset=windows-1252

Hi Joel,

> Here's the execution flow:
> 1. netlink_event
> 2. rtnl_message -> RTM_NEWROUTE (0.0.0.0)
> 3. rtnl_newroute
> 4. process_newroute
> 5. __connman_ipconfig_newroute
> 6. ipconfig->ops->route_set
> 7. service.c: service_route_changed
> 8. service.c: settings_changed
> 9. __connman_notifier_ipconfig_changed
> 10. notifier->ipconfig_changed
> 11. session.c: ipconfig_changed
> 12. update_routing_table
> 13. del_default_route
> 14. __connman_inet_del_default_from_table -> RTM_DELROUTE
> 15. Return to update_routing_table , add_default_route
> 16. __connman_inet_add_default_to_table -> RTM_NEWROUTE

First thanks for taking the time to debug this to this point.
It might be a problem with the session code:

 ../git/src/session.c:add_default_route() index 4 routing table 256 default 
gateway 192.168.254.254

Here we install the default route in the policy table for a session

 ../git/src/inet.c:__connman_inet_rtnl_open() fd 18
 ../git/src/inet.c:__connman_inet_rtnl_talk() handle 0xbe83ecd8 len 52
 ../git/src/inet.c:__connman_inet_rtnl_close() handle 0xbe83ecd8
 ../git/src/session.c:session_notify() session 0xdc058 owner :1.5 notify_path 
/com/garmin/wln/connman_notify
 ../git/src/service.c:default_changed() current default (nil)
 ../git/src/service.c:default_changed() new default 0xe4bb0 
wifi_104e8951d601_46726f6e74696572363637325f3547_managed_psk
 Failed to set domainname to frontierlocal.net
 ../git/src/service.c:service_save() service 0xe4bb0 new 0
 ../git/src/service.c:append_nameservers() servers[0] 192.168.254.254 available 
1
 ../git/src/connection.c:__connman_connection_update_gateway() default 0xe7050
 ../git/src/rtnl.c:netlink_event() Event, cond: 1
 ../git/src/rtnl.c:rtnl_message() buf 0xbe83eba0 len 88
 ../git/src/rtnl.c:rtnl_message() NEWADDR len 88 type 20 flags 0x0000 seq 1 pid 
-1618658980
 wlan0 {add} address 192.168.254.15/24 label wlan0 family 2

And here we get a new RNTL message which

 ../git/src/service.c:service_ip_bound() wlan0 ip bound
 ../git/src/service.c:service_ip_bound() service 0xe4bb0 ipconfig 0xe4dc0 type 
1 method 4
 ../git/src/session.c:ipconfig_changed() service 0xe4bb0 ipconfig 0xe4dc0

makes the session code believe something has
changed and now first it deletes the routing entry (note only in the policy 
table)

 ../git/src/session.c:del_default_route() index 4 routing table 256 default 
gateway 192.168.254.254

 ../git/src/inet.c:__connman_inet_rtnl_open() fd 14
 ../git/src/inet.c:__connman_inet_rtnl_talk() handle 0xbe83e510 len 52
 ../git/src/inet.c:__connman_inet_rtnl_close() handle 0xbe83e510

and adds it back 

 ../git/src/session.c:add_default_route() index 4 routing table 256 default 
gateway 192.168.254.254
 ../git/src/inet.c:__connman_inet_rtnl_open() fd 14
 ../git/src/inet.c:__connman_inet_rtnl_talk() handle 0xbe83e508 len 52
 ../git/src/inet.c:__connman_inet_rtnl_close() handle 0xbe83e508
 ../git/src/service.c:service_send_changed()
 ../git/src/service.c:service_append_added_foreach() changed 
/net/connman/service/wifi_104e8951d601_46726f6e74696572363637325f3547_managed_psk
 ../git/src/service.c:service_append_added_foreach() changed 
/net/connman/service/wifi_104e8951d601_46726f6e7469657236363732_managed_psk
 ../git/src/service.c:service_append_added_foreach() changed 
/net/connman/service/wifi_104e8951d601_hidden_managed_psk
 ../git/src/service.c:service_append_added_foreach() changed 
/net/connman/service/wifi_104e8951d601_4d79537065637472756d5769466938322d3247_managed_psk
 ../git/src/service.c:service_append_added_foreach() changed 
/net/connman/service/wifi_104e8951d601_4e696768746861776b20323a20456c65637472696320426f6f67616c6f6f_managed_psk
 ../git/src/service.c:service_append_added_foreach() changed 
/net/connman/service/wifi_104e8951d601_4d79537065637472756d5769466938322d3547_managed_psk
 ../git/src/service.c:service_append_added_foreach() changed 
/net/connman/service/gadget_224242424242_usb
 ../git/src/rtnl.c:netlink_event() Event, cond: 1
 ../git/src/rtnl.c:rtnl_message() buf 0xbe83eba0 len 60
 ../git/src/rtnl.c:rtnl_message() NEWROUTE len 60 type 24 flags 0x0600 seq 0 
pid 0
 ../git/src/rtnl.c:netlink_event() Event, cond: 1
 ../git/src/rtnl.c:rtnl_message() buf 0xbe83eba0 len 60
 ../git/src/rtnl.c:rtnl_message() NEWROUTE len 60 type 24 flags 0x0600 seq 0 
pid 0
 ../git/src/rtnl.c:netlink_event() Event, cond: 1
 ../git/src/rtnl.c:rtnl_message() buf 0xbe83eba0 len 60
 ../git/src/rtnl.c:rtnl_message() NEWROUTE len 60 type 24 flags 0x0600 seq 0 
pid 0
 wlan0 {add} route 192.168.254.0 gw 0.0.0.0 scope 253 <LINK>

And eventually the kernel tells us that we have a new route.

 ../git/src/rtnl.c:netlink_event() Event, cond: 1
 ../git/src/rtnl.c:rtnl_message() buf 0xbe83eba0 len 60
 ../git/src/rtnl.c:rtnl_message() NEWROUTE len 60 type 24 flags 0x0600 seq 0 
pid 0
 ../git/src/rtnl.c:netlink_event() Event, cond: 1
 ../git/src/rtnl.c:rtnl_message() buf 0xbe83eba0 len 52
 ../git/src/rtnl.c:rtnl_message() NEWROUTE len 52 type 24 flags 0x0600 seq 0 
pid 0
 wlan0 {add} route 192.168.254.254 gw 0.0.0.0 scope 253 <LINK>
 ../git/src/rtnl.c:netlink_event() Event, cond: 1
 ../git/src/rtnl.c:rtnl_message() buf 0xbe83eba0 len 52
 ../git/src/rtnl.c:rtnl_message() NEWROUTE len 52 type 24 flags 0x0600 seq 
1529074798 pid -665395655
 ../git/src/service.c:service_route_changed() wlan0 route changed

And now we are back at the beginning.

> For RTM_DELROUTE, it also calls route_unset which is the same
> service.c: service_route_changed, so both messages cause the same
> cycle
Not sure why I haven't seen this before. It could be just a race thing. 
If the RTNL arrives in the right time window we might just 
start to oscillate. 

> It's not clear to me why we are deleting the default route and adding
> it back again when receiveing either NEWROUTE or DELROUTE.  Seems
> like Connman should just update its local state rather than issue
> RTM_DELROUTE then RTM_NEWROUTE.
I guess we need to make the session code a bit smarter. The current
approach is for changes, to remove everything and then add it back.
That makes it pretty robust, but in your case we start to oscillate.

> Any insight is appreciated!

I haven't look at the session code for a while but I suggest
you start at session.c:ipconfig_changed() and try to filter the

 wlan0 {add} route 192.168.254.254 gw 0.0.0.0 scope 253 <LINK>

for the policy routing table. I don't know if the RNTL message 
tells us for which table it is.

HTH!

Thanks,
Daniel


------------------------------

Message: 2
Date: Fri, 15 Jun 2018 21:56:00 +0000
From: "Cunningham, Joel" <[email protected]>
To: Daniel Wagner <[email protected]>
Cc: "[email protected]" <[email protected]>
Subject: RE: rtnl sockets breaks with ENOBUFS after endless
        NEWROUTE/DELROUTE
Message-ID: <[email protected]>
Content-Type: text/plain; charset="us-ascii"

> -----Original Message-----
> From: Daniel Wagner [mailto:[email protected]]
> Sent: Friday, June 15, 2018 1:31 PM
> To: Cunningham, Joel <[email protected]>
> Cc: [email protected]
> Subject: Re: rtnl sockets breaks with ENOBUFS after endless
> NEWROUTE/DELROUTE
>
> Hi Joel,
>
> > Here's the execution flow:
> > 1. netlink_event
> > 2. rtnl_message -> RTM_NEWROUTE (0.0.0.0) 3. rtnl_newroute 4.
> > process_newroute 5. __connman_ipconfig_newroute 6.
> > ipconfig->ops->route_set 7. service.c: service_route_changed 8.
> > service.c: settings_changed 9. __connman_notifier_ipconfig_changed
> > 10. notifier->ipconfig_changed
> > 11. session.c: ipconfig_changed
> > 12. update_routing_table
> > 13. del_default_route
> > 14. __connman_inet_del_default_from_table -> RTM_DELROUTE 15.
> Return
> > to update_routing_table , add_default_route 16.
> > __connman_inet_add_default_to_table -> RTM_NEWROUTE
>
> First thanks for taking the time to debug this to this point.
> It might be a problem with the session code:

No problem, thanks for quickly looking at this!

> > It's not clear to me why we are deleting the default route and adding
> > it back again when receiveing either NEWROUTE or DELROUTE.  Seems like
> > Connman should just update its local state rather than issue
> > RTM_DELROUTE then RTM_NEWROUTE.
> I guess we need to make the session code a bit smarter. The current
> approach is for changes, to remove everything and then add it back.
> That makes it pretty robust, but in your case we start to oscillate.
>
> > Any insight is appreciated!
>
> I haven't look at the session code for a while but I suggest you start at
> session.c:ipconfig_changed() and try to filter the
>
>  wlan0 {add} route 192.168.254.254 gw 0.0.0.0 scope 253 <LINK>
>
> for the policy routing table. I don't know if the RNTL message tells us for
> which table it is.

struct rtmsg has the rtm_table member and RTA_TABLE will be present if 
rtm_table is RT_TABLE_UNSPEC. I was able to pass that through to 
__connman_ipconfig_newroute and __connman_ipconfig_delroute.

What I found out was for additional Adds/Deletes that come back, i.e. wlan0 
{add} route 192.168.254.254 gw 0.0.0.0 scope 253 <LINK>, is for table 254 
(Main). I don't have a lot of experience with Linux routing policies, but does 
it sound right that we'd add a default route for our custom table (256) and 
then the kernel would make updates to the Main table?

I was able to do some simple filtering in 
__connman_ipconfig_newroute/__connman_ipconfig_delroute so that it ignores the 
route change if table < 256.  If that sounds correct, I can put together a 
patch.  Further thinking, I'm wondering if we should only be matching the route 
changes if the table ID matches the session's table ID.  This would require 
either maintaining a route table list in ipconfig.c or sending the table ID 
further down so it can be dropped before applying to the session in 
session.c:ipconfig_changed

I have attached a new log that shows the table output (this log if of the 
failure case and doesn't have the filter).

Joel

________________________________

CONFIDENTIALITY NOTICE: This email and any attachments are for the sole use of 
the intended recipient(s) and contain information that may be Garmin 
confidential and/or Garmin legally privileged. If you have received this email 
in error, please notify the sender by reply email and delete the message. Any 
disclosure, copying, distribution or use of this communication (including 
attachments) by someone other than the intended recipient is prohibited. Thank 
you.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: new-del-route-with-table.log
Type: application/octet-stream
Size: 144667 bytes
Desc: new-del-route-with-table.log
URL: 
<http://lists.01.org/pipermail/connman/attachments/20180615/986fd053/attachment.obj>

------------------------------

Subject: Digest Footer

_______________________________________________
connman mailing list
[email protected]
https://lists.01.org/mailman/listinfo/connman


------------------------------

End of connman Digest, Vol 32, Issue 8
**************************************

Reply via email to