From patchwork Thu Nov 19 23:52:55 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Ramsay, Lincoln" X-Patchwork-Id: 1403371 Return-Path: X-Original-To: patchwork-incoming-netdev@ozlabs.org Delivered-To: patchwork-incoming-netdev@ozlabs.org Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=vger.kernel.org (client-ip=23.128.96.18; helo=vger.kernel.org; envelope-from=netdev-owner@vger.kernel.org; receiver=) Authentication-Results: ozlabs.org; dmarc=pass (p=reject dis=none) header.from=digi.com Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=digi.com header.i=@digi.com header.a=rsa-sha256 header.s=selector1 header.b=GBCbc5hT; dkim-atps=neutral Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by ozlabs.org (Postfix) with ESMTP id 4Ccc203vQ6z9sTL for ; Fri, 20 Nov 2020 10:53:12 +1100 (AEDT) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726768AbgKSXxL (ORCPT ); Thu, 19 Nov 2020 18:53:11 -0500 Received: from outbound-ip24a.ess.barracuda.com ([209.222.82.206]:58092 "EHLO outbound-ip24a.ess.barracuda.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726449AbgKSXxL (ORCPT ); Thu, 19 Nov 2020 18:53:11 -0500 Received: from NAM10-DM6-obe.outbound.protection.outlook.com (mail-dm6nam10lp2105.outbound.protection.outlook.com [104.47.58.105]) by mx6.us-east-2a.ess.aws.cudaops.com (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NO); Thu, 19 Nov 2020 23:52:57 +0000 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=FTOBFlMjlfyPE3D0WZflCvO66v9cG/M7pf5Vm+rIT1XwYJHNzApgP3bM0un8qjnCTcPZk6j+MwyP5jPjMbPnhmrtY84auyQkKZP2mqnHo7ho+Hy2KpDa4T+0OnVaUJx9viT82qyILSo8OzLciW2bjsKYrGQs6MYjYNQDAXuqGeioAjZ3F5KSw7xfR8ag2lkZ2KvoW7qCr90mhEI7bzkhooNW98K3c5w2+LEyeRqikbznOOo92XNZPg04oKGJbBQgzYFCSXGTrW3TM27mjgo0M2XQZzOP2GcsB1cnBuSl5aGKPdxgyUVsnYZ57Xh6sqjvSPOM4qnYqN55Qg82ab7+EQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=tcWeT6uiq9dXdrw+K85E+sYkS2/D5EmDD+a17jr28r8=; b=ZYowpmb3IrZQCixTjFSEqUlnDxqqS4Kr1tO2VQ78Sl9UeKwFlXCDInoLhSvrB4GufvWjGXl9Hc16JJe9gJ1V0/FngtmvrppaYGNUT134Cq2VQ3ra9YvnsoNJUJLg6+2rLc8RaEDQaaRqG7afisyUSf565PtvYMkOf2vTULt936qTLA+iRt9oh42bWpp+I1L3Y1ITCAxvJ85vt+R3lCVLa/Y5nLqYJCMO5PKHOyKN74Y1NQQdFtxXdK5Awh/M/Xs01rElyD5gtbux5NnoQ6XxAypAVybPCmfdUr8icO5QNrYYCWWVjW5IzRWYUdcoo9duhCxJ3a1i3iMZxGkjjyEWsQ== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=digi.com; dmarc=pass action=none header.from=digi.com; dkim=pass header.d=digi.com; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=digi.com; s=selector1; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=tcWeT6uiq9dXdrw+K85E+sYkS2/D5EmDD+a17jr28r8=; b=GBCbc5hT2Vyj5DzUcJgjl9kaw6ynKD0BJaQ60CeEVDNDi5P7zakNZi/BHEyZCNGL+5YkoSqmyZGkKP1Nt1l2QHWVJ3agfKsebyhOicJnuw2pSFlm9ciQbwaVo3K2iY/L3vu+Ufcq/fwjc96Hi5lyoPdGjrn40/0Qd3kN7nQOhV0= Received: from CY4PR1001MB2311.namprd10.prod.outlook.com (2603:10b6:910:44::24) by CY4PR1001MB2087.namprd10.prod.outlook.com (2603:10b6:910:47::15) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.3589.22; Thu, 19 Nov 2020 23:52:55 +0000 Received: from CY4PR1001MB2311.namprd10.prod.outlook.com ([fe80::a956:bdc0:5119:197]) by CY4PR1001MB2311.namprd10.prod.outlook.com ([fe80::a956:bdc0:5119:197%6]) with mapi id 15.20.3564.033; Thu, 19 Nov 2020 23:52:55 +0000 From: "Ramsay, Lincoln" To: Florian Westphal CC: Igor Russkikh , "David S. Miller" , Jakub Kicinski , "netdev@vger.kernel.org" , Dmitry Bogdanov Subject: [PATCH v4] aquantia: Remove the build_skb path Thread-Topic: [PATCH v4] aquantia: Remove the build_skb path Thread-Index: AQHWvs8ZM2n6oge2KEC89hbATIw5VQ== Date: Thu, 19 Nov 2020 23:52:55 +0000 Message-ID: References: <2b392026-c077-2871-3492-eb5ddd582422@marvell.com> <20201119221510.GI15137@breakpoint.cc> <20201119222800.GJ15137@breakpoint.cc> ,<20201119225842.GK15137@breakpoint.cc> In-Reply-To: <20201119225842.GK15137@breakpoint.cc> Accept-Language: en-AU, en-US Content-Language: en-AU X-MS-Has-Attach: X-MS-TNEF-Correlator: authentication-results: strlen.de; dkim=none (message not signed) header.d=none;strlen.de; dmarc=none action=none header.from=digi.com; x-originating-ip: [158.140.192.185] x-ms-publictraffictype: Email x-ms-office365-filtering-correlation-id: d2c004b5-1ec5-4436-f782-08d88ce63c4f x-ms-traffictypediagnostic: CY4PR1001MB2087: x-microsoft-antispam-prvs: x-ms-oob-tlc-oobclassifiers: OLM:9508; x-ms-exchange-senderadcheck: 1 x-microsoft-antispam: BCL:0; x-microsoft-antispam-message-info: M06sX9dAqKvsLUW9KAQ2KpEZ6z2GVR7seeUCvgE2i4UwHazoeMwknNkwzInn2iKuM7kZhLg/3A2Z6DIUDD0732G9C/6IFeIAYoXO5zfhb4LGBTy+vXfg7s6gsKAOyiU6VJVbKgFEhV6tO6J2u26REO2wtqolYGieHKnteLTP/Zk9oe2AfP4RN1P0UHhVJwh2mXVMgA7Araqk/+qLbkELZ88qUPLakwU7So8Ectp9W1STVE6fUTL7S48cDcAwQnPQKoWxtkaZjRMtB+YASKgMtI2TJHhAkb6ydPIrzWMpKvmg42FuDhyFayHT2Iz3G5fC x-forefront-antispam-report: CIP:255.255.255.255;CTRY:;LANG:en;SCL:1;SRV:;IPV:NLI;SFV:NSPM;H:CY4PR1001MB2311.namprd10.prod.outlook.com;PTR:;CAT:NONE;SFS:(4636009)(376002)(346002)(39850400004)(366004)(136003)(396003)(6916009)(66476007)(186003)(66946007)(64756008)(9686003)(76116006)(26005)(91956017)(52536014)(66446008)(86362001)(55016002)(2906002)(7696005)(5660300002)(478600001)(66556008)(83380400001)(6506007)(4326008)(54906003)(8936002)(33656002)(316002)(8676002)(71200400001);DIR:OUT;SFP:1102; x-ms-exchange-antispam-messagedata: OSaKtAh4TFQ0j2SYgR5/WKON/FZYkFCTLJfb00dEERJgLvlfTcAJtb2ybKrSptGzEvxZC3GjKwvhqrC7Br90dVdUtjwf1+kEPGo1hVcp3NBp9u2kMxLlk2tsyng+neCNN9d9EnpefkX6pMoO22+x/c4VrhlZBd8CjW9nMsyH/3pyplvMUvEvJLoTRKOCHWu6RgKVI100JGcjeQex0lrFAVwVl9pzZJxx9hHm6UKpLOHHmXK7zmjhyDDXpVP6BPJ/RyooigYKHS6egXfYAE7JO2TCgTB6XPPvKV+2smISU0LNuwQd7/P6vrFRCBK51PiNkk1pMIQxTSvpV8izjkXxFMmbcZzQSxJU+gbU+4yv0kCXJqHzdpXMdemUh0EaYIlgIw6KFl3R2+WaaUrd/j6bNQITQL3RC300Anijt+G5cSuvRRtvWqMZukhjt/gpVI63+RCOf0Wuz5HcK9EP+sc7UU0HZcPEy/DHz9ZSerTGvqM10yjCMySnbzkdZLJ4WrHAaR4y0auSZLFdXzW1kf2KpyBtkfKW9V+qlA6EnQxvXONdHhKypvVC9U7bMSOp2f0JpcZuEnijZogPyYUdjPYD0aI7NxBjlmnFRgkpa9axNDrUtgB2jqThHWVRKCHph1hpcDpBxoIkoVgzo++fcTqlthThhWVuK1Xm5q+Z3un8sg8S1qDPHMayUhSQhWmoeSyd/8oPnPnXTEg1Wd5u0hA3q0IZGBdb5etbO7SUhEbN/VcOsgegSsoXKCCzJc/7RHXLBQCnJdLe+srebggzZ/gr9ZfC32QNPF/LBFrAQ+cj+Rp9t4z37HT3qJ6RT7przM7FJc66NtWVtLv1SX60UzDcCjLYqr/bJMxLxiImJW+DOsyMn4JwqXSd3logiVmT1EP3ohucLSEYJ2wVoRB0UdAcYg== x-ms-exchange-transport-forked: True MIME-Version: 1.0 X-OriginatorOrg: digi.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-AuthSource: CY4PR1001MB2311.namprd10.prod.outlook.com X-MS-Exchange-CrossTenant-Network-Message-Id: d2c004b5-1ec5-4436-f782-08d88ce63c4f X-MS-Exchange-CrossTenant-originalarrivaltime: 19 Nov 2020 23:52:55.4236 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: abb4cdb7-1b7e-483e-a143-7ebfd1184b9e X-MS-Exchange-CrossTenant-mailboxtype: HOSTED X-MS-Exchange-CrossTenant-userprincipalname: SRigNZ2hYPgL7npLT4aUWfaD9I6zmol4iPGUbgzt0E977L5q05g4JAMfAJbMfTeb8Sgz+/njen5iNqu4z8pwjg== X-MS-Exchange-Transport-CrossTenantHeadersStamped: CY4PR1001MB2087 X-BESS-ID: 1605829976-893010-28038-126-1 X-BESS-VER: 2019.1_20201119.2339 X-BESS-Apparent-Source-IP: 104.47.58.105 X-BESS-Outbound-Spam-Score: 0.00 X-BESS-Outbound-Spam-Report: Code version 3.2, rules version 3.2.2.228306 [from cloudscan14-191.us-east-2a.ess.aws.cudaops.com] Rule breakdown below pts rule name description ---- ---------------------- -------------------------------- 0.00 BSF_BESS_OUTBOUND META: BESS Outbound X-BESS-Outbound-Spam-Status: SCORE=0.00 using account:ESS112744 scores of KILL_LEVEL=7.0 tests=BSF_BESS_OUTBOUND X-BESS-BRTS-Status: 1 Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org When performing IPv6 forwarding, there is an expectation that SKBs will have some headroom. When forwarding a packet from the aquantia driver, this does not always happen, triggering a kernel warning. aq_ring.c has this code (edited slightly for brevity): if (buff->is_eop && buff->len <= AQ_CFG_RX_FRAME_MAX - AQ_SKB_ALIGN) { skb = build_skb(aq_buf_vaddr(&buff->rxdata), AQ_CFG_RX_FRAME_MAX); } else { skb = napi_alloc_skb(napi, AQ_CFG_RX_HDR_SIZE); There is a significant difference between the SKB produced by these 2 code paths. When napi_alloc_skb creates an SKB, there is a certain amount of headroom reserved. However, this is not done in the build_skb codepath. As the hardware buffer that build_skb is built around does not handle the presence of the SKB header, this code path is being removed and the napi_alloc_skb path will always be used. This code path does have to copy the packet header into the SKB, but it adds the packet data as a frag. Signed-off-by: Lincoln Ramsay --- > For build_skb path to work the buffer scheme would need to be changed > to reserve headroom, so yes, I think that the proposed patch is the > most convenient solution. I don't know about benefits/feasibility, but I did wonder if (in the event that the "fast path" is possible), the dma_mapping could use an offset? The page would include the skb header but the dma mapping would not. If that was done though, only 1 RX frame would fit into the page (at least on my system, where the RX frame seems to be 2k and the page is 4k). Also, there's a possibility to set the "order" variable, so that multiple pages are created at once and I'm not sure if this would work in that case. > This only copies the initial part and then the rest is added as a frag. Oh yeah. That's not as bad as I had thought then :) I wonder though... if the "fast path" is possible, could the whole packet (including header) be added as a frag, avoiding the header copy? Or is that not how SKBs work? .../net/ethernet/aquantia/atlantic/aq_ring.c | 127 ++++++++---------- 1 file changed, 53 insertions(+), 74 deletions(-) diff --git a/drivers/net/ethernet/aquantia/atlantic/aq_ring.c b/drivers/net/ethernet/aquantia/atlantic/aq_ring.c index 4f913658eea4..425e8e5afec7 100644 --- a/drivers/net/ethernet/aquantia/atlantic/aq_ring.c +++ b/drivers/net/ethernet/aquantia/atlantic/aq_ring.c @@ -413,85 +413,64 @@ int aq_ring_rx_clean(struct aq_ring_s *self, buff->rxdata.pg_off, buff->len, DMA_FROM_DEVICE); - /* for single fragment packets use build_skb() */ - if (buff->is_eop && - buff->len <= AQ_CFG_RX_FRAME_MAX - AQ_SKB_ALIGN) { - skb = build_skb(aq_buf_vaddr(&buff->rxdata), + skb = napi_alloc_skb(napi, AQ_CFG_RX_HDR_SIZE); + if (unlikely(!skb)) { + u64_stats_update_begin(&self->stats.rx.syncp); + self->stats.rx.skb_alloc_fails++; + u64_stats_update_end(&self->stats.rx.syncp); + err = -ENOMEM; + goto err_exit; + } + if (is_ptp_ring) + buff->len -= + aq_ptp_extract_ts(self->aq_nic, skb, + aq_buf_vaddr(&buff->rxdata), + buff->len); + + hdr_len = buff->len; + if (hdr_len > AQ_CFG_RX_HDR_SIZE) + hdr_len = eth_get_headlen(skb->dev, + aq_buf_vaddr(&buff->rxdata), + AQ_CFG_RX_HDR_SIZE); + + memcpy(__skb_put(skb, hdr_len), aq_buf_vaddr(&buff->rxdata), + ALIGN(hdr_len, sizeof(long))); + + if (buff->len - hdr_len > 0) { + skb_add_rx_frag(skb, 0, buff->rxdata.page, + buff->rxdata.pg_off + hdr_len, + buff->len - hdr_len, AQ_CFG_RX_FRAME_MAX); - if (unlikely(!skb)) { - u64_stats_update_begin(&self->stats.rx.syncp); - self->stats.rx.skb_alloc_fails++; - u64_stats_update_end(&self->stats.rx.syncp); - err = -ENOMEM; - goto err_exit; - } - if (is_ptp_ring) - buff->len -= - aq_ptp_extract_ts(self->aq_nic, skb, - aq_buf_vaddr(&buff->rxdata), - buff->len); - skb_put(skb, buff->len); page_ref_inc(buff->rxdata.page); - } else { - skb = napi_alloc_skb(napi, AQ_CFG_RX_HDR_SIZE); - if (unlikely(!skb)) { - u64_stats_update_begin(&self->stats.rx.syncp); - self->stats.rx.skb_alloc_fails++; - u64_stats_update_end(&self->stats.rx.syncp); - err = -ENOMEM; - goto err_exit; - } - if (is_ptp_ring) - buff->len -= - aq_ptp_extract_ts(self->aq_nic, skb, - aq_buf_vaddr(&buff->rxdata), - buff->len); - - hdr_len = buff->len; - if (hdr_len > AQ_CFG_RX_HDR_SIZE) - hdr_len = eth_get_headlen(skb->dev, - aq_buf_vaddr(&buff->rxdata), - AQ_CFG_RX_HDR_SIZE); - - memcpy(__skb_put(skb, hdr_len), aq_buf_vaddr(&buff->rxdata), - ALIGN(hdr_len, sizeof(long))); - - if (buff->len - hdr_len > 0) { - skb_add_rx_frag(skb, 0, buff->rxdata.page, - buff->rxdata.pg_off + hdr_len, - buff->len - hdr_len, - AQ_CFG_RX_FRAME_MAX); - page_ref_inc(buff->rxdata.page); - } + } - if (!buff->is_eop) { - buff_ = buff; - i = 1U; - do { - next_ = buff_->next, - buff_ = &self->buff_ring[next_]; + if (!buff->is_eop) { + buff_ = buff; + i = 1U; + do { + next_ = buff_->next, + buff_ = &self->buff_ring[next_]; - dma_sync_single_range_for_cpu( - aq_nic_get_dev(self->aq_nic), - buff_->rxdata.daddr, - buff_->rxdata.pg_off, - buff_->len, - DMA_FROM_DEVICE); - skb_add_rx_frag(skb, i++, - buff_->rxdata.page, - buff_->rxdata.pg_off, - buff_->len, - AQ_CFG_RX_FRAME_MAX); - page_ref_inc(buff_->rxdata.page); - buff_->is_cleaned = 1; - - buff->is_ip_cso &= buff_->is_ip_cso; - buff->is_udp_cso &= buff_->is_udp_cso; - buff->is_tcp_cso &= buff_->is_tcp_cso; - buff->is_cso_err |= buff_->is_cso_err; + dma_sync_single_range_for_cpu( + aq_nic_get_dev(self->aq_nic), + buff_->rxdata.daddr, + buff_->rxdata.pg_off, + buff_->len, + DMA_FROM_DEVICE); + skb_add_rx_frag(skb, i++, + buff_->rxdata.page, + buff_->rxdata.pg_off, + buff_->len, + AQ_CFG_RX_FRAME_MAX); + page_ref_inc(buff_->rxdata.page); + buff_->is_cleaned = 1; - } while (!buff_->is_eop); - } + buff->is_ip_cso &= buff_->is_ip_cso; + buff->is_udp_cso &= buff_->is_udp_cso; + buff->is_tcp_cso &= buff_->is_tcp_cso; + buff->is_cso_err |= buff_->is_cso_err; + + } while (!buff_->is_eop); } if (buff->is_vlan)