On Thu, Jan 25, 2024 at 06:01:14AM +0100, Theo Buehler wrote:
> On Wed, Jan 24, 2024 at 03:43:29PM +0100, Claudio Jeker wrote:
> > This switches IMSG_CTL_SHOW_RIB_COMMUNITIES over to use ibufs in bgpctl.
> > This converts some of the community code in bgpctl but not all. The
> > reminder will follow at a later stage. Those code paths are only used by
> > the mrt parser.
> > 
> > This big change is how fmt_ext_community() works. It now just operates on
> > a uint64_t instead of passing in unchecked pointers.
> 
> I don't understand the default case of the switch in fmt_ext_community().
> Otherwise this looks good.
> 
> >  	case EXT_COMMUNITY_NON_TRANS_OPAQUE:
> > -		memcpy(&ext, data, sizeof(ext));
> > -		ext = be64toh(ext) & 0xffffffffffffLL;
> > +		ext &= 0xffffffffffffULL;
> >  		if (subtype == EXT_COMMUNITY_SUBTYPE_OVS) {
> >  			switch (ext) {
> >  			case EXT_COMMUNITY_OVS_VALID:
> > @@ -1119,10 +1111,10 @@ fmt_ext_community(uint8_t *data)
> >  		}
> >  		break;
> >  	default:
> > -		memcpy(&ext, data, sizeof(ext));
> > +		ext &= 0xffffffffffffULL;
> 
> Why is this truncated to the lower 32 bits?

It is currently 48bits. Extended communities are a nightmare since the
subtype can potentially be part of the value. We don't support any and
also log_ext_subtype() is not properly prepared for this.

Guess we should just dump them as 0x%llx", (unsigned long long)ext))
without any masking since we have no code for them and all bets are off.
Will change the code accordingly.

I hate ext-communities...
 
> >  		snprintf(buf, sizeof(buf), "%s 0x%llx",
> >  		    log_ext_subtype(type, subtype),
> > -		    (unsigned long long)be64toh(ext));
> > +		    (unsigned long long)ext);
> >  		return buf;
> >  	}
> >  }
> 

-- 
:wq Claudio