Re: [BUG] can: RX frames silently dropped in raw_rcv() with RPS enabled since d4fb6514ff8e

[Oliver Hartkopp <socketcan@hartkopp.net>]

Hi Joerg,

many thanks for the report!

On 28.09.26 13:55, Jörg Willmann wrote:

> since commit d4fb6514ff8e ("can: use skb hash instead of private
>   variable in headroom") we are losing received CAN frames on raw sockets
>   when RPS is enabled on the CAN interface. The problem also shows up in
>   the stable backports (seen on [6.12.109]).
> 
>   Setup:
>   - Kernel: 6.12.109
>   - Platform: OpenWrt 25.12.5, 4 CPU cores
>   - CAN controller / driver: mcp251xfd
>   - OpenWrt enables packet steering (RPS) by default on multi-core
>   systems, so rps_cpus is non-zero on can0
> 
>   Symptom:
>   The application on a CAN_RAW socket misses frames. /proc/net/can/stats
>   counts more RX frames than the application receives. With
> 
>   echo 0 > /sys/class/net/can0/queues/rx-0/rps_cpus
> 
>   the losses disappear completely. [Optional: rate/amount of loss, number
>   of filters on the socket, whether join_filters is used.]
> 
>   Analysis (as far as I understand it):
>   1. RX skbs from the driver start with skb->hash == 0.
>   2. With RPS enabled, get_rps_cpu() calls skb_get_hash() in the
>   netif_rx() path before can_rcv() is reached. The flow dissector
>   finds no L3/L4 headers in CAN skbs, so every CAN frame gets the
>   same non-zero hash with sw_hash = 1.

Ouch! I wasn't aware of this.

>   3. can_set_skb_uid() only assigns a new counter value while
>   skb->hash is 0, so the RPS hash is kept and the frame gets no
>   unique identifier.

Correct! That's bad and not intended here.

>   4. raw_rcv() treats a frame as a duplicate when both the skb pointer
>   and skb->hash match the per-CPU uniq entry. Since the slab
>   allocator often returns the same address for the next skb, a new
>   frame with the same (constant) hash is dropped as a duplicate.

Right!

>   Before d4fb6514ff8e this could not happen because the unique id was
>   kept in can_skb_priv::skbcnt, independent of skb->hash.

Yes. We won't get can_skb_priv::skbcnt back but I will take a look at it.

Best regards,
Oliver