| Message ID | 20201114174618.24471-1-rdunlap@infradead.org (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net-next,v2] net: linux/skbuff.h: combine SKB_EXTENSIONS + KCOV handling | expand |
| Context | Check | Description |
|---|---|---|
| netdev/cover_letter | success | Link |
| netdev/fixes_present | success | Link |
| netdev/patch_count | success | Link |
| netdev/tree_selection | success | Clearly marked for net-next |
| netdev/subject_prefix | success | Link |
| netdev/source_inline | success | Was 0 now: 0 |
| netdev/verify_signedoff | success | Link |
| netdev/module_param | success | Was 0 now: 0 |
| netdev/build_32bit | success | Errors and warnings before: 9600 this patch: 9600 |
| netdev/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| netdev/verify_fixes | success | Link |
| netdev/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 24 lines checked |
| netdev/build_allmodconfig_warn | success | Errors and warnings before: 9969 this patch: 9969 |
| netdev/header_inline | success | Link |
| netdev/stable | success | Stable not CCed |
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. 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? > 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 */
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.
--- 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) 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 */
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; include/linux/skbuff.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)