From mboxrd@z Thu Jan 1 00:00:00 1970 From: Mat Martineau Subject: Re: [PATCH net-next 02/13] sk_buff: add skb extension infrastructure Date: Tue, 11 Dec 2018 16:07:06 -0800 (PST) Message-ID: References: <20181210145006.19098-1-fw@strlen.de> <20181210145006.19098-3-fw@strlen.de> Mime-Version: 1.0 Content-Type: multipart/mixed; BOUNDARY="0-119641190-1544573195=:20227" Cc: netdev@vger.kernel.org To: Florian Westphal Return-path: Received: from mga12.intel.com ([192.55.52.136]:58806 "EHLO mga12.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726183AbeLLAHI (ORCPT ); Tue, 11 Dec 2018 19:07:08 -0500 In-Reply-To: Content-ID: Sender: netdev-owner@vger.kernel.org List-ID: This message is in MIME format. The first part should be readable text, while the remaining parts are likely unreadable without MIME-aware tools. --0-119641190-1544573195=:20227 Content-Type: text/plain; CHARSET=ISO-8859-15; FORMAT=flowed Content-Transfer-Encoding: 8BIT Content-ID: On Mon, 11 Dec 2018, Florian Westphal wrote: ... > diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h > index b1831a5ca173..d715736eb734 100644 > --- a/include/linux/skbuff.h > +++ b/include/linux/skbuff.h ... > @@ -3896,6 +3906,113 @@ static inline void nf_conntrack_get(struct nf_conntrack *nfct) >                 atomic_inc(&nfct->use); >  } >  #endif > + > +#ifdef CONFIG_SKB_EXTENSIONS > +enum skb_ext_id { > +#if IS_ENABLED(CONFIG_BRIDGE_NETFILTER) > +       SKB_EXT_BRIDGE_NF, > +#endif > +       SKB_EXT_NUM, /* must be last */ > +}; Could enum skb_ext_id always be defined, with none of the SKB_EXT_* values conditionally excluded? In combination with some alternate function definitions below (see later comments) I think this could reduce the need for CONFIG_SKB_EXTENSIONS preprocessor conditionals throughout the net code. > + > +/** > + *     struct skb_ext - sk_buff extensions > + *     @refcnt: 1 on allocation, deallocated on 0 > + *     @offset: offset to add to @data to obtain extension address > + *     @chunks: size currently allocated, stored in SKB_EXT_ALIGN_SHIFT units > + *     @data: start of extension data, variable sized > + * > + *     Note: offsets/lengths are stored in chunks of 8 bytes, this allows > + *     to use 'u8' types while allowing up to 2kb worth of extension data. > + */ > +struct skb_ext { > +       refcount_t refcnt; > +       u8 offset[SKB_EXT_NUM]; /* in chunks of 8 bytes */ > +       u8 chunks;              /* same */ > +       char data[0] __aligned(8); > +}; > + > +void *skb_ext_add(struct sk_buff *skb, enum skb_ext_id id); > +void __skb_ext_del(struct sk_buff *skb, enum skb_ext_id id); > +void __skb_ext_free(struct skb_ext *ext); > + > +static inline void __skb_ext_put(struct skb_ext *ext) > +{ > +       if (ext && refcount_dec_and_test(&ext->refcnt)) > +               __skb_ext_free(ext); > +} > + > +static inline void skb_ext_put(struct sk_buff *skb) > +{ > +       if (skb->active_extensions) > +               __skb_ext_put(skb->extensions); > +} > + > +static inline void skb_ext_get(struct sk_buff *skb) > +{ > +       if (skb->active_extensions) { > +               struct skb_ext *ext = skb->extensions; > + > +               if (ext) > +                       refcount_inc(&ext->refcnt); > +       } > +} > + > +static inline void __skb_ext_copy(struct sk_buff *dst, > +                                 const struct sk_buff *src) > +{ > +       dst->active_extensions = src->active_extensions; > + > +       if (src->active_extensions) { > +               struct skb_ext *ext = src->extensions; > + > +               if (ext) > +                       refcount_inc(&ext->refcnt); > +               dst->extensions = ext; > +       } > +} > + > +static inline void skb_ext_copy(struct sk_buff *dst, const struct sk_buff *src) > +{ > +       skb_ext_put(dst); > +       __skb_ext_copy(dst, src); > +} > + > +static inline bool __skb_ext_exist(const struct skb_ext *ext, enum skb_ext_id i) > +{ > +       return !!ext->offset[i]; > +} > + > +static inline bool skb_ext_exist(const struct sk_buff *skb, enum skb_ext_id id) > +{ > +       return skb->active_extensions & (1 << id); > +} > + > +static inline void skb_ext_del(struct sk_buff *skb, enum skb_ext_id id) > +{ > +       if (skb_ext_exist(skb, id)) > +               __skb_ext_del(skb, id); > +} > + > +static inline void *skb_ext_find(const struct sk_buff *skb, enum skb_ext_id id) > +{ > +       if (skb_ext_exist(skb, id)) { > +               struct skb_ext *ext = skb->extensions; > + > +               if (ext && __skb_ext_exist(ext, id)) > +                       return (void *)ext + (ext->offset[id] << 3); > +       } > + > +       return NULL; > +} > +#else > +static inline void skb_ext_put(struct sk_buff *skb) {} > +static inline void skb_ext_get(struct sk_buff *skb) {} > +static inline void skb_ext_del(struct sk_buff *skb, int unused) {} > +static inline void __skb_ext_copy(struct sk_buff *d, const struct sk_buff *s) {} > +static inline void skb_ext_copy(struct sk_buff *dst, const struct sk_buff *s) {} For the !CONFIG_SKB_EXTENSIONS case, an alternate definition of skb_ext_exist() that always returns false would be useful to reduce the need for preprocessor conditionals. A similar skb_ext_find() that always returns NULL might also be helpful. > +#endif /* CONFIG_SKB_EXTENSIONS */ > + >  #if IS_ENABLED(CONFIG_BRIDGE_NETFILTER) >  static inline void nf_bridge_put(struct nf_bridge_info *nf_bridge) >  { Thanks, --0-119641190-1544573195=:20227--