From mboxrd@z Thu Jan 1 00:00:00 1970 From: Marcelo Ricardo Leitner 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 13:41:58 -0200 Message-ID: <20180125154158.GA14290@localhost.localdomain> References: <20180125001753.30408-1-jakub.kicinski@netronome.com> <20180125001753.30408-2-jakub.kicinski@netronome.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: davem@davemloft.net, jiri@resnulli.us, dsahern@gmail.com, netdev@vger.kernel.org, oss-drivers@netronome.com To: Jakub Kicinski Return-path: Received: from mail-wr0-f194.google.com ([209.85.128.194]:46809 "EHLO mail-wr0-f194.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750980AbeAYPnF (ORCPT ); Thu, 25 Jan 2018 10:43:05 -0500 Received: by mail-wr0-f194.google.com with SMTP id g21so8108035wrb.13 for ; Thu, 25 Jan 2018 07:43:05 -0800 (PST) Content-Disposition: inline In-Reply-To: <20180125001753.30408-2-jakub.kicinski@netronome.com> Sender: netdev-owner@vger.kernel.org List-ID: 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 >