search

aparcar / openwrt

Staging tree of Paul Spooren
Other
9 stars 1 forks source link

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

Closed aparcar closed 6 years ago

aparcar commented 6 years ago

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====

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

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====

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

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>
aparcar commented 6 years ago

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);
aparcar commented 6 years ago

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;

aparcar commented 6 years ago

nbd:

Fixed in r6552-d290024c42, thanks.

aparcar commented 6 years ago

jeffsf:

Thanks!!