| Message ID | 20211108105711.16200-4-fw@strlen.de (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Delegated to: | Mat Martineau |
| Headers | show |
| Series | support for setsockopt(TCP_INQ) | expand |
| Context | Check | Description |
|---|---|---|
| matttbe/build | success | Build and static analysis OK |
| matttbe/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 70 lines checked |
| matttbe/KVM_Validation__normal | warning | Unstable: 2 failed test(s): packetdrill_sockopts selftest_mptcp_join |
| matttbe/KVM_Validation__debug | warning | Unstable: 1 failed test(s): packetdrill_sockopts |
Hi Florian, On 08/11/2021 11:57, Florian Westphal wrote: > Allows to query in-sequence data ready for read(), total bytes in > write queue and total bytes in write queue that have not yet been sent. Thank you for looking at that! (...) > +static int mptcp_ioctl(struct sock *sk, int cmd, unsigned long arg) > +{ > + struct mptcp_sock *msk = mptcp_sk(sk); > + int answ; > + bool slow; Very very small detail: we should swap the two lines. I can do that when applying the patches if no v2 is needed. Cheers, Matt
Hello, On Mon, 2021-11-08 at 11:57 +0100, Florian Westphal wrote: > Allows to query in-sequence data ready for read(), total bytes in > write queue and total bytes in write queue that have not yet been sent. > > Signed-off-by: Florian Westphal <fw@strlen.de> > --- > net/mptcp/protocol.c | 52 ++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 52 insertions(+) > > diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c > index b4263bf821ac..29b6f57b917e 100644 > --- a/net/mptcp/protocol.c > +++ b/net/mptcp/protocol.c > @@ -22,6 +22,7 @@ > #endif > #include <net/mptcp.h> > #include <net/xfrm.h> > +#include <asm/ioctls.h> > #include "protocol.h" > #include "mib.h" > > @@ -3260,6 +3261,56 @@ static int mptcp_forward_alloc_get(const struct sock *sk) > return sk->sk_forward_alloc + mptcp_sk(sk)->rmem_fwd_alloc; > } > > +static int mptcp_ioctl_outq(const struct mptcp_sock *msk, u64 v) > +{ > + const struct sock *sk = (void *)msk; > + u64 delta; > + > + if (sk->sk_state == TCP_LISTEN) > + return -EINVAL; > + > + if ((1 << sk->sk_state) & (TCPF_SYN_SENT | TCPF_SYN_RECV)) > + return 0; > + > + delta = READ_ONCE(msk->write_seq) - v; This is under the msk socket lock and write_seq is protected by the full/plain msk socket lock so READ_ONCE should not be necessary, I think. The same for 'snd_nxt' below. 'snd_una' instead is updated under the msk data lock, so the read once there should be required. Cheers, Paolo
Paolo Abeni <pabeni@redhat.com> wrote: > > + if (sk->sk_state == TCP_LISTEN) > > + return -EINVAL; > > + > > + if ((1 << sk->sk_state) & (TCPF_SYN_SENT | TCPF_SYN_RECV)) > > + return 0; > > + > > + delta = READ_ONCE(msk->write_seq) - v; > > This is under the msk socket lock and write_seq is protected by the > full/plain msk socket lock so READ_ONCE should not be necessary, I > think. The same for 'snd_nxt' below. Then why is write_seq updated with WRITE_ONCE in lots of places?
diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c index b4263bf821ac..29b6f57b917e 100644 --- a/net/mptcp/protocol.c +++ b/net/mptcp/protocol.c @@ -22,6 +22,7 @@ #endif #include <net/mptcp.h> #include <net/xfrm.h> +#include <asm/ioctls.h> #include "protocol.h" #include "mib.h" @@ -3260,6 +3261,56 @@ static int mptcp_forward_alloc_get(const struct sock *sk) return sk->sk_forward_alloc + mptcp_sk(sk)->rmem_fwd_alloc; } +static int mptcp_ioctl_outq(const struct mptcp_sock *msk, u64 v) +{ + const struct sock *sk = (void *)msk; + u64 delta; + + if (sk->sk_state == TCP_LISTEN) + return -EINVAL; + + if ((1 << sk->sk_state) & (TCPF_SYN_SENT | TCPF_SYN_RECV)) + return 0; + + delta = READ_ONCE(msk->write_seq) - v; + if (delta > INT_MAX) + delta = INT_MAX; + + return (int)delta; +} + +static int mptcp_ioctl(struct sock *sk, int cmd, unsigned long arg) +{ + struct mptcp_sock *msk = mptcp_sk(sk); + int answ; + bool slow; + + switch (cmd) { + case SIOCINQ: + if (sk->sk_state == TCP_LISTEN) + return -EINVAL; + + slow = lock_sock_fast(sk); + answ = mptcp_inq_hint(sk); + unlock_sock_fast(sk, slow); + break; + case SIOCOUTQ: + slow = lock_sock_fast(sk); + answ = mptcp_ioctl_outq(msk, READ_ONCE(msk->snd_una)); + unlock_sock_fast(sk, slow); + break; + case SIOCOUTQNSD: + slow = lock_sock_fast(sk); + answ = mptcp_ioctl_outq(msk, READ_ONCE(msk->snd_nxt)); + unlock_sock_fast(sk, slow); + break; + default: + return -ENOIOCTLCMD; + } + + return put_user(answ, (int __user *)arg); +} + static struct proto mptcp_prot = { .name = "MPTCP", .owner = THIS_MODULE, @@ -3272,6 +3323,7 @@ static struct proto mptcp_prot = { .shutdown = mptcp_shutdown, .destroy = mptcp_destroy, .sendmsg = mptcp_sendmsg, + .ioctl = mptcp_ioctl, .recvmsg = mptcp_recvmsg, .release_cb = mptcp_release_cb, .hash = mptcp_hash,
Allows to query in-sequence data ready for read(), total bytes in write queue and total bytes in write queue that have not yet been sent. Signed-off-by: Florian Westphal <fw@strlen.de> --- net/mptcp/protocol.c | 52 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 52 insertions(+)