[PATCH 1/1] macsec_linux: exit early when missing macsec kernel module

[Sabrina Dubroca]

Hello Michael,

2017-08-18, 19:44:20 +0200, Michael Braun wrote:
> Using driver macsec_linux makes no sense without macsec kernel module loaded.
> 
> Signed-off-by: Michael Braun <michael-dev at fami-braun.de>
> ---
>  src/drivers/driver_macsec_linux.c | 35 +++++++++++++++++++++++++++++++++++
>  1 file changed, 35 insertions(+)
> 
> diff --git a/src/drivers/driver_macsec_linux.c b/src/drivers/driver_macsec_linux.c
> index 0694e60..df2197b 100644
> --- a/src/drivers/driver_macsec_linux.c
> +++ b/src/drivers/driver_macsec_linux.c
> @@ -234,10 +234,45 @@ static void macsec_drv_wpa_deinit(void *priv)
>  }
>  
>  
> +static int macsec_check_macsec()
> +{
> +	struct nl_sock *sk;
> +	int err = -1;
> +
> +	sk = nl_socket_alloc();
> +	if (!sk) {
> +		wpa_printf(MSG_ERROR, DRV_PREFIX "failed to alloc genl socket");
> +		return -1;
> +	}
> +
> +	if (genl_connect(sk) < 0) {
> +		wpa_printf(MSG_ERROR,
> +			   DRV_PREFIX "connection to genl socket failed");
> +		goto out_free;
> +	}
> +
> +	int macsec_genl_id = genl_ctrl_resolve(sk, "macsec");

I'm not sure what version of the C standard hostap follows, but I
don't remember seeing mixed declarations and code.


> +	if (macsec_genl_id < 0) {
> +		wpa_printf(MSG_ERROR, DRV_PREFIX "genl resolve failed");
> +		goto out_free;
> +	}
> +
> +	err = 0;
> +
> +out_free:
> +	nl_socket_free(sk);
> +	return err;
> +}
> +
>  static void * macsec_drv_wpa_init(void *ctx, const char *ifname)
>  {
>  	struct macsec_drv_data *drv;
>  
> +	if (macsec_check_macsec() < 0) {
> +		wpa_printf(MSG_ERROR, DRV_PREFIX "macsec kernel module missing");

That's not quite true. It's unlikely, but possible, that the check
failed in the alloc or connect steps. Since all the failure cases
already output an error, I'd drop this one, or add it to the "genl
resolve failed" ("genl resolve failed -- the macsec kernel module is
probably missing" or similar).


BTW, your patch led me to check if we could make the macsec module
autoload, and I just submitted this patch to the kernel:
https://patchwork.ozlabs.org/patch/804437/


> +		return NULL;
> +	}
> +
>  	drv = os_zalloc(sizeof(*drv));
>  	if (!drv)
>  		return NULL;
> -- 
> 2.1.4


Thanks.

-- 
Sabrina