From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-7.1 required=3.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY, SPF_HELO_NONE,SPF_PASS autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id D5CAAC10F25 for ; Mon, 9 Mar 2020 21:36:34 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id A980321D7E for ; Mon, 9 Mar 2020 21:36:34 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1583789794; bh=SdWtHnMAwBNnf4keZ7lmm+B5l2JSIfY6JcxS+mLFFco=; h=Date:From:To:Cc:Subject:In-Reply-To:References:List-ID:From; b=eUjR9SZvSwZgXYOyMr0St/1UHNoDiszaXjruGbnSoYQ/w1NZlRyLnTa/mPvi8QnvO e4xkjXKIzCykkfyVKLQuryLUo96+v2On0tNN6YZ04H7d3HPqL/KS9A9+mEF1THpYuS wY+WZTmRMw+StqadEOJ2DH/9qC+sEinlWut26J28= Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726968AbgCIVgd (ORCPT ); Mon, 9 Mar 2020 17:36:33 -0400 Received: from mail.kernel.org ([198.145.29.99]:40514 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726872AbgCIVgd (ORCPT ); Mon, 9 Mar 2020 17:36:33 -0400 Received: from kicinski-fedora-PC1C0HJN (unknown [163.114.132.128]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id EC77624649; Mon, 9 Mar 2020 21:36:31 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1583789792; bh=SdWtHnMAwBNnf4keZ7lmm+B5l2JSIfY6JcxS+mLFFco=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=ZP+D641yB/RbUbAEV/InmiMOGnk/uQrptpkg44eNb4CkVI2/EO7IaKMYbvdYrfASp A+bAP+oZebkMoG+bTxdVKNj5fW7BeiPkDIetytB23GJQn8k9H/J1iPJQ2QBBy6cWCo 5KvfI0c2g+t8uUabvj6g1y9JuXPE7c6071JeAsso= Date: Mon, 9 Mar 2020 14:36:30 -0700 From: Jakub Kicinski To: Edward Cree Cc: Jiri Pirko , , , , , , , , , , , , , , , Subject: Re: [patch net-next v4 01/10] flow_offload: Introduce offload of HW stats type Message-ID: <20200309143630.2f83476f@kicinski-fedora-PC1C0HJN> In-Reply-To: <1b7ddf97-5626-e58c-0468-eae83ad020b3@solarflare.com> References: <20200307114020.8664-1-jiri@resnulli.us> <20200307114020.8664-2-jiri@resnulli.us> <1b7ddf97-5626-e58c-0468-eae83ad020b3@solarflare.com> MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org On Mon, 9 Mar 2020 16:52:16 +0000 Edward Cree wrote: > On 07/03/2020 11:40, Jiri Pirko wrote: > > From: Jiri Pirko > > > > Initially, pass "ANY" (struct is zeroed) to the drivers as that is the > > current implicit value coming down to flow_offload. Add a bool > > indicating that entries have mixed HW stats type. > > > > Signed-off-by: Jiri Pirko > > --- > > v3->v4: > > - fixed member alignment > > v2->v3: > > - moved to bitfield > > - removed "mixed" bool > > v1->v2: > > - moved to actions > > - add mixed bool > > --- > > include/net/flow_offload.h | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/include/net/flow_offload.h b/include/net/flow_offload.h > > index cd3510ac66b0..93d17f37e980 100644 > > --- a/include/net/flow_offload.h > > +++ b/include/net/flow_offload.h > > @@ -154,6 +154,8 @@ enum flow_action_mangle_base { > > FLOW_ACT_MANGLE_HDR_TYPE_UDP, > > }; > > =20 > > +#define FLOW_ACTION_HW_STATS_TYPE_ANY 0 =20 > I'm not quite sure why switching to a bit fieldapproach means these > =C2=A0haveto become #defines rather than enums... Perhaps having enum defining the values could be argued... > > + > > typedef void (*action_destr)(void *priv); > > =20 > > struct flow_action_cookie { > > @@ -168,6 +170,7 @@ void flow_action_cookie_destroy(struct flow_action_= cookie *cookie); > > =20 > > struct flow_action_entry { > > enum flow_action_id id; > > + u8 hw_stats_type; =20 > ... causing this to become a u8with nothing obviously preventing > =C2=A0a HW_STATS_TYPE bigger than 255 getting defined. > An enum type seems safer. ...but using the type for fields which are very likely to contain values from outside of the enumeration seems confusing, IMHO. Driver author can understandably try to simply handle all the values=20 in a switch statement and be unpleasantly surprised.