| Message ID | 20190319155236.GA10709@makrotopia.org |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [OpenWrt-Devel] wireguard: do not add host-dependencies if fwmark is set | expand |
On Tue, 19 Mar 2019 at 23:53, Daniel Golle <daniel@makrotopia.org> wrote: > > The 'fwmark' option is used to define routing traffic to > wireguard endpoints to go through specific routing tables. > In that case it doesn't make sense to setup routes for > host-dependencies in the 'main' table, so skip setting host > dependencies if 'fwmark' is set. > > Signed-off-by: Daniel Golle <daniel@makrotopia.org> I would suggest keeping the host route there. The direct effect of fwmark option is that traffic generated by wireguard will have that mark. The actual rules and routes will need to be setup by userspace. If I understand it correctly, it's mainly for using wireguard as the default route while not colliding with existing local lan routes and dhcp processes (supress_prefixlength 0). It fits in quite well across different network managers. OpenWrt has its own way of managing rules and routes and does not depend on wg-quick script. So in this context I think the host dependency route fits specifically well in OpenWrt environment ;) Regards, yousong > --- > .../network/services/wireguard/files/wireguard.sh | 14 ++++++++------ > 1 file changed, 8 insertions(+), 6 deletions(-) > > diff --git a/package/network/services/wireguard/files/wireguard.sh b/package/network/services/wireguard/files/wireguard.sh > index 96fa7215ff..58e47f9450 100644 > --- a/package/network/services/wireguard/files/wireguard.sh > +++ b/package/network/services/wireguard/files/wireguard.sh > @@ -176,12 +176,14 @@ proto_wireguard_setup() { > done > > # endpoint dependency > - wg show "${config}" endpoints | \ > - sed -E 's/\[?([0-9.:a-f]+)\]?:([0-9]+)/\1 \2/' | \ > - while IFS=$'\t ' read -r key address port; do > - [ -n "${port}" ] || continue > - proto_add_host_dependency "${config}" "${address}" > - done > + if [ ! "${fwmark}" ]; then > + wg show "${config}" endpoints | \ > + sed -E 's/\[?([0-9.:a-f]+)\]?:([0-9]+)/\1 \2/' | \ > + while IFS=$'\t ' read -r key address port; do > + [ -n "${port}" ] || continue > + proto_add_host_dependency "${config}" "${address}" > + done > + fi > > proto_send_update "${config}" > } > -- > 2.21.0 > > > _______________________________________________ > openwrt-devel mailing list > openwrt-devel@lists.openwrt.org > https://lists.openwrt.org/mailman/listinfo/openwrt-devel
On Wed, Mar 20, 2019 at 04:31:25PM +0800, Yousong Zhou wrote: > On Tue, 19 Mar 2019 at 23:53, Daniel Golle <daniel@makrotopia.org> wrote: > > > > The 'fwmark' option is used to define routing traffic to > > wireguard endpoints to go through specific routing tables. > > In that case it doesn't make sense to setup routes for > > host-dependencies in the 'main' table, so skip setting host > > dependencies if 'fwmark' is set. > > > > Signed-off-by: Daniel Golle <daniel@makrotopia.org> > > I would suggest keeping the host route there. The direct effect of > fwmark option is that traffic generated by wireguard will have that > mark. The actual rules and routes will need to be setup by userspace. > If I understand it correctly, it's mainly for using wireguard as the > default route while not colliding with existing local lan routes and > dhcp processes (supress_prefixlength 0). It fits in quite well across > different network managers. OpenWrt has its own way of managing rules > and routes and does not depend on wg-quick script. So in this context > I think the host dependency route fits specifically well in OpenWrt > environment ;) So maybe we can add an explicit option 'nohostroute' instead of making it depend on 'fwmark'? Currently I'm locally patching OpenWrt's wireguard scripts, because setting those host-routes in the 'main' routing table creates black- holes in my setup (due to firewalling). Cheers Daniel > > Regards, > yousong > > > --- > > .../network/services/wireguard/files/wireguard.sh | 14 ++++++++------ > > 1 file changed, 8 insertions(+), 6 deletions(-) > > > > diff --git a/package/network/services/wireguard/files/wireguard.sh b/package/network/services/wireguard/files/wireguard.sh > > index 96fa7215ff..58e47f9450 100644 > > --- a/package/network/services/wireguard/files/wireguard.sh > > +++ b/package/network/services/wireguard/files/wireguard.sh > > @@ -176,12 +176,14 @@ proto_wireguard_setup() { > > done > > > > # endpoint dependency > > - wg show "${config}" endpoints | \ > > - sed -E 's/\[?([0-9.:a-f]+)\]?:([0-9]+)/\1 \2/' | \ > > - while IFS=$'\t ' read -r key address port; do > > - [ -n "${port}" ] || continue > > - proto_add_host_dependency "${config}" "${address}" > > - done > > + if [ ! "${fwmark}" ]; then > > + wg show "${config}" endpoints | \ > > + sed -E 's/\[?([0-9.:a-f]+)\]?:([0-9]+)/\1 \2/' | \ > > + while IFS=$'\t ' read -r key address port; do > > + [ -n "${port}" ] || continue > > + proto_add_host_dependency "${config}" "${address}" > > + done > > + fi > > > > proto_send_update "${config}" > > } > > -- > > 2.21.0 > > > > > > _______________________________________________ > > openwrt-devel mailing list > > openwrt-devel@lists.openwrt.org > > https://lists.openwrt.org/mailman/listinfo/openwrt-devel
HI, I too think that making host route installation dependent on the fwmark option is not intuitive. > So maybe we can add an explicit option 'nohostroute' instead of making > it depend on 'fwmark'? A "nohostroute" (I'd prefer "peerroute" with default "1") would make sense imho. It should also be protocol agnostic because other tunnel protocols (e.g. 6in4) could benefit from it as well. I'd propose a generic "option peerroute 0" which inhibits the host route side-effect of "proto_add_host_dependency". The term "peerroute" was chosen because it falls in line with other options in the same group ("peeraddr", "peerdns"). ~ Jo
diff --git a/package/network/services/wireguard/files/wireguard.sh b/package/network/services/wireguard/files/wireguard.sh index 96fa7215ff..58e47f9450 100644 --- a/package/network/services/wireguard/files/wireguard.sh +++ b/package/network/services/wireguard/files/wireguard.sh @@ -176,12 +176,14 @@ proto_wireguard_setup() { done # endpoint dependency - wg show "${config}" endpoints | \ - sed -E 's/\[?([0-9.:a-f]+)\]?:([0-9]+)/\1 \2/' | \ - while IFS=$'\t ' read -r key address port; do - [ -n "${port}" ] || continue - proto_add_host_dependency "${config}" "${address}" - done + if [ ! "${fwmark}" ]; then + wg show "${config}" endpoints | \ + sed -E 's/\[?([0-9.:a-f]+)\]?:([0-9]+)/\1 \2/' | \ + while IFS=$'\t ' read -r key address port; do + [ -n "${port}" ] || continue + proto_add_host_dependency "${config}" "${address}" + done + fi proto_send_update "${config}" }
The 'fwmark' option is used to define routing traffic to wireguard endpoints to go through specific routing tables. In that case it doesn't make sense to setup routes for host-dependencies in the 'main' table, so skip setting host dependencies if 'fwmark' is set. Signed-off-by: Daniel Golle <daniel@makrotopia.org> --- .../network/services/wireguard/files/wireguard.sh | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-)