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]

Message-ID: <87edeb52-c159-2f72-1daa-75731f0c9d42@redhat.com>
Date:   Thu, 3 May 2018 20:55:32 +0800
From:   Jason Wang <jasowang@...hat.com>
To:     Paolo Abeni <pabeni@...hat.com>, netdev@...r.kernel.org
Cc:     "David S. Miller" <davem@...emloft.net>,
        Eric Dumazet <edumazet@...gle.com>
Subject: Re: [PATCH net-next] net: core: rework skb_probe_transport_header()



On 2018年05月03日 17:35, Paolo Abeni wrote:
> 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>
> ---
>   include/linux/skbuff.h       | 7 +++++--
>   include/net/flow_dissector.h | 5 +++++
>   net/core/flow_dissector.c    | 1 +
>   3 files changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
> index 908d66e55b14..63cb523d3519 100644
> --- a/include/linux/skbuff.h
> +++ b/include/linux/skbuff.h
> @@ -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,
> +			       0, 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..ac7b4de4a0f0 100644
> --- a/net/core/flow_dissector.c
> +++ b/net/core/flow_dissector.c
> @@ -1418,6 +1418,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)
>   {

Acked-by: Jason Wang <jasowang@...hat.com>

Just curious, I believe this happens only when csum offload is disabled 
which is not the common case?

Thanks