1. Patchew
  2. linux
  3. [PATCH net-next V2 00/11] net/mlx5e: Add support for devmem and io_uring TCP zero-copy
  4. View patch

[PATCH net-next V2 02/11] net: Add skb_can_coalesce for netmem

Tariq Toukan posted 11 patches 6 months, 3 weeks ago
There is a newer version of this series
[PATCH net-next V2 02/11] net: Add skb_can_coalesce for netmem
Posted by Tariq Toukan 6 months, 3 weeks ago 
From: Dragos Tatulea <dtatulea@nvidia.com>

Allow drivers that have moved over to netmem to do fragment coalescing.

Signed-off-by: Dragos Tatulea <dtatulea@nvidia.com>
Signed-off-by: Cosmin Ratiu <cratiu@nvidia.com>
Signed-off-by: Tariq Toukan <tariqt@nvidia.com>
---
 include/linux/skbuff.h | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 5520524c93bf..e8e2860183b4 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -3887,6 +3887,18 @@ static inline bool skb_can_coalesce(struct sk_buff *skb, int i,
 	return false;
 }
 
+static inline bool skb_can_coalesce_netmem(struct sk_buff *skb, int i,
+					   const netmem_ref netmem, int off)
+{
+	if (i) {
+		const skb_frag_t *frag = &skb_shinfo(skb)->frags[i - 1];
+
+		return netmem == skb_frag_netmem(frag) &&
+		       off == skb_frag_off(frag) + skb_frag_size(frag);
+	}
+	return false;
+}
+
 static inline int __skb_linearize(struct sk_buff *skb)
 {
 	return __pskb_pull_tail(skb, skb->data_len) ? 0 : -ENOMEM;
-- 
2.31.1
Re: [PATCH net-next V2 02/11] net: Add skb_can_coalesce for netmem
Posted by Mina Almasry 6 months, 3 weeks ago 
On Thu, May 22, 2025 at 2:43 PM Tariq Toukan <tariqt@nvidia.com> wrote:
>
> From: Dragos Tatulea <dtatulea@nvidia.com>
>
> Allow drivers that have moved over to netmem to do fragment coalescing.
>
> Signed-off-by: Dragos Tatulea <dtatulea@nvidia.com>
> Signed-off-by: Cosmin Ratiu <cratiu@nvidia.com>
> Signed-off-by: Tariq Toukan <tariqt@nvidia.com>
> ---
>  include/linux/skbuff.h | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
>
> diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
> index 5520524c93bf..e8e2860183b4 100644
> --- a/include/linux/skbuff.h
> +++ b/include/linux/skbuff.h
> @@ -3887,6 +3887,18 @@ static inline bool skb_can_coalesce(struct sk_buff *skb, int i,
>         return false;
>  }
>
> +static inline bool skb_can_coalesce_netmem(struct sk_buff *skb, int i,
> +                                          const netmem_ref netmem, int off)
> +{
> +       if (i) {
> +               const skb_frag_t *frag = &skb_shinfo(skb)->frags[i - 1];
> +
> +               return netmem == skb_frag_netmem(frag) &&
> +                      off == skb_frag_off(frag) + skb_frag_size(frag);
> +       }
> +       return false;
> +}
> +

Can we limit the code duplication by changing skb_can_coalesce to call
skb_can_coalesce_netmem? Or is that too bad for perf?

static inline bool skb_can_coalesce(struct sk_buff *skb, int i, const
struct page *page, int off) {
    skb_can_coalesce_netmem(skb, i, page_to_netmem(page), off);
}

It's always safe to cast a page to netmem.

-- 
Thanks,
Mina
Re: [PATCH net-next V2 02/11] net: Add skb_can_coalesce for netmem
Posted by Dragos Tatulea 6 months, 3 weeks ago 
On Thu, May 22, 2025 at 04:09:35PM -0700, Mina Almasry wrote:
> On Thu, May 22, 2025 at 2:43 PM Tariq Toukan <tariqt@nvidia.com> wrote:
> >
> > From: Dragos Tatulea <dtatulea@nvidia.com>
> >
> > Allow drivers that have moved over to netmem to do fragment coalescing.
> >
> > Signed-off-by: Dragos Tatulea <dtatulea@nvidia.com>
> > Signed-off-by: Cosmin Ratiu <cratiu@nvidia.com>
> > Signed-off-by: Tariq Toukan <tariqt@nvidia.com>
> > ---
> >  include/linux/skbuff.h | 12 ++++++++++++
> >  1 file changed, 12 insertions(+)
> >
> > diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
> > index 5520524c93bf..e8e2860183b4 100644
> > --- a/include/linux/skbuff.h
> > +++ b/include/linux/skbuff.h
> > @@ -3887,6 +3887,18 @@ static inline bool skb_can_coalesce(struct sk_buff *skb, int i,
> >         return false;
> >  }
> >
> > +static inline bool skb_can_coalesce_netmem(struct sk_buff *skb, int i,
> > +                                          const netmem_ref netmem, int off)
> > +{
> > +       if (i) {
> > +               const skb_frag_t *frag = &skb_shinfo(skb)->frags[i - 1];
> > +
> > +               return netmem == skb_frag_netmem(frag) &&
> > +                      off == skb_frag_off(frag) + skb_frag_size(frag);
> > +       }
> > +       return false;
> > +}
> > +
> 
> Can we limit the code duplication by changing skb_can_coalesce to call
> skb_can_coalesce_netmem? Or is that too bad for perf?
>
> static inline bool skb_can_coalesce(struct sk_buff *skb, int i, const
> struct page *page, int off) {
>     skb_can_coalesce_netmem(skb, i, page_to_netmem(page), off);
> }
> 
> It's always safe to cast a page to netmem.
>
I think it makes sense and I don't see an issue with perf as everything
stays inline and the cast should be free.

As netmems are used only for rx and skb_zcopy() seems to be used
only for tx (IIUC), maybe it makes sense to keep the skb_zcopy() check
within skb_can_coalesce(). Like below. Any thoughts?

--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -3873,15 +3873,22 @@ static inline bool skb_can_coalesce(struct sk_buff *skb, int i,
 {
 	if (skb_zcopy(skb))
 		return false;
-	if (i) {
-		const skb_frag_t *frag = &skb_shinfo(skb)->frags[i - 1];
 
-		return page == skb_frag_page(frag) &&
-		       off == skb_frag_off(frag) + skb_frag_size(frag);
-	}
-	return false;
+	return skb_can_coalesce_netmem(skb, i, page_to_netmem(page), off);
 }
 
+   static inline bool skb_can_coalesce_netmem(struct sk_buff *skb, int i,
+                                              const netmem_ref netmem, int off)
+   {
+          if (i) {
+                  const skb_frag_t *frag = &skb_shinfo(skb)->frags[i - 1];
+
+                  return netmem == skb_frag_netmem(frag) &&
+                         off == skb_frag_off(frag) + skb_frag_size(frag);
+          }
+          return false;
+   }
+

Thanks,
Dragos
Re: [PATCH net-next V2 02/11] net: Add skb_can_coalesce for netmem
Posted by Mina Almasry 6 months, 3 weeks ago 
On Sun, May 25, 2025 at 6:04 AM Dragos Tatulea <dtatulea@nvidia.com> wrote:
>
> On Thu, May 22, 2025 at 04:09:35PM -0700, Mina Almasry wrote:
> > On Thu, May 22, 2025 at 2:43 PM Tariq Toukan <tariqt@nvidia.com> wrote:
> > >
> > > From: Dragos Tatulea <dtatulea@nvidia.com>
> > >
> > > Allow drivers that have moved over to netmem to do fragment coalescing.
> > >
> > > Signed-off-by: Dragos Tatulea <dtatulea@nvidia.com>
> > > Signed-off-by: Cosmin Ratiu <cratiu@nvidia.com>
> > > Signed-off-by: Tariq Toukan <tariqt@nvidia.com>
> > > ---
> > >  include/linux/skbuff.h | 12 ++++++++++++
> > >  1 file changed, 12 insertions(+)
> > >
> > > diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
> > > index 5520524c93bf..e8e2860183b4 100644
> > > --- a/include/linux/skbuff.h
> > > +++ b/include/linux/skbuff.h
> > > @@ -3887,6 +3887,18 @@ static inline bool skb_can_coalesce(struct sk_buff *skb, int i,
> > >         return false;
> > >  }
> > >
> > > +static inline bool skb_can_coalesce_netmem(struct sk_buff *skb, int i,
> > > +                                          const netmem_ref netmem, int off)
> > > +{
> > > +       if (i) {
> > > +               const skb_frag_t *frag = &skb_shinfo(skb)->frags[i - 1];
> > > +
> > > +               return netmem == skb_frag_netmem(frag) &&
> > > +                      off == skb_frag_off(frag) + skb_frag_size(frag);
> > > +       }
> > > +       return false;
> > > +}
> > > +
> >
> > Can we limit the code duplication by changing skb_can_coalesce to call
> > skb_can_coalesce_netmem? Or is that too bad for perf?
> >
> > static inline bool skb_can_coalesce(struct sk_buff *skb, int i, const
> > struct page *page, int off) {
> >     skb_can_coalesce_netmem(skb, i, page_to_netmem(page), off);
> > }
> >
> > It's always safe to cast a page to netmem.
> >
> I think it makes sense and I don't see an issue with perf as everything
> stays inline and the cast should be free.
>
> As netmems are used only for rx and skb_zcopy() seems to be used
> only for tx (IIUC), maybe it makes sense to keep the skb_zcopy() check
> within skb_can_coalesce(). Like below. Any thoughts?
>

[net|dev]mems can now be in the TX path too:
https://lore.kernel.org/netdev/20250508004830.4100853-1-almasrymina@google.com/

And even without explicit TX support, IIUC from Kuba RX packets can
always be looped back to the TX path via forwarding or tc and what
not. So let's leave the skb_zcopy check in the common path for now
unless we're sure the move is safe.

-- 
Thanks,
Mina
Re: [PATCH net-next V2 02/11] net: Add skb_can_coalesce for netmem
Posted by Dragos Tatulea 6 months, 2 weeks ago 
On Sun, May 25, 2025 at 10:44:43AM -0700, Mina Almasry wrote:
> On Sun, May 25, 2025 at 6:04 AM Dragos Tatulea <dtatulea@nvidia.com> wrote:
> >
> > On Thu, May 22, 2025 at 04:09:35PM -0700, Mina Almasry wrote:
> > > On Thu, May 22, 2025 at 2:43 PM Tariq Toukan <tariqt@nvidia.com> wrote:
> > > >
> > > > From: Dragos Tatulea <dtatulea@nvidia.com>
> > > >
> > > > Allow drivers that have moved over to netmem to do fragment coalescing.
> > > >
> > > > Signed-off-by: Dragos Tatulea <dtatulea@nvidia.com>
> > > > Signed-off-by: Cosmin Ratiu <cratiu@nvidia.com>
> > > > Signed-off-by: Tariq Toukan <tariqt@nvidia.com>
> > > > ---
> > > >  include/linux/skbuff.h | 12 ++++++++++++
> > > >  1 file changed, 12 insertions(+)
> > > >
> > > > diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
> > > > index 5520524c93bf..e8e2860183b4 100644
> > > > --- a/include/linux/skbuff.h
> > > > +++ b/include/linux/skbuff.h
> > > > @@ -3887,6 +3887,18 @@ static inline bool skb_can_coalesce(struct sk_buff *skb, int i,
> > > >         return false;
> > > >  }
> > > >
> > > > +static inline bool skb_can_coalesce_netmem(struct sk_buff *skb, int i,
> > > > +                                          const netmem_ref netmem, int off)
> > > > +{
> > > > +       if (i) {
> > > > +               const skb_frag_t *frag = &skb_shinfo(skb)->frags[i - 1];
> > > > +
> > > > +               return netmem == skb_frag_netmem(frag) &&
> > > > +                      off == skb_frag_off(frag) + skb_frag_size(frag);
> > > > +       }
> > > > +       return false;
> > > > +}
> > > > +
> > >
> > > Can we limit the code duplication by changing skb_can_coalesce to call
> > > skb_can_coalesce_netmem? Or is that too bad for perf?
> > >
> > > static inline bool skb_can_coalesce(struct sk_buff *skb, int i, const
> > > struct page *page, int off) {
> > >     skb_can_coalesce_netmem(skb, i, page_to_netmem(page), off);
> > > }
> > >
> > > It's always safe to cast a page to netmem.
> > >
> > I think it makes sense and I don't see an issue with perf as everything
> > stays inline and the cast should be free.
> >
> > As netmems are used only for rx and skb_zcopy() seems to be used
> > only for tx (IIUC), maybe it makes sense to keep the skb_zcopy() check
> > within skb_can_coalesce(). Like below. Any thoughts?
> >
> 
> [net|dev]mems can now be in the TX path too:
> https://lore.kernel.org/netdev/20250508004830.4100853-1-almasrymina@google.com/
> 
> And even without explicit TX support, IIUC from Kuba RX packets can
> always be looped back to the TX path via forwarding or tc and what
> not. So let's leave the skb_zcopy check in the common path for now
> unless we're sure the move is safe.
>
Makes sense. Will be done.

Thanks,
Dragos

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.