[PATCH 2/9]: Query supported CCIDs

[PATCH 2/9]: Query supported CCIDs

[Gerrit Renker]

[DCCP]: Query supported CCIDs

This provides a data structure to record which CCIDs are locally supported
and three accessor functions:
 - a test function for internal use which is used to validate CCID requests
   made by the user;
 - a copy function so that the list can be used for feature-negotiation;   
 - documented getsockopt() support so that the user can query capabilities.

The data structure is a table which is filled in at compile-time with the
list of available CCIDs (which in turn depends on the Kconfig choices).

Using the copy function for cloning the list of supported CCIDs is useful for
feature negotiation, since the negotiation is now with the full list of available
CCIDs (e.g. {2, 3}) instead of the default value {2}. This means negotiation will
not fail if the peer requests to use CCID3 instead of CCID2. 

Signed-off-by: Gerrit Renker <gerrit@erg.abdn.ac.uk>
---
 Documentation/networking/dccp.txt |    4 +++
 include/linux/dccp.h              |    1 
 net/dccp/ccid.c                   |   48 ++++++++++++++++++++++++++++++++++++++
 net/dccp/ccid.h                   |    5 +++
 net/dccp/feat.c                   |    4 +++
 net/dccp/proto.c                  |    2 +
 6 files changed, 64 insertions(+)

--- a/net/dccp/ccid.h
+++ b/net/dccp/ccid.h
@@ -80,6 +80,11 @@ static inline void *ccid_priv(const stru
 	return (void *)ccid->ccid_priv;
 }
 
+extern int  ccid_get_default_ccids(u8 **ccid_array, u8 *array_len);
+extern bool ccid_support_check(u8 const *ccid_array, u8 array_len);
+extern int  ccid_getsockopt_available_ccids(struct sock *sk, int len,
+					    char __user *, int __user *);
+
 extern struct ccid *ccid_new(unsigned char id, struct sock *sk, int rx,
 			     gfp_t gfp);
 
--- a/net/dccp/ccid.c
+++ b/net/dccp/ccid.c
@@ -13,6 +13,13 @@
 
 #include "ccid.h"
 
+static u8 builtin_ccids[] = {
+	DCCPC_CCID2,		/* CCID2 is supported by default */
+#if defined(CONFIG_IP_DCCP_CCID3) || defined(CONFIG_IP_DCCP_CCID3_MODULE)
+	DCCPC_CCID3,
+#endif
+};
+
 static struct ccid_operations *ccids[CCID_MAX];
 #if defined(CONFIG_SMP) || defined(CONFIG_PREEMPT)
 static atomic_t ccids_lockct = ATOMIC_INIT(0);
@@ -86,6 +93,47 @@ static void ccid_kmem_cache_destroy(stru
 	}
 }
 
+/* check that up to @array_len members in @ccid_array are supported */
+bool ccid_support_check(u8 const *ccid_array, u8 array_len)
+{
+	u8 i, j, found;
+
+	for (i = 0, found = 0; i < array_len; i++, found = 0) {
+		for (j = 0; !found && j < ARRAY_SIZE(builtin_ccids); j++)
+			found = (ccid_array[i] = builtin_ccids[j]);
+		if (!found)
+			return false;
+	}
+	return true;
+}
+
+/**
+ * ccid_get_default_ccids  -  Provide copy of `builtin' CCID array
+ * @ccid_array: pointer to copy into
+ * @array_len: value to return length into
+ * This function allocates memory - caller must see that it is freed after use.
+ */
+int ccid_get_default_ccids(u8 **ccid_array, u8 *array_len)
+{
+	*ccid_array = kmemdup(builtin_ccids, sizeof(builtin_ccids), gfp_any());
+	if (*ccid_array = NULL)
+		return -ENOBUFS;
+	*array_len = ARRAY_SIZE(builtin_ccids);
+	return 0;
+}
+
+int ccid_getsockopt_available_ccids(struct sock *sk, int len,
+				    char __user *optval, int __user *optlen)
+{
+	if (len < sizeof(builtin_ccids))
+		return -EINVAL;
+
+	if (put_user(sizeof(builtin_ccids), optlen) ||
+	    copy_to_user(optval, builtin_ccids, sizeof(builtin_ccids)))
+		return -EFAULT;
+	return 0;
+}
+
 int ccid_register(struct ccid_operations *ccid_ops)
 {
 	int err = -ENOBUFS;
--- a/net/dccp/feat.c
+++ b/net/dccp/feat.c
@@ -374,6 +374,10 @@ static int dccp_feat_register_sp(struct 
 	    !dccp_feat_sp_list_ok(feat, sp_val, sp_len))
 		return -EINVAL;
 
+	/* Avoid negotiating alien CCIDs by only advertising supported ones */
+	if (feat = DCCPF_CCID && !ccid_support_check(sp_val, sp_len))
+		return -EOPNOTSUPP;
+
 	if (dccp_feat_clone_sp_val(&fval, sp_val, sp_len))
 		return -ENOMEM;
 
--- a/Documentation/networking/dccp.txt
+++ b/Documentation/networking/dccp.txt
@@ -57,6 +57,10 @@ can be set before calling bind().
 DCCP_SOCKOPT_GET_CUR_MPS is read-only and retrieves the current maximum packet
 size (application payload size) in bytes, see RFC 4340, section 14.
 
+DCCP_SOCKOPT_AVAILABLE_CCIDS is also read-only and returns the list of CCIDs
+supported by the endpoint (see include/linux/dccp.h for symbolic constants).
+The caller needs to provide a sufficiently large (> 2) array of type uint8_t.
+
 DCCP_SOCKOPT_SERVER_TIMEWAIT enables the server (listening socket) to hold
 timewait state when closing the connection (RFC 4340, 8.3). The usual case is
 that the closing server sends a CloseReq, whereupon the client holds timewait
--- a/include/linux/dccp.h
+++ b/include/linux/dccp.h
@@ -212,6 +212,7 @@ struct dccp_so_feat {
 #define DCCP_SOCKOPT_SERVER_TIMEWAIT	6
 #define DCCP_SOCKOPT_SEND_CSCOV		10
 #define DCCP_SOCKOPT_RECV_CSCOV		11
+#define DCCP_SOCKOPT_AVAILABLE_CCIDS	12
 #define DCCP_SOCKOPT_CCID_RX_INFO	128
 #define DCCP_SOCKOPT_CCID_TX_INFO	192
 
--- a/net/dccp/proto.c
+++ b/net/dccp/proto.c
@@ -617,6 +617,8 @@ static int do_dccp_getsockopt(struct soc
 	case DCCP_SOCKOPT_GET_CUR_MPS:
 		val = dp->dccps_mss_cache;
 		break;
+	case DCCP_SOCKOPT_AVAILABLE_CCIDS:
+		return ccid_getsockopt_available_ccids(sk, len, optval, optlen);
 	case DCCP_SOCKOPT_SERVER_TIMEWAIT:
 		val = dp->dccps_server_timewait;
 		break;

Re: [PATCH 2/9]: Query supported CCIDs

[Arnaldo Carvalho de Melo]

Em Fri, Sep 28, 2007 at 04:30:17PM +0100, Gerrit Renker escreveu:
> [DCCP]: Query supported CCIDs
> 
> This provides a data structure to record which CCIDs are locally supported
> and three accessor functions:
>  - a test function for internal use which is used to validate CCID requests
>    made by the user;
>  - a copy function so that the list can be used for feature-negotiation;   
>  - documented getsockopt() support so that the user can query capabilities.
> 
> The data structure is a table which is filled in at compile-time with the
> list of available CCIDs (which in turn depends on the Kconfig choices).
> 
> Using the copy function for cloning the list of supported CCIDs is useful for
> feature negotiation, since the negotiation is now with the full list of available
> CCIDs (e.g. {2, 3}) instead of the default value {2}. This means negotiation will
> not fail if the peer requests to use CCID3 instead of CCID2. 
> 
> Signed-off-by: Gerrit Renker <gerrit@erg.abdn.ac.uk>
> ---
>  Documentation/networking/dccp.txt |    4 +++
>  include/linux/dccp.h              |    1 
>  net/dccp/ccid.c                   |   48 ++++++++++++++++++++++++++++++++++++++
>  net/dccp/ccid.h                   |    5 +++
>  net/dccp/feat.c                   |    4 +++
>  net/dccp/proto.c                  |    2 +
>  6 files changed, 64 insertions(+)
> 
> --- a/net/dccp/ccid.h
> +++ b/net/dccp/ccid.h
> @@ -80,6 +80,11 @@ static inline void *ccid_priv(const stru
>  	return (void *)ccid->ccid_priv;
>  }
>  
> +extern int  ccid_get_default_ccids(u8 **ccid_array, u8 *array_len);
> +extern bool ccid_support_check(u8 const *ccid_array, u8 array_len);
> +extern int  ccid_getsockopt_available_ccids(struct sock *sk, int len,
> +					    char __user *, int __user *);
> +
>  extern struct ccid *ccid_new(unsigned char id, struct sock *sk, int rx,
>  			     gfp_t gfp);
>  
> --- a/net/dccp/ccid.c
> +++ b/net/dccp/ccid.c
> @@ -13,6 +13,13 @@
>  
>  #include "ccid.h"
>  
> +static u8 builtin_ccids[] = {
> +	DCCPC_CCID2,		/* CCID2 is supported by default */
> +#if defined(CONFIG_IP_DCCP_CCID3) || defined(CONFIG_IP_DCCP_CCID3_MODULE)
> +	DCCPC_CCID3,
> +#endif
> +};

This considers only code that was already merged. What about a new CCID
that is not yet merged? I guess the table should be managed at ccid
module registration time... but that would prevent autoloading on
demand, so I think that a sysctl interface that tells the DCCP core
infrastructure which are the ccids that are available is better.

Think also about an administrator deciding that even with the Linux
kernel provided by his vendor containing modules for ccid2, 3, etc, he
only accepts ccid3 or some other subset.

We could have as default a table built with the modules we know at dccp
core build time to be available in the current kernel build, but not
prevent others from being added or existing ones to be disabled.

So I think we can start with what you provided and implement my
suggestions later.

- Arnaldo

Re: [PATCH 2/9]: Query supported CCIDs

[Gerrit Renker]

Quoting Arnaldo Carvalho de Melo:
|  > +static u8 builtin_ccids[] = {
|  > +     DCCPC_CCID2,            /* CCID2 is supported by default */
|  > +#if defined(CONFIG_IP_DCCP_CCID3) || defined(CONFIG_IP_DCCP_CCID3_MODULE)
|  > +     DCCPC_CCID3,
|  > +#endif
|  > +};
|  
|  This considers only code that was already merged. What about a new CCID
|  that is not yet merged? I guess the table should be managed at ccid
|  module registration time... but that would prevent autoloading on
|  demand, so I think that a sysctl interface that tells the DCCP core
|  infrastructure which are the ccids that are available is better.
|
For each new CCID (which is likely to have a Kconfig option), the table would
need a new entry. The table is not necessary for autoloading, this is handled
separately, and there is flexibility in handling this. Currently (subsequent
patch), all advertised CCIDs are autoloaded at socket creation time (only if
this is the first socket call, not when the CCID modules are already loaded),
since ccid_new() is later only called in interrupt context. But this has the
nice byproduct that module loading is no longer needed - firing up the applications
at either end is sufficient to have everything loaded.
 
|  Think also about an administrator deciding that even with the Linux
|  kernel provided by his vendor containing modules for ccid2, 3, etc, he
|  only accepts ccid3 or some other subset.
That is an interesting thought - defining system-wide policies. The above table is
much more primitive, it mainly is book-keeping to see what is there.
Do you mean something like net.ipv4.tcp_available_congestion_control ?

|  
|  We could have as default a table built with the modules we know at dccp
|  core build time to be available in the current kernel build, but not
|  prevent others from being added or existing ones to be disabled.
The disabling can also be done on a per-connection basis, via socket options.
One pain with the server-priority negotiation (and reason for the table) is that
when only a single value is used, then it only works if both ends happen to hit
the same value. One can exploit this fact to limit the choices.


|  So I think we can start with what you provided and implement my
|  suggestions later.
I am saving this in my todo folder and hope we can continue this thread.

Re: [PATCH 2/9]: Query supported CCIDs

[Arnaldo Carvalho de Melo]

Em Sat, Sep 29, 2007 at 05:24:18PM +0100, Gerrit Renker escreveu:
> Quoting Arnaldo Carvalho de Melo:
> |  > +static u8 builtin_ccids[] = {
> |  > +     DCCPC_CCID2,            /* CCID2 is supported by default */
> |  > +#if defined(CONFIG_IP_DCCP_CCID3) || defined(CONFIG_IP_DCCP_CCID3_MODULE)
> |  > +     DCCPC_CCID3,
> |  > +#endif
> |  > +};
> |  
> |  This considers only code that was already merged. What about a new CCID
> |  that is not yet merged? I guess the table should be managed at ccid
> |  module registration time... but that would prevent autoloading on
> |  demand, so I think that a sysctl interface that tells the DCCP core
> |  infrastructure which are the ccids that are available is better.
> |
> For each new CCID (which is likely to have a Kconfig option), the table would
> need a new entry. The table is not necessary for autoloading, this is handled
> separately, and there is flexibility in handling this. Currently (subsequent
> patch), all advertised CCIDs are autoloaded at socket creation time (only if
> this is the first socket call, not when the CCID modules are already loaded),
> since ccid_new() is later only called in interrupt context. But this has the
> nice byproduct that module loading is no longer needed - firing up the applications
> at either end is sufficient to have everything loaded.
>  
> |  Think also about an administrator deciding that even with the Linux
> |  kernel provided by his vendor containing modules for ccid2, 3, etc, he
> |  only accepts ccid3 or some other subset.
> That is an interesting thought - defining system-wide policies. The above table is
> much more primitive, it mainly is book-keeping to see what is there.
> Do you mean something like net.ipv4.tcp_available_congestion_control ?

I haven't looked, but perhaps, its almost the same problem space. Almost
because TCP doesn't have feature negotiation, its the app that can use a
setsockopt to say what is the cong control module it wants to use,
despite what the other end wants, heck, the other end can and probably
will not even know if there were different cong control modules.
 
> |  
> |  We could have as default a table built with the modules we know at dccp
> |  core build time to be available in the current kernel build, but not
> |  prevent others from being added or existing ones to be disabled.
> The disabling can also be done on a per-connection basis, via socket options.
> One pain with the server-priority negotiation (and reason for the table) is that
> when only a single value is used, then it only works if both ends happen to hit
> the same value. One can exploit this fact to limit the choices.
> 
> 
> |  So I think we can start with what you provided and implement my
> |  suggestions later.
> I am saving this in my todo folder and hope we can continue this thread.

OK! For now my thoughts are more on what-is-best-in-the-long-term, what
you have done so far is OK by me and I'll be reviewing those patches
soon. One thing is the pointer in the request_sock case that I have to
figure out what is best and possibly rework your patches to provide what
I think is best.

- Arnaldo

Re: [PATCH 2/9]: Query supported CCIDs

[Gerrit Renker]

|  One thing is the pointer in the request_sock case that I have to
|  figure out what is best and possibly rework your patches to provide what
|  I think is best.
|  
I am looking forward to that - if you want me to change single patches, let me know.

I had one more thought regarding timestamping: currently CCID2 uses the old RFC2988 
RTT mechanism from Jacobson's old 1988 paper. RFC 4341 says that the sampling should 
be done at most once per cwnd, but RFC 1323 says that this is insufficient for 
high-speed networking. And today it is almost impossible to buy a computer /not/ with
a Gbps ethernet card.

In short - if you change the architecture, is is possible to keep a door open to
add a timestamping-based RTT algorithm to CCID2? That has been your suggestion 
anyway some emails ago and I like the idea.

There is further stuff that can be shared - I have some ideas for using the ICSK_RETRANS
to replace a lot of code duplication for the RTO timer in CCID2. Would make sense, since
in OPEN/PARTOPEN state the retransmit timer is never used (only for Request/Close/CloseReq).

Re: [PATCH 2/9]: Query supported CCIDs

[Ian McDonald]

On 9/29/07, Gerrit Renker <gerrit@erg.abdn.ac.uk> wrote:
> [DCCP]: Query supported CCIDs
>
> This provides a data structure to record which CCIDs are locally supported
> and three accessor functions:
>  - a test function for internal use which is used to validate CCID requests
>    made by the user;
>  - a copy function so that the list can be used for feature-negotiation;
>  - documented getsockopt() support so that the user can query capabilities.
>
> The data structure is a table which is filled in at compile-time with the
> list of available CCIDs (which in turn depends on the Kconfig choices).
>
> Using the copy function for cloning the list of supported CCIDs is useful for
> feature negotiation, since the negotiation is now with the full list of available
> CCIDs (e.g. {2, 3}) instead of the default value {2}. This means negotiation will
> not fail if the peer requests to use CCID3 instead of CCID2.
>
> Signed-off-by: Gerrit Renker <gerrit@erg.abdn.ac.uk>
Acked-by: Ian McDonald <ian.mcdonald@jandi.co.nz>