netdev - Re: [PATCH net-next] net: core: rework skb_probe_transport

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 03 May 2018 13:32:54 -0400 (EDT)
From:   David Miller <davem@...emloft.net>
To:     pabeni@...hat.com
Cc:     netdev@...r.kernel.org, edumazet@...gle.com, jasowang@...hat.com
Subject: Re: [PATCH net-next] net: core: rework skb_probe_transport_header()

From: Paolo Abeni <pabeni@...hat.com>
Date: Thu,  3 May 2018 11:35:35 +0200

> When the transport header is not available, skb_probe_transport_header()
> resorts to fully dissect the flow keys, even if it only needs the
> ransport offset. We can obtain the latter using a simpler flow dissector -
> flow_keys_buf_dissector - and a smaller struct for key storage.
> 
> The above gives ~50% performance improvement in micro benchmarking around
> skb_probe_transport_header(), mostly due to the smaller memset. Small, but
> measurable improvement is measured also in macro benchmarking - raw xmit
> tput from a VM.
> 
> Signed-off-by: Paolo Abeni <pabeni@...hat.com>

Please make this optimization generally, then every driver using
eth_get_headlen() (11 or so) will get the same improvement, for all
traffic!

Something like this:

diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 908d66e55b14..e635e9ab818a 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -1171,7 +1171,7 @@ void __skb_get_hash(struct sk_buff *skb);
 u32 __skb_get_hash_symmetric(const struct sk_buff *skb);
 u32 skb_get_poff(const struct sk_buff *skb);
 u32 __skb_get_poff(const struct sk_buff *skb, void *data,
-		   const struct flow_keys *keys, int hlen);
+		   const struct flow_keys_basic *keys, int hlen);
 __be32 __skb_flow_get_ports(const struct sk_buff *skb, int thoff, u8 ip_proto,
 			    void *data, int hlen_proto);
 
@@ -1208,7 +1208,7 @@ static inline bool skb_flow_dissect_flow_keys(const struct sk_buff *skb,
 				  NULL, 0, 0, 0, flags);
 }
 
-static inline bool skb_flow_dissect_flow_keys_buf(struct flow_keys *flow,
+static inline bool skb_flow_dissect_flow_keys_buf(struct flow_keys_basic *flow,
 						  void *data, __be16 proto,
 						  int nhoff, int hlen,
 						  unsigned int flags)
@@ -2350,11 +2350,14 @@ static inline void skb_pop_mac_header(struct sk_buff *skb)
 static inline void skb_probe_transport_header(struct sk_buff *skb,
 					      const int offset_hint)
 {
-	struct flow_keys keys;
+	struct flow_keys_basic keys;
 
 	if (skb_transport_header_was_set(skb))
 		return;
-	else if (skb_flow_dissect_flow_keys(skb, &keys, 0))
+
+	memset(&keys, 0, sizeof(keys));
+	if (__skb_flow_dissect(skb, &flow_keys_buf_dissector, &keys,
+			       NULL, 0, 0, 0, 0))
 		skb_set_transport_header(skb, keys.control.thoff);
 	else
 		skb_set_transport_header(skb, offset_hint);
diff --git a/include/net/flow_dissector.h b/include/net/flow_dissector.h
index 9a074776f70b..e81dab6e9ac6 100644
--- a/include/net/flow_dissector.h
+++ b/include/net/flow_dissector.h
@@ -226,6 +226,11 @@ struct flow_dissector {
 	unsigned short int offset[FLOW_DISSECTOR_KEY_MAX];
 };
 
+struct flow_keys_basic {
+	struct flow_dissector_key_control control;
+	struct flow_dissector_key_basic basic;
+};
+
 struct flow_keys {
 	struct flow_dissector_key_control control;
 #define FLOW_KEYS_HASH_START_FIELD basic
diff --git a/net/core/flow_dissector.c b/net/core/flow_dissector.c
index d29f09bc5ff9..50d68b54f03a 100644
--- a/net/core/flow_dissector.c
+++ b/net/core/flow_dissector.c
@@ -1253,7 +1253,7 @@ __u32 skb_get_hash_perturb(const struct sk_buff *skb, u32 perturb)
 EXPORT_SYMBOL(skb_get_hash_perturb);
 
 u32 __skb_get_poff(const struct sk_buff *skb, void *data,
-		   const struct flow_keys *keys, int hlen)
+		   const struct flow_keys_basic *keys, int hlen)
 {
 	u32 poff = keys->control.thoff;
 
@@ -1314,9 +1314,11 @@ u32 __skb_get_poff(const struct sk_buff *skb, void *data,
  */
 u32 skb_get_poff(const struct sk_buff *skb)
 {
-	struct flow_keys keys;
+	struct flow_keys_basic keys;
 
-	if (!skb_flow_dissect_flow_keys(skb, &keys, 0))
+	memset(&keys, 0, sizeof(keys));
+	if (!__skb_flow_dissect(skb, &flow_keys_buf_dissector, &keys,
+				NULL, 0, 0, 0, 0))
 		return 0;
 
 	return __skb_get_poff(skb, skb->data, &keys, skb_headlen(skb));
@@ -1418,6 +1420,7 @@ struct flow_dissector flow_keys_dissector __read_mostly;
 EXPORT_SYMBOL(flow_keys_dissector);
 
 struct flow_dissector flow_keys_buf_dissector __read_mostly;
+EXPORT_SYMBOL(flow_keys_buf_dissector);
 
 static int __init init_default_flow_dissectors(void)
 {
diff --git a/net/ethernet/eth.c b/net/ethernet/eth.c
index eaeba9b99a73..873eca643f99 100644
--- a/net/ethernet/eth.c
+++ b/net/ethernet/eth.c
@@ -128,7 +128,7 @@ u32 eth_get_headlen(void *data, unsigned int len)
 {
 	const unsigned int flags = FLOW_DISSECTOR_F_PARSE_1ST_FRAG;
 	const struct ethhdr *eth = (const struct ethhdr *)data;
-	struct flow_keys keys;
+	struct flow_keys_basic keys;
 
 	/* this should never happen, but better safe than sorry */
 	if (unlikely(len < sizeof(*eth)))