All of lore.kernel.org
 help / color / mirror / Atom feed
From: Florian Westphal <fw@strlen.de>
To: Randy Dunlap <rdunlap@infradead.org>
Cc: Jakub Kicinski <kuba@kernel.org>, Florian Westphal <fw@strlen.de>,
	Matthieu Baerts <matthieu.baerts@tessares.net>,
	linux-kernel@vger.kernel.org, kernel test robot <lkp@intel.com>,
	Aleksandr Nogikh <nogikh@google.com>,
	Willem de Bruijn <willemb@google.com>,
	linux-next@vger.kernel.org, netdev@vger.kernel.org
Subject: Re: [PATCH net-next v4] net: linux/skbuff.h: combine SKB_EXTENSIONS + KCOV handling
Date: Mon, 16 Nov 2020 18:16:00 +0100	[thread overview]
Message-ID: <20201116171600.GD22792@breakpoint.cc> (raw)
In-Reply-To: <53e73344-3a3a-4a11-9914-8490efa1a3b9@infradead.org>

Randy Dunlap <rdunlap@infradead.org> wrote:
> On 11/16/20 7:30 AM, Jakub Kicinski wrote:
> > On Mon, 16 Nov 2020 15:31:21 +0100 Florian Westphal wrote:
> >>>> @@ -4151,12 +4150,11 @@ enum skb_ext_id {
> >>>>   #if IS_ENABLED(CONFIG_MPTCP)
> >>>>   	SKB_EXT_MPTCP,
> >>>>   #endif
> >>>> -#if IS_ENABLED(CONFIG_KCOV)
> >>>>   	SKB_EXT_KCOV_HANDLE,
> >>>> -#endif  
> >>>
> >>> I don't think we should remove this #ifdef: the number of extensions are
> >>> currently limited to 8, we might not want to always have KCOV there even if
> >>> we don't want it. I think adding items in this enum only when needed was the
> >>> intension of Florian (+cc) when creating these SKB extensions.
> >>> Also, this will increase a tiny bit some structures, see "struct skb_ext()".  
> >>
> >> Yes, I would also prefer to retrain the ifdef.
> >>
> >> Another reason was to make sure that any skb_ext_add(..., MY_EXT) gives
> >> a compile error if the extension is not enabled.
> > 
> > Oh well, sorry for taking you down the wrong path Randy!
> 
> No problem.
> So we are back to v2, right?

Yes, you can still drop the line

>> +#if IS_ENABLED(CONFIG_KCOV) && IS_ENABLED(CONFIG_SKB_EXTENSIONS)

for enum skb_ext_id (alreadyt under SKB_EXTENSIONS).

Other than that v2 looks good to me.

Thanks!

      reply	other threads:[~2020-11-16 17:16 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-11-16  3:17 [PATCH net-next v4] net: linux/skbuff.h: combine SKB_EXTENSIONS + KCOV handling Randy Dunlap
2020-11-16 13:54 ` Matthieu Baerts
2020-11-16 14:31   ` Florian Westphal
2020-11-16 15:30     ` Jakub Kicinski
2020-11-16 17:08       ` Randy Dunlap
2020-11-16 17:16         ` Florian Westphal [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20201116171600.GD22792@breakpoint.cc \
    --to=fw@strlen.de \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-next@vger.kernel.org \
    --cc=lkp@intel.com \
    --cc=matthieu.baerts@tessares.net \
    --cc=netdev@vger.kernel.org \
    --cc=nogikh@google.com \
    --cc=rdunlap@infradead.org \
    --cc=willemb@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.