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

From: Marcelo Ricardo Leitner <[email-masked]>
Date: Thu, 25 Jan 2018 13:41:58 -0200

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
>