netdev - Re: [PATCHv2 net-next 7/8] net: sched: cls: add extack support for tc_setup_cb

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20180117172528.39c16815@cakuba.netronome.com>
Date:   Wed, 17 Jan 2018 17:25:28 -0800
From:   Jakub Kicinski <kubakici@...pl>
To:     Alexander Aring <aring@...atatu.com>
Cc:     jhs@...atatu.com, xiyou.wangcong@...il.com, jiri@...nulli.us,
        davem@...emloft.net, netdev@...r.kernel.org, kernel@...atatu.com,
        David Ahern <dsahern@...il.com>
Subject: Re: [PATCHv2 net-next 7/8] net: sched: cls: add extack support for
 tc_setup_cb_call

On Wed, 17 Jan 2018 17:40:26 -0500, Alexander Aring wrote:
> diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c
> index 9f88107c29c5..e864ad523800 100644
> --- a/net/sched/cls_api.c
> +++ b/net/sched/cls_api.c
> @@ -1566,21 +1566,26 @@ static int tc_exts_setup_cb_egdev_call(struct tcf_exts *exts,
>  }
>  
>  int tc_setup_cb_call(struct tcf_block *block, struct tcf_exts *exts,
> -		     enum tc_setup_type type, void *type_data, bool err_stop)
> +		     enum tc_setup_type type, void *type_data, bool err_stop,
> +		     struct netlink_ext_ack *extack)
>  {
>  	int ok_count;
>  	int ret;
>  
>  	ret = tcf_block_cb_call(block, type, type_data, err_stop);
> -	if (ret < 0)
> +	if (ret < 0) {
> +		NL_SET_ERR_MSG(extack, "Failed to initialize tcf block");

This has nothing to do with block init.

>  		return ret;
> +	}
>  	ok_count = ret;
>  
>  	if (!exts)
>  		return ok_count;
>  	ret = tc_exts_setup_cb_egdev_call(exts, type, type_data, err_stop);
> -	if (ret < 0)
> +	if (ret < 0) {
> +		NL_SET_ERR_MSG(extack, "Failed to initialize tcf block extensions");

Ditto, plus this is about redirections to other devices (hence
eg[ress ]dev).  exts part is an internal detail.

>  		return ret;
> +	}
>  	ok_count += ret;
>  
>  	return ok_count;

These messages are completely off-target, and this is not a right place
to handle any of this.

Quentin's patches do the correct thing for offload, please drop this
patch from the series and let us handle the offload cases.

Thank you.