[PATCH RFC 1/9] sk_buff: track nfct status in newly added skb->_state

[PATCH RFC 1/9] sk_buff: track nfct status in newly added skb->_state

[Paolo Abeni]

so that we can skip initizialzing such field at skb
allocation and move such field after 'tail'.

_state uses one byte hole in the header section.

Signed-off-by: Paolo Abeni <pabeni@redhat.com>
---
v1 -> v2:
 - : NULL
 - has_nfct = !!nfct -> ovs uses skb_set_nfct(NULL, 0) to clear skb->_nfct

should skb_nfct()/skb_get_nfct() return IP_CT_UNTRACKED
if SKB_HAS_NFCT is not set?
---
 include/linux/skbuff.h | 19 ++++++++++++++-----
 1 file changed, 14 insertions(+), 5 deletions(-)

diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index f19190820e63..ec3d34d8022f 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -689,6 +689,8 @@ typedef unsigned char *sk_buff_data_t;
  *		CHECKSUM_UNNECESSARY (max 3)
  *	@dst_pending_confirm: need to confirm neighbour
  *	@decrypted: Decrypted SKB
+ *	@_state: bitmap reporting the presence of some skb state info
+ *	@has_nfct: @_state bit for nfct info
  *	@napi_id: id of the NAPI struct this skb came from
  *	@sender_cpu: (aka @napi_id) source CPU in XPS
  *	@secmark: security marking
@@ -765,9 +767,6 @@ struct sk_buff {
 #endif
 	};
 
-#if defined(CONFIG_NF_CONNTRACK) || defined(CONFIG_NF_CONNTRACK_MODULE)
-	unsigned long		 _nfct;
-#endif
 	unsigned int		len,
 				data_len;
 	__u16			mac_len,
@@ -870,6 +869,12 @@ struct sk_buff {
 #ifdef CONFIG_TLS_DEVICE
 	__u8			decrypted:1;
 #endif
+	union {
+		__u8		_state;		/* state of extended fields */
+		struct {
+			__u8	has_nfct:1;
+		};
+	};
 
 #ifdef CONFIG_NET_SCHED
 	__u16			tc_index;	/* traffic control index */
@@ -936,6 +941,9 @@ struct sk_buff {
 	/* only useable after checking ->active_extensions != 0 */
 	struct skb_ext		*extensions;
 #endif
+#if defined(CONFIG_NF_CONNTRACK) || defined(CONFIG_NF_CONNTRACK_MODULE)
+	unsigned long		 _nfct;
+#endif
 };
 
 #ifdef __KERNEL__
@@ -4198,7 +4206,7 @@ static inline void skb_remcsum_process(struct sk_buff *skb, void *ptr,
 static inline struct nf_conntrack *skb_nfct(const struct sk_buff *skb)
 {
 #if IS_ENABLED(CONFIG_NF_CONNTRACK)
-	return (void *)(skb->_nfct & NFCT_PTRMASK);
+	return skb->has_nfct ? (void *)(skb->_nfct & NFCT_PTRMASK) : NULL;
 #else
 	return NULL;
 #endif
@@ -4207,7 +4215,7 @@ static inline struct nf_conntrack *skb_nfct(const struct sk_buff *skb)
 static inline unsigned long skb_get_nfct(const struct sk_buff *skb)
 {
 #if IS_ENABLED(CONFIG_NF_CONNTRACK)
-	return skb->_nfct;
+	return skb->has_nfct ? skb->_nfct : 0;
 #else
 	return 0UL;
 #endif
@@ -4216,6 +4224,7 @@ static inline unsigned long skb_get_nfct(const struct sk_buff *skb)
 static inline void skb_set_nfct(struct sk_buff *skb, unsigned long nfct)
 {
 #if IS_ENABLED(CONFIG_NF_CONNTRACK)
+	skb->has_nfct = !!nfct;
 	skb->_nfct = nfct;
 #endif
 }
-- 
2.26.3

[PATCH RFC 2/9] sk_buff: track dst status in skb->_state

[Paolo Abeni]

Similar to the previous patch, covering the dst field,
but limited to tracking only the dst status.

Signed-off-by: Paolo Abeni <pabeni@redhat.com>
---
 include/linux/skbuff.h | 4 ++++
 include/net/dst.h      | 3 +++
 2 files changed, 7 insertions(+)

diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index ec3d34d8022f..1b811585f6fc 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -691,6 +691,7 @@ typedef unsigned char *sk_buff_data_t;
  *	@decrypted: Decrypted SKB
  *	@_state: bitmap reporting the presence of some skb state info
  *	@has_nfct: @_state bit for nfct info
+ *	@has_dst: @_state bit for dst pointer
  *	@napi_id: id of the NAPI struct this skb came from
  *	@sender_cpu: (aka @napi_id) source CPU in XPS
  *	@secmark: security marking
@@ -873,6 +874,7 @@ struct sk_buff {
 		__u8		_state;		/* state of extended fields */
 		struct {
 			__u8	has_nfct:1;
+			__u8	has_dst:1;
 		};
 	};
 
@@ -998,6 +1000,7 @@ static inline struct dst_entry *skb_dst(const struct sk_buff *skb)
  */
 static inline void skb_dst_set(struct sk_buff *skb, struct dst_entry *dst)
 {
+	skb->has_dst = !!dst;
 	skb->_skb_refdst = (unsigned long)dst;
 }
 
@@ -1014,6 +1017,7 @@ static inline void skb_dst_set(struct sk_buff *skb, struct dst_entry *dst)
 static inline void skb_dst_set_noref(struct sk_buff *skb, struct dst_entry *dst)
 {
 	WARN_ON(!rcu_read_lock_held() && !rcu_read_lock_bh_held());
+	skb->has_dst = !!dst;
 	skb->_skb_refdst = (unsigned long)dst | SKB_DST_NOREF;
 }
 
diff --git a/include/net/dst.h b/include/net/dst.h
index 75b1e734e9c2..2cb765dabc6f 100644
--- a/include/net/dst.h
+++ b/include/net/dst.h
@@ -272,11 +272,13 @@ static inline void skb_dst_drop(struct sk_buff *skb)
 	if (skb->_skb_refdst) {
 		refdst_drop(skb->_skb_refdst);
 		skb->_skb_refdst = 0UL;
+		skb->has_dst = 0;
 	}
 }
 
 static inline void __skb_dst_copy(struct sk_buff *nskb, unsigned long refdst)
 {
+	nskb->has_dst = !!refdst;
 	nskb->_skb_refdst = refdst;
 	if (!(nskb->_skb_refdst & SKB_DST_NOREF))
 		dst_clone(skb_dst(nskb));
@@ -316,6 +318,7 @@ static inline bool skb_dst_force(struct sk_buff *skb)
 			dst = NULL;
 
 		skb->_skb_refdst = (unsigned long)dst;
+		skb->has_dst = !!dst;
 	}
 
 	return skb->_skb_refdst != 0UL;
-- 
2.26.3

[PATCH RFC 3/9] sk_buff: move the active_extensions into the state bitfield

[Paolo Abeni]

No functional change intended

Signed-off-by: Paolo Abeni <pabeni@redhat.com>
---
v1 -> v2:
 - add CHECK_SKB_FIELD(_state) in __copy_skb_header
2 problems:
- this restrict the storage for new skb extensions to 0 or at most 1
- can't provide a build time check to ensure SKB_EXT do not exceed
  active_extensions

I'm wondering about moving 2 random bits from the header section to
the old active_extensions location (and explicitly copy them on clone)
so that we can keep using 1 byte for extension and 1 byte for other
state things
---
 include/linux/skbuff.h | 11 +++++------
 net/core/skbuff.c      |  1 +
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 1b811585f6fc..03be9a774c58 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -670,7 +670,6 @@ typedef unsigned char *sk_buff_data_t;
  *	@pfmemalloc: skbuff was allocated from PFMEMALLOC reserves
  *	@pp_recycle: mark the packet for recycling instead of freeing (implies
  *		page_pool support on driver)
- *	@active_extensions: active extensions (skb_ext_id types)
  *	@ndisc_nodetype: router type (from link layer)
  *	@ooo_okay: allow the mapping of a socket to a queue to be changed
  *	@l4_hash: indicate hash is a canonical 4-tuple hash over transport
@@ -692,6 +691,7 @@ typedef unsigned char *sk_buff_data_t;
  *	@_state: bitmap reporting the presence of some skb state info
  *	@has_nfct: @_state bit for nfct info
  *	@has_dst: @_state bit for dst pointer
+ *	@active_extensions: @_state bits for active extensions (skb_ext_id types)
  *	@napi_id: id of the NAPI struct this skb came from
  *	@sender_cpu: (aka @napi_id) source CPU in XPS
  *	@secmark: security marking
@@ -796,9 +796,6 @@ struct sk_buff {
 				head_frag:1,
 				pfmemalloc:1,
 				pp_recycle:1; /* page_pool recycle indicator */
-#ifdef CONFIG_SKB_EXTENSIONS
-	__u8			active_extensions;
-#endif
 
 	/* fields enclosed in headers_start/headers_end are copied
 	 * using a single memcpy() in __copy_skb_header()
@@ -875,6 +872,9 @@ struct sk_buff {
 		struct {
 			__u8	has_nfct:1;
 			__u8	has_dst:1;
+#ifdef CONFIG_SKB_EXTENSIONS
+			__u8	active_extensions:5;
+#endif
 		};
 	};
 
@@ -4283,8 +4283,6 @@ static inline void skb_ext_put(struct sk_buff *skb)
 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;
 
@@ -4296,6 +4294,7 @@ static inline void __skb_ext_copy(struct sk_buff *dst,
 static inline void skb_ext_copy(struct sk_buff *dst, const struct sk_buff *src)
 {
 	skb_ext_put(dst);
+	dst->active_extensions = src->active_extensions;
 	__skb_ext_copy(dst, src);
 }
 
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index e94805bd8656..2ffe18595635 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -1001,6 +1001,7 @@ static void __copy_skb_header(struct sk_buff *new, const struct sk_buff *old)
 	memcpy(&new->headers_start, &old->headers_start,
 	       offsetof(struct sk_buff, headers_end) -
 	       offsetof(struct sk_buff, headers_start));
+	CHECK_SKB_FIELD(_state);
 	CHECK_SKB_FIELD(protocol);
 	CHECK_SKB_FIELD(csum);
 	CHECK_SKB_FIELD(hash);
-- 
2.26.3

[PATCH RFC 4/9] net: optimize GRO for the common case.

[Paolo Abeni]

After the previous patches, at GRO time, skb->_state is
usually 0, unless the packets comes from some H/W offload
slowpath or tunnel without rx checksum offload.

We can optimize the GRO code assuming !skb->_state is likely.
This remove multiple conditionals in the fast-path, at the
price of an additional one when we hit the above "slow-paths".

Signed-off-by: Paolo Abeni <pabeni@redhat.com>
---
 net/core/dev.c    | 29 +++++++++++++++++++++--------
 net/core/skbuff.c |  8 +++++---
 2 files changed, 26 insertions(+), 11 deletions(-)

diff --git a/net/core/dev.c b/net/core/dev.c
index 3ee58876e8f5..70c24ed9ca67 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -6002,7 +6002,6 @@ static void gro_list_prepare(const struct list_head *head,
 		diffs |= skb_vlan_tag_present(p) ^ skb_vlan_tag_present(skb);
 		if (skb_vlan_tag_present(p))
 			diffs |= skb_vlan_tag_get(p) ^ skb_vlan_tag_get(skb);
-		diffs |= skb_metadata_dst_cmp(p, skb);
 		diffs |= skb_metadata_differs(p, skb);
 		if (maclen == ETH_HLEN)
 			diffs |= compare_ether_header(skb_mac_header(p),
@@ -6012,17 +6011,29 @@ static void gro_list_prepare(const struct list_head *head,
 				       skb_mac_header(skb),
 				       maclen);
 
-		diffs |= skb_get_nfct(p) ^ skb_get_nfct(skb);
+		/* in most common scenarions _state is 0
+		 * otherwise we are already on some slower paths
+		 * either skip all the infrequent tests altogether or
+		 * avoid trying too hard to skip each of them individually
+		 */
+		if (!diffs && unlikely(skb->_state | p->_state)) {
+#if IS_ENABLED(CONFIG_SKB_EXTENSIONS) && IS_ENABLED(CONFIG_NET_TC_SKB_EXT)
+			struct tc_skb_ext *skb_ext;
+			struct tc_skb_ext *p_ext;
+#endif
+
+			diffs |= skb_metadata_dst_cmp(p, skb);
+			diffs |= skb_get_nfct(p) ^ skb_get_nfct(skb);
+
 #if IS_ENABLED(CONFIG_SKB_EXTENSIONS) && IS_ENABLED(CONFIG_NET_TC_SKB_EXT)
-		if (!diffs) {
-			struct tc_skb_ext *skb_ext = skb_ext_find(skb, TC_SKB_EXT);
-			struct tc_skb_ext *p_ext = skb_ext_find(p, TC_SKB_EXT);
+			skb_ext = skb_ext_find(skb, TC_SKB_EXT);
+			p_ext = skb_ext_find(p, TC_SKB_EXT);
 
 			diffs |= (!!p_ext) ^ (!!skb_ext);
 			if (!diffs && unlikely(skb_ext))
 				diffs |= p_ext->chain ^ skb_ext->chain;
-		}
 #endif
+		}
 
 		NAPI_GRO_CB(p)->same_flow = !diffs;
 	}
@@ -6287,8 +6298,10 @@ static void napi_reuse_skb(struct napi_struct *napi, struct sk_buff *skb)
 	skb->encapsulation = 0;
 	skb_shinfo(skb)->gso_type = 0;
 	skb->truesize = SKB_TRUESIZE(skb_end_offset(skb));
-	skb_ext_reset(skb);
-	nf_reset_ct(skb);
+	if (unlikely(skb->_state)) {
+		skb_ext_reset(skb);
+		nf_reset_ct(skb);
+	}
 
 	napi->skb = skb;
 }
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index 2ffe18595635..befb49d1a756 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -943,9 +943,11 @@ void __kfree_skb_defer(struct sk_buff *skb)
 
 void napi_skb_free_stolen_head(struct sk_buff *skb)
 {
-	nf_reset_ct(skb);
-	skb_dst_drop(skb);
-	skb_ext_put(skb);
+	if (unlikely(skb->_state)) {
+		nf_reset_ct(skb);
+		skb_dst_drop(skb);
+		skb_ext_put(skb);
+	}
 	napi_skb_cache_put(skb);
 }
 
-- 
2.26.3

[PATCH RFC 5/9] skbuff: introduce has_sk state bit.

[Paolo Abeni]

This change leverages the infrastructure introduced by the previous
patches to allow soft devices passing to the GRO engine owned skbs
without impacting the fast-path.

It's up to the GRO caller ensuring the bit validity before
invoking the GRO engine with the new helper skb_prepare_for_gro().

If the bit is set only skb with equal sk will be aggregated.
Additionally, skb truesize on GRO recycle and free is correctly
updated so that sk wmem is not changed by the GRO processing.

Signed-off-by: Paolo Abeni <pabeni@redhat.com>
---
 include/linux/skbuff.h |  2 ++
 include/net/sock.h     |  9 +++++++++
 net/core/dev.c         |  2 ++
 net/core/skbuff.c      | 13 +++++++++++--
 4 files changed, 24 insertions(+), 2 deletions(-)

diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 03be9a774c58..ea9fdcc7c7ca 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -691,6 +691,7 @@ typedef unsigned char *sk_buff_data_t;
  *	@_state: bitmap reporting the presence of some skb state info
  *	@has_nfct: @_state bit for nfct info
  *	@has_dst: @_state bit for dst pointer
+ *	@has_sk: @_state bit for sk pointer, only relevant at GRO time
  *	@active_extensions: @_state bits for active extensions (skb_ext_id types)
  *	@napi_id: id of the NAPI struct this skb came from
  *	@sender_cpu: (aka @napi_id) source CPU in XPS
@@ -872,6 +873,7 @@ struct sk_buff {
 		struct {
 			__u8	has_nfct:1;
 			__u8	has_dst:1;
+			__u8	has_sk:1;
 #ifdef CONFIG_SKB_EXTENSIONS
 			__u8	active_extensions:5;
 #endif
diff --git a/include/net/sock.h b/include/net/sock.h
index f23cb259b0e2..c1f2d896794b 100644
--- a/include/net/sock.h
+++ b/include/net/sock.h
@@ -2249,6 +2249,15 @@ static inline __must_check bool skb_set_owner_sk_safe(struct sk_buff *skb, struc
 	return false;
 }
 
+static inline void skb_prepare_for_gro(struct sk_buff *skb)
+{
+	if (skb->destructor != sock_wfree) {
+		skb_orphan(skb);
+		return;
+	}
+	skb->has_sk = 1;
+}
+
 void sk_reset_timer(struct sock *sk, struct timer_list *timer,
 		    unsigned long expires);
 
diff --git a/net/core/dev.c b/net/core/dev.c
index 70c24ed9ca67..2ef087958fc9 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -6022,6 +6022,7 @@ static void gro_list_prepare(const struct list_head *head,
 			struct tc_skb_ext *p_ext;
 #endif
 
+			diffs |= p->sk != skb->sk;
 			diffs |= skb_metadata_dst_cmp(p, skb);
 			diffs |= skb_get_nfct(p) ^ skb_get_nfct(skb);
 
@@ -6299,6 +6300,7 @@ static void napi_reuse_skb(struct napi_struct *napi, struct sk_buff *skb)
 	skb_shinfo(skb)->gso_type = 0;
 	skb->truesize = SKB_TRUESIZE(skb_end_offset(skb));
 	if (unlikely(skb->_state)) {
+		skb_orphan(skb);
 		skb_ext_reset(skb);
 		nf_reset_ct(skb);
 	}
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index befb49d1a756..9ed754da6e13 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -947,6 +947,7 @@ void napi_skb_free_stolen_head(struct sk_buff *skb)
 		nf_reset_ct(skb);
 		skb_dst_drop(skb);
 		skb_ext_put(skb);
+		skb_orphan(skb);
 	}
 	napi_skb_cache_put(skb);
 }
@@ -3884,6 +3885,9 @@ int skb_gro_receive_list(struct sk_buff *p, struct sk_buff *skb)
 	NAPI_GRO_CB(p)->last = skb;
 	NAPI_GRO_CB(p)->count++;
 	p->data_len += skb->len;
+
+	/* sk owenrship - if any - completely transferred to the aggregated packet */
+	skb->destructor = NULL;
 	p->truesize += skb->truesize;
 	p->len += skb->len;
 
@@ -4285,7 +4289,7 @@ int skb_gro_receive(struct sk_buff *p, struct sk_buff *skb)
 		delta_truesize = skb->truesize -
 				 SKB_TRUESIZE(skb_end_offset(skb));
 
-		skb->truesize -= skb->data_len;
+		/* napi_reuse_skb() will always re-init 'truesize' */
 		skb->len -= skb->data_len;
 		skb->data_len = 0;
 
@@ -4297,6 +4301,7 @@ int skb_gro_receive(struct sk_buff *p, struct sk_buff *skb)
 		struct page *page = virt_to_head_page(skb->head);
 		unsigned int first_size = headlen - offset;
 		unsigned int first_offset;
+		unsigned int new_truesize;
 
 		if (nr_frags + 1 + skbinfo->nr_frags > MAX_SKB_FRAGS)
 			goto merge;
@@ -4314,12 +4319,16 @@ int skb_gro_receive(struct sk_buff *p, struct sk_buff *skb)
 		memcpy(frag + 1, skbinfo->frags, sizeof(*frag) * skbinfo->nr_frags);
 		/* We dont need to clear skbinfo->nr_frags here */
 
-		delta_truesize = skb->truesize - SKB_DATA_ALIGN(sizeof(struct sk_buff));
+		new_truesize = SKB_TRUESIZE(sizeof(struct sk_buff));
+		delta_truesize = skb->truesize - new_truesize;
+		skb->truesize = new_truesize;
 		NAPI_GRO_CB(skb)->free = NAPI_GRO_FREE_STOLEN_HEAD;
 		goto done;
 	}
 
 merge:
+	/* sk owenrship - if any - completely transferred to the aggregated packet */
+	skb->destructor = NULL;
 	delta_truesize = skb->truesize;
 	if (offset > headlen) {
 		unsigned int eat = offset - headlen;
-- 
2.26.3

[PATCH RFC 6/9] veth: use skb_prepare_for_gro()

[Paolo Abeni]

Leveraging the previous patch we can now avoid orphaning the
skb in the veth gro path, allowing correct backpressure.

Signed-off-by: Paolo Abeni <pabeni@redhat.com>
---
 drivers/net/veth.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/veth.c b/drivers/net/veth.c
index 381670c08ba7..50eb43e5bf45 100644
--- a/drivers/net/veth.c
+++ b/drivers/net/veth.c
@@ -713,7 +713,7 @@ static struct sk_buff *veth_xdp_rcv_skb(struct veth_rq *rq,
 	int mac_len, delta, off;
 	struct xdp_buff xdp;
 
-	skb_orphan_partial(skb);
+	skb_prepare_for_gro(skb);
 
 	rcu_read_lock();
 	xdp_prog = rcu_dereference(rq->xdp_prog);
-- 
2.26.3

[PATCH RFC 7/9] sk_buff: move inner header fields after tail

[Paolo Abeni]

all the inner header fields are valid only if the 'encaspulation'
flag is set, and the relevant fields are always initialized when
the field is set: we don't need to initialize them at skb allocation
time

Signed-off-by: Paolo Abeni <pabeni@redhat.com>
---
 v1 -> v2:
  - add CHECK_SKB_FIELD(__encapsulation_offset) in __copy_skb_header
---
 include/linux/skbuff.h | 31 ++++++++++++++++++++++---------
 net/core/skbuff.c      |  6 ++----
 2 files changed, 24 insertions(+), 13 deletions(-)

diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index ea9fdcc7c7ca..a3e756575aa7 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -822,6 +822,9 @@ struct sk_buff {
 	__u8			ip_summed:2;
 	__u8			ooo_okay:1;
 
+	/* private: */
+	__u8			__pkt_encapsulation_offset[0];
+	/* public: */
 	__u8			l4_hash:1;
 	__u8			sw_hash:1;
 	__u8			wifi_acked_valid:1;
@@ -911,15 +914,6 @@ struct sk_buff {
 		__u32		reserved_tailroom;
 	};
 
-	union {
-		__be16		inner_protocol;
-		__u8		inner_ipproto;
-	};
-
-	__u16			inner_transport_header;
-	__u16			inner_network_header;
-	__u16			inner_mac_header;
-
 	__be16			protocol;
 	__u16			transport_header;
 	__u16			network_header;
@@ -948,6 +942,19 @@ struct sk_buff {
 #if defined(CONFIG_NF_CONNTRACK) || defined(CONFIG_NF_CONNTRACK_MODULE)
 	unsigned long		 _nfct;
 #endif
+	union {
+		struct {
+			union {
+			__be16	inner_protocol;
+			__u8	inner_ipproto;
+			};
+
+			__u16	inner_transport_header;
+			__u16	inner_network_header;
+			__u16	inner_mac_header;
+		};
+		__u64		inner_headers;
+	};
 };
 
 #ifdef __KERNEL__
@@ -2449,6 +2456,12 @@ static inline void skb_tailroom_reserve(struct sk_buff *skb, unsigned int mtu,
 #define ENCAP_TYPE_ETHER	0
 #define ENCAP_TYPE_IPPROTO	1
 
+static inline void __skb_copy_inner_headers(struct sk_buff *dst, const struct sk_buff *src)
+{
+	if (src->encapsulation)
+		dst->inner_headers = src->inner_headers;
+}
+
 static inline void skb_set_inner_protocol(struct sk_buff *skb,
 					  __be16 protocol)
 {
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index 9ed754da6e13..53b8db10e567 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -995,6 +995,7 @@ static void __copy_skb_header(struct sk_buff *new, const struct sk_buff *old)
 	skb_dst_copy(new, old);
 	__skb_ext_copy(new, old);
 	__nf_copy(new, old, false);
+	__skb_copy_inner_headers(new, old);
 
 	/* Note : this field could be in headers_start/headers_end section
 	 * It is not yet because we do not want to have a 16 bit hole
@@ -1005,6 +1006,7 @@ static void __copy_skb_header(struct sk_buff *new, const struct sk_buff *old)
 	       offsetof(struct sk_buff, headers_end) -
 	       offsetof(struct sk_buff, headers_start));
 	CHECK_SKB_FIELD(_state);
+	CHECK_SKB_FIELD(__pkt_encapsulation_offset);
 	CHECK_SKB_FIELD(protocol);
 	CHECK_SKB_FIELD(csum);
 	CHECK_SKB_FIELD(hash);
@@ -1015,10 +1017,6 @@ static void __copy_skb_header(struct sk_buff *new, const struct sk_buff *old)
 	CHECK_SKB_FIELD(transport_header);
 	CHECK_SKB_FIELD(network_header);
 	CHECK_SKB_FIELD(mac_header);
-	CHECK_SKB_FIELD(inner_protocol);
-	CHECK_SKB_FIELD(inner_transport_header);
-	CHECK_SKB_FIELD(inner_network_header);
-	CHECK_SKB_FIELD(inner_mac_header);
 	CHECK_SKB_FIELD(mark);
 #ifdef CONFIG_NETWORK_SECMARK
 	CHECK_SKB_FIELD(secmark);
-- 
2.26.3

[PATCH RFC 8/9] sk_buff: move vlan field after tail.

[Paolo Abeni]

Such field validity is already tracked by the existing
'vlan_present' bit. Move them after tail and conditinally copy
as needed.

Signed-off-by: Paolo Abeni <pabeni@redhat.com>
---
 include/linux/skbuff.h | 10 ++++++++--
 net/core/skbuff.c      |  5 +++--
 2 files changed, 11 insertions(+), 4 deletions(-)

diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index a3e756575aa7..7acf2a203918 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -897,8 +897,6 @@ struct sk_buff {
 	__u32			priority;
 	int			skb_iif;
 	__u32			hash;
-	__be16			vlan_proto;
-	__u16			vlan_tci;
 #if defined(CONFIG_NET_RX_BUSY_POLL) || defined(CONFIG_XPS)
 	union {
 		unsigned int	napi_id;
@@ -955,6 +953,14 @@ struct sk_buff {
 		};
 		__u64		inner_headers;
 	};
+
+	union {
+		struct {
+			__be16	vlan_proto;
+			__u16	vlan_tci;
+		};
+		__u32		vlan_info;
+	};
 };
 
 #ifdef __KERNEL__
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index 53b8db10e567..c59e90db80d5 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -996,6 +996,8 @@ static void __copy_skb_header(struct sk_buff *new, const struct sk_buff *old)
 	__skb_ext_copy(new, old);
 	__nf_copy(new, old, false);
 	__skb_copy_inner_headers(new, old);
+	if (old->vlan_present)
+		new->vlan_info = old->vlan_info;
 
 	/* Note : this field could be in headers_start/headers_end section
 	 * It is not yet because we do not want to have a 16 bit hole
@@ -1007,13 +1009,12 @@ static void __copy_skb_header(struct sk_buff *new, const struct sk_buff *old)
 	       offsetof(struct sk_buff, headers_start));
 	CHECK_SKB_FIELD(_state);
 	CHECK_SKB_FIELD(__pkt_encapsulation_offset);
+	CHECK_SKB_FIELD(__pkt_vlan_present_offset);
 	CHECK_SKB_FIELD(protocol);
 	CHECK_SKB_FIELD(csum);
 	CHECK_SKB_FIELD(hash);
 	CHECK_SKB_FIELD(priority);
 	CHECK_SKB_FIELD(skb_iif);
-	CHECK_SKB_FIELD(vlan_proto);
-	CHECK_SKB_FIELD(vlan_tci);
 	CHECK_SKB_FIELD(transport_header);
 	CHECK_SKB_FIELD(network_header);
 	CHECK_SKB_FIELD(mac_header);
-- 
2.26.3

[PATCH RFC 9/9] sk_buff: access secmark via getter/setter

[Paolo Abeni]

So we can track the field status and move it after tail.

After this commit the skb lifecycle for simple cases (no ct, no secmark,
no vlan, no UDP tunnel) uses 3 cacheline instead of 4 cachelines required
before this series.

e.g. GRO for non vlan traffic will consistently uses 3 cacheline for
each packet.

Signed-off-by: Paolo Abeni <pabeni@redhat.com>
---
 include/linux/skbuff.h           | 40 ++++++++++++++++++++++----------
 net/core/skbuff.c                |  7 +++---
 net/netfilter/nfnetlink_queue.c  |  6 +++--
 net/netfilter/nft_meta.c         |  6 ++---
 net/netfilter/xt_CONNSECMARK.c   |  8 +++----
 net/netfilter/xt_SECMARK.c       |  2 +-
 security/apparmor/lsm.c          | 15 +++++++-----
 security/selinux/hooks.c         | 10 ++++----
 security/smack/smack_lsm.c       |  4 ++--
 security/smack/smack_netfilter.c |  4 ++--
 10 files changed, 62 insertions(+), 40 deletions(-)

diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 7acf2a203918..941c0f858c65 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -688,6 +688,7 @@ typedef unsigned char *sk_buff_data_t;
  *		CHECKSUM_UNNECESSARY (max 3)
  *	@dst_pending_confirm: need to confirm neighbour
  *	@decrypted: Decrypted SKB
+ *	@secmark_present: the secmark tag is present
  *	@_state: bitmap reporting the presence of some skb state info
  *	@has_nfct: @_state bit for nfct info
  *	@has_dst: @_state bit for dst pointer
@@ -695,7 +696,7 @@ typedef unsigned char *sk_buff_data_t;
  *	@active_extensions: @_state bits for active extensions (skb_ext_id types)
  *	@napi_id: id of the NAPI struct this skb came from
  *	@sender_cpu: (aka @napi_id) source CPU in XPS
- *	@secmark: security marking
+ *	@_secmark: security marking
  *	@mark: Generic packet mark
  *	@reserved_tailroom: (aka @mark) number of bytes of free space available
  *		at the tail of an sk_buff
@@ -870,6 +871,9 @@ struct sk_buff {
 #endif
 #ifdef CONFIG_TLS_DEVICE
 	__u8			decrypted:1;
+#endif
+#ifdef CONFIG_NETWORK_SECMARK
+	__u8			secmark_present:1;
 #endif
 	union {
 		__u8		_state;		/* state of extended fields */
@@ -903,9 +907,6 @@ struct sk_buff {
 		unsigned int	sender_cpu;
 	};
 #endif
-#ifdef CONFIG_NETWORK_SECMARK
-	__u32		secmark;
-#endif
 
 	union {
 		__u32		mark;
@@ -961,6 +962,9 @@ struct sk_buff {
 		};
 		__u32		vlan_info;
 	};
+#ifdef CONFIG_NETWORK_SECMARK
+	__u32			_secmark;
+#endif
 };
 
 #ifdef __KERNEL__
@@ -4228,6 +4232,23 @@ static inline void skb_remcsum_process(struct sk_buff *skb, void *ptr,
 	skb->csum = csum_add(skb->csum, delta);
 }
 
+static inline __u32 skb_secmark(const struct sk_buff *skb)
+{
+#if IS_ENABLED(CONFIG_NETWORK_SECMARK)
+	return skb->secmark_present ? skb->_secmark : 0;
+#else
+	return NULL;
+#endif
+}
+
+static inline void skb_set_secmark(struct sk_buff *skb, __u32 secmark)
+{
+#if IS_ENABLED(CONFIG_NETWORK_SECMARK)
+	skb->secmark_present = 1;
+	skb->_secmark = secmark;
+#endif
+}
+
 static inline struct nf_conntrack *skb_nfct(const struct sk_buff *skb)
 {
 #if IS_ENABLED(CONFIG_NF_CONNTRACK)
@@ -4414,19 +4435,14 @@ static inline void nf_copy(struct sk_buff *dst, const struct sk_buff *src)
 #ifdef CONFIG_NETWORK_SECMARK
 static inline void skb_copy_secmark(struct sk_buff *to, const struct sk_buff *from)
 {
-	to->secmark = from->secmark;
-}
-
-static inline void skb_init_secmark(struct sk_buff *skb)
-{
-	skb->secmark = 0;
+	to->secmark_present = from->secmark_present;
+	if (from->_secmark)
+		to->_secmark = from->_secmark;
 }
 #else
 static inline void skb_copy_secmark(struct sk_buff *to, const struct sk_buff *from)
 { }
 
-static inline void skb_init_secmark(struct sk_buff *skb)
-{ }
 #endif
 
 static inline int secpath_exists(const struct sk_buff *skb)
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index c59e90db80d5..704aecbde60d 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -998,6 +998,10 @@ static void __copy_skb_header(struct sk_buff *new, const struct sk_buff *old)
 	__skb_copy_inner_headers(new, old);
 	if (old->vlan_present)
 		new->vlan_info = old->vlan_info;
+#ifdef CONFIG_NETWORK_SECMARK
+	if (old->_secmark)
+		new->_secmark = old->_secmark;
+#endif
 
 	/* Note : this field could be in headers_start/headers_end section
 	 * It is not yet because we do not want to have a 16 bit hole
@@ -1019,9 +1023,6 @@ static void __copy_skb_header(struct sk_buff *new, const struct sk_buff *old)
 	CHECK_SKB_FIELD(network_header);
 	CHECK_SKB_FIELD(mac_header);
 	CHECK_SKB_FIELD(mark);
-#ifdef CONFIG_NETWORK_SECMARK
-	CHECK_SKB_FIELD(secmark);
-#endif
 #ifdef CONFIG_NET_RX_BUSY_POLL
 	CHECK_SKB_FIELD(napi_id);
 #endif
diff --git a/net/netfilter/nfnetlink_queue.c b/net/netfilter/nfnetlink_queue.c
index f774de0fc24f..cf00d4286187 100644
--- a/net/netfilter/nfnetlink_queue.c
+++ b/net/netfilter/nfnetlink_queue.c
@@ -304,14 +304,16 @@ static int nfqnl_put_sk_uidgid(struct sk_buff *skb, struct sock *sk)
 static u32 nfqnl_get_sk_secctx(struct sk_buff *skb, char **secdata)
 {
 	u32 seclen = 0;
+	u32 secmark;
 #if IS_ENABLED(CONFIG_NETWORK_SECMARK)
 	if (!skb || !sk_fullsock(skb->sk))
 		return 0;
 
 	read_lock_bh(&skb->sk->sk_callback_lock);
 
-	if (skb->secmark)
-		security_secid_to_secctx(skb->secmark, secdata, &seclen);
+	secmark = skb_secmark(skb);
+	if (secmark)
+		security_secid_to_secctx(secmark, secdata, &seclen);
 
 	read_unlock_bh(&skb->sk->sk_callback_lock);
 #endif
diff --git a/net/netfilter/nft_meta.c b/net/netfilter/nft_meta.c
index a7e01e9952f1..da4bc455d8bd 100644
--- a/net/netfilter/nft_meta.c
+++ b/net/netfilter/nft_meta.c
@@ -363,7 +363,7 @@ void nft_meta_get_eval(const struct nft_expr *expr,
 #endif
 #ifdef CONFIG_NETWORK_SECMARK
 	case NFT_META_SECMARK:
-		*dest = skb->secmark;
+		*dest = skb_secmark(skb);
 		break;
 #endif
 	case NFT_META_PKTTYPE:
@@ -451,7 +451,7 @@ void nft_meta_set_eval(const struct nft_expr *expr,
 		break;
 #ifdef CONFIG_NETWORK_SECMARK
 	case NFT_META_SECMARK:
-		skb->secmark = value;
+		skb_set_secmark(skb, value);
 		break;
 #endif
 	default:
@@ -833,7 +833,7 @@ static void nft_secmark_obj_eval(struct nft_object *obj, struct nft_regs *regs,
 	const struct nft_secmark *priv = nft_obj_data(obj);
 	struct sk_buff *skb = pkt->skb;
 
-	skb->secmark = priv->secid;
+	skb_set_secmark(skb, priv->secid);
 }
 
 static int nft_secmark_obj_init(const struct nft_ctx *ctx,
diff --git a/net/netfilter/xt_CONNSECMARK.c b/net/netfilter/xt_CONNSECMARK.c
index 76acecf3e757..26f4fbc04c0b 100644
--- a/net/netfilter/xt_CONNSECMARK.c
+++ b/net/netfilter/xt_CONNSECMARK.c
@@ -31,13 +31,13 @@ MODULE_ALIAS("ip6t_CONNSECMARK");
  */
 static void secmark_save(const struct sk_buff *skb)
 {
-	if (skb->secmark) {
+	if (skb_secmark(skb)) {
 		struct nf_conn *ct;
 		enum ip_conntrack_info ctinfo;
 
 		ct = nf_ct_get(skb, &ctinfo);
 		if (ct && !ct->secmark) {
-			ct->secmark = skb->secmark;
+			ct->secmark = skb_secmark(skb);
 			nf_conntrack_event_cache(IPCT_SECMARK, ct);
 		}
 	}
@@ -49,13 +49,13 @@ static void secmark_save(const struct sk_buff *skb)
  */
 static void secmark_restore(struct sk_buff *skb)
 {
-	if (!skb->secmark) {
+	if (!skb_secmark(skb)) {
 		const struct nf_conn *ct;
 		enum ip_conntrack_info ctinfo;
 
 		ct = nf_ct_get(skb, &ctinfo);
 		if (ct && ct->secmark)
-			skb->secmark = ct->secmark;
+			skb_set_secmark(skb, ct->secmark);
 	}
 }
 
diff --git a/net/netfilter/xt_SECMARK.c b/net/netfilter/xt_SECMARK.c
index 498a0bf6f044..bc383bc2bba9 100644
--- a/net/netfilter/xt_SECMARK.c
+++ b/net/netfilter/xt_SECMARK.c
@@ -36,7 +36,7 @@ secmark_tg(struct sk_buff *skb, const struct xt_secmark_target_info_v1 *info)
 		BUG();
 	}
 
-	skb->secmark = secmark;
+	skb_set_secmark(skb, secmark);
 	return XT_CONTINUE;
 }
 
diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c
index f72406fe1bf2..afbae187b920 100644
--- a/security/apparmor/lsm.c
+++ b/security/apparmor/lsm.c
@@ -1053,12 +1053,13 @@ static int apparmor_socket_shutdown(struct socket *sock, int how)
 static int apparmor_socket_sock_rcv_skb(struct sock *sk, struct sk_buff *skb)
 {
 	struct aa_sk_ctx *ctx = SK_CTX(sk);
+	u32 secmark = skb_secmark(skb);
 
-	if (!skb->secmark)
+	if (!secmark)
 		return 0;
 
 	return apparmor_secmark_check(ctx->label, OP_RECVMSG, AA_MAY_RECEIVE,
-				      skb->secmark, sk);
+				      secmark, sk);
 }
 #endif
 
@@ -1160,12 +1161,13 @@ static int apparmor_inet_conn_request(const struct sock *sk, struct sk_buff *skb
 				      struct request_sock *req)
 {
 	struct aa_sk_ctx *ctx = SK_CTX(sk);
+	u32 secmark = skb_secmark(skb);
 
-	if (!skb->secmark)
+	if (!secmark)
 		return 0;
 
 	return apparmor_secmark_check(ctx->label, OP_CONNECT, AA_MAY_CONNECT,
-				      skb->secmark, sk);
+				      secmark, sk);
 }
 #endif
 
@@ -1754,10 +1756,11 @@ static unsigned int apparmor_ip_postroute(void *priv,
 					  struct sk_buff *skb,
 					  const struct nf_hook_state *state)
 {
+	u32 secmark = skb_secmark(skb);
 	struct aa_sk_ctx *ctx;
 	struct sock *sk;
 
-	if (!skb->secmark)
+	if (!secmark)
 		return NF_ACCEPT;
 
 	sk = skb_to_full_sk(skb);
@@ -1766,7 +1769,7 @@ static unsigned int apparmor_ip_postroute(void *priv,
 
 	ctx = SK_CTX(sk);
 	if (!apparmor_secmark_check(ctx->label, OP_SENDMSG, AA_MAY_SEND,
-				    skb->secmark, sk))
+				    secmark, sk))
 		return NF_ACCEPT;
 
 	return NF_DROP_ERR(-ECONNREFUSED);
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index b0032c42333e..898b81ba7566 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -5138,7 +5138,7 @@ static int selinux_sock_rcv_skb_compat(struct sock *sk, struct sk_buff *skb,
 
 	if (selinux_secmark_enabled()) {
 		err = avc_has_perm(&selinux_state,
-				   sk_sid, skb->secmark, SECCLASS_PACKET,
+				   sk_sid, skb_secmark(skb), SECCLASS_PACKET,
 				   PACKET__RECV, &ad);
 		if (err)
 			return err;
@@ -5214,7 +5214,7 @@ static int selinux_socket_sock_rcv_skb(struct sock *sk, struct sk_buff *skb)
 
 	if (secmark_active) {
 		err = avc_has_perm(&selinux_state,
-				   sk_sid, skb->secmark, SECCLASS_PACKET,
+				   sk_sid, skb_secmark(skb), SECCLASS_PACKET,
 				   PACKET__RECV, &ad);
 		if (err)
 			return err;
@@ -5727,7 +5727,7 @@ static unsigned int selinux_ip_forward(struct sk_buff *skb,
 
 	if (secmark_active)
 		if (avc_has_perm(&selinux_state,
-				 peer_sid, skb->secmark,
+				 peer_sid, skb_secmark(skb),
 				 SECCLASS_PACKET, PACKET__FORWARD_IN, &ad))
 			return NF_DROP;
 
@@ -5840,7 +5840,7 @@ static unsigned int selinux_ip_postroute_compat(struct sk_buff *skb,
 
 	if (selinux_secmark_enabled())
 		if (avc_has_perm(&selinux_state,
-				 sksec->sid, skb->secmark,
+				 sksec->sid, skb_secmark(skb),
 				 SECCLASS_PACKET, PACKET__SEND, &ad))
 			return NF_DROP_ERR(-ECONNREFUSED);
 
@@ -5964,7 +5964,7 @@ static unsigned int selinux_ip_postroute(struct sk_buff *skb,
 
 	if (secmark_active)
 		if (avc_has_perm(&selinux_state,
-				 peer_sid, skb->secmark,
+				 peer_sid, skb_secmark(skb),
 				 SECCLASS_PACKET, secmark_perm, &ad))
 			return NF_DROP_ERR(-ECONNREFUSED);
 
diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
index 223a6da0e6dc..2ed19e2db66a 100644
--- a/security/smack/smack_lsm.c
+++ b/security/smack/smack_lsm.c
@@ -3840,10 +3840,10 @@ static int smk_skb_to_addr_ipv6(struct sk_buff *skb, struct sockaddr_in6 *sip)
 #ifdef CONFIG_NETWORK_SECMARK
 static struct smack_known *smack_from_skb(struct sk_buff *skb)
 {
-	if (skb == NULL || skb->secmark == 0)
+	if (skb == NULL || skb_secmark(skb) == 0)
 		return NULL;
 
-	return smack_from_secid(skb->secmark);
+	return smack_from_secid(skb_secmark(skb));
 }
 #else
 static inline struct smack_known *smack_from_skb(struct sk_buff *skb)
diff --git a/security/smack/smack_netfilter.c b/security/smack/smack_netfilter.c
index fc7399b45373..881143e62eb4 100644
--- a/security/smack/smack_netfilter.c
+++ b/security/smack/smack_netfilter.c
@@ -31,7 +31,7 @@ static unsigned int smack_ipv6_output(void *priv,
 	if (sk && sk->sk_security) {
 		ssp = sk->sk_security;
 		skp = ssp->smk_out;
-		skb->secmark = skp->smk_secid;
+		skb_set_secmark(skb, skp->smk_secid);
 	}
 
 	return NF_ACCEPT;
@@ -49,7 +49,7 @@ static unsigned int smack_ipv4_output(void *priv,
 	if (sk && sk->sk_security) {
 		ssp = sk->sk_security;
 		skp = ssp->smk_out;
-		skb->secmark = skp->smk_secid;
+		skb_set_secmark(skb, skp->smk_secid);
 	}
 
 	return NF_ACCEPT;
-- 
2.26.3

Re: [PATCH RFC 9/9] sk_buff: access secmark via getter/setter

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 7658 bytes --]

Hi Paolo,

[FYI, it's a private test report for your RFC patch.]
[auto build test WARNING on nf/master]
[also build test WARNING on linus/master v5.14-rc2 next-20210721]
[cannot apply to nf-next/master]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Paolo-Abeni/sk_buff-track-nfct-status-in-newly-added-skb-_state/20210722-014505
base:   https://git.kernel.org/pub/scm/linux/kernel/git/pablo/nf.git master
config: nios2-defconfig (attached as .config)
compiler: nios2-linux-gcc (GCC) 10.3.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/973d99858ab4990b527f70441c4dae8b87637c41
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Paolo-Abeni/sk_buff-track-nfct-status-in-newly-added-skb-_state/20210722-014505
        git checkout 973d99858ab4990b527f70441c4dae8b87637c41
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-10.3.0 make.cross ARCH=nios2 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

   In file included from include/uapi/linux/posix_types.h:5,
                    from include/uapi/linux/types.h:14,
                    from include/linux/types.h:6,
                    from include/linux/kasan-checks.h:5,
                    from include/asm-generic/rwonce.h:26,
                    from ./arch/nios2/include/generated/asm/rwonce.h:1,
                    from include/linux/compiler.h:264,
                    from include/linux/err.h:5,
                    from include/linux/kthread.h:5,
                    from include/trace/events/sched.h:8,
                    from kernel/sched/core.c:10:
   include/linux/skbuff.h: In function 'skb_secmark':
>> include/linux/stddef.h:8:14: warning: returning 'void *' from a function with return type '__u32' {aka 'unsigned int'} makes integer from pointer without a cast [-Wint-conversion]
       8 | #define NULL ((void *)0)
         |              ^
   include/linux/skbuff.h:4240:9: note: in expansion of macro 'NULL'
    4240 |  return NULL;
         |         ^~~~
   kernel/sched/core.c: In function 'ttwu_stat':
   kernel/sched/core.c:3270:13: warning: variable 'rq' set but not used [-Wunused-but-set-variable]
    3270 |  struct rq *rq;
         |             ^~
--
   In file included from include/uapi/linux/posix_types.h:5,
                    from include/uapi/linux/types.h:14,
                    from include/linux/types.h:6,
                    from include/uapi/linux/sched.h:5,
                    from include/linux/sched.h:10,
                    from kernel/sched/sched.h:5,
                    from kernel/sched/loadavg.c:9:
   include/linux/skbuff.h: In function 'skb_secmark':
>> include/linux/stddef.h:8:14: warning: returning 'void *' from a function with return type '__u32' {aka 'unsigned int'} makes integer from pointer without a cast [-Wint-conversion]
       8 | #define NULL ((void *)0)
         |              ^
   include/linux/skbuff.h:4240:9: note: in expansion of macro 'NULL'
    4240 |  return NULL;
         |         ^~~~
--
   In file included from include/uapi/linux/posix_types.h:5,
                    from include/uapi/linux/types.h:14,
                    from include/linux/types.h:6,
                    from include/uapi/linux/sched.h:5,
                    from include/linux/sched.h:10,
                    from kernel/sched/sched.h:5,
                    from kernel/sched/fair.c:23:
   include/linux/skbuff.h: In function 'skb_secmark':
>> include/linux/stddef.h:8:14: warning: returning 'void *' from a function with return type '__u32' {aka 'unsigned int'} makes integer from pointer without a cast [-Wint-conversion]
       8 | #define NULL ((void *)0)
         |              ^
   include/linux/skbuff.h:4240:9: note: in expansion of macro 'NULL'
    4240 |  return NULL;
         |         ^~~~
   kernel/sched/fair.c: At top level:
   kernel/sched/fair.c:591:5: warning: no previous prototype for 'sched_update_scaling' [-Wmissing-prototypes]
     591 | int sched_update_scaling(void)
         |     ^~~~~~~~~~~~~~~~~~~~
   kernel/sched/fair.c:5438:6: warning: no previous prototype for 'init_cfs_bandwidth' [-Wmissing-prototypes]
    5438 | void init_cfs_bandwidth(struct cfs_bandwidth *cfs_b) {}
         |      ^~~~~~~~~~~~~~~~~~
   kernel/sched/fair.c:11457:6: warning: no previous prototype for 'free_fair_sched_group' [-Wmissing-prototypes]
   11457 | void free_fair_sched_group(struct task_group *tg) { }
         |      ^~~~~~~~~~~~~~~~~~~~~
   kernel/sched/fair.c:11459:5: warning: no previous prototype for 'alloc_fair_sched_group' [-Wmissing-prototypes]
   11459 | int alloc_fair_sched_group(struct task_group *tg, struct task_group *parent)
         |     ^~~~~~~~~~~~~~~~~~~~~~
   kernel/sched/fair.c:11464:6: warning: no previous prototype for 'online_fair_sched_group' [-Wmissing-prototypes]
   11464 | void online_fair_sched_group(struct task_group *tg) { }
         |      ^~~~~~~~~~~~~~~~~~~~~~~
   kernel/sched/fair.c:11466:6: warning: no previous prototype for 'unregister_fair_sched_group' [-Wmissing-prototypes]
   11466 | void unregister_fair_sched_group(struct task_group *tg) { }
         |      ^~~~~~~~~~~~~~~~~~~~~~~~~~~
--
   In file included from include/uapi/linux/posix_types.h:5,
                    from include/uapi/linux/types.h:14,
                    from include/linux/types.h:6,
                    from include/uapi/linux/sched.h:5,
                    from include/linux/sched.h:10,
                    from kernel/sched/sched.h:5,
                    from kernel/sched/rt.c:6:
   include/linux/skbuff.h: In function 'skb_secmark':
>> include/linux/stddef.h:8:14: warning: returning 'void *' from a function with return type '__u32' {aka 'unsigned int'} makes integer from pointer without a cast [-Wint-conversion]
       8 | #define NULL ((void *)0)
         |              ^
   include/linux/skbuff.h:4240:9: note: in expansion of macro 'NULL'
    4240 |  return NULL;
         |         ^~~~
   kernel/sched/rt.c: At top level:
   kernel/sched/rt.c:253:6: warning: no previous prototype for 'free_rt_sched_group' [-Wmissing-prototypes]
     253 | void free_rt_sched_group(struct task_group *tg) { }
         |      ^~~~~~~~~~~~~~~~~~~
   kernel/sched/rt.c:255:5: warning: no previous prototype for 'alloc_rt_sched_group' [-Wmissing-prototypes]
     255 | int alloc_rt_sched_group(struct task_group *tg, struct task_group *parent)
         |     ^~~~~~~~~~~~~~~~~~~~
   kernel/sched/rt.c:669:6: warning: no previous prototype for 'sched_rt_bandwidth_account' [-Wmissing-prototypes]
     669 | bool sched_rt_bandwidth_account(struct rt_rq *rt_rq)
         |      ^~~~~~~~~~~~~~~~~~~~~~~~~~


vim +8 include/linux/stddef.h

^1da177e4c3f41 Linus Torvalds   2005-04-16  6  
^1da177e4c3f41 Linus Torvalds   2005-04-16  7  #undef NULL
^1da177e4c3f41 Linus Torvalds   2005-04-16 @8  #define NULL ((void *)0)
6e218287432472 Richard Knutsson 2006-09-30  9  

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 10072 bytes --]

Re: [PATCH RFC 9/9] sk_buff: access secmark via getter/setter

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 9340 bytes --]

Hi Paolo,

[FYI, it's a private test report for your RFC patch.]
[auto build test WARNING on nf/master]
[also build test WARNING on linus/master v5.14-rc2 next-20210721]
[cannot apply to nf-next/master security/next-testing]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Paolo-Abeni/sk_buff-track-nfct-status-in-newly-added-skb-_state/20210722-014505
base:   https://git.kernel.org/pub/scm/linux/kernel/git/pablo/nf.git master
config: mips-randconfig-r001-20210720 (attached as .config)
compiler: clang version 13.0.0 (https://github.com/llvm/llvm-project c781eb153bfbd1b52b03efe34f56bbeccbb8aba6)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install mips cross compiling tool for clang build
        # apt-get install binutils-mips-linux-gnu
        # https://github.com/0day-ci/linux/commit/973d99858ab4990b527f70441c4dae8b87637c41
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Paolo-Abeni/sk_buff-track-nfct-status-in-newly-added-skb-_state/20210722-014505
        git checkout 973d99858ab4990b527f70441c4dae8b87637c41
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=mips 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

   In file included from kernel/sched/core.c:13:
   In file included from kernel/sched/sched.h:50:
   In file included from include/linux/init_task.h:18:
   In file included from include/net/net_namespace.h:39:
>> include/linux/skbuff.h:4240:9: warning: incompatible pointer to integer conversion returning 'void *' from a function with result type '__u32' (aka 'unsigned int') [-Wint-conversion]
           return NULL;
                  ^~~~
   include/linux/stddef.h:8:14: note: expanded from macro 'NULL'
   #define NULL ((void *)0)
                ^~~~~~~~~~~
   kernel/sched/core.c:3260:20: warning: unused function 'rq_has_pinned_tasks' [-Wunused-function]
   static inline bool rq_has_pinned_tasks(struct rq *rq)
                      ^
   kernel/sched/core.c:5111:20: warning: unused function 'sched_tick_start' [-Wunused-function]
   static inline void sched_tick_start(int cpu) { }
                      ^
   kernel/sched/core.c:5112:20: warning: unused function 'sched_tick_stop' [-Wunused-function]
   static inline void sched_tick_stop(int cpu) { }
                      ^
   kernel/sched/core.c:5769:20: warning: unused function 'sched_core_cpu_starting' [-Wunused-function]
   static inline void sched_core_cpu_starting(unsigned int cpu) {}
                      ^
   5 warnings generated.
--
   In file included from kernel/sched/loadavg.c:9:
   In file included from kernel/sched/sched.h:50:
   In file included from include/linux/init_task.h:18:
   In file included from include/net/net_namespace.h:39:
>> include/linux/skbuff.h:4240:9: warning: incompatible pointer to integer conversion returning 'void *' from a function with result type '__u32' (aka 'unsigned int') [-Wint-conversion]
           return NULL;
                  ^~~~
   include/linux/stddef.h:8:14: note: expanded from macro 'NULL'
   #define NULL ((void *)0)
                ^~~~~~~~~~~
   1 warning generated.
--
   In file included from kernel/sched/fair.c:23:
   In file included from kernel/sched/sched.h:50:
   In file included from include/linux/init_task.h:18:
   In file included from include/net/net_namespace.h:39:
>> include/linux/skbuff.h:4240:9: warning: incompatible pointer to integer conversion returning 'void *' from a function with result type '__u32' (aka 'unsigned int') [-Wint-conversion]
           return NULL;
                  ^~~~
   include/linux/stddef.h:8:14: note: expanded from macro 'NULL'
   #define NULL ((void *)0)
                ^~~~~~~~~~~
   kernel/sched/fair.c:591:5: warning: no previous prototype for function 'sched_update_scaling' [-Wmissing-prototypes]
   int sched_update_scaling(void)
       ^
   kernel/sched/fair.c:591:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
   int sched_update_scaling(void)
   ^
   static 
   kernel/sched/fair.c:2939:20: warning: unused function 'account_numa_enqueue' [-Wunused-function]
   static inline void account_numa_enqueue(struct rq *rq, struct task_struct *p)
                      ^
   kernel/sched/fair.c:2943:20: warning: unused function 'account_numa_dequeue' [-Wunused-function]
   static inline void account_numa_dequeue(struct rq *rq, struct task_struct *p)
                      ^
   kernel/sched/fair.c:2947:20: warning: unused function 'update_scan_period' [-Wunused-function]
   static inline void update_scan_period(struct task_struct *p, int new_cpu)
                      ^
   kernel/sched/fair.c:4110:20: warning: unused function 'cfs_rq_is_decayed' [-Wunused-function]
   static inline bool cfs_rq_is_decayed(struct cfs_rq *cfs_rq)
                      ^
   kernel/sched/fair.c:5432:19: warning: unused function 'throttled_lb_pair' [-Wunused-function]
   static inline int throttled_lb_pair(struct task_group *tg,
                     ^
   kernel/sched/fair.c:5449:20: warning: unused function 'update_runtime_enabled' [-Wunused-function]
   static inline void update_runtime_enabled(struct rq *rq) {}
                      ^
   kernel/sched/fair.c:5450:20: warning: unused function 'unthrottle_offline_cfs_rqs' [-Wunused-function]
   static inline void unthrottle_offline_cfs_rqs(struct rq *rq) {}
                      ^
   9 warnings generated.
--
   In file included from kernel/sched/rt.c:6:
   In file included from kernel/sched/sched.h:50:
   In file included from include/linux/init_task.h:18:
   In file included from include/net/net_namespace.h:39:
>> include/linux/skbuff.h:4240:9: warning: incompatible pointer to integer conversion returning 'void *' from a function with result type '__u32' (aka 'unsigned int') [-Wint-conversion]
           return NULL;
                  ^~~~
   include/linux/stddef.h:8:14: note: expanded from macro 'NULL'
   #define NULL ((void *)0)
                ^~~~~~~~~~~
   kernel/sched/rt.c:669:6: warning: no previous prototype for function 'sched_rt_bandwidth_account' [-Wmissing-prototypes]
   bool sched_rt_bandwidth_account(struct rt_rq *rt_rq)
        ^
   kernel/sched/rt.c:669:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
   bool sched_rt_bandwidth_account(struct rt_rq *rt_rq)
   ^
   static 
   kernel/sched/rt.c:421:20: warning: unused function 'need_pull_rt_task' [-Wunused-function]
   static inline bool need_pull_rt_task(struct rq *rq, struct task_struct *prev)
                      ^
   kernel/sched/rt.c:426:20: warning: unused function 'pull_rt_task' [-Wunused-function]
   static inline void pull_rt_task(struct rq *this_rq)
                      ^
   kernel/sched/rt.c:476:20: warning: unused function 'rt_task_fits_capacity' [-Wunused-function]
   static inline bool rt_task_fits_capacity(struct task_struct *p, int cpu)
                      ^
   kernel/sched/rt.c:1113:6: warning: unused function 'inc_rt_prio_smp' [-Wunused-function]
   void inc_rt_prio_smp(struct rt_rq *rt_rq, int prio, int prev_prio) {}
        ^
   kernel/sched/rt.c:1115:6: warning: unused function 'dec_rt_prio_smp' [-Wunused-function]
   void dec_rt_prio_smp(struct rt_rq *rt_rq, int prio, int prev_prio) {}
        ^
   7 warnings generated.
--
   In file included from kernel/sched/deadline.c:18:
   In file included from kernel/sched/sched.h:50:
   In file included from include/linux/init_task.h:18:
   In file included from include/net/net_namespace.h:39:
>> include/linux/skbuff.h:4240:9: warning: incompatible pointer to integer conversion returning 'void *' from a function with result type '__u32' (aka 'unsigned int') [-Wint-conversion]
           return NULL;
                  ^~~~
   include/linux/stddef.h:8:14: note: expanded from macro 'NULL'
   #define NULL ((void *)0)
                ^~~~~~~~~~~
   kernel/sched/deadline.c:686:20: warning: unused function 'need_pull_dl_task' [-Wunused-function]
   static inline bool need_pull_dl_task(struct rq *rq, struct task_struct *prev)
                      ^
   kernel/sched/deadline.c:691:20: warning: unused function 'pull_dl_task' [-Wunused-function]
   static inline void pull_dl_task(struct rq *rq)
                      ^
   3 warnings generated.


vim +4240 include/linux/skbuff.h

  4234	
  4235	static inline __u32 skb_secmark(const struct sk_buff *skb)
  4236	{
  4237	#if IS_ENABLED(CONFIG_NETWORK_SECMARK)
  4238		return skb->secmark_present ? skb->_secmark : 0;
  4239	#else
> 4240		return NULL;
  4241	#endif
  4242	}
  4243	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 34316 bytes --]