Re: [PATCH net-next] llc: avoid sparse cast-truncates warning in counter clamps

[David Laight]

On Wed, 13 May 2026 14:52:53 +0530
Avinash Duduskar <avinash.duduskar@xxxxxxxxx> wrote:

> llc_conn_ac_inc_npta_value() and llc_conn_ac_inc_tx_win_size()
> clamp their counters to the maximum valid 7-bit value via
> (u8) ~LLC_2_SEQ_NBR_MODULO. LLC_2_SEQ_NBR_MODULO is defined as
> ((u8) 128) in include/net/llc_pdu.h,

That (u8) cast is entirely pointless.
Whenever the value is used it is subject to integer promotion
and becomes a signed 128.

> but the (u8) cast does not
> prevent integer promotion of the operand of ~: ~128 is computed
> as int (0xffffff7f), and the surrounding (u8) cast truncates
> back to 0x7f. The result is correct (127), but the implicit
> truncation is flagged by sparse:
> 
>   net/llc/llc_c_ac.c:1008:38: warning: cast truncates bits from
>       constant value (ffffff7f becomes 7f)
>   (and three more at lines 1009, 1099, 1100)
> 
> Replace the (u8) ~LLC_2_SEQ_NBR_MODULO expression with
> LLC_2_SEQ_NBR_MODULO - 1, which evaluates to 127 directly and
> silences sparse.
> 
> The same ~LLC_2_SEQ_NBR_MODULO pattern also appears in
> include/net/llc_pdu.h:148 as part of PDU_GET_NEXT_Vr, but there
> the result is immediately &-masked, so the int promotion is
> harmless and sparse does not flag it; it is left alone.
> 
> This patch is the minimum diff to silence the warning. The
> counter-clamp idiom itself could be modernized to
> min_t(u8, ..., LLC_2_SEQ_NBR_MODULO - 1), but that is a
> separate cleanup left for another patch.

Using min_t() (especially with u8 or u16) is a very bad idea.
Consider what would would happen if the limit were 255.

> 
> No functional change.
> 
> Signed-off-by: Avinash Duduskar <avinash.duduskar@xxxxxxxxx>
> ---
> Built with CONFIG_LLC2=m; full kernel build clean, sparse no
> longer warns on the touched lines.
> 
>  net/llc/llc_c_ac.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/net/llc/llc_c_ac.c b/net/llc/llc_c_ac.c
> index 0779daa8aa8f..ab86c720b3ec 100644
> --- a/net/llc/llc_c_ac.c
> +++ b/net/llc/llc_c_ac.c
> @@ -1005,8 +1005,8 @@ static int llc_conn_ac_inc_npta_value(struct sock *sk, struct sk_buff *skb)
>  		llc->dec_step = 0;
>  		llc->dec_cntr = llc->inc_cntr = 2;
>  		++llc->npta;
> -		if (llc->npta > (u8) ~LLC_2_SEQ_NBR_MODULO)
> -			llc->npta = (u8) ~LLC_2_SEQ_NBR_MODULO;
> +		if (llc->npta > LLC_2_SEQ_NBR_MODULO - 1)
> +			llc->npta = LLC_2_SEQ_NBR_MODULO - 1;

If you rewrite it, do the check before the increment.
The compiler doesn't know that llc->npta isn't 255 - when the result
of the above is 0. That can generate rather more code.
Probably best to do:
		if (llc->npta < LLC_2_SEQ_NBR_MODULO - 1)
			llc->npta++;

-- David

>  	} else
>  		--llc->inc_cntr;
>  	return 0;
> @@ -1096,8 +1096,8 @@ int llc_conn_ac_inc_tx_win_size(struct sock *sk, struct sk_buff *skb)
>  	struct llc_sock *llc = llc_sk(sk);
>  
>  	llc->k += 1;
> -	if (llc->k > (u8) ~LLC_2_SEQ_NBR_MODULO)
> -		llc->k = (u8) ~LLC_2_SEQ_NBR_MODULO;
> +	if (llc->k > LLC_2_SEQ_NBR_MODULO - 1)
> +		llc->k = LLC_2_SEQ_NBR_MODULO - 1;
>  	return 0;
>  }
>  
> 
> base-commit: 8ebd24a7822cbae25beeafba49b2159d6a68a5f2