diff mbox series

[net-next,v4,5/5] net: Document netmem driver support

Message ID 20241211212033.1684197-6-almasrymina@google.com (mailing list archive)
State Changes Requested
Delegated to: Netdev Maintainers
Headers show
Series devmem TCP fixes | expand

Checks

Context Check Description
netdev/series_format success Posting correctly formatted
netdev/tree_selection success Clearly marked for net-next
netdev/ynl success Generated files up to date; no warnings/errors; no diff in generated;
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 0 this patch: 0
netdev/build_tools success No tools touched, skip
netdev/cc_maintainers success CCed 7 of 7 maintainers
netdev/build_clang success Errors and warnings before: 1 this patch: 1
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 5 this patch: 5
netdev/checkpatch warning WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
netdev/build_clang_rust success No Rust files in patch. Skipping build
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Mina Almasry Dec. 11, 2024, 9:20 p.m. UTC
Document expectations from drivers looking to add support for device
memory tcp or other netmem based features.

Signed-off-by: Mina Almasry <almasrymina@google.com>

---

v4:
- Address comments from Randy.
- Change docs to netmem focus (Jakub).
- Address comments from Jakub.

---
 Documentation/networking/index.rst  |  1 +
 Documentation/networking/netmem.rst | 62 +++++++++++++++++++++++++++++
 2 files changed, 63 insertions(+)
 create mode 100644 Documentation/networking/netmem.rst

Comments

Nelson, Shannon Dec. 11, 2024, 10:58 p.m. UTC | #1
On 12/11/2024 1:20 PM, Mina Almasry wrote:
> 
> Document expectations from drivers looking to add support for device
> memory tcp or other netmem based features.
> 
> Signed-off-by: Mina Almasry <almasrymina@google.com>

Hi Mina,

Just a couple thoughts as this passed by me.  These can be saved for a 
later update if the rest of this patchset is ready to go.

> 
> ---
> 
> v4:
> - Address comments from Randy.
> - Change docs to netmem focus (Jakub).
> - Address comments from Jakub.
> 
> ---
>   Documentation/networking/index.rst  |  1 +
>   Documentation/networking/netmem.rst | 62 +++++++++++++++++++++++++++++
>   2 files changed, 63 insertions(+)
>   create mode 100644 Documentation/networking/netmem.rst
> 
> diff --git a/Documentation/networking/index.rst b/Documentation/networking/index.rst
> index 46c178e564b3..058193ed2eeb 100644
> --- a/Documentation/networking/index.rst
> +++ b/Documentation/networking/index.rst
> @@ -86,6 +86,7 @@ Contents:
>      netdevices
>      netfilter-sysctl
>      netif-msg
> +   netmem
>      nexthop-group-resilient
>      nf_conntrack-sysctl
>      nf_flowtable
> diff --git a/Documentation/networking/netmem.rst b/Documentation/networking/netmem.rst
> new file mode 100644
> index 000000000000..f9f03189c53c
> --- /dev/null
> +++ b/Documentation/networking/netmem.rst
> @@ -0,0 +1,62 @@
> +.. SPDX-License-Identifier: GPL-2.0
> +
> +================
> +Netmem
> +================
> +
> +
> +Introduction
> +============
> +
> +Device memory TCP, and likely more upcoming features, are reliant on netmem

Device memory TCP is singular, so s/are/is/

> +support in the driver. This outlines what drivers need to do to support netmem.

Can we get a summary of what netmem itself is and what it is for?  There 
is a bit of explanation buried below in (3), but it would be good to 
have something here at the top.

> +
> +
> +Driver support
> +==============
> +
> +1. The driver must support page_pool. The driver must not do its own recycling
> +   on top of page_pool.
> +
> +2. The driver must support the tcp-data-split ethtool option.
> +
> +3. The driver must use the page_pool netmem APIs. The netmem APIs are
> +   currently 1-to-1 correspond with page APIs. Conversion to netmem should be
> +   achievable by switching the page APIs to netmem APIs and tracking memory via
> +   netmem_refs in the driver rather than struct page * :
> +
> +   - page_pool_alloc -> page_pool_alloc_netmem
> +   - page_pool_get_dma_addr -> page_pool_get_dma_addr_netmem
> +   - page_pool_put_page -> page_pool_put_netmem
> +
> +   Not all page APIs have netmem equivalents at the moment. If your driver
> +   relies on a missing netmem API, feel free to add and propose to netdev@ or
> +   reach out to almasrymina@google.com for help adding the netmem API.

You may want to replace your name with "the maintainers" and let the 
MAINTAINERS file keep track of who currently takes care of netmem 
things, rather than risk this email getting stale and forgotten.


> +
> +4. The driver must use the following PP_FLAGS:
> +
> +   - PP_FLAG_DMA_MAP: netmem is not dma-mappable by the driver. The driver
> +     must delegate the dma mapping to the page_pool.

This is a bit confusing... if not dma-mappable, then why use 
PP_FLAG_DMA_MAP to ask page_pool to do it?  A little more info might be 
useful such as,
" ... must delegate the dma mapping to the page_pool which knows when 
dma-mapping is or is not appropriate".

Thanks,
sln


> +   - PP_FLAG_DMA_SYNC_DEV: netmem dma addr is not necessarily dma-syncable
> +     by the driver. The driver must delegate the dma syncing to the page_pool.
> +   - PP_FLAG_ALLOW_UNREADABLE_NETMEM. The driver must specify this flag iff
> +     tcp-data-split is enabled.
> +
> +5. The driver must not assume the netmem is readable and/or backed by pages.
> +   The netmem returned by the page_pool may be unreadable, in which case
> +   netmem_address() will return NULL. The driver must correctly handle
> +   unreadable netmem, i.e. don't attempt to handle its contents when
> +   netmem_address() is NULL.
> +
> +   Ideally, drivers should not have to check the underlying netmem type via
> +   helpers like netmem_is_net_iov() or convert the netmem to any of its
> +   underlying types via netmem_to_page() or netmem_to_net_iov(). In most cases,
> +   netmem or page_pool helpers that abstract this complexity are provided
> +   (and more can be added).
> +
> +6. The driver must use page_pool_dma_sync_netmem_for_cpu() in lieu of
> +   dma_sync_single_range_for_cpu(). For some memory providers, dma_syncing for
> +   CPU will be done by the page_pool, for others (particularly dmabuf memory
> +   provider), dma syncing for CPU is the responsibility of the userspace using
> +   dmabuf APIs. The driver must delegate the entire dma-syncing operation to
> +   the page_pool which will do it correctly.
> --
> 2.47.0.338.g60cca15819-goog
> 
>
Jakub Kicinski Dec. 13, 2024, 2:53 a.m. UTC | #2
On Wed, 11 Dec 2024 21:20:32 +0000 Mina Almasry wrote:
> +
> +================
> +Netmem
> +================

The length of the ==== lines must match the title.

> +Introduction
> +============
> +
> +Device memory TCP, and likely more upcoming features, are reliant on netmem
> +support in the driver. This outlines what drivers need to do to support netmem.
> +
> +
> +Driver support
> +==============
> +
> +1. The driver must support page_pool. The driver must not do its own recycling
> +   on top of page_pool.

We discussed this one, probably needs a bit of rewording at least.

> +2. The driver must support the tcp-data-split ethtool option.
> +
> +3. The driver must use the page_pool netmem APIs.

We should probably mention that this is only for payload pages?
Mina Almasry Dec. 17, 2024, 7:27 p.m. UTC | #3
On Wed, Dec 11, 2024 at 2:58 PM Nelson, Shannon <shannon.nelson@amd.com> wrote:
> > +
> > +Driver support
> > +==============
> > +
> > +1. The driver must support page_pool. The driver must not do its own recycling
> > +   on top of page_pool.
> > +
> > +2. The driver must support the tcp-data-split ethtool option.
> > +
> > +3. The driver must use the page_pool netmem APIs. The netmem APIs are
> > +   currently 1-to-1 correspond with page APIs. Conversion to netmem should be
> > +   achievable by switching the page APIs to netmem APIs and tracking memory via
> > +   netmem_refs in the driver rather than struct page * :
> > +
> > +   - page_pool_alloc -> page_pool_alloc_netmem
> > +   - page_pool_get_dma_addr -> page_pool_get_dma_addr_netmem
> > +   - page_pool_put_page -> page_pool_put_netmem
> > +
> > +   Not all page APIs have netmem equivalents at the moment. If your driver
> > +   relies on a missing netmem API, feel free to add and propose to netdev@ or
> > +   reach out to almasrymina@google.com for help adding the netmem API.
>
> You may want to replace your name with "the maintainers" and let the
> MAINTAINERS file keep track of who currently takes care of netmem
> things, rather than risk this email getting stale and forgotten.
>

If it's OK with you, I'll change this to "the maintainers and/or
almasrymina@google.com".

Reasoning is that currently Jakub really has reviewed all the netmem
stuff very closely, and I'm hesitant to practically point to him to
all future netmem questions or issues, especially since I can help
with all the low hanging fruit. I don't currently show up in the
maintainers file, and unless called for there is no entry for netmem
maintenance. Wording the docs like this gives me the chance to help
with some of the low hanging fruit without going so far as to add
myself to maintainers.

If the email does go stale we can always update it and if there
becomes a dedicated entry in MAINTAINERS for netmem we can always
remove this.

Will address all the other points, thanks!
Nelson, Shannon Dec. 17, 2024, 7:40 p.m. UTC | #4
On 12/17/2024 11:27 AM, Mina Almasry wrote:
> On Wed, Dec 11, 2024 at 2:58 PM Nelson, Shannon <shannon.nelson@amd.com> wrote:
>>> +
>>> +Driver support
>>> +==============
>>> +
>>> +1. The driver must support page_pool. The driver must not do its own recycling
>>> +   on top of page_pool.
>>> +
>>> +2. The driver must support the tcp-data-split ethtool option.
>>> +
>>> +3. The driver must use the page_pool netmem APIs. The netmem APIs are
>>> +   currently 1-to-1 correspond with page APIs. Conversion to netmem should be
>>> +   achievable by switching the page APIs to netmem APIs and tracking memory via
>>> +   netmem_refs in the driver rather than struct page * :
>>> +
>>> +   - page_pool_alloc -> page_pool_alloc_netmem
>>> +   - page_pool_get_dma_addr -> page_pool_get_dma_addr_netmem
>>> +   - page_pool_put_page -> page_pool_put_netmem
>>> +
>>> +   Not all page APIs have netmem equivalents at the moment. If your driver
>>> +   relies on a missing netmem API, feel free to add and propose to netdev@ or
>>> +   reach out to almasrymina@google.com for help adding the netmem API.
>>
>> You may want to replace your name with "the maintainers" and let the
>> MAINTAINERS file keep track of who currently takes care of netmem
>> things, rather than risk this email getting stale and forgotten.
>>
> 
> If it's OK with you, I'll change this to "the maintainers and/or
> almasrymina@google.com".
 > > Reasoning is that currently Jakub really has reviewed all the netmem
> stuff very closely, and I'm hesitant to practically point to him to
> all future netmem questions or issues, especially since I can help
> with all the low hanging fruit. I don't currently show up in the
> maintainers file, and unless called for there is no entry for netmem
> maintenance. Wording the docs like this gives me the chance to help
> with some of the low hanging fruit without going so far as to add
> myself to maintainers.
> 
> If the email does go stale we can always update it and if there
> becomes a dedicated entry in MAINTAINERS for netmem we can always
> remove this.

Fine with me, thanks for the note.

> 
> Will address all the other points, thanks!
> 
> --
> Thanks,
> Mina

Cheers,
sln
diff mbox series

Patch

diff --git a/Documentation/networking/index.rst b/Documentation/networking/index.rst
index 46c178e564b3..058193ed2eeb 100644
--- a/Documentation/networking/index.rst
+++ b/Documentation/networking/index.rst
@@ -86,6 +86,7 @@  Contents:
    netdevices
    netfilter-sysctl
    netif-msg
+   netmem
    nexthop-group-resilient
    nf_conntrack-sysctl
    nf_flowtable
diff --git a/Documentation/networking/netmem.rst b/Documentation/networking/netmem.rst
new file mode 100644
index 000000000000..f9f03189c53c
--- /dev/null
+++ b/Documentation/networking/netmem.rst
@@ -0,0 +1,62 @@ 
+.. SPDX-License-Identifier: GPL-2.0
+
+================
+Netmem
+================
+
+
+Introduction
+============
+
+Device memory TCP, and likely more upcoming features, are reliant on netmem
+support in the driver. This outlines what drivers need to do to support netmem.
+
+
+Driver support
+==============
+
+1. The driver must support page_pool. The driver must not do its own recycling
+   on top of page_pool.
+
+2. The driver must support the tcp-data-split ethtool option.
+
+3. The driver must use the page_pool netmem APIs. The netmem APIs are
+   currently 1-to-1 correspond with page APIs. Conversion to netmem should be
+   achievable by switching the page APIs to netmem APIs and tracking memory via
+   netmem_refs in the driver rather than struct page * :
+
+   - page_pool_alloc -> page_pool_alloc_netmem
+   - page_pool_get_dma_addr -> page_pool_get_dma_addr_netmem
+   - page_pool_put_page -> page_pool_put_netmem
+
+   Not all page APIs have netmem equivalents at the moment. If your driver
+   relies on a missing netmem API, feel free to add and propose to netdev@ or
+   reach out to almasrymina@google.com for help adding the netmem API.
+
+4. The driver must use the following PP_FLAGS:
+
+   - PP_FLAG_DMA_MAP: netmem is not dma-mappable by the driver. The driver
+     must delegate the dma mapping to the page_pool.
+   - PP_FLAG_DMA_SYNC_DEV: netmem dma addr is not necessarily dma-syncable
+     by the driver. The driver must delegate the dma syncing to the page_pool.
+   - PP_FLAG_ALLOW_UNREADABLE_NETMEM. The driver must specify this flag iff
+     tcp-data-split is enabled.
+
+5. The driver must not assume the netmem is readable and/or backed by pages.
+   The netmem returned by the page_pool may be unreadable, in which case
+   netmem_address() will return NULL. The driver must correctly handle
+   unreadable netmem, i.e. don't attempt to handle its contents when
+   netmem_address() is NULL.
+
+   Ideally, drivers should not have to check the underlying netmem type via
+   helpers like netmem_is_net_iov() or convert the netmem to any of its
+   underlying types via netmem_to_page() or netmem_to_net_iov(). In most cases,
+   netmem or page_pool helpers that abstract this complexity are provided
+   (and more can be added).
+
+6. The driver must use page_pool_dma_sync_netmem_for_cpu() in lieu of
+   dma_sync_single_range_for_cpu(). For some memory providers, dma_syncing for
+   CPU will be done by the page_pool, for others (particularly dmabuf memory
+   provider), dma syncing for CPU is the responsibility of the userspace using
+   dmabuf APIs. The driver must delegate the entire dma-syncing operation to
+   the page_pool which will do it correctly.