[PATCH 8/14]: Remove manual influence on NDP Count feature

[PATCH 8/14]: Remove manual influence on NDP Count feature

[Gerrit Renker]

[DCCP]: Remove manual influence on NDP Count feature

The NDP count feature is handled automatically now by the feature negotiation code:
 * for CCID2 it is disabled, since the code does not use or even refer to NDP counts;
 * for CCID3 it is enabled, since NDP counts are used to determine loss lengths (RFC 4342, 6.1).

Allowing the user to change these values leads to unpredictable and failing behaviour, since it 
is then possible to disable NDP counts even when they are needed (e.g. in CCID3). This means that
only those user settings are sensible that agree with the values for Send NDP Count implied by the
choice of CCID. But those settings are already activated by the feature negotiation (CCID dependency
tracking), which means that the user settings are redundant. 

I thus think that it is the best choice to remove this sysctl. Leaving it in brings no
advantages, but opens a door to creating non-operable conditions.

At startup the initialisation of the NDP count feature is with the default value of 0, which is
done implicitly by the zeroing-out of the socket when it is allocated. If the choice of CCID or
feature negotiation enables NDP count, this will then be updated via the NDP activation handler.

Signed-off-by: Gerrit Renker <gerrit@erg.abdn.ac.uk>
---
 Documentation/networking/dccp.txt |    3 ---
 net/dccp/dccp.h                   |    1 -
 net/dccp/minisocks.c              |    1 -
 net/dccp/sysctl.c                 |    8 --------
 4 files changed, 13 deletions(-)

--- a/net/dccp/sysctl.c
+++ b/net/dccp/sysctl.c
@@ -20,7 +20,6 @@ int sysctl_dccp_feat_rx_ccid	     = DCCP
 int sysctl_dccp_feat_tx_ccid	     = DCCPF_INITIAL_CCID;
 int sysctl_dccp_feat_ack_ratio	     = DCCPF_INITIAL_ACK_RATIO;
 int sysctl_dccp_feat_send_ack_vector = DCCPF_INITIAL_SEND_ACK_VECTOR;
-int sysctl_dccp_feat_send_ndp_count  = DCCPF_INITIAL_SEND_NDP_COUNT;
 
 /* the maximum queue length for tx in packets. 0 is no limit */
 int sysctl_dccp_tx_qlen		__read_mostly = 5;
@@ -72,13 +71,6 @@ static struct ctl_table dccp_default_tab
 		.proc_handler	= proc_dointvec,
 	},
 	{
-		.procname	= "send_ndp",
-		.data		= &sysctl_dccp_feat_send_ndp_count,
-		.maxlen		= sizeof(sysctl_dccp_feat_send_ndp_count),
-		.mode		= 0644,
-		.proc_handler	= proc_dointvec,
-	},
-	{
 		.procname	= "request_retries",
 		.data		= &sysctl_dccp_request_retries,
 		.maxlen		= sizeof(sysctl_dccp_request_retries),
--- a/Documentation/networking/dccp.txt
+++ b/Documentation/networking/dccp.txt
@@ -133,9 +133,6 @@ retries2
 	importance for retransmitted acknowledgments and feature negotiation,
 	data packets are never retransmitted. Analogue of tcp_retries2.
 
-send_ndp = 1
-	Whether or not to send NDP count options (sec. 7.7.2).
-
 tx_ccid = 2
 	Default CCID for the sender-receiver half-connection. Depending on the
 	choice of CCID, the Send Ack Vector feature is enabled automatically.
--- a/net/dccp/dccp.h
+++ b/net/dccp/dccp.h
@@ -100,7 +100,6 @@ extern int  sysctl_dccp_feat_rx_ccid;
 extern int  sysctl_dccp_feat_tx_ccid;
 extern int  sysctl_dccp_feat_ack_ratio;
 extern int  sysctl_dccp_feat_send_ack_vector;
-extern int  sysctl_dccp_feat_send_ndp_count;
 extern int  sysctl_dccp_tx_qlen;
 extern int  sysctl_dccp_sync_ratelimit;
 
--- a/net/dccp/minisocks.c
+++ b/net/dccp/minisocks.c
@@ -47,7 +47,6 @@ void dccp_minisock_init(struct dccp_mini
 	dmsk->dccpms_sequence_window = sysctl_dccp_feat_sequence_window;
 	dmsk->dccpms_ack_ratio	     = sysctl_dccp_feat_ack_ratio;
 	dmsk->dccpms_send_ack_vector = sysctl_dccp_feat_send_ack_vector;
-	dmsk->dccpms_send_ndp_count  = sysctl_dccp_feat_send_ndp_count;
 }
 
 void dccp_time_wait(struct sock *sk, int state, int timeo)

Re: [PATCH 8/14]: Remove manual influence on NDP Count feature

[Eddie Kohler]

One slight comment.  NDP Count is not required by CCID 3's *specification*. 
Without NDP Count, the receiver will simply treat ack losses like data losses; 
this can lead to lower send rates when acks are lost, but does not harm 
interoperability.  However, it is totally reasonable for an *implementation* 
such as Linux to require NDP Count for CCID 3, so there are no problems with 
the patch.

Eddie


Gerrit Renker wrote:
> [DCCP]: Remove manual influence on NDP Count feature
> 
> The NDP count feature is handled automatically now by the feature negotiation code:
>  * for CCID2 it is disabled, since the code does not use or even refer to NDP counts;
>  * for CCID3 it is enabled, since NDP counts are used to determine loss lengths (RFC 4342, 6.1).
> 
> Allowing the user to change these values leads to unpredictable and failing behaviour, since it 
> is then possible to disable NDP counts even when they are needed (e.g. in CCID3). This means that
> only those user settings are sensible that agree with the values for Send NDP Count implied by the
> choice of CCID. But those settings are already activated by the feature negotiation (CCID dependency
> tracking), which means that the user settings are redundant. 
> 
> I thus think that it is the best choice to remove this sysctl. Leaving it in brings no
> advantages, but opens a door to creating non-operable conditions.
> 
> At startup the initialisation of the NDP count feature is with the default value of 0, which is
> done implicitly by the zeroing-out of the socket when it is allocated. If the choice of CCID or
> feature negotiation enables NDP count, this will then be updated via the NDP activation handler.
> 
> Signed-off-by: Gerrit Renker <gerrit@erg.abdn.ac.uk>
> ---
>  Documentation/networking/dccp.txt |    3 ---
>  net/dccp/dccp.h                   |    1 -
>  net/dccp/minisocks.c              |    1 -
>  net/dccp/sysctl.c                 |    8 --------
>  4 files changed, 13 deletions(-)
> 
> --- a/net/dccp/sysctl.c
> +++ b/net/dccp/sysctl.c
> @@ -20,7 +20,6 @@ int sysctl_dccp_feat_rx_ccid	     = DCCP
>  int sysctl_dccp_feat_tx_ccid	     = DCCPF_INITIAL_CCID;
>  int sysctl_dccp_feat_ack_ratio	     = DCCPF_INITIAL_ACK_RATIO;
>  int sysctl_dccp_feat_send_ack_vector = DCCPF_INITIAL_SEND_ACK_VECTOR;
> -int sysctl_dccp_feat_send_ndp_count  = DCCPF_INITIAL_SEND_NDP_COUNT;
>  
>  /* the maximum queue length for tx in packets. 0 is no limit */
>  int sysctl_dccp_tx_qlen		__read_mostly = 5;
> @@ -72,13 +71,6 @@ static struct ctl_table dccp_default_tab
>  		.proc_handler	= proc_dointvec,
>  	},
>  	{
> -		.procname	= "send_ndp",
> -		.data		= &sysctl_dccp_feat_send_ndp_count,
> -		.maxlen		= sizeof(sysctl_dccp_feat_send_ndp_count),
> -		.mode		= 0644,
> -		.proc_handler	= proc_dointvec,
> -	},
> -	{
>  		.procname	= "request_retries",
>  		.data		= &sysctl_dccp_request_retries,
>  		.maxlen		= sizeof(sysctl_dccp_request_retries),
> --- a/Documentation/networking/dccp.txt
> +++ b/Documentation/networking/dccp.txt
> @@ -133,9 +133,6 @@ retries2
>  	importance for retransmitted acknowledgments and feature negotiation,
>  	data packets are never retransmitted. Analogue of tcp_retries2.
>  
> -send_ndp = 1
> -	Whether or not to send NDP count options (sec. 7.7.2).
> -
>  tx_ccid = 2
>  	Default CCID for the sender-receiver half-connection. Depending on the
>  	choice of CCID, the Send Ack Vector feature is enabled automatically.
> --- a/net/dccp/dccp.h
> +++ b/net/dccp/dccp.h
> @@ -100,7 +100,6 @@ extern int  sysctl_dccp_feat_rx_ccid;
>  extern int  sysctl_dccp_feat_tx_ccid;
>  extern int  sysctl_dccp_feat_ack_ratio;
>  extern int  sysctl_dccp_feat_send_ack_vector;
> -extern int  sysctl_dccp_feat_send_ndp_count;
>  extern int  sysctl_dccp_tx_qlen;
>  extern int  sysctl_dccp_sync_ratelimit;
>  
> --- a/net/dccp/minisocks.c
> +++ b/net/dccp/minisocks.c
> @@ -47,7 +47,6 @@ void dccp_minisock_init(struct dccp_mini
>  	dmsk->dccpms_sequence_window = sysctl_dccp_feat_sequence_window;
>  	dmsk->dccpms_ack_ratio	     = sysctl_dccp_feat_ack_ratio;
>  	dmsk->dccpms_send_ack_vector = sysctl_dccp_feat_send_ack_vector;
> -	dmsk->dccpms_send_ndp_count  = sysctl_dccp_feat_send_ndp_count;
>  }
>  
>  void dccp_time_wait(struct sock *sk, int state, int timeo)
> -
> To unsubscribe from this list: send the line "unsubscribe dccp" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 8/14]: Remove manual influence on NDP Count feature

[Ian McDonald]

On 10/4/07, Gerrit Renker <gerrit@erg.abdn.ac.uk> wrote:
> [DCCP]: Remove manual influence on NDP Count feature
>
> The NDP count feature is handled automatically now by the feature negotiation code:
>  * for CCID2 it is disabled, since the code does not use or even refer to NDP counts;
>  * for CCID3 it is enabled, since NDP counts are used to determine loss lengths (RFC 4342, 6.1).
>
> Allowing the user to change these values leads to unpredictable and failing behaviour, since it
> is then possible to disable NDP counts even when they are needed (e.g. in CCID3). This means that
> only those user settings are sensible that agree with the values for Send NDP Count implied by the
> choice of CCID. But those settings are already activated by the feature negotiation (CCID dependency
> tracking), which means that the user settings are redundant.
>
> I thus think that it is the best choice to remove this sysctl. Leaving it in brings no
> advantages, but opens a door to creating non-operable conditions.
>
> At startup the initialisation of the NDP count feature is with the default value of 0, which is
> done implicitly by the zeroing-out of the socket when it is allocated. If the choice of CCID or
> feature negotiation enables NDP count, this will then be updated via the NDP activation handler.
>
> Signed-off-by: Gerrit Renker <gerrit@erg.abdn.ac.uk>

Acked-by: Ian McDonald <ian.mcdonald@jandi.co.nz>