Re: [PATCH net-next v2 05/18] xdp, xsk: constify read-only arguments of some static inline helpers

[Maciej Fijalkowski <maciej.fijalkowski@intel.com>]

On Tue, Oct 15, 2024 at 04:53:37PM +0200, Alexander Lobakin wrote:
> Lots of read-only helpers for &xdp_buff and &xdp_frame, such as getting
> the frame length, skb_shared_info etc., don't have their arguments
> marked with `const` for no reason. Add the missing annotations to leave
> less place for mistakes and more for optimization.

Same comment as in previous patch.
Good stuff regardless.

> 
> Signed-off-by: Alexander Lobakin <aleksander.lobakin@intel.com>
> ---
>  include/net/xdp.h           | 29 +++++++++++++++++------------
>  include/net/xdp_sock_drv.h  | 11 ++++++-----
>  include/net/xsk_buff_pool.h |  2 +-
>  3 files changed, 24 insertions(+), 18 deletions(-)
> 
> diff --git a/include/net/xdp.h b/include/net/xdp.h
> index e6770dd40c91..197808df1ee1 100644
> --- a/include/net/xdp.h
> +++ b/include/net/xdp.h
> @@ -88,7 +88,7 @@ struct xdp_buff {
>  	u32 flags; /* supported values defined in xdp_buff_flags */
>  };
>  
> -static __always_inline bool xdp_buff_has_frags(struct xdp_buff *xdp)
> +static __always_inline bool xdp_buff_has_frags(const struct xdp_buff *xdp)
>  {
>  	return !!(xdp->flags & XDP_FLAGS_HAS_FRAGS);
>  }
> @@ -103,7 +103,8 @@ static __always_inline void xdp_buff_clear_frags_flag(struct xdp_buff *xdp)
>  	xdp->flags &= ~XDP_FLAGS_HAS_FRAGS;
>  }
>  
> -static __always_inline bool xdp_buff_is_frag_pfmemalloc(struct xdp_buff *xdp)
> +static __always_inline bool
> +xdp_buff_is_frag_pfmemalloc(const struct xdp_buff *xdp)
>  {
>  	return !!(xdp->flags & XDP_FLAGS_FRAGS_PF_MEMALLOC);
>  }
> @@ -144,15 +145,16 @@ xdp_prepare_buff(struct xdp_buff *xdp, unsigned char *hard_start,
>  	 SKB_DATA_ALIGN(sizeof(struct skb_shared_info)))
>  
>  static inline struct skb_shared_info *
> -xdp_get_shared_info_from_buff(struct xdp_buff *xdp)
> +xdp_get_shared_info_from_buff(const struct xdp_buff *xdp)
>  {
>  	return (struct skb_shared_info *)xdp_data_hard_end(xdp);
>  }
>  
> -static __always_inline unsigned int xdp_get_buff_len(struct xdp_buff *xdp)
> +static __always_inline unsigned int
> +xdp_get_buff_len(const struct xdp_buff *xdp)
>  {
>  	unsigned int len = xdp->data_end - xdp->data;
> -	struct skb_shared_info *sinfo;
> +	const struct skb_shared_info *sinfo;
>  
>  	if (likely(!xdp_buff_has_frags(xdp)))
>  		goto out;
> @@ -177,12 +179,13 @@ struct xdp_frame {
>  	u32 flags; /* supported values defined in xdp_buff_flags */
>  };
>  
> -static __always_inline bool xdp_frame_has_frags(struct xdp_frame *frame)
> +static __always_inline bool xdp_frame_has_frags(const struct xdp_frame *frame)
>  {
>  	return !!(frame->flags & XDP_FLAGS_HAS_FRAGS);
>  }
>  
> -static __always_inline bool xdp_frame_is_frag_pfmemalloc(struct xdp_frame *frame)
> +static __always_inline bool
> +xdp_frame_is_frag_pfmemalloc(const struct xdp_frame *frame)
>  {
>  	return !!(frame->flags & XDP_FLAGS_FRAGS_PF_MEMALLOC);
>  }
> @@ -201,7 +204,7 @@ static __always_inline void xdp_frame_bulk_init(struct xdp_frame_bulk *bq)
>  }
>  
>  static inline struct skb_shared_info *
> -xdp_get_shared_info_from_frame(struct xdp_frame *frame)
> +xdp_get_shared_info_from_frame(const struct xdp_frame *frame)
>  {
>  	void *data_hard_start = frame->data - frame->headroom - sizeof(*frame);
>  
> @@ -249,7 +252,8 @@ int xdp_alloc_skb_bulk(void **skbs, int n_skb, gfp_t gfp);
>  struct xdp_frame *xdpf_clone(struct xdp_frame *xdpf);
>  
>  static inline
> -void xdp_convert_frame_to_buff(struct xdp_frame *frame, struct xdp_buff *xdp)
> +void xdp_convert_frame_to_buff(const struct xdp_frame *frame,
> +			       struct xdp_buff *xdp)
>  {
>  	xdp->data_hard_start = frame->data - frame->headroom - sizeof(*frame);
>  	xdp->data = frame->data;
> @@ -260,7 +264,7 @@ void xdp_convert_frame_to_buff(struct xdp_frame *frame, struct xdp_buff *xdp)
>  }
>  
>  static inline
> -int xdp_update_frame_from_buff(struct xdp_buff *xdp,
> +int xdp_update_frame_from_buff(const struct xdp_buff *xdp,
>  			       struct xdp_frame *xdp_frame)
>  {
>  	int metasize, headroom;
> @@ -317,9 +321,10 @@ void xdp_flush_frame_bulk(struct xdp_frame_bulk *bq);
>  void xdp_return_frame_bulk(struct xdp_frame *xdpf,
>  			   struct xdp_frame_bulk *bq);
>  
> -static __always_inline unsigned int xdp_get_frame_len(struct xdp_frame *xdpf)
> +static __always_inline unsigned int
> +xdp_get_frame_len(const struct xdp_frame *xdpf)
>  {
> -	struct skb_shared_info *sinfo;
> +	const struct skb_shared_info *sinfo;
>  	unsigned int len = xdpf->len;
>  
>  	if (likely(!xdp_frame_has_frags(xdpf)))
> diff --git a/include/net/xdp_sock_drv.h b/include/net/xdp_sock_drv.h
> index 40085afd9160..f3175a5d28f7 100644
> --- a/include/net/xdp_sock_drv.h
> +++ b/include/net/xdp_sock_drv.h
> @@ -101,7 +101,7 @@ static inline struct xdp_buff *xsk_buff_alloc(struct xsk_buff_pool *pool)
>  	return xp_alloc(pool);
>  }
>  
> -static inline bool xsk_is_eop_desc(struct xdp_desc *desc)
> +static inline bool xsk_is_eop_desc(const struct xdp_desc *desc)
>  {
>  	return !xp_mb_desc(desc);
>  }
> @@ -143,7 +143,7 @@ static inline void xsk_buff_add_frag(struct xdp_buff *xdp)
>  	list_add_tail(&frag->list_node, &frag->pool->xskb_list);
>  }
>  
> -static inline struct xdp_buff *xsk_buff_get_frag(struct xdp_buff *first)
> +static inline struct xdp_buff *xsk_buff_get_frag(const struct xdp_buff *first)
>  {
>  	struct xdp_buff_xsk *xskb = container_of(first, struct xdp_buff_xsk, xdp);
>  	struct xdp_buff *ret = NULL;
> @@ -200,7 +200,8 @@ static inline void *xsk_buff_raw_get_data(struct xsk_buff_pool *pool, u64 addr)
>  		XDP_TXMD_FLAGS_CHECKSUM | \
>  	0)
>  
> -static inline bool xsk_buff_valid_tx_metadata(struct xsk_tx_metadata *meta)
> +static inline bool
> +xsk_buff_valid_tx_metadata(const struct xsk_tx_metadata *meta)
>  {
>  	return !(meta->flags & ~XDP_TXMD_FLAGS_VALID);
>  }
> @@ -337,7 +338,7 @@ static inline struct xdp_buff *xsk_buff_alloc(struct xsk_buff_pool *pool)
>  	return NULL;
>  }
>  
> -static inline bool xsk_is_eop_desc(struct xdp_desc *desc)
> +static inline bool xsk_is_eop_desc(const struct xdp_desc *desc)
>  {
>  	return false;
>  }
> @@ -360,7 +361,7 @@ static inline void xsk_buff_add_frag(struct xdp_buff *xdp)
>  {
>  }
>  
> -static inline struct xdp_buff *xsk_buff_get_frag(struct xdp_buff *first)
> +static inline struct xdp_buff *xsk_buff_get_frag(const struct xdp_buff *first)
>  {
>  	return NULL;
>  }
> diff --git a/include/net/xsk_buff_pool.h b/include/net/xsk_buff_pool.h
> index bb03cee716b3..3832997cc605 100644
> --- a/include/net/xsk_buff_pool.h
> +++ b/include/net/xsk_buff_pool.h
> @@ -183,7 +183,7 @@ static inline bool xp_desc_crosses_non_contig_pg(struct xsk_buff_pool *pool,
>  	       !(pool->dma_pages[addr >> PAGE_SHIFT] & XSK_NEXT_PG_CONTIG_MASK);
>  }
>  
> -static inline bool xp_mb_desc(struct xdp_desc *desc)
> +static inline bool xp_mb_desc(const struct xdp_desc *desc)
>  {
>  	return desc->options & XDP_PKT_CONTD;
>  }
> -- 
> 2.46.2
>