* [MPTCP] Re: [PATCH] Squash-to: "mptcp: Handle MP_CAPABLE options for outgoing connections"
@ 2019-12-16 21:30 Florian Westphal
0 siblings, 0 replies; 4+ messages in thread
From: Florian Westphal @ 2019-12-16 21:30 UTC (permalink / raw)
To: mptcp
[-- Attachment #1: Type: text/plain, Size: 200 bytes --]
Paolo Abeni <pabeni(a)redhat.com> wrote:
> mptcp_established_options() definition will be changed in a couple of patches,
> let's introduce early it's final form
ACK, I think this is preferrable.
^ permalink raw reply [flat|nested] 4+ messages in thread
* [MPTCP] Re: [PATCH] Squash-to: "mptcp: Handle MP_CAPABLE options for outgoing connections"
@ 2019-12-16 22:05 Mat Martineau
0 siblings, 0 replies; 4+ messages in thread
From: Mat Martineau @ 2019-12-16 22:05 UTC (permalink / raw)
To: mptcp
[-- Attachment #1: Type: text/plain, Size: 3208 bytes --]
On Mon, 16 Dec 2019, Paolo Abeni wrote:
> mptcp_established_options() definition will be changed in a couple of patches,
> let's introduce early it's final form
>
> Signed-off-by: Paolo Abeni <pabeni(a)redhat.com>
> ---
> include/net/mptcp.h | 5 ++++-
> net/ipv4/tcp_output.c | 9 ++++-----
> net/mptcp/options.c | 3 ++-
> 3 files changed, 10 insertions(+), 7 deletions(-)
>
> diff --git a/include/net/mptcp.h b/include/net/mptcp.h
> index d7af03848f5a..588abcb76da3 100644
> --- a/include/net/mptcp.h
> +++ b/include/net/mptcp.h
> @@ -55,7 +55,8 @@ bool mptcp_syn_options(struct sock *sk, unsigned int *size,
> void mptcp_rcv_synsent(struct sock *sk);
> bool mptcp_synack_options(const struct request_sock *req, unsigned int *size,
> struct mptcp_out_options *opts);
> -bool mptcp_established_options(struct sock *sk, unsigned int *size,
> +bool mptcp_established_options(struct sock *sk, struct sk_buff *skb,
> + unsigned int *size, unsigned int remaining,
> struct mptcp_out_options *opts);
>
> void mptcp_write_options(__be32 *ptr, struct mptcp_out_options *opts);
> @@ -104,7 +105,9 @@ static inline bool mptcp_synack_options(const struct request_sock *req,
> }
>
> static inline bool mptcp_established_options(struct sock *sk,
> + struct sk_buff *skb,
> unsigned int *size,
> + unsigned int remaining,
> struct mptcp_out_options *opts)
> {
> return false;
> diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
> index a4bf1197f577..d123d67026d5 100644
> --- a/net/ipv4/tcp_output.c
> +++ b/net/ipv4/tcp_output.c
> @@ -798,11 +798,10 @@ static unsigned int tcp_established_options(struct sock *sk, struct sk_buff *skb
> unsigned int remaining = MAX_TCP_OPTION_SPACE - size;
> unsigned int opt_size = 0;
>
> - if (mptcp_established_options(sk, &opt_size, &opts->mptcp)) {
> - if (remaining >= opt_size) {
This length check goes away... (see below)
> - opts->options |= OPTION_MPTCP;
> - size += opt_size;
> - }
> + if (mptcp_established_options(sk, skb, &opt_size, remaining,
> + &opts->mptcp)) {
> + opts->options |= OPTION_MPTCP;
> + size += opt_size;
> }
> }
>
> diff --git a/net/mptcp/options.c b/net/mptcp/options.c
> index 26203dfb6be6..89aef0d0beb1 100644
> --- a/net/mptcp/options.c
> +++ b/net/mptcp/options.c
> @@ -196,7 +196,8 @@ void mptcp_rcv_synsent(struct sock *sk)
> }
> }
>
> -bool mptcp_established_options(struct sock *sk, unsigned int *size,
> +bool mptcp_established_options(struct sock *sk, struct sk_buff *skb,
> + unsigned int *size, unsigned int remaining,
> struct mptcp_out_options *opts)
> {
> struct mptcp_subflow_context *subflow = mptcp_subflow_ctx(sk);
...but doesn't get added to mptcp_established_options() until 5 patches
later. At this point the function only handles MP_CAPABLE, so maybe
there's not much risk of overrunning the option space anyway.
> --
> 2.21.0
> _______________________________________________
> mptcp mailing list -- mptcp(a)lists.01.org
> To unsubscribe send an email to mptcp-leave(a)lists.01.org
>
--
Mat Martineau
Intel
^ permalink raw reply [flat|nested] 4+ messages in thread
* [MPTCP] Re: [PATCH] Squash-to: "mptcp: Handle MP_CAPABLE options for outgoing connections"
@ 2019-12-17 15:00 Paolo Abeni
0 siblings, 0 replies; 4+ messages in thread
From: Paolo Abeni @ 2019-12-17 15:00 UTC (permalink / raw)
To: mptcp
[-- Attachment #1: Type: text/plain, Size: 3490 bytes --]
On Mon, 2019-12-16 at 14:05 -0800, Mat Martineau wrote:
> On Mon, 16 Dec 2019, Paolo Abeni wrote:
>
> > mptcp_established_options() definition will be changed in a couple of patches,
> > let's introduce early it's final form
> >
> > Signed-off-by: Paolo Abeni <pabeni(a)redhat.com>
> > ---
> > include/net/mptcp.h | 5 ++++-
> > net/ipv4/tcp_output.c | 9 ++++-----
> > net/mptcp/options.c | 3 ++-
> > 3 files changed, 10 insertions(+), 7 deletions(-)
> >
> > diff --git a/include/net/mptcp.h b/include/net/mptcp.h
> > index d7af03848f5a..588abcb76da3 100644
> > --- a/include/net/mptcp.h
> > +++ b/include/net/mptcp.h
> > @@ -55,7 +55,8 @@ bool mptcp_syn_options(struct sock *sk, unsigned int *size,
> > void mptcp_rcv_synsent(struct sock *sk);
> > bool mptcp_synack_options(const struct request_sock *req, unsigned int *size,
> > struct mptcp_out_options *opts);
> > -bool mptcp_established_options(struct sock *sk, unsigned int *size,
> > +bool mptcp_established_options(struct sock *sk, struct sk_buff *skb,
> > + unsigned int *size, unsigned int remaining,
> > struct mptcp_out_options *opts);
> >
> > void mptcp_write_options(__be32 *ptr, struct mptcp_out_options *opts);
> > @@ -104,7 +105,9 @@ static inline bool mptcp_synack_options(const struct request_sock *req,
> > }
> >
> > static inline bool mptcp_established_options(struct sock *sk,
> > + struct sk_buff *skb,
> > unsigned int *size,
> > + unsigned int remaining,
> > struct mptcp_out_options *opts)
> > {
> > return false;
> > diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
> > index a4bf1197f577..d123d67026d5 100644
> > --- a/net/ipv4/tcp_output.c
> > +++ b/net/ipv4/tcp_output.c
> > @@ -798,11 +798,10 @@ static unsigned int tcp_established_options(struct sock *sk, struct sk_buff *skb
> > unsigned int remaining = MAX_TCP_OPTION_SPACE - size;
> > unsigned int opt_size = 0;
> >
> > - if (mptcp_established_options(sk, &opt_size, &opts->mptcp)) {
> > - if (remaining >= opt_size) {
>
> This length check goes away... (see below)
>
> > - opts->options |= OPTION_MPTCP;
> > - size += opt_size;
> > - }
> > + if (mptcp_established_options(sk, skb, &opt_size, remaining,
> > + &opts->mptcp)) {
> > + opts->options |= OPTION_MPTCP;
> > + size += opt_size;
> > }
> > }
> >
> > diff --git a/net/mptcp/options.c b/net/mptcp/options.c
> > index 26203dfb6be6..89aef0d0beb1 100644
> > --- a/net/mptcp/options.c
> > +++ b/net/mptcp/options.c
> > @@ -196,7 +196,8 @@ void mptcp_rcv_synsent(struct sock *sk)
> > }
> > }
> >
> > -bool mptcp_established_options(struct sock *sk, unsigned int *size,
> > +bool mptcp_established_options(struct sock *sk, struct sk_buff *skb,
> > + unsigned int *size, unsigned int remaining,
> > struct mptcp_out_options *opts)
> > {
> > struct mptcp_subflow_context *subflow = mptcp_subflow_ctx(sk);
>
> ...but doesn't get added to mptcp_established_options() until 5 patches
> later. At this point the function only handles MP_CAPABLE, so maybe
> there's not much risk of overrunning the option space anyway.
Yep, additionally my understanding is that intermediate status for this
kind of patchset is expected to be 'non functional', at least WRT the
to-be-implemented feature.
IIRC, with DSS, MP_CAPABLE or MP_JOIN we can't overrun the option
space.
Cheers,
Paolo
^ permalink raw reply [flat|nested] 4+ messages in thread
* [MPTCP] Re: [PATCH] Squash-to: "mptcp: Handle MP_CAPABLE options for outgoing connections"
@ 2019-12-17 18:42 Matthieu Baerts
0 siblings, 0 replies; 4+ messages in thread
From: Matthieu Baerts @ 2019-12-17 18:42 UTC (permalink / raw)
To: mptcp
[-- Attachment #1: Type: text/plain, Size: 4060 bytes --]
Hi Paolo, Mat, Florian,
On 17/12/2019 16:00, Paolo Abeni wrote:
> On Mon, 2019-12-16 at 14:05 -0800, Mat Martineau wrote:
>> On Mon, 16 Dec 2019, Paolo Abeni wrote:
>>
>>> mptcp_established_options() definition will be changed in a couple of patches,
>>> let's introduce early it's final form
>>>
>>> Signed-off-by: Paolo Abeni <pabeni(a)redhat.com>
>>> ---
>>> include/net/mptcp.h | 5 ++++-
>>> net/ipv4/tcp_output.c | 9 ++++-----
>>> net/mptcp/options.c | 3 ++-
>>> 3 files changed, 10 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/include/net/mptcp.h b/include/net/mptcp.h
>>> index d7af03848f5a..588abcb76da3 100644
>>> --- a/include/net/mptcp.h
>>> +++ b/include/net/mptcp.h
>>> @@ -55,7 +55,8 @@ bool mptcp_syn_options(struct sock *sk, unsigned int *size,
>>> void mptcp_rcv_synsent(struct sock *sk);
>>> bool mptcp_synack_options(const struct request_sock *req, unsigned int *size,
>>> struct mptcp_out_options *opts);
>>> -bool mptcp_established_options(struct sock *sk, unsigned int *size,
>>> +bool mptcp_established_options(struct sock *sk, struct sk_buff *skb,
>>> + unsigned int *size, unsigned int remaining,
>>> struct mptcp_out_options *opts);
>>>
>>> void mptcp_write_options(__be32 *ptr, struct mptcp_out_options *opts);
>>> @@ -104,7 +105,9 @@ static inline bool mptcp_synack_options(const struct request_sock *req,
>>> }
>>>
>>> static inline bool mptcp_established_options(struct sock *sk,
>>> + struct sk_buff *skb,
>>> unsigned int *size,
>>> + unsigned int remaining,
>>> struct mptcp_out_options *opts)
>>> {
>>> return false;
>>> diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
>>> index a4bf1197f577..d123d67026d5 100644
>>> --- a/net/ipv4/tcp_output.c
>>> +++ b/net/ipv4/tcp_output.c
>>> @@ -798,11 +798,10 @@ static unsigned int tcp_established_options(struct sock *sk, struct sk_buff *skb
>>> unsigned int remaining = MAX_TCP_OPTION_SPACE - size;
>>> unsigned int opt_size = 0;
>>>
>>> - if (mptcp_established_options(sk, &opt_size, &opts->mptcp)) {
>>> - if (remaining >= opt_size) {
>>
>> This length check goes away... (see below)
>>
>>> - opts->options |= OPTION_MPTCP;
>>> - size += opt_size;
>>> - }
>>> + if (mptcp_established_options(sk, skb, &opt_size, remaining,
>>> + &opts->mptcp)) {
>>> + opts->options |= OPTION_MPTCP;
>>> + size += opt_size;
>>> }
>>> }
>>>
>>> diff --git a/net/mptcp/options.c b/net/mptcp/options.c
>>> index 26203dfb6be6..89aef0d0beb1 100644
>>> --- a/net/mptcp/options.c
>>> +++ b/net/mptcp/options.c
>>> @@ -196,7 +196,8 @@ void mptcp_rcv_synsent(struct sock *sk)
>>> }
>>> }
>>>
>>> -bool mptcp_established_options(struct sock *sk, unsigned int *size,
>>> +bool mptcp_established_options(struct sock *sk, struct sk_buff *skb,
>>> + unsigned int *size, unsigned int remaining,
>>> struct mptcp_out_options *opts)
>>> {
>>> struct mptcp_subflow_context *subflow = mptcp_subflow_ctx(sk);
>>
>> ...but doesn't get added to mptcp_established_options() until 5 patches
>> later. At this point the function only handles MP_CAPABLE, so maybe
>> there's not much risk of overrunning the option space anyway.
>
> Yep, additionally my understanding is that intermediate status for this
> kind of patchset is expected to be 'non functional', at least WRT the
> to-be-implemented feature.
>
> IIRC, with DSS, MP_CAPABLE or MP_JOIN we can't overrun the option
> space.
(accepted on IRC)
Thank you for the patch and the reviews!
- 5228f011ef2f: "squashed" in "mptcp: Handle MP_CAPABLE options for
outgoing connections"
- 3bbaca7e464b: conflict in
t/mptcp-Write-MPTCP-DSS-headers-to-outgoing-data-packets
- 118ffba5ca21..4acf1c9f40a5: result (empty, as expected, right?)
Cheers,
Matt
--
Matthieu Baerts | R&D Engineer
matthieu.baerts(a)tessares.net
Tessares SA | Hybrid Access Solutions
www.tessares.net
1 Avenue Jean Monnet, 1348 Louvain-la-Neuve, Belgium
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2019-12-17 18:42 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-12-16 21:30 [MPTCP] Re: [PATCH] Squash-to: "mptcp: Handle MP_CAPABLE options for outgoing connections" Florian Westphal
-- strict thread matches above, loose matches on Subject: below --
2019-12-16 22:05 Mat Martineau
2019-12-17 15:00 Paolo Abeni
2019-12-17 18:42 Matthieu Baerts
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.