-----Original Message-----
From: Ying Xue <ying....@windriver.com>
Sent: Monday, May 11, 2020 8:23 PM
To: Tuong Tong Lien <tuong.t.l...@dektech.com.au>; jma...@redhat.com;
ma...@donjonn.com; tipc-discussion@lists.sourceforge.net
Cc: tipc-dek <tipc-...@dektech.com.au>
Subject: Re: [net 2/2] tipc: fix failed service subscription deletion
On 5/7/20 7:12 PM, Tuong Lien wrote:
> When a service subscription is expired or canceled by user, it needs to
> be deleted from the subscription list, so that new subscriptions can be
> registered (max = 65535 per net). However, there are two issues in code
> that can cause such an unused subscription to persist:
>
> 1) The 'tipc_conn_delete_sub()' has a loop on the subscription list but
> it makes a break shortly when the 1st subscription differs from the one
> specified, so the subscription will not be deleted.
>
> 2) In case a subscription is canceled, the code to remove the
> 'TIPC_SUB_CANCEL' flag from the subscription filter does not work if it
> is a local subscription (i.e. the little endian isn't involved). So, it
> will be no matches when looking for the subscription to delete later.
>
> The subscription(s) will be removed eventually when the user terminates
> its topology connection but that could be a long time later. Meanwhile,
> the number of available subscriptions may be exhausted.
>
> This commit fixes the two issues above, so as needed a subscription can
> be deleted correctly.
>
> Signed-off-by: Tuong Lien <tuong.t.l...@dektech.com.au>
> ---
> net/tipc/topsrv.c | 7 ++++---
> 1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/net/tipc/topsrv.c b/net/tipc/topsrv.c
> index 399a89f6f1bf..44609238849e 100644
> --- a/net/tipc/topsrv.c
> +++ b/net/tipc/topsrv.c
> @@ -237,8 +237,6 @@ static void tipc_conn_delete_sub(struct tipc_conn *con,
> struct tipc_subscr *s)
> if (!s || !memcmp(s, &sub->evt.s, sizeof(*s))) {
> tipc_sub_unsubscribe(sub);
> atomic_dec(&tn->subscription_count);
> - } else if (s) {
> - break;
I am quite surprised at this issue. The issue really exists as you
describe above, but it's better to fix it as follows:
@@ -237,8 +237,8 @@ static void tipc_conn_delete_sub(struct tipc_conn
*con, struct tipc_subscr *s)
if (!s || !memcmp(s, &sub->evt.s, sizeof(*s))) {
tipc_sub_unsubscribe(sub);
atomic_dec(&tn->subscription_count);
- } else if (s) {
- break;
+ if (s)
+ break;
}
}
[Tuong]: Yes, this has been included in v3 of the patch along with the change
for the sub write below, please see from there.
BR/Tuong
> }
> }
> spin_unlock_bh(&con->sub_lock);
> @@ -364,7 +362,10 @@ static int tipc_conn_rcv_sub(struct tipc_topsrv *srv,
> struct tipc_subscription *sub;
>
> if (tipc_sub_read(s, filter) & TIPC_SUB_CANCEL) {
> - s->filter &= __constant_ntohl(~TIPC_SUB_CANCEL);
> + if (!(s->filter & TIPC_FILTER_MASK))
> + s->filter &= __constant_ntohl(~TIPC_SUB_CANCEL);
> + else
> + s->filter &= ~TIPC_SUB_CANCEL;
> tipc_conn_delete_sub(con, s);
> return 0;
> }
>
_______________________________________________
tipc-discussion mailing list
tipc-discussion@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/tipc-discussion
