Skip to content

Commit ba6f5e3

Browse files
marceloleitnerdavem330
marceloleitner
authored and
davem330
committed
sctp: avoid refreshing heartbeat timer too often
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. As suggested by David Laight this patch now removes this timer update from hot path by leaving the timer on and re-evaluating upon its expiration if the heartbeat is still needed or not, similarly to what is done for TCP. If it's not needed anymore the timer is re-scheduled to the new timeout, considering the time already elapsed. For this, we now record the last tx timestamp per transport, updated in the same spots as hb timer was restarted on tx. Also split up sctp_transport_reset_timers into sctp_transport_reset_t3_rtx and sctp_transport_reset_hb_timer, so we can re-arm T3 without re-arming the heartbeat one. On loopback with MTU of 65535 and data chunks with 1636, so that we have a considerable amount of chunks without stressing system calls, netperf -t SCTP_STREAM -l 30, perf looked like this before: Samples: 103K of event 'cpu-clock', Event count (approx.): 25833000000 Overhead Command Shared Object Symbol + 6,15% netperf [kernel.vmlinux] [k] copy_user_enhanced_fast_string - 5,43% netperf [kernel.vmlinux] [k] _raw_write_unlock_irqrestore - _raw_write_unlock_irqrestore - 96,54% _raw_spin_unlock_irqrestore - 36,14% mod_timer + 97,24% sctp_transport_reset_timers + 2,76% sctp_do_sm + 33,65% __wake_up_sync_key + 28,77% sctp_ulpq_tail_event + 1,40% del_timer - 1,84% mod_timer + 99,03% sctp_transport_reset_timers + 0,97% sctp_do_sm + 1,50% sctp_ulpq_tail_event And after this patch, now with netperf -l 60: Samples: 230K of event 'cpu-clock', Event count (approx.): 57707250000 Overhead Command Shared Object Symbol + 5,65% netperf [kernel.vmlinux] [k] memcpy_erms + 5,59% netperf [kernel.vmlinux] [k] copy_user_enhanced_fast_string - 5,05% netperf [kernel.vmlinux] [k] _raw_spin_unlock_irqrestore - _raw_spin_unlock_irqrestore + 49,89% __wake_up_sync_key + 45,68% sctp_ulpq_tail_event - 2,85% mod_timer + 76,51% sctp_transport_reset_t3_rtx + 23,49% sctp_do_sm + 1,55% del_timer + 2,50% netperf [sctp] [k] sctp_datamsg_from_user + 2,26% netperf [sctp] [k] sctp_sendmsg Throughput-wise, from 6800mbps without the patch to 7050mbps with it, ~3.7%. Signed-off-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com> Signed-off-by: David S. Miller <davem@davemloft.net>
1 parent 49dd48d commit ba6f5e3

File tree

5 files changed

+47
-34
lines changed

5 files changed

+47
-34
lines changed

‎include/net/sctp/structs.h

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -847,6 +847,11 @@ struct sctp_transport {
847847
*/
848848
ktime_t last_time_heard;
849849

850+
/* When was the last time that we sent a chunk using this
851+
* transport? We use this to check for idle transports
852+
*/
853+
unsigned long last_time_sent;
854+
850855
/* Last time(in jiffies) when cwnd is reduced due to the congestion
851856
* indication based on ECNE chunk.
852857
*/
@@ -952,7 +957,8 @@ void sctp_transport_route(struct sctp_transport *, union sctp_addr *,
952957
struct sctp_sock *);
953958
void sctp_transport_pmtu(struct sctp_transport *, struct sock *sk);
954959
void sctp_transport_free(struct sctp_transport *);
955-
void sctp_transport_reset_timers(struct sctp_transport *);
960+
void sctp_transport_reset_t3_rtx(struct sctp_transport *);
961+
void sctp_transport_reset_hb_timer(struct sctp_transport *);
956962
int sctp_transport_hold(struct sctp_transport *);
957963
void sctp_transport_put(struct sctp_transport *);
958964
void sctp_transport_update_rto(struct sctp_transport *, __u32);

‎net/sctp/outqueue.c

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -866,8 +866,10 @@ static int sctp_outq_flush(struct sctp_outq *q, int rtx_timeout, gfp_t gfp)
866866
* sender MUST assure that at least one T3-rtx
867867
* timer is running.
868868
*/
869-
if (chunk->chunk_hdr->type == SCTP_CID_FWD_TSN)
870-
sctp_transport_reset_timers(transport);
869+
if (chunk->chunk_hdr->type == SCTP_CID_FWD_TSN) {
870+
sctp_transport_reset_t3_rtx(transport);
871+
transport->last_time_sent = jiffies;
872+
}
871873
}
872874
break;
873875

@@ -924,8 +926,10 @@ static int sctp_outq_flush(struct sctp_outq *q, int rtx_timeout, gfp_t gfp)
924926
error = sctp_outq_flush_rtx(q, packet,
925927
rtx_timeout, &start_timer);
926928

927-
if (start_timer)
928-
sctp_transport_reset_timers(transport);
929+
if (start_timer) {
930+
sctp_transport_reset_t3_rtx(transport);
931+
transport->last_time_sent = jiffies;
932+
}
929933

930934
/* This can happen on COOKIE-ECHO resend. Only
931935
* one chunk can get bundled with a COOKIE-ECHO.
@@ -1062,7 +1066,8 @@ static int sctp_outq_flush(struct sctp_outq *q, int rtx_timeout, gfp_t gfp)
10621066
list_add_tail(&chunk->transmitted_list,
10631067
&transport->transmitted);
10641068

1065-
sctp_transport_reset_timers(transport);
1069+
sctp_transport_reset_t3_rtx(transport);
1070+
transport->last_time_sent = jiffies;
10661071

10671072
/* Only let one DATA chunk get bundled with a
10681073
* COOKIE-ECHO chunk.

‎net/sctp/sm_make_chunk.c

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3080,8 +3080,7 @@ static __be16 sctp_process_asconf_param(struct sctp_association *asoc,
30803080
return SCTP_ERROR_RSRC_LOW;
30813081

30823082
/* Start the heartbeat timer. */
3083-
if (!mod_timer(&peer->hb_timer, sctp_transport_timeout(peer)))
3084-
sctp_transport_hold(peer);
3083+
sctp_transport_reset_hb_timer(peer);
30853084
asoc->new_transport = peer;
30863085
break;
30873086
case SCTP_PARAM_DEL_IP:

‎net/sctp/sm_sideeffect.c

Lines changed: 16 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -69,8 +69,6 @@ static int sctp_side_effects(sctp_event_t event_type, sctp_subtype_t subtype,
6969
sctp_cmd_seq_t *commands,
7070
gfp_t gfp);
7171

72-
static void sctp_cmd_hb_timer_update(sctp_cmd_seq_t *cmds,
73-
struct sctp_transport *t);
7472
/********************************************************************
7573
* Helper functions
7674
********************************************************************/
@@ -367,6 +365,7 @@ void sctp_generate_heartbeat_event(unsigned long data)
367365
struct sctp_association *asoc = transport->asoc;
368366
struct sock *sk = asoc->base.sk;
369367
struct net *net = sock_net(sk);
368+
u32 elapsed, timeout;
370369

371370
bh_lock_sock(sk);
372371
if (sock_owned_by_user(sk)) {
@@ -378,6 +377,16 @@ void sctp_generate_heartbeat_event(unsigned long data)
378377
goto out_unlock;
379378
}
380379

380+
/* Check if we should still send the heartbeat or reschedule */
381+
elapsed = jiffies - transport->last_time_sent;
382+
timeout = sctp_transport_timeout(transport);
383+
if (elapsed < timeout) {
384+
elapsed = timeout - elapsed;
385+
if (!mod_timer(&transport->hb_timer, jiffies + elapsed))
386+
sctp_transport_hold(transport);
387+
goto out_unlock;
388+
}
389+
381390
error = sctp_do_sm(net, SCTP_EVENT_T_TIMEOUT,
382391
SCTP_ST_TIMEOUT(SCTP_EVENT_TIMEOUT_HEARTBEAT),
383392
asoc->state, asoc->ep, asoc,
@@ -507,7 +516,7 @@ static void sctp_do_8_2_transport_strike(sctp_cmd_seq_t *commands,
507516
0);
508517

509518
/* Update the hb timer to resend a heartbeat every rto */
510-
sctp_cmd_hb_timer_update(commands, transport);
519+
sctp_transport_reset_hb_timer(transport);
511520
}
512521

513522
if (transport->state != SCTP_INACTIVE &&
@@ -634,11 +643,8 @@ static void sctp_cmd_hb_timers_start(sctp_cmd_seq_t *cmds,
634643
* hold a reference on the transport to make sure none of
635644
* the needed data structures go away.
636645
*/
637-
list_for_each_entry(t, &asoc->peer.transport_addr_list, transports) {
638-
639-
if (!mod_timer(&t->hb_timer, sctp_transport_timeout(t)))
640-
sctp_transport_hold(t);
641-
}
646+
list_for_each_entry(t, &asoc->peer.transport_addr_list, transports)
647+
sctp_transport_reset_hb_timer(t);
642648
}
643649

644650
static void sctp_cmd_hb_timers_stop(sctp_cmd_seq_t *cmds,
@@ -669,15 +675,6 @@ static void sctp_cmd_t3_rtx_timers_stop(sctp_cmd_seq_t *cmds,
669675
}
670676

671677

672-
/* Helper function to update the heartbeat timer. */
673-
static void sctp_cmd_hb_timer_update(sctp_cmd_seq_t *cmds,
674-
struct sctp_transport *t)
675-
{
676-
/* Update the heartbeat timer. */
677-
if (!mod_timer(&t->hb_timer, sctp_transport_timeout(t)))
678-
sctp_transport_hold(t);
679-
}
680-
681678
/* Helper function to handle the reception of an HEARTBEAT ACK. */
682679
static void sctp_cmd_transport_on(sctp_cmd_seq_t *cmds,
683680
struct sctp_association *asoc,
@@ -742,8 +739,7 @@ static void sctp_cmd_transport_on(sctp_cmd_seq_t *cmds,
742739
sctp_transport_update_rto(t, (jiffies - hbinfo->sent_at));
743740

744741
/* Update the heartbeat timer. */
745-
if (!mod_timer(&t->hb_timer, sctp_transport_timeout(t)))
746-
sctp_transport_hold(t);
742+
sctp_transport_reset_hb_timer(t);
747743

748744
if (was_unconfirmed && asoc->peer.transport_count == 1)
749745
sctp_transport_immediate_rtx(t);
@@ -1614,7 +1610,7 @@ static int sctp_cmd_interpreter(sctp_event_t event_type,
16141610

16151611
case SCTP_CMD_HB_TIMER_UPDATE:
16161612
t = cmd->obj.transport;
1617-
sctp_cmd_hb_timer_update(commands, t);
1613+
sctp_transport_reset_hb_timer(t);
16181614
break;
16191615

16201616
case SCTP_CMD_HB_TIMERS_STOP:

‎net/sctp/transport.c

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -183,7 +183,7 @@ static void sctp_transport_destroy(struct sctp_transport *transport)
183183
/* Start T3_rtx timer if it is not already running and update the heartbeat
184184
* timer. This routine is called every time a DATA chunk is sent.
185185
*/
186-
void sctp_transport_reset_timers(struct sctp_transport *transport)
186+
void sctp_transport_reset_t3_rtx(struct sctp_transport *transport)
187187
{
188188
/* RFC 2960 6.3.2 Retransmission Timer Rules
189189
*
@@ -197,11 +197,18 @@ void sctp_transport_reset_timers(struct sctp_transport *transport)
197197
if (!mod_timer(&transport->T3_rtx_timer,
198198
jiffies + transport->rto))
199199
sctp_transport_hold(transport);
200+
}
201+
202+
void sctp_transport_reset_hb_timer(struct sctp_transport *transport)
203+
{
204+
unsigned long expires;
200205

201206
/* When a data chunk is sent, reset the heartbeat interval. */
202-
if (!mod_timer(&transport->hb_timer,
203-
sctp_transport_timeout(transport)))
204-
sctp_transport_hold(transport);
207+
expires = jiffies + sctp_transport_timeout(transport);
208+
if (time_before(transport->hb_timer.expires, expires) &&
209+
!mod_timer(&transport->hb_timer,
210+
expires + prandom_u32_max(transport->rto)))
211+
sctp_transport_hold(transport);
205212
}
206213

207214
/* This transport has been assigned to an association.
@@ -595,13 +602,13 @@ void sctp_transport_burst_reset(struct sctp_transport *t)
595602
unsigned long sctp_transport_timeout(struct sctp_transport *trans)
596603
{
597604
/* RTO + timer slack +/- 50% of RTO */
598-
unsigned long timeout = (trans->rto >> 1) + prandom_u32_max(trans->rto);
605+
unsigned long timeout = trans->rto >> 1;
599606

600607
if (trans->state != SCTP_UNCONFIRMED &&
601608
trans->state != SCTP_PF)
602609
timeout += trans->hbinterval;
603610

604-
return timeout + jiffies;
611+
return timeout;
605612
}
606613

607614
/* Reset transport variables to their initial values */

0 commit comments

Comments
 (0)