Re: [PATCH 1/1] [DCCP][QPOLICY]: Make information about qpolicies available to userspace

Re: [PATCH 1/1] [DCCP][QPOLICY]: Make information about qpolicies available to userspace

[Gerrit Renker]

[-- Attachment #1: Type: text/plain, Size: 1923 bytes --]

Hi Tomasz,

| Since packets with invalid cmsg parameters can be rejected by kernel there
| is a need to allow applications to access information on available policies
| and their respective cmsg parameters at runtime. This patch simplifies
| maintaining compatibility between userspace applications and DCCP code.
| 
The difference to querying supported CCIDs is that
 * CCIDs are all defined per RFC documents, and that
 * different combinations of CCIDs are possible due to the Kconfig options.

As far as I understand your patch, querying here has a different role -
ensuring compatibilities between kernel versions.

I think it might be too early for that:
 * it takes quite a long while until patches propagate through to 
   mainline (more than half a year), so that there is the time to
   come up with a single, well-tested interface;
 * at this stage it would be better to have documentation (man pages,
   web pages, sample application code etc.) to allow people to use 
   the interface - few will want to discover the interface by grepping
   through source code.

DCCP is full of half-finished things. I would much prefer to keep this
as simple as at all possible, to have time/room to fix the missing parts
(such as no ECN support) in DCCP.

As a possible suggestion, I came up with a minimalistic variant of 
querying the interface - only 7 lines, including documentation.

This is attached and it works by exploiting that
 * policy IDs are just numbers;
 * so that we could use the highest supported ID instead of an array;
 * parameters are tied to the individual policy, so that a second
   query (about which parameters are supported) is not necessary.
   
I have put this idea as a suggestion into the `qpolicy' subtree at
	git://eden-feed.erg.abdn.ac.uk/dccp_exp

A trivial test program using this interface can be downloaded from
http://www.erg.abdn.ac.uk/users/gerrit/dccp/query_ids.tar.gz

- Gerrit

[-- Attachment #2: Combination of querying-available-policies patches. --]
[-- Type: text/x-diff, Size: 1577 bytes --]

This is a smaller diff, combined from the two patches currently in
the qpolicy subtree.
--
 Documentation/networking/dccp.txt |    3 +++
 include/linux/dccp.h              |    1 +
 net/dccp/proto.c                  |    3 +++
 3 files changed, 7 insertions(+)

--- a/Documentation/networking/dccp.txt
+++ b/Documentation/networking/dccp.txt
@@ -45,6 +45,9 @@ http://linux-net.osdl.org/index.php/DCCP
 
 Socket options
 ==============
+DCCP_SOCKOPT_QPOLICY_AVAILABLE returns the highest supported policy number. The
+policy IDs start with 0 (default policy) and are consecutively numbered.
+
 DCCP_SOCKOPT_QPOLICY_ID sets the dequeuing policy for outgoing packets. It takes
 a policy ID as argument and can only be set before the connection (i.e. changes
 during an established connection are not supported). Currently, two policies are
--- a/include/linux/dccp.h
+++ b/include/linux/dccp.h
@@ -223,6 +223,7 @@ enum dccp_packet_dequeueing_policy {
 #define DCCP_SOCKOPT_RX_CCID		15
 #define DCCP_SOCKOPT_QPOLICY_ID		16
 #define DCCP_SOCKOPT_QPOLICY_TXQLEN	17
+#define DCCP_SOCKOPT_QPOLICY_AVAILABLE	18
 #define DCCP_SOCKOPT_CCID_RX_INFO	128
 #define DCCP_SOCKOPT_CCID_TX_INFO	192
 
--- a/net/dccp/proto.c
+++ b/net/dccp/proto.c
@@ -666,6 +666,9 @@ static int do_dccp_getsockopt(struct soc
 	case DCCP_SOCKOPT_QPOLICY_TXQLEN:
 		val = dp->dccps_tx_qlen;
 		break;
+	case DCCP_SOCKOPT_QPOLICY_AVAILABLE:
+		val = DCCPQ_POLICY_MAX - 1;
+		break;
 	case 128 ... 191:
 		return ccid_hc_rx_getsockopt(dp->dccps_hc_rx_ccid, sk, optname,
 					     len, (u32 __user *)optval, optlen);

Re: [PATCH 1/1] [DCCP][QPOLICY]: Make information about qpolicies available to userspace

[Tomasz Grobelny]

Dnia Wednesday 21 of May 2008, Gerrit Renker napisał:
> Hi Tomasz,
>
> | Since packets with invalid cmsg parameters can be rejected by kernel
> | there is a need to allow applications to access information on available
> | policies and their respective cmsg parameters at runtime. This patch
> | simplifies maintaining compatibility between userspace applications and
> | DCCP code.
>
> The difference to querying supported CCIDs is that
>  * CCIDs are all defined per RFC documents, and that
Yes, but does that change anything?

>  * different combinations of CCIDs are possible due to the Kconfig options.
>
Here we can have various kernel versions which is not much different. In both 
cases we need runtime feature detection.

> As far as I understand your patch, querying here has a different role -
> ensuring compatibilities between kernel versions.
>
Yes.

> I think it might be too early for that:
>  * it takes quite a long while until patches propagate through to
>    mainline (more than half a year), so that there is the time to
>    come up with a single, well-tested interface;
But when it gets into mainline people should be able to write applications 
that will be forward and backward compatible (as far as possible) with 
various kernel versions.

>  * at this stage it would be better to have documentation (man pages,
>    web pages, sample application code etc.) to allow people to use
>    the interface - few will want to discover the interface by grepping
>    through source code.
>
Sure it would be nice to have applications that use this interface. But in my 
opinion the basic interface is not yet ready to be included in mainline 
applications. It especially lacks features that will allow application 
developers to maintain compatibility.

> DCCP is full of half-finished things. 
That's why I'm trying to finish the qpolicy framework. Without providing 
runtime information it will turn out that application developers will be 
afraid to use new features just because the application won't be able to 
check if given parameter is supported or not.

> I would much prefer to keep this 
> as simple as at all possible, to have time/room to fix the missing parts
> (such as no ECN support) in DCCP.
>
I understand that there might be some missing features in DCCP that are more 
important than yet another informational getsockopt. But these features are 
IMO highly independent and can be developed in parallel.

> As a possible suggestion, I came up with a minimalistic variant of
> querying the interface - only 7 lines, including documentation.
>
> This is attached and it works by exploiting that
>  * policy IDs are just numbers;
>  * so that we could use the highest supported ID instead of an array;
I don't like the "highest supported ID" approach. But if you don't like arrays 
a simple "bool getsockopt" would do, we could just ask the kernel if it 
supports given qpolicy. Like (in pseudocode):
bool supports_prio=getsockopt(sk,DCCP_SOCKOPT_QPOLICY_AVAILABLE,QPOLICY_PRIO);
Would that be ok?

>  * parameters are tied to the individual policy, so that a second
>    query (about which parameters are supported) is not necessary.
>
Can you guarantee that nobody ever will add a parameter? In fact even today I 
can think of two parameters that could be added: the previously discussed 
timeout and my new concept - the identity or category parameter (which is a 
subject for a separate thread).

BTW, the somewhat big code was meant to act as a framework for other features. 
Like getting information about percent of dropped packets in prio qpolicy 
queue.
-- 
Regards,
Tomasz Grobelny

Re: [PATCH 1/1] [DCCP][QPOLICY]: Make information about qpolicies available to userspace

[Gerrit Renker]

| But when it gets into mainline people should be able to write applications 
| that will be forward and backward compatible (as far as possible) with 
| various kernel versions.
| 
The main answer to that is below, but compare for instance with TCP -
few socket options need runtime discovery. The type of their parameters
and their option values are all described in a manpage. On the other
hand, I like your approach of trying to come up with a novel solution.

| > DCCP is full of half-finished things. 
| That's why I'm trying to finish the qpolicy framework. Without providing 
| runtime information it will turn out that application developers will be 
| afraid to use new features just because the application won't be able to 
| check if given parameter is supported or not.
| 
And you have done a good job so far, it is a very useful addition.


| > I would much prefer to keep this 
| > as simple as at all possible, to have time/room to fix the missing parts
| > (such as no ECN support) in DCCP.
| >
| I understand that there might be some missing features in DCCP that are more 
| important than yet another informational getsockopt. But these features are 
| IMO highly independent and can be developed in parallel.
| 
Agreed.

| > As a possible suggestion, I came up with a minimalistic variant of
| > querying the interface - only 7 lines, including documentation.
| >
| > This is attached and it works by exploiting that
| >  * policy IDs are just numbers;
| >  * so that we could use the highest supported ID instead of an array;
| I don't like the "highest supported ID" approach. But if you don't like arrays 
| a simple "bool getsockopt" would do, we could just ask the kernel if it 
| supports given qpolicy. Like (in pseudocode):
| bool supports_prio=getsockopt(sk,DCCP_SOCKOPT_QPOLICY_AVAILABLE,QPOLICY_PRIO);
| Would that be ok?
| 
What I sent is just a sketch, nothing set in stone. I am happy to take
my patch out of the repository if there are better solutions.


| >  * parameters are tied to the individual policy, so that a second
| >    query (about which parameters are supported) is not necessary.
| >
| Can you guarantee that nobody ever will add a parameter? In fact even today I 
| can think of two parameters that could be added: the previously discussed 
| timeout and my new concept - the identity or category parameter (which is a 
| subject for a separate thread).
| 
During the design phase it is almost certain to change several times.

When the design phase is over, it will have some parts that will not
likely change.

Hence I am trying to figure out - how much change is due to re-designing
things and how much change is de facto required for a useful dynamic
interface?

| BTW, the somewhat big code was meant to act as a framework for other features. 
| Like getting information about percent of dropped packets in prio qpolicy 
| queue.
| -- 
Ah - didn't understand what the qpolicy-specific getsockopt hooks were for.

I think there is a way of doing this, but it requires a bit more work. 
This would be to use netlink sockets to communicate/request/poll information. 

The two alternatives I am aware of are

 * kernel/userspace connector
   [ Documentation/connector/connector.txt ]
 * generic netlink interface
   http://www.linux-foundation.org/en/Net:Generic_Netlink_HOWTO

If done well, the interface could be very useful for several other
things in DCCP:
 * querying qpolicy interface (with regard to this email thread)	
 * querying the CCIDs (available, set preference list)
 * getting much-needed runtime statistics into user-space
   - this is a feature missing in UDP (and partly also TCP)
   - so far only via CCID-3 getsockopt (tx_info/rx_info)
   - there is research which says it would be nice to have the
     current RTT, loss statistics etc in user-space
 * thus calling for a generic solution.

What do you think about this alternative?

- Gerrit

Re: [PATCH 1/1] [DCCP][QPOLICY]: Make information about qpolicies available to userspace

[Tomasz Grobelny]

Dnia Thursday 22 of May 2008, Gerrit Renker napisał:
> | But when it gets into mainline people should be able to write
> | applications that will be forward and backward compatible (as far as
> | possible) with various kernel versions.
>
> The main answer to that is below, but compare for instance with TCP -
> few socket options need runtime discovery. The type of their parameters
> and their option values are all described in a manpage. On the other
> hand, I like your approach of trying to come up with a novel solution.
>
TCP was designed before the era of having everything pluggable. DCCP on the 
other hand is prepared for adding new CCIDs. Logically then, it should be 
extensible in other areas too.

> | > As a possible suggestion, I came up with a minimalistic variant of
> | > querying the interface - only 7 lines, including documentation.
> | >
> | > This is attached and it works by exploiting that
> | >  * policy IDs are just numbers;
> | >  * so that we could use the highest supported ID instead of an array;
> |
> | I don't like the "highest supported ID" approach. But if you don't like
> | arrays a simple "bool getsockopt" would do, we could just ask the kernel
> | if it supports given qpolicy. Like (in pseudocode):
> | bool
> | supports_prio=getsockopt(sk,DCCP_SOCKOPT_QPOLICY_AVAILABLE,QPOLICY_PRIO);
> | Would that be ok?
>
> What I sent is just a sketch, nothing set in stone. I am happy to take
> my patch out of the repository if there are better solutions.
>
The question is: would you consider such a solution better? 

> | >  * parameters are tied to the individual policy, so that a second
> | >    query (about which parameters are supported) is not necessary.
> |
> | Can you guarantee that nobody ever will add a parameter? In fact even
> | today I can think of two parameters that could be added: the previously
> | discussed timeout and my new concept - the identity or category parameter
> | (which is a subject for a separate thread).
>
> During the design phase it is almost certain to change several times.
>
> When the design phase is over, it will have some parts that will not
> likely change.
>
> Hence I am trying to figure out - how much change is due to re-designing
> things and how much change is de facto required for a useful dynamic
> interface?
>
IMO we could divide the interface into two parts:
 - operations - they are likely to stabilize before pushing the code to 
mainline,
 - parameters (all kinds of numbers like DCCP_SCM_xxx, qpolicy ids, etc.) - 
there should always be a room for new ones (and maybe even deleting old 
ones). And one should be able to verify which numbers are valid - that's why 
we need operations for this purpose that are available from the very 
beginning.

> | BTW, the somewhat big code was meant to act as a framework for other
> | features. Like getting information about percent of dropped packets in
> | prio qpolicy queue.
> | --
>
> Ah - didn't understand what the qpolicy-specific getsockopt hooks were for.
>
> I think there is a way of doing this, but it requires a bit more work.
> This would be to use netlink sockets to communicate/request/poll
> information.
>
> The two alternatives I am aware of are
>
>  * kernel/userspace connector
>    [ Documentation/connector/connector.txt ]
>  * generic netlink interface
>    http://www.linux-foundation.org/en/Net:Generic_Netlink_HOWTO
>
As far as I understand netlink is a kind of intermodule communication 
mechanism (where module may be either in-kernel or userspace application).

> If done well, the interface could be very useful for several other
> things in DCCP:
>  * querying qpolicy interface (with regard to this email thread)
>  * querying the CCIDs (available, set preference list)
>  * getting much-needed runtime statistics into user-space
>    - this is a feature missing in UDP (and partly also TCP)
>    - so far only via CCID-3 getsockopt (tx_info/rx_info)
>    - there is research which says it would be nice to have the
>      current RTT, loss statistics etc in user-space
>  * thus calling for a generic solution.
>
> What do you think about this alternative?
>
I have nothing against creating generic interface for obtaining information 
about various aspects of dccp. But I can't see how netlink can help us here. 
From the documents I've read I understand that generic netlink requires you 
to create a special kind of socket type and then you can send or receive 
information through it. But it seems to be something separate from dccp 
sockets.

As for information we can get from kernel wrt dccp we have at least:
1. fixed information that depends only on kernel version. For example list of 
ccids, list of available qpolicies, list of parameters for given qpolicy, 
etc. These are system wide and don't need reference to socket. These could 
even be exposed by read-only entries in /sys or /proc.
2. typical socket options. Among others: currently used rx/tx ccid, currently 
used qpolicy, queue and buffer sizes, etc. For these we need reference to 
socket and get/setsockopts are the natural choice. And I guess no changes 
should happen here.
3. statistics. These are not options in the sense that you cannot set them. 
You also cannot expect that two consecutive reads will return the same value. 
They depend on certain asynchronous events in various parts of dccp code. The 
first question is: should we pass those raw events to applications (such 
as "prio qpolicy: packet dropped") or aggregate data ("in the last 100 
packets 17 were dropped before sending")? For this kind of information we 
need reference to socket (struct sock *). And I can't see how we could obtain 
it from functions not mentioned in struct proto.

I suggest that we discuss points 1 and 3 separately because it may turn that 
different mechanisms will be applicable. getsockopt is possible in both which 
doesn't mean that it is the best solution.
-- 
Regards,
Tomasz Grobelny

Re: [PATCH 1/1] [DCCP][QPOLICY]: Make information about qpolicies available to userspace

[Gerrit Renker]

| > | I don't like the "highest supported ID" approach. But if you don't like
| > | arrays a simple "bool getsockopt" would do, we could just ask the kernel
| > | if it supports given qpolicy. Like (in pseudocode):
| > | bool
| > | supports_prio=getsockopt(sk,DCCP_SOCKOPT_QPOLICY_AVAILABLE,QPOLICY_PRIO);
| > | Would that be ok?
| >
| > What I sent is just a sketch, nothing set in stone. I am happy to take
| > my patch out of the repository if there are better solutions.
| >
| The question is: would you consider such a solution better? 
| 
I think it is too early for this question, it needs some practical experience.
It may be that a third or fourth solution proves more useful.


| > Hence I am trying to figure out - how much change is due to re-designing
| > things and how much change is de facto required for a useful dynamic
| > interface?
| >
| IMO we could divide the interface into two parts:
|  - operations - they are likely to stabilize before pushing the code to 
| mainline,
|  - parameters (all kinds of numbers like DCCP_SCM_xxx, qpolicy ids, etc.) - 
| there should always be a room for new ones (and maybe even deleting old 
| ones). And one should be able to verify which numbers are valid - that's why 
| we need operations for this purpose that are available from the very 
| beginning.
| 
I think this problem appears in other kernel areas as well, such as
finding consistent identifiers for identifiers, flags, symbolic names.

Haven't grepped through, but there is likely a smart solution available
already somewhere. 

For IETF specifications, the IANA handles all numbers that appear in
packets, a similar thing appears here. But with some discipline it
should not cause a problem. 


| I have nothing against creating generic interface for obtaining information 
| about various aspects of dccp. But I can't see how netlink can help us here. 
| From the documents I've read I understand that generic netlink requires you 
| to create a special kind of socket type and then you can send or receive 
| information through it. But it seems to be something separate from dccp 
| sockets.
| 
Yes, the netlink socket would be different from the DCCP socket and it
is actually a good point - it shows that this interface can only be used
for global parameters or settings. Or maybe there is a way of
associating both sockets.


| As for information we can get from kernel wrt dccp we have at least:
| 1. fixed information that depends only on kernel version. For example list of 
| ccids, list of available qpolicies, list of parameters for given qpolicy, 
| etc. These are system wide and don't need reference to socket. These could 
| even be exposed by read-only entries in /sys or /proc.
That is a good point - I think Arnaldo had a similar idea once for implenting
system-wide policies regarding which CCIDs are supported. Something like
net.ipv4.tcp_available_congestion_control.

| 2. typical socket options. Among others: currently used rx/tx ccid, currently 
| used qpolicy, queue and buffer sizes, etc. For these we need reference to 
| socket and get/setsockopts are the natural choice. And I guess no changes 
| should happen here.
Agree.

| 3. statistics. These are not options in the sense that you cannot set them. 
| You also cannot expect that two consecutive reads will return the same value. 
| They depend on certain asynchronous events in various parts of dccp code. The 
| first question is: should we pass those raw events to applications (such 
| as "prio qpolicy: packet dropped") or aggregate data ("in the last 100 
| packets 17 were dropped before sending")? For this kind of information we 
| need reference to socket (struct sock *). And I can't see how we could obtain 
| it from functions not mentioned in struct proto.
| 
It was for this part that I looked at netlink. This goes into the
direction of a new API or at least API extensions.

For global statistics there is already the DCCP MIB, but it needs some
more work (in /proc/snmp there are no entries at the moment), a ToDo.

For per-socket statistics there is indeed a need for a notification
mechanism. If a synchronous mechanism is sufficient, then something like
the current CCID-3 getsockopt(DCCP_SOCKOPT_CCID_{RX,TX}_INFO) can be used.

For asynchronous notification, using a special signal comes to mind,
where the application installs a signal handler as notifier.

The best that I can think of is to map a segment of userspace memory
into the kernel and use this memory area as a (ready-only) statistics
struct. That would save the system call overhead of using getsockopt().

That is my list of ideas at the moment. Probably not exhaustive.

- Gerrit

Re: [PATCH 1/1] [DCCP][QPOLICY]: Make information about qpolicies available to userspace

[Tomasz Grobelny]

Dnia Monday 26 of May 2008, Gerrit Renker napisał:
> | I have nothing against creating generic interface for obtaining
> | information about various aspects of dccp. But I can't see how netlink
> | can help us here. From the documents I've read I understand that generic
> | netlink requires you to create a special kind of socket type and then you
> | can send or receive information through it. But it seems to be something
> | separate from dccp sockets.
>
> Yes, the netlink socket would be different from the DCCP socket and it
> is actually a good point - it shows that this interface can only be used
> for global parameters or settings. Or maybe there is a way of
> associating both sockets.
>
I haven't seen any. But I'm quite new to kernel development (and to netlink in 
particular).

> | As for information we can get from kernel wrt dccp we have at least:
> | 1. fixed information that depends only on kernel version. For example
> | list of ccids, list of available qpolicies, list of parameters for given
> | qpolicy, etc. These are system wide and don't need reference to socket.
> | These could even be exposed by read-only entries in /sys or /proc.
>
> That is a good point - I think Arnaldo had a similar idea once for
> implenting system-wide policies regarding which CCIDs are supported.
> Something like net.ipv4.tcp_available_congestion_control.
>
Ok, we may go this way. I'll try to write a patch in a few days (quite busy 
now).

> | 3. statistics. These are not options in the sense that you cannot set
> | them. You also cannot expect that two consecutive reads will return the
> | same value. They depend on certain asynchronous events in various parts
> | of dccp code. The first question is: should we pass those raw events to
> | applications (such as "prio qpolicy: packet dropped") or aggregate data
> | ("in the last 100 packets 17 were dropped before sending")? For this kind
> | of information we need reference to socket (struct sock *). And I can't
> | see how we could obtain it from functions not mentioned in struct proto.
>
> It was for this part that I looked at netlink. This goes into the
> direction of a new API or at least API extensions.
> 
We basically need a callback from code that can access struct sock...

> (...)
> For per-socket statistics there is indeed a need for a notification
> mechanism. If a synchronous mechanism is sufficient, then something like
> the current CCID-3 getsockopt(DCCP_SOCKOPT_CCID_{RX,TX}_INFO) can be used.
>
Synchronous mechanism is acceptable but callback (at least as an option) would 
be way nicer.

> For asynchronous notification, using a special signal comes to mind,
> where the application installs a signal handler as notifier.
>
We'd have to pass affected socket identifier (process may use more than one 
socket). To me it looks like a not-so-nice hack. But still I don't have 
better ideas on how to implement asynchronous event notification (callback). 
Ok, but if we are at hackish ideas what about this one: use dccp_ioctl to 
wait on a mutex. If an event occurs we release mutex thus allowing dccp_ioctl 
to finish execution and consequently informing user that something 
interesting has just happened - what it exactly is would have to be 
determined by other means.

> The best that I can think of is to map a segment of userspace memory
> into the kernel and use this memory area as a (ready-only) statistics
> struct. That would save the system call overhead of using getsockopt().
>
I don't think the syscall overhead is big enough to care. After all you don't 
need statistics that often. Shared memory creates potential problems with 
synchronization (ok, it is read-only but you need to provide consistent 
data). Plus you must update them whether needed or not because you never know 
when the user will want to read them (it may be a problem in some scenarios 
and may not be a problem in others). IMO more problems that it's worth.
-- 
Regards,
Tomasz Grobelny

Re: [PATCH 1/1] [DCCP][QPOLICY]: Make information about qpolicies available to userspace

[Tomasz Grobelny]

Dnia Monday 26 of May 2008, napisałeś:
> Dnia Monday 26 of May 2008, Gerrit Renker napisał:
> > | As for information we can get from kernel wrt dccp we have at least:
> > | 1. fixed information that depends only on kernel version. For example
> > | list of ccids, list of available qpolicies, list of parameters for
> > | given qpolicy, etc. These are system wide and don't need reference to
> > | socket. These could even be exposed by read-only entries in /sys or
> > | /proc.
> >
> > That is a good point - I think Arnaldo had a similar idea once for
> > implenting system-wide policies regarding which CCIDs are supported.
> > Something like net.ipv4.tcp_available_congestion_control.
>
> Ok, we may go this way. I'll try to write a patch in a few days (quite busy
> now).
>
Now that I had a closer look at implementing this functionality I have a few 
questions:
1. Where should information about available qpolicies and their parametrs be 
exported? Would /proc/net/dccp/qpolicies/ be a good choice?
2. I guess we should have at least one file per qpolicy with parameters listed 
inside. Like that:
/proc/.../qpolicies/simple: <empty>
/proc/.../qpolicies/prio: 1 (DCCP_SCM_PRIORITY) 2 (DCCP_SCM_TIMEOUT)
But we could also have qpolicy represented by directory and parameters by 
files:
/proc/.../qpolicies/simple/
/proc/.../qpolicies/prio/
/proc/.../qpolicies/prio/priority: <empty>
/proc/.../qpolicies/prio/timeout: <empty>
Which layout do you find better?
-- 
Regards,
Tomasz Grobelny

Re: [PATCH 1/1] [DCCP][QPOLICY]: Make information about qpolicies available to userspace

[Gerrit Renker]

| > It was for this part that I looked at netlink. This goes into the
| > direction of a new API or at least API extensions.
| > 
| We basically need a callback from code that can access struct sock...
| 
| > (...)
| > For per-socket statistics there is indeed a need for a notification
| > mechanism. If a synchronous mechanism is sufficient, then something like
| > the current CCID-3 getsockopt(DCCP_SOCKOPT_CCID_{RX,TX}_INFO) can be used.
| >
| Synchronous mechanism is acceptable but callback (at least as an option) would 
| be way nicer.
| 
I have been looking around for alternatives - there are some
very good solutions for such aspects in the SCTP subsystem. 

They implement the subscription of event notifications on a socket for
all sorts of events.

A socket option performs this subscription, notifications are then passed
along with data (I think also without, have not dug deep enough into the
sources), via recvmsg(). This is the inverse path to qpolicies: the
ancillary data is passed via cmsg(3) wrappers to the user.

To distinguish events from normal (ancillary) data, SCTP sets the
flag fields to MSG_NOTIFICATION.

There are two good papers on this (the second one is a bit older):
http://www.bsdcan.org/2008/schedule/events/91.en.html
http://lwn.net/2001/features/OLS/pdf/pdf/sctp.pdf

The sources are in net/sctp, the lksctp project is hosted at
http://sourceforge.net/projects/lksctp

I think that this approach is elegant and has something to offer for
DCCP as well. What do you think?

| Ok, but if we are at hackish ideas what about this one: use dccp_ioctl to 
| wait on a mutex. If an event occurs we release mutex thus allowing dccp_ioctl 
| to finish execution and consequently informing user that something 
| interesting has just happened - what it exactly is would have to be 
| determined by other means.
| 
I am trying to think in terms of how much documentation this would need
in order to explain to someone how to use it. It seems feasible, but to
me a bit complicated.


| > The best that I can think of is to map a segment of userspace memory
| > into the kernel and use this memory area as a (ready-only) statistics
| > struct. That would save the system call overhead of using getsockopt().
| >
| I don't think the syscall overhead is big enough to care. After all you don't 
| need statistics that often. Shared memory creates potential problems with 
| synchronization (ok, it is read-only but you need to provide consistent 
| data). Plus you must update them whether needed or not because you never know 
| when the user will want to read them (it may be a problem in some scenarios 
| and may not be a problem in others). IMO more problems that it's worth.
Yes that was why I had reservations against using the ringbuffer API by Lai/Kohler
(implemented for TCP in	http://heim.ifi.uio.no/paalh/publications/files/ism07.pdf ).

We could consider this for later, but it does not seem easy to do this
both in a safe and in a non-complicated way.

Re: [PATCH 1/1] [DCCP][QPOLICY]: Make information about qpolicies available to userspace

[Tomasz Grobelny]

Dnia Monday 02 of June 2008, Gerrit Renker napisał:
> | > It was for this part that I looked at netlink. This goes into the
> | > direction of a new API or at least API extensions.
> |
> | We basically need a callback from code that can access struct sock...
> |
> | > (...)
> | > For per-socket statistics there is indeed a need for a notification
> | > mechanism. If a synchronous mechanism is sufficient, then something
> | > like the current CCID-3 getsockopt(DCCP_SOCKOPT_CCID_{RX,TX}_INFO) can
> | > be used.
> |
> | Synchronous mechanism is acceptable but callback (at least as an option)
> | would be way nicer.
>
> I have been looking around for alternatives - there are some
> very good solutions for such aspects in the SCTP subsystem.
>
> They implement the subscription of event notifications on a socket for
> all sorts of events.
>
> A socket option performs this subscription, notifications are then passed
> along with data (I think also without, have not dug deep enough into the
> sources), via recvmsg(). This is the inverse path to qpolicies: the
> ancillary data is passed via cmsg(3) wrappers to the user.
>
> To distinguish events from normal (ancillary) data, SCTP sets the
> flag fields to MSG_NOTIFICATION.
>
> There are two good papers on this (the second one is a bit older):
> http://www.bsdcan.org/2008/schedule/events/91.en.html
> http://lwn.net/2001/features/OLS/pdf/pdf/sctp.pdf
>
> The sources are in net/sctp, the lksctp project is hosted at
> http://sourceforge.net/projects/lksctp
>
> I think that this approach is elegant and has something to offer for
> DCCP as well. What do you think?
>
I also thought about such a solution but I didn't state it because I thought 
it is even more hackish than the one below. Mostly because it 
interleaves "real data" with events (and there is no logical connection 
between the two as opposed to sending scenario). But if it is used in SCTP 
then maybe the whole idea is not that bad...
Anyway, I'll have a look at the documents and at the SCTP implementation.

> | Ok, but if we are at hackish ideas what about this one: use dccp_ioctl to
> | wait on a mutex. If an event occurs we release mutex thus allowing
> | dccp_ioctl to finish execution and consequently informing user that
> | something interesting has just happened - what it exactly is would have
> | to be determined by other means.
>
> I am trying to think in terms of how much documentation this would need
> in order to explain to someone how to use it. It seems feasible, but to
> me a bit complicated.
>
The basic idea is the same as above. The difference is that it would overload 
ioctl, not recv. But maybe we should use recv. Not because it is in any way 
better but just because it is already used that way in SCTP and has a few 
documents describing the approach.
-- 
Regards,
Tomasz Grobelny

Re: [PATCH 1/1] [DCCP][QPOLICY]: Make information about qpolicies available to userspace

[Gerrit Renker]

| Now that I had a closer look at implementing this functionality I have a few 
| questions:
| 1. Where should information about available qpolicies and their parametrs be 
| exported? Would /proc/net/dccp/qpolicies/ be a good choice?
For a sketch or a first implementation, procfs sounds like a good starting point.

But since it is about dynamic runtime configuration, how about using sysfs or configfs
instead? This is a brainstorming question, I think that sysfs is generally preferred.
I don't know how well configfs has taken off, it is similar, but needs to be
added in the configuration (under Pseudeo Filesystems, CONFIG_CONFIGFS_FS=y|m)
http://lwn.net/Articles/148973/
and Documentation/filesystems/configs. But this could be done later as well.


| 2. I guess we should have at least one file per qpolicy with parameters listed 
| inside. Like that:
| /proc/.../qpolicies/simple: <empty>
| /proc/.../qpolicies/prio: 1 (DCCP_SCM_PRIORITY) 2 (DCCP_SCM_TIMEOUT)
Hm this is a "policy" question -- isn't the `timeout' policy a
standalone variant?

| But we could also have qpolicy represented by directory and parameters by files:
| /proc/.../qpolicies/simple/
| /proc/.../qpolicies/prio/
| /proc/.../qpolicies/prio/priority: <empty>
| /proc/.../qpolicies/prio/timeout: <empty>
| Which layout do you find better?
| -- 
I don't like the empty files. In the procfs for thinkpad Acpi
configuration, for example, there is a line that says which commands are
acceptable, similar for /sys/power/state. In that way, the (sysfs|procfs) file
documents itself and tells the user what can be done with it. It would
be great if the qpolicies could do something similar.

I would start with the utterly simplest possible implementation and
leave complex cases for later. For a sophisticated, elegant
implementation, I would seriously consider sysfs or configs.

Re: [PATCH 1/1] [DCCP][QPOLICY]: Make information about qpolicies available to userspace

[Tomasz Grobelny]

Dnia Wednesday 04 of June 2008, Gerrit Renker napisał:
> | Now that I had a closer look at implementing this functionality I have a
> | few questions:
> | 1. Where should information about available qpolicies and their parametrs
> | be exported? Would /proc/net/dccp/qpolicies/ be a good choice?
>
> For a sketch or a first implementation, procfs sounds like a good starting
> point.
>
> But since it is about dynamic runtime configuration, how about using sysfs
> or configfs instead? This is a brainstorming question, I think that sysfs
> is generally preferred. I don't know how well configfs has taken off, it is
> similar, but needs to be added in the configuration (under Pseudeo
> Filesystems, CONFIG_CONFIGFS_FS=y|m) http://lwn.net/Articles/148973/
> and Documentation/filesystems/configs. But this could be done later as
> well.
>
procfs has some fine example of being used for this kind of information, 
namely /proc/cpuinfo and /proc/meminfo
sysfs: from sysfs-rules.txt: "(...) the sysfs interface cannot provide a 
stable interface (...)"
configfs is for configuring kernel from userspace. Which is quite opposite to  
what we want.

> | 2. I guess we should have at least one file per qpolicy with parameters
> | listed inside. Like that:
> | /proc/.../qpolicies/simple: <empty>
> | /proc/.../qpolicies/prio: 1 (DCCP_SCM_PRIORITY) 2 (DCCP_SCM_TIMEOUT)
>
> Hm this is a "policy" question -- isn't the `timeout' policy a
> standalone variant?
>
It seems we have a bit different views on the subject. But that's to be 
decided when somebody comes round to implementing timeout.

> | But we could also have qpolicy represented by directory and parameters by
> | files: /proc/.../qpolicies/simple/
> | /proc/.../qpolicies/prio/
> | /proc/.../qpolicies/prio/priority: <empty>
> | /proc/.../qpolicies/prio/timeout: <empty>
> | Which layout do you find better?
> | --
>
> I don't like the empty files. In the procfs for thinkpad Acpi
> configuration, for example, there is a line that says which commands are
> acceptable, similar for /sys/power/state. In that way, the (sysfs|procfs)
> file documents itself and tells the user what can be done with it. It would
> be great if the qpolicies could do something similar.
>
Ok, I'll try to write a simple patch for that.
-- 
Regards,
Tomasz Grobelny

Re: [PATCH 1/1] [DCCP][QPOLICY]: Make information about qpolicies available to userspace

[Gerrit Renker]

| > But since it is about dynamic runtime configuration, how about using sysfs
| > or configfs instead? This is a brainstorming question, I think that sysfs
| > is generally preferred. I don't know how well configfs has taken off, it is
| > similar, but needs to be added in the configuration (under Pseudeo
| > Filesystems, CONFIG_CONFIGFS_FS=y|m) http://lwn.net/Articles/148973/
| > and Documentation/filesystems/configs. But this could be done later as
| > well.
| >
| procfs has some fine example of being used for this kind of information, 
| namely /proc/cpuinfo and /proc/meminfo

| sysfs: from sysfs-rules.txt: "(...) the sysfs interface cannot provide a 
| stable interface (...)"
Yes that is a key point and I think we are talking about the same point here.

I had mentioned sysfs/procfs as alternatives not because they are the best
possible match, but since it shows that similar problems (and likely solutions)
exist also in other areas.

But there are also areas where the rate of change is relatively low or
even absent: Documentation/ABI/README for instance mentions 4 different
levels of stability (stable/testing/obsolete/removed), the most entries
are under `testing'.

| configfs is for configuring kernel from userspace. Which is quite opposite to  
| what we want.
| 
Ok I think I understand now where you are heading - but then we can go a much
simpler route: why not implement a sysctl (which is also mirrored in /proc/sys)
that contains all available/implemented qpolicies as a space-separated
string, such as

	cat /proc/sys/net/ipv4/tcp_available_congestion_control ?

And I think that it is unnecessarily complex to add the available parameters
belonging to each qpolicy as well.

If we agree on using unique strings to identify qpolicies then we can
keep the runtime discovery much simpler. I think a manpage would be more
helpful here, since the runtime discovery of parameters is not
immediately obvious.

- Gerrit

Re: [PATCH 1/1] [DCCP][QPOLICY]: Make information about qpolicies available to userspace

[Tomasz Grobelny]

Dnia Tuesday 10 of June 2008, Gerrit Renker napisał:
> | > But since it is about dynamic runtime configuration, how about using
> | > sysfs or configfs instead? This is a brainstorming question, I think
> | > that sysfs is generally preferred. I don't know how well configfs has
> | > taken off, it is similar, but needs to be added in the configuration
> | > (under Pseudeo Filesystems, CONFIG_CONFIGFS_FS=y|m)
> | > http://lwn.net/Articles/148973/ and Documentation/filesystems/configs.
> | > But this could be done later as well.
> |
> | procfs has some fine example of being used for this kind of information,
> | namely /proc/cpuinfo and /proc/meminfo
> |
> | sysfs: from sysfs-rules.txt: "(...) the sysfs interface cannot provide a
> | stable interface (...)"
>
> Yes that is a key point and I think we are talking about the same point
> here.
>
> I had mentioned sysfs/procfs as alternatives not because they are the best
> possible match, but since it shows that similar problems (and likely
> solutions) exist also in other areas.
>
That's ok, I just mention what I know about them so that we can have complete 
argumentation for/against given solution.

> But there are also areas where the rate of change is relatively low or
> even absent: Documentation/ABI/README for instance mentions 4 different
> levels of stability (stable/testing/obsolete/removed), the most entries
> are under `testing'.
>
This one should probably be marked as testing too. And probably be moved to 
stable with time (when it proves useful for applications).

> | configfs is for configuring kernel from userspace. Which is quite
> | opposite to what we want.
>
> Ok I think I understand now where you are heading - but then we can go a
> much simpler route: why not implement a sysctl (which is also mirrored in
> /proc/sys) that contains all available/implemented qpolicies as a
> space-separated string, such as
>
> 	cat /proc/sys/net/ipv4/tcp_available_congestion_control ?
>
Could be /proc/sys. I just had an impression that sysctls are for setting 
certain variables used in kernel. But as far as I understand it the 
difference is highly subjective and I'm ok with both /proc and /proc/sys. 
Would be nice to have more opinions (Arnaldo? others?) whether we should 
favour one over other.

> And I think that it is unnecessarily complex to add the available
> parameters belonging to each qpolicy as well.
>
I think it is critical. If the application just wants to know which qpolicies 
are available it can try to set it with setsockopt(). If it gets an error 
given qpolicy is unavailable. As simple as that. No need for additional 
interface.
But if the application wants information about implemented parameters there is 
currently no way to get it. It can try to send() packet and check return 
value (which is quite a hack to be honest). It will work if the parameter is 
not implemented at all. But what happens when you try to set priority for a 
packet and use "simple" qpolicy? AFAIR nothing - you get no information 
whatsoever that this parameter will not be used inside qpolicy.

And that's why we need information about parameters in /proc or /proc/sys.

> If we agree on using unique strings to identify qpolicies then we can
> keep the runtime discovery much simpler. I think a manpage would be more
> helpful here, since the runtime discovery of parameters is not
> immediately obvious.
>
Manpage of what? Documentation is certainly needed but before writing one we 
need to have a basic implementation we agree upon.
-- 
Regards,
Tomasz Grobelny

Re: [PATCH 1/1] [DCCP][QPOLICY]: Make information about qpolicies available to userspace

[Gerrit Renker]

| > But there are also areas where the rate of change is relatively low or
| > even absent: Documentation/ABI/README for instance mentions 4 different
| > levels of stability (stable/testing/obsolete/removed), the most entries
| > are under `testing'.
| >
| This one should probably be marked as testing too. And probably be moved to 
| stable with time (when it proves useful for applications).
| 
In the test tree there are two levels of testing:

 * any subtrees tree are open for writing experimental patches,
   for refinement, and for patches that may be rewritten later;

 * the test tree itself is for later consideration in mainline and therefore
   is more conservative. The main reason for keeping the patches in that tree
   is to extend the phase of patch review, to filter out bugs that may still
   lurk somewhere, so that regressions are minimised when submitting.


| > why not implement a sysctl (which is also mirrored in
| > /proc/sys) that contains all available/implemented qpolicies as a
| > space-separated string, such as
| >
| > 	cat /proc/sys/net/ipv4/tcp_available_congestion_control ?
| >
| Could be /proc/sys. I just had an impression that sysctls are for setting 
| certain variables used in kernel. But as far as I understand it the 
| difference is highly subjective and I'm ok with both /proc and /proc/sys. 
Yes that is not immediately obvious, there are far more sysctls with write
permissions than informative/read-only sysctls:
 * read-only:   7 entries with  `find /proc/sys/net/ -perm  444` 
 * write:     486 entries with  `find /proc/sys/net/ -perm /222`


| Would be nice to have more opinions (Arnaldo? others?) whether we should 
| favour one over other.
Agree with this call. 


| But if the application wants information about implemented parameters there is 
| currently no way to get it. It can try to send() packet and check return 
| value (which is quite a hack to be honest). It will work if the parameter is 
| not implemented at all. But what happens when you try to set priority for a 
| packet and use "simple" qpolicy? AFAIR nothing - you get no information 
| whatsoever that this parameter will not be used inside qpolicy.
| 
| And that's why we need information about parameters in /proc or /proc/sys.
| 
Sorry I can't see here why we need information about parameters. When
introducing a new socket option there is a convention of how to use it.

For sockopts, the type of the argument is listed for each option type.

With qpolicies, there is one more indirection, since there is no
immediate return value. But even for this kind of socket option there
exist similar cases, for instance in the manpage of ipv6(7)
 * IPV6_RECVERR (boolean) sets delivery of error messages which return a "struct
   sock_extended_err" via cmsg(3) when MSG_ERRQUEUE is set in recvmsg(2)
 * IPV6_PKTINFO (boolean) sets the delivery of IPV6_PKTINFO control messages,
   and this one has an associated constraint - works only on SOCK_DGRAM/SOCK_RAW


| > If we agree on using unique strings to identify qpolicies then we can
| > keep the runtime discovery much simpler. I think a manpage would be more
| > helpful here, since the runtime discovery of parameters is not
| > immediately obvious.
| >
| Manpage of what? Documentation is certainly needed but before writing one we 
| need to have a basic implementation we agree upon.
| -- 
There is indeed a misunderstanding.

We have already an implementation we agreed upon:
http://eden-feed.erg.abdn.ac.uk/cgi-bin/gitweb.cgi?p=dccp_exp.git;a=commitdiff;h=87cf8b3ce96397f934775617ba24eb1ff404747a

With a manpage I mean to document 
 * how the two currently available qpolicies ("simple" and "prio") are enabled;
 * which additional constraints they have (e.g. how they interpret tx_qlen);
 * information specific to a policy (e.g. that higher priority values in the
   "prio" policy mean a higher priority, comparable to SO_PRIORITY in socket(7));
 * which ancillary data the policies require (the procfs parameters)
   - simple: no ancillary parameters
   - prio:   u32 priority value, passed as DCCP_SCM_PRIORITY via cmsg(3),
             using a level of SOL_DCCP and a length of CMSG_LEN(sizeof(uint32_t));
 * ... probably I have missed something here

With that kind of documentation, I think, we would not need runtime discovery
of parameters.

Re: [PATCH 1/1] [DCCP][QPOLICY]: Make information about qpolicies available to userspace

[Tomasz Grobelny]

Dnia Thursday 12 of June 2008, Gerrit Renker napisał:
> | But if the application wants information about implemented parameters
> | there is currently no way to get it. It can try to send() packet and
> | check return value (which is quite a hack to be honest). It will work if
> | the parameter is not implemented at all. But what happens when you try to
> | set priority for a packet and use "simple" qpolicy? AFAIR nothing - you
> | get no information whatsoever that this parameter will not be used inside
> | qpolicy.
> |
> | And that's why we need information about parameters in /proc or
> | /proc/sys.
>
> Sorry I can't see here why we need information about parameters. When
> introducing a new socket option there is a convention of how to use it.
>
> For sockopts, the type of the argument is listed for each option type.
>
You seem to assume that a given qpolicy is a fixed set of functionality at the 
time of submission to the mainline kernel. In that case you are right, the 
application developer knows which parameters he can attach to a packet to be 
sent (and (s)he knows it at the time of development), so no parameter 
detection is necessary.

But I view it a bit differently. I think we have to go back to the discussion 
about how to add the "timeout" parameter (or any other). Basically there are 
two ways:
 1. add a new "timeout" qpolicy (as you proposed),
 2. extend an existing "prio" qpolicy adding DCCP_SCM_TIMEOUT parameter (as I 
propose). BTW, the "prio" name is a bit misleading, the qpolicy should 
probably be named "advanced" or something like that.

In other words the basic question is: do we want to add new parameters to 
existing qpolicies (then we need parameter discovery) or we don't want new 
parameters (then we don't need information about parameters available at 
runtime).

Having defined the alternatives it's time to decide which is better. I, of 
course, claim that mine (which is adding new parameters to existing 
qpolicies). That's simply because I think that providing both 
DCCP_SCM_TIMEOUT and DCCP_SCM_PRIORITY parameters may be useful. And I don't 
see an obvoius way of achieving that goal with "new policy for new parameter" 
approach.

I hope this lengthy text is understandable. Either way, the question is 
simple: what is your concept of adding DCCP_SCM_TIMEOUT parameter?

> | > If we agree on using unique strings to identify qpolicies then we can
> | > keep the runtime discovery much simpler. I think a manpage would be
> | > more helpful here, since the runtime discovery of parameters is not
> | > immediately obvious.
> |
> | Manpage of what? Documentation is certainly needed but before writing one
> | we need to have a basic implementation we agree upon.
> | --
>
> There is indeed a misunderstanding.
>
> We have already an implementation we agreed upon:
> http://eden-feed.erg.abdn.ac.uk/cgi-bin/gitweb.cgi?p=dccp_exp.git;a=commitd
>iff;h=87cf8b3ce96397f934775617ba24eb1ff404747a
>
Yes, but I would consider it not yet ready for application development. Why? 
See subject of this mail and previous paragraphs.

> With a manpage I mean to document
Is there any "man dccp"?

>  * how the two currently available qpolicies ("simple" and "prio") are
> enabled; * which additional constraints they have (e.g. how they interpret
> tx_qlen); * information specific to a policy (e.g. that higher priority
> values in the "prio" policy mean a higher priority, comparable to
> SO_PRIORITY in socket(7)); * which ancillary data the policies require (the
> procfs parameters) - simple: no ancillary parameters
>    - prio:   u32 priority value, passed as DCCP_SCM_PRIORITY via cmsg(3),
>              using a level of SOL_DCCP and a length of
> CMSG_LEN(sizeof(uint32_t)); * ... probably I have missed something here
>
Fine. But where exactly should it be described? Which file? I downloaded 
latest man pages from [1] but dccp is not even mentioned there. Or should a 
new man page be created (which would involve explaining what is dccp in the 
first place)?

[1] http://www.kernel.org/pub/linux/docs/manpages/man-pages-2.80.tar.bz2

> With that kind of documentation, I think, we would not need runtime
> discovery of parameters.
I still don't agree, see previous paragraphs.
-- 
Regards,
Tomasz Grobelny

Re: [PATCH 1/1] [DCCP][QPOLICY]: Make information about qpolicies available to userspace

[Gerrit Renker]

| In other words the basic question is: do we want to add new parameters to
| existing qpolicies (then we need parameter discovery) or we don't want new
| parameters (then we don't need information about parameters available at
| runtime).
|
| Having defined the alternatives it's time to decide which is better. I, of
| course, claim that mine (which is adding new parameters to existing
| qpolicies). That's simply because I think that providing both
| DCCP_SCM_TIMEOUT and DCCP_SCM_PRIORITY parameters may be useful. And I don't
| see an obvoius way of achieving that goal with "new policy for new parameter"
| approach.
I agree that providing both parameters may be useful, but don't see this
as a place of contradiction.

In the kernel I think it is best to make it type-safe, i.e. no new parameters
to already-defined policies. But I can't see how this would restrict the use.

In particular, the following is possible:

 (a) defer the dynamic runtime discovery to an application-library (wrappers
     around system calls which, according to the presented information,
     select an appropriate qpolicy and fill in its parameters);
 (b) add rules to allow runtime-switching of policies: for example,a user first
     selects "prio", but then provides a timeout parameter. Instead of returning
     "parameter not understood", the interface could simply `upgrade' the current
     policy from "prio" to "timed-prio". A matrix is below.

| I hope this lengthy text is understandable. Either way, the question is
| simple: what is your concept of adding DCCP_SCM_TIMEOUT parameter?
We find both 'atomic' and 'aggregate' (combined) policies:
 1) "prio"    standalone,
 2) "timeout" standalone,
 3) "prio"    combined with "timeout" (called something like "timed-prio").

With regard to parameters, this leads to the following matrix:

    +-------------+--------+-------------------+------------------+
    | Policy Name | Policy |         Presence of Parameters       |
    |             | ID#    | DCCP_SCM_PRIORITY | DCCP_SCM_TIMEOUT |
    +-------------+--------+-------------------+------------------+
    | "simple     |   0    |        no         |       no         |
    | "prio"      |   1    |        YES        |       no         |
    | "timed-prio"|   2?   |        YES        |       YES        |
    +-------------+--------+-------------------+------------------+


| > With a manpage I mean to document
| Is there any "man dccp"?
|
Not yet. If you or someone else can find time to contribute towards a DCCP manpage,
that would be just great. The best available information so far is on the OSDL pages
for DCCP and Documentation/networking/dccp.txt.

I think there is a special maintainer for the kernel manpages, who could
be emailed with a basic manpage. But ther ere is a similar problem - if the
API changes frequently, then this requires to update the manpage accordingly.

As alternatives to documentation, there are for example providing
 *running source code,
 * web pages or
 * use cases posted to this list
 * ...

Re: [PATCH 1/1] [DCCP][QPOLICY]: Make information about qpolicies available to userspace

[Tomasz Grobelny]

Dnia Tuesday 17 of June 2008, Gerrit Renker napisał:
> | In other words the basic question is: do we want to add new parameters to
> | existing qpolicies (then we need parameter discovery) or we don't want
> | new parameters (then we don't need information about parameters available
> | at runtime).
> |
> | Having defined the alternatives it's time to decide which is better. I,
> | of course, claim that mine (which is adding new parameters to existing
> | qpolicies). That's simply because I think that providing both
> | DCCP_SCM_TIMEOUT and DCCP_SCM_PRIORITY parameters may be useful. And I
> | don't see an obvoius way of achieving that goal with "new policy for new
> | parameter" approach.
>
> I agree that providing both parameters may be useful, but don't see this
> as a place of contradiction.
>
> In the kernel I think it is best to make it type-safe, i.e. no new
> parameters to already-defined policies. But I can't see how this would
> restrict the use.
> (...)
>
So we would need new policy for each new parameter, right? I somehow don't 
like it but it should work. You are right that this way runtime parameter 
discovery is not necessary. BTW, we will have a bit of a problem with naming 
qpolicies ;-)

> | > With a manpage I mean to document
> |
> | Is there any "man dccp"?
>
> Not yet. If you or someone else can find time to contribute towards a DCCP
> manpage, that would be just great. The best available information so far is
> on the OSDL pages for DCCP and Documentation/networking/dccp.txt.
>
But it's not in kernel sources? Then probably a new page would not be accepted 
as it would describe an experimental API. The above mentioned dccp.txt is 
probably the best place for now.

> I think there is a special maintainer for the kernel manpages, who could
> be emailed with a basic manpage. But ther ere is a similar problem - if the
> API changes frequently, then this requires to update the manpage
> accordingly.
>
I guess these would mostly be API additions, without any incompatible changes.

> As alternatives to documentation, there are for example providing
>  *running source code,
I will for sure be working on some apps to demonstrate how the new interface 
can be used. But first I need to create a simple server/client transmitting 
G.726 encoded voice over UDP/RTP (any hints on that?)...

>  * web pages or
Is there any official, publicly editable DCCP wiki? I'd rather not create yet 
another page...
-- 
Regards,
Tomasz Grobelny

Re: [PATCH 1/1] [DCCP][QPOLICY]: Make information about qpolicies available to userspace

[Randy.Dunlap]

On Tue, 17 Jun 2008, Tomasz Grobelny wrote:

> Dnia Tuesday 17 of June 2008, Gerrit Renker napisa?:
> > | In other words the basic question is: do we want to add new parameters to
> > | existing qpolicies (then we need parameter discovery) or we don't want
> > | new parameters (then we don't need information about parameters available
> > | at runtime).
> > |
> > | Having defined the alternatives it's time to decide which is better. I,
> > | of course, claim that mine (which is adding new parameters to existing
> > | qpolicies). That's simply because I think that providing both
> > | DCCP_SCM_TIMEOUT and DCCP_SCM_PRIORITY parameters may be useful. And I
> > | don't see an obvoius way of achieving that goal with "new policy for new
> > | parameter" approach.
> >
> > I agree that providing both parameters may be useful, but don't see this
> > as a place of contradiction.
> >
> > In the kernel I think it is best to make it type-safe, i.e. no new
> > parameters to already-defined policies. But I can't see how this would
> > restrict the use.
> > (...)
> >
> So we would need new policy for each new parameter, right? I somehow don't 
> like it but it should work. You are right that this way runtime parameter 
> discovery is not necessary. BTW, we will have a bit of a problem with naming 
> qpolicies ;-)
> 
> > | > With a manpage I mean to document
> > |
> > | Is there any "man dccp"?
> >
> > Not yet. If you or someone else can find time to contribute towards a DCCP
> > manpage, that would be just great. The best available information so far is
> > on the OSDL pages for DCCP and Documentation/networking/dccp.txt.
> >
> But it's not in kernel sources? Then probably a new page would not be accepted 
> as it would describe an experimental API. The above mentioned dccp.txt is 
> probably the best place for now.
> 
> > I think there is a special maintainer for the kernel manpages, who could
> > be emailed with a basic manpage. But ther ere is a similar problem - if the
> > API changes frequently, then this requires to update the manpage
> > accordingly.

Michael's contact info is at http://kernel.org/doc/man-pages/ .

> I guess these would mostly be API additions, without any incompatible changes.
> 
> > As alternatives to documentation, there are for example providing
> >  *running source code,
> I will for sure be working on some apps to demonstrate how the new interface 
> can be used. But first I need to create a simple server/client transmitting 
> G.726 encoded voice over UDP/RTP (any hints on that?)...
> 
> >  * web pages or
> Is there any official, publicly editable DCCP wiki? I'd rather not create yet 
> another page...

The Linux Foundation wiki is probably as close to official as it gets.

  http://www.linuxfoundation.org/en/Net:Main_Page

-- 
~Randy

Re: [PATCH 1/1] [DCCP][QPOLICY]: Make information about qpolicies available to userspace

[Gerrit Renker]

| > In the kernel I think it is best to make it type-safe, i.e. no new
| > parameters to already-defined policies. But I can't see how this would
| > restrict the use.
| > (...)
| >
| So we would need new policy for each new parameter, right? I somehow don't 
| like it but it should work. You are right that this way runtime parameter 
| discovery is not necessary. BTW, we will have a bit of a problem with naming 
| qpolicies ;-)
| 
Yes, not claiming that my suggestions were very good.

By the way, did you notice that the matrix was incomplete - timeout standalone
was missing, the corrected version would look something like

    +-------------+--------+-------------------+------------------+
    | Policy Name | Policy |         Presence of Parameters       |
    |             | ID#    | DCCP_SCM_PRIORITY | DCCP_SCM_TIMEOUT |
    +-------------+--------+-------------------+------------------+
    | "simple     |   0    |        no         |        no        |
    | "prio"      |   1    |       YES         |        no        |
    | "timeout"   |   2?   |        no         |       YES        |
    | "timed-prio"|   3?   |       YES         |       YES        |
    +-------------+--------+-------------------+------------------+

There is a special trick for the aggregate "timed-prio" policy: it is
the logical-OR of the "prio" and "timeout" policy IDs. Perhaps composite
policy IDs could be defined in this way.


| I will for sure be working on some apps to demonstrate how the new interface 
| can be used. But first I need to create a simple server/client transmitting 
| G.726 encoded voice over UDP/RTP (any hints on that?)...
| 
Tommi Saviranta has been working on a DCCP port for SpeexComm:
	http://tuomas.kulve.fi/projects/speexcomm/
The port is available via SVN:
	svn co http://tuomas.kulve.fi/svn/speexcomm

Leandro Sales de Melo has developed a DCCP plugin for gstreamer and used
it with VoIP/speex. It comes with example applications for these:
https://garage.maemo.org/frs/?group_id=297
https://garage.maemo.org/projects/ephone
http://www.mail-archive.com/dccp@vger.kernel.org/msg03101.html

Re: [PATCH 1/1] [DCCP][QPOLICY]: Make information about qpolicies available to userspace

[Tomasz Grobelny]

Dnia Wednesday 18 of June 2008, Gerrit Renker napisał:
> | > In the kernel I think it is best to make it type-safe, i.e. no new
> | > parameters to already-defined policies. But I can't see how this would
> | > restrict the use.
> | > (...)
> |
> | So we would need new policy for each new parameter, right? I somehow
> | don't like it but it should work. You are right that this way runtime
> | parameter discovery is not necessary. BTW, we will have a bit of a
> | problem with naming qpolicies ;-)
>
> Yes, not claiming that my suggestions were very good.
>
> By the way, did you notice that the matrix was incomplete - timeout
> standalone was missing, the corrected version would look something like
>
Well, I assumed that you did that on purpose because that would imply that we 
need more or less N policies for N parameters added one at a time. When we 
want separate policy for just timeout we end up with 2^N policies. And that 
would be just mad because the same effect can easily be achieved with setting 
priority to the same value on all packets.

>     +-------------+--------+-------------------+------------------+
>
>     | Policy Name | Policy |         Presence of Parameters       |
>     |
>     |             | ID#    | DCCP_SCM_PRIORITY | DCCP_SCM_TIMEOUT |
>
>     +-------------+--------+-------------------+------------------+
>
>     | "simple     |   0    |        no         |        no        |
>     | "prio"      |   1    |       YES         |        no        |
>     | "timeout"   |   2?   |        no         |       YES        |
>     | "timed-prio"|   3?   |       YES         |       YES        |
>
>     +-------------+--------+-------------------+------------------+
>
> There is a special trick for the aggregate "timed-prio" policy: it is
> the logical-OR of the "prio" and "timeout" policy IDs. Perhaps composite
> policy IDs could be defined in this way.
>
Then we would have straight mapping between possible parameters and policy 
numbers. That is, effectively we would not set the policy number but request 
the capability of processing certain parameters, right? So that the 
application could specify 0, DCCP_SCM_PRIORITY, DCCP_SCM_TIMEOUT or 
DCCP_SCM_PRIORITY|DCCP_SCM_TIMEOUT as the policy ID?

Could be nice, but what happens if we have two policies with the same set of 
supported parameters? How do we differentiate them? In the above: how do we 
diffrentiate between simple policy (which doesn't have any parameters ever) 
and prio policy with no parameters (just because we don't need to provide any 
for one particular case)?

> | I will for sure be working on some apps to demonstrate how the new
> | interface can be used. But first I need to create a simple server/client
> | transmitting G.726 encoded voice over UDP/RTP (any hints on that?)...
>
> Tommi Saviranta has been working on a DCCP port for SpeexComm:
> 	http://tuomas.kulve.fi/projects/speexcomm/
> The port is available via SVN:
> 	svn co http://tuomas.kulve.fi/svn/speexcomm
>
> Leandro Sales de Melo has developed a DCCP plugin for gstreamer and used
> it with VoIP/speex. It comes with example applications for these:
> https://garage.maemo.org/frs/?group_id=297
> https://garage.maemo.org/projects/ephone
> http://www.mail-archive.com/dccp@vger.kernel.org/msg03101.html
> 
I'll have a look at these. It seems that GStreamer is the way to go. However, 
there is a minor glitch. GStreamer doesn't support a clean way of passing 
priority data between plugins, so the code will probably be a bit ugly.
-- 
Regards,
Tomasz Grobelny

Re: [PATCH 1/1] [DCCP][QPOLICY]: Make information about qpolicies available to userspace

[Gerrit Renker]

| > By the way, did you notice that the matrix was incomplete - timeout
| > standalone was missing, the corrected version would look something like
| >
| Well, I assumed that you did that on purpose because that would imply that we 
| need more or less N policies for N parameters added one at a time. When we 
| want separate policy for just timeout we end up with 2^N policies. And that 
| would be just mad because the same effect can easily be achieved with setting 
| priority to the same value on all packets.
| 
No it was just forgotten. 

With regard to below, given N parameters, 2^N is the number of all
order-independent combinations of 0..N parameters. E.g. for parameters A,B,C:
 * {}                 (no parameters at all)
 * {A} {B} {C}	      (non-composite: 1 parameter)
 * {A,B} {A,C} {B,C}  (composite of N-1 parameters)	
 * {A,B,C}            (composite of N   parameters)

which gives 2^3 = 8 cases. The empty set corresponds to the "simple" policy.

| >     +-------------+--------+-------------------+------------------+
| >     | Policy Name | Policy |         Presence of Parameters       |
| >     |
| >     |             | ID#    | DCCP_SCM_PRIORITY | DCCP_SCM_TIMEOUT |
| >     +-------------+--------+-------------------+------------------+
| >     | "simple     |   0    |        no         |        no        |
| >     | "prio"      |   1    |       YES         |        no        |
| >     | "timeout"   |   2?   |        no         |       YES        |
| >     | "timed-prio"|   3?   |       YES         |       YES        |
| >
| >     +-------------+--------+-------------------+------------------+
| >
| > There is a special trick for the aggregate "timed-prio" policy: it is
| > the logical-OR of the "prio" and "timeout" policy IDs. Perhaps composite
| > policy IDs could be defined in this way.
| >
| Then we would have straight mapping between possible parameters and policy 
| numbers. That is, effectively we would not set the policy number but request 
| the capability of processing certain parameters, right? So that the 
| application could specify 0, DCCP_SCM_PRIORITY, DCCP_SCM_TIMEOUT or 
| DCCP_SCM_PRIORITY|DCCP_SCM_TIMEOUT as the policy ID?
| 
I think that this would make the 2^N mentioned by you above manageable.
We only need N parameters, but gain 2^N combinations practically "for free".

But such a bottom-up approach would not necessarily make sense, since
likely the policy will do a bit more work than just reacting to the
parameters it uses, which leads to the next point.

| Could be nice, but what happens if we have two policies with the same set of 
| supported parameters? How do we differentiate them? In the above: how do we 
| diffrentiate between simple policy (which doesn't have any parameters ever) 
| and prio policy with no parameters (just because we don't need to provide any 
| for one particular case)?
We could still encode the parameters in the policy ID, by splitting the
bitmask, e.g. the low 24 bits to encode parameters, and the high 8 bits
to encode the policies using the same combination of parameters.
Numbers depend on how many different parameters/policies to support:

 * e.g. using x = 32 bits, 
 * l: lower bits  corresponding to the number of parameters
 * h: higher bits; 2^h = number of policies with same parameters

It seems h may not need to be very high. How many policies with the
same parameters should we support -- 2/4/8/16 (corresponding to	h=1/2/3/4)?
That would still leave room for up to 28 different parameters.

| > Leandro Sales de Melo has developed a DCCP plugin for gstreamer and used
| > it with VoIP/speex. It comes with example applications for these:
| > https://garage.maemo.org/frs/?group_id=297
| > https://garage.maemo.org/projects/ephone
| > http://www.mail-archive.com/dccp@vger.kernel.org/msg03101.html
| > 
| I'll have a look at these. It seems that GStreamer is the way to go. However, 
| there is a minor glitch. GStreamer doesn't support a clean way of passing 
| priority data between plugins, so the code will probably be a bit ugly.
| -- 
It is probably best to ask Leandro directly as he knows the code well and has
done a lot of work on gstreamer.

Re: [PATCH 1/1] [DCCP][QPOLICY]: Make information about qpolicies available to userspace

[Tomasz Grobelny]

Dnia Thursday 19 of June 2008, Gerrit Renker napisał:
> But such a bottom-up approach would not necessarily make sense, since
> likely the policy will do a bit more work than just reacting to the
> parameters it uses, which leads to the next point.
>
Agreed.

> | Could be nice, but what happens if we have two policies with the same set
> | of supported parameters? How do we differentiate them? In the above: how
> | do we diffrentiate between simple policy (which doesn't have any
> | parameters ever) and prio policy with no parameters (just because we
> | don't need to provide any for one particular case)?
>
> We could still encode the parameters in the policy ID, by splitting the
> bitmask, e.g. the low 24 bits to encode parameters, and the high 8 bits
> to encode the policies using the same combination of parameters.
>
Yes, technically we could do it this way. Encoding accepted parameters in 
policy ID would provide applications with means to check whether certain 
parameters are supported. But should we mix two IMO distinct concepts (policy 
behaviour and accepted parameters) in one bitfield? I'd rather have them 
separated.

Keeping policy IDs as they are and using a bitfield for just parameters could 
be a nice idea. It would certainly  simplify checking which parameters are 
supported - just a simple & and == would suffice.
-- 
Regards,
Tomasz Grobelny

Re: [PATCH 1/1] [DCCP][QPOLICY]: Make information about qpolicies available to userspace

[Gerrit Renker]

| > We could still encode the parameters in the policy ID, by splitting the
| > bitmask, e.g. the low 24 bits to encode parameters, and the high 8 bits
| > to encode the policies using the same combination of parameters.
| >
| Yes, technically we could do it this way. Encoding accepted parameters in 
| policy ID would provide applications with means to check whether certain 
| parameters are supported. But should we mix two IMO distinct concepts (policy 
| behaviour and accepted parameters) in one bitfield? I'd rather have them 
| separated.
| 
Yes having them separate gives a clearer semantics. But it comes at a price - 
to retrieve the parameters, we need an extra function or lookup table. 

Maybe there is an elegant solution which allows to encode the required
parameters while keeping the semantics clear?

| Keeping policy IDs as they are and using a bitfield for just parameters could 
| be a nice idea. It would certainly  simplify checking which parameters are 
| supported - just a simple & and == would suffice.
| -- 
And it is fast, too.

Re: [PATCH 1/1] [DCCP][QPOLICY]: Make information about qpolicies available to userspace

[Tomasz Grobelny]

Dnia Monday 23 of June 2008, Gerrit Renker napisał:
> | > We could still encode the parameters in the policy ID, by splitting the
> | > bitmask, e.g. the low 24 bits to encode parameters, and the high 8 bits
> | > to encode the policies using the same combination of parameters.
> |
> | Yes, technically we could do it this way. Encoding accepted parameters in
> | policy ID would provide applications with means to check whether certain
> | parameters are supported. But should we mix two IMO distinct concepts
> | (policy behaviour and accepted parameters) in one bitfield? I'd rather
> | have them separated.
>
> Yes having them separate gives a clearer semantics. But it comes at a price
> - to retrieve the parameters, we need an extra function or lookup table.
>
An additional bitfield in dccp_qpolicy_operations should do it when it comes 
to storing that information. When it comes to retrieving see next point...

> Maybe there is an elegant solution which allows to encode the required
> parameters while keeping the semantics clear?
>
What about something like:
	setsockopt(sockfd, DCCP_SOCKOPT_QPOLICY_ID, DCCPQ_POLICY_PRIO);
to choose policy (exactly as it is now) and a new function to ensure that the 
kernel understands parameters:
	setsockopt(sockfd, DCCP_SOCKOPT_QPOLICY_PARAMS, DCCP_SCM_PRIORITY | 
DCCP_SCM_TIMEOUT);

The second call would return an error if the kernel does not know anything 
about DCCP_SCM_TIMEOUT (or a choosen policy does not support it). This would 
have the additional benefit (over similar getsockopt) that the kernel will be 
able to optimize its behaviour knowning the set of parameters that will be 
used.

What do you think about such an interface?
-- 
Regards,
Tomasz Grobelny

Re: [PATCH 1/1] [DCCP][QPOLICY]: Make information about qpolicies available to userspace

[Gerrit Renker]

| > Yes having them separate gives a clearer semantics. But it comes at a price
| > - to retrieve the parameters, we need an extra function or lookup table.
| >
| An additional bitfield in dccp_qpolicy_operations should do it when it comes 
| to storing that information. When it comes to retrieving see next point...
| 
| > Maybe there is an elegant solution which allows to encode the required
| > parameters while keeping the semantics clear?
| >
| What about something like:
| 	setsockopt(sockfd, DCCP_SOCKOPT_QPOLICY_ID, DCCPQ_POLICY_PRIO);
| to choose policy (exactly as it is now) and a new function to ensure that the 
| kernel understands parameters:
| 	setsockopt(sockfd, DCCP_SOCKOPT_QPOLICY_PARAMS, DCCP_SCM_PRIORITY | DCCP_SCM_TIMEOUT);
| 
| The second call would return an error if the kernel does not know anything 
| about DCCP_SCM_TIMEOUT (or a choosen policy does not support it). This would 
| have the additional benefit (over similar getsockopt) that the kernel will be 
| able to optimize its behaviour knowning the set of parameters that will be 
| used.
| 
| What do you think about such an interface?
| -- 
In both solutions there is a need to maintain an integrity constraint
between the qpolicy ID and the set of parameters that the qpolicy uses.

With the bitmask approach this constraint is expressed only once, when
the policy is defined in the source file.

With the socket approach, the constraint needs to be expressed each time
the policy is used.

I don't know your reservations against the bitmask approach (other than the
semantics), but I find having to define the parameters each time the qpolicy
is used cumbersome, and it can also be a source of errors.

Gerrit

Re: [PATCH 1/1] [DCCP][QPOLICY]: Make information about qpolicies available to userspace

[Tomasz Grobelny]

Dnia Wednesday 25 of June 2008, Gerrit Renker napisał:
> | > Yes having them separate gives a clearer semantics. But it comes at a
> | > price - to retrieve the parameters, we need an extra function or lookup
> | > table.
> |
> | An additional bitfield in dccp_qpolicy_operations should do it when it
> | comes to storing that information. When it comes to retrieving see next
> | point...
> |
> | > Maybe there is an elegant solution which allows to encode the required
> | > parameters while keeping the semantics clear?
> |
> | What about something like:
> | 	setsockopt(sockfd, DCCP_SOCKOPT_QPOLICY_ID, DCCPQ_POLICY_PRIO);
> | to choose policy (exactly as it is now) and a new function to ensure that
> | the kernel understands parameters:
> | 	setsockopt(sockfd, DCCP_SOCKOPT_QPOLICY_PARAMS, DCCP_SCM_PRIORITY |
> | DCCP_SCM_TIMEOUT);
> |
> | The second call would return an error if the kernel does not know
> | anything about DCCP_SCM_TIMEOUT (or a choosen policy does not support
> | it). This would have the additional benefit (over similar getsockopt)
> | that the kernel will be able to optimize its behaviour knowning the set
> | of parameters that will be used.
> |
> | What do you think about such an interface?
> | --
>
> In both solutions there is a need to maintain an integrity constraint
> between the qpolicy ID and the set of parameters that the qpolicy uses.
>
It is always the case, either explicit (as I propose) or implicit (when it 
would be a part of policy id).

> With the bitmask approach this constraint is expressed only once, when
> the policy is defined in the source file.
>
In what I propose it would also be defined once. Instead of:
	[DCCPQ_POLICY_PRIO] = {
		.push	= qpolicy_simple_push,
		.full	= qpolicy_prio_full,
		.top	= qpolicy_prio_best_skb,
	},
we would simply have:
	[DCCPQ_POLICY_PRIO] = {
		.push	= qpolicy_simple_push,
		.full	= qpolicy_prio_full,
		.top	= qpolicy_prio_best_skb,
		.params = DCCP_SCM_PRIORITY | DCCP_SCM_TIMEOUT,
	},
Doesn't seem complicated, does it?

> With the socket approach, the constraint needs to be expressed each time
> the policy is used.
>
Yes. Two points:
1. The other option is /proc I guess... would that be better? I think not 
(even though I had other optinion on this subject earlier).
2. I think that it is actually good to enforce on application developers the 
need to declare which parameters they want to use. This way they almost 
cannot do it wrong.

> I don't know your reservations against the bitmask approach (other than the
> semantics), but I find having to define the parameters each time the
> qpolicy is used cumbersome, and it can also be a source of errors.
>
The bitmask approach is ok but I view combining parameters with policy id 
counterintuitive.

To make things clear: we both agree that the application should be able to get 
information if the parameters it wants to use are supported by the kernel 
it's running on, right?

The place where we differ is that you would prefer to have:
setsockopt(sockfd, DCCP_POLICY_ID, DCCPQ_POLICY_PRIO|(DCCP_SCM_PRIORITY<<8));

and I would like to have:
setsockopt(sockfd, DCCP_POLICY_ID, DCCPQ_POLICY_PRIO);
setsockopt(sockfd, DCCP_POLICY_PARAMS, DCCP_SCM_PRIORITY);

Is that right?
-- 
Regards,
Tomasz Grobelny

Re: [PATCH 1/1] [DCCP][QPOLICY]: Make information about qpolicies available to userspace

[Gerrit Renker]

| > With the bitmask approach this constraint is expressed only once, when
| > the policy is defined in the source file.
| >
| In what I propose it would also be defined once. Instead of:
| 	[DCCPQ_POLICY_PRIO] = {
| 		.push	= qpolicy_simple_push,
| 		.full	= qpolicy_prio_full,
| 		.top	= qpolicy_prio_best_skb,
| 	},
| we would simply have:
| 	[DCCPQ_POLICY_PRIO] = {
| 		.push	= qpolicy_simple_push,
| 		.full	= qpolicy_prio_full,
| 		.top	= qpolicy_prio_best_skb,
| 		.params = DCCP_SCM_PRIORITY | DCCP_SCM_TIMEOUT,
| 	},
| Doesn't seem complicated, does it?
| 
... and is also simpler than the bitmask approach. Very good idea: let's use this.


| > With the socket approach, the constraint needs to be expressed each time
| > the policy is used.
| >
| Yes. Two points:
| 1. The other option is /proc I guess... would that be better? I think not 
| (even though I had other optinion on this subject earlier).
Why put so much effort into this? All that is required is to refuse to
accept nonsensical parameters.

| 2. I think that it is actually good to enforce on application developers the 
| need to declare which parameters they want to use. This way they almost 
| cannot do it wrong.
| 
I don't agree: such a safety-net will only annoy good programmers who understand
what they are doing. And it will not help bad programmers much, since their problems
lie elsewhere. 

Further, the protection is not a strong one: nothing stops the user from first
declaring parameters X/Y and then supply something completely different.

| To make things clear: we both agree that the application should be able to get 
| information if the parameters it wants to use are supported by the kernel 
| it's running on, right?
| 
That is the central point. What we agree on is that that the policy should validate
the parameters that the user supplies via cmsg. We don't need the bitmask approach,
and with your suggested solution we have a mapping (.params field) between parameters
and policy IDs.

Which is sufficient to find out which parameters are acceptable for a policy.

| The place where we differ is that you would prefer to have:
| setsockopt(sockfd, DCCP_POLICY_ID, DCCPQ_POLICY_PRIO|(DCCP_SCM_PRIORITY<<8));
| 
| and I would like to have:
| setsockopt(sockfd, DCCP_POLICY_ID, DCCPQ_POLICY_PRIO);
| setsockopt(sockfd, DCCP_POLICY_PARAMS, DCCP_SCM_PRIORITY);
| 
| Is that right?
| -- 
Not quite. The first is just one way of relating policy IDs and parameters. First, as
above, I prefer your approach of using a .params field because it is simpler.    

With regard to the second point, I don't like the DCCP_POLICY_PARAMS socket option
since it does not prevent the user from using nonsensical parameters.

To summarise, what we have so far is:
 * qpolicy parameters are declared as disjoint bit values so that they can
   be or-ed together and tested with `&'/`==' operations;
 * the qpol_table gets a new (u32) field to keep the list of parameters
   that a policy accepts;
 * dccp_msghdr_parse() (net/dccp/proto.c) calls a qpolicy check routine to see
   if cmsg->cmsg_type is a parameter which is allowed by the  current qpolicy
 * if the parameter is not mentioned in the `.params' field of the qpol_table,
   dccp_msghdr_parse() returns -EINVAL.

Re: [PATCH 1/1] [DCCP][QPOLICY]: Make information about qpolicies available to userspace

[Tomasz Grobelny]

Gerrit Renker pisze:
> | > With the bitmask approach this constraint is expressed only once, when
> | > the policy is defined in the source file.
> | >
> | In what I propose it would also be defined once. Instead of:
> | 	[DCCPQ_POLICY_PRIO] = {
> | 		.push	= qpolicy_simple_push,
> | 		.full	= qpolicy_prio_full,
> | 		.top	= qpolicy_prio_best_skb,
> | 	},
> | we would simply have:
> | 	[DCCPQ_POLICY_PRIO] = {
> | 		.push	= qpolicy_simple_push,
> | 		.full	= qpolicy_prio_full,
> | 		.top	= qpolicy_prio_best_skb,
> | 		.params = DCCP_SCM_PRIORITY | DCCP_SCM_TIMEOUT,
> | 	},
> | Doesn't seem complicated, does it?
> | 
> ... and is also simpler than the bitmask approach. Very good idea: let's use this.
> 
Ok, so now we know how to store information about parameters in kernel
structures. What remains to be discussed is how to pass that information
to userspace.

> | > With the socket approach, the constraint needs to be expressed each time
> | > the policy is used.
> | >
> | Yes. Two points:
> | 1. The other option is /proc I guess... would that be better? I think not 
> | (even though I had other optinion on this subject earlier).
> Why put so much effort into this?
Now I think that /proc is overly complicated approach.

> All that is required is to refuse to
> accept nonsensical parameters.
> 
You can refuse to accept them on sendmsg(). And IMO the application 
should be able to determine which parameters are correct earlier.

> | 2. I think that it is actually good to enforce on application developers the 
> | need to declare which parameters they want to use. This way they almost 
> | cannot do it wrong.
> | 
> I don't agree: such a safety-net will only annoy good programmers who understand
> what they are doing. And it will not help bad programmers much, since their problems
> lie elsewhere. 
> 
The history of software development shows that with newer programming 
languages you can do yourself less and less harm. Think of machine code 
-> assembler -> C -> C++ -> Java/C#. Ok, the last two languages assume 
the programmer is dumb but their success shows that people want 
fool-proof development environments (even if it restricts capabilities). 
In fact they want everything to be fool-proof (from toothbrush to car).

> Further, the protection is not a strong one: nothing stops the user from first
> declaring parameters X/Y and then supply something completely different.
> 
We could make it strong that is enforce that when a parameter was not 
declared it cannot be used.

> | To make things clear: we both agree that the application should be able to get 
> | information if the parameters it wants to use are supported by the kernel 
> | it's running on, right?
> | 
> That is the central point. What we agree on is that that the policy should validate
> the parameters that the user supplies via cmsg. 
Yes. But it's only part of the problem. We should also allow 
applications to detect which parameters are ok before calling sendmsg().

> We don't need the bitmask approach,
> and with your suggested solution we have a mapping (.params field) between parameters
> and policy IDs.
> 
> Which is sufficient to find out which parameters are acceptable for a policy.
> 
The .param field idea only shows how we store that information in 
kernel. It doesn't show how the user can retrieve that information.

> | The place where we differ is that you would prefer to have:
> | setsockopt(sockfd, DCCP_POLICY_ID, DCCPQ_POLICY_PRIO|(DCCP_SCM_PRIORITY<<8));
> | 
> | and I would like to have:
> | setsockopt(sockfd, DCCP_POLICY_ID, DCCPQ_POLICY_PRIO);
> | setsockopt(sockfd, DCCP_POLICY_PARAMS, DCCP_SCM_PRIORITY);
> | 
> | Is that right?
> | -- 
> Not quite. 
I knew we must have been talking different languages ;-)

> The first is just one way of relating policy IDs and parameters. First, as
> above, I prefer your approach of using a .params field because it is simpler.    
> 
Ok.

> To summarise, what we have so far is:
>  * qpolicy parameters are declared as disjoint bit values so that they can
>    be or-ed together and tested with `&'/`==' operations;
>  * the qpol_table gets a new (u32) field to keep the list of parameters
>    that a policy accepts;
>  * dccp_msghdr_parse() (net/dccp/proto.c) calls a qpolicy check routine to see
>    if cmsg->cmsg_type is a parameter which is allowed by the  current qpolicy
>  * if the parameter is not mentioned in the `.params' field of the qpol_table,
>    dccp_msghdr_parse() returns -EINVAL.
Yes, that's better than what we currently have (and I'll try to 
implement a patch for this).

But this still means that the application that wants to use 
DCCP_SCM_TIMEOUT can get information that it's not supported on calling 
sendmsg(). Which is IMO too late.
-- 
Regards,
Tomasz Grobelny