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

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

[Jörg Willmann]

Hi Oliver, hi Marc,

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.
 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.
 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.

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

 Thanks,
 Joerg

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

[Oliver Hartkopp]

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

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

[Oliver Hartkopp]

Hi Jörg!

I've sent out a v2 patch set where the third patch addresses your 
reported RPS related bug.

https://patchwork.kernel.org/series/1175458/

The third patch needs patch 2 as both are changing code in can*_rcv.

Can you please test the patch set in your setup?

Many thanks,
Oliver

On 28.09.26 14:16, Oliver Hartkopp wrote:
> 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
> 
>