[PATCH 7/7] [DCCP]: Remove socket option

[PATCH 7/7] [DCCP]: Remove socket option

[Ian McDonald]

This removes DCCP_SOCKOPT_PACKET_SIZE for two reasons:
* the current code doesn't work
* tx and rx should be different (introduced in former patch)

Signed-off-by: Ian McDonald <ian.mcdonald@jandi.co.nz>
---
diff --git a/include/linux/dccp.h b/include/linux/dccp.h
index ef1c57b..18fbbb4 100644
--- a/include/linux/dccp.h
+++ b/include/linux/dccp.h
@@ -196,7 +196,6 @@ struct dccp_so_feat {
 };
 
 /* DCCP socket options */
-#define DCCP_SOCKOPT_PACKET_SIZE	1
 #define DCCP_SOCKOPT_SERVICE		2
 #define DCCP_SOCKOPT_CHANGE_L		3
 #define DCCP_SOCKOPT_CHANGE_R		4
@@ -465,7 +464,6 @@ struct dccp_sock {
 	struct dccp_service_list	*dccps_service_list;
 	struct timeval			dccps_timestamp_time;
 	__u32				dccps_timestamp_echo;
-	__u32				dccps_packet_size;
 	__u16				dccps_l_ack_ratio;
 	__u16				dccps_r_ack_ratio;
 	unsigned long			dccps_ndp_count;
diff --git a/net/dccp/proto.c b/net/dccp/proto.c
index c8f7d5a..c8c884e 100644
--- a/net/dccp/proto.c
+++ b/net/dccp/proto.c
@@ -458,7 +458,6 @@ out_free_val:
 static int do_dccp_setsockopt(struct sock *sk, int level, int optname,
 		char __user *optval, int optlen)
 {
-	struct dccp_sock *dp = dccp_sk(sk);
 	struct dccp_minisock *dmsk = dccp_msk(sk);
 	struct ccid3_hc_tx_sock *hctx;
 	struct ccid3_hc_rx_sock *hcrx;
@@ -478,10 +477,6 @@ static int do_dccp_setsockopt(struct soc
 	err = 0;
 
 	switch (optname) {
-	case DCCP_SOCKOPT_PACKET_SIZE:
-		dp->dccps_packet_size = val;
-		break;
-
 	case DCCP_SOCKOPT_CHANGE_L:
 		if (optlen != sizeof(struct dccp_so_feat))
 			err = -EINVAL;
@@ -605,10 +600,6 @@ static int do_dccp_getsockopt(struct soc
 		return -EINVAL;
 
 	switch (optname) {
-	case DCCP_SOCKOPT_PACKET_SIZE:
-		val = dp->dccps_packet_size;
-		len = sizeof(dp->dccps_packet_size);
-		break;
 	case DCCP_SOCKOPT_TX_PACKET_SIZE:
 		if (dmsk->dccpms_tx_ccid != DCCPC_CCID3)
 			return -EINVAL;

Re: [PATCH 7/7] [DCCP]: Remove socket option

[Gerrit Renker]

|  This removes DCCP_SOCKOPT_PACKET_SIZE for two reasons:
|  * the current code doesn't work
|  * tx and rx should be different (introduced in former patch)

Agree that DCCP_SOCKOPT_PACKET_SIZE is redundant, patch looks good and
has been tested.

Acked-by: Gerrit Renker <gerrit@erg.abdn.ac.uk>
|  Signed-off-by: Ian McDonald <ian.mcdonald@jandi.co.nz>

Re: [PATCH 7/7] [DCCP]: Remove socket option

[Gerrit Renker]

I think that the PACKET_SIZE socket option is redundant, since
it can be derived from
 * the buflen argument of dccp_sendmsg()
 * the size of received packets
independent of whether the CCID is for fixed packet sizes or not.

Virtually all known CCIDs have fixed packet sizes; for such CCIDs
there are two cases:

 (i)  buffer length never changes and is equal to PACKET_SIZE
      --> then PACKET_SIZE is clearly redundant
 (ii) PACKET_SIZE gives the intention of an average packet size
      which implies that PACKET_SIZE gives an average of buflen

So the relevant case to consider is (ii) and here I think that
using a PACKET_SIZE option is also redundant: the application
could simply lie and state the wrong PACKET_SIZE.

Below are excerpts from the literature to substantiate the above.

Fixed-size CCIDS 2 and 3
============
Both CCID 2 and 3 are meant for applications that use fixed packet sizes.

* [RFC 4341, sec. 5.3]: "CCID 2 is optimized for applications that
  generally use a fixed packet size and vary their sending rate in
  packets per second in response to congestion."

* [RFC 4341, sec. 5.3]: "CCID 3 is intended for applications that use
  a fixed packet size, and that vary their sending rate in packets per
  second in response to congestion."

* [draft-floyd-rfc3448bis-00, sec. 1]: "TFRC is designed for applications that
  use a fixed segment size, and vary their sending rate [...]  Some applications
  [...] vary their segment size instead of their packet rate in response to 
  congestion.  The congestion control mechanism in this document is not designed
  for those applications."

When applications don't know their average packet (aka segment) size, 
[draft-floyd-rfc3448bis-00, sec. 4.1] suggests the following:

 "For the first class of applications where the segment size varies depending on
  the data, the sender MAY estimate the segment size s as the average segment size
  over the last four loss intervals.  The sender MAY also estimate the average segment
  size over longer time intervals, if so desired."

This means that for CCIDs 2 and 3 the PACKET_SIZE option is redundant, since the
sender is asked to take care of the averaging itself. 

Future CCIDs with varying packet sizes
===================
In the section 6 of draft-ietf-dccp-tfrc-voip-05.txt (the upcoming CCID 4), 
"TFRC-SP with Applications that Modify the Packet Size", the issue of varying packet
sizes is addressed as follows:

   "One possible use for TFRC-SP would be with applications [...] modify
    their packet size in response to congestion.  [...]

    There are many questions about how an adaptive application would use
    TFRC-SP that are beyond the scope of this document.  [...] The details
    of how long an application might wait before reducing the packet size can be
    addressed in documents that are more application-specific.

    Similarly, an application using TFRC-SP might only have a few packet
    sizes that it is able to use.  [...] Again, the details will have to
    be addressed in documents that are more application-specific."

So it is the problem of the application and we don't need/want to know about
packet size in the kernel.

-- Gerrit

Re: [PATCH 7/7] [DCCP]: Remove socket option

[Eddie Kohler]

Hi, a bit of re-clarification:  CCIDs 2 and 3 are *not* meant for apps that 
NEVER vary their packet size.  Rather, they are meant for apps that very 
packet size *for application reasons* (such as codec output), but *not* in 
response to congestion.  CCIDs 2 and 3 expect to reduce application RATES in 
response to congestion.

Applications that lie about the packet size parameter are a concern, but 
perhaps one could handle that as it came up.  Our simulations indicated that 
applications got little benefit from manipulating packet size.  Note also that 
the RFC explicitly allows CCID 3 to use the largest packet size possible for 
the 's' parameter, regardless of the actual packet size.  If the option value 
was bounded above by this maximum segment size, then I believe the app would 
get little benefit from lying.  (Would need to check.)

In summary, in the longer-term deriving 's' from observations would work, but 
I don't see any objection to this socket option in the short term or the long 
term.  It allows the application to explicitly state its intent, which is 
usually useful.

Eddie


Gerrit Renker wrote:
> I think that the PACKET_SIZE socket option is redundant, since
> it can be derived from
>  * the buflen argument of dccp_sendmsg()
>  * the size of received packets
> independent of whether the CCID is for fixed packet sizes or not.
> 
> Virtually all known CCIDs have fixed packet sizes; for such CCIDs
> there are two cases:
> 
>  (i)  buffer length never changes and is equal to PACKET_SIZE
>       --> then PACKET_SIZE is clearly redundant
>  (ii) PACKET_SIZE gives the intention of an average packet size
>       which implies that PACKET_SIZE gives an average of buflen
> 
> So the relevant case to consider is (ii) and here I think that
> using a PACKET_SIZE option is also redundant: the application
> could simply lie and state the wrong PACKET_SIZE.
> 
> Below are excerpts from the literature to substantiate the above.
> 
> Fixed-size CCIDS 2 and 3
> ============
> Both CCID 2 and 3 are meant for applications that use fixed packet sizes.
> 
> * [RFC 4341, sec. 5.3]: "CCID 2 is optimized for applications that
>   generally use a fixed packet size and vary their sending rate in
>   packets per second in response to congestion."
> 
> * [RFC 4341, sec. 5.3]: "CCID 3 is intended for applications that use
>   a fixed packet size, and that vary their sending rate in packets per
>   second in response to congestion."
> 
> * [draft-floyd-rfc3448bis-00, sec. 1]: "TFRC is designed for applications that
>   use a fixed segment size, and vary their sending rate [...]  Some applications
>   [...] vary their segment size instead of their packet rate in response to 
>   congestion.  The congestion control mechanism in this document is not designed
>   for those applications."
> 
> When applications don't know their average packet (aka segment) size, 
> [draft-floyd-rfc3448bis-00, sec. 4.1] suggests the following:
> 
>  "For the first class of applications where the segment size varies depending on
>   the data, the sender MAY estimate the segment size s as the average segment size
>   over the last four loss intervals.  The sender MAY also estimate the average segment
>   size over longer time intervals, if so desired."
> 
> This means that for CCIDs 2 and 3 the PACKET_SIZE option is redundant, since the
> sender is asked to take care of the averaging itself. 
> 
> Future CCIDs with varying packet sizes
> ===================
> In the section 6 of draft-ietf-dccp-tfrc-voip-05.txt (the upcoming CCID 4), 
> "TFRC-SP with Applications that Modify the Packet Size", the issue of varying packet
> sizes is addressed as follows:
> 
>    "One possible use for TFRC-SP would be with applications [...] modify
>     their packet size in response to congestion.  [...]
> 
>     There are many questions about how an adaptive application would use
>     TFRC-SP that are beyond the scope of this document.  [...] The details
>     of how long an application might wait before reducing the packet size can be
>     addressed in documents that are more application-specific.
> 
>     Similarly, an application using TFRC-SP might only have a few packet
>     sizes that it is able to use.  [...] Again, the details will have to
>     be addressed in documents that are more application-specific."
> 
> So it is the problem of the application and we don't need/want to know about
> packet size in the kernel.
> 
> -- Gerrit
> -
> 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 7/7] [DCCP]: Remove socket option

[Gerrit Renker]

|  Hi, a bit of re-clarification:  CCIDs 2 and 3 are *not* meant for apps that 
|  NEVER vary their packet size.  Rather, they are meant for apps that very 
|  packet size *for application reasons* (such as codec output), but *not* in 
|  response to congestion.  CCIDs 2 and 3 expect to reduce application RATES in 
|  response to congestion.
My understanding is that there is very little wisdom currently regarding applications
which vary their packet sizes due to application reasons; I have copied the quotes
I am referring to below. The work-in-progress draft-ietf-dccp-tfrc-voip-05.txt
is not very outspoken about what happens if the application changes its packet size.

[RFC 4341, sec. 5.3]: "CCID 2 is optimized for applications that generally use a fixed
                       packet size and vary their sending rate in packets per second in
                       response to congestion."
[RFC 4342, sec. 5.3]: "CCID 3 is intended for applications that use a fixed packet size, and
                       that vary their sending rate in packets per second in response to
                       congestion."

If we allow applications to violate these premises then I don't know how to fix this in the code.
  

|  In summary, in the longer-term deriving 's' from observations would work, but 
|  I don't see any objection to this socket option in the short term or the long 
|  term.  It allows the application to explicitly state its intent, which is 
|  usually useful.
I would like to suggest a compromise: retaining an experimental patch which allows
this socket option and can be used for people interested in experimenting with these
ideas, to gain new insights; and to leave the main kernel API as simple as it can
possibly be made. 


--Gerrit

Re: [PATCH 7/7] [DCCP]: Remove socket option

[Ian McDonald]

> I would like to suggest a compromise: retaining an experimental patch which allows
> this socket option and can be used for people interested in experimenting with these
> ideas, to gain new insights; and to leave the main kernel API as simple as it can
> possibly be made.

That is exactly what my patch is. The socket option is not required so
much of the debate is pointless!

You could argue over the correct default for a socket but my patch
doesn't alter that.

After seeing all the discussion I would still like the patch in.

Ian
-- 
Ian McDonald
Web: http://wand.net.nz/~iam4
Blog: http://imcdnzl.blogspot.com
WAND Network Research Group
Department of Computer Science
University of Waikato
New Zealand

Re: [PATCH 7/7] [DCCP]: Remove socket option

[Ian McDonald]

On 9/23/06, Ian McDonald <ian.mcdonald@jandi.co.nz> wrote:
> > I would like to suggest a compromise: retaining an experimental patch which allows
> > this socket option and can be used for people interested in experimenting with these
> > ideas, to gain new insights; and to leave the main kernel API as simple as it can
> > possibly be made.
>
> That is exactly what my patch is. The socket option is not required so
> much of the debate is pointless!
>
> You could argue over the correct default for a socket but my patch
> doesn't alter that.
>
> After seeing all the discussion I would still like the patch in.
>
And what's worse is that the existing socket option doesn't work!

To do the research needed to answer some of the questions being
debated we need this option available. Please merge!!
-- 
Ian McDonald
Web: http://wand.net.nz/~iam4
Blog: http://imcdnzl.blogspot.com
WAND Network Research Group
Department of Computer Science
University of Waikato
New Zealand

Re: [PATCH 7/7] [DCCP]: Remove socket option

[Gerrit Renker]

Ian McDonald wrote:
|  > I would like to suggest a compromise: retaining an experimental patch which allows
|  > this socket option and can be used for people interested in experimenting with these
|  > ideas, to gain new insights; and to leave the main kernel API as simple as it can
|  > possibly be made.
|  
|  That is exactly what my patch is. The socket option is not required so
|  much of the debate is pointless!
|  
|  You could argue over the correct default for a socket but my patch
|  doesn't alter that.
|  
|  After seeing all the discussion I would still like the patch in.
I fully agree with Ian.