From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jason Wang Subject: Re: [PATCH net-next] net: core: rework skb_probe_transport_header() Date: Thu, 3 May 2018 20:55:32 +0800 Message-ID: <87edeb52-c159-2f72-1daa-75731f0c9d42@redhat.com> References: <7cbdf466f4a1bf44ddbb948428dc7bb0dad091a7.1525340013.git.pabeni@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit Cc: "David S. Miller" , Eric Dumazet To: Paolo Abeni , netdev@vger.kernel.org Return-path: Received: from mx3-rdu2.redhat.com ([66.187.233.73]:56572 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1750781AbeECMzi (ORCPT ); Thu, 3 May 2018 08:55:38 -0400 In-Reply-To: <7cbdf466f4a1bf44ddbb948428dc7bb0dad091a7.1525340013.git.pabeni@redhat.com> Content-Language: en-US Sender: netdev-owner@vger.kernel.org List-ID: 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 > --- > 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 Just curious, I believe this happens only when csum offload is disabled which is not the common case? Thanks