From fb8fb8f8159b0ef529ccdfbb732a5e4cf1501069 Mon Sep 17 00:00:00 2001 From: Randall Stewart Date: Tue, 30 Oct 2007 14:09:24 +0000 Subject: [PATCH] - Change the Time Wait of vtags value to match the cookie-life - Select a tag gains ability to optionally save new tags off in the timewait system. - When looking up associations do not give back a stcb that is in the about-to-be-freed state, and instead continue looking for other candiates. - New function to query to see if value is in time-wait. - Timewait had a time comparison error that caused very few vtags to actually stay in time-wait. - When setting tags in time-wait, we now use the time requested NOT a fixed constant value. - sstat now gets the proper associd when we do the query. - When we process an association, we expect the tag chosen (if we have one from a cookie) to be in time-wait. Before we would NOT allow the assoc up by checking if its good. In theory this should have caused almost all assoc not to come up except for the time-comparison bug above (this bug was hidden by the time comparison bug :-D). - Don't save tags for nonce values in the time-wait cache since these are used only during cookie collisions and do not matter if they are unique or not. MFC after: 1 week --- sys/netinet/sctp_constants.h | 5 ++- sys/netinet/sctp_output.c | 4 +-- sys/netinet/sctp_pcb.c | 66 +++++++++++++++++++++++++++++++----- sys/netinet/sctp_pcb.h | 4 ++- sys/netinet/sctp_usrreq.c | 1 + sys/netinet/sctputil.c | 18 ++++++---- sys/netinet/sctputil.h | 2 +- 7 files changed, 78 insertions(+), 22 deletions(-) diff --git a/sys/netinet/sctp_constants.h b/sys/netinet/sctp_constants.h index f2174df02530..6f2fd6111882 100644 --- a/sys/netinet/sctp_constants.h +++ b/sys/netinet/sctp_constants.h @@ -1007,10 +1007,9 @@ __FBSDID("$FreeBSD$"); */ /* - * Number of seconds of time wait, tied to MSL value (2 minutes), so 2 * MSL - * = 4 minutes or 480 seconds. + * Number of seconds of time wait for a vtag. */ -#define SCTP_TIME_WAIT 480 +#define SCTP_TIME_WAIT 60 /* This time wait is the same as the default cookie life * since we now enter a tag in every time we send a cookie. diff --git a/sys/netinet/sctp_output.c b/sys/netinet/sctp_output.c index d384a65adede..fccd2bd93503 100644 --- a/sys/netinet/sctp_output.c +++ b/sys/netinet/sctp_output.c @@ -4955,7 +4955,7 @@ sctp_send_initiate_ack(struct sctp_inpcb *inp, struct sctp_tcb *stcb, if (asoc) { atomic_add_int(&asoc->refcnt, 1); SCTP_TCB_UNLOCK(stcb); - vtag = sctp_select_a_tag(inp); + vtag = sctp_select_a_tag(inp, 1); initackm_out->msg.init.initiate_tag = htonl(vtag); /* get a TSN to use too */ itsn = sctp_select_initial_TSN(&inp->sctp_ep); @@ -4963,7 +4963,7 @@ sctp_send_initiate_ack(struct sctp_inpcb *inp, struct sctp_tcb *stcb, SCTP_TCB_LOCK(stcb); atomic_add_int(&asoc->refcnt, -1); } else { - vtag = sctp_select_a_tag(inp); + vtag = sctp_select_a_tag(inp, 1); initackm_out->msg.init.initiate_tag = htonl(vtag); /* get a TSN to use too */ initackm_out->msg.init.initial_tsn = htonl(sctp_select_initial_TSN(&inp->sctp_ep)); diff --git a/sys/netinet/sctp_pcb.c b/sys/netinet/sctp_pcb.c index c5ee0fe9e8ef..fef23bf638e9 100644 --- a/sys/netinet/sctp_pcb.c +++ b/sys/netinet/sctp_pcb.c @@ -928,6 +928,11 @@ sctp_tcb_special_locate(struct sctp_inpcb **inp_p, struct sockaddr *from, SCTP_INP_RUNLOCK(inp); continue; } + if (stcb->asoc.state & SCTP_STATE_ABOUT_TO_BE_FREED) { + SCTP_TCB_UNLOCK(stcb); + SCTP_INP_RUNLOCK(inp); + continue; + } /* Does this TCB have a matching address? */ TAILQ_FOREACH(net, &stcb->asoc.nets, sctp_next) { @@ -1045,11 +1050,16 @@ sctp_findassociation_ep_addr(struct sctp_inpcb **inp_p, struct sockaddr *remote, goto null_return; } SCTP_TCB_LOCK(stcb); + if (stcb->rport != rport) { /* remote port does not match. */ SCTP_TCB_UNLOCK(stcb); goto null_return; } + if (stcb->asoc.state & SCTP_STATE_ABOUT_TO_BE_FREED) { + SCTP_TCB_UNLOCK(stcb); + goto null_return; + } /* now look at the list of remote addresses */ TAILQ_FOREACH(net, &stcb->asoc.nets, sctp_next) { #ifdef INVARIANTS @@ -1128,8 +1138,12 @@ sctp_findassociation_ep_addr(struct sctp_inpcb **inp_p, struct sockaddr *remote, /* remote port does not match */ continue; } - /* now look at the list of remote addresses */ SCTP_TCB_LOCK(stcb); + if (stcb->asoc.state & SCTP_STATE_ABOUT_TO_BE_FREED) { + SCTP_TCB_UNLOCK(stcb); + continue; + } + /* now look at the list of remote addresses */ TAILQ_FOREACH(net, &stcb->asoc.nets, sctp_next) { #ifdef INVARIANTS if (net == (TAILQ_NEXT(net, sctp_next))) { @@ -1250,6 +1264,9 @@ sctp_findassociation_ep_asocid(struct sctp_inpcb *inp, sctp_assoc_t asoc_id, int SCTP_INP_RUNLOCK(stcb->sctp_ep); continue; } + if (stcb->asoc.state & SCTP_STATE_ABOUT_TO_BE_FREED) { + continue; + } if (want_lock) { SCTP_TCB_LOCK(stcb); } @@ -1272,6 +1289,9 @@ sctp_findassociation_ep_asocid(struct sctp_inpcb *inp, sctp_assoc_t asoc_id, int SCTP_INP_RUNLOCK(stcb->sctp_ep); continue; } + if (stcb->asoc.state & SCTP_STATE_ABOUT_TO_BE_FREED) { + continue; + } if (want_lock) { SCTP_TCB_LOCK(stcb); } @@ -1699,6 +1719,10 @@ sctp_findassoc_by_vtag(struct sockaddr *from, uint32_t vtag, SCTP_TCB_UNLOCK(stcb); continue; } + if (stcb->asoc.state & SCTP_STATE_ABOUT_TO_BE_FREED) { + SCTP_TCB_UNLOCK(stcb); + continue; + } if (skip_src_check) { *netp = NULL; /* unknown */ if (inp_p) @@ -3922,6 +3946,31 @@ sctp_delete_from_timewait(uint32_t tag) } } +int +sctp_is_in_timewait(uint32_t tag) +{ + struct sctpvtaghead *chain; + struct sctp_tagblock *twait_block; + int found = 0; + int i; + + chain = &sctppcbinfo.vtag_timewait[(tag % SCTP_STACK_VTAG_HASH_SIZE)]; + if (!SCTP_LIST_EMPTY(chain)) { + LIST_FOREACH(twait_block, chain, sctp_nxt_tagblock) { + for (i = 0; i < SCTP_NUMBER_IN_VTAG_BLOCK; i++) { + if (twait_block->vtag_block[i].v_tag == tag) { + found = 1; + break; + } + } + if (found) + break; + } + } + return (found); +} + + void sctp_add_vtag_to_timewait(uint32_t tag, uint32_t time) { @@ -3944,14 +3993,13 @@ sctp_add_vtag_to_timewait(uint32_t tag, uint32_t time) twait_block->vtag_block[i].v_tag = tag; set = 1; } else if ((twait_block->vtag_block[i].v_tag) && - ((long)twait_block->vtag_block[i].tv_sec_at_expire > - now.tv_sec)) { + ((long)twait_block->vtag_block[i].tv_sec_at_expire < now.tv_sec)) { /* Audit expires this guy */ twait_block->vtag_block[i].tv_sec_at_expire = 0; twait_block->vtag_block[i].v_tag = 0; if (set == 0) { /* Reuse it for my new tag */ - twait_block->vtag_block[0].tv_sec_at_expire = now.tv_sec + SCTP_TIME_WAIT; + twait_block->vtag_block[0].tv_sec_at_expire = now.tv_sec + time; twait_block->vtag_block[0].v_tag = tag; set = 1; } @@ -3975,8 +4023,7 @@ sctp_add_vtag_to_timewait(uint32_t tag, uint32_t time) } memset(twait_block, 0, sizeof(struct sctp_tagblock)); LIST_INSERT_HEAD(chain, twait_block, sctp_nxt_tagblock); - twait_block->vtag_block[0].tv_sec_at_expire = now.tv_sec + - SCTP_TIME_WAIT; + twait_block->vtag_block[0].tv_sec_at_expire = now.tv_sec + time; twait_block->vtag_block[0].v_tag = tag; } } @@ -5738,7 +5785,7 @@ sctp_set_primary_addr(struct sctp_tcb *stcb, struct sockaddr *sa, } int -sctp_is_vtag_good(struct sctp_inpcb *inp, uint32_t tag, struct timeval *now) +sctp_is_vtag_good(struct sctp_inpcb *inp, uint32_t tag, struct timeval *now, int save_in_twait) { /* * This function serves two purposes. It will see if a TAG can be @@ -5805,7 +5852,7 @@ check_time_wait: if (twait_block->vtag_block[i].v_tag == 0) { /* not used */ continue; - } else if ((long)twait_block->vtag_block[i].tv_sec_at_expire > + } else if ((long)twait_block->vtag_block[i].tv_sec_at_expire < now->tv_sec) { /* Audit expires this guy */ twait_block->vtag_block[i].tv_sec_at_expire = 0; @@ -5827,7 +5874,8 @@ check_time_wait: * add this tag to the assoc hash we need to purge it from * the t-wait hash. */ - sctp_add_vtag_to_timewait(tag, TICKS_TO_SEC(inp->sctp_ep.def_cookie_life)); + if (save_in_twait) + sctp_add_vtag_to_timewait(tag, TICKS_TO_SEC(inp->sctp_ep.def_cookie_life)); SCTP_INP_INFO_WUNLOCK(); return (1); } diff --git a/sys/netinet/sctp_pcb.h b/sys/netinet/sctp_pcb.h index facf2de4a8c8..65e4b505c0c8 100644 --- a/sys/netinet/sctp_pcb.h +++ b/sys/netinet/sctp_pcb.h @@ -531,6 +531,8 @@ int sctp_free_assoc(struct sctp_inpcb *, struct sctp_tcb *, int, int); void sctp_delete_from_timewait(uint32_t); +int sctp_is_in_timewait(uint32_t tag); + void sctp_add_vtag_to_timewait(uint32_t, uint32_t); @@ -562,7 +564,7 @@ int sctp_set_primary_addr(struct sctp_tcb *, struct sockaddr *, struct sctp_nets *); -int sctp_is_vtag_good(struct sctp_inpcb *, uint32_t, struct timeval *); +int sctp_is_vtag_good(struct sctp_inpcb *, uint32_t, struct timeval *, int); /* void sctp_drain(void); */ diff --git a/sys/netinet/sctp_usrreq.c b/sys/netinet/sctp_usrreq.c index 24b916e14368..070c9a747e1d 100644 --- a/sys/netinet/sctp_usrreq.c +++ b/sys/netinet/sctp_usrreq.c @@ -2266,6 +2266,7 @@ flags_out: * land. */ sstat->sstat_state = stcb->asoc.state; + sstat->sstat_assoc_id = sctp_get_associd(stcb); sstat->sstat_rwnd = stcb->asoc.peers_rwnd; sstat->sstat_unackdata = stcb->asoc.sent_queue_cnt; /* diff --git a/sys/netinet/sctputil.c b/sys/netinet/sctputil.c index c1ffe81565af..2d76c4e18a74 100644 --- a/sys/netinet/sctputil.c +++ b/sys/netinet/sctputil.c @@ -844,7 +844,7 @@ retry: } uint32_t -sctp_select_a_tag(struct sctp_inpcb *inp) +sctp_select_a_tag(struct sctp_inpcb *inp, int save_in_twait) { u_long x, not_done; struct timeval now; @@ -857,7 +857,7 @@ sctp_select_a_tag(struct sctp_inpcb *inp) /* we never use 0 */ continue; } - if (sctp_is_vtag_good(inp, x, &now)) { + if (sctp_is_vtag_good(inp, x, &now, save_in_twait)) { not_done = 0; } } @@ -908,19 +908,25 @@ sctp_init_asoc(struct sctp_inpcb *m, struct sctp_tcb *stcb, struct timeval now; (void)SCTP_GETTIME_TIMEVAL(&now); - if (sctp_is_vtag_good(m, override_tag, &now)) { + if (sctp_is_in_timewait(override_tag)) { + /* + * It must be in the time-wait hash, we put it there + * when we aloc one. If not the peer is playing + * games. + */ asoc->my_vtag = override_tag; } else { SCTP_LTRACE_ERR_RET(NULL, stcb, NULL, SCTP_FROM_SCTPUTIL, ENOMEM); + panic("Huh is_in_timewait fails"); return (ENOMEM); } } else { - asoc->my_vtag = sctp_select_a_tag(m); + asoc->my_vtag = sctp_select_a_tag(m, 1); } /* Get the nonce tags */ - asoc->my_vtag_nonce = sctp_select_a_tag(m); - asoc->peer_vtag_nonce = sctp_select_a_tag(m); + asoc->my_vtag_nonce = sctp_select_a_tag(m, 0); + asoc->peer_vtag_nonce = sctp_select_a_tag(m, 0); asoc->vrf_id = vrf_id; if (sctp_is_feature_on(m, SCTP_PCB_FLAGS_DONOT_HEARTBEAT)) diff --git a/sys/netinet/sctputil.h b/sys/netinet/sctputil.h index be03027c5cea..24157e33e310 100644 --- a/sys/netinet/sctputil.h +++ b/sys/netinet/sctputil.h @@ -77,7 +77,7 @@ struct sctp_ifa * uint32_t sctp_select_initial_TSN(struct sctp_pcb *); -uint32_t sctp_select_a_tag(struct sctp_inpcb *); +uint32_t sctp_select_a_tag(struct sctp_inpcb *, int); int sctp_init_asoc(struct sctp_inpcb *, struct sctp_tcb *, int, uint32_t, uint32_t);