Hi, On Thu, 14 Jan 2021 08:54:36 +0900 Yuichiro NAITO <naito.yuich...@gmail.com> wrote: > Does anybody please review my code? > > Yasuoka-san is my coleague of my work. > So, he is interested in this topic. That’s why I CCed this mail. > I don’t mean he is an reviewer. > >> 2021/01/12 11:27、Yuichiro NAITO <naito.yuich...@gmail.com>のメール: >> I have set up multiple peers in a wg0 interface, >> and tried to remove more than one peers at once. >> Ifconfig(1) only removes the first peer. >> >> Command line was like following. >> >> ``` >> # ifconfig wg0 -wgpeer <keyA> -wgpeer <keyB> -wgpeer <keyC> >> ``` >> >> Only <keyA> was removed. >> >> I think next peer pointer isn't calculated in case of removing peer >> in sys/net/if_wg.c: wg_ioctl_set() function. >> >> I have tried following patch that can fix this problem.
Yes, the diff seems good. I made the following whitespace change. > @@ -2333,6 +2333,11 @@ wg_ioctl_set(struct wg_softc *sc, struct wg_data_io > *data) > } > > peer_p = (struct wg_peer_io *)aip_p; > + continue; > + next_peer: > + aip_p = &peer_p->p_aips[0]; > + aip_p += peer_o.p_aips_count; > + peer_p = (struct wg_peer_io *)aip_p; > } > > error: It seems we prefer putting goto labels at the beginning of the line. ok? Fix wg(4) ioctl to be able to handle multiple wgpeers. Diff from Yuichiro NAITO. Index: sys/net/if_wg.c =================================================================== RCS file: /cvs/src/sys/net/if_wg.c,v retrieving revision 1.14 diff -u -p -r1.14 if_wg.c --- sys/net/if_wg.c 1 Sep 2020 19:06:59 -0000 1.14 +++ sys/net/if_wg.c 14 Jan 2021 07:26:48 -0000 @@ -2270,7 +2270,7 @@ wg_ioctl_set(struct wg_softc *sc, struct /* Peer must have public key */ if (!(peer_o.p_flags & WG_PEER_HAS_PUBLIC)) - continue; + goto next_peer; /* 0 = latest protocol, 1 = this protocol */ if (peer_o.p_protocol_version != 0) { @@ -2283,7 +2283,7 @@ wg_ioctl_set(struct wg_softc *sc, struct /* Get local public and check that peer key doesn't match */ if (noise_local_keys(&sc->sc_local, public, NULL) == 0 && bcmp(public, peer_o.p_public, WG_KEY_SIZE) == 0) - continue; + goto next_peer; /* Lookup peer, or create if it doesn't exist */ if ((peer = wg_peer_lookup(sc, peer_o.p_public)) == NULL) { @@ -2291,7 +2291,7 @@ wg_ioctl_set(struct wg_softc *sc, struct * Also, don't create a new one if we only want to * update. */ if (peer_o.p_flags & (WG_PEER_REMOVE|WG_PEER_UPDATE)) - continue; + goto next_peer; if ((peer = wg_peer_create(sc, peer_o.p_public)) == NULL) { @@ -2303,7 +2303,7 @@ wg_ioctl_set(struct wg_softc *sc, struct /* Remove peer and continue if specified */ if (peer_o.p_flags & WG_PEER_REMOVE) { wg_peer_destroy(peer); - continue; + goto next_peer; } if (peer_o.p_flags & WG_PEER_HAS_ENDPOINT) @@ -2332,6 +2332,11 @@ wg_ioctl_set(struct wg_softc *sc, struct aip_p++; } + peer_p = (struct wg_peer_io *)aip_p; + continue; +next_peer: + aip_p = &peer_p->p_aips[0]; + aip_p += peer_o.p_aips_count; peer_p = (struct wg_peer_io *)aip_p; }