Hi,
On Sun, 27 Mar 2022 18:25:18 +0900 (JST)
YASUOKA Masahiko <yasu...@openbsd.org> wrote:
> On Wed, 9 Mar 2022 15:28:44 +0900
> Yuichiro NAITO <naito.yuich...@gmail.com> wrote:
>> I see 'Address already in use' message,
>> when I change wgrtable for a running wg interface.
>> It doesn't make sense to me.
>>
>> It can be reproduced by the following command sequence.
>>
>> ```
>> # route -T1 add default `cat /etc/mygate`
>> # ifconfig wg0 create wgport 7111 wgkey `cat /etc/mykey.wg0`
>> # ifconfig wg0 up
>> # ifconfig wg0 wgrtable 1
>> ifconfig: SIOCSWG: Address already in use
>> ```
>>
>> When I down wg0 interface before changing wgrtable,
>> It succeeds and no messages are shown.
>>
>> I investigated the reason why 'Address already in use' is shown.
>>
>> If wgrtable is specified by ifconfig argument,
>> `wg_ioctl_set` function in `sys/net/if_wg.c` is called.
>>
>> And if the wg interface is running, `wg_bind` function is called.
>> `wg_bind` creates new sockets (IPv4 and 6) and replace them from old
>> ones.
>>
>> If only wgrtable is changed, `wg_bind` binds as same port as existing
>> sockets.
>> So 'Address already in use' is shown.
>>
>> Here is a simple patch to close existing sockets before `wg_bind`.
>> It works for me but I'm not 100% sure this is right fix.
>>
>> Any other ideas?
>>
>> ```
>> diff --git a/sys/net/if_wg.c b/sys/net/if_wg.c
>> index 4dae3e3c976..0159664fb34 100644
>> --- a/sys/net/if_wg.c
>> +++ b/sys/net/if_wg.c
>> @@ -2253,11 +2253,14 @@ wg_ioctl_set(struct wg_softc *sc, struct
>> wg_data_io *data)
>> if (port != sc->sc_udp_port || rtable != sc->sc_udp_rtable) {
>> TAILQ_FOREACH(peer, &sc->sc_peer_seq, p_seq_entry)
>> wg_peer_clear_src(peer);
>>
>> - if (sc->sc_if.if_flags & IFF_RUNNING)
>> + if (sc->sc_if.if_flags & IFF_RUNNING) {
>> + if (port == sc->sc_udp_port)
>> + wg_unbind(sc);
>> if ((ret = wg_bind(sc, &port, &rtable)) != 0)
>> goto error;
>> + }
>>
>> sc->sc_udp_port = port;
>> sc->sc_udp_rtable = rtable;
>> }
>> ```
>
> If rdomain 1 exists, the error will not shown.
>
> # ifconfig vether0 rdomain 1 up
> # ifconfig wg0 create wgport 7111 wgkey `openssl rand -base64 32` up
> # ifconfig wg0 wgrtable 1
> #
>
> In the case which you reported to, it is supposed that rtable 1 exists
> but rdomain 1 doesn't exist.
>
> Even when "wgtable 1" is configured, becase there is no dedicated
> rdomain, rdomain 0 will be used to bind the UDP port.
This is not correct.
Configuring "wgtable 1" when rdomain 1 doesn't exist should mean that
default(rdomain 0) is used for inbound and rtable 1 used for outbound.
Binding the same address/port causes EADDRINUSE even if the rtable is
different. This is the original problem. Then we need to unbind the
old socket.
So your original diff has been basically OK. New diff is almost the
same of that diff, but tweak a condition which checks rdomain is the
same before unbinding the socket.
I belive this diff is ok.
Index: sys/net/if_wg.c
===================================================================
RCS file: /disk/cvs/openbsd/src/sys/net/if_wg.c,v
retrieving revision 1.25
diff -u -p -r1.25 if_wg.c
--- sys/net/if_wg.c 6 Jun 2022 14:45:41 -0000 1.25
+++ sys/net/if_wg.c 11 Jun 2022 10:55:18 -0000
@@ -751,6 +751,16 @@ wg_bind(struct wg_softc *sc, in_port_t *
int retries = 0;
retry:
#endif
+
+ if (port == sc->sc_udp_port &&
+ rtable_l2(rtable) == rtable_l2(sc->sc_udp_rtable)) {
+ /* changing rtable in the same domain */
+ wg_socket_close(&sc->sc_so4);
+#ifdef INET6
+ wg_socket_close(&sc->sc_so6);
+#endif
+ }
+
if ((ret = wg_socket_open(&so4, AF_INET, &port, &rtable, sc)) != 0)
return ret;
