lkml.org 
[lkml]   [2020]   [Nov]   [14]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH net-next v2] net: linux/skbuff.h: combine SKB_EXTENSIONS + KCOV handling
From
Date
On 11/14/20 11:54 AM, Jakub Kicinski wrote:
> On Sat, 14 Nov 2020 09:46:18 -0800 Randy Dunlap wrote:
>> The previous Kconfig patch led to some other build errors as
>> reported by the 0day bot and my own overnight build testing.
>>
>> These are all in <linux/skbuff.h> when KCOV is enabled but
>> SKB_EXTENSIONS is not enabled, so fix those by combining those conditions
>> in the header file.
>>
>> Fixes: 6370cc3bbd8a ("net: add kcov handle to skb extensions")
>> Fixes: 85ce50d337d1 ("net: kcov: don't select SKB_EXTENSIONS when there is no NET")
>> Signed-off-by: Randy Dunlap <rdunlap@infradead.org>
>> Reported-by: kernel test robot <lkp@intel.com>
>> Cc: Aleksandr Nogikh <nogikh@google.com>
>> Cc: Willem de Bruijn <willemb@google.com>
>> Cc: Jakub Kicinski <kuba@kernel.org>
>> Cc: linux-next@vger.kernel.org
>> Cc: netdev@vger.kernel.org
>> ---
>> v2: (as suggested by Matthieu Baerts <matthieu.baerts@tessares.net>)
>> drop an extraneous space in a comment;
>> use CONFIG_SKB_EXTENSIONS instead of CONFIG_NET;
>
> Thanks for the fix Randy!
>
>> --- linux-next-20201113.orig/include/linux/skbuff.h
>> +++ linux-next-20201113/include/linux/skbuff.h
>> @@ -4151,7 +4151,7 @@ enum skb_ext_id {
>> #if IS_ENABLED(CONFIG_MPTCP)
>> SKB_EXT_MPTCP,
>> #endif
>> -#if IS_ENABLED(CONFIG_KCOV)
>> +#if IS_ENABLED(CONFIG_KCOV) && IS_ENABLED(CONFIG_SKB_EXTENSIONS)
>
> I don't think this part is necessary, this is already under an ifdef:
>
> #ifdef CONFIG_SKB_EXTENSIONS
> enum skb_ext_id {
>
> if I'm reading the code right.

Oops, you are correct. Sorry I missed that.


> That said I don't know why the enum is under CONFIG_SKB_EXTENSIONS in
> the first place.
>
> If extensions are not used doesn't matter if we define the enum and with
> how many entries.
>
> At the same time if we take the enum from under the ifdef and add stubs
> for skb_ext_add() and skb_ext_find() we could actually remove the stubs
> for kcov-related helpers. That seems cleaner and less ifdefy to me.
>
> WDYT?

Good thing I am on my third cup of coffee.

OK, it looks like that should work -- with less #ifdef-ery.

I'll work at it...


>> SKB_EXT_KCOV_HANDLE,
>> #endif
>> SKB_EXT_NUM, /* must be last */
>> @@ -4608,7 +4608,7 @@ static inline void skb_reset_redirect(st
>> #endif
>> }
>>
>> -#ifdef CONFIG_KCOV
>> +#if IS_ENABLED(CONFIG_KCOV) && IS_ENABLED(CONFIG_SKB_EXTENSIONS)
>> static inline void skb_set_kcov_handle(struct sk_buff *skb,
>> const u64 kcov_handle)
>> {
>> @@ -4636,7 +4636,7 @@ static inline u64 skb_get_kcov_handle(st
>> static inline void skb_set_kcov_handle(struct sk_buff *skb,
>> const u64 kcov_handle) { }
>> static inline u64 skb_get_kcov_handle(struct sk_buff *skb) { return 0; }
>> -#endif /* CONFIG_KCOV */
>> +#endif /* CONFIG_KCOV && CONFIG_SKB_EXTENSIONS */
>>
>> #endif /* __KERNEL__ */
>> #endif /* _LINUX_SKBUFF_H */
>

thanks.
--
~Randy

\
 
 \ /
  Last update: 2020-11-14 21:17    [W:0.032 / U:1.720 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site