From mboxrd@z Thu Jan  1 00:00:00 1970
From: Jakub Kicinski <jakub.kicinski@netronome.com>
Subject: Re: [PATCH net-next 1/8] pkt_cls: add new tc cls helper to check
 offload flag and chain index
Date: Thu, 25 Jan 2018 11:25:51 -0800
Message-ID: <20180125112551.598b522c@cakuba.netronome.com>
References: <20180125001753.30408-1-jakub.kicinski@netronome.com>
        <20180125001753.30408-2-jakub.kicinski@netronome.com>
        <20180125154158.GA14290@localhost.localdomain>
Mime-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
Cc: davem@davemloft.net, jiri@resnulli.us, dsahern@gmail.com,
        netdev@vger.kernel.org, oss-drivers@netronome.com
To: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
Return-path: <netdev-owner@vger.kernel.org>
Received: from mail-qt0-f179.google.com ([209.85.216.179]:32770 "EHLO
        mail-qt0-f179.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
        with ESMTP id S1751011AbeAYTZy (ORCPT
        <rfc822;netdev@vger.kernel.org>); Thu, 25 Jan 2018 14:25:54 -0500
Received: by mail-qt0-f179.google.com with SMTP id d8so22081983qtm.0
        for <netdev@vger.kernel.org>; Thu, 25 Jan 2018 11:25:54 -0800 (PST)
In-Reply-To: <20180125154158.GA14290@localhost.localdomain>
Sender: netdev-owner@vger.kernel.org
List-ID: <netdev.vger.kernel.org>

On Thu, 25 Jan 2018 13:41:58 -0200, Marcelo Ricardo Leitner wrote:
> 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.

Sure, can do.