Re: [PATCH net-next] ppp: don't byte-swap at run time

[Paolo Abeni <pabeni@redhat.com>]

On 2/7/26 7:47 AM, Qingfang Deng wrote:
> @@ -312,7 +312,26 @@ static inline struct ppp_net *ppp_pernet(struct net *net)
>  }
>  
>  /* Translates a PPP protocol number to a NP index (NP == network protocol) */
> -static inline int proto_to_npindex(int proto)
> +static __always_inline int proto_to_npindex(__be16 proto)

This has just 2 callers; does the compiler inline it anyway?!?

> +{
> +	switch (proto) {
> +	case htons(PPP_IP):
> +		return NP_IP;
> +	case htons(PPP_IPV6):
> +		return NP_IPV6;
> +	case htons(PPP_IPX):
> +		return NP_IPX;
> +	case htons(PPP_AT):
> +		return NP_AT;
> +	case htons(PPP_MPLS_UC):
> +		return NP_MPLS_UC;
> +	case htons(PPP_MPLS_MC):
> +		return NP_MPLS_MC;
> +	}
> +	return -EINVAL;
> +}
> +
> +static __always_inline int proto_to_npindex_user(int proto)

This is slowpath and definitely does not need the inline annotation

>  {
>  	switch (proto) {
>  	case PPP_IP:
> @@ -332,44 +351,44 @@ static inline int proto_to_npindex(int proto)
>  }
>  
>  /* Translates an NP index into a PPP protocol number */
> -static const int npindex_to_proto[NUM_NP] = {
> -	PPP_IP,
> -	PPP_IPV6,
> -	PPP_IPX,
> -	PPP_AT,
> -	PPP_MPLS_UC,
> -	PPP_MPLS_MC,
> +static const __be16 npindex_to_proto[NUM_NP] = {
> +	htons(PPP_IP),
> +	htons(PPP_IPV6),
> +	htons(PPP_IPX),
> +	htons(PPP_AT),
> +	htons(PPP_MPLS_UC),
> +	htons(PPP_MPLS_MC),
>  };
>  
>  /* Translates an ethertype into an NP index */
> -static inline int ethertype_to_npindex(int ethertype)
> +static inline int ethertype_to_npindex(__be16 ethertype)

This has a single caller, please drop the inline annotation.

I'm not sure the code churn is justified by the gain; please include
also actual perf figures in the commit message.

If you are willing to invest a significant amount of time in this area,
I suggest to implement first some self-tests and than reconsider the
locking schema: I suspect RCU usage could avoid some lock(s) in the
datapath.

/P