Pre-RFC: Implementing Add, Sub, BitAnd, BitOr for IpAddr, Ipv4Addr, Ipv6Addr

[krisprice]

Hi all, I’m wondering what the appetite is for implementing Add, Sub, BitAnd, BitOr operations to IpAddr, Ipv4Addr, and Ipv6Addr. These are pretty common operations people do on IP addresses.

I’ve been doing this with an extension trait over at [1], but having it in the stdlib would be a precursor for things like doing Range operations on IP addresses.

Should I write up a RFC for this?

[1] https://github.com/krisprice/ipnet/blob/master/src/ipext.rs

[oli-obk]

I’m confused about the semantics of adding two ip addresses. That makes little sense to me. Adding integers or obtaining integers via subtracting two ip addresses sounds fine to me though.

[ExpHP]

Perhaps this is just me, but I find an unsigned difference type along with saturating_sub (as is done in your link) to be a bit scary.

[kornel]

BitOr and BitAnd are useful for subnets and such, so are probably uncontroversial.

Add/Sub are not so clear. IMHO IP + integer makes sense for iteration, but IP + IP is weird. IP - IP is used by some algorithms like DHT to compute “distance”, but that may be too specific for stdlib.

[Nemo157]

To support Range<IpAddr> being an Iterator<Item=IpAddr> requires impl<'a> Add<&'a IpAddr> for &'a IpAddr { type Output = IpAddr; } (along with an unstable impl Step for IpAddr. Being able to iterate the range seems like something that could be useful, but being able to add two IpAddrs together does seem confusing (and what would happen if you do something like (for ip in "127.0.0.1".parse::<IpAddr>().unwrap() .. "fe80::1".parse::<IpAddr>().unwrap()?)

[scottmcm]

That’s a problem with Range (well, with Step), not with IpAddr, IMHO.

It’d be better fixed with a change like this one, using usize steps instead of +:

Redesign the std::iter::Step trait, tweak related iterator impls for ranges

[krisprice]

Yep, subtracting IpAddr from IpAddr makes more obvious sense, e.g. as a way to get the “size” between two IP addresses. Addition is more for cases of iteration, or for example with an IP subnet, if you want to get the first valid host address that would be the subnet address + 1, or the last valid address would be the broadcast address - 1.

[krisprice]

I’m not sure why saturating_sub() is scary My thinking in the case of IP addresses is saturating_add/sub makes sense because you don’t expect it to overflow. But this is my first Rust project, I’m totally open to feedback across the whole crate BTW.

[krisprice]

@scottmcm, @Nemo157:

With the new Step trait will Add still be required for Range? One concern I have with using usize for Add/Sub type operations on IpAddr is the size of usize. E.g. for an IPv6Addr a step might very often be over 2^64. Is usize 128 bit?

EDIT: Answer my own question, usize is pointer sized so no. In that case it would seem that if Add/Sub on IpAddr with other IpAddr’s doesn’t make sense, then it should be just their equivalent uint (or all ints?), which means waiting for u128 to stabilize for Ipv6Addr.

@kornel: Agree BitOr/BitAnd seem easier candidates for inclusion.

[krisprice]

but being able to add two IpAddrs together does seem confusing (and what would happen if you do something like (for ip in “127.0.0.1”.parse::<IpAddr>().unwrap() … “fe80::1”.parse::<IpAddr>().unwrap()?)

As a user I’d expect this to yield the same result as trying to add a u128 to a u32, so it should be denied by the compiler. But I’m not sure how that can be done with the IpAddr enum (is there a way? I’m too much of a Rust newb). It can be done if we only implement Add/Sub for the Ipv4Addr and Ipv6Addr types.

[ExpHP]

What bothers me is that the lack of negative differences means that, for a,b : IpAddr, we do not necessarily have a + (b - a) == b

[troplin]

I don’t think that those operators should be implemented on IP addresses. It just doesn’t make sense that the difference or sum of two IP addresses is again an IP address. The same goes for subnet masks. Those are not addresses and should not be treated as such.

Both, subnet masks and some kind of “distance” between addresses (whatever that is) should be their own type and arithmetic operators should not be abused for this.

[krisprice]

I don’t think that those operators should be implemented on IP addresses. It just doesn’t make sense that the difference or sum of two IP addresses is again an IP address.

For Add and Sub operations: what about if the result wasn’t another IP address but a u32 or u128? And do you have any thoughts on how such things should be handled instead of having these operations?

For example, if I am a DHCP server and want to determine the number of IP addresses in a user configured range. Or if I’m scanning IP addresses (for whatever reason) how would the next IP address be determined in a user defined range? Without allowing operations we are forced to convert to integer, operate on integer, and convert the integer back to IpAddr. Maybe the better option is to implement Range ops somehow for IpAddr in the stdlib?

The same goes for subnet masks. Those are not addresses and should not be treated as such. Both, subnet masks and some kind of “distance” between addresses (whatever that is) should be their own type and arithmetic operators should not be abused for this.

The idea of some kind of IpMask type is interesting. Certainly you’re right, representing a mask using an IpAddr type does have problems, e.g. what’s the meaning of having is_loopback(), etc., on a mask?

What do you think about BitAnd and BitOr operations? For example, how should we determine the subnet or broadcast address? Either we convert to integer, mask, convert back to IpAddr. Or we allow these operations on IpAddr for some kind of other type (u32, u128, IpMask…).

With an IpMask type would you expect something like this?

let ip = Ipv4Addr::new(10, 1, 2, 3);
let mask = Ipv4Mask::new(24);
let subnet_addr = ip & mask; // <-- still need impl BitAnd<Ipv4Mask> for Ipv4Addr here

Or alternatively we implement something like a network(prefix: u8) and broadcast(prefix: u8) on IpAddrs:

let subnet_addr = ip.network(24);
let broadcast_addr = ip.broadcast(24);

[oli-obk]

I think there’s a set of operations that makes sense:

• ip + int = ip
• ip - ip = int
• ip & ipmask = ip
• ip … ip should be iterable and produce ips

Anything else (like ip + ip) make no sense in my head.

[Nemo157]

If all of these are related to operations involving ranges maybe having a dedicated IpAddrRange (and Ipv4AddrRange, Ipv6AddrRange) class would be useful. That would also allow adding FromStr implementations for CIDR notation (I guess std is able to add impl FromStr for Range<IpvAddr> itself, but that seems like a little bit of abuse of power since no third-party library could do the same).

Although, there are different types of IP ranges I guess, complete CIDR ranges for subnet details, and incomplete ranges as used in something like a DHCP server for the pool of addresses to hand out, not sure if a single API could nicely cover both of those use-cases.

let range = Ipv4AddrRange::new(Ipv4Addr::new(10, 1, 2, 6), 30);
range.network(); // 10.1.2.4
range.host(); // 10.1.2.6
range.hosts().collect(); // [10.1.2.5, 10.1.2.6]
range.broadcast(); // 10.1.2.7
range.subnet_mask(); // 255.255.255.252
range.contains(Ipv4Addr::new(10.1.2.5)); // true

[krisprice]

let range = Ipv4AddrRange::new(Ipv4Addr::new(10, 1, 2, 6), 30); …snip…

This is pretty much what I’m going for with the IpNet crate: https://crates.io/crates/ipnet

A range of IP addresses is a distinct thing from an IP prefix though, it may cross valid IP prefix boundaries for example.

I like @ker’s suggestion, these seem a reasonable set of operations:

ip + int = ip
ip - ip = int
ip & ipmask = ip
ip … ip should be iterable and produce ips 

I would add ip | !ipmask which gives the broadcast address.

[system]

This topic was automatically closed 90 days after the last reply. New replies are no longer allowed.