tipc-discussion

[tipc-discussion] [PATCH net-next] tipc: fix cancellation of topology subscriptions

[<eri...@gm...>]

From: Erik Hugne <eri...@gm...>

When cancelling a subscription, we have to clear the cancel bit
in the request before iterating over any established subscriptions
with memcmp. Otherwise no subscription will ever be found, and it
will not be possible to unsubscribe.

Signed-off-by: Erik Hugne <eri...@gm...>
---
Ignore my previous garbage patch, this solve the problem with one line.
Looking through the history of topsrv.c, i cannot find that this
has ever worked.

 net/tipc/topsrv.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/net/tipc/topsrv.c b/net/tipc/topsrv.c
index 4a708a4e8583..b05a89a723b1 100644
--- a/net/tipc/topsrv.c
+++ b/net/tipc/topsrv.c
@@ -363,6 +363,7 @@ 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);
 		tipc_conn_delete_sub(con, s);
 		return 0;
 	}
-- 
2.14.1

Re: [tipc-discussion] [PATCH net-next] tipc: fix cancellation of topology subscriptions

[Jon M. <jon...@er...>]

Acked-by: Jon.
Why did you mark it "net-next"? To me this looks like a bug fix and should go to 'net'.
I'll post it for you if you want.

///jon

> -----Original Message-----
> From: eri...@gm... <eri...@gm...>
> Sent: 19-Mar-19 11:08
> To: tip...@li...; Jon Maloy
> <jon...@er...>; yin...@wi...
> Cc: Erik Hugne <eri...@gm...>
> Subject: [PATCH net-next] tipc: fix cancellation of topology subscriptions
> 
> From: Erik Hugne <eri...@gm...>
> 
> When cancelling a subscription, we have to clear the cancel bit in the request
> before iterating over any established subscriptions with memcmp. Otherwise
> no subscription will ever be found, and it will not be possible to unsubscribe.
> 
> Signed-off-by: Erik Hugne <eri...@gm...>
> ---
> Ignore my previous garbage patch, this solve the problem with one line.
> Looking through the history of topsrv.c, i cannot find that this has ever
> worked.
> 
>  net/tipc/topsrv.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/net/tipc/topsrv.c b/net/tipc/topsrv.c index
> 4a708a4e8583..b05a89a723b1 100644
> --- a/net/tipc/topsrv.c
> +++ b/net/tipc/topsrv.c
> @@ -363,6 +363,7 @@ 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);
>  		tipc_conn_delete_sub(con, s);
>  		return 0;
>  	}
> --
> 2.14.1

Re: [tipc-discussion] [PATCH net-next] tipc: fix cancellation of topology subscriptions

[Erik H. <eri...@gm...>]

Well, i couldnt find any commit where the bug was introduced.. but i agree
that it should probably be in net.
Could you change the subject header and submit?

//E

On Tue, 19 Mar 2019, 11:56 Jon Maloy, <jon...@er...> wrote:

> Acked-by: Jon.
> Why did you mark it "net-next"? To me this looks like a bug fix and should
> go to 'net'.
> I'll post it for you if you want.
>
> ///jon
>
> > -----Original Message-----
> > From: eri...@gm... <eri...@gm...>
> > Sent: 19-Mar-19 11:08
> > To: tip...@li...; Jon Maloy
> > <jon...@er...>; yin...@wi...
> > Cc: Erik Hugne <eri...@gm...>
> > Subject: [PATCH net-next] tipc: fix cancellation of topology
> subscriptions
> >
> > From: Erik Hugne <eri...@gm...>
> >
> > When cancelling a subscription, we have to clear the cancel bit in the
> request
> > before iterating over any established subscriptions with memcmp.
> Otherwise
> > no subscription will ever be found, and it will not be possible to
> unsubscribe.
> >
> > Signed-off-by: Erik Hugne <eri...@gm...>
> > ---
> > Ignore my previous garbage patch, this solve the problem with one line.
> > Looking through the history of topsrv.c, i cannot find that this has ever
> > worked.
> >
> >  net/tipc/topsrv.c | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/net/tipc/topsrv.c b/net/tipc/topsrv.c index
> > 4a708a4e8583..b05a89a723b1 100644
> > --- a/net/tipc/topsrv.c
> > +++ b/net/tipc/topsrv.c
> > @@ -363,6 +363,7 @@ 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);
> >               tipc_conn_delete_sub(con, s);
> >               return 0;
> >       }
> > --
> > 2.14.1
>
>

Re: [tipc-discussion] [PATCH net-next] tipc: fix cancellation of topology subscriptions

[Jon M. <jon...@er...>]

No problem. I’ll do that.

///jon


From: Erik Hugne <eri...@gm...>
Sent: 19-Mar-19 12:55
To: Jon Maloy <jon...@er...>
Cc: tip...@li...; Xue, Ying <yin...@wi...>
Subject: Re: [PATCH net-next] tipc: fix cancellation of topology subscriptions

Well, i couldnt find any commit where the bug was introduced.. but i agree that it should probably be in net.
Could you change the subject header and submit?

//E

On Tue, 19 Mar 2019, 11:56 Jon Maloy, <jon...@er...<mailto:jon...@er...>> wrote:
Acked-by: Jon.
Why did you mark it "net-next"? To me this looks like a bug fix and should go to 'net'.
I'll post it for you if you want.

///jon

> -----Original Message-----
> From: eri...@gm...<mailto:eri...@gm...> <eri...@gm...<mailto:eri...@gm...>>
> Sent: 19-Mar-19 11:08
> To: tip...@li...<mailto:tip...@li...>; Jon Maloy
> <jon...@er...<mailto:jon...@er...>>; yin...@wi...<mailto:yin...@wi...>
> Cc: Erik Hugne <eri...@gm...<mailto:eri...@gm...>>
> Subject: [PATCH net-next] tipc: fix cancellation of topology subscriptions
>
> From: Erik Hugne <eri...@gm...<mailto:eri...@gm...>>
>
> When cancelling a subscription, we have to clear the cancel bit in the request
> before iterating over any established subscriptions with memcmp. Otherwise
> no subscription will ever be found, and it will not be possible to unsubscribe.
>
> Signed-off-by: Erik Hugne <eri...@gm...<mailto:eri...@gm...>>
> ---
> Ignore my previous garbage patch, this solve the problem with one line.
> Looking through the history of topsrv.c, i cannot find that this has ever
> worked.
>
>  net/tipc/topsrv.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/net/tipc/topsrv.c b/net/tipc/topsrv.c index
> 4a708a4e8583..b05a89a723b1 100644
> --- a/net/tipc/topsrv.c
> +++ b/net/tipc/topsrv.c
> @@ -363,6 +363,7 @@ 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);
>               tipc_conn_delete_sub(con, s);
>               return 0;
>       }
> --
> 2.14.1

Re: [tipc-discussion] [PATCH net-next] tipc: fix cancellation of topology subscriptions

[Partha <par...@gm...>]

Hi Erik / Jon,

The bug was introduced by:
commit 8985ecc7c1e07c42acc1e44ac56fa224f8a5c62f
Author: Jon Maloy <jon...@er...>
Date:   Thu Feb 15 10:40:46 2018 +0100

     tipc: simplify endianness handling in topology subscriber


-       /* Detect & process a subscription cancellation request */
-       if (s->filter & htohl(TIPC_SUB_CANCEL, swap)) {
-               s->filter &= ~htohl(TIPC_SUB_CANCEL, swap);
+       if (tipc_sub_read(s, filter) & TIPC_SUB_CANCEL) {
                 tipc_con_delete_sub(con, s);
                 return 0;
         }

/Partha

On 2019-03-19 12:55, Erik Hugne wrote:
> Well, i couldnt find any commit where the bug was introduced.. but i agree
> that it should probably be in net.
> Could you change the subject header and submit?
> 
> //E
> 
> On Tue, 19 Mar 2019, 11:56 Jon Maloy, <jon...@er...> wrote:
> 
>> Acked-by: Jon.
>> Why did you mark it "net-next"? To me this looks like a bug fix and should
>> go to 'net'.
>> I'll post it for you if you want.
>>
>> ///jon
>>
>>> -----Original Message-----
>>> From: eri...@gm... <eri...@gm...>
>>> Sent: 19-Mar-19 11:08
>>> To: tip...@li...; Jon Maloy
>>> <jon...@er...>; yin...@wi...
>>> Cc: Erik Hugne <eri...@gm...>
>>> Subject: [PATCH net-next] tipc: fix cancellation of topology
>> subscriptions
>>>
>>> From: Erik Hugne <eri...@gm...>
>>>
>>> When cancelling a subscription, we have to clear the cancel bit in the
>> request
>>> before iterating over any established subscriptions with memcmp.
>> Otherwise
>>> no subscription will ever be found, and it will not be possible to
>> unsubscribe.
>>>
>>> Signed-off-by: Erik Hugne <eri...@gm...>
>>> ---
>>> Ignore my previous garbage patch, this solve the problem with one line.
>>> Looking through the history of topsrv.c, i cannot find that this has ever
>>> worked.
>>>
>>>   net/tipc/topsrv.c | 1 +
>>>   1 file changed, 1 insertion(+)
>>>
>>> diff --git a/net/tipc/topsrv.c b/net/tipc/topsrv.c index
>>> 4a708a4e8583..b05a89a723b1 100644
>>> --- a/net/tipc/topsrv.c
>>> +++ b/net/tipc/topsrv.c
>>> @@ -363,6 +363,7 @@ 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);
>>>                tipc_conn_delete_sub(con, s);
>>>                return 0;
>>>        }
>>> --
>>> 2.14.1
>>
>>
> 
> _______________________________________________
> tipc-discussion mailing list
> tip...@li...
> https://lists.sourceforge.net/lists/listinfo/tipc-discussion
> 

Re: [tipc-discussion] [PATCH net-next] tipc: fix cancellation of topology subscriptions

[Jon M. <jon...@er...>]

Yes. I already added it to the patch.

///jon


> -----Original Message-----
> From: Partha <par...@gm...>
> Sent: 20-Mar-19 09:34
> To: Erik Hugne <eri...@gm...>; Jon Maloy
> <jon...@er...>
> Cc: tip...@li...
> Subject: Re: [tipc-discussion] [PATCH net-next] tipc: fix cancellation of
> topology subscriptions
> 
> Hi Erik / Jon,
> 
> The bug was introduced by:
> commit 8985ecc7c1e07c42acc1e44ac56fa224f8a5c62f
> Author: Jon Maloy <jon...@er...>
> Date:   Thu Feb 15 10:40:46 2018 +0100
> 
>      tipc: simplify endianness handling in topology subscriber
> 
> 
> -       /* Detect & process a subscription cancellation request */
> -       if (s->filter & htohl(TIPC_SUB_CANCEL, swap)) {
> -               s->filter &= ~htohl(TIPC_SUB_CANCEL, swap);
> +       if (tipc_sub_read(s, filter) & TIPC_SUB_CANCEL) {
>                  tipc_con_delete_sub(con, s);
>                  return 0;
>          }
> 
> /Partha
> 
> On 2019-03-19 12:55, Erik Hugne wrote:
> > Well, i couldnt find any commit where the bug was introduced.. but i
> > agree that it should probably be in net.
> > Could you change the subject header and submit?
> >
> > //E
> >
> > On Tue, 19 Mar 2019, 11:56 Jon Maloy, <jon...@er...> wrote:
> >
> >> Acked-by: Jon.
> >> Why did you mark it "net-next"? To me this looks like a bug fix and
> >> should go to 'net'.
> >> I'll post it for you if you want.
> >>
> >> ///jon
> >>
> >>> -----Original Message-----
> >>> From: eri...@gm... <eri...@gm...>
> >>> Sent: 19-Mar-19 11:08
> >>> To: tip...@li...; Jon Maloy
> >>> <jon...@er...>; yin...@wi...
> >>> Cc: Erik Hugne <eri...@gm...>
> >>> Subject: [PATCH net-next] tipc: fix cancellation of topology
> >> subscriptions
> >>>
> >>> From: Erik Hugne <eri...@gm...>
> >>>
> >>> When cancelling a subscription, we have to clear the cancel bit in
> >>> the
> >> request
> >>> before iterating over any established subscriptions with memcmp.
> >> Otherwise
> >>> no subscription will ever be found, and it will not be possible to
> >> unsubscribe.
> >>>
> >>> Signed-off-by: Erik Hugne <eri...@gm...>
> >>> ---
> >>> Ignore my previous garbage patch, this solve the problem with one line.
> >>> Looking through the history of topsrv.c, i cannot find that this has
> >>> ever worked.
> >>>
> >>>   net/tipc/topsrv.c | 1 +
> >>>   1 file changed, 1 insertion(+)
> >>>
> >>> diff --git a/net/tipc/topsrv.c b/net/tipc/topsrv.c index
> >>> 4a708a4e8583..b05a89a723b1 100644
> >>> --- a/net/tipc/topsrv.c
> >>> +++ b/net/tipc/topsrv.c
> >>> @@ -363,6 +363,7 @@ 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);
> >>>                tipc_conn_delete_sub(con, s);
> >>>                return 0;
> >>>        }
> >>> --
> >>> 2.14.1
> >>
> >>
> >
> > _______________________________________________
> > tipc-discussion mailing list
> > tip...@li...
> > https://lists.sourceforge.net/lists/listinfo/tipc-discussion
> >