AW: [PATCH] can: isotp: omit unintended hrtimer restart on socket release
From: Sven Schuchmann <hidden>
Date: 2021-08-28 13:20:26
Hello, sorry, I'm late for the party :-) But I found that this patch decreases the performance of ISO-TP Stack. 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? Sven 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
--- isotp_test.sh ---#!/bin/bash
databyte=01
numbytes=1000
iface=vcan0
echo "Test iso-tp with $numbytes byte frames on $iface (data:$databyte)"
message="01 " # invalid service 01
for (( i=1; i<=$numbytes; i++ ))
do
message="$message$databyte "
done
i=1
timemin=10000
timemax=0
timesum=0
while true; do
ts=$(date +%s%N)
exec 3< <(isotprecv -s 714 -d 77E -b 5 $iface)
rxpid=$!
echo $message | isotpsend -s 714 -d 77E -p AA:AA -L 16:8:0 $iface
wait $rxpid
output=$(cat <&3)
timediff=$((($(date +%s%N) - $ts)/1000000))
if [ "7F 01 11 " != "$output" ]; then
echo "ERROR: $i >$output<"
break
fi
if [ $timediff -gt $timemax ]; then
timemax=$timediff
fi
if [ $timediff -lt $timemin ]; then
timemin=$timediff
fi
((timesum=timesum+timediff))
timeavg=$(echo "$timesum / $i" | bc -l)
printf "%5d / curr:%5d / min:%5d / max:%5d / avg:%7.1f\n" $i $timediff $timemin $timemax $timeavg
((i=i+1))
done
------
Sven Schuchmann
Schleißheimer Soft- und
Hardwareentwicklung GmbH
Am Kalkofen 10
61206 Nieder-Wöllstadt
GERMANY
Phone: +49 6034 9148 711
Fax: +49 6034 9148 91
Email: schuchmann@schleissheimer.de
Court of Registration: Amtsgericht Friedberg
Registration Number: HRB 1581
Management Board:
Hans-Joachim Schleißheimer
Christine Schleißheimer
quoted hunk ↗ jump to hunk
-----Ursprüngliche Nachricht----- Von: Oliver Hartkopp [off-list ref] Gesendet: Freitag, 18. Juni 2021 19:37 An: linux-can@vger.kernel.org Cc: Oliver Hartkopp <socketcan@hartkopp.net> Betreff: [PATCH] can: isotp: omit unintended hrtimer restart on socket release When closing the isotp socket the potentially running hrtimers are canceled before removing the subscription for CAN idendifiers via can_rx_unregister(). This may lead to an unintended (re)start of a hrtimer in isotp_rcv_cf() and isotp_rcv_fc() in the case that a CAN frame is received by isotp_rcv() while the subscription removal is processed. However, isotp_rcv() is called under RCU protection, so after calling can_rx_unregister, we may call synchronize_rcu in order to wait for any RCU read-side critical sections to finish. This prevents the reception of CAN frames after hrtimer_cancel() and therefore the unintended (re)start of the hrtimers. Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net> --- net/can/isotp.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-)diff --git a/net/can/isotp.c b/net/can/isotp.c index be6183f8ca11..234cc4ad179a 100644 --- a/net/can/isotp.c +++ b/net/can/isotp.c@@ -1026,13 +1026,10 @@ static int isotp_release(struct socket *sock) list_del(&so->notifier); spin_unlock(&isotp_notifier_lock); lock_sock(sk); - hrtimer_cancel(&so->txtimer); - hrtimer_cancel(&so->rxtimer); - /* remove current filters & unregister */ if (so->bound && (!(so->opt.flags & CAN_ISOTP_SF_BROADCAST))) { if (so->ifindex) { struct net_device *dev;@@ -1040,14 +1037,18 @@ static int isotp_release(struct socket *sock) if (dev) { can_rx_unregister(net, dev, so->rxid, SINGLE_MASK(so->rxid), isotp_rcv, sk); dev_put(dev); + synchronize_rcu(); } } } + hrtimer_cancel(&so->txtimer); + hrtimer_cancel(&so->rxtimer); + so->ifindex = 0; so->bound = 0; sock_orphan(sk); sock->sk = NULL; --2.30.2