From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753447AbbIPVjG (ORCPT ); Wed, 16 Sep 2015 17:39:06 -0400 Received: from mail-ig0-f170.google.com ([209.85.213.170]:37113 "EHLO mail-ig0-f170.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752468AbbIPVjB (ORCPT ); Wed, 16 Sep 2015 17:39:01 -0400 Subject: Re: [PATCH net-next RFC] net: increase LL_MAX_HEADER for Hyper-V To: KY Srinivasan , David Laight , Haiyang Zhang , Vitaly Kuznetsov , "netdev@vger.kernel.org" References: <1442418625-11805-1-git-send-email-vkuznets@redhat.com> <063D6719AE5E284EB5DD2968C1650D6D1CB9800B@AcuExch.aculab.com> Cc: "David S. Miller" , "linux-kernel@vger.kernel.org" , Jason Wang From: Alexander Duyck Message-ID: <55F9E172.2040008@gmail.com> Date: Wed, 16 Sep 2015 14:38:58 -0700 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.1.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 09/16/2015 10:55 AM, KY Srinivasan wrote: > >> -----Original Message----- >> From: David Laight [mailto:David.Laight@ACULAB.COM] >> Sent: Wednesday, September 16, 2015 9:25 AM >> To: Haiyang Zhang ; Vitaly Kuznetsov >> ; netdev@vger.kernel.org >> Cc: David S. Miller ; linux-kernel@vger.kernel.org; >> KY Srinivasan ; Jason Wang >> Subject: RE: [PATCH net-next RFC] net: increase LL_MAX_HEADER for Hyper- >> V >> >> From: Haiyang Zhang >>> Sent: 16 September 2015 17:09 >>>> -----Original Message----- >>>> From: Vitaly Kuznetsov [mailto:vkuznets@redhat.com] >>>> Sent: Wednesday, September 16, 2015 11:50 AM >>>> To: netdev@vger.kernel.org >>>> Cc: David S. Miller ; linux- >> kernel@vger.kernel.org; >>>> KY Srinivasan ; Haiyang Zhang >>>> ; Jason Wang >>>> Subject: [PATCH net-next RFC] net: increase LL_MAX_HEADER for Hyper- >> V >>>> Commit b08cc79155fc26d0d112b1470d1ece5034651a4b ("hv_netvsc: >> Eliminate >>>> memory allocation in the packet send path") introduced skb headroom >>>> request for Hyper-V netvsc driver: >>>> >>>> max_needed_headroom = sizeof(struct hv_netvsc_packet) + >>>> sizeof(struct rndis_message) + >>>> NDIS_VLAN_PPI_SIZE + NDIS_CSUM_PPI_SIZE + >>>> NDIS_LSO_PPI_SIZE + NDIS_HASH_PPI_SIZE; >>>> ... >>>> net->needed_headroom = max_needed_headroom; >>>> >>>> max_needed_headroom is 220 bytes, it significantly exceeds the >>>> LL_MAX_HEADER setting. This causes each skb to be cloned on send >> path, >>>> e.g. for IPv4 case we fall into the following clause >>>> (ip_finish_output2()): >>>> >>>> if (unlikely(skb_headroom(skb) < hh_len && dev->header_ops)) { >>>> ... >>>> skb2 = skb_realloc_headroom(skb, LL_RESERVED_SPACE(dev)); >>>> ... >>>> } >>>> >>>> leading to a significant performance regression. Increase >> LL_MAX_HEADER >>>> to make it suitable for netvsc, make it 224 to be 16-aligned. >>>> Alternatively we could (partially) revert the commit which introduced >>>> skb >>>> headroom request restoring manual memory allocation on transmit path. >>>> >>>> Signed-off-by: Vitaly Kuznetsov >>>> --- >>>> include/linux/netdevice.h | 4 +++- >>>> 1 file changed, 3 insertions(+), 1 deletion(-) >>>> >>>> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h >>>> index 88a0069..7233790 100644 >>>> --- a/include/linux/netdevice.h >>>> +++ b/include/linux/netdevice.h >>>> @@ -132,7 +132,9 @@ static inline bool dev_xmit_complete(int rc) >>>> * used. >>>> */ >>>> >>>> -#if defined(CONFIG_WLAN) || IS_ENABLED(CONFIG_AX25) >>>> +#if IS_ENABLED(CONFIG_HYPERV_NET) >>>> +# define LL_MAX_HEADER 224 >>>> +#elif defined(CONFIG_WLAN) || IS_ENABLED(CONFIG_AX25) >>>> # if defined(CONFIG_MAC80211_MESH) >>>> # define LL_MAX_HEADER 128 >>>> # else >>> Thanks for the patch. >>> To avoid we forget to update that 224 number when we add more things >>> into netvsc header, I suggest that we define a macro in netdevice.h such >>> as: >>> #define HVNETVSC_MAX_HEADER 224 >>> #define LL_MAX_HEADER HVNETVSC_MAX_HEADER >>> >>> And, put a note in netvsc code saying the header reservation shouldn't >>> exceed HVNETVSC_MAX_HEADER, or you need to update >> HVNETVSC_MAX_HEADER. >> >> Am I right in thinking this is adding an extra 96 unused bytes to the front >> of almost all skb just so that hyper-v can make its link level header >> contiguous with whatever follows (IP header ?). >> >> Doesn't sound ideal. > Remote NDIS is the protocol used to send packets from the guest to the host. Every packet > needs to be decorated with the RNDIS header and the maximum room needed for the RNDIS > header is the hreadroom we want. I think we get that. The question is does the Remote NDIS header and packet info actually need to be a part of the header data? I would argue that it probably doesn't. So for example in netvsc_start_xmit it looks like you are calling init_page_array in order to populate a set of page buffers, but the first buffer for the Remote NDIS protocol is populated as a separate page and offset. As such it doesn't seem like it necessarily needs to be a part of the header data but could be maintained perhaps in a separate ring buffer, or perhaps just be a separate page that you break up to use for each header. - Alex