netdev - Re: [PATCH net-next] eth: bnxt: add netmem TX support

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: <CAMArcTVpYct8FP1r3UsVaHDW+fSY6FG9NNmhDfyiTjgCkiYsxw@mail.gmail.com>
Date: Wed, 18 Jun 2025 22:20:59 +0900
From: Taehee Yoo <ap420073@...il.com>
To: Mina Almasry <almasrymina@...gle.com>
Cc: Pranjal Shrivastava <praan@...gle.com>, Shivaji Kant <shivajikant@...gle.com>, 
	Stanislav Fomichev <sdf@...ichev.me>, Pavel Begunkov <asml.silence@...il.com>, davem@...emloft.net, 
	kuba@...nel.org, pabeni@...hat.com, edumazet@...gle.com, 
	andrew+netdev@...n.ch, horms@...nel.org, michael.chan@...adcom.com, 
	pavan.chebbi@...adcom.com, netdev@...r.kernel.org
Subject: Re: [PATCH net-next] eth: bnxt: add netmem TX support

On Wed, Jun 18, 2025 at 5:14 AM Mina Almasry <almasrymina@...gle.com> wrote:
>

Hi Mina,
Thanks a lot for your review!

> On Tue, Jun 17, 2025 at 2:45 AM Taehee Yoo <ap420073@...il.com> wrote:
> >
> > Use netmem_dma_*() helpers and declare netmem_tx to support netmem TX.
> > By this change, all bnxt devices will support the netmem TX.
> >
> > bnxt_start_xmit() uses memcpy() if a packet is too small. However,
>
> nit: this is slightly inaccurate. memcpy itself is not a problem (via
> skb_copy_from_linear_data) is not an issue because I think that's
> copying the linear part of the skb. What is really a is
> skb_frag_address_safe(). Unreadable skbs have no valid address.
>
> This made me realize that skb_frag_address_safe() is broken :( it
> needs this check, similar to skb_frag_address():

You're right!
The real problem is the skb_frag_address_safe(), as you mentioned.
I will fix a git commit message in the v2.

Thanks a lot!
Taehee Yoo

>
> ```
> diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
> index c05057869e08..da03ff71b05e 100644
> --- a/include/linux/skbuff.h
> +++ b/include/linux/skbuff.h
> @@ -3681,7 +3681,12 @@ static inline void *skb_frag_address(const
> skb_frag_t *frag)
>   */
>  static inline void *skb_frag_address_safe(const skb_frag_t *frag)
>  {
> -       void *ptr = page_address(skb_frag_page(frag));
> +       void *ptr;
> +
> +       if (!skb_frag_page(frag))
> +               return NULL;
> +
> +       ptr = page_address(skb_frag_page(frag));
>         if (unlikely(!ptr))
>                 return NULL;
> ```
>
> I guess I'll send this fix to net.
>
> > netmem packets are unreadable, so memcpy() is not allowed.
> > It should check whether an skb is readable, and if an SKB is unreadable,
> > it is processed by the normal transmission logic.
> >
> > netmem TX can be tested with ncdevmem.c
> >
> > Signed-off-by: Taehee Yoo <ap420073@...il.com>
>
> Seems like a straightforward conversion to using the netmem dma
> mapping API. I don't see anything concerning/unusualy.
>
> Acked-by: Mina Almasry <almasrymina@...gle.com>
>
> --
> Thanks,
> Mina