From: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
To: David Laight <David.Laight@ACULAB.COM>,
"netdev@vger.kernel.org" <netdev@vger.kernel.org>
Cc: Neil Horman <nhorman@tuxdriver.com>,
Vlad Yasevich <vyasevich@gmail.com>,
"linux-sctp@vger.kernel.org" <linux-sctp@vger.kernel.org>
Subject: Re: [PATCH] sctp: avoid refreshing heartbeat timer too often
Date: Wed, 30 Mar 2016 12:13:18 +0000 [thread overview]
Message-ID: <56FBC2DE.3000207@gmail.com> (raw)
In-Reply-To: <063D6719AE5E284EB5DD2968C1650D6D54DBFFCD@AcuExch.aculab.com>
Em 30-03-2016 06:37, David Laight escreveu:
> From: Marcelo Ricardo Leitner
>> Sent: 29 March 2016 14:42
>>
>> Currently on high rate SCTP streams the heartbeat timer refresh can
>> consume quite a lot of resources as timer updates are costly and it
>> contains a random factor, which a) is also costly and b) invalidates
>> mod_timer() optimization for not editing a timer to the same value.
>> It may even cause the timer to be slightly advanced, for no good reason.
>
> Interesting thoughts:
> 1) Is it necessary to use a different 'random factor' until the timer actually
> expires?
I don't understand you fully here, but we have to have a random factor
on timer expire. As noted by Daniel Borkmann on his commit 8f61059a96c2
("net: sctp: improve timer slack calculation for transport HBs"):
RFC4960, section 8.3 says:
On an idle destination address that is allowed to heartbeat,
it is recommended that a HEARTBEAT chunk is sent once per RTO
of that destination address plus the protocol parameter
'HB.interval', with jittering of +/- 50% of the RTO value,
and exponential backoff of the RTO if the previous HEARTBEAT
is unanswered.
Previous to his commit, it was using a random factor based on jiffies.
This patch then assumes that random_A+2 is just as random as random_B as
long as it is within the allowed range, avoiding the unnecessary updates.
> 2) It might be better to allow the heartbeat timer to expire, on expiry work
> out the new interval based on when the last 'refresh' was done.
Cool, I thought about this too. It would introduce some extra complexity
that is not really worth I think, specially because now we may be doing
more timer updates even with this patch but it's not triggering any wake
ups and we would need at least 2 wake ups then: one for the first
timeout event, and then re-schedule the timer for the next updated one,
and maybe again, and again.. less timer updates but more wake ups, one
at every heartbeat interval even on a busy transport. Seems it's cheaper
to just update the timer then.
Thanks,
Marcelo
WARNING: multiple messages have this Message-ID (diff)
From: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
To: David Laight <David.Laight@ACULAB.COM>,
"netdev@vger.kernel.org" <netdev@vger.kernel.org>
Cc: Neil Horman <nhorman@tuxdriver.com>,
Vlad Yasevich <vyasevich@gmail.com>,
"linux-sctp@vger.kernel.org" <linux-sctp@vger.kernel.org>
Subject: Re: [PATCH] sctp: avoid refreshing heartbeat timer too often
Date: Wed, 30 Mar 2016 09:13:18 -0300 [thread overview]
Message-ID: <56FBC2DE.3000207@gmail.com> (raw)
In-Reply-To: <063D6719AE5E284EB5DD2968C1650D6D54DBFFCD@AcuExch.aculab.com>
Em 30-03-2016 06:37, David Laight escreveu:
> From: Marcelo Ricardo Leitner
>> Sent: 29 March 2016 14:42
>>
>> Currently on high rate SCTP streams the heartbeat timer refresh can
>> consume quite a lot of resources as timer updates are costly and it
>> contains a random factor, which a) is also costly and b) invalidates
>> mod_timer() optimization for not editing a timer to the same value.
>> It may even cause the timer to be slightly advanced, for no good reason.
>
> Interesting thoughts:
> 1) Is it necessary to use a different 'random factor' until the timer actually
> expires?
I don't understand you fully here, but we have to have a random factor
on timer expire. As noted by Daniel Borkmann on his commit 8f61059a96c2
("net: sctp: improve timer slack calculation for transport HBs"):
RFC4960, section 8.3 says:
On an idle destination address that is allowed to heartbeat,
it is recommended that a HEARTBEAT chunk is sent once per RTO
of that destination address plus the protocol parameter
'HB.interval', with jittering of +/- 50% of the RTO value,
and exponential backoff of the RTO if the previous HEARTBEAT
is unanswered.
Previous to his commit, it was using a random factor based on jiffies.
This patch then assumes that random_A+2 is just as random as random_B as
long as it is within the allowed range, avoiding the unnecessary updates.
> 2) It might be better to allow the heartbeat timer to expire, on expiry work
> out the new interval based on when the last 'refresh' was done.
Cool, I thought about this too. It would introduce some extra complexity
that is not really worth I think, specially because now we may be doing
more timer updates even with this patch but it's not triggering any wake
ups and we would need at least 2 wake ups then: one for the first
timeout event, and then re-schedule the timer for the next updated one,
and maybe again, and again.. less timer updates but more wake ups, one
at every heartbeat interval even on a busy transport. Seems it's cheaper
to just update the timer then.
Thanks,
Marcelo
next prev parent reply other threads:[~2016-03-30 12:13 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-03-29 13:41 [PATCH] sctp: avoid refreshing heartbeat timer too often Marcelo Ricardo Leitner
2016-03-29 13:41 ` Marcelo Ricardo Leitner
2016-03-30 9:37 ` David Laight
2016-03-30 12:13 ` Marcelo Ricardo Leitner [this message]
2016-03-30 12:13 ` Marcelo Ricardo Leitner
2016-03-31 11:16 ` David Laight
2016-03-31 21:25 ` 'Marcelo Ricardo Leitner'
2016-03-31 21:25 ` 'Marcelo Ricardo Leitner'
2016-04-01 0:24 ` Marcelo Ricardo Leitner
2016-04-01 0:24 ` Marcelo Ricardo Leitner
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=56FBC2DE.3000207@gmail.com \
--to=marcelo.leitner@gmail.com \
--cc=David.Laight@ACULAB.COM \
--cc=linux-sctp@vger.kernel.org \
--cc=netdev@vger.kernel.org \
--cc=nhorman@tuxdriver.com \
--cc=vyasevich@gmail.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.