On Fri, Jul 26, 2019 at 6:58 PM Peter Geoghegan <p...@bowt.ie> wrote: > I found this part of your approach confusing: > > > + /* > > + * Number of bits in subnet. e.g. An IPv4 that's /24 is 32 - 24 = 8. > > + * > > + * However, only some of the bits may have made it into the fixed sized > > + * datum, so take the smallest number between bits in the subnet and > > bits > > + * in the datum which are not part of the network. > > + */ > > + datum_subnet_size = Min(ip_maxbits(authoritative) - > > ip_bits(authoritative), > > + SIZEOF_DATUM * BITS_PER_BYTE - > > ip_bits(authoritative)); > > The way that you put a Min() on the subnet size potentially constrains > the size of the bitmask used for the network component of the > abbreviated key (the component that comes immediately after the > ipfamily status bit). Why not just let the bitmask be a bitmask, > without bringing SIZEOF_DATUM into it? Doing it that way allowed for a > more streamlined approach, with significantly fewer special cases. I'm > not sure whether or not your approach had bugs, but I didn't like the > way you sometimes did a straight "network = ipaddr_datum" assignment > without masking.
I guess that the idea here was to prevent masking on ipv6 addresses, though not on ipv4 addresses. Obviously we're only dealing with a prefix with ipv6 addresses, whereas we usually have the whole raw ipaddr with ipv4. Not sure if I'm doing the right thing there in v3, even though the tests pass. In any case, this will need to be a lot clearer in the final version. -- Peter Geoghegan
