[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