Message ID | 20240305020153.2787423-1-almasrymina@google.com (mailing list archive) |
---|---|
Headers |
Received: from am.mirrors.kernel.org ([147.75.80.249]) by linuxtv.org with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.96) (envelope-from <linux-media+bounces-6387-patchwork=linuxtv.org@vger.kernel.org>) id 1rhK8E-0006ev-07 for patchwork@linuxtv.org; Tue, 05 Mar 2024 02:02:17 +0000 Received: from smtp.subspace.kernel.org (wormhole.subspace.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by am.mirrors.kernel.org (Postfix) with ESMTPS id 302E31F24D29 for <patchwork@linuxtv.org>; Tue, 5 Mar 2024 02:02:10 +0000 (UTC) Received: from localhost.localdomain (localhost.localdomain [127.0.0.1]) by smtp.subspace.kernel.org (Postfix) with ESMTP id EC0E017C9E; Tue, 5 Mar 2024 02:02:01 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b="34ZrN8xf" X-Original-To: linux-media@vger.kernel.org Received: from mail-yw1-f201.google.com (mail-yw1-f201.google.com [209.85.128.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 6A5AB17C95 for <linux-media@vger.kernel.org>; Tue, 5 Mar 2024 02:01:58 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.128.201 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1709604121; cv=none; b=kA5PvINc7e/KcAhp24COjkSkTTuWy7NmPXEZqqGjthekdVcpt4kG8UGi4mPr50p3myItyGLMhnWZk1+tupVWYYKA3m5RTaYvfgoFrMqluT/mA1TwAgIEXUSggFOgjEcNhKVENTZGkY4yydKs3bFcPHr8Ue6h7EOtKmcxLGXIM7k= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1709604121; c=relaxed/simple; bh=5SJCpvoT4gZvZaTMx89Ghuq+/9NUs5gsmK3PbeWX42Y=; h=Date:Mime-Version:Message-ID:Subject:From:To:Cc:Content-Type; b=MIzzPOu4JYe31BjlJRWcn15IyMSaNbQ5j/tLHNVRNjGkJwujZ1SsEhcCcBnejt3hCeG3+ZZqoasfcaqtyf4eJZfYzlqs8H1lsyvXkVP/7oRsl6Xh7QWWksYvJC0jpaRcePktl1F72PADQKSHai47Fen/NsY6NjjUP8SiaLBUAQo= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=google.com; spf=pass smtp.mailfrom=flex--almasrymina.bounces.google.com; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b=34ZrN8xf; arc=none smtp.client-ip=209.85.128.201 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=google.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=flex--almasrymina.bounces.google.com Received: by mail-yw1-f201.google.com with SMTP id 00721157ae682-6047fed0132so78786117b3.1 for <linux-media@vger.kernel.org>; Mon, 04 Mar 2024 18:01:58 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1709604117; x=1710208917; darn=vger.kernel.org; h=content-transfer-encoding:cc:to:from:subject:message-id :mime-version:date:from:to:cc:subject:date:message-id:reply-to; bh=55SlYXskgAbUC07Ocxvb4GPfka9yxb5WpBWQrml1mno=; b=34ZrN8xfC1aNn9ebckh9Lj67fgvG6ae773KeainKXvBMwapu14aA9leQgVh5fsDy8Z 3KBLQFhaoin+n26WL9z5+4wIqcwug6D3+tVWEqlUWz9Nr7qeiNV4JSqEyG2BgO1MRZ5S DVTK1oePmCQkEZHZDTZIWM1UdWkapyFzu0nzABJOhhThkMMQev48f1KQwpER7wesukTU O/VSuU7s+RvbMyadbpQlfbsr15XmqUS1C0ivwmGU/LRlHELyYuNyJoYDxH/1gzWerToK MtamZdOsHCi/4wk3c6IlL8gbVG2HwT6ky8LE2RhzGp/7HCpLVcV4d4E8sKorWbXTo1z8 iWmg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1709604117; x=1710208917; h=content-transfer-encoding:cc:to:from:subject:message-id :mime-version:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=55SlYXskgAbUC07Ocxvb4GPfka9yxb5WpBWQrml1mno=; b=VLgC86mRlZJmNwo6zo2r7zfrBbONX5KTf4dqWe57rKt5B6MC3wtxgUlK3YdUDYfZgQ MjIHu7lEAWgMDdIlVmGS98nleb+IQzLfVvNejL3mzdTU/ZoeXZTWsBxvb/VXtmi8JdAt 4Qx6minf4lVQZMd2jjDpjCkQCrPmGxwMphU/Sj6p7XZNW0Gty0EW+PZcMVa/loD0WabV l5Lu/S4x026nkujC66rODDsE8q8pcRR1J4nrO/ikq8LdC9W8Ku/X077uEdSI5SZlFu6m myL0D/jBZ+UuliMQXfYPF3GCs+5yjxkJnjE5N5wlXBl8AYYXhT6edtlRdQGZPQOI/1Oa KPng== X-Forwarded-Encrypted: i=1; AJvYcCWK3Od2lKOGkQhLBDg5G/FDUr4QT67RBFdyPnzXDRUFhvhXNQQeoyh6R1GgQLR72E75Y3kI3jAf56qgmjspXGU2N31ruecIhZF9bP4= X-Gm-Message-State: AOJu0Yy6RIyEUWiO7YEk5pG3mxP3tHdh9OCqrkEjHiMPq72ZsRNmob50 3N78PADh5JvSKFtfEwDMl2LuFJPHcM4XZjccA2oU7phmR0JbSzSz6cXiMy1ZeKfWTaGdDJ6UztA eUedFXu63PWfJ1YSzEbQFhQ== X-Google-Smtp-Source: AGHT+IGgP9lxCpbwb0hThtM10r7DBqXWOVEk2Uw392R6Iks3K5NMb+79+YKWvZAz/yW3nimazxZaTBRJtMLl6xuDug== X-Received: from almasrymina.svl.corp.google.com ([2620:15c:2c4:200:b614:914c:63cd:3830]) (user=almasrymina job=sendgmr) by 2002:a25:c794:0:b0:dcb:c2c0:b319 with SMTP id w142-20020a25c794000000b00dcbc2c0b319mr462976ybe.9.1709604117458; Mon, 04 Mar 2024 18:01:57 -0800 (PST) Date: Mon, 4 Mar 2024 18:01:35 -0800 Precedence: bulk X-Mailing-List: linux-media@vger.kernel.org List-Id: <linux-media.vger.kernel.org> List-Subscribe: <mailto:linux-media+subscribe@vger.kernel.org> List-Unsubscribe: <mailto:linux-media+unsubscribe@vger.kernel.org> Mime-Version: 1.0 X-Mailer: git-send-email 2.44.0.rc1.240.g4c46232300-goog Message-ID: <20240305020153.2787423-1-almasrymina@google.com> Subject: [RFC PATCH net-next v6 00/15] Device Memory TCP From: Mina Almasry <almasrymina@google.com> To: netdev@vger.kernel.org, linux-kernel@vger.kernel.org, linux-doc@vger.kernel.org, linux-alpha@vger.kernel.org, linux-mips@vger.kernel.org, linux-parisc@vger.kernel.org, sparclinux@vger.kernel.org, linux-trace-kernel@vger.kernel.org, linux-arch@vger.kernel.org, bpf@vger.kernel.org, linux-kselftest@vger.kernel.org, linux-media@vger.kernel.org, dri-devel@lists.freedesktop.org Cc: Mina Almasry <almasrymina@google.com>, "David S. Miller" <davem@davemloft.net>, Eric Dumazet <edumazet@google.com>, Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>, Jonathan Corbet <corbet@lwn.net>, Richard Henderson <richard.henderson@linaro.org>, Ivan Kokshaysky <ink@jurassic.park.msu.ru>, Matt Turner <mattst88@gmail.com>, Thomas Bogendoerfer <tsbogend@alpha.franken.de>, "James E.J. Bottomley" <James.Bottomley@HansenPartnership.com>, Helge Deller <deller@gmx.de>, Andreas Larsson <andreas@gaisler.com>, Jesper Dangaard Brouer <hawk@kernel.org>, Ilias Apalodimas <ilias.apalodimas@linaro.org>, Steven Rostedt <rostedt@goodmis.org>, Masami Hiramatsu <mhiramat@kernel.org>, Mathieu Desnoyers <mathieu.desnoyers@efficios.com>, Arnd Bergmann <arnd@arndb.de>, Alexei Starovoitov <ast@kernel.org>, Daniel Borkmann <daniel@iogearbox.net>, Andrii Nakryiko <andrii@kernel.org>, Martin KaFai Lau <martin.lau@linux.dev>, Eduard Zingerman <eddyz87@gmail.com>, Song Liu <song@kernel.org>, Yonghong Song <yonghong.song@linux.dev>, John Fastabend <john.fastabend@gmail.com>, KP Singh <kpsingh@kernel.org>, Stanislav Fomichev <sdf@google.com>, Hao Luo <haoluo@google.com>, Jiri Olsa <jolsa@kernel.org>, David Ahern <dsahern@kernel.org>, Willem de Bruijn <willemdebruijn.kernel@gmail.com>, Shuah Khan <shuah@kernel.org>, Sumit Semwal <sumit.semwal@linaro.org>, " =?utf-8?q?Christian_K=C3=B6nig?= " <christian.koenig@amd.com>, Pavel Begunkov <asml.silence@gmail.com>, David Wei <dw@davidwei.uk>, Jason Gunthorpe <jgg@ziepe.ca>, Yunsheng Lin <linyunsheng@huawei.com>, Shailend Chand <shailend@google.com>, Harshitha Ramamurthy <hramamurthy@google.com>, Shakeel Butt <shakeelb@google.com>, Jeroen de Borst <jeroendb@google.com>, Praveen Kaligineedi <pkaligineedi@google.com> Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-LSpam-Score: -9.9 (---------) X-LSpam-Report: No, score=-9.9 required=5.0 tests=ARC_SIGNED=0.001,ARC_VALID=-0.1,DKIMWL_WL_MED=-1,DKIM_SIGNED=0.1,DKIM_VALID=-0.1,DKIM_VALID_AU=-0.1,DMARC_PASS=-0.001,HEADER_FROM_DIFFERENT_DOMAINS=0.5,MAILING_LIST_MULTI=-1,RCVD_IN_DNSWL_LOW=-0.7,SPF_HELO_NONE=0.001,SPF_PASS=-0.001,USER_IN_DEF_DKIM_WL=-7.5 autolearn=unavailable autolearn_force=no |
Series |
Device Memory TCP
|
|
Message
Mina Almasry
March 5, 2024, 2:01 a.m. UTC
RFC v6: ======= Major Changes: -------------- This revision largely rebases on top of net-next and addresses the little feedback RFCv5 received. The series remains in RFC because the queue-API ndos defined in this series are not yet implemented. I have a GVE implementation I carry out of tree for my testing. A upstreamable GVE implementation is in the works. Aside from that, in my estimation all the patches are ready for review/merge. Please do take a look. As usual the full devmem TCP changes including the full GVE driver implementation is here: https://github.com/mina/linux/commits/tcpdevmem-v6/ This version also comes with some performance data recorded in the cover letter (see below changelog). Detailed changelog: - Rebased on top of the merged netmem_ref changes. - Converted skb->dmabuf to skb->readable (Pavel). Pavel's original suggestion was to remove the skb->dmabuf flag entirely, but when I looked into it closely, I found the issue that if we remove the flag we have to dereference the shinfo(skb) pointer to obtain the first frag to tell whether an skb is readable or not. This can cause a performance regression if it dirties the cache line when the shinfo(skb) was not really needed. Instead, I converted the skb->dmabuf flag into a generic skb->readable flag which can be re-used by io_uring 0-copy RX. - Squashed a few locking optimizations from Eric Dumazet in the RX path and the DEVMEM_DONTNEED setsockopt. - Expanded the tests a bit. Added validation for invalid scenarios and added some more coverage. Perf - page-pool benchmark: --------------------------- bench_page_pool_simple.ko tests with and without these changes: https://pastebin.com/raw/ncHDwAbn AFAIK the number that really matters in the perf tests is the 'tasklet_page_pool01_fast_path Per elem'. This one measures at about 8 cycles without the changes but there is some 1 cycle noise in some results. With the patches this regresses to 9 cycles with the changes but there is 1 cycle noise occasionally running this test repeatedly. Lastly I tried disable the static_branch_unlikely() in netmem_is_net_iov() check. To my surprise disabling the static_branch_unlikely() check reduces the fast path back to 8 cycles, but the 1 cycle noise remains. Perf - Devmem TCP benchmark: --------------------- 189/200gbps bi-directional throughput with RX devmem TCP and regular TCP TX i.e. ~95% line rate. Major changes in RFC v5: ======================== 1. Rebased on top of 'Abstract page from net stack' series and used the new netmem type to refer to LSB set pointers instead of re-using struct page. 2. Downgraded this series back to RFC and called it RFC v5. This is because this series is now dependent on 'Abstract page from net stack'[1] and the queue API. Both are removed from the series to reduce the patch # and those bits are fairly independent or pre-requisite work. 3. Reworked the page_pool devmem support to use netmem and for some more unified handling. 4. Reworked the reference counting of net_iov (renamed from page_pool_iov) to use pp_ref_count for refcounting. The full changes including the dependent series and GVE page pool support is here: https://github.com/mina/linux/commits/tcpdevmem-rfcv5/ [1] https://patchwork.kernel.org/project/netdevbpf/list/?series=810774 Major changes in v1: ==================== 1. Implemented MVP queue API ndos to remove the userspace-visible driver reset. 2. Fixed issues in the napi_pp_put_page() devmem frag unref path. 3. Removed RFC tag. Many smaller addressed comments across all the patches (patches have individual change log). Full tree including the rest of the GVE driver changes: https://github.com/mina/linux/commits/tcpdevmem-v1 Changes in RFC v3: ================== 1. Pulled in the memory-provider dependency from Jakub's RFC[1] to make the series reviewable and mergeable. 2. Implemented multi-rx-queue binding which was a todo in v2. 3. Fix to cmsg handling. The sticking point in RFC v2[2] was the device reset required to refill the device rx-queues after the dmabuf bind/unbind. The solution suggested as I understand is a subset of the per-queue management ops Jakub suggested or similar: https://lore.kernel.org/netdev/20230815171638.4c057dcd@kernel.org/ This is not addressed in this revision, because: 1. This point was discussed at netconf & netdev and there is openness to using the current approach of requiring a device reset. 2. Implementing individual queue resetting seems to be difficult for my test bed with GVE. My prototype to test this ran into issues with the rx-queues not coming back up properly if reset individually. At the moment I'm unsure if it's a mistake in the POC or a genuine issue in the virtualization stack behind GVE, which currently doesn't test individual rx-queue restart. 3. Our usecases are not bothered by requiring a device reset to refill the buffer queues, and we'd like to support NICs that run into this limitation with resetting individual queues. My thought is that drivers that have trouble with per-queue configs can use the support in this series, while drivers that support new netdev ops to reset individual queues can automatically reset the queue as part of the dma-buf bind/unbind. The same approach with device resets is presented again for consideration with other sticking points addressed. This proposal includes the rx devmem path only proposed for merge. For a snapshot of my entire tree which includes the GVE POC page pool support & device memory support: https://github.com/torvalds/linux/compare/master...mina:linux:tcpdevmem-v3 [1] https://lore.kernel.org/netdev/f8270765-a27b-6ccf-33ea-cda097168d79@redhat.com/T/ [2] https://lore.kernel.org/netdev/CAHS8izOVJGJH5WF68OsRWFKJid1_huzzUK+hpKbLcL4pSOD1Jw@mail.gmail.com/T/ Changes in RFC v2: ================== The sticking point in RFC v1[1] was the dma-buf pages approach we used to deliver the device memory to the TCP stack. RFC v2 is a proof-of-concept that attempts to resolve this by implementing scatterlist support in the networking stack, such that we can import the dma-buf scatterlist directly. This is the approach proposed at a high level here[2]. Detailed changes: 1. Replaced dma-buf pages approach with importing scatterlist into the page pool. 2. Replace the dma-buf pages centric API with a netlink API. 3. Removed the TX path implementation - there is no issue with implementing the TX path with scatterlist approach, but leaving out the TX path makes it easier to review. 4. Functionality is tested with this proposal, but I have not conducted perf testing yet. I'm not sure there are regressions, but I removed perf claims from the cover letter until they can be re-confirmed. 5. Added Signed-off-by: contributors to the implementation. 6. Fixed some bugs with the RX path since RFC v1. Any feedback welcome, but specifically the biggest pending questions needing feedback IMO are: 1. Feedback on the scatterlist-based approach in general. 2. Netlink API (Patch 1 & 2). 3. Approach to handle all the drivers that expect to receive pages from the page pool (Patch 6). [1] https://lore.kernel.org/netdev/dfe4bae7-13a0-3c5d-d671-f61b375cb0b4@gmail.com/T/ [2] https://lore.kernel.org/netdev/CAHS8izPm6XRS54LdCDZVd0C75tA1zHSu6jLVO8nzTLXCc=H7Nw@mail.gmail.com/ ================== * TL;DR: Device memory TCP (devmem TCP) is a proposal for transferring data to and/or from device memory efficiently, without bouncing the data to a host memory buffer. * Problem: A large amount of data transfers have device memory as the source and/or destination. Accelerators drastically increased the volume of such transfers. Some examples include: - ML accelerators transferring large amounts of training data from storage into GPU/TPU memory. In some cases ML training setup time can be as long as 50% of TPU compute time, improving data transfer throughput & efficiency can help improving GPU/TPU utilization. - Distributed training, where ML accelerators, such as GPUs on different hosts, exchange data among them. - Distributed raw block storage applications transfer large amounts of data with remote SSDs, much of this data does not require host processing. Today, the majority of the Device-to-Device data transfers the network are implemented as the following low level operations: Device-to-Host copy, Host-to-Host network transfer, and Host-to-Device copy. The implementation is suboptimal, especially for bulk data transfers, and can put significant strains on system resources, such as host memory bandwidth, PCIe bandwidth, etc. One important reason behind the current state is the kernel’s lack of semantics to express device to network transfers. * Proposal: In this patch series we attempt to optimize this use case by implementing socket APIs that enable the user to: 1. send device memory across the network directly, and 2. receive incoming network packets directly into device memory. Packet _payloads_ go directly from the NIC to device memory for receive and from device memory to NIC for transmit. Packet _headers_ go to/from host memory and are processed by the TCP/IP stack normally. The NIC _must_ support header split to achieve this. Advantages: - Alleviate host memory bandwidth pressure, compared to existing network-transfer + device-copy semantics. - Alleviate PCIe BW pressure, by limiting data transfer to the lowest level of the PCIe tree, compared to traditional path which sends data through the root complex. * Patch overview: ** Part 1: netlink API Gives user ability to bind dma-buf to an RX queue. ** Part 2: scatterlist support Currently the standard for device memory sharing is DMABUF, which doesn't generate struct pages. On the other hand, networking stack (skbs, drivers, and page pool) operate on pages. We have 2 options: 1. Generate struct pages for dmabuf device memory, or, 2. Modify the networking stack to process scatterlist. Approach #1 was attempted in RFC v1. RFC v2 implements approach #2. ** part 3: page pool support We piggy back on page pool memory providers proposal: https://github.com/kuba-moo/linux/tree/pp-providers It allows the page pool to define a memory provider that provides the page allocation and freeing. It helps abstract most of the device memory TCP changes from the driver. ** part 4: support for unreadable skb frags Page pool iovs are not accessible by the host; we implement changes throughput the networking stack to correctly handle skbs with unreadable frags. ** Part 5: recvmsg() APIs We define user APIs for the user to send and receive device memory. Not included with this series is the GVE devmem TCP support, just to simplify the review. Code available here if desired: https://github.com/mina/linux/tree/tcpdevmem This series is built on top of net-next with Jakub's pp-providers changes cherry-picked. * NIC dependencies: 1. (strict) Devmem TCP require the NIC to support header split, i.e. the capability to split incoming packets into a header + payload and to put each into a separate buffer. Devmem TCP works by using device memory for the packet payload, and host memory for the packet headers. 2. (optional) Devmem TCP works better with flow steering support & RSS support, i.e. the NIC's ability to steer flows into certain rx queues. This allows the sysadmin to enable devmem TCP on a subset of the rx queues, and steer devmem TCP traffic onto these queues and non devmem TCP elsewhere. The NIC I have access to with these properties is the GVE with DQO support running in Google Cloud, but any NIC that supports these features would suffice. I may be able to help reviewers bring up devmem TCP on their NICs. * Testing: The series includes a udmabuf kselftest that show a simple use case of devmem TCP and validates the entire data path end to end without a dependency on a specific dmabuf provider. ** Test Setup Kernel: net-next with this series and memory provider API cherry-picked locally. Hardware: Google Cloud A3 VMs. NIC: GVE with header split & RSS & flow steering support. Cc: Pavel Begunkov <asml.silence@gmail.com> Cc: David Wei <dw@davidwei.uk> Cc: Jason Gunthorpe <jgg@ziepe.ca> Cc: Yunsheng Lin <linyunsheng@huawei.com> Cc: Shailend Chand <shailend@google.com> Cc: Harshitha Ramamurthy <hramamurthy@google.com> Cc: Shakeel Butt <shakeelb@google.com> Cc: Jeroen de Borst <jeroendb@google.com> Cc: Praveen Kaligineedi <pkaligineedi@google.com> Jakub Kicinski (1): net: page_pool: create hooks for custom page providers Mina Almasry (14): queue_api: define queue api net: page_pool: factor out page_pool recycle check net: netdev netlink api to bind dma-buf to a net device netdev: support binding dma-buf to netdevice netdev: netdevice devmem allocator page_pool: convert to use netmem page_pool: devmem support memory-provider: dmabuf devmem memory provider net: support non paged skb frags net: add support for skbs with unreadable frags tcp: RX path for devmem TCP net: add SO_DEVMEM_DONTNEED setsockopt to release RX frags net: add devmem TCP documentation selftests: add ncdevmem, netcat for devmem TCP Documentation/netlink/specs/netdev.yaml | 52 +++ Documentation/networking/devmem.rst | 271 ++++++++++++ Documentation/networking/index.rst | 1 + arch/alpha/include/uapi/asm/socket.h | 6 + arch/mips/include/uapi/asm/socket.h | 6 + arch/parisc/include/uapi/asm/socket.h | 6 + arch/sparc/include/uapi/asm/socket.h | 6 + include/linux/netdevice.h | 24 ++ include/linux/skbuff.h | 67 ++- include/linux/socket.h | 1 + include/net/devmem.h | 127 ++++++ include/net/netdev_rx_queue.h | 1 + include/net/netmem.h | 234 +++++++++- include/net/page_pool/helpers.h | 154 +++++-- include/net/page_pool/types.h | 28 +- include/net/sock.h | 2 + include/net/tcp.h | 5 +- include/trace/events/page_pool.h | 29 +- include/uapi/asm-generic/socket.h | 6 + include/uapi/linux/netdev.h | 19 + include/uapi/linux/uio.h | 14 + net/bpf/test_run.c | 5 +- net/core/Makefile | 2 +- net/core/datagram.c | 6 + net/core/dev.c | 6 +- net/core/devmem.c | 413 ++++++++++++++++++ net/core/gro.c | 7 +- net/core/netdev-genl-gen.c | 19 + net/core/netdev-genl-gen.h | 2 + net/core/netdev-genl.c | 123 ++++++ net/core/page_pool.c | 362 +++++++++------- net/core/skbuff.c | 110 ++++- net/core/sock.c | 61 +++ net/ipv4/tcp.c | 257 ++++++++++- net/ipv4/tcp_input.c | 13 +- net/ipv4/tcp_ipv4.c | 9 + net/ipv4/tcp_output.c | 5 +- net/packet/af_packet.c | 4 +- tools/include/uapi/linux/netdev.h | 19 + tools/testing/selftests/net/.gitignore | 1 + tools/testing/selftests/net/Makefile | 5 + tools/testing/selftests/net/ncdevmem.c | 546 ++++++++++++++++++++++++ 42 files changed, 2764 insertions(+), 270 deletions(-) create mode 100644 Documentation/networking/devmem.rst create mode 100644 include/net/devmem.h create mode 100644 net/core/devmem.c create mode 100644 tools/testing/selftests/net/ncdevmem.c
Comments
On 2024/3/5 10:01, Mina Almasry wrote: ... > > Perf - page-pool benchmark: > --------------------------- > > bench_page_pool_simple.ko tests with and without these changes: > https://pastebin.com/raw/ncHDwAbn > > AFAIK the number that really matters in the perf tests is the > 'tasklet_page_pool01_fast_path Per elem'. This one measures at about 8 > cycles without the changes but there is some 1 cycle noise in some > results. > > With the patches this regresses to 9 cycles with the changes but there > is 1 cycle noise occasionally running this test repeatedly. > > Lastly I tried disable the static_branch_unlikely() in > netmem_is_net_iov() check. To my surprise disabling the > static_branch_unlikely() check reduces the fast path back to 8 cycles, > but the 1 cycle noise remains. > The last sentence seems to be suggesting the above 1 ns regresses is caused by the static_branch_unlikely() checking?
On Tue, Mar 5, 2024 at 4:54 AM Yunsheng Lin <linyunsheng@huawei.com> wrote: > > On 2024/3/5 10:01, Mina Almasry wrote: > > ... > > > > > Perf - page-pool benchmark: > > --------------------------- > > > > bench_page_pool_simple.ko tests with and without these changes: > > https://pastebin.com/raw/ncHDwAbn > > > > AFAIK the number that really matters in the perf tests is the > > 'tasklet_page_pool01_fast_path Per elem'. This one measures at about 8 > > cycles without the changes but there is some 1 cycle noise in some > > results. > > > > With the patches this regresses to 9 cycles with the changes but there > > is 1 cycle noise occasionally running this test repeatedly. > > > > Lastly I tried disable the static_branch_unlikely() in > > netmem_is_net_iov() check. To my surprise disabling the > > static_branch_unlikely() check reduces the fast path back to 8 cycles, > > but the 1 cycle noise remains. > > > > The last sentence seems to be suggesting the above 1 ns regresses is caused > by the static_branch_unlikely() checking? Note it's not a 1ns regression, it's looks like maybe a 1 cycle regression (slightly less than 1ns if I'm reading the output of the test correctly): # clean net-next time_bench: Type:tasklet_page_pool01_fast_path Per elem: 8 cycles(tsc) 2.993 ns (step:0) # with patches time_bench: Type:tasklet_page_pool01_fast_path Per elem: 9 cycles(tsc) 3.679 ns (step:0) # with patches and with diff that disables static branching: time_bench: Type:tasklet_page_pool01_fast_path Per elem: 8 cycles(tsc) 3.248 ns (step:0) I do see noise in the test results between run and run, and any regression (if any) is slightly obfuscated by the noise, so it's a bit hard to make confident statements. So far it looks like a ~0.25ns regression without static branch and about ~0.65ns with static branch. Honestly when I saw all 3 results were within some noise I did not investigate more, but if this looks concerning to you I can dig further. I likely need to gather a few test runs to filter out the noise and maybe investigate the assembly my compiler is generating to maybe narrow down what changes there.
On 2024/3/6 3:38, Mina Almasry wrote: > On Tue, Mar 5, 2024 at 4:54 AM Yunsheng Lin <linyunsheng@huawei.com> wrote: >> >> On 2024/3/5 10:01, Mina Almasry wrote: >> >> ... >> >>> >>> Perf - page-pool benchmark: >>> --------------------------- >>> >>> bench_page_pool_simple.ko tests with and without these changes: >>> https://pastebin.com/raw/ncHDwAbn >>> >>> AFAIK the number that really matters in the perf tests is the >>> 'tasklet_page_pool01_fast_path Per elem'. This one measures at about 8 >>> cycles without the changes but there is some 1 cycle noise in some >>> results. >>> >>> With the patches this regresses to 9 cycles with the changes but there >>> is 1 cycle noise occasionally running this test repeatedly. >>> >>> Lastly I tried disable the static_branch_unlikely() in >>> netmem_is_net_iov() check. To my surprise disabling the >>> static_branch_unlikely() check reduces the fast path back to 8 cycles, >>> but the 1 cycle noise remains. >>> >> >> The last sentence seems to be suggesting the above 1 ns regresses is caused >> by the static_branch_unlikely() checking? > > Note it's not a 1ns regression, it's looks like maybe a 1 cycle > regression (slightly less than 1ns if I'm reading the output of the > test correctly): > > # clean net-next > time_bench: Type:tasklet_page_pool01_fast_path Per elem: 8 cycles(tsc) > 2.993 ns (step:0) > > # with patches > time_bench: Type:tasklet_page_pool01_fast_path Per elem: 9 cycles(tsc) > 3.679 ns (step:0) > > # with patches and with diff that disables static branching: > time_bench: Type:tasklet_page_pool01_fast_path Per elem: 8 cycles(tsc) > 3.248 ns (step:0) > > I do see noise in the test results between run and run, and any > regression (if any) is slightly obfuscated by the noise, so it's a bit > hard to make confident statements. So far it looks like a ~0.25ns > regression without static branch and about ~0.65ns with static branch. > > Honestly when I saw all 3 results were within some noise I did not > investigate more, but if this looks concerning to you I can dig > further. I likely need to gather a few test runs to filter out the > noise and maybe investigate the assembly my compiler is generating to > maybe narrow down what changes there. Yes, that is confusing enough that need more investigation. >
On Tue, Mar 5, 2024 at 11:38 AM Mina Almasry <almasrymina@google.com> wrote: > > On Tue, Mar 5, 2024 at 4:54 AM Yunsheng Lin <linyunsheng@huawei.com> wrote: > > > > On 2024/3/5 10:01, Mina Almasry wrote: > > > > ... > > > > > > > > Perf - page-pool benchmark: > > > --------------------------- > > > > > > bench_page_pool_simple.ko tests with and without these changes: > > > https://pastebin.com/raw/ncHDwAbn > > > > > > AFAIK the number that really matters in the perf tests is the > > > 'tasklet_page_pool01_fast_path Per elem'. This one measures at about 8 > > > cycles without the changes but there is some 1 cycle noise in some > > > results. > > > > > > With the patches this regresses to 9 cycles with the changes but there > > > is 1 cycle noise occasionally running this test repeatedly. > > > > > > Lastly I tried disable the static_branch_unlikely() in > > > netmem_is_net_iov() check. To my surprise disabling the > > > static_branch_unlikely() check reduces the fast path back to 8 cycles, > > > but the 1 cycle noise remains. > > > > > > > The last sentence seems to be suggesting the above 1 ns regresses is caused > > by the static_branch_unlikely() checking? > > Note it's not a 1ns regression, it's looks like maybe a 1 cycle > regression (slightly less than 1ns if I'm reading the output of the > test correctly): > > # clean net-next > time_bench: Type:tasklet_page_pool01_fast_path Per elem: 8 cycles(tsc) > 2.993 ns (step:0) > > # with patches > time_bench: Type:tasklet_page_pool01_fast_path Per elem: 9 cycles(tsc) > 3.679 ns (step:0) > > # with patches and with diff that disables static branching: > time_bench: Type:tasklet_page_pool01_fast_path Per elem: 8 cycles(tsc) > 3.248 ns (step:0) > > I do see noise in the test results between run and run, and any > regression (if any) is slightly obfuscated by the noise, so it's a bit > hard to make confident statements. So far it looks like a ~0.25ns > regression without static branch and about ~0.65ns with static branch. > > Honestly when I saw all 3 results were within some noise I did not > investigate more, but if this looks concerning to you I can dig > further. I likely need to gather a few test runs to filter out the > noise and maybe investigate the assembly my compiler is generating to > maybe narrow down what changes there. > I did some more investigation here to gather more data to filter out the noise, and recorded the summary here: https://pastebin.com/raw/v5dYRg8L Long story short, the page_pool benchmark results are consistent with some outlier noise results that I'm discounting here. Currently page_pool fast path is at 8 cycles [ 2115.724510] time_bench: Type:tasklet_page_pool01_fast_path Per elem: 8 cycles(tsc) 3.187 ns (step:0) - (measurement period time:0.031870585 sec time_interval:31870585) - (invoke count:10000000 tsc_interval:86043192) and with this patch series it degrades to 10 cycles, or about a 0.7ns degradation or so: [ 498.226127] time_bench: Type:tasklet_page_pool01_fast_path Per elem: 10 cycles(tsc) 3.944 ns (step:0) - (measurement period time:0.039442539 sec time_interval:39442539) - (invoke count:10000000 tsc_interval:106485268) I took the time to dig into where the degradation comes from, and to my surprise we can shave off 1 cycle in perf by removing the static_branch_unlikely check in netmem_is_net_iov() like so: diff --git a/include/net/netmem.h b/include/net/netmem.h index fe354d11a421..2b4310ac1115 100644 --- a/include/net/netmem.h +++ b/include/net/netmem.h @@ -122,8 +122,7 @@ typedef unsigned long __bitwise netmem_ref; static inline bool netmem_is_net_iov(const netmem_ref netmem) { #ifdef CONFIG_PAGE_POOL - return static_branch_unlikely(&page_pool_mem_providers) && - (__force unsigned long)netmem & NET_IOV; + return (__force unsigned long)netmem & NET_IOV; #else return false; #endif With this change, the fast path is 9 cycles, only a 1 cycle (~0.35ns) regression: [ 199.184429] time_bench: Type:tasklet_page_pool01_fast_path Per elem: 9 cycles(tsc) 3.552 ns (step:0) - (measurement period time:0.035524013 sec time_interval:35524013) - (invoke count:10000000 tsc_interval:95907775) I did some digging with YiFei on why the static_branch_unlikely appears to be causing a 1 cycle regression, but could not get an answer that makes sense. The # of instructions in page_pool_return_page() with the static_branch_unlikely and without is about the same in the compiled .o file, and my understanding is that static_branch will cause code re-writing anyway so looking at the compiled code may not be representative. Worthy of note is that I get ~95% line rate of devmem TCP regardless of the static_branch_unlikely() or not, so impact of the static_branch is not large enough to be measurable end-to-end. I'm thinking I want to drop the static_branch_unlikely() in the next RFC since it doesn't improve the end-to-end throughput number and is resulting in a measurable improvement in the page pool benchmark.
On 2024/3/26 8:28, Mina Almasry wrote: > On Tue, Mar 5, 2024 at 11:38 AM Mina Almasry <almasrymina@google.com> wrote: >> >> On Tue, Mar 5, 2024 at 4:54 AM Yunsheng Lin <linyunsheng@huawei.com> wrote: >>> >>> On 2024/3/5 10:01, Mina Almasry wrote: >>> >>> ... >>> >>>> >>>> Perf - page-pool benchmark: >>>> --------------------------- >>>> >>>> bench_page_pool_simple.ko tests with and without these changes: >>>> https://pastebin.com/raw/ncHDwAbn >>>> >>>> AFAIK the number that really matters in the perf tests is the >>>> 'tasklet_page_pool01_fast_path Per elem'. This one measures at about 8 >>>> cycles without the changes but there is some 1 cycle noise in some >>>> results. >>>> >>>> With the patches this regresses to 9 cycles with the changes but there >>>> is 1 cycle noise occasionally running this test repeatedly. >>>> >>>> Lastly I tried disable the static_branch_unlikely() in >>>> netmem_is_net_iov() check. To my surprise disabling the >>>> static_branch_unlikely() check reduces the fast path back to 8 cycles, >>>> but the 1 cycle noise remains. >>>> >>> >>> The last sentence seems to be suggesting the above 1 ns regresses is caused >>> by the static_branch_unlikely() checking? >> >> Note it's not a 1ns regression, it's looks like maybe a 1 cycle >> regression (slightly less than 1ns if I'm reading the output of the >> test correctly): >> >> # clean net-next >> time_bench: Type:tasklet_page_pool01_fast_path Per elem: 8 cycles(tsc) >> 2.993 ns (step:0) >> >> # with patches >> time_bench: Type:tasklet_page_pool01_fast_path Per elem: 9 cycles(tsc) >> 3.679 ns (step:0) >> >> # with patches and with diff that disables static branching: >> time_bench: Type:tasklet_page_pool01_fast_path Per elem: 8 cycles(tsc) >> 3.248 ns (step:0) >> >> I do see noise in the test results between run and run, and any >> regression (if any) is slightly obfuscated by the noise, so it's a bit >> hard to make confident statements. So far it looks like a ~0.25ns >> regression without static branch and about ~0.65ns with static branch. >> >> Honestly when I saw all 3 results were within some noise I did not >> investigate more, but if this looks concerning to you I can dig >> further. I likely need to gather a few test runs to filter out the >> noise and maybe investigate the assembly my compiler is generating to >> maybe narrow down what changes there. >> > > I did some more investigation here to gather more data to filter out > the noise, and recorded the summary here: > > https://pastebin.com/raw/v5dYRg8L > > Long story short, the page_pool benchmark results are consistent with > some outlier noise results that I'm discounting here. Currently > page_pool fast path is at 8 cycles > > [ 2115.724510] time_bench: Type:tasklet_page_pool01_fast_path Per > elem: 8 cycles(tsc) 3.187 ns (step:0) - (measurement period > time:0.031870585 sec time_interval:31870585) - (invoke count:10000000 > tsc_interval:86043192) > > and with this patch series it degrades to 10 cycles, or about a 0.7ns > degradation or so: Even if the absolute value for the overhead is small, we seems have a degradation of about 20% for tasklet_page_pool01_fast_path testcase, which seems scary. I am assuming that every page is recyclable for tasklet_page_pool01_fast_path testcase, and that code path matters for page_pool, it would be good to remove any additional checking for that code path. And we already have pool->has_init_callback checking when we have to use a new page, it may make sense to refactor that to share the same checking for provider to avoid the overhead as much as possible. Also, I am not sure if it really matter that much, as with the introducing of netmem_is_net_iov() checking spreading in the networking, the overhead might add up for other case too. > > [ 498.226127] time_bench: Type:tasklet_page_pool01_fast_path Per > elem: 10 cycles(tsc) 3.944 ns (step:0) - (measurement period > time:0.039442539 sec time_interval:39442539) - (invoke count:10000000 > tsc_interval:106485268) > > I took the time to dig into where the degradation comes from, and to > my surprise we can shave off 1 cycle in perf by removing the > static_branch_unlikely check in netmem_is_net_iov() like so: > > diff --git a/include/net/netmem.h b/include/net/netmem.h > index fe354d11a421..2b4310ac1115 100644 > --- a/include/net/netmem.h > +++ b/include/net/netmem.h > @@ -122,8 +122,7 @@ typedef unsigned long __bitwise netmem_ref; > static inline bool netmem_is_net_iov(const netmem_ref netmem) > { > #ifdef CONFIG_PAGE_POOL > - return static_branch_unlikely(&page_pool_mem_providers) && > - (__force unsigned long)netmem & NET_IOV; > + return (__force unsigned long)netmem & NET_IOV; > #else > return false; > #endif > > With this change, the fast path is 9 cycles, only a 1 cycle (~0.35ns) > regression: > > [ 199.184429] time_bench: Type:tasklet_page_pool01_fast_path Per > elem: 9 cycles(tsc) 3.552 ns (step:0) - (measurement period > time:0.035524013 sec time_interval:35524013) - (invoke count:10000000 > tsc_interval:95907775) > > I did some digging with YiFei on why the static_branch_unlikely > appears to be causing a 1 cycle regression, but could not get an > answer that makes sense. The # of instructions in > page_pool_return_page() with the static_branch_unlikely and without is > about the same in the compiled .o file, and my understanding is that > static_branch will cause code re-writing anyway so looking at the > compiled code may not be representative. > > Worthy of note is that I get ~95% line rate of devmem TCP regardless > of the static_branch_unlikely() or not, so impact of the static_branch > is not large enough to be measurable end-to-end. I'm thinking I want > to drop the static_branch_unlikely() in the next RFC since it doesn't > improve the end-to-end throughput number and is resulting in a > measurable improvement in the page pool benchmark. >
On Tue, Mar 26, 2024 at 5:47 AM Yunsheng Lin <linyunsheng@huawei.com> wrote: > > On 2024/3/26 8:28, Mina Almasry wrote: > > On Tue, Mar 5, 2024 at 11:38 AM Mina Almasry <almasrymina@google.com> wrote: > >> > >> On Tue, Mar 5, 2024 at 4:54 AM Yunsheng Lin <linyunsheng@huawei.com> wrote: > >>> > >>> On 2024/3/5 10:01, Mina Almasry wrote: > >>> > >>> ... > >>> > >>>> > >>>> Perf - page-pool benchmark: > >>>> --------------------------- > >>>> > >>>> bench_page_pool_simple.ko tests with and without these changes: > >>>> https://pastebin.com/raw/ncHDwAbn > >>>> > >>>> AFAIK the number that really matters in the perf tests is the > >>>> 'tasklet_page_pool01_fast_path Per elem'. This one measures at about 8 > >>>> cycles without the changes but there is some 1 cycle noise in some > >>>> results. > >>>> > >>>> With the patches this regresses to 9 cycles with the changes but there > >>>> is 1 cycle noise occasionally running this test repeatedly. > >>>> > >>>> Lastly I tried disable the static_branch_unlikely() in > >>>> netmem_is_net_iov() check. To my surprise disabling the > >>>> static_branch_unlikely() check reduces the fast path back to 8 cycles, > >>>> but the 1 cycle noise remains. > >>>> > >>> > >>> The last sentence seems to be suggesting the above 1 ns regresses is caused > >>> by the static_branch_unlikely() checking? > >> > >> Note it's not a 1ns regression, it's looks like maybe a 1 cycle > >> regression (slightly less than 1ns if I'm reading the output of the > >> test correctly): > >> > >> # clean net-next > >> time_bench: Type:tasklet_page_pool01_fast_path Per elem: 8 cycles(tsc) > >> 2.993 ns (step:0) > >> > >> # with patches > >> time_bench: Type:tasklet_page_pool01_fast_path Per elem: 9 cycles(tsc) > >> 3.679 ns (step:0) > >> > >> # with patches and with diff that disables static branching: > >> time_bench: Type:tasklet_page_pool01_fast_path Per elem: 8 cycles(tsc) > >> 3.248 ns (step:0) > >> > >> I do see noise in the test results between run and run, and any > >> regression (if any) is slightly obfuscated by the noise, so it's a bit > >> hard to make confident statements. So far it looks like a ~0.25ns > >> regression without static branch and about ~0.65ns with static branch. > >> > >> Honestly when I saw all 3 results were within some noise I did not > >> investigate more, but if this looks concerning to you I can dig > >> further. I likely need to gather a few test runs to filter out the > >> noise and maybe investigate the assembly my compiler is generating to > >> maybe narrow down what changes there. > >> > > > > I did some more investigation here to gather more data to filter out > > the noise, and recorded the summary here: > > > > https://pastebin.com/raw/v5dYRg8L > > > > Long story short, the page_pool benchmark results are consistent with > > some outlier noise results that I'm discounting here. Currently > > page_pool fast path is at 8 cycles > > > > [ 2115.724510] time_bench: Type:tasklet_page_pool01_fast_path Per > > elem: 8 cycles(tsc) 3.187 ns (step:0) - (measurement period > > time:0.031870585 sec time_interval:31870585) - (invoke count:10000000 > > tsc_interval:86043192) > > > > and with this patch series it degrades to 10 cycles, or about a 0.7ns > > degradation or so: > > Even if the absolute value for the overhead is small, we seems have a > degradation of about 20% for tasklet_page_pool01_fast_path testcase, > which seems scary. > > I am assuming that every page is recyclable for tasklet_page_pool01_fast_path > testcase, and that code path matters for page_pool, it would be good to > remove any additional checking for that code path. > We can remove the usage of static_branch_unlikely in the net_iov check, which reduces the overhead to 1 cycle (8->9), only 12.5% overhead. The addition of the static_branch_unlikely is not improving the performance of devmem TCP anyway. From previous discussions with Jesper he deemed a 1 cycle degradation acceptable, but he hasn't commented in a while, he may have changed his mind but so far no complaints. We can additionally only add the check only if CONFIG_SHARED_DMA_BUFFER is enabled. I've tested that and the fast path goes back to 8 cycles (0 overhead). If CONFIG_SHARED_DMA_BUFFER is not enabled then netmem can't be dmabuf anyway, so no reason to check. > And we already have pool->has_init_callback checking when we have to use > a new page, it may make sense to refactor that to share the same checking > for provider to avoid the overhead as much as possible. > > Also, I am not sure if it really matter that much, as with the introducing > of netmem_is_net_iov() checking spreading in the networking, the overhead > might add up for other case too.