* [PATCH] Fixes a null pointer dereference in ptp_ioctl
From: Yuran Pereira @ 2023-11-06 0:48 UTC (permalink / raw)
To: richardcochran, netdev
Cc: Yuran Pereira, linux-kernel, linux-kernel-mentees,
syzbot+8a78ecea7ac1a2ea26e5
Syzkaller found a null pointer dereference in ptp_ioctl
originating from the lack of a null check for tsevq.
```
general protection fault, probably for non-canonical
address 0xdffffc000000020b: 0000 [#1] PREEMPT SMP KASAN
KASAN: probably user-memory-access in range
[0x0000000000001058-0x000000000000105f]
CPU: 0 PID: 5053 Comm: syz-executor353 Not tainted
6.6.0-syzkaller-10396-g4652b8e4f3ff #0
Hardware name: Google Google Compute Engine/Google Compute Engine,
BIOS Google 10/09/2023
RIP: 0010:ptp_ioctl+0xcb7/0x1d10 drivers/ptp/ptp_chardev.c:476
...
Call Trace:
<TASK>
posix_clock_ioctl+0xf8/0x160 kernel/time/posix-clock.c:86
vfs_ioctl fs/ioctl.c:51 [inline]
__do_sys_ioctl fs/ioctl.c:871 [inline]
__se_sys_ioctl fs/ioctl.c:857 [inline]
__x64_sys_ioctl+0x18f/0x210 fs/ioctl.c:857
do_syscall_x64 arch/x86/entry/common.c:51 [inline]
do_syscall_64+0x3f/0x110 arch/x86/entry/common.c:82
entry_SYSCALL_64_after_hwframe+0x63/0x6b
```
This patch fixes the issue by adding a check for tsevq and
ensuring ptp_ioctl returns with an error if tsevq is null.
Dashboard link: https://syzkaller.appspot.com/bug?extid=8a78ecea7ac1a2ea26e5
Reported-by: syzbot+8a78ecea7ac1a2ea26e5@syzkaller.appspotmail.com
Fixes: c5a445b1e934 ("ptp: support event queue reader channel masks")
Signed-off-by: Yuran Pereira <yuran.pereira@hotmail.com>
---
drivers/ptp/ptp_chardev.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/ptp/ptp_chardev.c b/drivers/ptp/ptp_chardev.c
index 282cd7d24077..5b36c34629a0 100644
--- a/drivers/ptp/ptp_chardev.c
+++ b/drivers/ptp/ptp_chardev.c
@@ -173,6 +173,8 @@ long ptp_ioctl(struct posix_clock_context *pccontext, unsigned int cmd,
int enable, err = 0;
tsevq = pccontext->private_clkdata;
+ if (!tsevq)
+ return -EINVAL;
switch (cmd) {
--
2.25.1
^ permalink raw reply related
* [PATCH] ptp: fix null ptr deref in ptp_ioctrl
From: Lizhi Xu @ 2023-11-06 1:19 UTC (permalink / raw)
To: syzbot+8a78ecea7ac1a2ea26e5
Cc: davem, linux-kernel, netdev, reibax, richardcochran, rrameshbabu,
syzkaller-bugs
In-Reply-To: <000000000000e7b62806096c7d67@google.com>
In the logs recorded in the strace log,
https://syzkaller.appspot.com/text?tag=CrashLog&x=11aa125f680000
...
openat(AT_FDCWD, "/dev/ptp0", O_RDONLY) = 3
read(3, 0x20000080, 90) = -1 EINVAL (Invalid argument)
general protection fault, probably for non-canonical address 0xdffffc000000020b: 0000 [#1] PREEMPT SMP KASAN
KASAN: probably user-memory-access in range [0x0000000000001058-0x000000000000105f]
CPU: 0 PID: 5053 Comm: syz-executor353 Not tainted 6.6.0-syzkaller-10396-g4652b8e4f3ff #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 10/09/2023
RIP: 0010:ptp_ioctl+0xcb7/0x1d10
Code: 81 fe 13 3d 00 00 0f 85 9c 02 00 00 e8 c2 83 23 fa 49 8d bc 24 58 10 00 00 48 b8 00 00 00 00 00 fc ff df 48 89 fa 48 c1 ea 03 <80> 3c 02 00 0f 85 dc 0e 00 00 49 8b bc 24 58 10 00 00 ba 00 01 00
RSP: 0018:ffffc90003a37ba0 EFLAGS: 00010212
RAX: dffffc0000000000 RBX: ffff88814a78a000 RCX: ffffffff8764f81f
RDX: 000000000000020b RSI: ffffffff8765028e RDI: 0000000000001058
RBP: ffffc90003a37ec0 R08: 0000000000000005 R09: ffffc90003a37c40
R10: 0000000000003d13 R11: 0000000000000000 R12: 0000000000000000
R13: ffffc90003a37c80 R14: 0000000000003d13 R15: ffffffff92ac78e8
FS: 00005555569a9380(0000) GS:ffff8880b9800000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 0000000020000040 CR3: 0000000076e09000 CR4: 0000000000350ef0
Call Trace:
<TASK>
? show_regs+0x8f/0xa0
? die_addr+0x4f/0xd0
? exc_general_protection+0x154/0x230
? asm_exc_general_protection+0x26/0x30
? ptp_ioctl+0x22f/0x1d10
? ptp_ioctl+0xc9e/0x1d10
? ptp_ioctl+0xcb7/0x1d10
? ptp_release+0x2b0/0x2b0
? lockdep_hardirqs_on_prepare+0x410/0x410
? lock_sync+0x190/0x190
? find_held_lock+0x2d/0x110
? ptp_release+0x2b0/0x2b0
posix_clock_ioctl+0xf8/0x160
...
It can be confirmed that after the execution of "read (3, 0x20000080, 90)",
ptp_release() will be called to release the queue and set
pccontext->private_clkdata = NULL at the same time, this is unreasonable and
incorrect. The queue is not the memory requested in ptp_read() and should not
be released in ptp_read().
Signed-off-by: Lizhi Xu <lizhi.xu@windriver.com>
---
drivers/ptp/ptp_chardev.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/ptp/ptp_chardev.c b/drivers/ptp/ptp_chardev.c
index 282cd7d24077..27c1ef493617 100644
--- a/drivers/ptp/ptp_chardev.c
+++ b/drivers/ptp/ptp_chardev.c
@@ -585,7 +585,5 @@ ssize_t ptp_read(struct posix_clock_context *pccontext, uint rdflags,
free_event:
kfree(event);
exit:
- if (result < 0)
- ptp_release(pccontext);
return result;
}
--
2.25.1
^ permalink raw reply related
* Re: [PATCH] ptp: fix null ptr deref in ptp_ioctrl
From: Rahul Rameshbabu @ 2023-11-06 2:12 UTC (permalink / raw)
To: Lizhi Xu
Cc: syzbot+8a78ecea7ac1a2ea26e5, davem, linux-kernel, netdev, reibax,
richardcochran, syzkaller-bugs
In-Reply-To: <20231106011926.2928881-1-lizhi.xu@windriver.com>
On Mon, 06 Nov, 2023 09:19:26 +0800 Lizhi Xu <lizhi.xu@windriver.com> wrote:
<snip>
>
> It can be confirmed that after the execution of "read (3, 0x20000080, 90)",
> ptp_release() will be called to release the queue and set
> pccontext->private_clkdata = NULL at the same time, this is unreasonable and
> incorrect. The queue is not the memory requested in ptp_read() and should not
> be released in ptp_read().
>
> Signed-off-by: Lizhi Xu <lizhi.xu@windriver.com>
> ---
> drivers/ptp/ptp_chardev.c | 2 --
> 1 file changed, 2 deletions(-)
>
> diff --git a/drivers/ptp/ptp_chardev.c b/drivers/ptp/ptp_chardev.c
> index 282cd7d24077..27c1ef493617 100644
> --- a/drivers/ptp/ptp_chardev.c
> +++ b/drivers/ptp/ptp_chardev.c
> @@ -585,7 +585,5 @@ ssize_t ptp_read(struct posix_clock_context *pccontext, uint rdflags,
> free_event:
> kfree(event);
> exit:
> - if (result < 0)
> - ptp_release(pccontext);
> return result;
> }
There is already an ongoing review for a patch submission that covers this.
https://lore.kernel.org/netdev/tencent_856E1C97CCE9E2ED66CC087B526CD42ED50A@qq.com/
--
Thanks,
Rahul Rameshbabu
^ permalink raw reply
* Re: [PATCH net-next v10 12/13] net:ethernet:realtek: Update the Makefile and Kconfig in the realtek folder
From: kernel test robot @ 2023-11-06 2:26 UTC (permalink / raw)
To: Justin Lai, kuba
Cc: oe-kbuild-all, davem, edumazet, pabeni, linux-kernel, netdev,
andrew, pkshih, larry.chiu, Justin Lai
In-Reply-To: <20231102154505.940783-13-justinlai0215@realtek.com>
Hi Justin,
kernel test robot noticed the following build errors:
[auto build test ERROR on net-next/main]
url: https://github.com/intel-lab-lkp/linux/commits/Justin-Lai/net-ethernet-realtek-rtase-Add-pci-table-supported-in-this-module/20231103-032946
base: net-next/main
patch link: https://lore.kernel.org/r/20231102154505.940783-13-justinlai0215%40realtek.com
patch subject: [PATCH net-next v10 12/13] net:ethernet:realtek: Update the Makefile and Kconfig in the realtek folder
config: alpha-allyesconfig (https://download.01.org/0day-ci/archive/20231106/202311060957.C85OYvxq-lkp@intel.com/config)
compiler: alpha-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231106/202311060957.C85OYvxq-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202311060957.C85OYvxq-lkp@intel.com/
All errors (new ones prefixed by >>):
>> drivers/net/ethernet/realtek/rtase/rtase_main.c:68:10: fatal error: net/page_pool.h: No such file or directory
68 | #include <net/page_pool.h>
| ^~~~~~~~~~~~~~~~~
compilation terminated.
vim +68 drivers/net/ethernet/realtek/rtase/rtase_main.c
db2657d0fa3a98 Justin Lai 2023-11-02 @68 #include <net/page_pool.h>
db2657d0fa3a98 Justin Lai 2023-11-02 69 #include <net/pkt_cls.h>
db2657d0fa3a98 Justin Lai 2023-11-02 70
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply
* [RFC PATCH v3 00/12] Device Memory TCP
From: Mina Almasry @ 2023-11-06 2:43 UTC (permalink / raw)
To: netdev, linux-kernel, linux-arch, linux-kselftest, linux-media,
dri-devel, linaro-mm-sig
Cc: Mina Almasry, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Jesper Dangaard Brouer, Ilias Apalodimas,
Arnd Bergmann, David Ahern, Willem de Bruijn, Shuah Khan,
Sumit Semwal, Christian König, Shakeel Butt, Jeroen de Borst,
Praveen Kaligineedi
Changes in RFC v3:
------------------
1. Pulled in the memory-provider dependency from Jakub's RFC[1] to make the
series reviewable and mergable.
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/
Cc: Shakeel Butt <shakeelb@google.com>
Cc: Jeroen de Borst <jeroendb@google.com>
Cc: Praveen Kaligineedi <pkaligineedi@google.com>
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 RFC is the GVE devmem TCP support, just to
simplify the review. Code available here if desired:
https://github.com/mina/linux/tree/tcpdevmem
This RFC 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 RFC and memory provider API cherry-picked
locally.
Hardware: Google Cloud A3 VMs.
NIC: GVE with header split & RSS & flow steering support.
Jakub Kicinski (2):
net: page_pool: factor out releasing DMA from releasing the page
net: page_pool: create hooks for custom page providers
Mina Almasry (10):
net: netdev netlink api to bind dma-buf to a net device
netdev: support binding dma-buf to netdevice
netdev: netdevice devmem allocator
memory-provider: dmabuf devmem memory provider
page-pool: device memory support
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 pages
selftests: add ncdevmem, netcat for devmem TCP
Documentation/netlink/specs/netdev.yaml | 28 ++
include/linux/netdevice.h | 93 ++++
include/linux/skbuff.h | 56 ++-
include/linux/socket.h | 1 +
include/net/netdev_rx_queue.h | 1 +
include/net/page_pool/helpers.h | 151 ++++++-
include/net/page_pool/types.h | 55 +++
include/net/sock.h | 2 +
include/net/tcp.h | 5 +-
include/uapi/asm-generic/socket.h | 6 +
include/uapi/linux/netdev.h | 10 +
include/uapi/linux/uio.h | 10 +
net/core/datagram.c | 6 +
net/core/dev.c | 240 +++++++++++
net/core/gro.c | 7 +-
net/core/netdev-genl-gen.c | 14 +
net/core/netdev-genl-gen.h | 1 +
net/core/netdev-genl.c | 118 +++++
net/core/page_pool.c | 209 +++++++--
net/core/skbuff.c | 80 +++-
net/core/sock.c | 36 ++
net/ipv4/tcp.c | 205 ++++++++-
net/ipv4/tcp_input.c | 13 +-
net/ipv4/tcp_ipv4.c | 7 +
net/ipv4/tcp_output.c | 5 +-
net/packet/af_packet.c | 4 +-
tools/include/uapi/linux/netdev.h | 10 +
tools/net/ynl/generated/netdev-user.c | 42 ++
tools/net/ynl/generated/netdev-user.h | 47 ++
tools/testing/selftests/net/.gitignore | 1 +
tools/testing/selftests/net/Makefile | 5 +
tools/testing/selftests/net/ncdevmem.c | 546 ++++++++++++++++++++++++
32 files changed, 1950 insertions(+), 64 deletions(-)
create mode 100644 tools/testing/selftests/net/ncdevmem.c
--
2.42.0.869.gea05f2083d-goog
^ permalink raw reply
* [RFC PATCH v3 01/12] net: page_pool: factor out releasing DMA from releasing the page
From: Mina Almasry @ 2023-11-06 2:44 UTC (permalink / raw)
To: netdev, linux-kernel, linux-arch, linux-kselftest, linux-media,
dri-devel, linaro-mm-sig
Cc: Mina Almasry, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Jesper Dangaard Brouer, Ilias Apalodimas,
Arnd Bergmann, David Ahern, Willem de Bruijn, Shuah Khan,
Sumit Semwal, Christian König, Shakeel Butt, Jeroen de Borst,
Praveen Kaligineedi
In-Reply-To: <20231106024413.2801438-1-almasrymina@google.com>
From: Jakub Kicinski <kuba@kernel.org>
Releasing the DMA mapping will be useful for other types
of pages, so factor it out. Make sure compiler inlines it,
to avoid any regressions.
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Signed-off-by: Mina Almasry <almasrymina@google.com>
---
This is implemented by Jakub in his RFC:
https://lore.kernel.org/netdev/f8270765-a27b-6ccf-33ea-cda097168d79@redhat.com/T/
I take no credit for the idea or implementation. This is a critical
dependency of device memory TCP and thus I'm pulling it into this series
to make it revewable and mergable.
---
net/core/page_pool.c | 25 ++++++++++++++++---------
1 file changed, 16 insertions(+), 9 deletions(-)
diff --git a/net/core/page_pool.c b/net/core/page_pool.c
index 5e409b98aba0..578b6f2eeb46 100644
--- a/net/core/page_pool.c
+++ b/net/core/page_pool.c
@@ -514,21 +514,16 @@ static s32 page_pool_inflight(struct page_pool *pool)
return inflight;
}
-/* Disconnects a page (from a page_pool). API users can have a need
- * to disconnect a page (from a page_pool), to allow it to be used as
- * a regular page (that will eventually be returned to the normal
- * page-allocator via put_page).
- */
-static void page_pool_return_page(struct page_pool *pool, struct page *page)
+static __always_inline
+void __page_pool_release_page_dma(struct page_pool *pool, struct page *page)
{
dma_addr_t dma;
- int count;
if (!(pool->p.flags & PP_FLAG_DMA_MAP))
/* Always account for inflight pages, even if we didn't
* map them
*/
- goto skip_dma_unmap;
+ return;
dma = page_pool_get_dma_addr(page);
@@ -537,7 +532,19 @@ static void page_pool_return_page(struct page_pool *pool, struct page *page)
PAGE_SIZE << pool->p.order, pool->p.dma_dir,
DMA_ATTR_SKIP_CPU_SYNC | DMA_ATTR_WEAK_ORDERING);
page_pool_set_dma_addr(page, 0);
-skip_dma_unmap:
+}
+
+/* Disconnects a page (from a page_pool). API users can have a need
+ * to disconnect a page (from a page_pool), to allow it to be used as
+ * a regular page (that will eventually be returned to the normal
+ * page-allocator via put_page).
+ */
+void page_pool_return_page(struct page_pool *pool, struct page *page)
+{
+ int count;
+
+ __page_pool_release_page_dma(pool, page);
+
page_pool_clear_pp_info(page);
/* This may be the last page returned, releasing the pool, so
--
2.42.0.869.gea05f2083d-goog
^ permalink raw reply related
* [RFC PATCH v3 02/12] net: page_pool: create hooks for custom page providers
From: Mina Almasry @ 2023-11-06 2:44 UTC (permalink / raw)
To: netdev, linux-kernel, linux-arch, linux-kselftest, linux-media,
dri-devel, linaro-mm-sig
Cc: Mina Almasry, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Jesper Dangaard Brouer, Ilias Apalodimas,
Arnd Bergmann, David Ahern, Willem de Bruijn, Shuah Khan,
Sumit Semwal, Christian König, Shakeel Butt, Jeroen de Borst,
Praveen Kaligineedi
In-Reply-To: <20231106024413.2801438-1-almasrymina@google.com>
From: Jakub Kicinski <kuba@kernel.org>
The page providers which try to reuse the same pages will
need to hold onto the ref, even if page gets released from
the pool - as in releasing the page from the pp just transfers
the "ownership" reference from pp to the provider, and provider
will wait for other references to be gone before feeding this
page back into the pool.
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Signed-off-by: Mina Almasry <almasrymina@google.com>
---
This is implemented by Jakub in his RFC:
https://lore.kernel.org/netdev/f8270765-a27b-6ccf-33ea-cda097168d79@redhat.com/T/
I take no credit for the idea or implementation; I only added minor
edits to make this workable with device memory TCP, and removed some
hacky test code. This is a critical dependency of device memory TCP
and thus I'm pulling it into this series to make it revewable and
mergable.
---
include/net/page_pool/types.h | 18 +++++++++++++
net/core/page_pool.c | 51 +++++++++++++++++++++++++++++++----
2 files changed, 64 insertions(+), 5 deletions(-)
diff --git a/include/net/page_pool/types.h b/include/net/page_pool/types.h
index 6fc5134095ed..d4bea053bb7e 100644
--- a/include/net/page_pool/types.h
+++ b/include/net/page_pool/types.h
@@ -60,6 +60,8 @@ struct page_pool_params {
int nid;
struct device *dev;
struct napi_struct *napi;
+ u8 memory_provider;
+ void *mp_priv;
enum dma_data_direction dma_dir;
unsigned int max_len;
unsigned int offset;
@@ -118,6 +120,19 @@ struct page_pool_stats {
};
#endif
+struct mem_provider;
+
+enum pp_memory_provider_type {
+ __PP_MP_NONE, /* Use system allocator directly */
+};
+
+struct pp_memory_provider_ops {
+ int (*init)(struct page_pool *pool);
+ void (*destroy)(struct page_pool *pool);
+ struct page *(*alloc_pages)(struct page_pool *pool, gfp_t gfp);
+ bool (*release_page)(struct page_pool *pool, struct page *page);
+};
+
struct page_pool {
struct page_pool_params p;
@@ -165,6 +180,9 @@ struct page_pool {
*/
struct ptr_ring ring;
+ const struct pp_memory_provider_ops *mp_ops;
+ void *mp_priv;
+
#ifdef CONFIG_PAGE_POOL_STATS
/* recycle stats are per-cpu to avoid locking */
struct page_pool_recycle_stats __percpu *recycle_stats;
diff --git a/net/core/page_pool.c b/net/core/page_pool.c
index 578b6f2eeb46..7ea1f4682479 100644
--- a/net/core/page_pool.c
+++ b/net/core/page_pool.c
@@ -23,6 +23,8 @@
#include <trace/events/page_pool.h>
+static DEFINE_STATIC_KEY_FALSE(page_pool_mem_providers);
+
#define DEFER_TIME (msecs_to_jiffies(1000))
#define DEFER_WARN_INTERVAL (60 * HZ)
@@ -172,6 +174,7 @@ static int page_pool_init(struct page_pool *pool,
const struct page_pool_params *params)
{
unsigned int ring_qsize = 1024; /* Default */
+ int err;
memcpy(&pool->p, params, sizeof(pool->p));
@@ -225,10 +228,34 @@ static int page_pool_init(struct page_pool *pool,
/* Driver calling page_pool_create() also call page_pool_destroy() */
refcount_set(&pool->user_cnt, 1);
+ switch (pool->p.memory_provider) {
+ case __PP_MP_NONE:
+ break;
+ default:
+ err = -EINVAL;
+ goto free_ptr_ring;
+ }
+
+ pool->mp_priv = pool->p.mp_priv;
+ if (pool->mp_ops) {
+ err = pool->mp_ops->init(pool);
+ if (err) {
+ pr_warn("%s() mem-provider init failed %d\n",
+ __func__, err);
+ goto free_ptr_ring;
+ }
+
+ static_branch_inc(&page_pool_mem_providers);
+ }
+
if (pool->p.flags & PP_FLAG_DMA_MAP)
get_device(pool->p.dev);
return 0;
+
+free_ptr_ring:
+ ptr_ring_cleanup(&pool->ring, NULL);
+ return err;
}
/**
@@ -490,7 +517,10 @@ struct page *page_pool_alloc_pages(struct page_pool *pool, gfp_t gfp)
return page;
/* Slow-path: cache empty, do real allocation */
- page = __page_pool_alloc_pages_slow(pool, gfp);
+ if (static_branch_unlikely(&page_pool_mem_providers) && pool->mp_ops)
+ page = pool->mp_ops->alloc_pages(pool, gfp);
+ else
+ page = __page_pool_alloc_pages_slow(pool, gfp);
return page;
}
EXPORT_SYMBOL(page_pool_alloc_pages);
@@ -542,10 +572,13 @@ void __page_pool_release_page_dma(struct page_pool *pool, struct page *page)
void page_pool_return_page(struct page_pool *pool, struct page *page)
{
int count;
+ bool put;
- __page_pool_release_page_dma(pool, page);
-
- page_pool_clear_pp_info(page);
+ put = true;
+ if (static_branch_unlikely(&page_pool_mem_providers) && pool->mp_ops)
+ put = pool->mp_ops->release_page(pool, page);
+ else
+ __page_pool_release_page_dma(pool, page);
/* This may be the last page returned, releasing the pool, so
* it is not safe to reference pool afterwards.
@@ -553,7 +586,10 @@ void page_pool_return_page(struct page_pool *pool, struct page *page)
count = atomic_inc_return_relaxed(&pool->pages_state_release_cnt);
trace_page_pool_state_release(pool, page, count);
- put_page(page);
+ if (put) {
+ page_pool_clear_pp_info(page);
+ put_page(page);
+ }
/* An optimization would be to call __free_pages(page, pool->p.order)
* knowing page is not part of page-cache (thus avoiding a
* __page_cache_release() call).
@@ -821,6 +857,11 @@ static void __page_pool_destroy(struct page_pool *pool)
if (pool->disconnect)
pool->disconnect(pool);
+ if (pool->mp_ops) {
+ pool->mp_ops->destroy(pool);
+ static_branch_dec(&page_pool_mem_providers);
+ }
+
ptr_ring_cleanup(&pool->ring, NULL);
if (pool->p.flags & PP_FLAG_DMA_MAP)
--
2.42.0.869.gea05f2083d-goog
^ permalink raw reply related
* [RFC PATCH v3 03/12] net: netdev netlink api to bind dma-buf to a net device
From: Mina Almasry @ 2023-11-06 2:44 UTC (permalink / raw)
To: netdev, linux-kernel, linux-arch, linux-kselftest, linux-media,
dri-devel, linaro-mm-sig
Cc: Mina Almasry, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Jesper Dangaard Brouer, Ilias Apalodimas,
Arnd Bergmann, David Ahern, Willem de Bruijn, Shuah Khan,
Sumit Semwal, Christian König, Shakeel Butt, Jeroen de Borst,
Praveen Kaligineedi, Stanislav Fomichev
In-Reply-To: <20231106024413.2801438-1-almasrymina@google.com>
API takes the dma-buf fd as input, and binds it to the netdevice. The
user can specify the rx queues to bind the dma-buf to.
Suggested-by: Stanislav Fomichev <sdf@google.com>
Signed-off-by: Mina Almasry <almasrymina@google.com>
---
Changes in v3:
- Support binding multiple rx rx-queues
---
Documentation/netlink/specs/netdev.yaml | 28 +++++++++++++++
include/uapi/linux/netdev.h | 10 ++++++
net/core/netdev-genl-gen.c | 14 ++++++++
net/core/netdev-genl-gen.h | 1 +
net/core/netdev-genl.c | 6 ++++
tools/include/uapi/linux/netdev.h | 10 ++++++
tools/net/ynl/generated/netdev-user.c | 42 ++++++++++++++++++++++
tools/net/ynl/generated/netdev-user.h | 47 +++++++++++++++++++++++++
8 files changed, 158 insertions(+)
diff --git a/Documentation/netlink/specs/netdev.yaml b/Documentation/netlink/specs/netdev.yaml
index 14511b13f305..2141c5f5c33e 100644
--- a/Documentation/netlink/specs/netdev.yaml
+++ b/Documentation/netlink/specs/netdev.yaml
@@ -86,6 +86,24 @@ attribute-sets:
See Documentation/networking/xdp-rx-metadata.rst for more details.
type: u64
enum: xdp-rx-metadata
+ -
+ name: bind-dmabuf
+ attributes:
+ -
+ name: ifindex
+ doc: netdev ifindex to bind the dma-buf to.
+ type: u32
+ checks:
+ min: 1
+ -
+ name: queues
+ doc: receive queues to bind the dma-buf to.
+ type: u32
+ multi-attr: true
+ -
+ name: dmabuf-fd
+ doc: dmabuf file descriptor to bind.
+ type: u32
operations:
list:
@@ -120,6 +138,16 @@ operations:
doc: Notification about device configuration being changed.
notify: dev-get
mcgrp: mgmt
+ -
+ name: bind-rx
+ doc: Bind dmabuf to netdev
+ attribute-set: bind-dmabuf
+ do:
+ request:
+ attributes:
+ - ifindex
+ - dmabuf-fd
+ - queues
mcast-groups:
list:
diff --git a/include/uapi/linux/netdev.h b/include/uapi/linux/netdev.h
index 2943a151d4f1..2cd367c498c7 100644
--- a/include/uapi/linux/netdev.h
+++ b/include/uapi/linux/netdev.h
@@ -64,11 +64,21 @@ enum {
NETDEV_A_DEV_MAX = (__NETDEV_A_DEV_MAX - 1)
};
+enum {
+ NETDEV_A_BIND_DMABUF_IFINDEX = 1,
+ NETDEV_A_BIND_DMABUF_QUEUES,
+ NETDEV_A_BIND_DMABUF_DMABUF_FD,
+
+ __NETDEV_A_BIND_DMABUF_MAX,
+ NETDEV_A_BIND_DMABUF_MAX = (__NETDEV_A_BIND_DMABUF_MAX - 1)
+};
+
enum {
NETDEV_CMD_DEV_GET = 1,
NETDEV_CMD_DEV_ADD_NTF,
NETDEV_CMD_DEV_DEL_NTF,
NETDEV_CMD_DEV_CHANGE_NTF,
+ NETDEV_CMD_BIND_RX,
__NETDEV_CMD_MAX,
NETDEV_CMD_MAX = (__NETDEV_CMD_MAX - 1)
diff --git a/net/core/netdev-genl-gen.c b/net/core/netdev-genl-gen.c
index ea9231378aa6..58300efaf4e5 100644
--- a/net/core/netdev-genl-gen.c
+++ b/net/core/netdev-genl-gen.c
@@ -15,6 +15,13 @@ static const struct nla_policy netdev_dev_get_nl_policy[NETDEV_A_DEV_IFINDEX + 1
[NETDEV_A_DEV_IFINDEX] = NLA_POLICY_MIN(NLA_U32, 1),
};
+/* NETDEV_CMD_BIND_RX - do */
+static const struct nla_policy netdev_bind_rx_nl_policy[NETDEV_A_BIND_DMABUF_DMABUF_FD + 1] = {
+ [NETDEV_A_BIND_DMABUF_IFINDEX] = NLA_POLICY_MIN(NLA_U32, 1),
+ [NETDEV_A_BIND_DMABUF_DMABUF_FD] = { .type = NLA_U32, },
+ [NETDEV_A_BIND_DMABUF_QUEUES] = { .type = NLA_U32, },
+};
+
/* Ops table for netdev */
static const struct genl_split_ops netdev_nl_ops[] = {
{
@@ -29,6 +36,13 @@ static const struct genl_split_ops netdev_nl_ops[] = {
.dumpit = netdev_nl_dev_get_dumpit,
.flags = GENL_CMD_CAP_DUMP,
},
+ {
+ .cmd = NETDEV_CMD_BIND_RX,
+ .doit = netdev_nl_bind_rx_doit,
+ .policy = netdev_bind_rx_nl_policy,
+ .maxattr = NETDEV_A_BIND_DMABUF_DMABUF_FD,
+ .flags = GENL_CMD_CAP_DO,
+ },
};
static const struct genl_multicast_group netdev_nl_mcgrps[] = {
diff --git a/net/core/netdev-genl-gen.h b/net/core/netdev-genl-gen.h
index 7b370c073e7d..5aaeb435ec08 100644
--- a/net/core/netdev-genl-gen.h
+++ b/net/core/netdev-genl-gen.h
@@ -13,6 +13,7 @@
int netdev_nl_dev_get_doit(struct sk_buff *skb, struct genl_info *info);
int netdev_nl_dev_get_dumpit(struct sk_buff *skb, struct netlink_callback *cb);
+int netdev_nl_bind_rx_doit(struct sk_buff *skb, struct genl_info *info);
enum {
NETDEV_NLGRP_MGMT,
diff --git a/net/core/netdev-genl.c b/net/core/netdev-genl.c
index fe61f85bcf33..59d3d512d9cc 100644
--- a/net/core/netdev-genl.c
+++ b/net/core/netdev-genl.c
@@ -129,6 +129,12 @@ int netdev_nl_dev_get_dumpit(struct sk_buff *skb, struct netlink_callback *cb)
return skb->len;
}
+/* Stub */
+int netdev_nl_bind_rx_doit(struct sk_buff *skb, struct genl_info *info)
+{
+ return 0;
+}
+
static int netdev_genl_netdevice_event(struct notifier_block *nb,
unsigned long event, void *ptr)
{
diff --git a/tools/include/uapi/linux/netdev.h b/tools/include/uapi/linux/netdev.h
index 2943a151d4f1..2cd367c498c7 100644
--- a/tools/include/uapi/linux/netdev.h
+++ b/tools/include/uapi/linux/netdev.h
@@ -64,11 +64,21 @@ enum {
NETDEV_A_DEV_MAX = (__NETDEV_A_DEV_MAX - 1)
};
+enum {
+ NETDEV_A_BIND_DMABUF_IFINDEX = 1,
+ NETDEV_A_BIND_DMABUF_QUEUES,
+ NETDEV_A_BIND_DMABUF_DMABUF_FD,
+
+ __NETDEV_A_BIND_DMABUF_MAX,
+ NETDEV_A_BIND_DMABUF_MAX = (__NETDEV_A_BIND_DMABUF_MAX - 1)
+};
+
enum {
NETDEV_CMD_DEV_GET = 1,
NETDEV_CMD_DEV_ADD_NTF,
NETDEV_CMD_DEV_DEL_NTF,
NETDEV_CMD_DEV_CHANGE_NTF,
+ NETDEV_CMD_BIND_RX,
__NETDEV_CMD_MAX,
NETDEV_CMD_MAX = (__NETDEV_CMD_MAX - 1)
diff --git a/tools/net/ynl/generated/netdev-user.c b/tools/net/ynl/generated/netdev-user.c
index b5ffe8cd1144..d5f4c6d4c2b2 100644
--- a/tools/net/ynl/generated/netdev-user.c
+++ b/tools/net/ynl/generated/netdev-user.c
@@ -18,6 +18,7 @@ static const char * const netdev_op_strmap[] = {
[NETDEV_CMD_DEV_ADD_NTF] = "dev-add-ntf",
[NETDEV_CMD_DEV_DEL_NTF] = "dev-del-ntf",
[NETDEV_CMD_DEV_CHANGE_NTF] = "dev-change-ntf",
+ [NETDEV_CMD_BIND_RX] = "bind-rx",
};
const char *netdev_op_str(int op)
@@ -72,6 +73,17 @@ struct ynl_policy_nest netdev_dev_nest = {
.table = netdev_dev_policy,
};
+struct ynl_policy_attr netdev_bind_dmabuf_policy[NETDEV_A_BIND_DMABUF_MAX + 1] = {
+ [NETDEV_A_BIND_DMABUF_IFINDEX] = { .name = "ifindex", .type = YNL_PT_U32, },
+ [NETDEV_A_BIND_DMABUF_QUEUES] = { .name = "queues", .type = YNL_PT_U32, },
+ [NETDEV_A_BIND_DMABUF_DMABUF_FD] = { .name = "dmabuf-fd", .type = YNL_PT_U32, },
+};
+
+struct ynl_policy_nest netdev_bind_dmabuf_nest = {
+ .max_attr = NETDEV_A_BIND_DMABUF_MAX,
+ .table = netdev_bind_dmabuf_policy,
+};
+
/* Common nested types */
/* ============== NETDEV_CMD_DEV_GET ============== */
/* NETDEV_CMD_DEV_GET - do */
@@ -197,6 +209,36 @@ void netdev_dev_get_ntf_free(struct netdev_dev_get_ntf *rsp)
free(rsp);
}
+/* ============== NETDEV_CMD_BIND_RX ============== */
+/* NETDEV_CMD_BIND_RX - do */
+void netdev_bind_rx_req_free(struct netdev_bind_rx_req *req)
+{
+ free(req->queues);
+ free(req);
+}
+
+int netdev_bind_rx(struct ynl_sock *ys, struct netdev_bind_rx_req *req)
+{
+ struct nlmsghdr *nlh;
+ int err;
+
+ nlh = ynl_gemsg_start_req(ys, ys->family_id, NETDEV_CMD_BIND_RX, 1);
+ ys->req_policy = &netdev_bind_dmabuf_nest;
+
+ if (req->_present.ifindex)
+ mnl_attr_put_u32(nlh, NETDEV_A_BIND_DMABUF_IFINDEX, req->ifindex);
+ if (req->_present.dmabuf_fd)
+ mnl_attr_put_u32(nlh, NETDEV_A_BIND_DMABUF_DMABUF_FD, req->dmabuf_fd);
+ for (unsigned int i = 0; i < req->n_queues; i++)
+ mnl_attr_put_u32(nlh, NETDEV_A_BIND_DMABUF_QUEUES, req->queues[i]);
+
+ err = ynl_exec(ys, nlh, NULL);
+ if (err < 0)
+ return -1;
+
+ return 0;
+}
+
static const struct ynl_ntf_info netdev_ntf_info[] = {
[NETDEV_CMD_DEV_ADD_NTF] = {
.alloc_sz = sizeof(struct netdev_dev_get_ntf),
diff --git a/tools/net/ynl/generated/netdev-user.h b/tools/net/ynl/generated/netdev-user.h
index 4fafac879df3..3cf9096d733a 100644
--- a/tools/net/ynl/generated/netdev-user.h
+++ b/tools/net/ynl/generated/netdev-user.h
@@ -87,4 +87,51 @@ struct netdev_dev_get_ntf {
void netdev_dev_get_ntf_free(struct netdev_dev_get_ntf *rsp);
+/* ============== NETDEV_CMD_BIND_RX ============== */
+/* NETDEV_CMD_BIND_RX - do */
+struct netdev_bind_rx_req {
+ struct {
+ __u32 ifindex:1;
+ __u32 dmabuf_fd:1;
+ } _present;
+
+ __u32 ifindex;
+ __u32 dmabuf_fd;
+ unsigned int n_queues;
+ __u32 *queues;
+};
+
+static inline struct netdev_bind_rx_req *netdev_bind_rx_req_alloc(void)
+{
+ return calloc(1, sizeof(struct netdev_bind_rx_req));
+}
+void netdev_bind_rx_req_free(struct netdev_bind_rx_req *req);
+
+static inline void
+netdev_bind_rx_req_set_ifindex(struct netdev_bind_rx_req *req, __u32 ifindex)
+{
+ req->_present.ifindex = 1;
+ req->ifindex = ifindex;
+}
+static inline void
+netdev_bind_rx_req_set_dmabuf_fd(struct netdev_bind_rx_req *req,
+ __u32 dmabuf_fd)
+{
+ req->_present.dmabuf_fd = 1;
+ req->dmabuf_fd = dmabuf_fd;
+}
+static inline void
+__netdev_bind_rx_req_set_queues(struct netdev_bind_rx_req *req, __u32 *queues,
+ unsigned int n_queues)
+{
+ free(req->queues);
+ req->queues = queues;
+ req->n_queues = n_queues;
+}
+
+/*
+ * Bind dmabuf to netdev
+ */
+int netdev_bind_rx(struct ynl_sock *ys, struct netdev_bind_rx_req *req);
+
#endif /* _LINUX_NETDEV_GEN_H */
--
2.42.0.869.gea05f2083d-goog
^ permalink raw reply related
* [RFC PATCH v3 05/12] netdev: netdevice devmem allocator
From: Mina Almasry @ 2023-11-06 2:44 UTC (permalink / raw)
To: netdev, linux-kernel, linux-arch, linux-kselftest, linux-media,
dri-devel, linaro-mm-sig
Cc: Mina Almasry, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Jesper Dangaard Brouer, Ilias Apalodimas,
Arnd Bergmann, David Ahern, Willem de Bruijn, Shuah Khan,
Sumit Semwal, Christian König, Shakeel Butt, Jeroen de Borst,
Praveen Kaligineedi, Willem de Bruijn, Kaiyuan Zhang
In-Reply-To: <20231106024413.2801438-1-almasrymina@google.com>
Implement netdev devmem allocator. The allocator takes a given struct
netdev_dmabuf_binding as input and allocates page_pool_iov from that
binding.
The allocation simply delegates to the binding's genpool for the
allocation logic and wraps the returned memory region in a page_pool_iov
struct.
page_pool_iov are refcounted and are freed back to the binding when the
refcount drops to 0.
Signed-off-by: Willem de Bruijn <willemb@google.com>
Signed-off-by: Kaiyuan Zhang <kaiyuanz@google.com>
Signed-off-by: Mina Almasry <almasrymina@google.com>
---
include/linux/netdevice.h | 13 ++++++++++++
include/net/page_pool/helpers.h | 28 +++++++++++++++++++++++++
net/core/dev.c | 37 +++++++++++++++++++++++++++++++++
3 files changed, 78 insertions(+)
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index eeeda849115c..1c351c138a5b 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -843,6 +843,9 @@ struct netdev_dmabuf_binding {
};
#ifdef CONFIG_DMA_SHARED_BUFFER
+struct page_pool_iov *
+netdev_alloc_devmem(struct netdev_dmabuf_binding *binding);
+void netdev_free_devmem(struct page_pool_iov *ppiov);
void __netdev_devmem_binding_free(struct netdev_dmabuf_binding *binding);
int netdev_bind_dmabuf(struct net_device *dev, unsigned int dmabuf_fd,
struct netdev_dmabuf_binding **out);
@@ -850,6 +853,16 @@ void netdev_unbind_dmabuf(struct netdev_dmabuf_binding *binding);
int netdev_bind_dmabuf_to_queue(struct net_device *dev, u32 rxq_idx,
struct netdev_dmabuf_binding *binding);
#else
+static inline struct page_pool_iov *
+netdev_alloc_devmem(struct netdev_dmabuf_binding *binding)
+{
+ return NULL;
+}
+
+static inline void netdev_free_devmem(struct page_pool_iov *ppiov)
+{
+}
+
static inline void
__netdev_devmem_binding_free(struct netdev_dmabuf_binding *binding)
{
diff --git a/include/net/page_pool/helpers.h b/include/net/page_pool/helpers.h
index 4ebd544ae977..78cbb040af94 100644
--- a/include/net/page_pool/helpers.h
+++ b/include/net/page_pool/helpers.h
@@ -83,6 +83,34 @@ static inline u64 *page_pool_ethtool_stats_get(u64 *data, void *stats)
}
#endif
+/* page_pool_iov support */
+
+static inline struct dmabuf_genpool_chunk_owner *
+page_pool_iov_owner(const struct page_pool_iov *ppiov)
+{
+ return ppiov->owner;
+}
+
+static inline unsigned int page_pool_iov_idx(const struct page_pool_iov *ppiov)
+{
+ return ppiov - page_pool_iov_owner(ppiov)->ppiovs;
+}
+
+static inline dma_addr_t
+page_pool_iov_dma_addr(const struct page_pool_iov *ppiov)
+{
+ struct dmabuf_genpool_chunk_owner *owner = page_pool_iov_owner(ppiov);
+
+ return owner->base_dma_addr +
+ ((dma_addr_t)page_pool_iov_idx(ppiov) << PAGE_SHIFT);
+}
+
+static inline struct netdev_dmabuf_binding *
+page_pool_iov_binding(const struct page_pool_iov *ppiov)
+{
+ return page_pool_iov_owner(ppiov)->binding;
+}
+
/**
* page_pool_dev_alloc_pages() - allocate a page.
* @pool: pool from which to allocate
diff --git a/net/core/dev.c b/net/core/dev.c
index c8c3709d42c8..2315bbc03ec8 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -156,6 +156,7 @@
#include <linux/genalloc.h>
#include <linux/dma-buf.h>
#include <net/page_pool/types.h>
+#include <net/page_pool/helpers.h>
#include "dev.h"
#include "net-sysfs.h"
@@ -2077,6 +2078,42 @@ void __netdev_devmem_binding_free(struct netdev_dmabuf_binding *binding)
kfree(binding);
}
+struct page_pool_iov *netdev_alloc_devmem(struct netdev_dmabuf_binding *binding)
+{
+ struct dmabuf_genpool_chunk_owner *owner;
+ struct page_pool_iov *ppiov;
+ unsigned long dma_addr;
+ ssize_t offset;
+ ssize_t index;
+
+ dma_addr = gen_pool_alloc_owner(binding->chunk_pool, PAGE_SIZE,
+ (void **)&owner);
+ if (!dma_addr)
+ return NULL;
+
+ offset = dma_addr - owner->base_dma_addr;
+ index = offset / PAGE_SIZE;
+ ppiov = &owner->ppiovs[index];
+
+ netdev_devmem_binding_get(binding);
+
+ return ppiov;
+}
+
+void netdev_free_devmem(struct page_pool_iov *ppiov)
+{
+ struct netdev_dmabuf_binding *binding = page_pool_iov_binding(ppiov);
+
+ refcount_set(&ppiov->refcount, 1);
+
+ if (gen_pool_has_addr(binding->chunk_pool,
+ page_pool_iov_dma_addr(ppiov), PAGE_SIZE))
+ gen_pool_free(binding->chunk_pool,
+ page_pool_iov_dma_addr(ppiov), PAGE_SIZE);
+
+ netdev_devmem_binding_put(binding);
+}
+
void netdev_unbind_dmabuf(struct netdev_dmabuf_binding *binding)
{
struct netdev_rx_queue *rxq;
--
2.42.0.869.gea05f2083d-goog
^ permalink raw reply related
* [RFC PATCH v3 04/12] netdev: support binding dma-buf to netdevice
From: Mina Almasry @ 2023-11-06 2:44 UTC (permalink / raw)
To: netdev, linux-kernel, linux-arch, linux-kselftest, linux-media,
dri-devel, linaro-mm-sig
Cc: Mina Almasry, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Jesper Dangaard Brouer, Ilias Apalodimas,
Arnd Bergmann, David Ahern, Willem de Bruijn, Shuah Khan,
Sumit Semwal, Christian König, Shakeel Butt, Jeroen de Borst,
Praveen Kaligineedi, Willem de Bruijn, Kaiyuan Zhang
In-Reply-To: <20231106024413.2801438-1-almasrymina@google.com>
Add a netdev_dmabuf_binding struct which represents the
dma-buf-to-netdevice binding. The netlink API will bind the dma-buf to
rx queues on the netdevice. On the binding, the dma_buf_attach
& dma_buf_map_attachment will occur. The entries in the sg_table from
mapping will be inserted into a genpool to make it ready
for allocation.
The chunks in the genpool are owned by a dmabuf_chunk_owner struct which
holds the dma-buf offset of the base of the chunk and the dma_addr of
the chunk. Both are needed to use allocations that come from this chunk.
We create a new type that represents an allocation from the genpool:
page_pool_iov. We setup the page_pool_iov allocation size in the
genpool to PAGE_SIZE for simplicity: to match the PAGE_SIZE normally
allocated by the page pool and given to the drivers.
The user can unbind the dmabuf from the netdevice by closing the netlink
socket that established the binding. We do this so that the binding is
automatically unbound even if the userspace process crashes.
The binding and unbinding leaves an indicator in struct netdev_rx_queue
that the given queue is bound, but the binding doesn't take effect until
the driver actually reconfigures its queues, and re-initializes its page
pool.
The netdev_dmabuf_binding struct is refcounted, and releases its
resources only when all the refs are released.
Signed-off-by: Willem de Bruijn <willemb@google.com>
Signed-off-by: Kaiyuan Zhang <kaiyuanz@google.com>
Signed-off-by: Mina Almasry <almasrymina@google.com>
---
RFC v3:
- Support multi rx-queue binding
---
include/linux/netdevice.h | 80 ++++++++++++++
include/net/netdev_rx_queue.h | 1 +
include/net/page_pool/types.h | 27 +++++
net/core/dev.c | 203 ++++++++++++++++++++++++++++++++++
net/core/netdev-genl.c | 116 ++++++++++++++++++-
5 files changed, 425 insertions(+), 2 deletions(-)
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index b8bf669212cc..eeeda849115c 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -52,6 +52,8 @@
#include <net/net_trackers.h>
#include <net/net_debug.h>
#include <net/dropreason-core.h>
+#include <linux/xarray.h>
+#include <linux/refcount.h>
struct netpoll_info;
struct device;
@@ -808,6 +810,84 @@ bool rps_may_expire_flow(struct net_device *dev, u16 rxq_index, u32 flow_id,
#endif
#endif /* CONFIG_RPS */
+struct netdev_dmabuf_binding {
+ struct dma_buf *dmabuf;
+ struct dma_buf_attachment *attachment;
+ struct sg_table *sgt;
+ struct net_device *dev;
+ struct gen_pool *chunk_pool;
+
+ /* The user holds a ref (via the netlink API) for as long as they want
+ * the binding to remain alive. Each page pool using this binding holds
+ * a ref to keep the binding alive. Each allocated page_pool_iov holds a
+ * ref.
+ *
+ * The binding undos itself and unmaps the underlying dmabuf once all
+ * those refs are dropped and the binding is no longer desired or in
+ * use.
+ */
+ refcount_t ref;
+
+ /* The portid of the user that owns this binding. Used for netlink to
+ * notify us of the user dropping the bind.
+ */
+ u32 owner_nlportid;
+
+ /* The list of bindings currently active. Used for netlink to notify us
+ * of the user dropping the bind.
+ */
+ struct list_head list;
+
+ /* rxq's this binding is active on. */
+ struct xarray bound_rxq_list;
+};
+
+#ifdef CONFIG_DMA_SHARED_BUFFER
+void __netdev_devmem_binding_free(struct netdev_dmabuf_binding *binding);
+int netdev_bind_dmabuf(struct net_device *dev, unsigned int dmabuf_fd,
+ struct netdev_dmabuf_binding **out);
+void netdev_unbind_dmabuf(struct netdev_dmabuf_binding *binding);
+int netdev_bind_dmabuf_to_queue(struct net_device *dev, u32 rxq_idx,
+ struct netdev_dmabuf_binding *binding);
+#else
+static inline void
+__netdev_devmem_binding_free(struct netdev_dmabuf_binding *binding)
+{
+}
+
+static inline int netdev_bind_dmabuf(struct net_device *dev,
+ unsigned int dmabuf_fd,
+ struct netdev_dmabuf_binding **out)
+{
+ return -EOPNOTSUPP;
+}
+static inline void netdev_unbind_dmabuf(struct netdev_dmabuf_binding *binding)
+{
+}
+
+static inline int
+netdev_bind_dmabuf_to_queue(struct net_device *dev, u32 rxq_idx,
+ struct netdev_dmabuf_binding *binding)
+{
+ return -EOPNOTSUPP;
+}
+#endif
+
+static inline void
+netdev_devmem_binding_get(struct netdev_dmabuf_binding *binding)
+{
+ refcount_inc(&binding->ref);
+}
+
+static inline void
+netdev_devmem_binding_put(struct netdev_dmabuf_binding *binding)
+{
+ if (!refcount_dec_and_test(&binding->ref))
+ return;
+
+ __netdev_devmem_binding_free(binding);
+}
+
/* XPS map type and offset of the xps map within net_device->xps_maps[]. */
enum xps_map_type {
XPS_CPUS = 0,
diff --git a/include/net/netdev_rx_queue.h b/include/net/netdev_rx_queue.h
index cdcafb30d437..1bfcf60a145d 100644
--- a/include/net/netdev_rx_queue.h
+++ b/include/net/netdev_rx_queue.h
@@ -21,6 +21,7 @@ struct netdev_rx_queue {
#ifdef CONFIG_XDP_SOCKETS
struct xsk_buff_pool *pool;
#endif
+ struct netdev_dmabuf_binding *binding;
} ____cacheline_aligned_in_smp;
/*
diff --git a/include/net/page_pool/types.h b/include/net/page_pool/types.h
index d4bea053bb7e..64386325d965 100644
--- a/include/net/page_pool/types.h
+++ b/include/net/page_pool/types.h
@@ -133,6 +133,33 @@ struct pp_memory_provider_ops {
bool (*release_page)(struct page_pool *pool, struct page *page);
};
+/* page_pool_iov support */
+
+/* Owner of the dma-buf chunks inserted into the gen pool. Each scatterlist
+ * entry from the dmabuf is inserted into the genpool as a chunk, and needs
+ * this owner struct to keep track of some metadata necessary to create
+ * allocations from this chunk.
+ */
+struct dmabuf_genpool_chunk_owner {
+ /* Offset into the dma-buf where this chunk starts. */
+ unsigned long base_virtual;
+
+ /* dma_addr of the start of the chunk. */
+ dma_addr_t base_dma_addr;
+
+ /* Array of page_pool_iovs for this chunk. */
+ struct page_pool_iov *ppiovs;
+ size_t num_ppiovs;
+
+ struct netdev_dmabuf_binding *binding;
+};
+
+struct page_pool_iov {
+ struct dmabuf_genpool_chunk_owner *owner;
+
+ refcount_t refcount;
+};
+
struct page_pool {
struct page_pool_params p;
diff --git a/net/core/dev.c b/net/core/dev.c
index a37a932a3e14..c8c3709d42c8 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -153,6 +153,9 @@
#include <linux/prandom.h>
#include <linux/once_lite.h>
#include <net/netdev_rx_queue.h>
+#include <linux/genalloc.h>
+#include <linux/dma-buf.h>
+#include <net/page_pool/types.h>
#include "dev.h"
#include "net-sysfs.h"
@@ -2040,6 +2043,206 @@ static int call_netdevice_notifiers_mtu(unsigned long val,
return call_netdevice_notifiers_info(val, &info.info);
}
+/* Device memory support */
+
+#ifdef CONFIG_DMA_SHARED_BUFFER
+static void netdev_devmem_free_chunk_owner(struct gen_pool *genpool,
+ struct gen_pool_chunk *chunk,
+ void *not_used)
+{
+ struct dmabuf_genpool_chunk_owner *owner = chunk->owner;
+
+ kvfree(owner->ppiovs);
+ kfree(owner);
+}
+
+void __netdev_devmem_binding_free(struct netdev_dmabuf_binding *binding)
+{
+ size_t size, avail;
+
+ gen_pool_for_each_chunk(binding->chunk_pool,
+ netdev_devmem_free_chunk_owner, NULL);
+
+ size = gen_pool_size(binding->chunk_pool);
+ avail = gen_pool_avail(binding->chunk_pool);
+
+ if (!WARN(size != avail, "can't destroy genpool. size=%lu, avail=%lu",
+ size, avail))
+ gen_pool_destroy(binding->chunk_pool);
+
+ dma_buf_unmap_attachment(binding->attachment, binding->sgt,
+ DMA_BIDIRECTIONAL);
+ dma_buf_detach(binding->dmabuf, binding->attachment);
+ dma_buf_put(binding->dmabuf);
+ kfree(binding);
+}
+
+void netdev_unbind_dmabuf(struct netdev_dmabuf_binding *binding)
+{
+ struct netdev_rx_queue *rxq;
+ unsigned long xa_idx;
+
+ if (!binding)
+ return;
+
+ list_del_rcu(&binding->list);
+
+ xa_for_each(&binding->bound_rxq_list, xa_idx, rxq)
+ if (rxq->binding == binding)
+ /* We hold the rtnl_lock while binding/unbinding
+ * dma-buf, so we can't race with another thread that
+ * is also modifying this value. However, the driver
+ * may read this config while it's creating its
+ * rx-queues. WRITE_ONCE() here to match the
+ * READ_ONCE() in the driver.
+ */
+ WRITE_ONCE(rxq->binding, NULL);
+
+ netdev_devmem_binding_put(binding);
+}
+
+int netdev_bind_dmabuf_to_queue(struct net_device *dev, u32 rxq_idx,
+ struct netdev_dmabuf_binding *binding)
+{
+ struct netdev_rx_queue *rxq;
+ u32 xa_idx;
+ int err;
+
+ rxq = __netif_get_rx_queue(dev, rxq_idx);
+
+ if (rxq->binding)
+ return -EEXIST;
+
+ err = xa_alloc(&binding->bound_rxq_list, &xa_idx, rxq, xa_limit_32b,
+ GFP_KERNEL);
+ if (err)
+ return err;
+
+ /*We hold the rtnl_lock while binding/unbinding dma-buf, so we can't
+ * race with another thread that is also modifying this value. However,
+ * the driver may read this config while it's creating its * rx-queues.
+ * WRITE_ONCE() here to match the READ_ONCE() in the driver.
+ */
+ WRITE_ONCE(rxq->binding, binding);
+
+ return 0;
+}
+
+int netdev_bind_dmabuf(struct net_device *dev, unsigned int dmabuf_fd,
+ struct netdev_dmabuf_binding **out)
+{
+ struct netdev_dmabuf_binding *binding;
+ struct scatterlist *sg;
+ struct dma_buf *dmabuf;
+ unsigned int sg_idx, i;
+ unsigned long virtual;
+ int err;
+
+ if (!capable(CAP_NET_ADMIN))
+ return -EPERM;
+
+ dmabuf = dma_buf_get(dmabuf_fd);
+ if (IS_ERR_OR_NULL(dmabuf))
+ return -EBADFD;
+
+ binding = kzalloc_node(sizeof(*binding), GFP_KERNEL,
+ dev_to_node(&dev->dev));
+ if (!binding) {
+ err = -ENOMEM;
+ goto err_put_dmabuf;
+ }
+
+ xa_init_flags(&binding->bound_rxq_list, XA_FLAGS_ALLOC);
+
+ refcount_set(&binding->ref, 1);
+
+ binding->dmabuf = dmabuf;
+
+ binding->attachment = dma_buf_attach(binding->dmabuf, dev->dev.parent);
+ if (IS_ERR(binding->attachment)) {
+ err = PTR_ERR(binding->attachment);
+ goto err_free_binding;
+ }
+
+ binding->sgt = dma_buf_map_attachment(binding->attachment,
+ DMA_BIDIRECTIONAL);
+ if (IS_ERR(binding->sgt)) {
+ err = PTR_ERR(binding->sgt);
+ goto err_detach;
+ }
+
+ /* For simplicity we expect to make PAGE_SIZE allocations, but the
+ * binding can be much more flexible than that. We may be able to
+ * allocate MTU sized chunks here. Leave that for future work...
+ */
+ binding->chunk_pool = gen_pool_create(PAGE_SHIFT,
+ dev_to_node(&dev->dev));
+ if (!binding->chunk_pool) {
+ err = -ENOMEM;
+ goto err_unmap;
+ }
+
+ virtual = 0;
+ for_each_sgtable_dma_sg(binding->sgt, sg, sg_idx) {
+ dma_addr_t dma_addr = sg_dma_address(sg);
+ struct dmabuf_genpool_chunk_owner *owner;
+ size_t len = sg_dma_len(sg);
+ struct page_pool_iov *ppiov;
+
+ owner = kzalloc_node(sizeof(*owner), GFP_KERNEL,
+ dev_to_node(&dev->dev));
+ owner->base_virtual = virtual;
+ owner->base_dma_addr = dma_addr;
+ owner->num_ppiovs = len / PAGE_SIZE;
+ owner->binding = binding;
+
+ err = gen_pool_add_owner(binding->chunk_pool, dma_addr,
+ dma_addr, len, dev_to_node(&dev->dev),
+ owner);
+ if (err) {
+ err = -EINVAL;
+ goto err_free_chunks;
+ }
+
+ owner->ppiovs = kvmalloc_array(owner->num_ppiovs,
+ sizeof(*owner->ppiovs),
+ GFP_KERNEL);
+ if (!owner->ppiovs) {
+ err = -ENOMEM;
+ goto err_free_chunks;
+ }
+
+ for (i = 0; i < owner->num_ppiovs; i++) {
+ ppiov = &owner->ppiovs[i];
+ ppiov->owner = owner;
+ refcount_set(&ppiov->refcount, 1);
+ }
+
+ dma_addr += len;
+ virtual += len;
+ }
+
+ *out = binding;
+
+ return 0;
+
+err_free_chunks:
+ gen_pool_for_each_chunk(binding->chunk_pool,
+ netdev_devmem_free_chunk_owner, NULL);
+ gen_pool_destroy(binding->chunk_pool);
+err_unmap:
+ dma_buf_unmap_attachment(binding->attachment, binding->sgt,
+ DMA_BIDIRECTIONAL);
+err_detach:
+ dma_buf_detach(dmabuf, binding->attachment);
+err_free_binding:
+ kfree(binding);
+err_put_dmabuf:
+ dma_buf_put(dmabuf);
+ return err;
+}
+#endif
+
#ifdef CONFIG_NET_INGRESS
static DEFINE_STATIC_KEY_FALSE(ingress_needed_key);
diff --git a/net/core/netdev-genl.c b/net/core/netdev-genl.c
index 59d3d512d9cc..2c2a62593217 100644
--- a/net/core/netdev-genl.c
+++ b/net/core/netdev-genl.c
@@ -129,10 +129,89 @@ int netdev_nl_dev_get_dumpit(struct sk_buff *skb, struct netlink_callback *cb)
return skb->len;
}
-/* Stub */
+static LIST_HEAD(netdev_rbinding_list);
+
int netdev_nl_bind_rx_doit(struct sk_buff *skb, struct genl_info *info)
{
- return 0;
+ struct netdev_dmabuf_binding *out_binding;
+ u32 ifindex, dmabuf_fd, rxq_idx;
+ struct net_device *netdev;
+ struct sk_buff *rsp;
+ int rem, err = 0;
+ void *hdr;
+ struct nlattr *attr;
+
+ if (GENL_REQ_ATTR_CHECK(info, NETDEV_A_DEV_IFINDEX) ||
+ GENL_REQ_ATTR_CHECK(info, NETDEV_A_BIND_DMABUF_DMABUF_FD) ||
+ GENL_REQ_ATTR_CHECK(info, NETDEV_A_BIND_DMABUF_QUEUES))
+ return -EINVAL;
+
+ ifindex = nla_get_u32(info->attrs[NETDEV_A_DEV_IFINDEX]);
+ dmabuf_fd = nla_get_u32(info->attrs[NETDEV_A_BIND_DMABUF_DMABUF_FD]);
+
+ rtnl_lock();
+
+ netdev = __dev_get_by_index(genl_info_net(info), ifindex);
+ if (!netdev) {
+ err = -ENODEV;
+ goto err_unlock;
+ }
+
+ err = netdev_bind_dmabuf(netdev, dmabuf_fd, &out_binding);
+ if (err)
+ goto err_unlock;
+
+ nla_for_each_attr(attr, genlmsg_data(info->genlhdr),
+ genlmsg_len(info->genlhdr), rem) {
+ switch (nla_type(attr)) {
+ case NETDEV_A_BIND_DMABUF_QUEUES:
+ rxq_idx = nla_get_u32(attr);
+
+ if (rxq_idx >= netdev->num_rx_queues) {
+ err = -ERANGE;
+ goto err_unbind;
+ }
+
+ err = netdev_bind_dmabuf_to_queue(netdev, rxq_idx,
+ out_binding);
+ if (err)
+ goto err_unbind;
+
+ break;
+ default:
+ break;
+ }
+ }
+
+ out_binding->owner_nlportid = info->snd_portid;
+ list_add_rcu(&out_binding->list, &netdev_rbinding_list);
+
+ rsp = genlmsg_new(GENLMSG_DEFAULT_SIZE, GFP_KERNEL);
+ if (!rsp) {
+ err = -ENOMEM;
+ goto err_unbind;
+ }
+
+ hdr = genlmsg_put(rsp, info->snd_portid, info->snd_seq,
+ &netdev_nl_family, 0, info->genlhdr->cmd);
+ if (!hdr) {
+ err = -EMSGSIZE;
+ goto err_genlmsg_free;
+ }
+
+ genlmsg_end(rsp, hdr);
+
+ rtnl_unlock();
+
+ return genlmsg_reply(rsp, info);
+
+err_genlmsg_free:
+ nlmsg_free(rsp);
+err_unbind:
+ netdev_unbind_dmabuf(out_binding);
+err_unlock:
+ rtnl_unlock();
+ return err;
}
static int netdev_genl_netdevice_event(struct notifier_block *nb,
@@ -155,10 +234,37 @@ static int netdev_genl_netdevice_event(struct notifier_block *nb,
return NOTIFY_OK;
}
+static int netdev_netlink_notify(struct notifier_block *nb, unsigned long state,
+ void *_notify)
+{
+ struct netlink_notify *notify = _notify;
+ struct netdev_dmabuf_binding *rbinding;
+
+ if (state != NETLINK_URELEASE || notify->protocol != NETLINK_GENERIC)
+ return NOTIFY_DONE;
+
+ rcu_read_lock();
+
+ list_for_each_entry_rcu(rbinding, &netdev_rbinding_list, list) {
+ if (rbinding->owner_nlportid == notify->portid) {
+ netdev_unbind_dmabuf(rbinding);
+ break;
+ }
+ }
+
+ rcu_read_unlock();
+
+ return NOTIFY_OK;
+}
+
static struct notifier_block netdev_genl_nb = {
.notifier_call = netdev_genl_netdevice_event,
};
+static struct notifier_block netdev_netlink_notifier = {
+ .notifier_call = netdev_netlink_notify,
+};
+
static int __init netdev_genl_init(void)
{
int err;
@@ -171,8 +277,14 @@ static int __init netdev_genl_init(void)
if (err)
goto err_unreg_ntf;
+ err = netlink_register_notifier(&netdev_netlink_notifier);
+ if (err)
+ goto err_unreg_family;
+
return 0;
+err_unreg_family:
+ genl_unregister_family(&netdev_nl_family);
err_unreg_ntf:
unregister_netdevice_notifier(&netdev_genl_nb);
return err;
--
2.42.0.869.gea05f2083d-goog
^ permalink raw reply related
* [RFC PATCH v3 06/12] memory-provider: dmabuf devmem memory provider
From: Mina Almasry @ 2023-11-06 2:44 UTC (permalink / raw)
To: netdev, linux-kernel, linux-arch, linux-kselftest, linux-media,
dri-devel, linaro-mm-sig
Cc: Mina Almasry, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Jesper Dangaard Brouer, Ilias Apalodimas,
Arnd Bergmann, David Ahern, Willem de Bruijn, Shuah Khan,
Sumit Semwal, Christian König, Shakeel Butt, Jeroen de Borst,
Praveen Kaligineedi, Willem de Bruijn, Kaiyuan Zhang
In-Reply-To: <20231106024413.2801438-1-almasrymina@google.com>
Implement a memory provider that allocates dmabuf devmem page_pool_iovs.
Support of PP_FLAG_DMA_MAP and PP_FLAG_DMA_SYNC_DEV is omitted for
simplicity.
The provider receives a reference to the struct netdev_dmabuf_binding
via the pool->mp_priv pointer. The driver needs to set this pointer for
the provider in the page_pool_params.
The provider obtains a reference on the netdev_dmabuf_binding which
guarantees the binding and the underlying mapping remains alive until
the provider is destroyed.
Signed-off-by: Willem de Bruijn <willemb@google.com>
Signed-off-by: Kaiyuan Zhang <kaiyuanz@google.com>
Signed-off-by: Mina Almasry <almasrymina@google.com>
---
include/net/page_pool/helpers.h | 40 +++++++++++++++++
include/net/page_pool/types.h | 10 +++++
net/core/page_pool.c | 76 +++++++++++++++++++++++++++++++++
3 files changed, 126 insertions(+)
diff --git a/include/net/page_pool/helpers.h b/include/net/page_pool/helpers.h
index 78cbb040af94..b93243c2a640 100644
--- a/include/net/page_pool/helpers.h
+++ b/include/net/page_pool/helpers.h
@@ -53,6 +53,7 @@
#define _NET_PAGE_POOL_HELPERS_H
#include <net/page_pool/types.h>
+#include <net/net_debug.h>
#ifdef CONFIG_PAGE_POOL_STATS
int page_pool_ethtool_stats_get_count(void);
@@ -111,6 +112,45 @@ page_pool_iov_binding(const struct page_pool_iov *ppiov)
return page_pool_iov_owner(ppiov)->binding;
}
+static inline int page_pool_iov_refcount(const struct page_pool_iov *ppiov)
+{
+ return refcount_read(&ppiov->refcount);
+}
+
+static inline void page_pool_iov_get_many(struct page_pool_iov *ppiov,
+ unsigned int count)
+{
+ refcount_add(count, &ppiov->refcount);
+}
+
+void __page_pool_iov_free(struct page_pool_iov *ppiov);
+
+static inline void page_pool_iov_put_many(struct page_pool_iov *ppiov,
+ unsigned int count)
+{
+ if (!refcount_sub_and_test(count, &ppiov->refcount))
+ return;
+
+ __page_pool_iov_free(ppiov);
+}
+
+/* page pool mm helpers */
+
+static inline bool page_is_page_pool_iov(const struct page *page)
+{
+ return (unsigned long)page & PP_DEVMEM;
+}
+
+static inline struct page_pool_iov *page_to_page_pool_iov(struct page *page)
+{
+ if (page_is_page_pool_iov(page))
+ return (struct page_pool_iov *)((unsigned long)page &
+ ~PP_DEVMEM);
+
+ DEBUG_NET_WARN_ON_ONCE(true);
+ return NULL;
+}
+
/**
* page_pool_dev_alloc_pages() - allocate a page.
* @pool: pool from which to allocate
diff --git a/include/net/page_pool/types.h b/include/net/page_pool/types.h
index 64386325d965..1e67f9466250 100644
--- a/include/net/page_pool/types.h
+++ b/include/net/page_pool/types.h
@@ -124,6 +124,7 @@ struct mem_provider;
enum pp_memory_provider_type {
__PP_MP_NONE, /* Use system allocator directly */
+ PP_MP_DMABUF_DEVMEM, /* dmabuf devmem provider */
};
struct pp_memory_provider_ops {
@@ -133,8 +134,15 @@ struct pp_memory_provider_ops {
bool (*release_page)(struct page_pool *pool, struct page *page);
};
+extern const struct pp_memory_provider_ops dmabuf_devmem_ops;
+
/* page_pool_iov support */
+/* We overload the LSB of the struct page pointer to indicate whether it's
+ * a page or page_pool_iov.
+ */
+#define PP_DEVMEM 0x01UL
+
/* Owner of the dma-buf chunks inserted into the gen pool. Each scatterlist
* entry from the dmabuf is inserted into the genpool as a chunk, and needs
* this owner struct to keep track of some metadata necessary to create
@@ -158,6 +166,8 @@ struct page_pool_iov {
struct dmabuf_genpool_chunk_owner *owner;
refcount_t refcount;
+
+ struct page_pool *pp;
};
struct page_pool {
diff --git a/net/core/page_pool.c b/net/core/page_pool.c
index 7ea1f4682479..138ddea0b28f 100644
--- a/net/core/page_pool.c
+++ b/net/core/page_pool.c
@@ -20,6 +20,7 @@
#include <linux/poison.h>
#include <linux/ethtool.h>
#include <linux/netdevice.h>
+#include <linux/genalloc.h>
#include <trace/events/page_pool.h>
@@ -231,6 +232,9 @@ static int page_pool_init(struct page_pool *pool,
switch (pool->p.memory_provider) {
case __PP_MP_NONE:
break;
+ case PP_MP_DMABUF_DEVMEM:
+ pool->mp_ops = &dmabuf_devmem_ops;
+ break;
default:
err = -EINVAL;
goto free_ptr_ring;
@@ -996,3 +1000,75 @@ void page_pool_update_nid(struct page_pool *pool, int new_nid)
}
}
EXPORT_SYMBOL(page_pool_update_nid);
+
+void __page_pool_iov_free(struct page_pool_iov *ppiov)
+{
+ if (ppiov->pp->mp_ops != &dmabuf_devmem_ops)
+ return;
+
+ netdev_free_devmem(ppiov);
+}
+EXPORT_SYMBOL_GPL(__page_pool_iov_free);
+
+/*** "Dmabuf devmem memory provider" ***/
+
+static int mp_dmabuf_devmem_init(struct page_pool *pool)
+{
+ struct netdev_dmabuf_binding *binding = pool->mp_priv;
+
+ if (!binding)
+ return -EINVAL;
+
+ if (pool->p.flags & PP_FLAG_DMA_MAP ||
+ pool->p.flags & PP_FLAG_DMA_SYNC_DEV)
+ return -EOPNOTSUPP;
+
+ netdev_devmem_binding_get(binding);
+ return 0;
+}
+
+static struct page *mp_dmabuf_devmem_alloc_pages(struct page_pool *pool,
+ gfp_t gfp)
+{
+ struct netdev_dmabuf_binding *binding = pool->mp_priv;
+ struct page_pool_iov *ppiov;
+
+ ppiov = netdev_alloc_devmem(binding);
+ if (!ppiov)
+ return NULL;
+
+ ppiov->pp = pool;
+ pool->pages_state_hold_cnt++;
+ trace_page_pool_state_hold(pool, (struct page *)ppiov,
+ pool->pages_state_hold_cnt);
+ return (struct page *)((unsigned long)ppiov | PP_DEVMEM);
+}
+
+static void mp_dmabuf_devmem_destroy(struct page_pool *pool)
+{
+ struct netdev_dmabuf_binding *binding = pool->mp_priv;
+
+ netdev_devmem_binding_put(binding);
+}
+
+static bool mp_dmabuf_devmem_release_page(struct page_pool *pool,
+ struct page *page)
+{
+ struct page_pool_iov *ppiov;
+
+ if (WARN_ON_ONCE(!page_is_page_pool_iov(page)))
+ return false;
+
+ ppiov = page_to_page_pool_iov(page);
+ page_pool_iov_put_many(ppiov, 1);
+ /* We don't want the page pool put_page()ing our page_pool_iovs. */
+ return false;
+}
+
+const struct pp_memory_provider_ops dmabuf_devmem_ops = {
+ .init = mp_dmabuf_devmem_init,
+ .destroy = mp_dmabuf_devmem_destroy,
+ .alloc_pages = mp_dmabuf_devmem_alloc_pages,
+ .release_page = mp_dmabuf_devmem_release_page,
+};
+EXPORT_SYMBOL(dmabuf_devmem_ops);
--
2.42.0.869.gea05f2083d-goog
^ permalink raw reply related
* [RFC PATCH v3 07/12] page-pool: device memory support
From: Mina Almasry @ 2023-11-06 2:44 UTC (permalink / raw)
To: netdev, linux-kernel, linux-arch, linux-kselftest, linux-media,
dri-devel, linaro-mm-sig
Cc: Mina Almasry, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Jesper Dangaard Brouer, Ilias Apalodimas,
Arnd Bergmann, David Ahern, Willem de Bruijn, Shuah Khan,
Sumit Semwal, Christian König, Shakeel Butt, Jeroen de Borst,
Praveen Kaligineedi
In-Reply-To: <20231106024413.2801438-1-almasrymina@google.com>
Overload the LSB of struct page* to indicate that it's a page_pool_iov.
Refactor mm calls on struct page* into helpers, and add page_pool_iov
handling on those helpers. Modify callers of these mm APIs with calls to
these helpers instead.
In areas where struct page* is dereferenced, add a check for special
handling of page_pool_iov.
Signed-off-by: Mina Almasry <almasrymina@google.com>
---
include/net/page_pool/helpers.h | 74 ++++++++++++++++++++++++++++++++-
net/core/page_pool.c | 63 ++++++++++++++++++++--------
2 files changed, 118 insertions(+), 19 deletions(-)
diff --git a/include/net/page_pool/helpers.h b/include/net/page_pool/helpers.h
index b93243c2a640..08f1a2cc70d2 100644
--- a/include/net/page_pool/helpers.h
+++ b/include/net/page_pool/helpers.h
@@ -151,6 +151,64 @@ static inline struct page_pool_iov *page_to_page_pool_iov(struct page *page)
return NULL;
}
+static inline int page_pool_page_ref_count(struct page *page)
+{
+ if (page_is_page_pool_iov(page))
+ return page_pool_iov_refcount(page_to_page_pool_iov(page));
+
+ return page_ref_count(page);
+}
+
+static inline void page_pool_page_get_many(struct page *page,
+ unsigned int count)
+{
+ if (page_is_page_pool_iov(page))
+ return page_pool_iov_get_many(page_to_page_pool_iov(page),
+ count);
+
+ return page_ref_add(page, count);
+}
+
+static inline void page_pool_page_put_many(struct page *page,
+ unsigned int count)
+{
+ if (page_is_page_pool_iov(page))
+ return page_pool_iov_put_many(page_to_page_pool_iov(page),
+ count);
+
+ if (count > 1)
+ page_ref_sub(page, count - 1);
+
+ put_page(page);
+}
+
+static inline bool page_pool_page_is_pfmemalloc(struct page *page)
+{
+ if (page_is_page_pool_iov(page))
+ return false;
+
+ return page_is_pfmemalloc(page);
+}
+
+static inline bool page_pool_page_is_pref_nid(struct page *page, int pref_nid)
+{
+ /* Assume page_pool_iov are on the preferred node without actually
+ * checking...
+ *
+ * This check is only used to check for recycling memory in the page
+ * pool's fast paths. Currently the only implementation of page_pool_iov
+ * is dmabuf device memory. It's a deliberate decision by the user to
+ * bind a certain dmabuf to a certain netdev, and the netdev rx queue
+ * would not be able to reallocate memory from another dmabuf that
+ * exists on the preferred node, so, this check doesn't make much sense
+ * in this case. Assume all page_pool_iovs can be recycled for now.
+ */
+ if (page_is_page_pool_iov(page))
+ return true;
+
+ return page_to_nid(page) == pref_nid;
+}
+
/**
* page_pool_dev_alloc_pages() - allocate a page.
* @pool: pool from which to allocate
@@ -301,6 +359,9 @@ static inline long page_pool_defrag_page(struct page *page, long nr)
{
long ret;
+ if (page_is_page_pool_iov(page))
+ return -EINVAL;
+
/* If nr == pp_frag_count then we have cleared all remaining
* references to the page:
* 1. 'n == 1': no need to actually overwrite it.
@@ -431,7 +492,12 @@ static inline void page_pool_free_va(struct page_pool *pool, void *va,
*/
static inline dma_addr_t page_pool_get_dma_addr(struct page *page)
{
- dma_addr_t ret = page->dma_addr;
+ dma_addr_t ret;
+
+ if (page_is_page_pool_iov(page))
+ return page_pool_iov_dma_addr(page_to_page_pool_iov(page));
+
+ ret = page->dma_addr;
if (PAGE_POOL_32BIT_ARCH_WITH_64BIT_DMA)
ret <<= PAGE_SHIFT;
@@ -441,6 +507,12 @@ static inline dma_addr_t page_pool_get_dma_addr(struct page *page)
static inline bool page_pool_set_dma_addr(struct page *page, dma_addr_t addr)
{
+ /* page_pool_iovs are mapped and their dma-addr can't be modified. */
+ if (page_is_page_pool_iov(page)) {
+ DEBUG_NET_WARN_ON_ONCE(true);
+ return false;
+ }
+
if (PAGE_POOL_32BIT_ARCH_WITH_64BIT_DMA) {
page->dma_addr = addr >> PAGE_SHIFT;
diff --git a/net/core/page_pool.c b/net/core/page_pool.c
index 138ddea0b28f..d211996d423b 100644
--- a/net/core/page_pool.c
+++ b/net/core/page_pool.c
@@ -317,7 +317,7 @@ static struct page *page_pool_refill_alloc_cache(struct page_pool *pool)
if (unlikely(!page))
break;
- if (likely(page_to_nid(page) == pref_nid)) {
+ if (likely(page_pool_page_is_pref_nid(page, pref_nid))) {
pool->alloc.cache[pool->alloc.count++] = page;
} else {
/* NUMA mismatch;
@@ -362,7 +362,15 @@ static void page_pool_dma_sync_for_device(struct page_pool *pool,
struct page *page,
unsigned int dma_sync_size)
{
- dma_addr_t dma_addr = page_pool_get_dma_addr(page);
+ dma_addr_t dma_addr;
+
+ /* page_pool_iov memory provider do not support PP_FLAG_DMA_SYNC_DEV */
+ if (page_is_page_pool_iov(page)) {
+ DEBUG_NET_WARN_ON_ONCE(true);
+ return;
+ }
+
+ dma_addr = page_pool_get_dma_addr(page);
dma_sync_size = min(dma_sync_size, pool->p.max_len);
dma_sync_single_range_for_device(pool->p.dev, dma_addr,
@@ -374,6 +382,12 @@ static bool page_pool_dma_map(struct page_pool *pool, struct page *page)
{
dma_addr_t dma;
+ if (page_is_page_pool_iov(page)) {
+ /* page_pool_iovs are already mapped */
+ DEBUG_NET_WARN_ON_ONCE(true);
+ return true;
+ }
+
/* Setup DMA mapping: use 'struct page' area for storing DMA-addr
* since dma_addr_t can be either 32 or 64 bits and does not always fit
* into page private data (i.e 32bit cpu with 64bit DMA caps)
@@ -405,22 +419,33 @@ static bool page_pool_dma_map(struct page_pool *pool, struct page *page)
static void page_pool_set_pp_info(struct page_pool *pool,
struct page *page)
{
- page->pp = pool;
- page->pp_magic |= PP_SIGNATURE;
-
- /* Ensuring all pages have been split into one fragment initially:
- * page_pool_set_pp_info() is only called once for every page when it
- * is allocated from the page allocator and page_pool_fragment_page()
- * is dirtying the same cache line as the page->pp_magic above, so
- * the overhead is negligible.
- */
- page_pool_fragment_page(page, 1);
+ if (!page_is_page_pool_iov(page)) {
+ page->pp = pool;
+ page->pp_magic |= PP_SIGNATURE;
+
+ /* Ensuring all pages have been split into one fragment
+ * initially:
+ * page_pool_set_pp_info() is only called once for every page
+ * when it is allocated from the page allocator and
+ * page_pool_fragment_page() is dirtying the same cache line as
+ * the page->pp_magic above, so * the overhead is negligible.
+ */
+ page_pool_fragment_page(page, 1);
+ } else {
+ page_to_page_pool_iov(page)->pp = pool;
+ }
+
if (pool->p.init_callback)
pool->p.init_callback(page, pool->p.init_arg);
}
static void page_pool_clear_pp_info(struct page *page)
{
+ if (page_is_page_pool_iov(page)) {
+ page_to_page_pool_iov(page)->pp = NULL;
+ return;
+ }
+
page->pp_magic = 0;
page->pp = NULL;
}
@@ -630,7 +655,7 @@ static bool page_pool_recycle_in_cache(struct page *page,
return false;
}
- /* Caller MUST have verified/know (page_ref_count(page) == 1) */
+ /* Caller MUST have verified/know (page_pool_page_ref_count(page) == 1) */
pool->alloc.cache[pool->alloc.count++] = page;
recycle_stat_inc(pool, cached);
return true;
@@ -655,9 +680,10 @@ __page_pool_put_page(struct page_pool *pool, struct page *page,
* refcnt == 1 means page_pool owns page, and can recycle it.
*
* page is NOT reusable when allocated when system is under
- * some pressure. (page_is_pfmemalloc)
+ * some pressure. (page_pool_page_is_pfmemalloc)
*/
- if (likely(page_ref_count(page) == 1 && !page_is_pfmemalloc(page))) {
+ if (likely(page_pool_page_ref_count(page) == 1 &&
+ !page_pool_page_is_pfmemalloc(page))) {
/* Read barrier done in page_ref_count / READ_ONCE */
if (pool->p.flags & PP_FLAG_DMA_SYNC_DEV)
@@ -772,7 +798,8 @@ static struct page *page_pool_drain_frag(struct page_pool *pool,
if (likely(page_pool_defrag_page(page, drain_count)))
return NULL;
- if (page_ref_count(page) == 1 && !page_is_pfmemalloc(page)) {
+ if (page_pool_page_ref_count(page) == 1 &&
+ !page_pool_page_is_pfmemalloc(page)) {
if (pool->p.flags & PP_FLAG_DMA_SYNC_DEV)
page_pool_dma_sync_for_device(pool, page, -1);
@@ -848,9 +875,9 @@ static void page_pool_empty_ring(struct page_pool *pool)
/* Empty recycle ring */
while ((page = ptr_ring_consume_bh(&pool->ring))) {
/* Verify the refcnt invariant of cached pages */
- if (!(page_ref_count(page) == 1))
+ if (!(page_pool_page_ref_count(page) == 1))
pr_crit("%s() page_pool refcnt %d violation\n",
- __func__, page_ref_count(page));
+ __func__, page_pool_page_ref_count(page));
page_pool_return_page(pool, page);
}
--
2.42.0.869.gea05f2083d-goog
^ permalink raw reply related
* [RFC PATCH v3 08/12] net: support non paged skb frags
From: Mina Almasry @ 2023-11-06 2:44 UTC (permalink / raw)
To: netdev, linux-kernel, linux-arch, linux-kselftest, linux-media,
dri-devel, linaro-mm-sig
Cc: Mina Almasry, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Jesper Dangaard Brouer, Ilias Apalodimas,
Arnd Bergmann, David Ahern, Willem de Bruijn, Shuah Khan,
Sumit Semwal, Christian König, Shakeel Butt, Jeroen de Borst,
Praveen Kaligineedi
In-Reply-To: <20231106024413.2801438-1-almasrymina@google.com>
Make skb_frag_page() fail in the case where the frag is not backed
by a page, and fix its relevent callers to handle this case.
Correctly handle skb_frag refcounting in the page_pool_iovs case.
Signed-off-by: Mina Almasry <almasrymina@google.com>
---
include/linux/skbuff.h | 42 +++++++++++++++++++++++++++++++++++-------
net/core/gro.c | 2 +-
net/core/skbuff.c | 3 +++
net/ipv4/tcp.c | 10 +++++++++-
4 files changed, 48 insertions(+), 9 deletions(-)
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 97bfef071255..1fae276c1353 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -37,6 +37,8 @@
#endif
#include <net/net_debug.h>
#include <net/dropreason-core.h>
+#include <net/page_pool/types.h>
+#include <net/page_pool/helpers.h>
/**
* DOC: skb checksums
@@ -3402,15 +3404,38 @@ static inline void skb_frag_off_copy(skb_frag_t *fragto,
fragto->bv_offset = fragfrom->bv_offset;
}
+/* Returns true if the skb_frag contains a page_pool_iov. */
+static inline bool skb_frag_is_page_pool_iov(const skb_frag_t *frag)
+{
+ return page_is_page_pool_iov(frag->bv_page);
+}
+
/**
* skb_frag_page - retrieve the page referred to by a paged fragment
* @frag: the paged fragment
*
- * Returns the &struct page associated with @frag.
+ * Returns the &struct page associated with @frag. Returns NULL if this frag
+ * has no associated page.
*/
static inline struct page *skb_frag_page(const skb_frag_t *frag)
{
- return frag->bv_page;
+ if (!page_is_page_pool_iov(frag->bv_page))
+ return frag->bv_page;
+
+ return NULL;
+}
+
+/**
+ * skb_frag_page_pool_iov - retrieve the page_pool_iov referred to by fragment
+ * @frag: the fragment
+ *
+ * Returns the &struct page_pool_iov associated with @frag. Returns NULL if this
+ * frag has no associated page_pool_iov.
+ */
+static inline struct page_pool_iov *
+skb_frag_page_pool_iov(const skb_frag_t *frag)
+{
+ return page_to_page_pool_iov(frag->bv_page);
}
/**
@@ -3421,7 +3446,7 @@ static inline struct page *skb_frag_page(const skb_frag_t *frag)
*/
static inline void __skb_frag_ref(skb_frag_t *frag)
{
- get_page(skb_frag_page(frag));
+ page_pool_page_get_many(frag->bv_page, 1);
}
/**
@@ -3441,13 +3466,13 @@ bool napi_pp_put_page(struct page *page, bool napi_safe);
static inline void
napi_frag_unref(skb_frag_t *frag, bool recycle, bool napi_safe)
{
- struct page *page = skb_frag_page(frag);
-
#ifdef CONFIG_PAGE_POOL
- if (recycle && napi_pp_put_page(page, napi_safe))
+ if (recycle && napi_pp_put_page(frag->bv_page, napi_safe))
return;
+ page_pool_page_put_many(frag->bv_page, 1);
+#else
+ put_page(skb_frag_page(frag));
#endif
- put_page(page);
}
/**
@@ -3487,6 +3512,9 @@ static inline void skb_frag_unref(struct sk_buff *skb, int f)
*/
static inline void *skb_frag_address(const skb_frag_t *frag)
{
+ if (!skb_frag_page(frag))
+ return NULL;
+
return page_address(skb_frag_page(frag)) + skb_frag_off(frag);
}
diff --git a/net/core/gro.c b/net/core/gro.c
index 0759277dc14e..42d7f6755f32 100644
--- a/net/core/gro.c
+++ b/net/core/gro.c
@@ -376,7 +376,7 @@ static inline void skb_gro_reset_offset(struct sk_buff *skb, u32 nhoff)
NAPI_GRO_CB(skb)->frag0 = NULL;
NAPI_GRO_CB(skb)->frag0_len = 0;
- if (!skb_headlen(skb) && pinfo->nr_frags &&
+ if (!skb_headlen(skb) && pinfo->nr_frags && skb_frag_page(frag0) &&
!PageHighMem(skb_frag_page(frag0)) &&
(!NET_IP_ALIGN || !((skb_frag_off(frag0) + nhoff) & 3))) {
NAPI_GRO_CB(skb)->frag0 = skb_frag_address(frag0);
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index c52ddd6891d9..13eca4fd25e1 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -2994,6 +2994,9 @@ static bool __skb_splice_bits(struct sk_buff *skb, struct pipe_inode_info *pipe,
for (seg = 0; seg < skb_shinfo(skb)->nr_frags; seg++) {
const skb_frag_t *f = &skb_shinfo(skb)->frags[seg];
+ if (WARN_ON_ONCE(!skb_frag_page(f)))
+ return false;
+
if (__splice_segment(skb_frag_page(f),
skb_frag_off(f), skb_frag_size(f),
offset, len, spd, false, sk, pipe))
diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index a86d8200a1e8..23b29dc49271 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -2155,6 +2155,9 @@ static int tcp_zerocopy_receive(struct sock *sk,
break;
}
page = skb_frag_page(frags);
+ if (WARN_ON_ONCE(!page))
+ break;
+
prefetchw(page);
pages[pages_to_map++] = page;
length += PAGE_SIZE;
@@ -4411,7 +4414,12 @@ int tcp_md5_hash_skb_data(struct tcp_md5sig_pool *hp,
for (i = 0; i < shi->nr_frags; ++i) {
const skb_frag_t *f = &shi->frags[i];
unsigned int offset = skb_frag_off(f);
- struct page *page = skb_frag_page(f) + (offset >> PAGE_SHIFT);
+ struct page *page = skb_frag_page(f);
+
+ if (WARN_ON_ONCE(!page))
+ return 1;
+
+ page += offset >> PAGE_SHIFT;
sg_set_page(&sg, page, skb_frag_size(f),
offset_in_page(offset));
--
2.42.0.869.gea05f2083d-goog
^ permalink raw reply related
* [RFC PATCH v3 09/12] net: add support for skbs with unreadable frags
From: Mina Almasry @ 2023-11-06 2:44 UTC (permalink / raw)
To: netdev, linux-kernel, linux-arch, linux-kselftest, linux-media,
dri-devel, linaro-mm-sig
Cc: Mina Almasry, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Jesper Dangaard Brouer, Ilias Apalodimas,
Arnd Bergmann, David Ahern, Willem de Bruijn, Shuah Khan,
Sumit Semwal, Christian König, Shakeel Butt, Jeroen de Borst,
Praveen Kaligineedi, Willem de Bruijn, Kaiyuan Zhang
In-Reply-To: <20231106024413.2801438-1-almasrymina@google.com>
For device memory TCP, we expect the skb headers to be available in host
memory for access, and we expect the skb frags to be in device memory
and unaccessible to the host. We expect there to be no mixing and
matching of device memory frags (unaccessible) with host memory frags
(accessible) in the same skb.
Add a skb->devmem flag which indicates whether the frags in this skb
are device memory frags or not.
__skb_fill_page_desc() now checks frags added to skbs for page_pool_iovs,
and marks the skb as skb->devmem accordingly.
Add checks through the network stack to avoid accessing the frags of
devmem skbs and avoid coalescing devmem skbs with non devmem skbs.
Signed-off-by: Willem de Bruijn <willemb@google.com>
Signed-off-by: Kaiyuan Zhang <kaiyuanz@google.com>
Signed-off-by: Mina Almasry <almasrymina@google.com>
---
include/linux/skbuff.h | 14 +++++++-
include/net/tcp.h | 5 +--
net/core/datagram.c | 6 ++++
net/core/gro.c | 5 ++-
net/core/skbuff.c | 77 ++++++++++++++++++++++++++++++++++++------
net/ipv4/tcp.c | 6 ++++
net/ipv4/tcp_input.c | 13 +++++--
net/ipv4/tcp_output.c | 5 ++-
net/packet/af_packet.c | 4 +--
9 files changed, 115 insertions(+), 20 deletions(-)
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 1fae276c1353..8fb468ff8115 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -805,6 +805,8 @@ typedef unsigned char *sk_buff_data_t;
* @csum_level: indicates the number of consecutive checksums found in
* the packet minus one that have been verified as
* CHECKSUM_UNNECESSARY (max 3)
+ * @devmem: indicates that all the fragments in this skb are backed by
+ * device memory.
* @dst_pending_confirm: need to confirm neighbour
* @decrypted: Decrypted SKB
* @slow_gro: state present at GRO time, slower prepare step required
@@ -991,7 +993,7 @@ struct sk_buff {
#if IS_ENABLED(CONFIG_IP_SCTP)
__u8 csum_not_inet:1;
#endif
-
+ __u8 devmem:1;
#if defined(CONFIG_NET_SCHED) || defined(CONFIG_NET_XGRESS)
__u16 tc_index; /* traffic control index */
#endif
@@ -1766,6 +1768,12 @@ static inline void skb_zcopy_downgrade_managed(struct sk_buff *skb)
__skb_zcopy_downgrade_managed(skb);
}
+/* Return true if frags in this skb are not readable by the host. */
+static inline bool skb_frags_not_readable(const struct sk_buff *skb)
+{
+ return skb->devmem;
+}
+
static inline void skb_mark_not_on_list(struct sk_buff *skb)
{
skb->next = NULL;
@@ -2468,6 +2476,10 @@ static inline void __skb_fill_page_desc(struct sk_buff *skb, int i,
struct page *page, int off, int size)
{
__skb_fill_page_desc_noacc(skb_shinfo(skb), i, page, off, size);
+ if (page_is_page_pool_iov(page)) {
+ skb->devmem = true;
+ return;
+ }
/* Propagate page pfmemalloc to the skb if we can. The problem is
* that not all callers have unique ownership of the page but rely
diff --git a/include/net/tcp.h b/include/net/tcp.h
index 39b731c900dd..1ae62d1e284b 100644
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -1012,7 +1012,7 @@ static inline int tcp_skb_mss(const struct sk_buff *skb)
static inline bool tcp_skb_can_collapse_to(const struct sk_buff *skb)
{
- return likely(!TCP_SKB_CB(skb)->eor);
+ return likely(!TCP_SKB_CB(skb)->eor && !skb_frags_not_readable(skb));
}
static inline bool tcp_skb_can_collapse(const struct sk_buff *to,
@@ -1020,7 +1020,8 @@ static inline bool tcp_skb_can_collapse(const struct sk_buff *to,
{
return likely(tcp_skb_can_collapse_to(to) &&
mptcp_skb_can_collapse(to, from) &&
- skb_pure_zcopy_same(to, from));
+ skb_pure_zcopy_same(to, from) &&
+ skb_frags_not_readable(to) == skb_frags_not_readable(from));
}
/* Events passed to congestion control interface */
diff --git a/net/core/datagram.c b/net/core/datagram.c
index 176eb5834746..cdd4fb129968 100644
--- a/net/core/datagram.c
+++ b/net/core/datagram.c
@@ -425,6 +425,9 @@ static int __skb_datagram_iter(const struct sk_buff *skb, int offset,
return 0;
}
+ if (skb_frags_not_readable(skb))
+ goto short_copy;
+
/* Copy paged appendix. Hmm... why does this look so complicated? */
for (i = 0; i < skb_shinfo(skb)->nr_frags; i++) {
int end;
@@ -616,6 +619,9 @@ int __zerocopy_sg_from_iter(struct msghdr *msg, struct sock *sk,
{
int frag;
+ if (skb_frags_not_readable(skb))
+ return -EFAULT;
+
if (msg && msg->msg_ubuf && msg->sg_from_iter)
return msg->sg_from_iter(sk, skb, from, length);
diff --git a/net/core/gro.c b/net/core/gro.c
index 42d7f6755f32..56046d65386a 100644
--- a/net/core/gro.c
+++ b/net/core/gro.c
@@ -390,6 +390,9 @@ static void gro_pull_from_frag0(struct sk_buff *skb, int grow)
{
struct skb_shared_info *pinfo = skb_shinfo(skb);
+ if (WARN_ON_ONCE(skb_frags_not_readable(skb)))
+ return;
+
BUG_ON(skb->end - skb->tail < grow);
memcpy(skb_tail_pointer(skb), NAPI_GRO_CB(skb)->frag0, grow);
@@ -411,7 +414,7 @@ static void gro_try_pull_from_frag0(struct sk_buff *skb)
{
int grow = skb_gro_offset(skb) - skb_headlen(skb);
- if (grow > 0)
+ if (grow > 0 && !skb_frags_not_readable(skb))
gro_pull_from_frag0(skb, grow);
}
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index 13eca4fd25e1..f01673ed2eff 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -1230,6 +1230,14 @@ void skb_dump(const char *level, const struct sk_buff *skb, bool full_pkt)
struct page *p;
u8 *vaddr;
+ if (skb_frag_is_page_pool_iov(frag)) {
+ printk("%sskb frag %d: not readable\n", level, i);
+ len -= frag->bv_len;
+ if (!len)
+ break;
+ continue;
+ }
+
skb_frag_foreach_page(frag, skb_frag_off(frag),
skb_frag_size(frag), p, p_off, p_len,
copied) {
@@ -1807,6 +1815,9 @@ int skb_copy_ubufs(struct sk_buff *skb, gfp_t gfp_mask)
if (skb_shared(skb) || skb_unclone(skb, gfp_mask))
return -EINVAL;
+ if (skb_frags_not_readable(skb))
+ return -EFAULT;
+
if (!num_frags)
goto release;
@@ -1977,8 +1988,12 @@ struct sk_buff *skb_copy(const struct sk_buff *skb, gfp_t gfp_mask)
{
int headerlen = skb_headroom(skb);
unsigned int size = skb_end_offset(skb) + skb->data_len;
- struct sk_buff *n = __alloc_skb(size, gfp_mask,
- skb_alloc_rx_flag(skb), NUMA_NO_NODE);
+ struct sk_buff *n;
+
+ if (skb_frags_not_readable(skb))
+ return NULL;
+
+ n = __alloc_skb(size, gfp_mask, skb_alloc_rx_flag(skb), NUMA_NO_NODE);
if (!n)
return NULL;
@@ -2304,14 +2319,16 @@ struct sk_buff *skb_copy_expand(const struct sk_buff *skb,
int newheadroom, int newtailroom,
gfp_t gfp_mask)
{
- /*
- * Allocate the copy buffer
- */
- struct sk_buff *n = __alloc_skb(newheadroom + skb->len + newtailroom,
- gfp_mask, skb_alloc_rx_flag(skb),
- NUMA_NO_NODE);
int oldheadroom = skb_headroom(skb);
int head_copy_len, head_copy_off;
+ struct sk_buff *n;
+
+ if (skb_frags_not_readable(skb))
+ return NULL;
+
+ /* Allocate the copy buffer */
+ n = __alloc_skb(newheadroom + skb->len + newtailroom, gfp_mask,
+ skb_alloc_rx_flag(skb), NUMA_NO_NODE);
if (!n)
return NULL;
@@ -2650,6 +2667,9 @@ void *__pskb_pull_tail(struct sk_buff *skb, int delta)
*/
int i, k, eat = (skb->tail + delta) - skb->end;
+ if (skb_frags_not_readable(skb))
+ return NULL;
+
if (eat > 0 || skb_cloned(skb)) {
if (pskb_expand_head(skb, 0, eat > 0 ? eat + 128 : 0,
GFP_ATOMIC))
@@ -2803,6 +2823,9 @@ int skb_copy_bits(const struct sk_buff *skb, int offset, void *to, int len)
to += copy;
}
+ if (skb_frags_not_readable(skb))
+ goto fault;
+
for (i = 0; i < skb_shinfo(skb)->nr_frags; i++) {
int end;
skb_frag_t *f = &skb_shinfo(skb)->frags[i];
@@ -2991,6 +3014,9 @@ static bool __skb_splice_bits(struct sk_buff *skb, struct pipe_inode_info *pipe,
/*
* then map the fragments
*/
+ if (skb_frags_not_readable(skb))
+ return false;
+
for (seg = 0; seg < skb_shinfo(skb)->nr_frags; seg++) {
const skb_frag_t *f = &skb_shinfo(skb)->frags[seg];
@@ -3214,6 +3240,9 @@ int skb_store_bits(struct sk_buff *skb, int offset, const void *from, int len)
from += copy;
}
+ if (skb_frags_not_readable(skb))
+ goto fault;
+
for (i = 0; i < skb_shinfo(skb)->nr_frags; i++) {
skb_frag_t *frag = &skb_shinfo(skb)->frags[i];
int end;
@@ -3293,6 +3322,9 @@ __wsum __skb_checksum(const struct sk_buff *skb, int offset, int len,
pos = copy;
}
+ if (skb_frags_not_readable(skb))
+ return 0;
+
for (i = 0; i < skb_shinfo(skb)->nr_frags; i++) {
int end;
skb_frag_t *frag = &skb_shinfo(skb)->frags[i];
@@ -3393,6 +3425,9 @@ __wsum skb_copy_and_csum_bits(const struct sk_buff *skb, int offset,
pos = copy;
}
+ if (skb_frags_not_readable(skb))
+ return 0;
+
for (i = 0; i < skb_shinfo(skb)->nr_frags; i++) {
int end;
@@ -3883,7 +3918,9 @@ static inline void skb_split_inside_header(struct sk_buff *skb,
skb_shinfo(skb1)->frags[i] = skb_shinfo(skb)->frags[i];
skb_shinfo(skb1)->nr_frags = skb_shinfo(skb)->nr_frags;
+ skb1->devmem = skb->devmem;
skb_shinfo(skb)->nr_frags = 0;
+ skb->devmem = 0;
skb1->data_len = skb->data_len;
skb1->len += skb1->data_len;
skb->data_len = 0;
@@ -3897,6 +3934,7 @@ static inline void skb_split_no_header(struct sk_buff *skb,
{
int i, k = 0;
const int nfrags = skb_shinfo(skb)->nr_frags;
+ const int devmem = skb->devmem;
skb_shinfo(skb)->nr_frags = 0;
skb1->len = skb1->data_len = skb->len - len;
@@ -3930,6 +3968,16 @@ static inline void skb_split_no_header(struct sk_buff *skb,
pos += size;
}
skb_shinfo(skb1)->nr_frags = k;
+
+ if (skb_shinfo(skb)->nr_frags)
+ skb->devmem = devmem;
+ else
+ skb->devmem = 0;
+
+ if (skb_shinfo(skb1)->nr_frags)
+ skb1->devmem = devmem;
+ else
+ skb1->devmem = 0;
}
/**
@@ -4165,6 +4213,9 @@ unsigned int skb_seq_read(unsigned int consumed, const u8 **data,
return block_limit - abs_offset;
}
+ if (skb_frags_not_readable(st->cur_skb))
+ return 0;
+
if (st->frag_idx == 0 && !st->frag_data)
st->stepped_offset += skb_headlen(st->cur_skb);
@@ -5779,7 +5830,10 @@ bool skb_try_coalesce(struct sk_buff *to, struct sk_buff *from,
(from->pp_recycle && skb_cloned(from)))
return false;
- if (len <= skb_tailroom(to)) {
+ if (skb_frags_not_readable(from) != skb_frags_not_readable(to))
+ return false;
+
+ if (len <= skb_tailroom(to) && !skb_frags_not_readable(from)) {
if (len)
BUG_ON(skb_copy_bits(from, 0, skb_put(to, len), len));
*delta_truesize = 0;
@@ -5954,6 +6008,9 @@ int skb_ensure_writable(struct sk_buff *skb, unsigned int write_len)
if (!pskb_may_pull(skb, write_len))
return -ENOMEM;
+ if (skb_frags_not_readable(skb))
+ return -EFAULT;
+
if (!skb_cloned(skb) || skb_clone_writable(skb, write_len))
return 0;
@@ -6608,7 +6665,7 @@ void skb_condense(struct sk_buff *skb)
{
if (skb->data_len) {
if (skb->data_len > skb->end - skb->tail ||
- skb_cloned(skb))
+ skb_cloned(skb) || skb_frags_not_readable(skb))
return;
/* Nice, we can free page frag(s) right now */
diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index 23b29dc49271..5c6fed52ed0e 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -2138,6 +2138,9 @@ static int tcp_zerocopy_receive(struct sock *sk,
skb = tcp_recv_skb(sk, seq, &offset);
}
+ if (skb_frags_not_readable(skb))
+ break;
+
if (TCP_SKB_CB(skb)->has_rxtstamp) {
tcp_update_recv_tstamps(skb, tss);
zc->msg_flags |= TCP_CMSG_TS;
@@ -4411,6 +4414,9 @@ int tcp_md5_hash_skb_data(struct tcp_md5sig_pool *hp,
if (crypto_ahash_update(req))
return 1;
+ if (skb_frags_not_readable(skb))
+ return 1;
+
for (i = 0; i < shi->nr_frags; ++i) {
const skb_frag_t *f = &shi->frags[i];
unsigned int offset = skb_frag_off(f);
diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index 18b858597af4..64643dad5e1a 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -5264,6 +5264,9 @@ tcp_collapse(struct sock *sk, struct sk_buff_head *list, struct rb_root *root,
for (end_of_skbs = true; skb != NULL && skb != tail; skb = n) {
n = tcp_skb_next(skb, list);
+ if (skb_frags_not_readable(skb))
+ goto skip_this;
+
/* No new bits? It is possible on ofo queue. */
if (!before(start, TCP_SKB_CB(skb)->end_seq)) {
skb = tcp_collapse_one(sk, skb, list, root);
@@ -5284,17 +5287,20 @@ tcp_collapse(struct sock *sk, struct sk_buff_head *list, struct rb_root *root,
break;
}
- if (n && n != tail && mptcp_skb_can_collapse(skb, n) &&
+ if (n && n != tail && !skb_frags_not_readable(n) &&
+ mptcp_skb_can_collapse(skb, n) &&
TCP_SKB_CB(skb)->end_seq != TCP_SKB_CB(n)->seq) {
end_of_skbs = false;
break;
}
+skip_this:
/* Decided to skip this, advance start seq. */
start = TCP_SKB_CB(skb)->end_seq;
}
if (end_of_skbs ||
- (TCP_SKB_CB(skb)->tcp_flags & (TCPHDR_SYN | TCPHDR_FIN)))
+ (TCP_SKB_CB(skb)->tcp_flags & (TCPHDR_SYN | TCPHDR_FIN)) ||
+ skb_frags_not_readable(skb))
return;
__skb_queue_head_init(&tmp);
@@ -5338,7 +5344,8 @@ tcp_collapse(struct sock *sk, struct sk_buff_head *list, struct rb_root *root,
if (!skb ||
skb == tail ||
!mptcp_skb_can_collapse(nskb, skb) ||
- (TCP_SKB_CB(skb)->tcp_flags & (TCPHDR_SYN | TCPHDR_FIN)))
+ (TCP_SKB_CB(skb)->tcp_flags & (TCPHDR_SYN | TCPHDR_FIN)) ||
+ skb_frags_not_readable(skb))
goto end;
#ifdef CONFIG_TLS_DEVICE
if (skb->decrypted != nskb->decrypted)
diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
index 2866ccbccde0..60df27f6c649 100644
--- a/net/ipv4/tcp_output.c
+++ b/net/ipv4/tcp_output.c
@@ -2309,7 +2309,8 @@ static bool tcp_can_coalesce_send_queue_head(struct sock *sk, int len)
if (unlikely(TCP_SKB_CB(skb)->eor) ||
tcp_has_tx_tstamp(skb) ||
- !skb_pure_zcopy_same(skb, next))
+ !skb_pure_zcopy_same(skb, next) ||
+ skb_frags_not_readable(skb) != skb_frags_not_readable(next))
return false;
len -= skb->len;
@@ -3193,6 +3194,8 @@ static bool tcp_can_collapse(const struct sock *sk, const struct sk_buff *skb)
return false;
if (skb_cloned(skb))
return false;
+ if (skb_frags_not_readable(skb))
+ return false;
/* Some heuristics for collapsing over SACK'd could be invented */
if (TCP_SKB_CB(skb)->sacked & TCPCB_SACKED_ACKED)
return false;
diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
index a84e00b5904b..8f6cca683939 100644
--- a/net/packet/af_packet.c
+++ b/net/packet/af_packet.c
@@ -2156,7 +2156,7 @@ static int packet_rcv(struct sk_buff *skb, struct net_device *dev,
}
}
- snaplen = skb->len;
+ snaplen = skb_frags_not_readable(skb) ? skb_headlen(skb) : skb->len;
res = run_filter(skb, sk, snaplen);
if (!res)
@@ -2279,7 +2279,7 @@ static int tpacket_rcv(struct sk_buff *skb, struct net_device *dev,
}
}
- snaplen = skb->len;
+ snaplen = skb_frags_not_readable(skb) ? skb_headlen(skb) : skb->len;
res = run_filter(skb, sk, snaplen);
if (!res)
--
2.42.0.869.gea05f2083d-goog
^ permalink raw reply related
* [RFC PATCH v3 10/12] tcp: RX path for devmem TCP
From: Mina Almasry @ 2023-11-06 2:44 UTC (permalink / raw)
To: netdev, linux-kernel, linux-arch, linux-kselftest, linux-media,
dri-devel, linaro-mm-sig
Cc: Mina Almasry, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Jesper Dangaard Brouer, Ilias Apalodimas,
Arnd Bergmann, David Ahern, Willem de Bruijn, Shuah Khan,
Sumit Semwal, Christian König, Shakeel Butt, Jeroen de Borst,
Praveen Kaligineedi, Willem de Bruijn, Kaiyuan Zhang
In-Reply-To: <20231106024413.2801438-1-almasrymina@google.com>
In tcp_recvmsg_locked(), detect if the skb being received by the user
is a devmem skb. In this case - if the user provided the MSG_SOCK_DEVMEM
flag - pass it to tcp_recvmsg_devmem() for custom handling.
tcp_recvmsg_devmem() copies any data in the skb header to the linear
buffer, and returns a cmsg to the user indicating the number of bytes
returned in the linear buffer.
tcp_recvmsg_devmem() then loops over the unaccessible devmem skb frags,
and returns to the user a cmsg_devmem indicating the location of the
data in the dmabuf device memory. cmsg_devmem contains this information:
1. the offset into the dmabuf where the payload starts. 'frag_offset'.
2. the size of the frag. 'frag_size'.
3. an opaque token 'frag_token' to return to the kernel when the buffer
is to be released.
The pages awaiting freeing are stored in the newly added
sk->sk_user_pages, and each page passed to userspace is get_page()'d.
This reference is dropped once the userspace indicates that it is
done reading this page. All pages are released when the socket is
destroyed.
Signed-off-by: Willem de Bruijn <willemb@google.com>
Signed-off-by: Kaiyuan Zhang <kaiyuanz@google.com>
Signed-off-by: Mina Almasry <almasrymina@google.com>
---
RFC v3:
- Fixed issue with put_cmsg() failing silently.
---
include/linux/socket.h | 1 +
include/net/page_pool/helpers.h | 9 ++
include/net/sock.h | 2 +
include/uapi/asm-generic/socket.h | 5 +
include/uapi/linux/uio.h | 6 +
net/ipv4/tcp.c | 189 +++++++++++++++++++++++++++++-
net/ipv4/tcp_ipv4.c | 7 ++
7 files changed, 214 insertions(+), 5 deletions(-)
diff --git a/include/linux/socket.h b/include/linux/socket.h
index cfcb7e2c3813..fe2b9e2081bb 100644
--- a/include/linux/socket.h
+++ b/include/linux/socket.h
@@ -326,6 +326,7 @@ struct ucred {
* plain text and require encryption
*/
+#define MSG_SOCK_DEVMEM 0x2000000 /* Receive devmem skbs as cmsg */
#define MSG_ZEROCOPY 0x4000000 /* Use user data in kernel path */
#define MSG_SPLICE_PAGES 0x8000000 /* Splice the pages from the iterator in sendmsg() */
#define MSG_FASTOPEN 0x20000000 /* Send data in TCP SYN */
diff --git a/include/net/page_pool/helpers.h b/include/net/page_pool/helpers.h
index 08f1a2cc70d2..95f4d579cbc4 100644
--- a/include/net/page_pool/helpers.h
+++ b/include/net/page_pool/helpers.h
@@ -106,6 +106,15 @@ page_pool_iov_dma_addr(const struct page_pool_iov *ppiov)
((dma_addr_t)page_pool_iov_idx(ppiov) << PAGE_SHIFT);
}
+static inline unsigned long
+page_pool_iov_virtual_addr(const struct page_pool_iov *ppiov)
+{
+ struct dmabuf_genpool_chunk_owner *owner = page_pool_iov_owner(ppiov);
+
+ return owner->base_virtual +
+ ((unsigned long)page_pool_iov_idx(ppiov) << PAGE_SHIFT);
+}
+
static inline struct netdev_dmabuf_binding *
page_pool_iov_binding(const struct page_pool_iov *ppiov)
{
diff --git a/include/net/sock.h b/include/net/sock.h
index 242590308d64..986d9da6e062 100644
--- a/include/net/sock.h
+++ b/include/net/sock.h
@@ -353,6 +353,7 @@ struct sk_filter;
* @sk_txtime_unused: unused txtime flags
* @ns_tracker: tracker for netns reference
* @sk_bind2_node: bind node in the bhash2 table
+ * @sk_user_pages: xarray of pages the user is holding a reference on.
*/
struct sock {
/*
@@ -545,6 +546,7 @@ struct sock {
struct rcu_head sk_rcu;
netns_tracker ns_tracker;
struct hlist_node sk_bind2_node;
+ struct xarray sk_user_pages;
};
enum sk_pacing {
diff --git a/include/uapi/asm-generic/socket.h b/include/uapi/asm-generic/socket.h
index 8ce8a39a1e5f..aacb97f16b78 100644
--- a/include/uapi/asm-generic/socket.h
+++ b/include/uapi/asm-generic/socket.h
@@ -135,6 +135,11 @@
#define SO_PASSPIDFD 76
#define SO_PEERPIDFD 77
+#define SO_DEVMEM_HEADER 98
+#define SCM_DEVMEM_HEADER SO_DEVMEM_HEADER
+#define SO_DEVMEM_OFFSET 99
+#define SCM_DEVMEM_OFFSET SO_DEVMEM_OFFSET
+
#if !defined(__KERNEL__)
#if __BITS_PER_LONG == 64 || (defined(__x86_64__) && defined(__ILP32__))
diff --git a/include/uapi/linux/uio.h b/include/uapi/linux/uio.h
index 059b1a9147f4..ae94763b1963 100644
--- a/include/uapi/linux/uio.h
+++ b/include/uapi/linux/uio.h
@@ -20,6 +20,12 @@ struct iovec
__kernel_size_t iov_len; /* Must be size_t (1003.1g) */
};
+struct cmsg_devmem {
+ __u64 frag_offset;
+ __u32 frag_size;
+ __u32 frag_token;
+};
+
/*
* UIO_MAXIOV shall be at least 16 1003.1g (5.4.1.1)
*/
diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index 5c6fed52ed0e..fd7f6d7e7671 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -461,6 +461,7 @@ void tcp_init_sock(struct sock *sk)
set_bit(SOCK_SUPPORT_ZC, &sk->sk_socket->flags);
sk_sockets_allocated_inc(sk);
+ xa_init_flags(&sk->sk_user_pages, XA_FLAGS_ALLOC1);
}
EXPORT_SYMBOL(tcp_init_sock);
@@ -2301,6 +2302,154 @@ static int tcp_inq_hint(struct sock *sk)
return inq;
}
+/* On error, returns the -errno. On success, returns number of bytes sent to the
+ * user. May not consume all of @remaining_len.
+ */
+static int tcp_recvmsg_devmem(const struct sock *sk, const struct sk_buff *skb,
+ unsigned int offset, struct msghdr *msg,
+ int remaining_len)
+{
+ struct cmsg_devmem cmsg_devmem = { 0 };
+ unsigned int start;
+ int i, copy, n;
+ int sent = 0;
+ int err = 0;
+
+ do {
+ start = skb_headlen(skb);
+
+ if (!skb_frags_not_readable(skb)) {
+ err = -ENODEV;
+ goto out;
+ }
+
+ /* Copy header. */
+ copy = start - offset;
+ if (copy > 0) {
+ copy = min(copy, remaining_len);
+
+ n = copy_to_iter(skb->data + offset, copy,
+ &msg->msg_iter);
+ if (n != copy) {
+ err = -EFAULT;
+ goto out;
+ }
+
+ offset += copy;
+ remaining_len -= copy;
+
+ /* First a cmsg_devmem for # bytes copied to user
+ * buffer.
+ */
+ memset(&cmsg_devmem, 0, sizeof(cmsg_devmem));
+ cmsg_devmem.frag_size = copy;
+ err = put_cmsg(msg, SOL_SOCKET, SO_DEVMEM_HEADER,
+ sizeof(cmsg_devmem), &cmsg_devmem);
+ if (err || msg->msg_flags & MSG_CTRUNC) {
+ msg->msg_flags &= ~MSG_CTRUNC;
+ if (!err)
+ err = -ETOOSMALL;
+ goto out;
+ }
+
+ sent += copy;
+
+ if (remaining_len == 0)
+ goto out;
+ }
+
+ /* after that, send information of devmem pages through a
+ * sequence of cmsg
+ */
+ for (i = 0; i < skb_shinfo(skb)->nr_frags; i++) {
+ const skb_frag_t *frag = &skb_shinfo(skb)->frags[i];
+ struct page_pool_iov *ppiov;
+ u64 frag_offset;
+ u32 user_token;
+ int end;
+
+ /* skb_frags_not_readable() should indicate that ALL the
+ * frags in this skb are unreadable page_pool_iovs.
+ * We're checking for that flag above, but also check
+ * individual pages here. If the tcp stack is not
+ * setting skb->devmem correctly, we still don't want to
+ * crash here when accessing pgmap or priv below.
+ */
+ if (!skb_frag_page_pool_iov(frag)) {
+ net_err_ratelimited("Found non-devmem skb with page_pool_iov");
+ err = -ENODEV;
+ goto out;
+ }
+
+ ppiov = skb_frag_page_pool_iov(frag);
+ end = start + skb_frag_size(frag);
+ copy = end - offset;
+
+ if (copy > 0) {
+ copy = min(copy, remaining_len);
+
+ frag_offset = page_pool_iov_virtual_addr(ppiov) +
+ skb_frag_off(frag) + offset -
+ start;
+ cmsg_devmem.frag_offset = frag_offset;
+ cmsg_devmem.frag_size = copy;
+ err = xa_alloc((struct xarray *)&sk->sk_user_pages,
+ &user_token, frag->bv_page,
+ xa_limit_31b, GFP_KERNEL);
+ if (err)
+ goto out;
+
+ cmsg_devmem.frag_token = user_token;
+
+ offset += copy;
+ remaining_len -= copy;
+
+ err = put_cmsg(msg, SOL_SOCKET,
+ SO_DEVMEM_OFFSET,
+ sizeof(cmsg_devmem),
+ &cmsg_devmem);
+ if (err || msg->msg_flags & MSG_CTRUNC) {
+ msg->msg_flags &= ~MSG_CTRUNC;
+ xa_erase((struct xarray *)&sk->sk_user_pages,
+ user_token);
+ if (!err)
+ err = -ETOOSMALL;
+ goto out;
+ }
+
+ page_pool_iov_get_many(ppiov, 1);
+
+ sent += copy;
+
+ if (remaining_len == 0)
+ goto out;
+ }
+ start = end;
+ }
+
+ if (!remaining_len)
+ goto out;
+
+ /* if remaining_len is not satisfied yet, we need to go to the
+ * next frag in the frag_list to satisfy remaining_len.
+ */
+ skb = skb_shinfo(skb)->frag_list ?: skb->next;
+
+ offset = offset - start;
+ } while (skb);
+
+ if (remaining_len) {
+ err = -EFAULT;
+ goto out;
+ }
+
+out:
+ if (!sent)
+ sent = err;
+
+ return sent;
+}
+
/*
* This routine copies from a sock struct into the user buffer.
*
@@ -2314,6 +2463,7 @@ static int tcp_recvmsg_locked(struct sock *sk, struct msghdr *msg, size_t len,
int *cmsg_flags)
{
struct tcp_sock *tp = tcp_sk(sk);
+ int last_copied_devmem = -1; /* uninitialized */
int copied = 0;
u32 peek_seq;
u32 *seq;
@@ -2491,15 +2641,44 @@ static int tcp_recvmsg_locked(struct sock *sk, struct msghdr *msg, size_t len,
}
if (!(flags & MSG_TRUNC)) {
- err = skb_copy_datagram_msg(skb, offset, msg, used);
- if (err) {
- /* Exception. Bailout! */
- if (!copied)
- copied = -EFAULT;
+ if (last_copied_devmem != -1 &&
+ last_copied_devmem != skb->devmem)
break;
+
+ if (!skb->devmem) {
+ err = skb_copy_datagram_msg(skb, offset, msg,
+ used);
+ if (err) {
+ /* Exception. Bailout! */
+ if (!copied)
+ copied = -EFAULT;
+ break;
+ }
+ } else {
+ if (!(flags & MSG_SOCK_DEVMEM)) {
+ /* skb->devmem skbs can only be received
+ * with the MSG_SOCK_DEVMEM flag.
+ */
+ if (!copied)
+ copied = -EFAULT;
+
+ break;
+ }
+
+ err = tcp_recvmsg_devmem(sk, skb, offset, msg,
+ used);
+ if (err <= 0) {
+ if (!copied)
+ copied = -EFAULT;
+
+ break;
+ }
+ used = err;
}
}
+ last_copied_devmem = skb->devmem;
+
WRITE_ONCE(*seq, *seq + used);
copied += used;
len -= used;
diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
index 7583d4e34c8c..4cc8be892f05 100644
--- a/net/ipv4/tcp_ipv4.c
+++ b/net/ipv4/tcp_ipv4.c
@@ -2299,6 +2299,13 @@ static int tcp_v4_init_sock(struct sock *sk)
void tcp_v4_destroy_sock(struct sock *sk)
{
struct tcp_sock *tp = tcp_sk(sk);
+ struct page *page;
+ unsigned long index;
+
+ xa_for_each(&sk->sk_user_pages, index, page)
+ page_pool_page_put_many(page, 1);
+
+ xa_destroy(&sk->sk_user_pages);
trace_tcp_destroy_sock(sk);
--
2.42.0.869.gea05f2083d-goog
^ permalink raw reply related
* [RFC PATCH v3 11/12] net: add SO_DEVMEM_DONTNEED setsockopt to release RX pages
From: Mina Almasry @ 2023-11-06 2:44 UTC (permalink / raw)
To: netdev, linux-kernel, linux-arch, linux-kselftest, linux-media,
dri-devel, linaro-mm-sig
Cc: Mina Almasry, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Jesper Dangaard Brouer, Ilias Apalodimas,
Arnd Bergmann, David Ahern, Willem de Bruijn, Shuah Khan,
Sumit Semwal, Christian König, Shakeel Butt, Jeroen de Borst,
Praveen Kaligineedi, Willem de Bruijn, Kaiyuan Zhang
In-Reply-To: <20231106024413.2801438-1-almasrymina@google.com>
Add an interface for the user to notify the kernel that it is done
reading the NET_RX dmabuf pages returned as cmsg. The kernel will
drop the reference on the NET_RX pages to make them available for
re-use.
Signed-off-by: Willem de Bruijn <willemb@google.com>
Signed-off-by: Kaiyuan Zhang <kaiyuanz@google.com>
Signed-off-by: Mina Almasry <almasrymina@google.com>
---
include/uapi/asm-generic/socket.h | 1 +
include/uapi/linux/uio.h | 4 ++++
net/core/sock.c | 36 +++++++++++++++++++++++++++++++
3 files changed, 41 insertions(+)
diff --git a/include/uapi/asm-generic/socket.h b/include/uapi/asm-generic/socket.h
index aacb97f16b78..eb93b43394d4 100644
--- a/include/uapi/asm-generic/socket.h
+++ b/include/uapi/asm-generic/socket.h
@@ -135,6 +135,7 @@
#define SO_PASSPIDFD 76
#define SO_PEERPIDFD 77
+#define SO_DEVMEM_DONTNEED 97
#define SO_DEVMEM_HEADER 98
#define SCM_DEVMEM_HEADER SO_DEVMEM_HEADER
#define SO_DEVMEM_OFFSET 99
diff --git a/include/uapi/linux/uio.h b/include/uapi/linux/uio.h
index ae94763b1963..71314bf41590 100644
--- a/include/uapi/linux/uio.h
+++ b/include/uapi/linux/uio.h
@@ -26,6 +26,10 @@ struct cmsg_devmem {
__u32 frag_token;
};
+struct devmemtoken {
+ __u32 token_start;
+ __u32 token_count;
+};
/*
* UIO_MAXIOV shall be at least 16 1003.1g (5.4.1.1)
*/
diff --git a/net/core/sock.c b/net/core/sock.c
index 1d28e3e87970..4ddc6b11d915 100644
--- a/net/core/sock.c
+++ b/net/core/sock.c
@@ -1051,6 +1051,39 @@ static int sock_reserve_memory(struct sock *sk, int bytes)
return 0;
}
+static noinline_for_stack int
+sock_devmem_dontneed(struct sock *sk, sockptr_t optval, unsigned int optlen)
+{
+ struct devmemtoken tokens[128];
+ unsigned int num_tokens, i, j;
+ int ret;
+
+ if (sk->sk_type != SOCK_STREAM || sk->sk_protocol != IPPROTO_TCP)
+ return -EBADF;
+
+ if (optlen % sizeof(struct devmemtoken) || optlen > sizeof(tokens))
+ return -EINVAL;
+
+ num_tokens = optlen / sizeof(struct devmemtoken);
+ if (copy_from_sockptr(tokens, optval, optlen))
+ return -EFAULT;
+
+ ret = 0;
+ for (i = 0; i < num_tokens; i++) {
+ for (j = 0; j < tokens[i].token_count; j++) {
+ struct page *page = xa_erase(&sk->sk_user_pages,
+ tokens[i].token_start + j);
+
+ if (page) {
+ page_pool_page_put_many(page, 1);
+ ret++;
+ }
+ }
+ }
+
+ return ret;
+}
+
void sockopt_lock_sock(struct sock *sk)
{
/* When current->bpf_ctx is set, the setsockopt is called from
@@ -1538,6 +1571,9 @@ int sk_setsockopt(struct sock *sk, int level, int optname,
break;
}
+ case SO_DEVMEM_DONTNEED:
+ ret = sock_devmem_dontneed(sk, optval, optlen);
+ break;
default:
ret = -ENOPROTOOPT;
break;
--
2.42.0.869.gea05f2083d-goog
^ permalink raw reply related
* [RFC PATCH v3 12/12] selftests: add ncdevmem, netcat for devmem TCP
From: Mina Almasry @ 2023-11-06 2:44 UTC (permalink / raw)
To: netdev, linux-kernel, linux-arch, linux-kselftest, linux-media,
dri-devel, linaro-mm-sig
Cc: Mina Almasry, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Jesper Dangaard Brouer, Ilias Apalodimas,
Arnd Bergmann, David Ahern, Willem de Bruijn, Shuah Khan,
Sumit Semwal, Christian König, Shakeel Butt, Jeroen de Borst,
Praveen Kaligineedi, Stanislav Fomichev
In-Reply-To: <20231106024413.2801438-1-almasrymina@google.com>
ncdevmem is a devmem TCP netcat. It works similarly to netcat, but it
sends and receives data using the devmem TCP APIs. It uses udmabuf as
the dmabuf provider. It is compatible with a regular netcat running on
a peer, or a ncdevmem running on a peer.
In addition to normal netcat support, ncdevmem has a validation mode,
where it sends a specific pattern and validates this pattern on the
receiver side to ensure data integrity.
Suggested-by: Stanislav Fomichev <sdf@google.com>
Signed-off-by: Mina Almasry <almasrymina@google.com>
---
RFC v2:
- General cleanups (Willem).
---
tools/testing/selftests/net/.gitignore | 1 +
tools/testing/selftests/net/Makefile | 5 +
tools/testing/selftests/net/ncdevmem.c | 546 +++++++++++++++++++++++++
3 files changed, 552 insertions(+)
create mode 100644 tools/testing/selftests/net/ncdevmem.c
diff --git a/tools/testing/selftests/net/.gitignore b/tools/testing/selftests/net/.gitignore
index 2f9d378edec3..b644dbae58b7 100644
--- a/tools/testing/selftests/net/.gitignore
+++ b/tools/testing/selftests/net/.gitignore
@@ -17,6 +17,7 @@ ipv6_flowlabel
ipv6_flowlabel_mgr
log.txt
msg_zerocopy
+ncdevmem
nettest
psock_fanout
psock_snd
diff --git a/tools/testing/selftests/net/Makefile b/tools/testing/selftests/net/Makefile
index b9804ceb9494..6c6e53c70e99 100644
--- a/tools/testing/selftests/net/Makefile
+++ b/tools/testing/selftests/net/Makefile
@@ -5,6 +5,10 @@ CFLAGS = -Wall -Wl,--no-as-needed -O2 -g
CFLAGS += -I../../../../usr/include/ $(KHDR_INCLUDES)
# Additional include paths needed by kselftest.h
CFLAGS += -I../
+CFLAGS += -I../../../net/ynl/generated/
+CFLAGS += -I../../../net/ynl/lib/
+
+LDLIBS += ../../../net/ynl/lib/ynl.a ../../../net/ynl/generated/protos.a
TEST_PROGS := run_netsocktests run_afpackettests test_bpf.sh netdevice.sh \
rtnetlink.sh xfrm_policy.sh test_blackhole_dev.sh
@@ -91,6 +95,7 @@ TEST_PROGS += test_bridge_neigh_suppress.sh
TEST_PROGS += test_vxlan_nolocalbypass.sh
TEST_PROGS += test_bridge_backup_port.sh
TEST_PROGS += fdb_flush.sh
+TEST_GEN_FILES += ncdevmem
TEST_FILES := settings
diff --git a/tools/testing/selftests/net/ncdevmem.c b/tools/testing/selftests/net/ncdevmem.c
new file mode 100644
index 000000000000..78bc3ad767ca
--- /dev/null
+++ b/tools/testing/selftests/net/ncdevmem.c
@@ -0,0 +1,546 @@
+// SPDX-License-Identifier: GPL-2.0
+#define _GNU_SOURCE
+#define __EXPORTED_HEADERS__
+
+#include <linux/uio.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <unistd.h>
+#include <stdbool.h>
+#include <string.h>
+#include <errno.h>
+#define __iovec_defined
+#include <fcntl.h>
+#include <malloc.h>
+
+#include <arpa/inet.h>
+#include <sys/socket.h>
+#include <sys/mman.h>
+#include <sys/ioctl.h>
+#include <sys/syscall.h>
+
+#include <linux/memfd.h>
+#include <linux/if.h>
+#include <linux/dma-buf.h>
+#include <linux/udmabuf.h>
+#include <libmnl/libmnl.h>
+#include <linux/types.h>
+#include <linux/netlink.h>
+#include <linux/genetlink.h>
+#include <linux/netdev.h>
+#include <time.h>
+
+#include "netdev-user.h"
+#include <ynl.h>
+
+#define PAGE_SHIFT 12
+#define TEST_PREFIX "ncdevmem"
+#define NUM_PAGES 16000
+
+#ifndef MSG_SOCK_DEVMEM
+#define MSG_SOCK_DEVMEM 0x2000000
+#endif
+
+/*
+ * tcpdevmem netcat. Works similarly to netcat but does device memory TCP
+ * instead of regular TCP. Uses udmabuf to mock a dmabuf provider.
+ *
+ * Usage:
+ *
+ * * Without validation:
+ *
+ * On server:
+ * ncdevmem -s <server IP> -c <client IP> -f eth1 -n 0000:06:00.0 -l \
+ * -p 5201
+ *
+ * On client:
+ * ncdevmem -s <server IP> -c <client IP> -f eth1 -n 0000:06:00.0 -p 5201
+ *
+ * * With Validation:
+ * On server:
+ * ncdevmem -s <server IP> -c <client IP> -l -f eth1 -n 0000:06:00.0 \
+ * -p 5202 -v 1
+ *
+ * On client:
+ * ncdevmem -s <server IP> -c <client IP> -f eth1 -n 0000:06:00.0 -p 5202 \
+ * -v 100000
+ *
+ * Note this is compatible with regular netcat. i.e. the sender or receiver can
+ * be replaced with regular netcat to test the RX or TX path in isolation.
+ */
+
+static char *server_ip = "192.168.1.4";
+static char *client_ip = "192.168.1.2";
+static char *port = "5201";
+static size_t do_validation;
+static int queue_num = 15;
+static char *ifname = "eth1";
+static char *nic_pci_addr = "0000:06:00.0";
+static unsigned int iterations;
+
+void print_bytes(void *ptr, size_t size)
+{
+ unsigned char *p = ptr;
+ int i;
+
+ for (i = 0; i < size; i++) {
+ printf("%02hhX ", p[i]);
+ }
+ printf("\n");
+}
+
+void print_nonzero_bytes(void *ptr, size_t size)
+{
+ unsigned char *p = ptr;
+ unsigned int i;
+
+ for (i = 0; i < size; i++)
+ putchar(p[i]);
+ printf("\n");
+}
+
+void validate_buffer(void *line, size_t size)
+{
+ static unsigned char seed = 1;
+ unsigned char *ptr = line;
+ int errors = 0;
+ size_t i;
+
+ for (i = 0; i < size; i++) {
+ if (ptr[i] != seed) {
+ fprintf(stderr,
+ "Failed validation: expected=%u, actual=%u, index=%lu\n",
+ seed, ptr[i], i);
+ errors++;
+ if (errors > 20)
+ exit(1);
+ }
+ seed++;
+ if (seed == do_validation)
+ seed = 0;
+ }
+
+ fprintf(stdout, "Validated buffer\n");
+}
+
+static void reset_flow_steering(void)
+{
+ char command[256];
+
+ memset(command, 0, sizeof(command));
+ snprintf(command, sizeof(command), "sudo ethtool -K %s ntuple off",
+ "eth1");
+ system(command);
+
+ memset(command, 0, sizeof(command));
+ snprintf(command, sizeof(command), "sudo ethtool -K %s ntuple on",
+ "eth1");
+ system(command);
+}
+
+static void configure_flow_steering(void)
+{
+ char command[256];
+
+ memset(command, 0, sizeof(command));
+ snprintf(command, sizeof(command),
+ "sudo ethtool -N %s flow-type tcp4 src-ip %s dst-ip %s src-port %s dst-port %s queue %d",
+ ifname, client_ip, server_ip, port, port, queue_num);
+ system(command);
+}
+
+/* Triggers a driver reset...
+ *
+ * The proper way to do this is probably 'ethtool --reset', but I don't have
+ * that supported on my current test bed. I resort to changing this
+ * configuration in the driver which also causes a driver reset...
+ */
+static void trigger_device_reset(void)
+{
+ char command[256];
+
+ memset(command, 0, sizeof(command));
+ snprintf(command, sizeof(command),
+ "sudo ethtool --set-priv-flags %s enable-header-split off",
+ ifname);
+ system(command);
+
+ memset(command, 0, sizeof(command));
+ snprintf(command, sizeof(command),
+ "sudo ethtool --set-priv-flags %s enable-header-split on",
+ ifname);
+ system(command);
+}
+
+static int bind_rx_queue(unsigned int ifindex, unsigned int dmabuf_fd,
+ __u32 *queue_idx, unsigned int n_queue_index,
+ struct ynl_sock **ys)
+{
+ struct netdev_bind_rx_req *req = NULL;
+ struct ynl_error yerr;
+ int ret = 0;
+
+ *ys = ynl_sock_create(&ynl_netdev_family, &yerr);
+ if (!*ys) {
+ fprintf(stderr, "YNL: %s\n", yerr.msg);
+ return -1;
+ }
+
+ if (ynl_subscribe(*ys, "mgmt"))
+ goto err_close;
+
+ req = netdev_bind_rx_req_alloc();
+ netdev_bind_rx_req_set_ifindex(req, ifindex);
+ netdev_bind_rx_req_set_dmabuf_fd(req, dmabuf_fd);
+ __netdev_bind_rx_req_set_queues(req, queue_idx, n_queue_index);
+
+ ret = netdev_bind_rx(*ys, req);
+ if (!ret) {
+ perror("netdev_bind_rx");
+ goto err_close;
+ }
+
+ netdev_bind_rx_req_free(req);
+
+ return 0;
+
+err_close:
+ fprintf(stderr, "YNL failed: %s\n", (*ys)->err.msg);
+ netdev_bind_rx_req_free(req);
+ ynl_sock_destroy(*ys);
+ return -1;
+}
+
+static void create_udmabuf(int *devfd, int *memfd, int *buf, size_t dmabuf_size)
+{
+ struct udmabuf_create create;
+ int ret;
+
+ *devfd = open("/dev/udmabuf", O_RDWR);
+ if (*devfd < 0) {
+ fprintf(stderr,
+ "%s: [skip,no-udmabuf: Unable to access DMA "
+ "buffer device file]\n",
+ TEST_PREFIX);
+ exit(70);
+ }
+
+ *memfd = memfd_create("udmabuf-test", MFD_ALLOW_SEALING);
+ if (*memfd < 0) {
+ printf("%s: [skip,no-memfd]\n", TEST_PREFIX);
+ exit(72);
+ }
+
+ ret = fcntl(*memfd, F_ADD_SEALS, F_SEAL_SHRINK);
+ if (ret < 0) {
+ printf("%s: [skip,fcntl-add-seals]\n", TEST_PREFIX);
+ exit(73);
+ }
+
+ ret = ftruncate(*memfd, dmabuf_size);
+ if (ret == -1) {
+ printf("%s: [FAIL,memfd-truncate]\n", TEST_PREFIX);
+ exit(74);
+ }
+
+ memset(&create, 0, sizeof(create));
+
+ create.memfd = *memfd;
+ create.offset = 0;
+ create.size = dmabuf_size;
+ *buf = ioctl(*devfd, UDMABUF_CREATE, &create);
+ if (*buf < 0) {
+ printf("%s: [FAIL, create udmabuf]\n", TEST_PREFIX);
+ exit(75);
+ }
+}
+
+int do_server(void)
+{
+ char ctrl_data[sizeof(int) * 20000];
+ size_t non_page_aligned_frags = 0;
+ struct sockaddr_in client_addr;
+ struct sockaddr_in server_sin;
+ size_t page_aligned_frags = 0;
+ int devfd, memfd, buf, ret;
+ size_t total_received = 0;
+ bool is_devmem = false;
+ char *buf_mem = NULL;
+ struct ynl_sock *ys;
+ size_t dmabuf_size;
+ char iobuf[819200];
+ char buffer[256];
+ int socket_fd;
+ int client_fd;
+ size_t i = 0;
+ int opt = 1;
+
+ dmabuf_size = getpagesize() * NUM_PAGES;
+
+ create_udmabuf(&devfd, &memfd, &buf, dmabuf_size);
+
+ __u32 *queue_idx = malloc(sizeof(__u32) * 2);
+
+ queue_idx[0] = 14;
+ queue_idx[1] = 15;
+ if (bind_rx_queue(3 /* index for eth1 */, buf, queue_idx, 2, &ys)) {
+ fprintf(stderr, "Failed to bind\n");
+ exit(1);
+ }
+
+ buf_mem = mmap(NULL, dmabuf_size, PROT_READ | PROT_WRITE, MAP_SHARED,
+ buf, 0);
+ if (buf_mem == MAP_FAILED) {
+ perror("mmap()");
+ exit(1);
+ }
+
+ /* Need to trigger the NIC to reallocate its RX pages, otherwise the
+ * bind doesn't take effect.
+ */
+ trigger_device_reset();
+
+ sleep(1);
+
+ reset_flow_steering();
+ configure_flow_steering();
+
+ server_sin.sin_family = AF_INET;
+ server_sin.sin_port = htons(atoi(port));
+
+ ret = inet_pton(server_sin.sin_family, server_ip, &server_sin.sin_addr);
+ if (socket < 0) {
+ printf("%s: [FAIL, create socket]\n", TEST_PREFIX);
+ exit(79);
+ }
+
+ socket_fd = socket(server_sin.sin_family, SOCK_STREAM, 0);
+ if (socket < 0) {
+ printf("%s: [FAIL, create socket]\n", TEST_PREFIX);
+ exit(76);
+ }
+
+ ret = setsockopt(socket_fd, SOL_SOCKET, SO_REUSEPORT, &opt,
+ sizeof(opt));
+ if (ret) {
+ printf("%s: [FAIL, set sock opt]: %s\n", TEST_PREFIX,
+ strerror(errno));
+ exit(76);
+ }
+ ret = setsockopt(socket_fd, SOL_SOCKET, SO_REUSEADDR, &opt,
+ sizeof(opt));
+ if (ret) {
+ printf("%s: [FAIL, set sock opt]: %s\n", TEST_PREFIX,
+ strerror(errno));
+ exit(76);
+ }
+ ret = setsockopt(socket_fd, SOL_SOCKET, SO_ZEROCOPY, &opt,
+ sizeof(opt));
+ if (ret) {
+ printf("%s: [FAIL, set sock opt]: %s\n", TEST_PREFIX,
+ strerror(errno));
+ exit(76);
+ }
+
+ printf("binding to address %s:%d\n", server_ip,
+ ntohs(server_sin.sin_port));
+
+ ret = bind(socket_fd, &server_sin, sizeof(server_sin));
+ if (ret) {
+ printf("%s: [FAIL, bind]: %s\n", TEST_PREFIX, strerror(errno));
+ exit(76);
+ }
+
+ ret = listen(socket_fd, 1);
+ if (ret) {
+ printf("%s: [FAIL, listen]: %s\n", TEST_PREFIX,
+ strerror(errno));
+ exit(76);
+ }
+
+ socklen_t client_addr_len = sizeof(client_addr);
+
+ inet_ntop(server_sin.sin_family, &server_sin.sin_addr, buffer,
+ sizeof(buffer));
+ printf("Waiting or connection on %s:%d\n", buffer,
+ ntohs(server_sin.sin_port));
+ client_fd = accept(socket_fd, &client_addr, &client_addr_len);
+
+ inet_ntop(client_addr.sin_family, &client_addr.sin_addr, buffer,
+ sizeof(buffer));
+ printf("Got connection from %s:%d\n", buffer,
+ ntohs(client_addr.sin_port));
+
+ while (1) {
+ struct iovec iov = { .iov_base = iobuf,
+ .iov_len = sizeof(iobuf) };
+ struct cmsg_devmem *cmsg_devmem = NULL;
+ struct dma_buf_sync sync = { 0 };
+ struct cmsghdr *cm = NULL;
+ struct msghdr msg = { 0 };
+ struct devmemtoken token;
+ ssize_t ret;
+
+ is_devmem = false;
+ printf("\n\n");
+
+ msg.msg_iov = &iov;
+ msg.msg_iovlen = 1;
+ msg.msg_control = ctrl_data;
+ msg.msg_controllen = sizeof(ctrl_data);
+ ret = recvmsg(client_fd, &msg, MSG_SOCK_DEVMEM);
+ printf("recvmsg ret=%ld\n", ret);
+ if (ret < 0 && (errno == EAGAIN || errno == EWOULDBLOCK)) {
+ continue;
+ }
+ if (ret < 0) {
+ perror("recvmsg");
+ continue;
+ }
+ if (ret == 0) {
+ printf("client exited\n");
+ goto cleanup;
+ }
+
+ i++;
+ for (cm = CMSG_FIRSTHDR(&msg); cm; cm = CMSG_NXTHDR(&msg, cm)) {
+ if (cm->cmsg_level != SOL_SOCKET ||
+ (cm->cmsg_type != SCM_DEVMEM_OFFSET &&
+ cm->cmsg_type != SCM_DEVMEM_HEADER)) {
+ fprintf(stdout, "skipping non-devmem cmsg\n");
+ continue;
+ }
+
+ cmsg_devmem = (struct cmsg_devmem *)CMSG_DATA(cm);
+ is_devmem = true;
+
+ if (cm->cmsg_type == SCM_DEVMEM_HEADER) {
+ /* TODO: process data copied from skb's linear
+ * buffer.
+ */
+ fprintf(stdout,
+ "SCM_DEVMEM_HEADER. "
+ "cmsg_devmem->frag_size=%u\n",
+ cmsg_devmem->frag_size);
+
+ continue;
+ }
+
+ token.token_start = cmsg_devmem->frag_token;
+ token.token_count = 1;
+
+ total_received += cmsg_devmem->frag_size;
+ printf("received frag_page=%llu, in_page_offset=%llu,"
+ " frag_offset=%llu, frag_size=%u, token=%u"
+ " total_received=%lu\n",
+ cmsg_devmem->frag_offset >> PAGE_SHIFT,
+ cmsg_devmem->frag_offset % getpagesize(),
+ cmsg_devmem->frag_offset, cmsg_devmem->frag_size,
+ cmsg_devmem->frag_token, total_received);
+
+ if (cmsg_devmem->frag_size % getpagesize())
+ non_page_aligned_frags++;
+ else
+ page_aligned_frags++;
+
+ sync.flags = DMA_BUF_SYNC_READ | DMA_BUF_SYNC_START;
+ ioctl(buf, DMA_BUF_IOCTL_SYNC, &sync);
+
+ if (do_validation)
+ validate_buffer(
+ ((unsigned char *)buf_mem) +
+ cmsg_devmem->frag_offset,
+ cmsg_devmem->frag_size);
+ else
+ print_nonzero_bytes(
+ ((unsigned char *)buf_mem) +
+ cmsg_devmem->frag_offset,
+ cmsg_devmem->frag_size);
+
+ sync.flags = DMA_BUF_SYNC_READ | DMA_BUF_SYNC_END;
+ ioctl(buf, DMA_BUF_IOCTL_SYNC, &sync);
+
+ ret = setsockopt(client_fd, SOL_SOCKET,
+ SO_DEVMEM_DONTNEED, &token,
+ sizeof(token));
+ if (ret != 1) {
+ perror("SO_DEVMEM_DONTNEED not enough tokens");
+ exit(1);
+ }
+ }
+ if (!is_devmem)
+ printf("flow steering error\n");
+
+ printf("total_received=%lu\n", total_received);
+ }
+
+ fprintf(stdout, "%s: ok\n", TEST_PREFIX);
+
+ fprintf(stdout, "page_aligned_frags=%lu, non_page_aligned_frags=%lu\n",
+ page_aligned_frags, non_page_aligned_frags);
+
+ fprintf(stdout, "page_aligned_frags=%lu, non_page_aligned_frags=%lu\n",
+ page_aligned_frags, non_page_aligned_frags);
+
+cleanup:
+
+ munmap(buf_mem, dmabuf_size);
+ close(client_fd);
+ close(socket_fd);
+ close(buf);
+ close(memfd);
+ close(devfd);
+ ynl_sock_destroy(ys);
+ trigger_device_reset();
+
+ return 0;
+}
+
+int main(int argc, char *argv[])
+{
+ int is_server = 0, opt;
+
+ while ((opt = getopt(argc, argv, "ls:c:p:v:q:f:n:i:")) != -1) {
+ switch (opt) {
+ case 'l':
+ is_server = 1;
+ break;
+ case 's':
+ server_ip = optarg;
+ break;
+ case 'c':
+ client_ip = optarg;
+ break;
+ case 'p':
+ port = optarg;
+ break;
+ case 'v':
+ do_validation = atoll(optarg);
+ break;
+ case 'q':
+ queue_num = atoi(optarg);
+ break;
+ case 'f':
+ ifname = optarg;
+ break;
+ case 'n':
+ nic_pci_addr = optarg;
+ break;
+ case 'i':
+ iterations = atoll(optarg);
+ break;
+ case '?':
+ printf("unknown option: %c\n", optopt);
+ break;
+ }
+ }
+
+ for (; optind < argc; optind++) {
+ printf("extra arguments: %s\n", argv[optind]);
+ }
+
+ if (is_server)
+ return do_server();
+
+ return 0;
+}
--
2.42.0.869.gea05f2083d-goog
^ permalink raw reply related
* Re: Bypass qdiscs?
From: Stephen Hemminger @ 2023-11-06 3:23 UTC (permalink / raw)
To: John Ousterhout; +Cc: Andrew Lunn, netdev
In-Reply-To: <CAGXJAmzn0vFtkVT=JQLQuZm6ae+Ms_nOcvebKPC6ARWfM9DwOw@mail.gmail.com>
On Sat, 4 Nov 2023 19:47:30 -0700
John Ousterhout <ouster@cs.stanford.edu> wrote:
> I haven't tried creating a "pass through" qdisc, but that seems like a
> reasonable approach if (as it seems) there isn't something already
> built-in that provides equivalent functionality.
>
> -John-
>
> P.S. If hardware starts supporting Homa, I hope that it will be
> possible to move the entire transport to the NIC, so that applications
> can bypass the kernel entirely, as with RDMA.
One old trick was setting netdev queue length to 0 to avoid qdisc.
^ permalink raw reply
* Re: [PATCH v3] tg3: Fix the TX ring stall
From: Michael Chan @ 2023-11-06 3:50 UTC (permalink / raw)
To: alexey.pakhunov
Cc: mchan, vincent.wong2, netdev, linux-kernel, siva.kallam, prashant
In-Reply-To: <20231105185828.287004-1-alexey.pakhunov@spacex.com>
[-- Attachment #1: Type: text/plain, Size: 2371 bytes --]
On Sun, Nov 5, 2023 at 10:58 AM <alexey.pakhunov@spacex.com> wrote:
>
> From: Alex Pakhunov <alexey.pakhunov@spacex.com>
>
> The TX ring maintained by the tg3 driver can end up in the state, when it
> has packets queued for sending but the NIC hardware is not informed, so no
> progress is made. This leads to a multi-second interruption in network
> traffic followed by dev_watchdog() firing and resetting the queue.
>
> The specific sequence of steps is:
>
> 1. tg3_start_xmit() is called at least once and queues packet(s) without
> updating tnapi->prodmbox (netdev_xmit_more() returns true)
> 2. tg3_start_xmit() is called with an SKB which causes tg3_tso_bug() to be
> called.
> 3. tg3_tso_bug() determines that the SKB is too large, ...
>
> if (unlikely(tg3_tx_avail(tnapi) <= frag_cnt_est)) {
>
> ... stops the queue, and returns NETDEV_TX_BUSY:
>
> netif_tx_stop_queue(txq);
> ...
> if (tg3_tx_avail(tnapi) <= frag_cnt_est)
> return NETDEV_TX_BUSY;
>
> 4. Since all tg3_tso_bug() call sites directly return, the code updating
> tnapi->prodmbox is skipped.
>
> 5. The queue is stuck now. tg3_start_xmit() is not called while the queue
> is stopped. The NIC is not processing new packets because
> tnapi->prodmbox wasn't updated. tg3_tx() is not called by
> tg3_poll_work() because the all TX descriptions that could be freed has
> been freed:
>
> /* run TX completion thread */
> if (tnapi->hw_status->idx[0].tx_consumer != tnapi->tx_cons) {
> tg3_tx(tnapi);
>
> 6. Eventually, dev_watchdog() fires triggering a reset of the queue.
>
> This fix makes sure that the tnapi->prodmbox update happens regardless of
> the reason tg3_start_xmit() returned.
>
> Signed-off-by: Alex Pakhunov <alexey.pakhunov@spacex.com>
> Signed-off-by: Vincent Wong <vincent.wong2@spacex.com>
> ---
> v3: Split "Fix the TX ring stall" into a standalone patch. No code changes from v2.
> v2: https://lore.kernel.org/netdev/CACKFLi=ZLAb1Y92LwvqjOGPCuinka7qbHwDP2pkG4-_a7DMorQ@mail.gmail.com/T/#t
> - Sort the local variables in tg3_start_xmit() in the RCS order
> v1: https://lore.kernel.org/netdev/20231101191858.2611154-1-alexey.pakhunov@spacex.com/T/#t
> ---
Thanks.
Reviewed-by: Michael Chan <michael.chan@broadcom.com>
[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4209 bytes --]
^ permalink raw reply
* Re: [PATCH v2 1/2] tg3: Increment tx_dropped in tg3_tso_bug()
From: Michael Chan @ 2023-11-06 3:59 UTC (permalink / raw)
To: Alex Pakhunov
Cc: linux-kernel, mchan, netdev, prashant, siva.kallam, vincent.wong2
In-Reply-To: <20231105192643.318753-1-alexey.pakhunov@spacex.com>
[-- Attachment #1: Type: text/plain, Size: 1068 bytes --]
On Sun, Nov 5, 2023 at 11:26 AM Alex Pakhunov
<alexey.pakhunov@spacex.com> wrote:
>
> > I recommend using per queue counters as briefly mentioned in my
> > earlier reply...
> > tg3_get_stats64() can just loop and sum all the tx_dropped and
> > rx_dropped counters in each tg3_napi struct. We don't worry about
> > locks here since we are just reading.
>
> Got it. So the core idea is to make sure there is a single writer for each
> counter which will make updating the counter race-free. It does not keep
> reading the counters from multiple queues completely race free, but, I
> guess, the assumption is that computing the aggregate counter to be
> slightly wrong is acceptable - it will be recomputed correctly next time.
Correct.
>
> There is still some gotchas on 32 bit machines though. 64 bit reads are not
> atomic there, so we have to make the counters 32bit to compensate:
These counters are currently defined as unsigned long which is 32-bit
on 32-bit CPUs and 64-bit on 64-bit CPUs. We can just keep them
unchanged. Thanks.
[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4209 bytes --]
^ permalink raw reply
* Re: [RFC v1 0/8] vhost-vdpa: add support for iommufd
From: Jason Wang @ 2023-11-06 4:11 UTC (permalink / raw)
To: Cindy Lu; +Cc: mst, yi.l.liu, jgg, linux-kernel, virtualization, netdev
In-Reply-To: <20231103171641.1703146-1-lulu@redhat.com>
On Sat, Nov 4, 2023 at 1:16 AM Cindy Lu <lulu@redhat.com> wrote:
>
>
> Hi All
> This code provides the iommufd support for vdpa device
> This code fixes the bugs from the last version and also add the asid support. rebase on kernel
> v6,6-rc3
> Test passed in the physical device (vp_vdpa), but there are still some problems in the emulated device (vdpa_sim_net),
> I will continue working on it
>
> The kernel code is
> https://gitlab.com/lulu6/vhost/-/tree/iommufdRFC_v1
>
> Signed-off-by: Cindy Lu <lulu@redhat.com>
It would be better to have a change summary here.
Thanks
>
>
> Cindy Lu (8):
> vhost/iommufd: Add the functions support iommufd
> Kconfig: Add the new file vhost/iommufd
> vhost: Add 3 new uapi to support iommufd
> vdpa: Add new vdpa_config_ops to support iommufd
> vdpa_sim :Add support for iommufd
> vdpa: change the map/unmap process to support iommufd
> vp_vdpa::Add support for iommufd
> iommu: expose the function iommu_device_use_default_domain
>
> drivers/iommu/iommu.c | 2 +
> drivers/vdpa/vdpa_sim/vdpa_sim.c | 8 ++
> drivers/vdpa/virtio_pci/vp_vdpa.c | 4 +
> drivers/vhost/Kconfig | 1 +
> drivers/vhost/Makefile | 1 +
> drivers/vhost/iommufd.c | 178 +++++++++++++++++++++++++
> drivers/vhost/vdpa.c | 210 +++++++++++++++++++++++++++++-
> drivers/vhost/vhost.h | 21 +++
> include/linux/vdpa.h | 38 +++++-
> include/uapi/linux/vhost.h | 66 ++++++++++
> 10 files changed, 525 insertions(+), 4 deletions(-)
> create mode 100644 drivers/vhost/iommufd.c
>
> --
> 2.34.3
>
^ permalink raw reply
* Re: Bypass qdiscs?
From: David Ahern @ 2023-11-06 4:27 UTC (permalink / raw)
To: Stephen Hemminger, John Ousterhout; +Cc: Andrew Lunn, netdev
In-Reply-To: <20231105192309.20416ff8@hermes.local>
On 11/5/23 8:23 PM, Stephen Hemminger wrote:
> On Sat, 4 Nov 2023 19:47:30 -0700
> John Ousterhout <ouster@cs.stanford.edu> wrote:
>
>> I haven't tried creating a "pass through" qdisc, but that seems like a
>> reasonable approach if (as it seems) there isn't something already
>> built-in that provides equivalent functionality.
>>
>> -John-
>>
>> P.S. If hardware starts supporting Homa, I hope that it will be
>> possible to move the entire transport to the NIC, so that applications
>> can bypass the kernel entirely, as with RDMA.
>
> One old trick was setting netdev queue length to 0 to avoid qdisc.
>
tc qdisc replace dev <name> root noqueue
should work
^ permalink raw reply
* Re: [PATCH v9 10/17] bpf,lsm: refactor bpf_map_alloc/bpf_map_free LSM hooks
From: Paul Moore @ 2023-11-06 5:01 UTC (permalink / raw)
To: Andrii Nakryiko, bpf, netdev, brauner
Cc: linux-fsdevel, linux-security-module, keescook, kernel-team,
sargun
In-Reply-To: <20231103190523.6353-11-andrii@kernel.org>
On Nov 3, 2023 Andrii Nakryiko <andrii@kernel.org> wrote:
>
> Similarly to bpf_prog_alloc LSM hook, rename and extend bpf_map_alloc
> hook into bpf_map_create, taking not just struct bpf_map, but also
> bpf_attr and bpf_token, to give a fuller context to LSMs.
>
> Unlike bpf_prog_alloc, there is no need to move the hook around, as it
> currently is firing right before allocating BPF map ID and FD, which
> seems to be a sweet spot.
>
> But like bpf_prog_alloc/bpf_prog_free combo, make sure that bpf_map_free
> LSM hook is called even if bpf_map_create hook returned error, as if few
> LSMs are combined together it could be that one LSM successfully
> allocated security blob for its needs, while subsequent LSM rejected BPF
> map creation. The former LSM would still need to free up LSM blob, so we
> need to ensure security_bpf_map_free() is called regardless of the
> outcome.
>
> Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
> ---
> include/linux/lsm_hook_defs.h | 5 +++--
> include/linux/security.h | 6 ++++--
> kernel/bpf/bpf_lsm.c | 6 +++---
> kernel/bpf/syscall.c | 4 ++--
> security/security.c | 16 ++++++++++------
> security/selinux/hooks.c | 7 ++++---
> 6 files changed, 26 insertions(+), 18 deletions(-)
Acked-by: Paul Moore <paul@paul-moore.com>
--
paul-moore.com
^ permalink raw reply
* Re: [PATCH v9 9/17] bpf,lsm: refactor bpf_prog_alloc/bpf_prog_free LSM hooks
From: Paul Moore @ 2023-11-06 5:01 UTC (permalink / raw)
To: Andrii Nakryiko, bpf, netdev, brauner
Cc: linux-fsdevel, linux-security-module, keescook, kernel-team,
sargun
In-Reply-To: <20231103190523.6353-10-andrii@kernel.org>
On Nov 3, 2023 Andrii Nakryiko <andrii@kernel.org> wrote:
>
> Based on upstream discussion ([0]), rework existing
> bpf_prog_alloc_security LSM hook. Rename it to bpf_prog_load and instead
> of passing bpf_prog_aux, pass proper bpf_prog pointer for a full BPF
> program struct. Also, we pass bpf_attr union with all the user-provided
> arguments for BPF_PROG_LOAD command. This will give LSMs as much
> information as we can basically provide.
>
> The hook is also BPF token-aware now, and optional bpf_token struct is
> passed as a third argument. bpf_prog_load LSM hook is called after
> a bunch of sanity checks were performed, bpf_prog and bpf_prog_aux were
> allocated and filled out, but right before performing full-fledged BPF
> verification step.
>
> bpf_prog_free LSM hook is now accepting struct bpf_prog argument, for
> consistency. SELinux code is adjusted to all new names, types, and
> signatures.
>
> Note, given that bpf_prog_load (previously bpf_prog_alloc) hook can be
> used by some LSMs to allocate extra security blob, but also by other
> LSMs to reject BPF program loading, we need to make sure that
> bpf_prog_free LSM hook is called after bpf_prog_load/bpf_prog_alloc one
> *even* if the hook itself returned error. If we don't do that, we run
> the risk of leaking memory. This seems to be possible today when
> combining SELinux and BPF LSM, as one example, depending on their
> relative ordering.
>
> Also, for BPF LSM setup, add bpf_prog_load and bpf_prog_free to
> sleepable LSM hooks list, as they are both executed in sleepable
> context. Also drop bpf_prog_load hook from untrusted, as there is no
> issue with refcount or anything else anymore, that originally forced us
> to add it to untrusted list in c0c852dd1876 ("bpf: Do not mark certain LSM
> hook arguments as trusted"). We now trigger this hook much later and it
> should not be an issue anymore.
See my comment below, but it isn't clear to me if this means it is okay
to have `BTF_ID(func, bpf_lsm_bpf_prog_free)` called twice. It probably
would be a good idea to get KP, BPF LSM maintainer, to review this change
as well to make sure this looks good to him.
> [0] https://lore.kernel.org/bpf/9fe88aef7deabbe87d3fc38c4aea3c69.paul@paul-moore.com/
>
> Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
> ---
> include/linux/lsm_hook_defs.h | 5 +++--
> include/linux/security.h | 12 +++++++-----
> kernel/bpf/bpf_lsm.c | 5 +++--
> kernel/bpf/syscall.c | 25 +++++++++++++------------
> security/security.c | 25 +++++++++++++++----------
> security/selinux/hooks.c | 15 ++++++++-------
> 6 files changed, 49 insertions(+), 38 deletions(-)
...
> diff --git a/kernel/bpf/bpf_lsm.c b/kernel/bpf/bpf_lsm.c
> index e14c822f8911..3e956f6302f3 100644
> --- a/kernel/bpf/bpf_lsm.c
> +++ b/kernel/bpf/bpf_lsm.c
> @@ -263,6 +263,8 @@ BTF_ID(func, bpf_lsm_bpf_map)
> BTF_ID(func, bpf_lsm_bpf_map_alloc_security)
> BTF_ID(func, bpf_lsm_bpf_map_free_security)
> BTF_ID(func, bpf_lsm_bpf_prog)
> +BTF_ID(func, bpf_lsm_bpf_prog_load)
> +BTF_ID(func, bpf_lsm_bpf_prog_free)
> BTF_ID(func, bpf_lsm_bprm_check_security)
> BTF_ID(func, bpf_lsm_bprm_committed_creds)
> BTF_ID(func, bpf_lsm_bprm_committing_creds)
> @@ -346,8 +348,7 @@ BTF_SET_END(sleepable_lsm_hooks)
>
> BTF_SET_START(untrusted_lsm_hooks)
> BTF_ID(func, bpf_lsm_bpf_map_free_security)
> -BTF_ID(func, bpf_lsm_bpf_prog_alloc_security)
> -BTF_ID(func, bpf_lsm_bpf_prog_free_security)
> +BTF_ID(func, bpf_lsm_bpf_prog_free)
> BTF_ID(func, bpf_lsm_file_alloc_security)
> BTF_ID(func, bpf_lsm_file_free_security)
> #ifdef CONFIG_SECURITY_NETWORK
It looks like you're calling the BTF_ID() macro on bpf_lsm_bpf_prog_free
twice? I would have expected a only one macro call for each bpf_prog_load
and bpf_prog_free, is that a bad assuption?
> diff --git a/security/security.c b/security/security.c
> index dcb3e7014f9b..5773d446210e 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -5180,16 +5180,21 @@ int security_bpf_map_alloc(struct bpf_map *map)
> }
>
> /**
> - * security_bpf_prog_alloc() - Allocate a bpf program LSM blob
> - * @aux: bpf program aux info struct
> + * security_bpf_prog_load() - Check if loading of BPF program is allowed
> + * @prog: BPF program object
> + * @attr: BPF syscall attributes used to create BPF program
> + * @token: BPF token used to grant user access to BPF subsystem
> *
> - * Initialize the security field inside bpf program.
> + * Do a check when the kernel allocates BPF program object and is about to
> + * pass it to BPF verifier for additional correctness checks. This is also the
> + * point where LSM blob is allocated for LSMs that need them.
This is pretty nitpicky, but I'm guessing you may need to make another
revision to this patchset, if you do please drop the BPF verifier remark
from the comment above.
Example: "Perform an access control check when the kernel loads a BPF
program and allocates the associated BPF program object. This hook is
also responsibile for allocating any required LSM state for the BPF
program."
> * Return: Returns 0 on success, error on failure.
> */
> -int security_bpf_prog_alloc(struct bpf_prog_aux *aux)
> +int security_bpf_prog_load(struct bpf_prog *prog, union bpf_attr *attr,
> + struct bpf_token *token)
> {
> - return call_int_hook(bpf_prog_alloc_security, 0, aux);
> + return call_int_hook(bpf_prog_load, 0, prog, attr, token);
> }
--
paul-moore.com
^ permalink raw reply
* Re: [PATCH v9 11/17] bpf,lsm: add BPF token LSM hooks
From: Paul Moore @ 2023-11-06 5:01 UTC (permalink / raw)
To: Andrii Nakryiko, bpf, netdev, brauner
Cc: linux-fsdevel, linux-security-module, keescook, kernel-team,
sargun
In-Reply-To: <20231103190523.6353-12-andrii@kernel.org>
On Nov 3, 2023 Andrii Nakryiko <andrii@kernel.org> wrote:
>
> Wire up bpf_token_create and bpf_token_free LSM hooks, which allow to
> allocate LSM security blob (we add `void *security` field to struct
> bpf_token for that), but also control who can instantiate BPF token.
> This follows existing pattern for BPF map and BPF prog.
>
> Also add security_bpf_token_allow_cmd() and security_bpf_token_capable()
> LSM hooks that allow LSM implementation to control and negate (if
> necessary) BPF token's delegation of a specific bpf_cmd and capability,
> respectively.
>
> Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
> ---
> include/linux/bpf.h | 3 ++
> include/linux/lsm_hook_defs.h | 5 +++
> include/linux/security.h | 25 +++++++++++++++
> kernel/bpf/bpf_lsm.c | 4 +++
> kernel/bpf/token.c | 13 ++++++--
> security/security.c | 60 +++++++++++++++++++++++++++++++++++
> 6 files changed, 107 insertions(+), 3 deletions(-)
...
> diff --git a/include/linux/security.h b/include/linux/security.h
> index 08fd777cbe94..1d6edbf45d1c 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -60,6 +60,7 @@ struct fs_parameter;
> enum fs_value_type;
> struct watch;
> struct watch_notification;
> +enum bpf_cmd;
Yes, I think it's fine to include bpf.h in security.h instead of the
forward declaration.
> /* Default (no) options for the capable function */
> #define CAP_OPT_NONE 0x0
> @@ -2031,6 +2032,11 @@ extern void security_bpf_map_free(struct bpf_map *map);
> extern int security_bpf_prog_load(struct bpf_prog *prog, union bpf_attr *attr,
> struct bpf_token *token);
> extern void security_bpf_prog_free(struct bpf_prog *prog);
> +extern int security_bpf_token_create(struct bpf_token *token, union bpf_attr *attr,
> + struct path *path);
> +extern void security_bpf_token_free(struct bpf_token *token);
> +extern int security_bpf_token_allow_cmd(const struct bpf_token *token, enum bpf_cmd cmd);
> +extern int security_bpf_token_capable(const struct bpf_token *token, int cap);
> #else
> static inline int security_bpf(int cmd, union bpf_attr *attr,
> unsigned int size)
> @@ -2065,6 +2071,25 @@ static inline int security_bpf_prog_load(struct bpf_prog *prog, union bpf_attr *
>
> static inline void security_bpf_prog_free(struct bpf_prog *prog)
> { }
> +
> +static inline int security_bpf_token_create(struct bpf_token *token, union bpf_attr *attr,
> + struct path *path)
> +{
> + return 0;
> +}
> +
> +static inline void security_bpf_token_free(struct bpf_token *token)
> +{ }
> +
> +static inline int security_bpf_token_allow_cmd(const struct bpf_token *token, enum bpf_cmd cmd)
> +{
> + return 0;
> +}
> +
> +static inline int security_bpf_token_capable(const struct bpf_token *token, int cap)
> +{
> + return 0;
> +}
Another nitpick, but I would prefer to shorten
security_bpf_token_allow_cmd() renamed to security_bpf_token_cmd() both
to shorten the name and to better fit convention. I realize the caller
is named bpf_token_allow_cmd() but I'd still rather see the LSM hook
with the shorter name.
> diff --git a/kernel/bpf/token.c b/kernel/bpf/token.c
> index 35e6f55c2a41..5d04da54faea 100644
> --- a/kernel/bpf/token.c
> +++ b/kernel/bpf/token.c
> @@ -7,11 +7,12 @@
> #include <linux/idr.h>
> #include <linux/namei.h>
> #include <linux/user_namespace.h>
> +#include <linux/security.h>
>
> bool bpf_token_capable(const struct bpf_token *token, int cap)
> {
> /* BPF token allows ns_capable() level of capabilities */
> - if (token) {
> + if (token && security_bpf_token_capable(token, cap) == 0) {
> if (ns_capable(token->userns, cap))
> return true;
> if (cap != CAP_SYS_ADMIN && ns_capable(token->userns, CAP_SYS_ADMIN))
We typically perform the capability based access controls prior to the
LSM controls, meaning if we want to the token controls to work in a
similar way we should do something like this:
bool bpf_token_capable(...)
{
if (token) {
if (ns_capable(token, cap) ||
(cap != ADMIN && ns_capable(token, ADMIN)))
return security_bpf_token_capable(token, cap);
}
return capable(cap) || (cap != ADMIN && capable(...))
}
> @@ -28,6 +29,7 @@ void bpf_token_inc(struct bpf_token *token)
>
> static void bpf_token_free(struct bpf_token *token)
> {
> + security_bpf_token_free(token);
> put_user_ns(token->userns);
> kvfree(token);
> }
> @@ -172,6 +174,10 @@ int bpf_token_create(union bpf_attr *attr)
> token->allowed_progs = mnt_opts->delegate_progs;
> token->allowed_attachs = mnt_opts->delegate_attachs;
>
> + err = security_bpf_token_create(token, attr, &path);
> + if (err)
> + goto out_token;
> +
> fd = get_unused_fd_flags(O_CLOEXEC);
> if (fd < 0) {
> err = fd;
> @@ -216,8 +222,9 @@ bool bpf_token_allow_cmd(const struct bpf_token *token, enum bpf_cmd cmd)
> {
> if (!token)
> return false;
> -
> - return token->allowed_cmds & (1ULL << cmd);
> + if (!(token->allowed_cmds & (1ULL << cmd)))
> + return false;
> + return security_bpf_token_allow_cmd(token, cmd) == 0;
I'm not sure how much it really matters, but someone might prefer
the '!!' approach/style over '== 0'.
> }
>
> bool bpf_token_allow_map_type(const struct bpf_token *token, enum bpf_map_type type)
--
paul-moore.com
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox