Messages in this thread | | | Date | Mon, 16 Nov 2020 15:31:21 +0100 | From | Florian Westphal <> | Subject | Re: [PATCH net-next v4] net: linux/skbuff.h: combine SKB_EXTENSIONS + KCOV handling |
| |
Matthieu Baerts <matthieu.baerts@tessares.net> wrote: > > --- 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()".
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.
> 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().
Yes, exactly, I did not add these stubs because I could not figure out a case where an empty skb_ext_{add,find} would be wanted.
If your code calls skb_ext_add() but no skb extensions exist you forgot a SELECT/DEPENDS SKB_EXTENSIONS in Kconfig & compiler error would tell you that.
| |