[ofa-general] Re: [PATCHv3] OpenSM/osm_sa_mcmember_record.c: Collapse all scopes when consolidating IPv6 SNM

[Hal Rosenstock]

Hi Sasha,

On Sat, 2008-05-31 at 18:37 +0300, Sasha Khapyorsky wrote:
> Hi Hal,
> 
> I agree about idea. But patch itself seems to not be against main stream
> (or any known published branch). Comments are below.

It is against master but I made a mistake in generating it.

> On 13:22 Fri 30 May     , Hal Rosenstock wrote:
> > OpenSM/osm_sa_mcmember_record.c: In __search_mgrp_by_mgid, collapse all
> > scopes when consolidating IPv6 SNM
> > 
> > v2 compares masked prefixes rather than actual prefix in MCMemberRecord MGID and MGRP
> > v1 had a minor comment change
> > 
> > Patch is cumulative on minor improvement patch to this file
> > 
> > Signed-off-by: Hal Rosenstock <hal at xsigo.com>
> > 
> > --- opensm/osm_sa_mcmember_record.c.1	2008-05-30 03:58:01.129544000 -0700
> > +++ opensm/osm_sa_mcmember_record.c	2008-05-30 13:13:59.344954000 -0700
> 
> Please next time generate a diff at least at one level above (but better
> is as usual - at git tree level + 1).
> 
> > @@ -1083,19 +1083,21 @@
> >  
> >  		if (sa->p_subn->opt.consolidate_ipv6_snm_req) {
> >  			/* Special Case IPv6 Solicited Node Multicast (SNM) addresses */
> > -			/* 0xff12601bXXXX0000 : 0x00000001ffYYYYYY */
> > -			/* Where XXXX is the P_Key and
> > +			/* 0xff1Z601bXXXX0000 : 0x00000001ffYYYYYY */
> > +			/* Where Z is the scope, XXXX is the P_Key, and
> >  			 * YYYYYY is the last 24 bits of the port guid */
> > -#define PREFIX_MASK (0xff10601b00000000ULL)
> 
> There I have 0xff12601b00000000ULL value (and likely it is what you
> wanted to fix :)).

Correct. That was where the generated patch was broken.

> > +#define PREFIX_MASK (0xff10ffff00000000ULL)
> > +#define PREFIX_SIGNATURE (0xff10601b00000000ULL)
> >  #define INT_ID_MASK (0x00000001ff000000ULL)
> >  			uint64_t g_prefix = cl_ntoh64(p_mgrp->mcmember_rec.mgid.unicast.prefix);
> >  			uint64_t g_interface_id = cl_ntoh64(p_mgrp->mcmember_rec.mgid.unicast.interface_id);
> >  			uint64_t rcv_prefix = cl_ntoh64(p_recvd_mgid->unicast.prefix);
> >  			uint64_t rcv_interface_id = cl_ntoh64(p_recvd_mgid->unicast.interface_id);
> >  
> > -			if ((rcv_prefix & PREFIX_MASK) == PREFIX_MASK &&
> > +			if ((rcv_prefix & PREFIX_MASK) == PREFIX_SIGNATURE &&
> 
> If you are changing PREFIX_MASK to 0xff10601b00000000ULL,

No, PREFIX_MASK is being changed to 0xff10ffff00000000ULL to:
1. eliminate the scope part, and
2. get the entire signature
for subsequent comparison.

>  why PREFIX_SIGNATURE is needed? Am I missing something?

Because PREFIX_MASK masks the bits to compare and SIGNATURE is what it
should be after masking.

I'll regenerate v4 and hopefully I'll get it right.

-- Hal

> Sasha
> 
> >  			    (rcv_interface_id & INT_ID_MASK) == INT_ID_MASK &&
> > -			    g_prefix == rcv_prefix &&
> > +			    (g_prefix & PREFIX_MASK) ==
> > +			     (rcv_prefix && PREFIX_MASK) &&
> >  			    (g_interface_id & INT_ID_MASK) ==
> >  			     (rcv_interface_id & INT_ID_MASK)) {
> >  				OSM_LOG(sa->p_log, OSM_LOG_INFO,
> > 
> > 
> _______________________________________________
> general mailing list
> general at lists.openfabrics.org
> http://lists.openfabrics.org/cgi-bin/mailman/listinfo/general
> 
> To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general