Pre-RFC: Bitwise Operations for IpAddr, Ipv4Addr, Ipv6Addr

[rye]

This is a bit of a follow-up to a prior post, with the adjustment that Add and Sub are not included in the scope of the proposal.

According to the IETF RFCs defining the internet protocol, internet addresses (both v4 and v6) consist of unsigned (32- or 128-bit, respectively) integers. To implement the

• BitAnd,
• BitOr,
• BitXor,
• BitAndAssign,
• BitOrAssign,
• BitXorAssign,
• Not,
• Shl,
• ShlAssign,
• Shr, and
• ShrAssign

traits would require minimal work, basically just taking the same operations down to the underlying inner types. So, those implementations would be added to the inner types and then exposed through the sys-independent API by just invoking the operation on the inner type from the main IpAddr, Ipv4Addr, or Ipv6Addr types. I'd be happy to work through the process of landing these.

These implementations would be useful in networking code, specifically when performing operations like masking network addresses.

Questions:

• Is there anything controversial about this set of traits and what would go into implementing them? That is to say, does this need to go through the RFC process?

• Would it be useful to add heterogeneous impls as well, e.g. impl BitAnd<Ipv4Addr> for IpAddr (and vice versa?) For prior art on this idea, #72239 which added PartialOrd to SocketAddr did a similar thing.

[jschievink]

I feel like you might want a different type to represent network masks, similar to how we have both Duration and Instant: Adding two Instants doesn't make sense, but adding two Durations does, and |ing the IpAddr of two devices doesn't make sense, while |ing an IpAddr with a network mask does.

What's the motivation for the bit shift and xor impls though?

[rye]

I feel like you might want a different type to represent network masks [...]

I agree. In fact I and others have written crates centered around implementing these things (netaddr2, though the ip_network crate is more fully-featured). Such types would contain an address and a mask and would be more semantically appropriate for use in network masking, but they still have to perform the bitwise operations at some point. And, indeed, some implementations will BitAnd an address and a mask to get the "network address," while others will store the address and mask separately.

So, this proposal really only concerns itself with implementing the operations, not necessarily with any semantic meaning attached to what they mean. Depending on context, Ipv4Addr & Ipv4Addr can mean entirely different things, but I think the underlying operations are the same regardless of context.

In my library I settled on writing my own trait, Mask, of similar form to BitAnd and whose implementation required converting to a type that did implement BitAnd et al., (u32). This just meant more conversions and complexity in my crate's code, because the only way to perform the actual underlying operations on the addresses was to first convert and then do the bitwise operation. That's the main motivator here — it simplifies code that deals with arithmetic on the underlying bit strings of Ip*Addr types.

What's the motivation for the bit shift and xor impls though?

Completeness, mostly. Bit shifts could be used to take a Ipv4Addr::from(u32::MAX) (255.255.255.255) and shift it left, like

let max = Ipv4Addr::from(u32::MAX);
let slash_22 = max << 10;

rather than the only way I've found to currently do that, by first converting to an integer type and then doing the operations before re-creating a new Ipv4Addr instead,

let slash_22 = Ipv4Addr::from(u32::MAX << 10);
// or, even worse,
let max = Ipv4Addr::from(u32::MAX);
let slash_22 = Ipv4Addr::from(u32::from(max) << 10);

[josh]

Rather than adding shift operations, perhaps it would make sense to add a new constructor for that? Ipv4Addr::subnet_slash(24) would give 255.255.255.0.

[scottmcm]

@rye

That is to say, does this need to go through the RFC process?

I would say yes. I consider this a convention change (not just a minor addition that follows existing patterns) which leans towards requiring an RFC, and trait impls are insta-stable which also makes them procedurally harder to add since there's no opportunity for experimentation.

I really like the idea of a new subnet type. That solves the procedural issue, as the type can be unstable. And it allows for more semantically-meaningful operations than just raw bitops -- they individually don't make sense to me, only in combinations, which says to me that those combinations should be methods. (And those methods can themselves also be experimented with as unstable.)

Of course, that leaves open the usual "and why does this need to be in std?" question...

[pickfire]

Why not Ipv4Addr::new(255, 255, 255, 255) / 24? Like 255.255.255.255/24.