2
0
mirror of https://gitlab.isc.org/isc-projects/kea synced 2025-08-30 13:37:55 +00:00

[#1584] Addressed minor review comments

modified:
    src/bin/dhcp4/dhcp4_srv.cc
    src/bin/dhcp4/tests/dora_unittest.cc
    src/bin/dhcp4/tests/hooks_unittest.cc
    src/lib/dhcpsrv/alloc_engine.cc
This commit is contained in:
Thomas Markwalder 2021-10-21 11:14:38 -04:00
parent 45e78cdcec
commit 49fe9fbf35
4 changed files with 11 additions and 12 deletions

View File

@ -2578,7 +2578,7 @@ Dhcpv4Srv::assignLease(Dhcpv4Exchange& ex) {
// Address might have been rejected via class guard (i.e. not // Address might have been rejected via class guard (i.e. not
// allowed for this client). We need to determine if we truly // allowed for this client). We need to determine if we truly
// do not know about the address or whether this client just // do not know about the address or whether this client just
// isn't allowed to have that address. We should only NAK // isn't allowed to have that address. We should only DHCPNAK
// For the latter. // For the latter.
while (s) { while (s) {
if (s->inPool(Lease::TYPE_V4, hint)) { if (s->inPool(Lease::TYPE_V4, hint)) {
@ -2589,7 +2589,7 @@ Dhcpv4Srv::assignLease(Dhcpv4Exchange& ex) {
} }
// If we didn't find a subnet, it's not an address we know about // If we didn't find a subnet, it's not an address we know about
// so we drop the NAK. // so we drop the DHCPNAK.
if (!s) { if (!s) {
LOG_DEBUG(bad_packet4_logger, DBG_DHCP4_DETAIL, LOG_DEBUG(bad_packet4_logger, DBG_DHCP4_DETAIL,
DHCP4_UNKNOWN_ADDRESS_REQUESTED) DHCP4_UNKNOWN_ADDRESS_REQUESTED)
@ -2886,9 +2886,9 @@ Dhcpv4Srv::adjustRemoteAddr(Dhcpv4Exchange& ex) {
} else if (!query->getCiaddr().isV4Zero()) { } else if (!query->getCiaddr().isV4Zero()) {
response->setRemoteAddr(query->getCiaddr()); response->setRemoteAddr(query->getCiaddr());
// We can't unicast the response to the client when sending NAK, // We can't unicast the response to the client when sending DHCPNAK,
// because we haven't allocated address for him. Therefore, // because we haven't allocated address for him. Therefore,
// NAK is broadcast. // DHCPNAK is broadcast.
} else if (response->getType() == DHCPNAK) { } else if (response->getType() == DHCPNAK) {
response->setRemoteAddr(IOAddress::IPV4_BCAST_ADDRESS()); response->setRemoteAddr(IOAddress::IPV4_BCAST_ADDRESS());

View File

@ -2001,10 +2001,10 @@ DORATest::reservationsWithConflicts() {
ASSERT_NO_THROW(client.doRequest()); ASSERT_NO_THROW(client.doRequest());
// The reservation has been removed. Since address that the client is // The reservation has been removed. Since address that the client is
// using doesn't belong a dynamic pool and the server is not // using doesn't belong to a dynamic pool and the server is not
// authoritative it should not send a DHCPNAK. // authoritative it should not send a DHCPNAK.
resp = client.getContext().response_; resp = client.getContext().response_;
ASSERT_FALSE(client.getContext().response_); ASSERT_FALSE(resp);
// A conforming client would go back to the server discovery. // A conforming client would go back to the server discovery.
client.setState(Dhcp4Client::SELECTING); client.setState(Dhcp4Client::SELECTING);

View File

@ -2040,14 +2040,13 @@ TEST_F(HooksDhcpv4SrvTest, leases4CommittedRequest) {
resetCalloutBuffers(); resetCalloutBuffers();
// Now request an address that can't be allocated. The client should receive // Now request an address that can't be allocated.
// the DHCPNAK.
client.ciaddr_ = IOAddress("10.0.0.1"); client.ciaddr_ = IOAddress("10.0.0.1");
ASSERT_NO_THROW(client.doRequest()); ASSERT_NO_THROW(client.doRequest());
// Make sure that we did not receive a response. If we're // Make sure that we did not receive a response. Since we're
// not authoritative, there should not be a NAK. // not authoritative, there should not be a DHCPNAK.
ASSERT_FALSE(client.getContext().response_); ASSERT_FALSE(client.getContext().response_);
// Check that no callback was not called. // Check that no callback was not called.

View File

@ -3600,7 +3600,7 @@ AllocEngine::findGlobalReservation(ClientContext4& ctx) {
Lease4Ptr Lease4Ptr
AllocEngine::discoverLease4(AllocEngine::ClientContext4& ctx) { AllocEngine::discoverLease4(AllocEngine::ClientContext4& ctx) {
// Find an existing lease for this client. This function will return true // Find an existing lease for this client. This function will return null
// if there is a conflict with existing lease and the allocation should // if there is a conflict with existing lease and the allocation should
// not be continued. // not be continued.
Lease4Ptr client_lease; Lease4Ptr client_lease;
@ -3711,7 +3711,7 @@ AllocEngine::discoverLease4(AllocEngine::ClientContext4& ctx) {
Lease4Ptr Lease4Ptr
AllocEngine::requestLease4(AllocEngine::ClientContext4& ctx) { AllocEngine::requestLease4(AllocEngine::ClientContext4& ctx) {
// Find an existing lease for this client. This function will return true // Find an existing lease for this client. This function will return null
// if there is a conflict with existing lease and the allocation should // if there is a conflict with existing lease and the allocation should
// not be continued. // not be continued.
Lease4Ptr client_lease; Lease4Ptr client_lease;