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 Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 9650FEC8750 for ; Thu, 7 Sep 2023 18:44:42 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1344278AbjIGSoo (ORCPT ); Thu, 7 Sep 2023 14:44:44 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:50364 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1344279AbjIGSom (ORCPT ); Thu, 7 Sep 2023 14:44:42 -0400 Received: from Chamillionaire.breakpoint.cc (Chamillionaire.breakpoint.cc [IPv6:2a0a:51c0:0:237:300::1]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 5B4FA132; Thu, 7 Sep 2023 11:44:37 -0700 (PDT) Received: from fw by Chamillionaire.breakpoint.cc with local (Exim 4.92) (envelope-from ) id 1qeJzS-0001hz-Iq; Thu, 07 Sep 2023 20:44:30 +0200 Date: Thu, 7 Sep 2023 20:44:30 +0200 From: Florian Westphal To: David Wang <00107082@163.com> Cc: Daniel Xu , Pablo Neira Ayuso , Jozsef Kadlecsik , Florian Westphal , netfilter-devel@vger.kernel.org, coreteam@netfilter.org, linux-kernel@vger.kernel.org, bpf@vger.kernel.org Subject: Re: [PATCH] uapi/netfilter: Change netfilter hook verdict code definition from macro to enum Message-ID: <20230907184430.GF20947@breakpoint.cc> References: <20230904130201.14632-1-00107082@163.com> <19d2362f.5c85.18a6647817b.Coremail.00107082@163.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <19d2362f.5c85.18a6647817b.Coremail.00107082@163.com> User-Agent: Mutt/1.10.1 (2018-07-13) Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org David Wang <00107082@163.com> wrote: > At 2023-09-06 00:38:02, "Daniel Xu" wrote: > >Hi David, > > > >On Mon, Sep 04, 2023 at 09:02:02PM +0800, David Wang wrote: > > >> #include > >> > >> /* Responses from hook functions. */ > >> -#define NF_DROP 0 > >> -#define NF_ACCEPT 1 > >> -#define NF_STOLEN 2 > >> -#define NF_QUEUE 3 > >> -#define NF_REPEAT 4 > >> -#define NF_STOP 5 /* Deprecated, for userspace nf_queue compatibility. */ > >> -#define NF_MAX_VERDICT NF_STOP > >> +enum { > >> + NF_DROP = 0, > >> + NF_ACCEPT = 1, > >> + NF_STOLEN = 2, > >> + NF_QUEUE = 3, > >> + NF_REPEAT = 4, > >> + NF_STOP = 5, /* Deprecated, for userspace nf_queue compatibility. */ > >> + NF_MAX_VERDICT = NF_STOP, > >> +}; > > > >Switching from macro to enum works for almost all use cases, but not > >all. If someone if #ifdefing the symbols (which is plausible) this > >change would break them. > > > >I think I've seen some other networking code define both enums and > >macros. But it was a little ugly. Not sure if that is acceptable here or > >not. > > > >[...] > > > >Thanks, > >Daniel > > > Thanks for the review~ > I do not have a strong reasoning to deny the possibility of breaking unexpected usage of this macros, > > but I also agree that it is ugly to use both enum and macro at the same time. > > Kind of don't know how to proceed from here now... I don't see anyone doing #ifdef tests on these, so I suggest we give your patch a try and see if anything breaks. Technically only ACCEPT and DROP can be used by bpf programs but splitting it in enum-for-accept-drop-and-define-for-the-rest looks even more silly.