mwan3: Error: Invalid gateway address with 2.8.9 /lib/mwan3.sh

[jamesmacwhite]

Maintainer: @feckert Environment: 19.07.3 mwan3 2.8.9

Description:

/lib/mwan3.sh currently throws Error: Invalid gateway address during a mwan3 restart and doesn't work correctly, reverting mwan3.sh to the version in 2.8.8 resolves it. I believe the recent optimisations with shellcheck may need reviewing in a few places. A couple of issues were identified yesterday in #12806, but as that was for a separate issue not related to the issues found in the comments, I thought it would be better to break it out here to discuss further outside of it.

https://github.com/openwrt/packages/issues/12806#issuecomment-658811853

[feckert]

@jamesmacwhite thanks for your work and bug hunting. I have staged a P/R https://github.com/TDT-AG/packages/tree/pr/20200716-mwan3 in my repository do address the two changes discussed in #12806.

• Fix quotation that generate this error. This was added in commit https://github.com/openwrt/packages/pull/12810/commits/feae9e57423897e14dfb9f45f2defb97f480a731
• Fix idx calculation that from my point of view is wrong and this was shown by the following message on the cmduci: Entry not found

Please give them a try and let me know if everything works again.

[jamesmacwhite]

@feckert Thank you. I don't get any Entry not set issues anymore, but now I have this, so things still aren't right, I don't think. I can do further xtrace info if you want. I'm assuming it is the shellcheck changes, potentially having a negative effect.

I think https://github.com/openwrt/packages/commit/feae9e57423897e14dfb9f45f2defb97f480a731 will fix the error with the probability variable which caused the iptables/ip6tables errors.

I don't know if the idx calculation was broken in the end. My router environment was causing all sorts of console output which wasn't mwan3 related in the end, so hard to see the wood through the trees so to speak! I think maybe the idx change should be reverted. My reason for thinking that is running mwan3 2.8.8 with the previous idx code, it is working OK with no errors, so this might not be required. The calculation is a bit weird to me readability wise, but if it works... Maybe we should just leave it!

I don't think any uci errors could have come from the idx, because of the use of >/dev/null 2>&1 and then further confirmed by finding out miniupnpd was the actual cause of the errors.

Error: Invalid gateway address.
uci: Invalid argument
uci: Invalid argument
uci: Invalid argument
uci: Invalid argument
uci: Invalid argument
uci: Invalid argument
uci: Invalid argument
uci: Invalid argument
uci: Invalid argument
uci: Invalid argument
uci: Invalid argument
uci: Invalid argument
uci: Invalid argument
uci: Invalid argument
uci: Invalid argument
uci: Invalid argument
Error: Invalid gateway address.
uci: Invalid argument
uci: Invalid argument
uci: Invalid argument
uci: Invalid argument
uci: Invalid argument
uci: Invalid argument
uci: Invalid argument
uci: Invalid argument
Error: Invalid gateway address.
uci: Invalid argument
uci: Invalid argument
uci: Invalid argument
uci: Invalid argument
uci: Invalid argument
uci: Invalid argument
uci: Invalid argument
uci: Invalid argument

[aaronjg]

I think that is broken by this:

https://github.com/openwrt/packages/pull/12810/files#diff-ccaf6dffccf2fc134d63b5c33d5ee8e1R504

Is it possible that network_get_gateway via "$1" is storing an empty string in via? That would cause via to be added to the ip command, but it would have no argument. It may need a line like:

[ -z "$via" ] && unset via

Before there also was a check if via was 0.0.0.0 or :: which was removed. So maybe it should be:

{ [ -z "$via" ] || [ "$via" -eq "0.0.0.0" ]] || [ "$via" -eq "::" ] } && unset via

[jamesmacwhite]

@aaronjg Seems likely, I had a brief email convo with @feckert on the issue, xtrace revealed it is specific to IPv6 routes:

}}' -e 'metric=@.interface[@.interface='"'"'wan6'"'"'].metric'
+ __tmp='export metric=1; '
+ '[' -z 'export metric=1; ' ]
+ eval 'export metric=1; '
+ export 'metric=1'
+ ip -6 route flush table 2
+ ip -6 route add table 2 default via :: metric 1 dev 6in4-wan6
Error: Invalid gateway address.

}}' -e 'metric=@.interface[@.interface='"'"'wg6'"'"'].metric'
+ __tmp='export metric=3; '
+ '[' -z 'export metric=3; ' ]
+ eval 'export metric=3; '
+ export 'metric=3'
+ ip -6 route flush table 6
+ ip -6 route add table 6 default via :: metric 3 dev wg
Error: Invalid gateway address.
+ mwan3_rtmon_ipv6

}}' -e 'metric=@.interface[@.interface='"'"'wgb6'"'"'].metric'
+ __tmp='export metric=4; '
+ '[' -z 'export metric=4; ' ]
+ eval 'export metric=4; '
+ export 'metric=4'
+ ip -6 route flush table 8
+ ip -6 route add table 8 default via :: metric 4 dev wgb
Error: Invalid gateway address. 

[jamesmacwhite]

Just confirming this line fixes the issue:

https://github.com/openwrt/packages/pull/12814/files#diff-ccaf6dffccf2fc134d63b5c33d5ee8e1R521

When the PR is merged this issue can be closed.