Subject: [PATCH] sctp: implement SIOCINQ ioctl()

This simple patch copies the current approach for SIOCINQ ioctl() from DCCP
into SCTp so that the userland code working with SCTP can use a similar
interface across different protocols to know how much space to allocate for
a buffer.
---
net/sctp/socket.c | 31 ++++++++++++++++++++++++++++++-
1 files changed, 30 insertions(+), 1 deletions(-)

diff --git a/net/sctp/socket.c b/net/sctp/socket.c
index 44a1ab0..3135b4e 100644
--- a/net/sctp/socket.c
+++ b/net/sctp/socket.c
@@ -3595,7 +3595,36 @@ out:
/* The SCTP ioctl handler. */
SCTP_STATIC int sctp_ioctl(struct sock *sk, int cmd, unsigned long arg)
{
- return -ENOIOCTLCMD;
+ int rc = -ENOTCONN;
+
+ sctp_lock_sock(sk);
+
+ if (sctp_sstate(sk, LISTENING))
+ goto out;
+
+ switch(cmd) {
+ case SIOCINQ: {
+ struct sk_buff *skb;
+ unsigned long amount = 0;
+
+ skb = skb_peek(&sk->sk_receive_queue);
+ if (skb != NULL) {
+ /*
+ * We will only return the amount of this packet since
+ * that is all that will be read.
+ */
+ amount = skb->len;
+ }
+ rc = put_user(amount, (int __user *)arg);
+ }
+ break;
+ default:
+ rc = -ENOIOCTLCMD;
+ break;
+ }
+out:
+ sctp_release_sock(sk);
+ return rc;
}

/* This is the function which gets called during socket creation to
--
1.7.1


2010-06-24 13:19:54

by Vlad Yasevich

[permalink] [raw]
Subject: Re: [PATCH] sctp: implement SIOCINQ ioctl()



Diego Elio 'Flameeyes' Petten? wrote:
> This simple patch copies the current approach for SIOCINQ ioctl() from DCCP
> into SCTp so that the userland code working with SCTP can use a similar
> interface across different protocols to know how much space to allocate for
> a buffer.
> ---
> net/sctp/socket.c | 31 ++++++++++++++++++++++++++++++-
> 1 files changed, 30 insertions(+), 1 deletions(-)
>
> diff --git a/net/sctp/socket.c b/net/sctp/socket.c
> index 44a1ab0..3135b4e 100644
> --- a/net/sctp/socket.c
> +++ b/net/sctp/socket.c
> @@ -3595,7 +3595,36 @@ out:
> /* The SCTP ioctl handler. */
> SCTP_STATIC int sctp_ioctl(struct sock *sk, int cmd, unsigned long arg)
> {
> - return -ENOIOCTLCMD;
> + int rc = -ENOTCONN;
> +
> + sctp_lock_sock(sk);
> +
> + if (sctp_sstate(sk, LISTENING))

This should add a check for sctp_style(sk, TCP), since one can't read from
a TCP style listening sockets, but can do so from UDP-style (SOCK_SEQPACKET).

-vlad

> + goto out;
> +
> + switch(cmd) {
> + case SIOCINQ: {
> + struct sk_buff *skb;
> + unsigned long amount = 0;
> +
> + skb = skb_peek(&sk->sk_receive_queue);
> + if (skb != NULL) {
> + /*
> + * We will only return the amount of this packet since
> + * that is all that will be read.
> + */
> + amount = skb->len;
> + }
> + rc = put_user(amount, (int __user *)arg);
> + }
> + break;
> + default:
> + rc = -ENOIOCTLCMD;
> + break;
> + }
> +out:
> + sctp_release_sock(sk);
> + return rc;
> }
>
> /* This is the function which gets called during socket creation to

Subject: Re: [PATCH] sctp: implement SIOCINQ ioctl()

Il giorno gio, 24/06/2010 alle 09.19 -0400, Vlad Yasevich ha scritto:
>
> This should add a check for sctp_style(sk, TCP), since one can't read
> from
> a TCP style listening sockets, but can do so from UDP-style
> (SOCK_SEQPACKET).

I don't want to sound arrogant but... are you sure?

I ask because the simple testcase I wrote to make sure I didn't get it
wrong opened the socket as SOCK_STREAM, and yet all of this worked fine
(I'm attaching the source, for the sake of it)...

I sure hope you're mistaken here and it is _supposed_ to work here as
well, as we cannot use SOCK_SEQPACKET in the software I'm writing this
for (feng, from the lscube project) as accept() fails on SOCK_SEQPACKET
(EOPNOTSUPP) -- which itslef is strange given that the man page for
accept(2) reports it's supported on SOCK_STREAM and SOCK_SEQPACKET.

--
Diego Elio Pettenò — “Flameeyes”
http://blog.flameeyes.eu/

If you found a .asc file in this mail and know not what it is,
it's a GnuPG digital signature: http://www.gnupg.org/


Attachments:
sctp-fionread.c (2.05 kB)

2010-06-24 13:40:55

by Vlad Yasevich

[permalink] [raw]
Subject: Re: [PATCH] sctp: implement SIOCINQ ioctl()



Diego Elio “Flameeyes” Pettenò wrote:
> Il giorno gio, 24/06/2010 alle 09.19 -0400, Vlad Yasevich ha scritto:
>> This should add a check for sctp_style(sk, TCP), since one can't read
>> from
>> a TCP style listening sockets, but can do so from UDP-style
>> (SOCK_SEQPACKET).
>
> I don't want to sound arrogant but... are you sure?

Yes, I am sure. :)

>
> I ask because the simple testcase I wrote to make sure I didn't get it
> wrong opened the socket as SOCK_STREAM, and yet all of this worked fine
> (I'm attaching the source, for the sake of it)...
>
> I sure hope you're mistaken here and it is _supposed_ to work here as
> well, as we cannot use SOCK_SEQPACKET in the software I'm writing this
> for (feng, from the lscube project) as accept() fails on SOCK_SEQPACKET
> (EOPNOTSUPP) -- which itslef is strange given that the man page for
> accept(2) reports it's supported on SOCK_STREAM and SOCK_SEQPACKET.
>

You don't call accept() on an SCTP SEQPACKET socket. You can just read from it.
SCTP uses SOCK_SEQPACKET to implement 1-to-many semantics, where you can have
multiple associations all serviced by the same socket. In that case, the socket
is placed into the listening state and then the application may simply read
from it.

Thus your code that just returns the size of the first skb should take that into
consideration.

So the condition really is:
if (sctp_style(sk, TCP) && sctp_state(sk, LISTENING))
goto out;

This way, you'll ignore listening SOCK_STREAM sockets which will not have any
data anyway, but you'll check out listening SOCK_SEQPACKET sockets that may have
data waiting on them.

-vlad

Subject: Re: [PATCH] sctp: implement SIOCINQ ioctl()

Il giorno gio, 24/06/2010 alle 09.40 -0400, Vlad Yasevich ha scritto:
> You don't call accept() on an SCTP SEQPACKET socket. You can just
> read from it.

Okay I guess I'll send a patch to the man page to fix that then.

> This way, you'll ignore listening SOCK_STREAM sockets which will not
> have any
> data anyway, but you'll check out listening SOCK_SEQPACKET sockets
> that may have
> data waiting on them.

Ah I think I see what you mean here: SIOCINQ would work for TCP-style
ESTABLISHED sockets _and_ SEQPACKET LISTENING sockets, right?

Will send the updated patch in a moment.

--
Diego Elio Pettenò — “Flameeyes”
http://blog.flameeyes.eu/

If you found a .asc file in this mail and know not what it is,
it's a GnuPG digital signature: http://www.gnupg.org/

2010-06-24 13:58:34

by Vlad Yasevich

[permalink] [raw]
Subject: Re: [PATCH] sctp: implement SIOCINQ ioctl()



Diego Elio “Flameeyes” Pettenò wrote:
> Il giorno gio, 24/06/2010 alle 09.40 -0400, Vlad Yasevich ha scritto:
>> You don't call accept() on an SCTP SEQPACKET socket. You can just
>> read from it.
>
> Okay I guess I'll send a patch to the man page to fix that then.

I don't think the man page needs an update. This is just an SCTP-ism.
There are other protocols using SOCK_SEQPACKET that implement and support
accept() call.

The SCTP api spec decided that it wouldn't support accept() and we have to
follow it.


>
>> This way, you'll ignore listening SOCK_STREAM sockets which will not
>> have any
>> data anyway, but you'll check out listening SOCK_SEQPACKET sockets
>> that may have
>> data waiting on them.
>
> Ah I think I see what you mean here: SIOCINQ would work for TCP-style
> ESTABLISHED sockets _and_ SEQPACKET LISTENING sockets, right?

Right.

-vlad

>
> Will send the updated patch in a moment.
>