Re: [PATCH net-next v20 09/14] net: rename skb_copy_to_page_nocache() helper

[Alexander Duyck]

On Tue, Oct 8, 2024 at 4:27 AM Yunsheng Lin <linyunsheng@xxxxxxxxxx> wrote:
>
> Rename skb_copy_to_page_nocache() to skb_add_frag_nocache()
> to avoid calling virt_to_page() as we are about to pass virtual
> address directly.
>
> CC: Alexander Duyck <alexander.duyck@xxxxxxxxx>
> Signed-off-by: Yunsheng Lin <linyunsheng@xxxxxxxxxx>
> ---
>  include/net/sock.h | 9 +++------
>  net/ipv4/tcp.c     | 7 +++----
>  net/kcm/kcmsock.c  | 7 +++----
>  3 files changed, 9 insertions(+), 14 deletions(-)
>
> diff --git a/include/net/sock.h b/include/net/sock.h
> index e282127092ab..e0b4e2daca5d 100644
> --- a/include/net/sock.h
> +++ b/include/net/sock.h
> @@ -2192,15 +2192,12 @@ static inline int skb_add_data_nocache(struct sock *sk, struct sk_buff *skb,
>         return err;
>  }
>
> -static inline int skb_copy_to_page_nocache(struct sock *sk, struct iov_iter *from,
> -                                          struct sk_buff *skb,
> -                                          struct page *page,
> -                                          int off, int copy)
> +static inline int skb_add_frag_nocache(struct sock *sk, struct iov_iter *from,
> +                                      struct sk_buff *skb, char *va, int copy)

This is not adding a frag. It is copying to a frag. This naming is a
hard no as there are functions that actually add frags to the skb and
this is not what this is doing. It sounds like it should be some
variant on skb_add_rx_frag and it isn't.

Instead of "_add_" I would suggest you stick with "_copy_to_" as the
action as the alternative would be confusing as it implies you are
going to be adding this to frags yourself.