2
0
mirror of https://gitlab.isc.org/isc-projects/kea synced 2025-08-29 13:07:50 +00:00

[#3683] Addressed more comments

This commit is contained in:
Francis Dupont 2025-03-01 01:29:14 +01:00
parent c2177325d2
commit 6861ed9c2a
13 changed files with 299 additions and 19 deletions

View File

@ -6869,6 +6869,13 @@ added or updated and a ADDR-REG-REPLY (37) is sent back to the client.
Kea accepts and handles the Option Request option in the ADDR-REG-INFORM
message (the RFC specifies the client MUST NOT put such option in it).
.. note::
Kea currently only accepts addresses which are "appropriate to the link"
but not yet "within a prefix delegated to the client" which is not
compatible with the way the topology is described in the Kea
configuration.
.. _dhcp6-stats:
Statistics in the DHCPv6 Server

View File

@ -2200,7 +2200,7 @@ mysql_lease6_stat_per_type() {
# insert a lease, update it, and delete checking the
# lease stat counts along the way. It assumes the
# database has been created but is empty.
# param addr - address to use to add to subnet 1
# param addr, addr1, addr2 - addresses to use to add to subnet 1
mysql_lease6_stat_registered() {
addr=$1;shift
addr1=$1;shift
@ -2288,7 +2288,7 @@ mysql_lease6_stat_test() {
# Test for address ::22, PD lease type
mysql_lease6_stat_per_type "::22" "::23" "::24" "1"
# Test for registered address ::33
# Test for registered addresses ::33, ::34 and ::35
mysql_lease6_stat_registered "::33" "::34" "::35"
# Let's wipe the whole database

View File

@ -1753,7 +1753,7 @@ pgsql_lease6_stat_per_type() {
# insert a lease, update it, and delete checking the
# lease stat counts along the way. It assumes the
# database has been created but is empty.
# param addr - address to use to add to subnet 1
# param addr, addr1, addr2 - addresses to use to add to subnet 1
pgsql_lease6_stat_registered() {
addr=$1;shift
addr1=$1;shift
@ -1841,7 +1841,7 @@ pgsql_lease6_stat_test() {
# Test for address 222, PD lease type
pgsql_lease6_stat_per_type "::22" "::23" "::24" "1"
# Test for registered address ::33
# Test for registered addresses ::33, ::34 and ::35
pgsql_lease6_stat_registered "::33" "::34" "::35"
# Let's wipe the whole database

View File

@ -3384,7 +3384,7 @@ Dhcpv6Srv::releaseIA_NA(const DuidPtr& duid, const Pkt6Ptr& query,
Lease6Ptr lease = LeaseMgrFactory::instance().getLease6(Lease::TYPE_NA,
release_addr->getAddress());
if (!lease) {
if (!lease || (lease->state_ == Lease::STATE_REGISTERED)) {
// client releasing a lease that we don't know about.
// Insert status code NoBinding.

View File

@ -181,7 +181,11 @@ public:
/// @brief Test that the registered lease is updated even it
/// belongs to another client.
void testAnother();
void testAnotherClient();
/// @brief Test that the registered lease is updated even it
/// belongs to another subnet.
void testAnotherSubnet();
/// @brief Common part of skip/drop callout.
void commonSkipOrDrop();
@ -904,7 +908,7 @@ TEST_F(AddrRegTest, renew) {
// Test that the registered lease is updated even it belongs to another client.
void
AddrRegTest::testAnother() {
AddrRegTest::testAnotherClient() {
// Create and add a lease for another client.
IOAddress addr("2001:db8:1::1");
DuidPtr duid(new DUID(vector<uint8_t>(8, 0x44)));
@ -961,12 +965,75 @@ AddrRegTest::testAnother() {
}
// Test that the registered lease is updated even it belongs to another client.
TEST_F(AddrRegTest, another) {
TEST_F(AddrRegTest, anotherClient) {
IfaceMgrTestConfig test_config(true);
ASSERT_NO_THROW(configure(config_));
testAnother();
testAnotherClient();
}
// Test that the registratered lease is updated even it belongs to
// another subnet.
void
AddrRegTest::testAnotherSubnet() {
// Create and add a lease.
IOAddress addr("2001:db8:1::1");
OptionPtr clientid = generateClientId();
Lease6Ptr lease(new Lease6(Lease::TYPE_NA, addr, duid_, 2345, 200, 300, 2));
lease->state_ = Lease::STATE_REGISTERED;
lease->cltt_ = 1234;
ASSERT_TRUE(LeaseMgrFactory::instance().addLease(lease));
addr_reg_inf_ = Pkt6Ptr(new Pkt6(DHCPV6_ADDR_REG_INFORM, 1234));
addr_reg_inf_->setRemoteAddr(addr);
addr_reg_inf_->setIface("eth0");
addr_reg_inf_->setIndex(ETH0_INDEX);
addr_reg_inf_->addOption(clientid);
auto ia = generateIA(D6O_IA_NA, 234, 1500, 3000);
ia->addOption(generateIAAddr(addr, 3000, 4000));
addr_reg_inf_->addOption(ia);
// Pass it to the server.
AllocEngine::ClientContext6 ctx;
bool drop = !srv_.earlyGHRLookup(addr_reg_inf_, ctx);
ASSERT_FALSE(drop);
ctx.subnet_ = srv_.selectSubnet(addr_reg_inf_, drop);
ASSERT_FALSE(drop);
srv_.initContext(ctx, drop);
ASSERT_FALSE(drop);
ASSERT_TRUE(ctx.subnet_);
// Verify the response.
Pkt6Ptr response = srv_.processAddrRegInform(ctx);
ASSERT_TRUE(response);
// Verify the updated lease.
Lease6Ptr l = LeaseMgrFactory::instance().getLease6(Lease::TYPE_NA, addr);
ASSERT_TRUE(l);
EXPECT_EQ(Lease::STATE_REGISTERED, l->state_);
EXPECT_EQ(addr, l->addr_);
ASSERT_TRUE(l->duid_);
EXPECT_TRUE(*l->duid_ == *duid_);
EXPECT_EQ(ia->getIAID(), l->iaid_);
EXPECT_EQ(1, l->subnet_id_);
EXPECT_FALSE(l->fqdn_fwd_);
EXPECT_FALSE(l->fqdn_rev_);
string expected = "DHCPSRV_MEMFILE_UPDATE_ADDR6 ";
expected += "updating IPv6 lease for address 2001:db8:1::1";
EXPECT_EQ(1, countFile(expected));
expected = "DHCP6_ADDR_REG_INFORM_CLIENT_CHANGE";
EXPECT_EQ(0, countFile(expected));
}
// Test that the registered lease is updated even it belongs to another subnet.
TEST_F(AddrRegTest, anotherSubnet) {
IfaceMgrTestConfig test_config(true);
ASSERT_NO_THROW(configure(config_));
testAnotherSubnet();
}
// Test that the FQDN option is handled.
@ -1357,7 +1424,7 @@ TEST_F(AddrRegTest, calloutRenew) {
}
// Test the callout in the another scenario.
TEST_F(AddrRegTest, calloutAnother) {
TEST_F(AddrRegTest, calloutAnotherClient) {
IfaceMgrTestConfig test_config(true);
ASSERT_NO_THROW(configure(config_));
@ -1365,7 +1432,7 @@ TEST_F(AddrRegTest, calloutAnother) {
// Install addr6_register_callout.
EXPECT_NO_THROW(HooksManager::preCalloutsLibraryHandle().registerCallout(
"addr6_register", addr6_register_callout));
testAnother();
testAnotherClient();
EXPECT_TRUE(callout_errmsg_.empty()) << callout_errmsg_;
checkCalloutHandleReset();
@ -1535,4 +1602,45 @@ TEST_F(AddrRegTest, statsRenew) {
EXPECT_EQ(20, stat->getInteger().first);
}
// Check the statictics for the another subnet scenario.
TEST_F(AddrRegTest, statsAnotherSubnet) {
IfaceMgrTestConfig test_config(true);
ASSERT_NO_THROW(configure(config_));
string registered_nas_name2 =
StatsMgr::generateName("subnet", 2, "registered-nas");
StatsMgr::instance().setValue(registered_nas_name2,
static_cast<int64_t>(3));
StatsMgr::instance().setValue(registered_nas_name_,
static_cast<int64_t>(5));
string cumulative_registered_nas_name2 =
StatsMgr::generateName("subnet", 2, "cumulative-registered-nas");
StatsMgr::instance().setValue(cumulative_registered_nas_name2,
static_cast<int64_t>(8));
StatsMgr::instance().setValue(cumulative_registered_nas_name_,
static_cast<int64_t>(10));
StatsMgr::instance().setValue("cumulative-registered-nas",
static_cast<int64_t>(20));
testAnotherSubnet();
// Statistics should have been not touched.
ObservationPtr stat;
stat = StatsMgr::instance().getObservation(registered_nas_name2);
ASSERT_TRUE(stat);
EXPECT_EQ(2, stat->getInteger().first);
stat = StatsMgr::instance().getObservation(registered_nas_name_);
ASSERT_TRUE(stat);
EXPECT_EQ(6, stat->getInteger().first);
stat = StatsMgr::instance().getObservation(cumulative_registered_nas_name2);
ASSERT_TRUE(stat);
EXPECT_EQ(8, stat->getInteger().first);
stat = StatsMgr::instance().getObservation(cumulative_registered_nas_name_);
ASSERT_TRUE(stat);
EXPECT_EQ(11, stat->getInteger().first);
stat = StatsMgr::instance().getObservation("cumulative-registered-nas");
EXPECT_EQ(20, stat->getInteger().first);
}
} // end of anonymous namespace

View File

@ -2313,6 +2313,47 @@ TEST_F(Dhcpv6SrvTest, pdReleaseReject) {
testReleaseReject(Lease::TYPE_PD, IOAddress("2001:db8:1:2::"));
}
// This test verifies that RELEASE doesn't work on registered leases.
TEST_F(Dhcpv6SrvTest, ReleaseRegistered) {
NakedDhcpv6Srv srv(0);
const IOAddress addr("2001:db8:1:2::");
const uint32_t transid = 1234;
const uint32_t iaid = 234;
// GenerateClientId() also sets duid_
OptionPtr clientid = generateClientId();
// Create a registered lease.
Lease6Ptr lease(new Lease6(Lease::TYPE_NA, addr, duid_, iaid, 501, 502,
subnet_->getID(), HWAddrPtr()));
lease->state_ = Lease::STATE_REGISTERED;
ASSERT_TRUE(LeaseMgrFactory::instance().addLease(lease));
// Let's create a RELEASE
Pkt6Ptr rel = createMessage(DHCPV6_RELEASE, Lease::TYPE_NA, addr, 128, iaid);
rel->addOption(clientid);
rel->addOption(srv.getServerID());
// Pass it to the server and hope for a REPLY
Pkt6Ptr reply = srv.processRelease(rel);
// Check if we get response at all
checkResponse(reply, DHCPV6_REPLY, transid);
OptionPtr tmp = reply->getOption(D6O_IA_NA);
ASSERT_TRUE(tmp);
// Check that IA_NA/IA_PD was returned and that there's status code in it
boost::shared_ptr<Option6IA> ia = boost::dynamic_pointer_cast<Option6IA>(tmp);
ASSERT_TRUE(ia);
checkIA_NAStatusCode(ia, STATUS_NoBinding, 0, 0);
checkMsgStatusCode(reply, STATUS_NoBinding);
// Check that the lease is not there
Lease6Ptr l = LeaseMgrFactory::instance().getLease6(Lease::TYPE_NA, addr);
ASSERT_TRUE(l);
ASSERT_EQ(Lease::STATE_REGISTERED, l->state_);
}
// This test verifies if the sanityCheck() really checks options presence.
TEST_F(Dhcpv6SrvTest, sanityCheck) {
NakedDhcpv6Srv srv(0);

View File

@ -1076,6 +1076,7 @@ TEST_F(RunScriptTest, lease6Decline) {
checkScriptResult();
}
// Check the addr6_register callout.
TEST_F(RunScriptTest, add6Register) {
impl.reset(new RunScriptImpl());
impl->setName(RUN_SCRIPT_TEST_SH);

View File

@ -477,11 +477,12 @@ AllocEngine::allocateLeases6(ClientContext6& ctx) {
ctx.currentIA().iaid_);
// Iterate over the leases and eliminate those that are outside of
// our shared network.
// our shared network or registered.
Lease6Collection leases;
while (subnet) {
for (auto const& l : all_leases) {
if ((l)->subnet_id_ == subnet->getID()) {
if (((l)->state_ != Lease::STATE_REGISTERED) &&
((l)->subnet_id_ == subnet->getID())) {
leases.push_back(l);
}
}
@ -2107,8 +2108,11 @@ AllocEngine::renewLeases6(ClientContext6& ctx) {
*ctx.duid_,
ctx.currentIA().iaid_,
subnet->getID());
leases.insert(leases.end(), leases_subnet.begin(), leases_subnet.end());
for (auto const& l : leases_subnet) {
if (l->state_ != Lease::STATE_REGISTERED) {
leases.push_back(l);
}
}
subnet = subnet->getNextSubnet(ctx.subnet_);
}

View File

@ -737,6 +737,49 @@ TEST_F(AllocEngine6Test, solicitReuseExpiredRegisteredLease6) {
EXPECT_FALSE(lease);
}
// This test checks a registered lease can't be reused in SOLICT.
TEST_F(AllocEngine6Test, solicitReuseRegisteredLease6) {
boost::scoped_ptr<AllocEngine> engine;
ASSERT_NO_THROW(engine.reset(new AllocEngine(100)));
ASSERT_TRUE(engine);
IOAddress addr("2001:db8:1::ad");
// Create one subnet with a pool holding one address.
initSubnet(IOAddress("2001:db8:1::"), addr, addr);
// Initialize FQDN data for the lease.
initFqdn("myhost.example.com", true, true);
Lease6Ptr lease(new Lease6(Lease::TYPE_NA, addr, duid_, iaid_,
501, 502, subnet_->getID(), HWAddrPtr()));
lease->state_ = Lease::STATE_REGISTERED;
lease->cltt_ = time(0) - 200; // Registered 200 seconds ago
lease->valid_lft_ = 400; // Lease was valid for 400 seconds
ASSERT_TRUE(LeaseMgrFactory::instance().addLease(lease));
// CASE 1: Asking for any address
AllocEngine::ClientContext6 ctx1(subnet_, duid_, fqdn_fwd_, fqdn_rev_, hostname_,
true, Pkt6Ptr(new Pkt6(DHCPV6_SOLICIT, 1234)));
ctx1.currentIA().iaid_ = iaid_;
EXPECT_NO_THROW(lease = expectOneLease(engine->allocateLeases6(ctx1)));
// Check that we did not get this single lease.
EXPECT_FALSE(lease);
// CASE 2: Asking specifically for this address
AllocEngine::ClientContext6 ctx2(subnet_, duid_, false, false, "", true,
Pkt6Ptr(new Pkt6(DHCPV6_REQUEST, 1234)));
ctx2.currentIA().iaid_ = iaid_;
ctx2.currentIA().addHint(addr);
EXPECT_NO_THROW(lease = expectOneLease(engine->allocateLeases6(ctx2)));
// Check that we did not get this single lease.
EXPECT_FALSE(lease);
}
// This test checks if an expired lease can be reused in REQUEST (actual allocation)
TEST_F(AllocEngine6Test, requestReuseExpiredLease6) {
boost::scoped_ptr<AllocEngine> engine;
@ -872,6 +915,51 @@ TEST_F(AllocEngine6Test, requestReuseExpiredRegisteredLease6) {
EXPECT_FALSE(old_lease);
}
// This test checks a registered lease can't be reused in REQUEST.
TEST_F(AllocEngine6Test, requestReuseRegisteredLease6) {
boost::scoped_ptr<AllocEngine> engine;
ASSERT_NO_THROW(engine.reset(new AllocEngine(100)));
ASSERT_TRUE(engine);
IOAddress addr("2001:db8:1::ad");
CfgMgr& cfg_mgr = CfgMgr::instance();
cfg_mgr.clear(); // Get rid of the default test configuration
// Create configuration similar to other tests, but with a single address pool
subnet_ = Subnet6::create(IOAddress("2001:db8:1::"), 56, 1, 2, 3, 4, SubnetID(10));
pool_ = Pool6Ptr(new Pool6(Lease::TYPE_NA, addr, addr)); // just a single address
subnet_->addPool(pool_);
cfg_mgr.getStagingCfg()->getCfgSubnets6()->add(subnet_);
cfg_mgr.commit();
// Let's create a registered lease
const SubnetID other_subnetid = 999;
Lease6Ptr lease(new Lease6(Lease::TYPE_NA, addr, duid_, iaid_,
501, 502, other_subnetid, HWAddrPtr()));
lease->state_ = Lease::STATE_REGISTERED;
lease->cltt_ = time(0) - 200; // Registered 200 seconds ago
lease->valid_lft_ = 400; // Lease was valid for 400 seconds
lease->fqdn_fwd_ = true;
lease->fqdn_rev_ = true;
lease->hostname_ = "myhost.example.com.";
ASSERT_TRUE(LeaseMgrFactory::instance().addLease(lease));
// A client comes along, asking specifically for this address
AllocEngine::ClientContext6 ctx(subnet_, duid_, false, false, "", false,
Pkt6Ptr(new Pkt6(DHCPV6_REQUEST, 1234)));
ctx.currentIA().iaid_ = iaid_;
ctx.currentIA().addHint(addr);
EXPECT_NO_THROW(lease = expectOneLease(engine->allocateLeases6(ctx)));
// Check that we did not get this single lease.
EXPECT_FALSE(lease);
// Check that no old lease has been returned.
Lease6Ptr old_lease = expectOneLease(ctx.currentIA().old_leases_);
EXPECT_FALSE(old_lease);
}
// This test checks if a released lease can be reused in REQUEST (actual allocation)
TEST_F(AllocEngine6Test, requestReuseReleasedLease6) {
boost::scoped_ptr<AllocEngine> engine;
@ -1042,6 +1130,37 @@ TEST_F(AllocEngine6Test, renewExtendLeaseLifetime) {
<< "Lease lifetime was not extended, but it should";
}
// Checks a registered lease can't be renewed.
TEST_F(AllocEngine6Test, renewRegisteredLease6) {
// Create a registered lease for the client.
IOAddress addr("2001:db8:1::15");
Lease6Ptr lease(new Lease6(Lease::TYPE_NA, addr,
duid_, iaid_, 300, 400,
subnet_->getID(), HWAddrPtr()));
lease->state_ = Lease::STATE_REGISTERED;
// Allocated 200 seconds ago - half of the lifetime.
time_t lease_cltt = time(0) - 200;
lease->cltt_ = lease_cltt;
ASSERT_TRUE(LeaseMgrFactory::instance().addLease(lease));
AllocEngine engine(100);
// This is what the client will send in his renew message.
AllocEngine::HintContainer hints;
hints.push_back(AllocEngine::Resource(addr, 128));
// Get renewed leases.
Lease6Collection renewed = renewTest(engine, pool_, hints, IN_SUBNET, IN_POOL);
for (auto const& l : renewed) {
// Not registered.
EXPECT_EQ(0, l->state_);
// Another address.
EXPECT_NE(addr, l->addr_);
}
}
// Checks that a renewed lease uses default lifetimes.
TEST_F(AllocEngine6Test, defaultRenewLeaseLifetime) {
// Create a lease for the client.

View File

@ -6169,7 +6169,7 @@ UPDATE schema_version
INSERT INTO lease_state VALUES (4, 'registered');
-- Update *_lease6_stat stored procedures to count registered leases.
-- Not *_lease6_pool_stat because a registered address in not from a pool
-- Not *_lease6_pool_stat because a registered address is not from a pool
-- Not *_lease6_stat_by_client_class because it is for state 0 only
DROP PROCEDURE IF EXISTS lease6_AINS_lease6_stat;

View File

@ -60,7 +60,7 @@ mysql "$@" <<EOF
INSERT INTO lease_state VALUES (4, 'registered');
-- Update *_lease6_stat stored procedures to count registered leases.
-- Not *_lease6_pool_stat because a registered address in not from a pool
-- Not *_lease6_pool_stat because a registered address is not from a pool
-- Not *_lease6_stat_by_client_class because it is for state 0 only
DROP PROCEDURE IF EXISTS lease6_AINS_lease6_stat;

View File

@ -6628,7 +6628,7 @@ UPDATE schema_version
INSERT INTO lease_state VALUES (4, 'registered');
-- Update *_lease6_stat stored procedures to count registered leases.
-- Not *_lease6_pool_stat because a registered address in not from a pool
-- Not *_lease6_pool_stat because a registered address is not from a pool
-- Not *_lease6_stat_by_client_class because it is for state 0 only
CREATE OR REPLACE FUNCTION lease6_AINS_lease6_stat(IN new_state BIGINT,

View File

@ -43,7 +43,7 @@ START TRANSACTION;
INSERT INTO lease_state VALUES (4, 'registered');
-- Update *_lease6_stat stored procedures to count registered leases.
-- Not *_lease6_pool_stat because a registered address in not from a pool
-- Not *_lease6_pool_stat because a registered address is not from a pool
-- Not *_lease6_stat_by_client_class because it is for state 0 only
CREATE OR REPLACE FUNCTION lease6_AINS_lease6_stat(IN new_state BIGINT,