| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Mon, 7 Mar 2011 23:50:31 -0500
From: Samuel Jero <sj323707@...o.edu>
To: Gerrit Renker <gerrit@....abdn.ac.uk>
CC: <dccp@...r.kernel.org>, <netdev@...r.kernel.org>
Subject: Re: dccp test-tree [RFC] [Patch 1/1] dccp: Only activate NN values
after receiving the Confirm option
> I am sending this as RFC since I have not yet deeply tested this. It makes
> the exchange of NN options in established state conform to RFC 4340, 6.6.1
> and thus actually is a bug fix.
I now had a chance to put this patch through extensive tests. I had to
modify two things about the patch to get good performance:
1)CCID2 now checks the congestion window against the currently
negotiating ack ratio value to determine whether to change the ack ratio
after an RTO, an idle period, or a loss. This prevents a problem where
an RTO or loss or idle period occurs and the ack ratio is less than the
congestion window but is in the process of being negotiated larger. That
negotiation will reach the sender on the first packet sent and RTOs will
result.
The primary change is adding a function called
dccp_feat_get_nn_next_val() to feat.c. This function returns the value
of the NN feature indicated that is is the process of being negotiated.
If the feature is not being negotiated, then the current value is
returned.
2)In a situation where the ack ratio has to be reduced because of an
RTO, idle period, or loss, CCID2 now sets the ack ratio to half of the
congestion window (or 1 if that's zero) instead of to the congestion
window. This should reduce the problems if one ack is lost (we have to
lose two acks to not acknowledge an entire congestion window and trigger
RTO)
Below is an improved patch. What are your thoughts on these changes?
Samuel Jero
Internetworking Research Group
Ohio University
>>>>>>>>>>>>>>>>>>>>>>>>>Patch<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<
dccp: Only activate NN values after receiving the Confirm option
This defers changing local values using exchange of NN options in
established connection state by only activating the value after
receiving the Confirm option, as mandated by RFC 4340, 6.6.1.
Signed-off-by: Samuel Jero
diff --git a/net/dccp/ccids/ccid2.c b/net/dccp/ccids/ccid2.c
--- a/net/dccp/ccids/ccid2.c
+++ b/net/dccp/ccids/ccid2.c
@@ -83,9 +82,13 @@ static int ccid2_hc_tx_send_packet(struct sock *sk, struct sk_buff *skb)
return CCID_PACKET_SEND_AT_ONCE;
}
+static __u64 ccid2_ack_ratio_next(struct sock *sk)
+{
+ return dccp_feat_get_nn_next_val(sk, DCCPF_ACK_RATIO);
+}
+
static void ccid2_change_l_ack_ratio(struct sock *sk, u32 val)
{
- struct dccp_sock *dp = dccp_sk(sk);
u32 max_ratio = DIV_ROUND_UP(ccid2_hc_tx_sk(sk)->tx_cwnd, 2);
/*
@@ -101,11 +104,10 @@ static void ccid2_change_l_ack_ratio(struct sock *sk, u32 val)
if (val > DCCPF_ACK_RATIO_MAX)
val = DCCPF_ACK_RATIO_MAX;
- if (val == dp->dccps_l_ack_ratio)
+ if (val == ccid2_ack_ratio_next(sk))
return;
ccid2_pr_debug("changing local ack ratio to %u\n", val);
- dp->dccps_l_ack_ratio = val;
dccp_feat_signal_nn_change(sk, DCCPF_ACK_RATIO, val);
}
@@ -117,11 +119,9 @@ static void ccid2_change_l_seq_window(struct sock *sk, u64 val)
val = DCCPF_SEQ_WMIN;
if (val > DCCPF_SEQ_WMAX)
val = DCCPF_SEQ_WMAX;
- if (val == dp->dccps_l_seq_win)
- return;
- dp->dccps_l_seq_win = val;
- dccp_feat_signal_nn_change(sk, DCCPF_SEQUENCE_WINDOW, val);
+ if (val != dp->dccps_l_seq_win)
+ dccp_feat_signal_nn_change(sk, DCCPF_SEQUENCE_WINDOW, val);
}
static void ccid2_hc_tx_rto_expire(unsigned long data)
@@ -203,6 +203,10 @@ static void ccid2_cwnd_application_limited(struct sock *sk, const u32 now)
}
hc->tx_cwnd_used = 0;
hc->tx_cwnd_stamp = now;
+
+ /* Avoid spurious timeouts resulting from Ack Ratio > cwnd */
+ if (ccid2_ack_ratio_next(sk) > hc->tx_cwnd)
+ ccid2_change_l_ack_ratio(sk, hc->tx_cwnd/2 ? : 1U);
}
/* This borrows the code of tcp_cwnd_restart() */
@@ -221,6 +225,10 @@ static void ccid2_cwnd_restart(struct sock *sk, const u32 now)
hc->tx_cwnd_stamp = now;
hc->tx_cwnd_used = 0;
+
+ /* Avoid spurious timeouts resulting from Ack Ratio > cwnd */
+ if (ccid2_ack_ratio_next(sk) > hc->tx_cwnd)
+ ccid2_change_l_ack_ratio(sk, hc->tx_cwnd/2 ? : 1U);
}
static void ccid2_hc_tx_packet_sent(struct sock *sk, unsigned int len)
@@ -491,8 +531,8 @@ static void ccid2_congestion_event(struct sock *sk, struct ccid2_seq *seqp)
hc->tx_ssthresh = max(hc->tx_cwnd, 2U);
/* Avoid spurious timeouts resulting from Ack Ratio > cwnd */
- if (dccp_sk(sk)->dccps_l_ack_ratio > hc->tx_cwnd)
- ccid2_change_l_ack_ratio(sk, hc->tx_cwnd);
+ if (ccid2_ack_ratio_next(sk) > hc->tx_cwnd)
+ ccid2_change_l_ack_ratio(sk, hc->tx_cwnd/2 ? : 1U);
}
static int ccid2_hc_tx_parse_options(struct sock *sk, u8 packet_type,
diff --git a/net/dccp/dccp.h b/net/dccp/dccp.h
index 26cba68..e8cf5a7 100644
--- a/net/dccp/dccp.h
+++ b/net/dccp/dccp.h
@@ -491,6 +491,7 @@ static inline int dccp_ack_pending(const struct sock *sk)
}
extern int dccp_feat_signal_nn_change(struct sock *sk, u8 feat, u64 nn_val);
+extern __u64 dccp_feat_get_nn_next_val(struct sock *sk, u8 feat);
extern int dccp_feat_finalise_settings(struct dccp_sock *dp);
extern int dccp_feat_server_ccid_dependencies(struct dccp_request_sock *dreq);
extern int dccp_feat_insert_opts(struct dccp_sock*, struct dccp_request_sock*,
diff --git a/net/dccp/feat.c b/net/dccp/feat.c
index 9a8c2ba..bc26f33 100644
--- a/net/dccp/feat.c
+++ b/net/dccp/feat.c
@@ -775,12 +775,7 @@ int dccp_feat_register_sp(struct sock *sk, u8 feat, u8 is_local,
* @sk: DCCP socket of an established connection
* @feat: NN feature number from %dccp_feature_numbers
* @nn_val: the new value to use
- * This function is used to communicate NN updates out-of-band. The difference
- * to feature negotiation during connection setup is that values are activated
- * immediately after validation, i.e. we don't wait for the Confirm: either the
- * value is accepted by the peer (and then the waiting is futile), or it is not
- * (Reset or empty Confirm). We don't accept empty Confirms - transmitted values
- * are validated, and the peer "MUST accept any valid value" (RFC 4340, 6.3.2).
+ * This function is used to communicate NN updates out-of-band.
*/
int dccp_feat_signal_nn_change(struct sock *sk, u8 feat, u64 nn_val)
{
@@ -805,9 +800,6 @@ int dccp_feat_signal_nn_change(struct sock *sk, u8 feat, u64 nn_val)
dccp_feat_list_pop(entry);
}
- if (dccp_feat_activate(sk, feat, 1, &fval))
- return -EADV;
-
inet_csk_schedule_ack(sk);
return dccp_feat_push_change(fn, feat, 1, 0, &fval);
}
@@ -1356,6 +1348,9 @@ static u8 dccp_feat_handle_nn_established(struct sock *sk, u8 mandatory, u8 opt,
if (fval.nn != entry->val.nn)
return 0;
+ /* Only activate after receiving the Confirm option (6.6.1). */
+ dccp_feat_activate(sk, feat, local, &fval);
+
/* It has been confirmed - so remove the entry */
dccp_feat_list_pop(entry);
@@ -1374,6 +1369,39 @@ fast_path_failed:
: DCCP_RESET_CODE_OPTION_ERROR;
}
+/*
+* dccp_feat_get_nn_next_val - Get the currently negotiating value of an NN
+* feature. If the feature isn't being currently
+* negotiated, return it's current value.
+* @sk: DCCP socket of an established connection
+* @feat: NN feature number from %dccp_feature_numbers
+*/
+__u64 dccp_feat_get_nn_next_val(struct sock *sk, u8 feat)
+{
+ struct list_head *fn = &dccp_sk(sk)->dccps_featneg;
+ struct dccp_feat_entry *entry;
+ u8 type = dccp_feat_type(feat);
+
+ if (type == FEAT_UNKNOWN || type != FEAT_NN)
+ return 0;
+
+ entry = dccp_feat_list_lookup(fn, feat, 1);
+ if (entry != NULL)
+ return entry->val.nn;
+
+ switch (feat) {
+ case DCCPF_ACK_RATIO:
+ return dccp_sk(sk)->dccps_l_ack_ratio;
+ case DCCPF_SEQUENCE_WINDOW:
+ return dccp_sk(sk)->dccps_l_seq_win;
+ default:
+ dccp_pr_debug("Attempting to get value \
+ for unknown NN feature");
+ }
+return 0;
+}
+EXPORT_SYMBOL_GPL(dccp_feat_get_nn_next_val);
+
/**
* dccp_feat_parse_options - Process Feature-Negotiation Options
* @sk: for general use and used by the client during connection setup
Download attachment "signature.asc" of type "application/pgp-signature" (199 bytes)
Powered by blists - more mailing lists