* [MPTCP] Re: [PATCH RFC 07/11] mptcp: subflow: add mptcp_subflow_init_cookie_req helper
@ 2020-07-14 19:32 Christoph Paasch
0 siblings, 0 replies; 2+ messages in thread
From: Christoph Paasch @ 2020-07-14 19:32 UTC (permalink / raw)
To: mptcp
[-- Attachment #1: Type: text/plain, Size: 4500 bytes --]
On 07/14/20 - 20:25, Florian Westphal wrote:
> Will be used to initialize the mptcp request socket when a
> MP_CAPABLE request was handled in syncookie mode, i.e. when a
> TCP ACK containing a MP_CAPABLE option is a valid syncookie value.
>
> Signed-off-by: Florian Westphal <fw(a)strlen.de>
> ---
> include/linux/tcp.h | 1 +
> include/net/mptcp.h | 10 +++++++++
> net/mptcp/subflow.c | 53 +++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 64 insertions(+)
>
> diff --git a/include/linux/tcp.h b/include/linux/tcp.h
> index 3bdec31ce8f4..1f706157b551 100644
> --- a/include/linux/tcp.h
> +++ b/include/linux/tcp.h
> @@ -118,6 +118,7 @@ struct tcp_request_sock {
> struct inet_request_sock req;
> const struct tcp_request_sock_ops *af_specific;
> u64 snt_synack; /* first SYNACK sent time */
> + u8 syncookie:1;
As this field will only be used in init_req, would it make sense to rather
pass want_cookie from tcp_conn_request as an argument to the callback?
Christoph
> bool tfo_listener;
> bool is_mptcp;
> #if IS_ENABLED(CONFIG_MPTCP)
> diff --git a/include/net/mptcp.h b/include/net/mptcp.h
> index 985344512f31..653acfe58d04 100644
> --- a/include/net/mptcp.h
> +++ b/include/net/mptcp.h
> @@ -131,6 +131,9 @@ static inline bool mptcp_skb_can_collapse(const struct sk_buff *to,
> }
>
> void mptcp_seq_show(struct seq_file *seq);
> +int mptcp_subflow_init_cookie_req(struct request_sock *req,
> + const struct sock *sk_listener,
> + struct sk_buff *skb);
> #else
>
> static inline void mptcp_init(void)
> @@ -204,6 +207,13 @@ static inline bool mptcp_skb_can_collapse(const struct sk_buff *to,
>
> static inline void mptcp_space(const struct sock *ssk, int *s, int *fs) { }
> static inline void mptcp_seq_show(struct seq_file *seq) { }
> +
> +static inline int mptcp_subflow_init_cookie_req(struct request_sock *req,
> + const struct sock *sk_listener,
> + struct sk_buff *skb)
> +{
> + return 0; /* TCP fallback */
> +}
> #endif /* CONFIG_MPTCP */
>
> #if IS_ENABLED(CONFIG_MPTCP_IPV6)
> diff --git a/net/mptcp/subflow.c b/net/mptcp/subflow.c
> index cc9eaec14037..943fa650f1e9 100644
> --- a/net/mptcp/subflow.c
> +++ b/net/mptcp/subflow.c
> @@ -154,6 +154,20 @@ static void subflow_init_req(struct request_sock *req,
> get_random_bytes(&subflow_req->local_key, sizeof(subflow_req->local_key));
> } while (subflow_req->local_key == 0);
>
> + if (unlikely(tcp_rsk(req)->syncookie)) {
> + mptcp_crypto_key_sha(subflow_req->local_key,
> + &subflow_req->token,
> + &subflow_req->idsn);
> + if (mptcp_token_get_sock(subflow_req->token)) {
> + if (retries-- > 0)
> + goto again;
> + } else {
> + subflow_req->mp_capable = 1;
> + }
> + subflow_req->ssn_offset = TCP_SKB_CB(skb)->seq;
> + return;
> + }
> +
> err = mptcp_token_new_request(req);
> if (err == 0)
> subflow_req->mp_capable = 1;
> @@ -174,6 +188,45 @@ static void subflow_init_req(struct request_sock *req,
> }
> }
>
> +int mptcp_subflow_init_cookie_req(struct request_sock *req,
> + const struct sock *sk_listener,
> + struct sk_buff *skb)
> +{
> + struct mptcp_subflow_context *listener = mptcp_subflow_ctx(sk_listener);
> + struct mptcp_subflow_request_sock *subflow_req = mptcp_subflow_rsk(req);
> + struct mptcp_options_received mp_opt;
> + int ret;
> +
> + ret = __subflow_init_req(req, sk_listener);
> + if (ret)
> + return ret;
> +
> + mptcp_get_options(skb, &mp_opt);
> + ret = __subflow_check_options(&mp_opt, req);
> + if (ret)
> + return ret;
> +
> + if (mp_opt.mp_capable && listener->request_mptcp) {
> + int err;
> +
> + if (mp_opt.sndr_key == 0)
> + return -EINVAL;
> +
> + subflow_req->local_key = mp_opt.rcvr_key;
> + err = mptcp_token_new_request(req);
> + if (err)
> + return err;
> +
> + subflow_req->mp_capable = 1;
> + subflow_req->ssn_offset = TCP_SKB_CB(skb)->seq - 1;
> + }
> + /* JOIN cannot be handled via cookies due to need
> + * to store the remote peer nonce value.
> + */
> +
> + return 0;
> +}
> +
> static void subflow_v4_init_req(struct request_sock *req,
> const struct sock *sk_listener,
> struct sk_buff *skb)
> --
> 2.26.2
> _______________________________________________
> mptcp mailing list -- mptcp(a)lists.01.org
> To unsubscribe send an email to mptcp-leave(a)lists.01.org
^ permalink raw reply [flat|nested] 2+ messages in thread
* [MPTCP] Re: [PATCH RFC 07/11] mptcp: subflow: add mptcp_subflow_init_cookie_req helper
@ 2020-07-15 13:13 Paolo Abeni
0 siblings, 0 replies; 2+ messages in thread
From: Paolo Abeni @ 2020-07-15 13:13 UTC (permalink / raw)
To: mptcp
[-- Attachment #1: Type: text/plain, Size: 3327 bytes --]
On Tue, 2020-07-14 at 12:32 -0700, Christoph Paasch wrote:
> On 07/14/20 - 20:25, Florian Westphal wrote:
> > Will be used to initialize the mptcp request socket when a
> > MP_CAPABLE request was handled in syncookie mode, i.e. when a
> > TCP ACK containing a MP_CAPABLE option is a valid syncookie value.
> >
> > Signed-off-by: Florian Westphal <fw(a)strlen.de>
> > ---
> > include/linux/tcp.h | 1 +
> > include/net/mptcp.h | 10 +++++++++
> > net/mptcp/subflow.c | 53 +++++++++++++++++++++++++++++++++++++++++++++
> > 3 files changed, 64 insertions(+)
> >
> > diff --git a/include/linux/tcp.h b/include/linux/tcp.h
> > index 3bdec31ce8f4..1f706157b551 100644
> > --- a/include/linux/tcp.h
> > +++ b/include/linux/tcp.h
> > @@ -118,6 +118,7 @@ struct tcp_request_sock {
> > struct inet_request_sock req;
> > const struct tcp_request_sock_ops *af_specific;
> > u64 snt_synack; /* first SYNACK sent time */
> > + u8 syncookie:1;
>
> As this field will only be used in init_req, would it make sense to rather
> pass want_cookie from tcp_conn_request as an argument to the callback?
Otherwise, why don't a 'bool' instead?
I think it would be nice nothing in the commmit message that this new
field fill a pre-existing one byte hole.
>
>
> Christoph
>
> > bool tfo_listener;
> > bool is_mptcp;
> > #if IS_ENABLED(CONFIG_MPTCP)
> > diff --git a/include/net/mptcp.h b/include/net/mptcp.h
> > index 985344512f31..653acfe58d04 100644
> > --- a/include/net/mptcp.h
> > +++ b/include/net/mptcp.h
> > @@ -131,6 +131,9 @@ static inline bool mptcp_skb_can_collapse(const struct sk_buff *to,
> > }
> >
> > void mptcp_seq_show(struct seq_file *seq);
> > +int mptcp_subflow_init_cookie_req(struct request_sock *req,
> > + const struct sock *sk_listener,
> > + struct sk_buff *skb);
> > #else
> >
> > static inline void mptcp_init(void)
> > @@ -204,6 +207,13 @@ static inline bool mptcp_skb_can_collapse(const struct sk_buff *to,
> >
> > static inline void mptcp_space(const struct sock *ssk, int *s, int *fs) { }
> > static inline void mptcp_seq_show(struct seq_file *seq) { }
> > +
> > +static inline int mptcp_subflow_init_cookie_req(struct request_sock *req,
> > + const struct sock *sk_listener,
> > + struct sk_buff *skb)
> > +{
> > + return 0; /* TCP fallback */
> > +}
> > #endif /* CONFIG_MPTCP */
> >
> > #if IS_ENABLED(CONFIG_MPTCP_IPV6)
> > diff --git a/net/mptcp/subflow.c b/net/mptcp/subflow.c
> > index cc9eaec14037..943fa650f1e9 100644
> > --- a/net/mptcp/subflow.c
> > +++ b/net/mptcp/subflow.c
> > @@ -154,6 +154,20 @@ static void subflow_init_req(struct request_sock *req,
> > get_random_bytes(&subflow_req->local_key, sizeof(subflow_req->local_key));
> > } while (subflow_req->local_key == 0);
> >
> > + if (unlikely(tcp_rsk(req)->syncookie)) {
> > + mptcp_crypto_key_sha(subflow_req->local_key,
> > + &subflow_req->token,
> > + &subflow_req->idsn);
> > + if (mptcp_token_get_sock(subflow_req->token)) {
Don't we need to sock_put() the returned sock here?
Perhpas we can use a new, different helper that just check for
existance, without touching the sock refcount? Looks like the sock
reference is not used below.
Thanks!
Paolo
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2020-07-15 13:13 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-07-14 19:32 [MPTCP] Re: [PATCH RFC 07/11] mptcp: subflow: add mptcp_subflow_init_cookie_req helper Christoph Paasch
-- strict thread matches above, loose matches on Subject: below --
2020-07-15 13:13 Paolo Abeni
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.