netdev - Re: [RFC net-next 2/2] eth: bnxt: add support rx side device memory TCP

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAHS8izNwpoH7qQbRqS3gpZaouVsR-8j5ju_ZRU6UmjO1ugbFWw@mail.gmail.com>
Date: Wed, 2 Apr 2025 15:11:39 -0700
From: Mina Almasry <almasrymina@...gle.com>
To: Jakub Kicinski <kuba@...nel.org>
Cc: Taehee Yoo <ap420073@...il.com>, davem@...emloft.net, pabeni@...hat.com, 
	edumazet@...gle.com, andrew+netdev@...n.ch, horms@...nel.org, 
	michael.chan@...adcom.com, pavan.chebbi@...adcom.com, 
	ilias.apalodimas@...aro.org, dw@...idwei.uk, netdev@...r.kernel.org, 
	kuniyu@...zon.com, sdf@...ichev.me, aleksander.lobakin@...el.com
Subject: Re: [RFC net-next 2/2] eth: bnxt: add support rx side device memory TCP

On Mon, Mar 31, 2025 at 11:50 AM Jakub Kicinski <kuba@...nel.org> wrote:
>
> On Mon, 31 Mar 2025 11:47:29 +0000 Taehee Yoo wrote:
> > @@ -2863,15 +2865,15 @@ static inline bool bnxt_sriov_cfg(struct bnxt *bp)
> >  #endif
> >  }
> >
> > -static inline u8 *bnxt_data_ptr(struct bnxt *bp, struct page *page,
> > +static inline u8 *bnxt_data_ptr(struct bnxt *bp, netmem_ref netmem,
> >                               unsigned int offset)
> >  {
> > -     return page_address(page) + offset + bp->rx_offset;
> > +     return netmem_address(netmem) + offset + bp->rx_offset;
> >  }
> >
> > -static inline u8 *bnxt_data(struct page *page, unsigned int offset)
> > +static inline u8 *bnxt_data(netmem_ref netmem, unsigned int offset)
> >  {
> > -     return page_address(page) + offset;
> > +     return netmem_address(netmem) + offset;
> >  }
>
> This is not great, seems like the unification of normal vs agg bd struct
> backfires here. unreadable netmem can only be populated in agg bds
> right? So why don't we keep the structs separate and avoid the need
> to convert from netmem back to a VA?
>

Another option for your consideration (I don't know if it's better):

static inline u8 *bnxt_data(netmem_ref netmem, unsigned int offset)
{
    void * addr = netmem_addr(netmem);
    if (!addr) return addr;
    return addr + offset;
}

That way you can combine the structs, but all users of the return
value of bnxt_data need to NULL check it.

This would more naturally extend to possible future readable net_iovs.

> > diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_xdp.h b/drivers/net/ethernet/broadcom/bnxt/bnxt_xdp.h
> > index 9592d04e0661..85b6df6a9e7f 100644
> > --- a/drivers/net/ethernet/broadcom/bnxt/bnxt_xdp.h
> > +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_xdp.h
> > @@ -18,7 +18,7 @@ struct bnxt_sw_tx_bd *bnxt_xmit_bd(struct bnxt *bp,
> >                                  struct xdp_buff *xdp);
> >  void bnxt_tx_int_xdp(struct bnxt *bp, struct bnxt_napi *bnapi, int budget);
> >  bool bnxt_rx_xdp(struct bnxt *bp, struct bnxt_rx_ring_info *rxr, u16 cons,
> > -              struct xdp_buff *xdp, struct page *page, u8 **data_ptr,
> > +              struct xdp_buff *xdp, netmem_ref netmem, u8 **data_ptr,
> >                unsigned int *len, u8 *event);
> >  int bnxt_xdp(struct net_device *dev, struct netdev_bpf *xdp);
> >  int bnxt_xdp_xmit(struct net_device *dev, int num_frames,
> > @@ -27,7 +27,7 @@ int bnxt_xdp_xmit(struct net_device *dev, int num_frames,
> >  bool bnxt_xdp_attached(struct bnxt *bp, struct bnxt_rx_ring_info *rxr);
> >
> >  void bnxt_xdp_buff_init(struct bnxt *bp, struct bnxt_rx_ring_info *rxr,
> > -                     u16 cons, struct page *page, unsigned int len,
> > +                     u16 cons, netmem_ref netmem, unsigned int len,
> >                       struct xdp_buff *xdp);
>
> We also shouldn't pass netmem to XDP init, it's strange conceptually.
> If we reach XDP it has to be a non-net_iov page.
>

Very noob question, but is XDP/netmem interactions completely
impossible for some reason? I was thinking XDP progs that only
touch/need the header may work with unreadable netmem, and if we ever
add readable net_iovs then those maybe can be exposed to XDP, no? Or
am I completely off the rails here?

--
Thanks,
Mina