Hi,
> On 14/01/2021, at 8:33 PM, YASUOKA Masahiko <[email protected]> wrote:
>
> Hi,
>
> On Thu, 14 Jan 2021 08:54:36 +0900
> Yuichiro NAITO <[email protected]> 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 <[email protected]>のメール:
>>> 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?
ok procter@
>
> 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;
> }
>
>
