* Re: [MPTCP] [PATCH RFC 09/10] mptcp: token: fold destroy_token helper
@ 2019-08-27 18:08 Florian Westphal
0 siblings, 0 replies; 4+ messages in thread
From: Florian Westphal @ 2019-08-27 18:08 UTC (permalink / raw)
To: mptcp
[-- Attachment #1: Type: text/plain, Size: 838 bytes --]
Matthieu Baerts <matthieu.baerts(a)tessares.net> wrote:
> > -void token_destroy_request(u32 token)
> > +/**
> > + * mptcp_token_destroy_request - remove mptcp connection/token
> > + * @token - token of mptcp connection to remove
> > + *
> > + * Remove the not-yey-fully-established incoming connection identified
>
> yey? or "yay!!!"? maybe more "yet" I guess :-D
Right. I'll run aspell on the series next time.
> > + * by @token. If the connection isn't found, no action is taken.
>
> Is it to explain why we don't do anything with *cur?
Right, I'll remove cur.
> > spin_lock_bh(&token_tree_lock);
> > - conn = destroy_token(token);
> > + conn = radix_tree_delete(&token_tree, token);
> > if (conn)
>
> Is it no longer needed to check "conn != &token_used" here?
Right, I'll add it back -- thanks!
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [MPTCP] [PATCH RFC 09/10] mptcp: token: fold destroy_token helper
@ 2019-08-28 0:13 Peter Krystad
0 siblings, 0 replies; 4+ messages in thread
From: Peter Krystad @ 2019-08-28 0:13 UTC (permalink / raw)
To: mptcp
[-- Attachment #1: Type: text/plain, Size: 4737 bytes --]
On Sun, 2019-08-25 at 20:59 +0200, Florian Westphal wrote:
> Signed-off-by: Florian Westphal <fw(a)strlen.de>
> ---
> net/mptcp/protocol.c | 2 +-
> net/mptcp/protocol.h | 4 ++--
> net/mptcp/subflow.c | 2 +-
> net/mptcp/token.c | 50 +++++++++++++++++++-------------------------
> 4 files changed, 25 insertions(+), 33 deletions(-)
>
> diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
> index f900303a1531..6a2dc2111316 100644
> --- a/net/mptcp/protocol.c
> +++ b/net/mptcp/protocol.c
> @@ -704,7 +704,7 @@ static void mptcp_destroy(struct sock *sk)
>
> pr_debug("msk=%p, subflow=%p", sk, msk->subflow->sk);
>
> - token_destroy(msk->token);
> + mptcp_token_destroy(msk->token);
> }
>
> /* Only pass a small subset of level/optnames that are considered safe.
> diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
> index a1e207db6703..94b06ec168ff 100644
> --- a/net/mptcp/protocol.h
> +++ b/net/mptcp/protocol.h
> @@ -230,13 +230,13 @@ bool mptcp_token_join_request(struct request_sock *req,
> const struct sk_buff *skb);
> bool mptcp_token_join_valid(const struct request_sock *req,
> const struct tcp_options_received *rx_opt);
> -void token_destroy_request(u32 token);
> +void mptcp_token_destroy_request(u32 token);
> int mptcp_token_new_connect(struct sock *sk);
> int mptcp_token_new_accept(u32 token);
> void mptcp_token_update_accept(struct sock *sk, struct sock *conn);
> struct mptcp_sock *mptcp_token_get_sock(u32 token);
> void token_release(u32 token);
> -void token_destroy(u32 token);
> +void mptcp_token_destroy(u32 token);
>
> void crypto_key_sha1(u64 key, u32 *token, u64 *idsn);
> static inline void crypto_key_gen_sha1(u64 *key, u32 *token, u64 *idsn)
> diff --git a/net/mptcp/subflow.c b/net/mptcp/subflow.c
> index 85fb7a52e274..1cfd18e74486 100644
> --- a/net/mptcp/subflow.c
> +++ b/net/mptcp/subflow.c
> @@ -45,7 +45,7 @@ static void subflow_req_destructor(struct request_sock *req)
> pr_debug("subflow_req=%p", subflow_req);
>
> if (subflow_req->mp_capable)
> - token_destroy_request(subflow_req->token);
> + mptcp_token_destroy_request(subflow_req->token);
> tcp_request_sock_ops.destructor(req);
> }
>
> diff --git a/net/mptcp/token.c b/net/mptcp/token.c
> index a0779fd0ae7c..3c1f43542e1e 100644
> --- a/net/mptcp/token.c
> +++ b/net/mptcp/token.c
> @@ -39,31 +39,7 @@ static int token_used __read_mostly;
>
> static struct sock *__token_lookup(u32 token)
> {
> - void *conn;
> -
> - pr_debug("token=%u", token);
> - conn = radix_tree_lookup(&token_tree, token);
> - return (struct sock *)conn;
> -}
> -
> -static void destroy_req_token(u32 token)
> -{
> - void *cur;
> -
> - cur = radix_tree_delete(&token_req_tree, token);
> - if (!cur)
> - pr_warn("token NOT FOUND!");
> -}
> -
> -static struct sock *destroy_token(u32 token)
> -{
> - void *conn;
> -
> - pr_debug("token=%u", token);
> - conn = radix_tree_delete(&token_tree, token);
> - if (conn && conn != &token_used)
> - return (struct sock *)conn;
> - return NULL;
> + return radix_tree_lookup(&token_tree, token);
> }
>
> /**
> @@ -321,12 +297,21 @@ struct mptcp_sock *mptcp_token_get_sock(u32 token)
> return mptcp_sk(conn);
> }
>
> -void token_destroy_request(u32 token)
> +/**
> + * mptcp_token_destroy_request - remove mptcp connection/token
> + * @token - token of mptcp connection to remove
> + *
> + * Remove the not-yey-fully-established incoming connection identified
> + * by @token. If the connection isn't found, no action is taken.
> + */
I would reword these comments to refer to a "connection request", there isn't
a connection to remove.
Peter.
> +void mptcp_token_destroy_request(u32 token)
> {
> + const void *cur;
> +
> pr_debug("token=%u", token);
>
> spin_lock_bh(&token_tree_lock);
> - destroy_req_token(token);
> + cur = radix_tree_delete(&token_req_tree, token);
> spin_unlock_bh(&token_tree_lock);
> }
>
> @@ -342,13 +327,20 @@ void token_release(u32 token)
> spin_unlock_bh(&token_tree_lock);
> }
>
> -void token_destroy(u32 token)
> +/**
> + * mptcp_token_destroy - remove mptcp connection/token
> + * @token - token of mptcp connection to remove
> + *
> + * Remove the connection identified by @token.
> + * If the connection isn't found, no action is taken.
> + */
> +void mptcp_token_destroy(u32 token)
> {
> struct sock *conn;
>
> pr_debug("token=%u", token);
> spin_lock_bh(&token_tree_lock);
> - conn = destroy_token(token);
> + conn = radix_tree_delete(&token_tree, token);
> if (conn)
> sock_put(conn);
> spin_unlock_bh(&token_tree_lock);
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [MPTCP] [PATCH RFC 09/10] mptcp: token: fold destroy_token helper
@ 2019-08-27 17:00 Matthieu Baerts
0 siblings, 0 replies; 4+ messages in thread
From: Matthieu Baerts @ 2019-08-27 17:00 UTC (permalink / raw)
To: mptcp
[-- Attachment #1: Type: text/plain, Size: 3082 bytes --]
Hi Florian,
On 25/08/2019 20:59, Florian Westphal wrote:
> Signed-off-by: Florian Westphal <fw(a)strlen.de>
> ---
> net/mptcp/protocol.c | 2 +-
> net/mptcp/protocol.h | 4 ++--
> net/mptcp/subflow.c | 2 +-
> net/mptcp/token.c | 50 +++++++++++++++++++-------------------------
> 4 files changed, 25 insertions(+), 33 deletions(-)
>
(...)
> diff --git a/net/mptcp/token.c b/net/mptcp/token.c
> index a0779fd0ae7c..3c1f43542e1e 100644
> --- a/net/mptcp/token.c
> +++ b/net/mptcp/token.c
> @@ -39,31 +39,7 @@ static int token_used __read_mostly;
>
> static struct sock *__token_lookup(u32 token)
> {
> - void *conn;
> -
> - pr_debug("token=%u", token);
> - conn = radix_tree_lookup(&token_tree, token);
> - return (struct sock *)conn;
> -}
> -
> -static void destroy_req_token(u32 token)
> -{
> - void *cur;
> -
> - cur = radix_tree_delete(&token_req_tree, token);
> - if (!cur)
> - pr_warn("token NOT FOUND!");
> -}
> -
> -static struct sock *destroy_token(u32 token)
> -{
> - void *conn;
> -
> - pr_debug("token=%u", token);
> - conn = radix_tree_delete(&token_tree, token);
> - if (conn && conn != &token_used)
> - return (struct sock *)conn;
> - return NULL;
> + return radix_tree_lookup(&token_tree, token);
> }
>
> /**
> @@ -321,12 +297,21 @@ struct mptcp_sock *mptcp_token_get_sock(u32 token)
> return mptcp_sk(conn);
> }
>
> -void token_destroy_request(u32 token)
> +/**
> + * mptcp_token_destroy_request - remove mptcp connection/token
> + * @token - token of mptcp connection to remove
> + *
> + * Remove the not-yey-fully-established incoming connection identified
yey? or "yay!!!"? maybe more "yet" I guess :-D
> + * by @token. If the connection isn't found, no action is taken.
Is it to explain why we don't do anything with *cur?
> + */
> +void mptcp_token_destroy_request(u32 token)
> {
> + const void *cur;
> +
> pr_debug("token=%u", token);
>
> spin_lock_bh(&token_tree_lock);
> - destroy_req_token(token);
> + cur = radix_tree_delete(&token_req_tree, token);
> spin_unlock_bh(&token_tree_lock);
> }
>
> @@ -342,13 +327,20 @@ void token_release(u32 token)
> spin_unlock_bh(&token_tree_lock);
> }
>
> -void token_destroy(u32 token)
> +/**
> + * mptcp_token_destroy - remove mptcp connection/token
> + * @token - token of mptcp connection to remove
> + *
> + * Remove the connection identified by @token.
> + * If the connection isn't found, no action is taken.
> + */
> +void mptcp_token_destroy(u32 token)
> {
> struct sock *conn;
>
> pr_debug("token=%u", token);
> spin_lock_bh(&token_tree_lock);
> - conn = destroy_token(token);
> + conn = radix_tree_delete(&token_tree, token);
> if (conn)
Is it no longer needed to check "conn != &token_used" here?
> sock_put(conn);
> spin_unlock_bh(&token_tree_lock);
>
--
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* [MPTCP] [PATCH RFC 09/10] mptcp: token: fold destroy_token helper
@ 2019-08-25 18:59 Florian Westphal
0 siblings, 0 replies; 4+ messages in thread
From: Florian Westphal @ 2019-08-25 18:59 UTC (permalink / raw)
To: mptcp
[-- Attachment #1: Type: text/plain, Size: 4294 bytes --]
Signed-off-by: Florian Westphal <fw(a)strlen.de>
---
net/mptcp/protocol.c | 2 +-
net/mptcp/protocol.h | 4 ++--
net/mptcp/subflow.c | 2 +-
net/mptcp/token.c | 50 +++++++++++++++++++-------------------------
4 files changed, 25 insertions(+), 33 deletions(-)
diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index f900303a1531..6a2dc2111316 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -704,7 +704,7 @@ static void mptcp_destroy(struct sock *sk)
pr_debug("msk=%p, subflow=%p", sk, msk->subflow->sk);
- token_destroy(msk->token);
+ mptcp_token_destroy(msk->token);
}
/* Only pass a small subset of level/optnames that are considered safe.
diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
index a1e207db6703..94b06ec168ff 100644
--- a/net/mptcp/protocol.h
+++ b/net/mptcp/protocol.h
@@ -230,13 +230,13 @@ bool mptcp_token_join_request(struct request_sock *req,
const struct sk_buff *skb);
bool mptcp_token_join_valid(const struct request_sock *req,
const struct tcp_options_received *rx_opt);
-void token_destroy_request(u32 token);
+void mptcp_token_destroy_request(u32 token);
int mptcp_token_new_connect(struct sock *sk);
int mptcp_token_new_accept(u32 token);
void mptcp_token_update_accept(struct sock *sk, struct sock *conn);
struct mptcp_sock *mptcp_token_get_sock(u32 token);
void token_release(u32 token);
-void token_destroy(u32 token);
+void mptcp_token_destroy(u32 token);
void crypto_key_sha1(u64 key, u32 *token, u64 *idsn);
static inline void crypto_key_gen_sha1(u64 *key, u32 *token, u64 *idsn)
diff --git a/net/mptcp/subflow.c b/net/mptcp/subflow.c
index 85fb7a52e274..1cfd18e74486 100644
--- a/net/mptcp/subflow.c
+++ b/net/mptcp/subflow.c
@@ -45,7 +45,7 @@ static void subflow_req_destructor(struct request_sock *req)
pr_debug("subflow_req=%p", subflow_req);
if (subflow_req->mp_capable)
- token_destroy_request(subflow_req->token);
+ mptcp_token_destroy_request(subflow_req->token);
tcp_request_sock_ops.destructor(req);
}
diff --git a/net/mptcp/token.c b/net/mptcp/token.c
index a0779fd0ae7c..3c1f43542e1e 100644
--- a/net/mptcp/token.c
+++ b/net/mptcp/token.c
@@ -39,31 +39,7 @@ static int token_used __read_mostly;
static struct sock *__token_lookup(u32 token)
{
- void *conn;
-
- pr_debug("token=%u", token);
- conn = radix_tree_lookup(&token_tree, token);
- return (struct sock *)conn;
-}
-
-static void destroy_req_token(u32 token)
-{
- void *cur;
-
- cur = radix_tree_delete(&token_req_tree, token);
- if (!cur)
- pr_warn("token NOT FOUND!");
-}
-
-static struct sock *destroy_token(u32 token)
-{
- void *conn;
-
- pr_debug("token=%u", token);
- conn = radix_tree_delete(&token_tree, token);
- if (conn && conn != &token_used)
- return (struct sock *)conn;
- return NULL;
+ return radix_tree_lookup(&token_tree, token);
}
/**
@@ -321,12 +297,21 @@ struct mptcp_sock *mptcp_token_get_sock(u32 token)
return mptcp_sk(conn);
}
-void token_destroy_request(u32 token)
+/**
+ * mptcp_token_destroy_request - remove mptcp connection/token
+ * @token - token of mptcp connection to remove
+ *
+ * Remove the not-yey-fully-established incoming connection identified
+ * by @token. If the connection isn't found, no action is taken.
+ */
+void mptcp_token_destroy_request(u32 token)
{
+ const void *cur;
+
pr_debug("token=%u", token);
spin_lock_bh(&token_tree_lock);
- destroy_req_token(token);
+ cur = radix_tree_delete(&token_req_tree, token);
spin_unlock_bh(&token_tree_lock);
}
@@ -342,13 +327,20 @@ void token_release(u32 token)
spin_unlock_bh(&token_tree_lock);
}
-void token_destroy(u32 token)
+/**
+ * mptcp_token_destroy - remove mptcp connection/token
+ * @token - token of mptcp connection to remove
+ *
+ * Remove the connection identified by @token.
+ * If the connection isn't found, no action is taken.
+ */
+void mptcp_token_destroy(u32 token)
{
struct sock *conn;
pr_debug("token=%u", token);
spin_lock_bh(&token_tree_lock);
- conn = destroy_token(token);
+ conn = radix_tree_delete(&token_tree, token);
if (conn)
sock_put(conn);
spin_unlock_bh(&token_tree_lock);
--
2.21.0
^ permalink raw reply related [flat|nested] 4+ messages in thread
end of thread, other threads:[~2019-08-28 0:13 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-08-27 18:08 [MPTCP] [PATCH RFC 09/10] mptcp: token: fold destroy_token helper Florian Westphal
-- strict thread matches above, loose matches on Subject: below --
2019-08-28 0:13 Peter Krystad
2019-08-27 17:00 Matthieu Baerts
2019-08-25 18:59 Florian Westphal
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.