netdev - Re: [PATCH net-next 1/8] pkt_cls: add new tc cls helper to check offload flag and chain index

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: <20180125154158.GA14290@localhost.localdomain>
Date:   Thu, 25 Jan 2018 13:41:58 -0200
From:   Marcelo Ricardo Leitner <marcelo.leitner@...il.com>
To:     Jakub Kicinski <jakub.kicinski@...ronome.com>
Cc:     davem@...emloft.net, jiri@...nulli.us, dsahern@...il.com,
        netdev@...r.kernel.org, oss-drivers@...ronome.com
Subject: Re: [PATCH net-next 1/8] pkt_cls: add new tc cls helper to check
 offload flag and chain index

On Wed, Jan 24, 2018 at 04:17:46PM -0800, Jakub Kicinski wrote:
...
> +static inline bool
> +tc_cls_can_offload_and_chain0(const struct net_device *dev,
> +			      struct tc_cls_common_offload *common)
> +{
> +	if (common->chain_index) {
> +		NL_SET_ERR_MSG(common->extack,
> +			       "Driver supports only offload of chain 0");
> +		return false;
> +	}
> +	return tc_can_offload_extack(dev, common->extack);

I know that most of the drivers updated in this patchset checks it
this way, but considering both checks end up being done anyway in the
success case and that performance POV on error path is irrelevant
here, wouldn't it be better to swap both conditions here? I.e., first
check if the device can offload, to only then check what is being
offloaded?

Otherwise the first error would be implying that the device can
offload, just not the specified chain.

> +}
> +
>  static inline bool tc_skip_hw(u32 flags)
>  {
>  	return (flags & TCA_CLS_FLAGS_SKIP_HW) ? true : false;
> -- 
> 2.15.1
>