From: Oliver Hartkopp <socketcan@hartkopp.net>
To: Manfred Schlaegl <manfred.schlaegl@gmx.at>,
Wolfgang Grandegger <wg@grandegger.com>,
Marc Kleine-Budde <mkl@pengutronix.de>
Cc: linux-can@vger.kernel.org, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org,
Manfred Schlaegl <manfred.schlaegl@ginzinger.com>,
"David S. Miller" <davem@davemloft.net>
Subject: Re: [PATCH] can: fix loss of frames due to wrong assumption in raw_rcv
Date: Mon, 22 Jun 2015 12:24:42 +0200 [thread overview]
Message-ID: <5587E26A.1070000@hartkopp.net> (raw)
In-Reply-To: <5587D9DA.6000102@gmx.at>
Hello Manfred,
On 22.06.2015 11:48, Manfred Schlaegl wrote:
>> Can you tell me the output of /proc/sys/net/core/netdev_tstamp_prequeue on
>> your machine?
>
> /proc/sys/net/core/netdev_tstamp_prequeue is set to 1 (unmodified, default)
>
> I tried to dig a little deeper in timestamping:
> 1. (net/core/dev.c) I found that static_key_false(&netstamp_needed) is always 0, resulting that the timestamp is never set by net_timestamp_check in netif_receive_skb_internal.
> 2. (net/core/dev.c) static_key_false(&netstamp_needed) is 0 because net_enable_timestamp is never called.
> 3. (net/core/sock.c) net_enable_timestamp is never called because SK_FLAGS_TIMESTAMP is not set
> 4. (net/core/sock.c) SK_FLAGS_TIMESTAMP is not set because neither of SOCK_TIMESTAMP or SOCK_TIMESTAMPING_RX_SOFTWARE is set
> 5. (net/core/sock.c) SOCK_TIMESTAMP or SOCK_TIMESTAMPING_RX_SOFTWARE is not set because timestamping is an optional feature (according to http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/Documentation/networking/timestamping.txt?id=b953c0d234bc72e8489d3bf51a276c5c4ec85345) not enabled in my use case (even if netdev_tstamp_prequeue is set to 1)
>
> So the original assumption for the was correct: The correctness of the skb equality check depends on a feature that is not enabled by default (respectively user configurable).
> Do you agree with this?
Yes.
But the point becomes an issue when there's no userspace application that
requires timestamps.
I did my testing wile having at least one "candump" instances running, which
enables timestamping. So when there's no one requesting timestamps the check
in can_rcv does not perform properly.
Therefor my patch grabs your idea to set the timestamps for CAN skbs
unconditionally. But there were some more places in the code where we need to
take care about that.
Regards,
Oliver
--
To unsubscribe from this list: send the line "unsubscribe linux-can" in
next prev parent reply other threads:[~2015-06-22 10:24 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-06-20 17:21 [PATCH] can: fix loss of frames due to wrong assumption in raw_rcv Manfred Schlaegl
2015-06-20 22:42 ` Oliver Hartkopp
2015-06-22 9:48 ` Manfred Schlaegl
2015-06-22 10:24 ` Oliver Hartkopp [this message]
-- strict thread matches above, loose matches on Subject: below --
2015-06-20 16:24 manfred.schlaegl
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=5587E26A.1070000@hartkopp.net \
--to=socketcan@hartkopp.net \
--cc=davem@davemloft.net \
--cc=linux-can@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=manfred.schlaegl@ginzinger.com \
--cc=manfred.schlaegl@gmx.at \
--cc=mkl@pengutronix.de \
--cc=netdev@vger.kernel.org \
--cc=wg@grandegger.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).