mptcp.lists.linux.dev archive mirror
 help / color / mirror / Atom feed
* [MPTCP][PATCH mptcp-next 0/2] mptcp_for_each_* cleanups
@ 2021-04-06 10:03 Geliang Tang
  2021-04-06 10:03 ` [MPTCP][PATCH mptcp-next 1/2] mptcp: add mptcp_for_each_msk helper Geliang Tang
  0 siblings, 1 reply; 8+ messages in thread
From: Geliang Tang @ 2021-04-06 10:03 UTC (permalink / raw)
  To: mptcp; +Cc: Geliang Tang

Two small cleanups using the macro helpers mptcp_for_each_*.

Geliang Tang (2):
  mptcp: add mptcp_for_each_msk helper
  mptcp: use mptcp_for_each_subflow in mptcp_close

 net/mptcp/pm_netlink.c | 39 +++++++++++++++++----------------------
 net/mptcp/protocol.c   |  2 +-
 2 files changed, 18 insertions(+), 23 deletions(-)

-- 
2.30.2


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [MPTCP][PATCH mptcp-next 1/2] mptcp: add mptcp_for_each_msk helper
  2021-04-06 10:03 [MPTCP][PATCH mptcp-next 0/2] mptcp_for_each_* cleanups Geliang Tang
@ 2021-04-06 10:03 ` Geliang Tang
  2021-04-06 10:03   ` [MPTCP][PATCH mptcp-next 2/2] mptcp: use mptcp_for_each_subflow in mptcp_close Geliang Tang
  2021-04-06 23:44   ` [MPTCP][PATCH mptcp-next 1/2] mptcp: add mptcp_for_each_msk helper Mat Martineau
  0 siblings, 2 replies; 8+ messages in thread
From: Geliang Tang @ 2021-04-06 10:03 UTC (permalink / raw)
  To: mptcp; +Cc: Geliang Tang

This patch added a new macro helper mptcp_for_each_msk to traverse each
existing msk socket in the net namespace.

Signed-off-by: Geliang Tang <geliangtang@gmail.com>
---
 net/mptcp/pm_netlink.c | 39 +++++++++++++++++----------------------
 1 file changed, 17 insertions(+), 22 deletions(-)

diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c
index 6ba040897738..b9044defd89b 100644
--- a/net/mptcp/pm_netlink.c
+++ b/net/mptcp/pm_netlink.c
@@ -58,6 +58,10 @@ struct pm_nl_pernet {
 #define MPTCP_PM_ADDR_MAX	8
 #define ADD_ADDR_RETRANS_MAX	3
 
+#define mptcp_for_each_msk(__net, __msk)				\
+	long __slot = 0, __num = 0;					\
+	while ((__msk = mptcp_token_iter_next(__net, &(__slot), &(__num))) != NULL)
+
 static bool addresses_equal(const struct mptcp_addr_info *a,
 			    struct mptcp_addr_info *b, bool use_port)
 {
@@ -985,9 +989,8 @@ static struct pm_nl_pernet *genl_info_pm_nl(struct genl_info *info)
 static int mptcp_nl_add_subflow_or_signal_addr(struct net *net)
 {
 	struct mptcp_sock *msk;
-	long s_slot = 0, s_num = 0;
 
-	while ((msk = mptcp_token_iter_next(net, &s_slot, &s_num)) != NULL) {
+	mptcp_for_each_msk(net, msk) {
 		struct sock *sk = (struct sock *)msk;
 
 		if (!READ_ONCE(msk->fully_established))
@@ -1096,14 +1099,9 @@ static int mptcp_nl_remove_subflow_and_signal_addr(struct net *net,
 						   struct mptcp_addr_info *addr)
 {
 	struct mptcp_sock *msk;
-	long s_slot = 0, s_num = 0;
-	struct mptcp_rm_list list = { .nr = 0 };
-
-	pr_debug("remove_id=%d", addr->id);
 
-	list.ids[list.nr++] = addr->id;
-
-	while ((msk = mptcp_token_iter_next(net, &s_slot, &s_num)) != NULL) {
+	mptcp_for_each_msk(net, msk) {
+		struct mptcp_rm_list list = { .nr = 0 };
 		struct sock *sk = (struct sock *)msk;
 		bool remove_subflow;
 
@@ -1112,6 +1110,8 @@ static int mptcp_nl_remove_subflow_and_signal_addr(struct net *net,
 			goto next;
 		}
 
+		list.ids[list.nr++] = addr->id;
+
 		lock_sock(sk);
 		remove_subflow = lookup_subflow_by_saddr(&msk->conn_list, addr);
 		mptcp_pm_remove_anno_addr(msk, addr, remove_subflow);
@@ -1162,13 +1162,10 @@ static void mptcp_pm_free_addr_entry(struct mptcp_pm_addr_entry *entry)
 static int mptcp_nl_remove_id_zero_address(struct net *net,
 					   struct mptcp_addr_info *addr)
 {
-	struct mptcp_rm_list list = { .nr = 0 };
-	long s_slot = 0, s_num = 0;
 	struct mptcp_sock *msk;
 
-	list.ids[list.nr++] = 0;
-
-	while ((msk = mptcp_token_iter_next(net, &s_slot, &s_num)) != NULL) {
+	mptcp_for_each_msk(net, msk) {
+		struct mptcp_rm_list list = { .nr = 0 };
 		struct sock *sk = (struct sock *)msk;
 		struct mptcp_addr_info msk_local;
 
@@ -1179,6 +1176,8 @@ static int mptcp_nl_remove_id_zero_address(struct net *net,
 		if (!addresses_equal(&msk_local, addr, addr->port))
 			goto next;
 
+		list.ids[list.nr++] = 0;
+
 		lock_sock(sk);
 		spin_lock_bh(&msk->pm.lock);
 		mptcp_pm_remove_addr(msk, &list);
@@ -1271,13 +1270,9 @@ static void mptcp_pm_remove_addrs_and_subflows(struct mptcp_sock *msk,
 static void mptcp_nl_remove_addrs_list(struct net *net,
 				       struct list_head *rm_list)
 {
-	long s_slot = 0, s_num = 0;
 	struct mptcp_sock *msk;
 
-	if (list_empty(rm_list))
-		return;
-
-	while ((msk = mptcp_token_iter_next(net, &s_slot, &s_num)) != NULL) {
+	mptcp_for_each_msk(net, msk) {
 		struct sock *sk = (struct sock *)msk;
 
 		lock_sock(sk);
@@ -1320,7 +1315,8 @@ static int mptcp_nl_cmd_flush_addrs(struct sk_buff *skb, struct genl_info *info)
 	pernet->next_id = 1;
 	bitmap_zero(pernet->id_bitmap, MAX_ADDR_ID + 1);
 	spin_unlock_bh(&pernet->lock);
-	mptcp_nl_remove_addrs_list(sock_net(skb->sk), &free_list);
+	if (!list_empty(&free_list))
+		mptcp_nl_remove_addrs_list(sock_net(skb->sk), &free_list);
 	__flush_addrs(&free_list);
 	return 0;
 }
@@ -1535,11 +1531,10 @@ static int mptcp_nl_addr_backup(struct net *net,
 				struct mptcp_addr_info *addr,
 				u8 bkup)
 {
-	long s_slot = 0, s_num = 0;
 	struct mptcp_sock *msk;
 	int ret = -EINVAL;
 
-	while ((msk = mptcp_token_iter_next(net, &s_slot, &s_num)) != NULL) {
+	mptcp_for_each_msk(net, msk) {
 		struct sock *sk = (struct sock *)msk;
 
 		if (list_empty(&msk->conn_list))
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [MPTCP][PATCH mptcp-next 2/2] mptcp: use mptcp_for_each_subflow in mptcp_close
  2021-04-06 10:03 ` [MPTCP][PATCH mptcp-next 1/2] mptcp: add mptcp_for_each_msk helper Geliang Tang
@ 2021-04-06 10:03   ` Geliang Tang
  2021-04-08 15:48     ` Geliang Tang
  2021-04-08 21:14     ` Mat Martineau
  2021-04-06 23:44   ` [MPTCP][PATCH mptcp-next 1/2] mptcp: add mptcp_for_each_msk helper Mat Martineau
  1 sibling, 2 replies; 8+ messages in thread
From: Geliang Tang @ 2021-04-06 10:03 UTC (permalink / raw)
  To: mptcp; +Cc: Geliang Tang

This patch used the macro helper mptcp_for_each_subflow() instead of
list_for_each_entry() in mptcp_close.

Signed-off-by: Geliang Tang <geliangtang@gmail.com>
---
 net/mptcp/protocol.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index 2d895c3c8746..3b7d1df8d019 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -2581,7 +2581,7 @@ static void mptcp_close(struct sock *sk, long timeout)
 cleanup:
 	/* orphan all the subflows */
 	inet_csk(sk)->icsk_mtup.probe_timestamp = tcp_jiffies32;
-	list_for_each_entry(subflow, &mptcp_sk(sk)->conn_list, node) {
+	mptcp_for_each_subflow(mptcp_sk(sk), subflow) {
 		struct sock *ssk = mptcp_subflow_tcp_sock(subflow);
 		bool slow = lock_sock_fast(ssk);
 
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [MPTCP][PATCH mptcp-next 1/2] mptcp: add mptcp_for_each_msk helper
  2021-04-06 10:03 ` [MPTCP][PATCH mptcp-next 1/2] mptcp: add mptcp_for_each_msk helper Geliang Tang
  2021-04-06 10:03   ` [MPTCP][PATCH mptcp-next 2/2] mptcp: use mptcp_for_each_subflow in mptcp_close Geliang Tang
@ 2021-04-06 23:44   ` Mat Martineau
  2021-04-07  7:55     ` Geliang Tang
  1 sibling, 1 reply; 8+ messages in thread
From: Mat Martineau @ 2021-04-06 23:44 UTC (permalink / raw)
  To: Geliang Tang; +Cc: mptcp


On Tue, 6 Apr 2021, Geliang Tang wrote:

> This patch added a new macro helper mptcp_for_each_msk to traverse each
> existing msk socket in the net namespace.
>
> Signed-off-by: Geliang Tang <geliangtang@gmail.com>
> ---
> net/mptcp/pm_netlink.c | 39 +++++++++++++++++----------------------
> 1 file changed, 17 insertions(+), 22 deletions(-)
>
> diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c
> index 6ba040897738..b9044defd89b 100644
> --- a/net/mptcp/pm_netlink.c
> +++ b/net/mptcp/pm_netlink.c
> @@ -58,6 +58,10 @@ struct pm_nl_pernet {
> #define MPTCP_PM_ADDR_MAX	8
> #define ADD_ADDR_RETRANS_MAX	3
>
> +#define mptcp_for_each_msk(__net, __msk)				\
> +	long __slot = 0, __num = 0;					\
> +	while ((__msk = mptcp_token_iter_next(__net, &(__slot), &(__num))) != NULL)

Hi Geliang,

I don't think it's good to move the variable declarations inside the macro 
like this. It looks like the compiler or sparse might have complained when 
the macro was used after a non-declaration line of code, since some other 
code (like list.ids[] initialization) was moved around. The macro 
shouldn't depend on careful placement in a code block this way - in my 
view, it's better to live with the existing syntax.


Mat


> +
> static bool addresses_equal(const struct mptcp_addr_info *a,
> 			    struct mptcp_addr_info *b, bool use_port)
> {
> @@ -985,9 +989,8 @@ static struct pm_nl_pernet *genl_info_pm_nl(struct genl_info *info)
> static int mptcp_nl_add_subflow_or_signal_addr(struct net *net)
> {
> 	struct mptcp_sock *msk;
> -	long s_slot = 0, s_num = 0;
>
> -	while ((msk = mptcp_token_iter_next(net, &s_slot, &s_num)) != NULL) {
> +	mptcp_for_each_msk(net, msk) {
> 		struct sock *sk = (struct sock *)msk;
>
> 		if (!READ_ONCE(msk->fully_established))
> @@ -1096,14 +1099,9 @@ static int mptcp_nl_remove_subflow_and_signal_addr(struct net *net,
> 						   struct mptcp_addr_info *addr)
> {
> 	struct mptcp_sock *msk;
> -	long s_slot = 0, s_num = 0;
> -	struct mptcp_rm_list list = { .nr = 0 };
> -
> -	pr_debug("remove_id=%d", addr->id);
>
> -	list.ids[list.nr++] = addr->id;
> -
> -	while ((msk = mptcp_token_iter_next(net, &s_slot, &s_num)) != NULL) {
> +	mptcp_for_each_msk(net, msk) {
> +		struct mptcp_rm_list list = { .nr = 0 };
> 		struct sock *sk = (struct sock *)msk;
> 		bool remove_subflow;
>
> @@ -1112,6 +1110,8 @@ static int mptcp_nl_remove_subflow_and_signal_addr(struct net *net,
> 			goto next;
> 		}
>
> +		list.ids[list.nr++] = addr->id;
> +
> 		lock_sock(sk);
> 		remove_subflow = lookup_subflow_by_saddr(&msk->conn_list, addr);
> 		mptcp_pm_remove_anno_addr(msk, addr, remove_subflow);
> @@ -1162,13 +1162,10 @@ static void mptcp_pm_free_addr_entry(struct mptcp_pm_addr_entry *entry)
> static int mptcp_nl_remove_id_zero_address(struct net *net,
> 					   struct mptcp_addr_info *addr)
> {
> -	struct mptcp_rm_list list = { .nr = 0 };
> -	long s_slot = 0, s_num = 0;
> 	struct mptcp_sock *msk;
>
> -	list.ids[list.nr++] = 0;
> -
> -	while ((msk = mptcp_token_iter_next(net, &s_slot, &s_num)) != NULL) {
> +	mptcp_for_each_msk(net, msk) {
> +		struct mptcp_rm_list list = { .nr = 0 };
> 		struct sock *sk = (struct sock *)msk;
> 		struct mptcp_addr_info msk_local;
>
> @@ -1179,6 +1176,8 @@ static int mptcp_nl_remove_id_zero_address(struct net *net,
> 		if (!addresses_equal(&msk_local, addr, addr->port))
> 			goto next;
>
> +		list.ids[list.nr++] = 0;
> +
> 		lock_sock(sk);
> 		spin_lock_bh(&msk->pm.lock);
> 		mptcp_pm_remove_addr(msk, &list);
> @@ -1271,13 +1270,9 @@ static void mptcp_pm_remove_addrs_and_subflows(struct mptcp_sock *msk,
> static void mptcp_nl_remove_addrs_list(struct net *net,
> 				       struct list_head *rm_list)
> {
> -	long s_slot = 0, s_num = 0;
> 	struct mptcp_sock *msk;
>
> -	if (list_empty(rm_list))
> -		return;
> -
> -	while ((msk = mptcp_token_iter_next(net, &s_slot, &s_num)) != NULL) {
> +	mptcp_for_each_msk(net, msk) {
> 		struct sock *sk = (struct sock *)msk;
>
> 		lock_sock(sk);
> @@ -1320,7 +1315,8 @@ static int mptcp_nl_cmd_flush_addrs(struct sk_buff *skb, struct genl_info *info)
> 	pernet->next_id = 1;
> 	bitmap_zero(pernet->id_bitmap, MAX_ADDR_ID + 1);
> 	spin_unlock_bh(&pernet->lock);
> -	mptcp_nl_remove_addrs_list(sock_net(skb->sk), &free_list);
> +	if (!list_empty(&free_list))
> +		mptcp_nl_remove_addrs_list(sock_net(skb->sk), &free_list);
> 	__flush_addrs(&free_list);
> 	return 0;
> }
> @@ -1535,11 +1531,10 @@ static int mptcp_nl_addr_backup(struct net *net,
> 				struct mptcp_addr_info *addr,
> 				u8 bkup)
> {
> -	long s_slot = 0, s_num = 0;
> 	struct mptcp_sock *msk;
> 	int ret = -EINVAL;
>
> -	while ((msk = mptcp_token_iter_next(net, &s_slot, &s_num)) != NULL) {
> +	mptcp_for_each_msk(net, msk) {
> 		struct sock *sk = (struct sock *)msk;
>
> 		if (list_empty(&msk->conn_list))
> -- 
> 2.30.2
>
>
>

--
Mat Martineau
Intel

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [MPTCP][PATCH mptcp-next 1/2] mptcp: add mptcp_for_each_msk helper
  2021-04-06 23:44   ` [MPTCP][PATCH mptcp-next 1/2] mptcp: add mptcp_for_each_msk helper Mat Martineau
@ 2021-04-07  7:55     ` Geliang Tang
  0 siblings, 0 replies; 8+ messages in thread
From: Geliang Tang @ 2021-04-07  7:55 UTC (permalink / raw)
  To: Mat Martineau; +Cc: mptcp

Mat Martineau <mathew.j.martineau@linux.intel.com> 于2021年4月7日周三 上午7:44写道:
>
>
> On Tue, 6 Apr 2021, Geliang Tang wrote:
>
> > This patch added a new macro helper mptcp_for_each_msk to traverse each
> > existing msk socket in the net namespace.
> >
> > Signed-off-by: Geliang Tang <geliangtang@gmail.com>
> > ---
> > net/mptcp/pm_netlink.c | 39 +++++++++++++++++----------------------
> > 1 file changed, 17 insertions(+), 22 deletions(-)
> >
> > diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c
> > index 6ba040897738..b9044defd89b 100644
> > --- a/net/mptcp/pm_netlink.c
> > +++ b/net/mptcp/pm_netlink.c
> > @@ -58,6 +58,10 @@ struct pm_nl_pernet {
> > #define MPTCP_PM_ADDR_MAX     8
> > #define ADD_ADDR_RETRANS_MAX  3
> >
> > +#define mptcp_for_each_msk(__net, __msk)                             \
> > +     long __slot = 0, __num = 0;                                     \
> > +     while ((__msk = mptcp_token_iter_next(__net, &(__slot), &(__num))) != NULL)
>
> Hi Geliang,
>
> I don't think it's good to move the variable declarations inside the macro
> like this. It looks like the compiler or sparse might have complained when
> the macro was used after a non-declaration line of code, since some other
> code (like list.ids[] initialization) was moved around. The macro
> shouldn't depend on careful placement in a code block this way - in my
> view, it's better to live with the existing syntax.

I agree with you, thanks Mat.

-Geliang

>
>
> Mat
>
>
> > +
> > static bool addresses_equal(const struct mptcp_addr_info *a,
> >                           struct mptcp_addr_info *b, bool use_port)
> > {
> > @@ -985,9 +989,8 @@ static struct pm_nl_pernet *genl_info_pm_nl(struct genl_info *info)
> > static int mptcp_nl_add_subflow_or_signal_addr(struct net *net)
> > {
> >       struct mptcp_sock *msk;
> > -     long s_slot = 0, s_num = 0;
> >
> > -     while ((msk = mptcp_token_iter_next(net, &s_slot, &s_num)) != NULL) {
> > +     mptcp_for_each_msk(net, msk) {
> >               struct sock *sk = (struct sock *)msk;
> >
> >               if (!READ_ONCE(msk->fully_established))
> > @@ -1096,14 +1099,9 @@ static int mptcp_nl_remove_subflow_and_signal_addr(struct net *net,
> >                                                  struct mptcp_addr_info *addr)
> > {
> >       struct mptcp_sock *msk;
> > -     long s_slot = 0, s_num = 0;
> > -     struct mptcp_rm_list list = { .nr = 0 };
> > -
> > -     pr_debug("remove_id=%d", addr->id);
> >
> > -     list.ids[list.nr++] = addr->id;
> > -
> > -     while ((msk = mptcp_token_iter_next(net, &s_slot, &s_num)) != NULL) {
> > +     mptcp_for_each_msk(net, msk) {
> > +             struct mptcp_rm_list list = { .nr = 0 };
> >               struct sock *sk = (struct sock *)msk;
> >               bool remove_subflow;
> >
> > @@ -1112,6 +1110,8 @@ static int mptcp_nl_remove_subflow_and_signal_addr(struct net *net,
> >                       goto next;
> >               }
> >
> > +             list.ids[list.nr++] = addr->id;
> > +
> >               lock_sock(sk);
> >               remove_subflow = lookup_subflow_by_saddr(&msk->conn_list, addr);
> >               mptcp_pm_remove_anno_addr(msk, addr, remove_subflow);
> > @@ -1162,13 +1162,10 @@ static void mptcp_pm_free_addr_entry(struct mptcp_pm_addr_entry *entry)
> > static int mptcp_nl_remove_id_zero_address(struct net *net,
> >                                          struct mptcp_addr_info *addr)
> > {
> > -     struct mptcp_rm_list list = { .nr = 0 };
> > -     long s_slot = 0, s_num = 0;
> >       struct mptcp_sock *msk;
> >
> > -     list.ids[list.nr++] = 0;
> > -
> > -     while ((msk = mptcp_token_iter_next(net, &s_slot, &s_num)) != NULL) {
> > +     mptcp_for_each_msk(net, msk) {
> > +             struct mptcp_rm_list list = { .nr = 0 };
> >               struct sock *sk = (struct sock *)msk;
> >               struct mptcp_addr_info msk_local;
> >
> > @@ -1179,6 +1176,8 @@ static int mptcp_nl_remove_id_zero_address(struct net *net,
> >               if (!addresses_equal(&msk_local, addr, addr->port))
> >                       goto next;
> >
> > +             list.ids[list.nr++] = 0;
> > +
> >               lock_sock(sk);
> >               spin_lock_bh(&msk->pm.lock);
> >               mptcp_pm_remove_addr(msk, &list);
> > @@ -1271,13 +1270,9 @@ static void mptcp_pm_remove_addrs_and_subflows(struct mptcp_sock *msk,
> > static void mptcp_nl_remove_addrs_list(struct net *net,
> >                                      struct list_head *rm_list)
> > {
> > -     long s_slot = 0, s_num = 0;
> >       struct mptcp_sock *msk;
> >
> > -     if (list_empty(rm_list))
> > -             return;
> > -
> > -     while ((msk = mptcp_token_iter_next(net, &s_slot, &s_num)) != NULL) {
> > +     mptcp_for_each_msk(net, msk) {
> >               struct sock *sk = (struct sock *)msk;
> >
> >               lock_sock(sk);
> > @@ -1320,7 +1315,8 @@ static int mptcp_nl_cmd_flush_addrs(struct sk_buff *skb, struct genl_info *info)
> >       pernet->next_id = 1;
> >       bitmap_zero(pernet->id_bitmap, MAX_ADDR_ID + 1);
> >       spin_unlock_bh(&pernet->lock);
> > -     mptcp_nl_remove_addrs_list(sock_net(skb->sk), &free_list);
> > +     if (!list_empty(&free_list))
> > +             mptcp_nl_remove_addrs_list(sock_net(skb->sk), &free_list);
> >       __flush_addrs(&free_list);
> >       return 0;
> > }
> > @@ -1535,11 +1531,10 @@ static int mptcp_nl_addr_backup(struct net *net,
> >                               struct mptcp_addr_info *addr,
> >                               u8 bkup)
> > {
> > -     long s_slot = 0, s_num = 0;
> >       struct mptcp_sock *msk;
> >       int ret = -EINVAL;
> >
> > -     while ((msk = mptcp_token_iter_next(net, &s_slot, &s_num)) != NULL) {
> > +     mptcp_for_each_msk(net, msk) {
> >               struct sock *sk = (struct sock *)msk;
> >
> >               if (list_empty(&msk->conn_list))
> > --
> > 2.30.2
> >
> >
> >
>
> --
> Mat Martineau
> Intel

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [MPTCP][PATCH mptcp-next 2/2] mptcp: use mptcp_for_each_subflow in mptcp_close
  2021-04-06 10:03   ` [MPTCP][PATCH mptcp-next 2/2] mptcp: use mptcp_for_each_subflow in mptcp_close Geliang Tang
@ 2021-04-08 15:48     ` Geliang Tang
  2021-04-08 21:14     ` Mat Martineau
  1 sibling, 0 replies; 8+ messages in thread
From: Geliang Tang @ 2021-04-08 15:48 UTC (permalink / raw)
  To: mptcp

Hi Mat,

I think this cleanup patch is still valid, isn't it?

Thanks.

-Geliang

Geliang Tang <geliangtang@gmail.com> 于2021年4月6日周二 下午6:03写道:

>
> This patch used the macro helper mptcp_for_each_subflow() instead of
> list_for_each_entry() in mptcp_close.
>
> Signed-off-by: Geliang Tang <geliangtang@gmail.com>
> ---
>  net/mptcp/protocol.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
> index 2d895c3c8746..3b7d1df8d019 100644
> --- a/net/mptcp/protocol.c
> +++ b/net/mptcp/protocol.c
> @@ -2581,7 +2581,7 @@ static void mptcp_close(struct sock *sk, long timeout)
>  cleanup:
>         /* orphan all the subflows */
>         inet_csk(sk)->icsk_mtup.probe_timestamp = tcp_jiffies32;
> -       list_for_each_entry(subflow, &mptcp_sk(sk)->conn_list, node) {
> +       mptcp_for_each_subflow(mptcp_sk(sk), subflow) {
>                 struct sock *ssk = mptcp_subflow_tcp_sock(subflow);
>                 bool slow = lock_sock_fast(ssk);
>
> --
> 2.30.2
>

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [MPTCP][PATCH mptcp-next 2/2] mptcp: use mptcp_for_each_subflow in mptcp_close
  2021-04-06 10:03   ` [MPTCP][PATCH mptcp-next 2/2] mptcp: use mptcp_for_each_subflow in mptcp_close Geliang Tang
  2021-04-08 15:48     ` Geliang Tang
@ 2021-04-08 21:14     ` Mat Martineau
  2021-04-14 13:52       ` Matthieu Baerts
  1 sibling, 1 reply; 8+ messages in thread
From: Mat Martineau @ 2021-04-08 21:14 UTC (permalink / raw)
  To: Geliang Tang; +Cc: mptcp

On Tue, 6 Apr 2021, Geliang Tang wrote:

> This patch used the macro helper mptcp_for_each_subflow() instead of
> list_for_each_entry() in mptcp_close.
>
> Signed-off-by: Geliang Tang <geliangtang@gmail.com>
> ---
> net/mptcp/protocol.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>

Thanks for the reminder Geliang. Yes, this patch is good!

Reviewed-by: Mat Martineau <mathew.j.martineau@linux.intel.com>

> diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
> index 2d895c3c8746..3b7d1df8d019 100644
> --- a/net/mptcp/protocol.c
> +++ b/net/mptcp/protocol.c
> @@ -2581,7 +2581,7 @@ static void mptcp_close(struct sock *sk, long timeout)
> cleanup:
> 	/* orphan all the subflows */
> 	inet_csk(sk)->icsk_mtup.probe_timestamp = tcp_jiffies32;
> -	list_for_each_entry(subflow, &mptcp_sk(sk)->conn_list, node) {
> +	mptcp_for_each_subflow(mptcp_sk(sk), subflow) {
> 		struct sock *ssk = mptcp_subflow_tcp_sock(subflow);
> 		bool slow = lock_sock_fast(ssk);
>
> -- 
> 2.30.2

--
Mat Martineau
Intel

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [MPTCP][PATCH mptcp-next 2/2] mptcp: use mptcp_for_each_subflow in mptcp_close
  2021-04-08 21:14     ` Mat Martineau
@ 2021-04-14 13:52       ` Matthieu Baerts
  0 siblings, 0 replies; 8+ messages in thread
From: Matthieu Baerts @ 2021-04-14 13:52 UTC (permalink / raw)
  To: Mat Martineau, Geliang Tang; +Cc: mptcp

Hi Geliang, Mat,

On 08/04/2021 23:14, Mat Martineau wrote:
> On Tue, 6 Apr 2021, Geliang Tang wrote:
> 
>> This patch used the macro helper mptcp_for_each_subflow() instead of
>> list_for_each_entry() in mptcp_close.
>>
>> Signed-off-by: Geliang Tang <geliangtang@gmail.com>
>> ---
>> net/mptcp/protocol.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
> 
> Thanks for the reminder Geliang. Yes, this patch is good!

Thank you for the patch and the review!

Just applied in our tree:

- 2f4679e077b6: mptcp: use mptcp_for_each_subflow in mptcp_close
- Results: 04d87efa6919..f90b3d445732

Cheers,
Matt
-- 
Tessares | Belgium | Hybrid Access Solutions
www.tessares.net

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2021-04-14 13:52 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-06 10:03 [MPTCP][PATCH mptcp-next 0/2] mptcp_for_each_* cleanups Geliang Tang
2021-04-06 10:03 ` [MPTCP][PATCH mptcp-next 1/2] mptcp: add mptcp_for_each_msk helper Geliang Tang
2021-04-06 10:03   ` [MPTCP][PATCH mptcp-next 2/2] mptcp: use mptcp_for_each_subflow in mptcp_close Geliang Tang
2021-04-08 15:48     ` Geliang Tang
2021-04-08 21:14     ` Mat Martineau
2021-04-14 13:52       ` Matthieu Baerts
2021-04-06 23:44   ` [MPTCP][PATCH mptcp-next 1/2] mptcp: add mptcp_for_each_msk helper Mat Martineau
2021-04-07  7:55     ` Geliang Tang

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).