Re: Linux 2.6.35/TIPC 2.0 ABI breaking changes

[Leandro Lucarella <luca@llucax.com.ar>]

Leandro Lucarella, el 19 de octubre a las 17:43 me escribiste:
> Neil Horman, el 19 de octubre a las 16:18 me escribiste:
> > Hey all-
> > 	Heres what I have so far.  Dave as a heads up please don't apply this
> > yet.  I'd like to go over it a bit more and be sure of the implications here
> > before I post it for inclusion officially.  I wanted Leandro to have a copy
> > though so he could confirm functionality for us.  Leandro, This patch lets me
> > pass the tipc test code for TIPC 1.6 that you posted earlier this morning.  If
> > you could confirm that it works for you that would be grand.  While your doing
> > that, I want to read over the spec for TIPC and make sure that I'm not breaking
> > anything new with this patch.
> 
> Thanks for the quick response. I didn't tried the patch yet, but I think
> spotted an error:
> 
> > diff --git a/include/linux/tipc.h b/include/linux/tipc.h
> > index 181c8d0..d8de884 100644
> > --- a/include/linux/tipc.h
> > +++ b/include/linux/tipc.h
> > @@ -127,9 +127,10 @@ static inline unsigned int tipc_node(__u32 addr)
> >   * TIPC topology subscription service definitions
> >   */
> >  
> > -#define TIPC_SUB_SERVICE     	0x00  	/* Filter for service availability    */
> > -#define TIPC_SUB_PORTS     	0x01  	/* Filter for port availability  */
> > +#define TIPC_SUB_SERVICE     	0x01  	/* Filter for service availability    */
> > +#define TIPC_SUB_PORTS     	0x02  	/* Filter for port availability  */
> >  #define TIPC_SUB_CANCEL         0x04    /* Cancel a subscription         */
> > +#define TIPC_SUB_MASK		(TIPC_SUB_SERVICE|TIPC_SUB_PORTS|TIPC_SUB_CANCEL)
> >  
> >  #define TIPC_WAIT_FOREVER	~0	/* timeout for permanent subscription */
> >  
> 
> The values of TIPC_SUB_SERVICE and TIPC_SUB_PORTS seem to be swapped
> compared to the old (TIPC 1.6) values:
> #define TIPC_SUB_PORTS     	0x01  	/* filter for port availability */
> #define TIPC_SUB_SERVICE     	0x02  	/* filter for service availability */
> 
> You might missed this error when trying the code I posted earlier (which
> BTW are the official TIPC demos) because the change of behaviour when
> using TIPC_SUB_SERVICE and TIPC_SUB_PORTS is very subtle (the former
> reports only the first published port name that matches, and the later
> all the published ports that match).

Also, I'm surprised the TIPC 1.6 demo program worked for you, as I don't
see any translation from the old TIPC_SUB_SERVICE value (2) to the new
one (0, or the absence of the TIPC_SUB_PORTS flag). I guess if this
subscription should be sent through the wire and should comply with
TIPC 2.0, filter & 2 should be 0.

Also, without such translation, I don't see how TIPC_SUB_SERVICE doesn't
trigger a subscription rejection with this check:

	if ((sub->filter && (sub->filter != TIPC_SUB_PORTS)) ||
			(sub->seq.lower > sub->seq.upper)) {
		warn("Subscription rejected, illegal request\n");

But if you tried the TIPC 1.6 demo and it worked, I guess I'm definitely
missing something (as I'm not familiar with TIPC code at all).

Anyway, I'll try the patch tomorrow morning and tell you how it went,
but I thought I should point out those things just in case.

Thanks.

-- 
Leandro Lucarella (AKA luca)                     http://llucax.com.ar/
----------------------------------------------------------------------
GPG Key: 5F5A8D05 (F8CD F9A7 BF00 5431 4145  104C 949E BFB6 5F5A 8D05)
----------------------------------------------------------------------
Be nice to nerds
Chances are you'll end up working for one