Re: [PATCH] net: wireguard: Allow binding to specific ifindex

[Simon Horman <horms@kernel.org>]

On Mon, Nov 25, 2024 at 01:21:11PM -0800, greearb@candelatech.com wrote:
> From: Ben Greear <greearb@candelatech.com>
> 
> Which allows us to bind to VRF.
> 
> Signed-off-by: Ben Greear <greearb@candelatech.com>
> ---
> 
> NOTE:  Modified user-space to utilize this may be found here:
> https://github.com/greearb/wireguard-tools-ct
> Only the 'wg' part has been tested with this new feature as of today.

...

> diff --git a/drivers/net/wireguard/socket.c b/drivers/net/wireguard/socket.c
> index 0414d7a6ce74..a7cb1c7c3112 100644
> --- a/drivers/net/wireguard/socket.c
> +++ b/drivers/net/wireguard/socket.c
> @@ -25,7 +25,8 @@ static int send4(struct wg_device *wg, struct sk_buff *skb,
>  		.daddr = endpoint->addr4.sin_addr.s_addr,
>  		.fl4_dport = endpoint->addr4.sin_port,
>  		.flowi4_mark = wg->fwmark,
> -		.flowi4_proto = IPPROTO_UDP
> +		.flowi4_proto = IPPROTO_UDP,
> +		.flowi4_oif = wg->lowerdev,
>  	};
>  	struct rtable *rt = NULL;
>  	struct sock *sock;
> @@ -111,6 +112,9 @@ static int send6(struct wg_device *wg, struct sk_buff *skb,
>  	struct sock *sock;
>  	int ret = 0;
>  
> +	if (wg->lowerdev)
> +		fl.flowi6_oif = wg->lowerdev,

Hi Ben,

I think that the trailing ',' on the line above should be a ';'.
As written, with a ',', the call to skb_mark_not_on_list() 
below will be included in the conditional block above.
And this doesn't seem to be the intention of the code based on indentation.

Flagged by clang-19 with -Wcomma

> +
>  	skb_mark_not_on_list(skb);
>  	skb->dev = wg->dev;
>  	skb->mark = wg->fwmark;

...