Re: [PATCH - regression 4.1-rc8] can: fix loss of CAN frames in raw_rcv

From: Oliver Hartkopp
Date: Mon Jun 22 2015 - 06:35:21 EST


Hello Manfred,

On 22.06.2015 12:10, Manfred Schlaegl wrote:
On 2015-06-21 18:50, Oliver Hartkopp wrote:
As reported by Manfred Schlaegl here

http://marc.info/?l=linux-netdev&m=143482089824232&w=2

commit 514ac99c64b "can: fix multiple delivery of a single CAN frame for
overlapping CAN filters" requires the skb->tstamp to be set to check for
identical CAN skbs.

As net timestamping is influenced by several players (netstamp_needed and
netdev_tstamp_prequeue) Manfred missed a proper timestamp which leads to
CAN frame loss.

As skb timestamping became now mandatory for CAN related skbs this patch
makes sure that received CAN skbs always have a proper timestamp set.
Maybe there's a better solution in the future but this patch fixes the
CAN frame loss so far.


I'm not sure, but maybe this patch (and also my original one) opens a new potential issue with timestamps.

If the timestamp is set at allocation time, this cancels setting the timestamp at delivery (by net_timestamp_check in, for example, netif_receive_skb_internal.) -> So it changes the behavior of timestamping (http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/Documentation/networking/timestamping.txt?id=b953c0d234bc72e8489d3bf51a276c5c4ec85345) generally.

The only change is that the timestamps for CAN skbs are generated always.
The idea behind the timestamping by user control is to omit the timestamping when it's not needed. There's no user visible change in behaviour when the timestamp is set in the CAN skbs all time.

Hypothetical example: If timestamping is enabled by the user and there is a significant delay between allocation and delivery of a skb (early allocation in driver or something) the timestamp does not reflect the reception time anymore.

The change only affects CAN skbs.
These skbs are allocated at CAN frame reception time, filled with content and then sent to the network layer.

AFAICS the timestamp becomes more precise for CAN related skbs.
I did not see any case of 'early allocation' in linux/drivers/net/can, did you?

Best regards,
Oliver

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
Please read the FAQ at http://www.tux.org/lkml/