Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FS#1452 - [netifd] gretap instantiation fails on and after commit 1cd76e2 on "master" #6421

Closed
openwrt-bot opened this issue Mar 21, 2018 · 4 comments
Labels

Comments

@openwrt-bot
Copy link

jeffsf:

====Summary====

Prior to the indicated commit, gretap interfaces could be initialized through ''/etc/config/network''

After the indicated commit, gretap interfaces fail to be configured.

gretap interfaces can be initialized "by hand" using ''ip'' from ''ip-full'' in both cases

====Impact====

Unable to utilize gretap interfaces with UCI-based configuration

====To Replicate====

  • Build an Archer C7 image and "clean flash" onto an Archer C7
  • Add the following stanza to ''/etc/network/config'' (also occurs without ''force_link'')

config interface 'gt'
option proto 'gretap'
option ipaddr '192.168.1.1'
option peeraddr '192.168.1.2'
option force_link '1'

  • reboot
  • note that ''iw link'' does not show the link as even present
  • note that the logs indicate that the ''gt'' interface comes up and is immediately brought down
root@OpenWrt:~# logread | fgrep gt Tue Mar 13 12:35:25 2018 daemon.notice netifd: Interface 'gt' is setting up now Tue Mar 13 12:35:26 2018 daemon.notice netifd: Interface 'gt' is now down

====Expected Behavior====

  • ''iw link'' would show the link present and up, as in the proceeding commit on ''master'', 9004fc3

13: gre4t-gt99@NONE: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1280 qdisc fq_codel state UNKNOWN qlen 1000
link/ether ba:e1:b8:b7:fa:f5 brd ff:ff:ff:ff:ff:ff

  • the link would be usable and connect to its peer (if available)
  • ''logread'' would show the interface coming up

root@OpenWrt:~# logread | fgrep gt
Tue Mar 13 12:17:18 2018 daemon.notice netifd: Interface 'gt' is setting up now
Tue Mar 13 12:17:19 2018 daemon.notice netifd: Interface 'gt' is now up
Tue Mar 13 12:17:19 2018 daemon.notice netifd: tunnel 'gre4t-gt' link is up
Tue Mar 13 12:17:20 2018 user.notice firewall: Reloading firewall due to ifup of gt (gre4t-gt)

====Potential Cause====

commit 1cd76e2d85d86356868db731a5cacfb84150b2a1
Author: Felix Fietkau nbd@nbd.name
Date: Tue Mar 13 13:33:04 2018 +0100

netifd: update to the latest version (fixes FS#1358)

1f5a29c ip: do not add local routes for host dependencies
c06f842 device: add support for setting the isolate options for bridge ports
69aeaab interface-ip: fix route selection for host dependencies

Signed-off-by: Felix Fietkau <nbd@nbd.name></code>
@openwrt-bot
Copy link
Author

jeffsf:

Looking at the ''netifd'' commits, it appears that the change in behavior was introduced at the commit shown below.

While a patch to reverse this commit "resolves" the symptom, it is not clear that it is the //cause// of the problem. The change in ''netifd'' may cause ''NULL'' to be returned in some cases, rather than ''iface'' which may cause change in behavior elsewhere.

commit 1f5a29c3de6e3fec5883796ee772e25d56db6a69
Author: Felix Fietkau nbd@nbd.name
Date: Wed Mar 7 23:14:57 2018 +0100

ip: do not add local routes for host dependencies

This avoids creating invalid routes in cases where another daemon is
handling local routes for an interface, e.g. on mesh interfaces

Signed-off-by: Felix Fietkau <nbd@nbd.name>

The "reversal" also removes the symptoms on recent ''master'' (but again may cause other breakage I am not aware of)
lede_source$ git log -2 --format=oneline
6d9356e568d49a7d0aa02f4a37e0af900bf5cec0 (HEAD -> master-with-netifd-patch) Add a patch to reverse the upstream change to netifd
0f30f56e3857f3271796f4e5fa8f651841ab1a94 (origin/master, origin/HEAD, master) firewall: update to latest git HEAD

lede_source$ git diff master
diff --git a/package/network/config/netifd/patches/500-reverse-1f5a29c3de6e3fec5883796ee772e25d56db6a69.patch b/package/network/config/netifd/patches/500-reverse-1f5a29c3de6e3fec5883796ee772e25d56db6a69.patch
new file mode 100644
index 0000000..7f8f3d3
--- /dev/null
+++ b/package/network/config/netifd/patches/500-reverse-1f5a29c3de6e3fec5883796ee772e25d56db6a69.patch
@@ -0,0 +1,21 @@
+diff --git a/interface-ip.c b/interface-ip.c
+index 6ccc03e..dcf3390 100644
+--- a/interface-ip.c
++++ b/interface-ip.c
+@@ -262,7 +262,6 @@ interface_ip_add_target_route(union if_addr *addr, bool v6, struct interface *if

  •           }
    
  •   }
    

+-done:

  •   if (!r_next) {
    
  •           free(route);
    
  •           return NULL;
    

+@@ -273,6 +272,8 @@ done:

  •   route->mtu = r_next->mtu;
    
  •   route->metric = r_next->metric;
    
  •   route->table = r_next->table;
    

++
++done:

  •   route->iface = iface;
    
  •   if (defaultroute_target)
    
  •           free(route);
    

@openwrt-bot
Copy link
Author

jeffsf:

Difference between "working" and "non-working" first show up to me in the call from within ''gre.sh'' that works down to
ubus -S call network.interface notify_proto { "action": 6, "host": "", "interface": "" }

On a "bad" build, the ''ubus'' call returns 4 ''UBUS_STATUS_NOT_FOUND''

On a "good" build, the ''ubus'' call returns 0

This likely comes from ''netifd_add_host_route()'' in ''ubus.c''
iface = interface_ip_add_target_route(&a, v6, iface);
if (!iface)
return UBUS_STATUS_NOT_FOUND;

The following is a work-around, as I don't know what logic there might be around the NULL being returned
--- a/interface-ip.c 2018-03-22 10:44:21.103529283 -0700
+++ b/interface-ip.c 2018-03-24 15:11:07.650107318 -0700
@@ -265,7 +265,7 @@
done:
if (!r_next) {
free(route);

  •   return NULL;
    
  •   return iface;
    

    }

    iface = r_next->iface;

@openwrt-bot
Copy link
Author

nbd:

Fixed in r6552-d290024c42, thanks.

@openwrt-bot
Copy link
Author

jeffsf:

Thanks!!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

1 participant