Messages in this thread | | | From | Matthieu Baerts <> | Subject | Re: [PATCH net-next v4] net: linux/skbuff.h: combine SKB_EXTENSIONS + KCOV handling | Date | Mon, 16 Nov 2020 14:54:17 +0100 |
| |
Hi Randy,
On 16/11/2020 04:17, 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. > > Also, add stubs for skb_ext_add() and skb_ext_find() to reduce the > amount of ifdef-ery. (Jakub)
It makes sense, good idea!
Thank you for the new version!
> --- linux-next-20201113.orig/include/linux/skbuff.h > +++ linux-next-20201113/include/linux/skbuff.h > @@ -4137,7 +4137,6 @@ static inline void skb_set_nfct(struct s > #endif > } > > -#ifdef CONFIG_SKB_EXTENSIONS > enum skb_ext_id { > #if IS_ENABLED(CONFIG_BRIDGE_NETFILTER) > SKB_EXT_BRIDGE_NF, > @@ -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()".
But apart from that, I think we are fine, even if we add new extensions in the future after this kcov one.
So if we think it is better to remove these #ifdef here, we should be OK. But if we prefer not to do that, we should then not add stubs for skb_ext_{add,find}() and keep the ones for skb_[gs]et_kcov_handle().
Cheers, Matt -- Tessares | Belgium | Hybrid Access Solutions www.tessares.net
| |