Re: [net-next v3 02/12] net: tso: Add tso_dma_map helpers

[Leon Romanovsky <leon@kernel.org>]

On Thu, Mar 19, 2026 at 10:09:06AM -0700, Joe Damato wrote:
> On Thu, Mar 19, 2026 at 09:39:59AM +0200, Leon Romanovsky wrote:
> > On Wed, Mar 18, 2026 at 12:13:07PM -0700, Joe Damato wrote:
> > > Adds skb_frag_phys() to skbuff.h, returning the physical address
> > > of a paged fragment's data, which is used by the tso_dma_map helpers
> > > introduced in this commit described below:
> > > 
> > > tso_dma_map_init(): DMA-maps the linear payload region and all frags
> > > upfront. Prefers the DMA IOVA API for a single contiguous mapping with
> > > one IOTLB sync; falls back to per-region dma_map_phys() otherwise.
> > > Returns 0 on success, cleans up partial mappings on failure.
> > > 
> > > tso_dma_map_cleanup(): Handles both IOVA and fallback teardown paths.
> > > 
> > > tso_dma_map_count(): counts how many descriptors the next N bytes of
> > > payload will need. Returns 1 if IOVA is used since the mapping is
> > > contiguous.
> > > 
> > > tso_dma_map_next(): yields the next (dma_addr, chunk_len) pair.
> > > On the IOVA path, each segment is a single contiguous chunk. On the
> > > fallback path, indicates when a chunk starts a new DMA mapping so the
> > > driver can set dma_unmap_len on that descriptor for completion-time
> > > unmapping.
> > > 
> > > Suggested-by: Jakub Kicinski <kuba@kernel.org>
> > > Signed-off-by: Joe Damato <joe@dama.to>
> > > ---
> > >  v3:
> > >    - Added skb_frag_phys helper include/linux/skbuff.h.
> > >    - Added tso_dma_map_use_iova() inline helper in tso.h.
> > >    - Updated the helpers to use the DMA IOVA API and falls back to per-region
> > >      mapping instead.
> > > 
> > >  include/linux/skbuff.h |  11 ++
> > >  include/net/tso.h      |  21 ++++
> > >  net/core/tso.c         | 274 +++++++++++++++++++++++++++++++++++++++++
> > >  3 files changed, 306 insertions(+)
> > > 
> > > diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
> > > index 9cc98f850f1d..d8630eb366c5 100644
> > > --- a/include/linux/skbuff.h
> > > +++ b/include/linux/skbuff.h
> > > @@ -3758,6 +3758,17 @@ static inline void *skb_frag_address_safe(const skb_frag_t *frag)
> > >  	return ptr + skb_frag_off(frag);
> > >  }
> > >  
> > > +/**
> > > + * skb_frag_phys - gets the physical address of the data in a paged fragment
> > > + * @frag: the paged fragment buffer
> > > + *
> > > + * Returns: the physical address of the data within @frag.
> > > + */
> > > +static inline phys_addr_t skb_frag_phys(const skb_frag_t *frag)
> > > +{
> > > +	return page_to_phys(skb_frag_page(frag)) + skb_frag_off(frag);
> > > +}
> > 
> > I skimmed through the patch and it looks generally correct to me. The one
> > thing that disappointed me is this function. It's unfortunate that you
> > have to implement it this way and cannot rely on phys_addr_t directly.
> 
> Thanks for taking a look.
> 
> Would you prefer if I modified this and created a netmem_to_phys() helper
> (page-only for now, but extensible later?) and use that instead?
> 
> Or is this patch acceptable as-is for this series with the understanding that
> it only handles page-backed frags?
> 
> Just want to make sure I'm following what you mean.

It is acceptable as is. I'm just expressing my view about performing so
much translations.

Thanks

> 
> Thanks.
>