linux-can.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Sven Schuchmann <schuchmann@schleissheimer.de>
To: Sven Schuchmann <schuchmann@schleissheimer.de>,
	Oliver Hartkopp <socketcan@hartkopp.net>,
	"linux-can@vger.kernel.org" <linux-can@vger.kernel.org>
Cc: Marc Kleine-Budde <mkl@pengutronix.de>
Subject: AW: AW: AW: [PATCH] can: isotp: omit unintended hrtimer restart on socket release
Date: Mon, 30 Aug 2021 07:55:59 +0000	[thread overview]
Message-ID: <DB8P190MB0634D4408A4A57A74134E698D9CB9@DB8P190MB0634.EURP190.PROD.OUTLOOK.COM> (raw)
In-Reply-To: <DB8P190MB0634C90D304A2AA97F481738D9CA9@DB8P190MB0634.EURP190.PROD.OUTLOOK.COM>

Hello Oliver,

> > >>> bring up a vcan0 interface with:
> > >>> sudo modprobe vcan
> > >>> sudo ip link add dev vcan0 type vcan
> > >>> sudo ifconfig vcan0 up
> > >>>
> > >>> here are the scripts:
> > >>> --- isotp_server.sh ---
> > >>> #!/bin/bash
> > >>> iface=vcan0
> > >>> echo "Wait for Messages on $iface"
> > >>> while true; do
> > >>>       exec 3< <(isotprecv -s 77E -d 714 -b F -p AA:AA $iface)
> > >>>       rxpid=$!
> > >>>       wait $rxpid
> > >>>       output=$(cat <&3)
> > >>>       echo "7F 01 11" | isotpsend -s 77E -d 714 -p AA:AA -L 16:8:0 $iface
> > >>> done
> > >>
> > >> IMO the issue arises with the use of isotpsend and isotprecv.
> > >> These tools are intended to get a hands-on impression how the isotp
> > >> stack works.
> > >>
> > >> This kind of use in a script leads to the creation and (now delayed)
> > >> *removal* of isotp sockets for *each* single PDU transfer.
> > >
> > > Maybe I am wrong but I see something different.
> > > e.g. without this patch:
> > >   (000.000240)  canfd0  714   [8]  2B 01 01 01 01 01 01 01
> > >   (000.000261)  canfd0  77E   [8]  30 0F 00 AA AA AA AA AA
> > >   (000.000496)  canfd0  714   [8]  2C 01 01 01 01 01 01 01
> > >
> > > and with this patch:
> > >   (000.000414)  canfd0  714   [8]  2B 01 01 01 01 01 01 01
> > >   (000.000262)  canfd0  77E   [8]  30 0F 00 AA AA AA AA AA
> > >   (000.001536)  canfd0  714   [8]  2C 01 01 01 01 01 01 01
> > >
> >
> > I'm running a 5.14.0-rc7-00011-g6e764bcd1cf7 kernel here and see this:
> >
> >   (000.000001)  vcan0  714   [8]  2B 01 01 01 01 01 01 01
> >   (000.000015)  vcan0  77E   [8]  30 0F 00 AA AA AA AA AA
> >   (000.000005)  vcan0  714   [8]  2C 01 01 01 01 01 01 01
> >
> > Test iso-tp with 1000 byte frames on vcan0 (data:01)
> >      1 / curr:   40 / min:   40 / max:   40 / avg:   40.0
> >      2 / curr:   30 / min:   30 / max:   40 / avg:   35.0
> >      3 / curr:   35 / min:   30 / max:   40 / avg:   35.0
> >      4 / curr:   52 / min:   30 / max:   52 / avg:   39.2
> >      5 / curr:   40 / min:   30 / max:   52 / avg:   39.4
> > (..)
> >
> > when running your scripts from the initial post.
> >
> > Is you canfd0 interface a real hardware?
> >
> Yes, the canfd0 interface is a real hardware (a MCP2518FD connected
> to a RaspberryPi-4-Compute-Module).
> 
> I am running a 5.10.60 kernel.
> With using the testscripts I provided I can see this:
> with the patch:
> 
>     1 / curr:  147 / min:  147 / max:  147 / avg:  147.0
>     2 / curr:  107 / min:  107 / max:  147 / avg:  127.0
>     3 / curr:  118 / min:  107 / max:  147 / avg:  124.0
>     4 / curr:  138 / min:  107 / max:  147 / avg:  127.5
>     5 / curr:  115 / min:  107 / max:  147 / avg:  125.0
>     6 / curr:  158 / min:  107 / max:  158 / avg:  130.5
>     7 / curr:  140 / min:  107 / max:  158 / avg:  131.9
> 
> and without the patch:
> 
>    39 / curr:   42 / min:   28 / max:   45 / avg:   30.9
>    40 / curr:   41 / min:   28 / max:   45 / avg:   31.2
>    41 / curr:   28 / min:   28 / max:   45 / avg:   31.1
>    42 / curr:   29 / min:   28 / max:   45 / avg:   31.0
>    43 / curr:   31 / min:   28 / max:   45 / avg:   31.0
>    44 / curr:   28 / min:   28 / max:   45 / avg:   31.0
> 
> but if I compare the candumps I can see:
> with the patch:
> 
>  (000.000008)  vcan0  714   [8]  2F 01 01 01 01 01 01 01
>  (000.000209)  vcan0  77E   [8]  30 0F 00 AA AA AA AA AA
>  (000.000061)  vcan0  714   [8]  20 01 01 01 01 01 01 01
> 
> and without:
> 
>  (000.000004)  vcan0  714   [8]  2F 01 01 01 01 01 01 01
>  (000.000069)  vcan0  77E   [8]  30 0F 00 AA AA AA AA AA
>  (000.000017)  vcan0  714   [8]  20 01 01 01 01 01 01 01
> 
> sorry, I missed that: Over here the delay seems to be in
> the FC and not in the CF after the FC. This is what is
> different compared to the real hardware.
> 
> So to me it seems that the rcu implementation
> has changed on the way from 5.10 to 5.14?

Just checked with a 5.14.0-rc6 which contains the patch, same result:

   93 / curr:  143 / min:  129 / max:  200 / avg:  156.2
   94 / curr:  144 / min:  129 / max:  200 / avg:  156.0
   95 / curr:  141 / min:  129 / max:  200 / avg:  155.9
   96 / curr:  171 / min:  129 / max:  200 / avg:  156.0
   97 / curr:  138 / min:  129 / max:  200 / avg:  155.8
   98 / curr:  137 / min:  129 / max:  200 / avg:  155.6

 (000.000011)  vcan0  714   [8]  2B 01 01 01 01 01 01 01
 (000.000193)  vcan0  77E   [8]  30 0F 00 AA AA AA AA AA
 (000.000037)  vcan0  714   [8]  2C 01 01 01 01 01 01 01

So maybe there is something wrong on the rpi?

Sven


  reply	other threads:[~2021-08-30  7:56 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-06-18 17:37 [PATCH] can: isotp: omit unintended hrtimer restart on socket release Oliver Hartkopp
2021-06-19 21:36 ` Marc Kleine-Budde
2021-08-28 13:20 ` AW: " Sven Schuchmann
2021-08-29 11:17   ` Oliver Hartkopp
2021-08-29 18:28     ` AW: " Sven Schuchmann
2021-08-29 20:14       ` Oliver Hartkopp
2021-08-29 20:28         ` Marc Kleine-Budde
2021-08-29 21:09         ` AW: AW: AW: " Sven Schuchmann
2021-08-30  7:55           ` Sven Schuchmann [this message]
2021-08-30 12:44             ` Oliver Hartkopp

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=DB8P190MB0634D4408A4A57A74134E698D9CB9@DB8P190MB0634.EURP190.PROD.OUTLOOK.COM \
    --to=schuchmann@schleissheimer.de \
    --cc=linux-can@vger.kernel.org \
    --cc=mkl@pengutronix.de \
    --cc=socketcan@hartkopp.net \
    /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).