Skip to content

Commit

Permalink
dccp ccid-3: Remove dead states
Browse files Browse the repository at this point in the history
This patch is thanks to an investigation by Leandro Sales de Melo and his
colleagues. They worked out two state diagrams which highlight the fact that
the xxx_TERM states in CCID-3/4 are in fact not necessary.

And this can be confirmed by in turn looking at the code: the xxx_TERM states
are only ever set in ccid3_hc_{rx,tx}_exit(). These two functions are part
of the following call chain:

 * ccid_hc_{tx,rx}_exit() are called from ccid_delete() only;
 * ccid_delete() invokes ccid_hc_{tx,rx}_exit() in the way of a destructor:
   after calling ccid_hc_{tx,rx}_exit(), the CCID is released from memory;
 * ccid_delete() is in turn called only by ccid_hc_{tx,rx}_delete();
 * ccid_hc_{tx,rx}_delete() is called only if 
   - feature negotiation failed   (dccp_feat_activate_values()),
   - when changing the RX/TX CCID (to eject the current CCID),
   - when destroying the socket   (in dccp_destroy_sock()).

In other words, when CCID-3 sets the state to xxx_TERM, it is at a time where
no more processing should be going on, hence it is not necessary to introduce
a dedicated exit state - this is implicit when unloading the CCID.

The patch removes this state, one switch-statement collapses as a result.

Signed-off-by: Gerrit Renker <gerrit@erg.abdn.ac.uk>
  • Loading branch information
Gerrit Renker committed Sep 4, 2008
1 parent 5fe9496 commit d0995e6
Show file tree
Hide file tree
Showing 2 changed files with 9 additions and 30 deletions.
37 changes: 9 additions & 28 deletions net/dccp/ccids/ccid3.c
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,6 @@ static const char *ccid3_tx_state_name(enum ccid3_hc_tx_states state)
[TFRC_SSTATE_NO_SENT] = "NO_SENT",
[TFRC_SSTATE_NO_FBACK] = "NO_FBACK",
[TFRC_SSTATE_FBACK] = "FBACK",
[TFRC_SSTATE_TERM] = "TERM",
};

return ccid3_state_names[state];
Expand Down Expand Up @@ -210,10 +209,13 @@ static void ccid3_hc_tx_no_feedback_timer(unsigned long data)
ccid3_pr_debug("%s(%p, state=%s) - entry \n", dccp_role(sk), sk,
ccid3_tx_state_name(hctx->state));

/* Ignore and do not restart after leaving the established state */
if ((1 << sk->sk_state) & ~(DCCPF_OPEN | DCCPF_PARTOPEN))
goto out;

/* Reset feedback state to "no feedback received" */
if (hctx->state == TFRC_SSTATE_FBACK)
ccid3_hc_tx_set_state(sk, TFRC_SSTATE_NO_FBACK);
else if (hctx->state != TFRC_SSTATE_NO_FBACK)
goto out;

/*
* Determine new allowed sending rate X as per draft rfc3448bis-00, 4.4
Expand Down Expand Up @@ -288,8 +290,7 @@ static int ccid3_hc_tx_send_packet(struct sock *sk, struct sk_buff *skb)
if (unlikely(skb->len == 0))
return -EBADMSG;

switch (hctx->state) {
case TFRC_SSTATE_NO_SENT:
if (hctx->state == TFRC_SSTATE_NO_SENT) {
sk_reset_timer(sk, &hctx->no_feedback_timer, (jiffies +
usecs_to_jiffies(TFRC_INITIAL_TIMEOUT)));
hctx->last_win_count = 0;
Expand Down Expand Up @@ -324,9 +325,8 @@ static int ccid3_hc_tx_send_packet(struct sock *sk, struct sk_buff *skb)
ccid3_update_send_interval(hctx);

ccid3_hc_tx_set_state(sk, TFRC_SSTATE_NO_FBACK);
break;
case TFRC_SSTATE_NO_FBACK:
case TFRC_SSTATE_FBACK:

} else {
delay = ktime_us_delta(hctx->t_nom, now);
ccid3_pr_debug("delay=%ld\n", (long)delay);
/*
Expand All @@ -341,10 +341,6 @@ static int ccid3_hc_tx_send_packet(struct sock *sk, struct sk_buff *skb)
return (u32)delay / USEC_PER_MSEC;

ccid3_hc_tx_update_win_count(hctx, now);
break;
case TFRC_SSTATE_TERM:
DCCP_BUG("%s(%p) - Illegal state TERM", dccp_role(sk), sk);
return -EINVAL;
}

/* prepare to send now (add options etc.) */
Expand Down Expand Up @@ -378,11 +374,6 @@ static void ccid3_hc_tx_packet_recv(struct sock *sk, struct sk_buff *skb)
if (!(DCCP_SKB_CB(skb)->dccpd_type == DCCP_PKT_ACK ||
DCCP_SKB_CB(skb)->dccpd_type == DCCP_PKT_DATAACK))
return;
/* ... and only in the established state */
if (hctx->state != TFRC_SSTATE_FBACK &&
hctx->state != TFRC_SSTATE_NO_FBACK)
return;

/*
* Locate the acknowledged packet in the TX history.
*
Expand Down Expand Up @@ -522,9 +513,7 @@ static void ccid3_hc_tx_exit(struct sock *sk)
{
struct ccid3_hc_tx_sock *hctx = ccid3_hc_tx_sk(sk);

ccid3_hc_tx_set_state(sk, TFRC_SSTATE_TERM);
sk_stop_timer(sk, &hctx->no_feedback_timer);

tfrc_tx_hist_purge(&hctx->hist);
}

Expand Down Expand Up @@ -583,7 +572,6 @@ static const char *ccid3_rx_state_name(enum ccid3_hc_rx_states state)
static char *ccid3_rx_state_names[] = {
[TFRC_RSTATE_NO_DATA] = "NO_DATA",
[TFRC_RSTATE_DATA] = "DATA",
[TFRC_RSTATE_TERM] = "TERM",
};

return ccid3_rx_state_names[state];
Expand All @@ -609,14 +597,9 @@ static void ccid3_hc_rx_send_feedback(struct sock *sk,
{
struct ccid3_hc_rx_sock *hcrx = ccid3_hc_rx_sk(sk);
struct dccp_sock *dp = dccp_sk(sk);
ktime_t now;
ktime_t now = ktime_get_real();
s64 delta = 0;

if (unlikely(hcrx->state == TFRC_RSTATE_TERM))
return;

now = ktime_get_real();

switch (fbtype) {
case CCID3_FBACK_INITIAL:
hcrx->x_recv = 0;
Expand Down Expand Up @@ -819,8 +802,6 @@ static void ccid3_hc_rx_exit(struct sock *sk)
{
struct ccid3_hc_rx_sock *hcrx = ccid3_hc_rx_sk(sk);

ccid3_hc_rx_set_state(sk, TFRC_RSTATE_TERM);

tfrc_rx_hist_purge(&hcrx->hist);
tfrc_lh_cleanup(&hcrx->li_hist);
}
Expand Down
2 changes: 0 additions & 2 deletions net/dccp/ccids/ccid3.h
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,6 @@ enum ccid3_hc_tx_states {
TFRC_SSTATE_NO_SENT = 1,
TFRC_SSTATE_NO_FBACK,
TFRC_SSTATE_FBACK,
TFRC_SSTATE_TERM,
};

/** struct ccid3_hc_tx_sock - CCID3 sender half-connection socket
Expand Down Expand Up @@ -126,7 +125,6 @@ static inline struct ccid3_hc_tx_sock *ccid3_hc_tx_sk(const struct sock *sk)
enum ccid3_hc_rx_states {
TFRC_RSTATE_NO_DATA = 1,
TFRC_RSTATE_DATA,
TFRC_RSTATE_TERM = 127,
};

/** struct ccid3_hc_rx_sock - CCID3 receiver half-connection socket
Expand Down

0 comments on commit d0995e6

Please sign in to comment.