Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

AK: Introduce IPAddressCidr datatype #25006

Draft
wants to merge 10 commits into
base: master
Choose a base branch
from
293 changes: 293 additions & 0 deletions AK/IpAddressCidr.h
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rename the header to IPAddressCidr please.

Also, question to other reviewers: Do we want Cidr or CIDR? @famfo has instinctively gone with Rust naming conventions here, but we don't always do that (see inconsistencies in LibAudio, Flac vs. WAV).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd prefer CIDR

Original file line number Diff line number Diff line change
@@ -0,0 +1,293 @@
/*
* Copyright (c) 2024, famfo <[email protected]>
*
* SPDX-License-Identifier: BSD-2-Clause
*/

#pragma once

#include <AK/Error.h>
#include <AK/IPv4Address.h>
#include <AK/IPv6Address.h>
#include <AK/UFixedBigInt.h>

namespace AK {

class IPv4AddressCidr;
class IPv6AddressCidr;

namespace Details {

template<typename Address>
class AddressTraits;

template<OneOf<IPv4AddressCidr, IPv6AddressCidr> AddressFamily>
class IPAddressCidr {
public:
enum class IPAddressCidrError {
CidrTooLong,
StringParsingFailed,
};

using IPAddress = Details::AddressTraits<AddressFamily>::IPAddress;

static constexpr ErrorOr<AddressFamily, IPAddressCidrError> create(IPAddress address, u8 length)
{
if (length > AddressFamily::MAX_LENGTH)
return IPAddressCidrError::CidrTooLong;

return AddressFamily(address, length);
}

constexpr IPAddress const& ip_address() const& { return m_address; }
constexpr u32 length() const { return m_length; }

constexpr void set_ip_address(IPAddress address) { m_address = address; }
constexpr ErrorOr<void, IPAddressCidrError> set_length(u32 length)
{
if (length > AddressFamily::MAX_LENGTH)
return IPAddressCidrError::CidrTooLong;

m_length = length;
return {};
}

constexpr static ErrorOr<AddressFamily, IPAddressCidrError> from_string(StringView string)
{
Vector<StringView> const parts = string.split_view('/');

if (parts.size() != 2)
return IPAddressCidrError::StringParsingFailed;

auto ip_address = IPAddress::from_string(parts[0]);
if (!ip_address.has_value())
return IPAddressCidrError::StringParsingFailed;

Optional<u8> length = parts[1].to_number<u8>();
famfo marked this conversation as resolved.
Show resolved Hide resolved
if (!length.has_value())
return IPAddressCidrError::StringParsingFailed;

return IPAddressCidr::create(ip_address.value(), length.release_value());
}

#ifdef KERNEL
ErrorOr<NonnullOwnPtr<Kernel::KString>> to_string() const
#else
ErrorOr<String> to_string() const
#endif
{
StringBuilder builder;

auto address_string = TRY(m_address.to_string());

#ifdef KERNEL
TRY(builder.try_append(address_string->view()));
#else
TRY(builder.try_append(address_string));
#endif

TRY(builder.try_append('/'));
TRY(builder.try_appendff("{}", m_length));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ideally this would just be a

#ifdef KERNEL
        return KSting::formatted("{}/{}", m_address, m_length());
#else
        return Sting::try_formatted("{}/{}", m_address, m_length());
#endif

but we're likely missing a custom formatter, are we?
Note that fallibility in the Userland case is not enforced (anymore)

Copy link
Contributor Author

@famfo famfo Sep 10, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just doing

#ifdef KERNEL
    ErrorOr<NonnullOwnPtr<Kernel::KString>> to_string() const
    {
        return KString::formatted("{}/{}", m_address, m_length);
    }
#else
    ErrorOr<String> to_string() const
    {
        return String::formatted("{}/{}", m_address, m_length);
    }
#endif

works is this case both for IPv4 and IPv6 in hosts and serenity tests. I'll adopt this for formatting.


#ifdef KERNEL
return Kernel::KString::try_create(builder.string_view());
#else
return builder.to_string();
#endif
}

constexpr bool operator==(IPAddressCidr const& other) const = default;
constexpr bool operator!=(IPAddressCidr const& other) const = default;

protected:
constexpr IPAddressCidr(IPAddress address, u8 length)
: m_address(address)
, m_length(length)
{
}

private:
IPAddress m_address;
u8 m_length;
};

template<>
class AddressTraits<IPv4AddressCidr> {
public:
using IPAddress = IPv4Address;
};

template<>
class AddressTraits<IPv6AddressCidr> {
public:
using IPAddress = IPv6Address;
};

}

class IPv4AddressCidr : public Details::IPAddressCidr<IPv4AddressCidr> {
public:
constexpr IPv4AddressCidr(IPv4Address address, u8 length)
: IPAddressCidr(address, length)
{
}

constexpr IPv4Address netmask() const
{
IPv4Address netmask;
u8 free_bits = MAX_LENGTH - length();

if (free_bits == 32) {
netmask = IPv4Address(0, 0, 0, 0);
} else {
NetworkOrdered<u32> mask = NumericLimits<u32>::max() << free_bits;

auto address = bit_cast<Array<u8, 4>>(mask);
netmask = IPv4Address(address.data());
}

return netmask;
}

constexpr IPv4Address first_address_of_subnet() const
{
u32 mask = netmask().to_u32();
return IPv4Address(ip_address().to_u32() & mask);
}

constexpr IPv4Address last_address_of_subnet() const
{
u32 mask = netmask().to_u32();
u32 first = ip_address().to_u32() & mask;
return IPv4Address(first | ~mask);
Comment on lines +148 to +150
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit:
Isn't this essentially

ip_address().to_u32() | ((1<<m_length) - 1)

But might be mistaken bc of endianness

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The entire IPv4 address is stored in reverse host endianness so this won't work like this:

Running test 'should_find_last_in_subnet'.
FAIL: TestIPv4AddressCidr.cpp:44: EXPECT_EQ(address.last_address_of_subnet(), IPv4Address(192, 0, 2, 255)) failed with lhs=255.255.255.1 and rhs=192.0.2.255

}

bool contains(IPv4Address other) const
{
IPv4AddressCidr other_cidr = MUST(IPv4AddressCidr::create(other, length()));
return first_address_of_subnet() == other_cidr.first_address_of_subnet();
}

static u8 const MAX_LENGTH = 32;
famfo marked this conversation as resolved.
Show resolved Hide resolved
};

template<>
struct Traits<IPv4AddressCidr> : public DefaultTraits<IPv4AddressCidr> {
static unsigned hash(IPv4AddressCidr const& address)
{
IPv4Address ip_address = address.ip_address();
return sip_hash_bytes<4, 8>({ &ip_address, address.length() });
}
};

#ifdef KERNEL
template<>
struct Formatter<IPv4AddressCidr> : Formatter<StringView> {
ErrorOr<void> format(FormatBuilder& builder, IPv4AddressCidr value)
{
return Formatter<StringView>::format(builder, TRY(value.to_string())->view());
}
};
#else
template<>
struct Formatter<IPv4AddressCidr> : Formatter<StringView> {
ErrorOr<void> format(FormatBuilder& builder, IPv4AddressCidr value)
{
return Formatter<StringView>::format(builder, TRY(value.to_string()));
}
};
Comment on lines +170 to +184
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See OG commit

#endif

class IPv6AddressCidr : public Details::IPAddressCidr<IPv6AddressCidr> {
public:
constexpr IPv6AddressCidr(IPv6Address address, u8 length)
: IPAddressCidr(address, length)
{
}

constexpr IPv6Address first_address_of_subnet() const
{
u8 address[16] = { 0 };
u8 free_bits = MAX_LENGTH - length();

if (free_bits != 128) {
NetworkOrdered<u128> mask = NumericLimits<u128>::max() << free_bits;
auto address_mask = bit_cast<Array<u8, 16>>(mask);

auto const* original_address = ip_address().to_in6_addr_t();
for (int i = 0; i < 16; i++) {
address[i] = original_address[i] & address_mask[i];
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit:
Would be nicer, if IPV6Address had a u128 accessor and constructor, so this could just be a single &

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

seconded, but it would be NetworkOrdered

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have added that in m commit from yesterday

}

return address;
famfo marked this conversation as resolved.
Show resolved Hide resolved
}

constexpr IPv6Address last_address_of_subnet() const
{
u8 address[16] = { 0 };
u8 free_bits = MAX_LENGTH - length();

NetworkOrdered<u128> inverse_mask = NumericLimits<u128>::max() >> (128 - free_bits);

if (free_bits != 128) {
NetworkOrdered<u128> mask = NumericLimits<u128>::max() << free_bits;
auto address_mask = bit_cast<Array<u8, 16>>(mask);

auto const* original_address = ip_address().to_in6_addr_t();
for (int i = 0; i < 16; i++) {
address[i] = original_address[i] & address_mask[i];
}
}

auto inverse_address_mask = bit_cast<Array<u8, 16>>(inverse_mask);

for (int i = 0; i < 16; i++) {
address[i] = address[i] | inverse_address_mask[i];
}

return address;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See above/V4 version

}

bool contains(IPv6Address other) const
{
IPv6AddressCidr other_cidr = IPv6AddressCidr::create(other, length()).value();
return first_address_of_subnet() == other_cidr.first_address_of_subnet();
}

static u8 const MAX_LENGTH = 128;
famfo marked this conversation as resolved.
Show resolved Hide resolved
};

template<>
struct Traits<IPv6AddressCidr> : public DefaultTraits<IPv6AddressCidr> {
static unsigned hash(IPv6AddressCidr const& address)
{
IPv6Address ip_address = address.ip_address();
return sip_hash_bytes<4, 8>({ &ip_address, address.length() });
}
};

#ifdef KERNEL
template<>
struct Formatter<IPv6AddressCidr> : Formatter<StringView> {
ErrorOr<void> format(FormatBuilder& builder, IPv6AddressCidr value)
{
return Formatter<StringView>::format(builder, TRY(value.to_string())->view());
}
};
#else
template<>
struct Formatter<IPv6AddressCidr> : Formatter<StringView> {
ErrorOr<void> format(FormatBuilder& builder, IPv6AddressCidr value)
{
return Formatter<StringView>::format(builder, TRY(value.to_string()));
}
};
Comment on lines +247 to +261
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See the OG commit

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What I meant with this, is that this could mirror the actual implementation of the CIDR to_string method,
maybe even replace it.

With some template shenanigans (Ie DerivedFrom<IpAddressCIDR>) all formatters could use the same code

#endif

}

#if USING_AK_GLOBALLY
using AK::IPv4AddressCidr;
using AK::IPv6AddressCidr;
#endif
2 changes: 2 additions & 0 deletions Tests/AK/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,9 @@ set(AK_TEST_SOURCES
TestHashTable.cpp
TestHex.cpp
TestIPv4Address.cpp
TestIPv4AddressCidr.cpp
TestIPv6Address.cpp
TestIPv6AddressCidr.cpp
TestIndexSequence.cpp
TestInsertionSort.cpp
TestIntegerMath.cpp
Expand Down
Loading
Loading