Quoting Samuel Jero:
| Remove Feature Negotiation Confirm Options from the list of options to
| send after they are sent once because such options are NOT supposed to
| be retransmitted and are ONLY supposed to be sent in response to a
| Change Option (RFC 4340 6.2).
|
| --- a/net/dccp/feat.c
| +++ b/net/dccp/feat.c
| @@ -690,11 +690,18 @@ int dccp_feat_insert_opts(struct dccp_sock *dp, struct dccp_request_sock *dreq,
| return -1;
| if (pos->needs_mandatory && dccp_insert_option_mandatory(skb))
| return -1;
| - /*
| - * Enter CHANGING after transmitting the Change option (6.6.2).
| - */
| - if (pos->state == FEAT_INITIALISING)
| - pos->state = FEAT_CHANGING;
| +
| + if (opt == DCCPO_CONFIRM_R || opt == DCCPO_CONFIRM_L) {
| + /*Confirms don't get retransmitted (6.6.3)*/
| + dccp_feat_list_pop(pos);
| + } else {
| + /*
| + * Enter CHANGING after transmitting
| + * the Change option (6.6.2).
| + */
| + if (pos->state == FEAT_INITIALISING)
| + pos->state = FEAT_CHANGING;
| + }
Your thinking is correct, but this needs more work.
In fact your code is almost the same as the one that had been reverted earlier:
if (pos->state == FEAT_INITIALISING)
pos->state = FEAT_CHANGING;
else if (pos->needs_confirm && dreq == NULL)
dccp_feat_list_pop(pos);
where the else-if part has been removed since it lead to several bugs when packets
got lost during the initial handshake. The problem is that client and server have
only 3 packets to get it right, if only 1 packet gets lost, and then retransmitted,
the negotiation does not complete.
( This part was only one half of the change, the other is in proto.c:dccp_set_state()
/* Client retransmits all Confirm options until entering OPEN */
if (oldstate == DCCP_PARTOPEN)
dccp_feat_list_purge(&dccp_sk(sk)->dccps_featneg);
)
It seems to me that what you would like to do is to send Confirm options only once
in established state? Then the dccp_feat_list_pop() should be state-dependent, i.e.
only apply in states > PARTOPEN. Otherwise, we will have the same bugs for the
initial feature negotiation that had already been fixed. This is documented on:
http://eden-feed.erg.abdn.ac.uk/dccp/notes/feature_negotiation/why_retransmit_confirms
Connection-state dependent handling of negotiation options is in dccp_feat_parse_options(),
don't have a patch to accomplish both the above at the moment.
--
To unsubscribe from this list: send the line "unsubscribe dccp" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
