'Re: [sr-dev] presence module (API)'

[prev in list] [next in list] [prev in thread] [next in thread] 

List:       sr-dev
Subject:    Re: [sr-dev] presence module (API)
From:       Andrew Mortensen <admorten () isc ! upenn ! edu>
Date:       2014-01-28 15:33:37
Message-ID: 1D9BE1A215229A47A863F68BC2867E0243A779 () exch-mbx03 ! exchange ! upenn ! edu
[Download RAW message or body]

On Jan 28, 2014, at 8:36 AM, Richard Good <richard.good@smilecoms.com> wrote:

> Hi
> 
> The attached patch fixes RLS module use of pres_delete_shtable  - its fairly \
> straight forward as pres_delete_shtable is only used once and not exported as I \
> thought it might be. 
> If everyone is in agreement I will commit this change and the presence module \
> change to the master branch.

I'm in favor.

andrew

> 
> 24 January 2014 18:04, Andrew Mortensen <admorten@isc.upenn.edu> wrote:
> 
> On Jan 24, 2014, at 10:54 AM, Richard Good <richard.good@smilecoms.com> wrote:
> 
> > Hi
> > 
> > The attached patch fixes our problem.
> > 
> > As you suggested delete_shtable takes a subs_t pointer as an argument and \
> > compares the dialog's full tag set before deleting. 
> > If everyone is OK with this patch, on Monday I will look at a patch for the RLS \
> > module - it looks a little bit tricky as I think RLS exports the \
> > pres_delete_shtable through its own API as rls_delete_shtable, though I'm not \
> > 100% sure on this.
> 
> Looks good to me. Thanks.
> 
> andrew
> 
> 
> 
> > 
> > On 23 January 2014 17:24, Andrew Mortensen <admorten@isc.upenn.edu> wrote:
> > 
> > On Jan 23, 2014, at 10:06 AM, Jason Penton <jason.penton@gmail.com> wrote:
> > 
> > > Hey Andrew,
> > > 
> > > Shot for the prompt response. I am happy to make these changes if we all \
> > > agree... I was concerned like you of the impact on the consumers, but as you \
> > > say just RLS. 
> > > Can I go ahead?
> > 
> > You have my support. Post the patch here for review.
> > 
> > andrew
> > 
> > 
> > 
> > > 
> > > On Thu, Jan 23, 2014 at 5:00 PM, Andrew Mortensen <admorten@isc.upenn.edu> \
> > > wrote: 
> > > On Jan 23, 2014, at 9:19 AM, Jason Penton <jason.penton@gmail.com> wrote:
> > > 
> > > > Hey all,
> > > > 
> > > > I was taking a look at the presence module API code, specfically the API code \
> > > > for the hash table. I see that when we delete a subscription according to \
> > > > hash.c: [snip]
> > > > Why are we only searching on to-tag? What if there is a collision on the hash \
> > > > AND the to-tag is the same for 2+ different subscriptions. This is even more \
> > > > likely of happening considering that the hash calculation is based only on \
> > > > the callid and to-tag...
> > > 
> > > At least nominally the tag should be globally unique, per RFC3261:
> > > 
> > > 19.3 ... When a tag is generated by a UA for insertion into a request or
> > > response, it MUST be globally unique and cryptographically random
> > > with at least 32 bits of randomness.
> > > 
> > > But I agree that poorly behaved UAs could cause problems here. (We actually ran \
> > > into duplicate to-tags recently with vendor handsets booted simultaneously.) 
> > > It looks to me like we should be calling search_shtable, which compares the \
> > > dialog's full tag set. The problem is that unlike other similar calls, \
> > > delete_shtable doesn't take a subs_t pointer as an argument, so the only point \
> > > of reference  is the to-tag. Fortunately it looks like there's only one place \
> > > delete_shtable's called in the presence module, so it shouldn't be much work to \
> > > start using search_shtable to find the subscription to delete. 
> > > To complicate things further, though, delete_shtable is exported as part of the \
> > > presence API, so we'd need to update all API consumers as well. That doesn't \
> > > seem unreasonable, as a quick git grep indicates the only current user of the \
> > > presence API is the rls module. 
> > > andrew
> > > _______________________________________________
> > > sr-dev mailing list
> > > sr-dev@lists.sip-router.org
> > > http://lists.sip-router.org/cgi-bin/mailman/listinfo/sr-dev
> > > 
> > > _______________________________________________
> > > sr-dev mailing list
> > > sr-dev@lists.sip-router.org
> > > http://lists.sip-router.org/cgi-bin/mailman/listinfo/sr-dev
> > 
> > 
> > _______________________________________________
> > sr-dev mailing list
> > sr-dev@lists.sip-router.org
> > http://lists.sip-router.org/cgi-bin/mailman/listinfo/sr-dev
> > 
> > This email is subject to the disclaimer of Smile Communications at \
> > http://www.smilecoms.com/disclaimer 
> > 
> > <presence_mod.diff>_______________________________________________
> > sr-dev mailing list
> > sr-dev@lists.sip-router.org
> > http://lists.sip-router.org/cgi-bin/mailman/listinfo/sr-dev
> 
> 
> _______________________________________________
> sr-dev mailing list
> sr-dev@lists.sip-router.org
> http://lists.sip-router.org/cgi-bin/mailman/listinfo/sr-dev
> 
> This email is subject to the disclaimer of Smile Communications at \
> http://www.smilecoms.com/disclaimer 
> 
> <rls_mod.diff>_______________________________________________
> sr-dev mailing list
> sr-dev@lists.sip-router.org
> http://lists.sip-router.org/cgi-bin/mailman/listinfo/sr-dev

_______________________________________________
sr-dev mailing list
sr-dev@lists.sip-router.org
http://lists.sip-router.org/cgi-bin/mailman/listinfo/sr-dev

[prev in list] [next in list] [prev in thread] [next in thread] 