linux-can.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Oliver Hartkopp <socketcan@hartkopp.net>
To: Sven Schuchmann <schuchmann@schleissheimer.de>,
	"linux-can@vger.kernel.org" <linux-can@vger.kernel.org>
Cc: Marc Kleine-Budde <mkl@pengutronix.de>
Subject: Re: AW: [PATCH] can: isotp: omit unintended hrtimer restart on socket release
Date: Sun, 29 Aug 2021 13:17:55 +0200	[thread overview]
Message-ID: <abc23fd0-9bb1-1cc7-fc67-0a3298673b86@hartkopp.net> (raw)
In-Reply-To: <DB8P190MB06343614CAD6B48C3EA0EF00D9C99@DB8P190MB0634.EURP190.PROD.OUTLOOK.COM>

Hello Sven,

On 28.08.21 15:20, Sven Schuchmann wrote:

> sorry, I'm late for the party :-)

NP ;-)

> But I found that this patch decreases the performance of ISO-TP Stack.

AFAICS the performance (aka throughput) of the ISO-TP stack is not 
touched but the grace period when closing an ISO-TP socket is increased.

> I have created two testscripts where one plays the server and the
> other one is running a test and measuring the time how long
> it takes to transfer an ISO-TP Frame with 1000 Bytes.
> 
> Without this patch it takes about 35ms to transfer the frame,
> with this patch it takes about 145ms over vcan0.
> 
> Anyone an idea on this?

Yes. We now syncronize the removal of data structures to prevent a 
use-after-free issue at socket close time.

The synchronize_rcu() call does this job at specific times which leads 
to this extended time the close() syscall needs to perform.

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

The better approach would be to write a C program that creates ONE 
socket and simply read() from that socket and write() to it.

This should boost your performance even more.

Is the performance a real requirement for your use-case or is this 
decreased socket close rate a finding which does not really affect your 
work?

Best regards,
Oliver


  reply	other threads:[~2021-08-29 11:21 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 [this message]
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
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=abc23fd0-9bb1-1cc7-fc67-0a3298673b86@hartkopp.net \
    --to=socketcan@hartkopp.net \
    --cc=linux-can@vger.kernel.org \
    --cc=mkl@pengutronix.de \
    --cc=schuchmann@schleissheimer.de \
    /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).