Skip to content

Commit

Permalink
[routing-manager] stop PD if there is another prefix with higher pref…
Browse files Browse the repository at this point in the history
…erence in netdata (openthread#10289)

This commit adds a new IDLE state to PdPrefixManager.

PdPrefixManager enters idle state when PD is enabled and there is
already a BR requesting PD prefix. When there are multiple BRs
publishing PD prefix at the same time, the one with lexcial smaller
prefix wins.
  • Loading branch information
erjiaqing authored Sep 30, 2024
1 parent 6e46e2e commit 7cd179e
Show file tree
Hide file tree
Showing 10 changed files with 232 additions and 13 deletions.
1 change: 1 addition & 0 deletions include/openthread/border_routing.h
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,7 @@ typedef enum
OT_BORDER_ROUTING_DHCP6_PD_STATE_DISABLED, ///< DHCPv6 PD is disabled on the border router.
OT_BORDER_ROUTING_DHCP6_PD_STATE_STOPPED, ///< DHCPv6 PD in enabled but won't try to request and publish a prefix.
OT_BORDER_ROUTING_DHCP6_PD_STATE_RUNNING, ///< DHCPv6 PD is enabled and will try to request and publish a prefix.
OT_BORDER_ROUTING_DHCP6_PD_STATE_IDLE, ///< DHCPv6 PD is idle; Higher-prf prefix published by other BRs.
} otBorderRoutingDhcp6PdState;

/**
Expand Down
2 changes: 1 addition & 1 deletion include/openthread/instance.h
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ extern "C" {
*
* @note This number versions both OpenThread platform and user APIs.
*/
#define OPENTHREAD_API_VERSION (450)
#define OPENTHREAD_API_VERSION (451)

/**
* @addtogroup api-instance
Expand Down
1 change: 1 addition & 0 deletions script/test
Original file line number Diff line number Diff line change
Expand Up @@ -353,6 +353,7 @@ do_build_otbr_docker()
"-DOT_UPTIME=ON"
"-DOTBR_DNS_UPSTREAM_QUERY=ON"
"-DOTBR_DUA_ROUTING=ON"
"-DOTBR_DHCP6_PD=ON"
)
local args=(
"BORDER_ROUTING=${BORDER_ROUTING}"
Expand Down
3 changes: 3 additions & 0 deletions src/cli/cli_br.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -564,6 +564,7 @@ template <> otError Br::Process<Cmd("pd")>(Arg aArgs[])
"disabled", // (0) OT_BORDER_ROUTING_DHCP6_PD_STATE_DISABLED
"stopped", // (1) OT_BORDER_ROUTING_DHCP6_PD_STATE_STOPPED
"running", // (2) OT_BORDER_ROUTING_DHCP6_PD_STATE_RUNNING
"idle", // (3) OT_BORDER_ROUTING_DHCP6_PD_STATE_IDLE
};

static_assert(0 == OT_BORDER_ROUTING_DHCP6_PD_STATE_DISABLED,
Expand All @@ -572,6 +573,8 @@ template <> otError Br::Process<Cmd("pd")>(Arg aArgs[])
"OT_BORDER_ROUTING_DHCP6_PD_STATE_STOPPED value is not expected!");
static_assert(2 == OT_BORDER_ROUTING_DHCP6_PD_STATE_RUNNING,
"OT_BORDER_ROUTING_DHCP6_PD_STATE_RUNNING value is not expected!");
static_assert(3 == OT_BORDER_ROUTING_DHCP6_PD_STATE_IDLE,
"OT_BORDER_ROUTING_DHCP6_PD_STATE_IDLE value is not expected!");

OutputLine("%s", Stringify(otBorderRoutingDhcp6PdGetState(GetInstancePtr()), kDhcpv6PdStateStrings));
}
Expand Down
43 changes: 38 additions & 5 deletions src/core/border_router/routing_manager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -462,6 +462,9 @@ void RoutingManager::EvaluateRoutingPolicy(void)
#if OPENTHREAD_CONFIG_NAT64_BORDER_ROUTING_ENABLE
mNat64PrefixManager.Evaluate();
#endif
#if OPENTHREAD_CONFIG_BORDER_ROUTING_DHCP6_PD_ENABLE
mPdPrefixManager.Evaluate();
#endif

if (IsInitialPolicyEvaluationDone())
{
Expand Down Expand Up @@ -2291,7 +2294,7 @@ void RoutingManager::OmrPrefixManager::Evaluate(void)
{
RemoveLocalFromNetData();
mLocalPrefix.mPrefix = Get<RoutingManager>().mPdPrefixManager.GetPrefix();
mLocalPrefix.mPreference = RoutePreference::kRoutePreferenceMedium;
mLocalPrefix.mPreference = PdPrefixManager::kPdRoutePreference;
mLocalPrefix.mIsDomainPrefix = false;
LogInfo("Setting local OMR prefix to PD prefix: %s", mLocalPrefix.GetPrefix().ToString().AsCString());
}
Expand Down Expand Up @@ -3845,7 +3848,8 @@ void RoutingManager::RsSender::HandleTimer(void)
RoutingManager::PdPrefixManager::PdPrefixManager(Instance &aInstance)
: InstanceLocator(aInstance)
, mEnabled(false)
, mIsRunning(false)
, mIsStarted(false)
, mIsPaused(false)
, mNumPlatformPioProcessed(0)
, mNumPlatformRaReceived(0)
, mLastPlatformRaTime(0)
Expand All @@ -3869,8 +3873,20 @@ void RoutingManager::PdPrefixManager::StartStop(bool aStart)
{
State oldState = GetState();

VerifyOrExit(aStart != mIsRunning);
mIsRunning = aStart;
VerifyOrExit(aStart != mIsStarted);
mIsStarted = aStart;
EvaluateStateChange(oldState);

exit:
return;
}

void RoutingManager::PdPrefixManager::PauseResume(bool aPause)
{
State oldState = GetState();

VerifyOrExit(aPause != mIsPaused);
mIsPaused = aPause;
EvaluateStateChange(oldState);

exit:
Expand All @@ -3883,12 +3899,22 @@ RoutingManager::PdPrefixManager::State RoutingManager::PdPrefixManager::GetState

if (mEnabled)
{
state = mIsRunning ? kDhcp6PdStateRunning : kDhcp6PdStateStopped;
state = mIsStarted ? (mIsPaused ? kDhcp6PdStateIdle : kDhcp6PdStateRunning) : kDhcp6PdStateStopped;
}

return state;
}

void RoutingManager::PdPrefixManager::Evaluate(void)
{
const FavoredOmrPrefix &favoredPrefix = Get<RoutingManager>().mOmrPrefixManager.GetFavoredPrefix();

bool shouldPause = !favoredPrefix.IsEmpty() && favoredPrefix.GetPrefix() != mPrefix.GetPrefix() &&
favoredPrefix.GetPreference() >= kPdRoutePreference;

PauseResume(/* aPause */ shouldPause);
}

void RoutingManager::PdPrefixManager::EvaluateStateChange(Dhcp6PdState aOldState)
{
State newState = GetState();
Expand All @@ -3900,12 +3926,17 @@ void RoutingManager::PdPrefixManager::EvaluateStateChange(Dhcp6PdState aOldState
{
case kDhcp6PdStateDisabled:
case kDhcp6PdStateStopped:
case kDhcp6PdStateIdle:
WithdrawPrefix();
break;
case kDhcp6PdStateRunning:
break;
}

// When the prefix is replaced, there will be a short period when the old prefix is still in the netdata, and PD
// manager will refuse to request the prefix.
// TODO: Either update the comment for the state callback or add a random delay when notifing the upper layer for
// state change.
mStateCallback.InvokeIfSet(static_cast<otBorderRoutingDhcp6PdState>(newState));

exit:
Expand Down Expand Up @@ -4129,11 +4160,13 @@ const char *RoutingManager::PdPrefixManager::StateToString(State aState)
"Disabled", // (0) kDisabled
"Stopped", // (1) kStopped
"Running", // (2) kRunning
"Idle", // (3) kIdle
};

static_assert(0 == kDhcp6PdStateDisabled, "kDhcp6PdStateDisabled value is incorrect");
static_assert(1 == kDhcp6PdStateStopped, "kDhcp6PdStateStopped value is incorrect");
static_assert(2 == kDhcp6PdStateRunning, "kDhcp6PdStateRunning value is incorrect");
static_assert(3 == kDhcp6PdStateIdle, "kDhcp6PdStateIdle value is incorrect");

return kStateStrings[aState];
}
Expand Down
11 changes: 9 additions & 2 deletions src/core/border_router/routing_manager.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,7 @@ class RoutingManager : public InstanceLocator
kDhcp6PdStateDisabled = OT_BORDER_ROUTING_DHCP6_PD_STATE_DISABLED, ///< Disabled.
kDhcp6PdStateStopped = OT_BORDER_ROUTING_DHCP6_PD_STATE_STOPPED, ///< Enabled, but currently stopped.
kDhcp6PdStateRunning = OT_BORDER_ROUTING_DHCP6_PD_STATE_RUNNING, ///< Enabled, and running.
kDhcp6PdStateIdle = OT_BORDER_ROUTING_DHCP6_PD_STATE_IDLE, ///< Enabled, but is not requesting prefix.
};

/**
Expand Down Expand Up @@ -1457,6 +1458,8 @@ class RoutingManager : public InstanceLocator

typedef Dhcp6PdState State;

static constexpr RoutePreference kPdRoutePreference = RoutePreference::kRoutePreferenceMedium;

explicit PdPrefixManager(Instance &aInstance);

void SetEnabled(bool aEnabled);
Expand All @@ -1473,6 +1476,7 @@ class RoutingManager : public InstanceLocator
Error GetProcessedRaInfo(PdProcessedRaInfo &aPdProcessedRaInfo) const;
void HandleTimer(void) { WithdrawPrefix(); }
void SetStateCallback(PdCallback aCallback, void *aContext) { mStateCallback.Set(aCallback, aContext); }
void Evaluate(void);

private:
class PrefixEntry : public OnLinkPrefix
Expand All @@ -1489,14 +1493,17 @@ class RoutingManager : public InstanceLocator
void EvaluateStateChange(State aOldState);
void WithdrawPrefix(void);
void StartStop(bool aStart);
void PauseResume(bool aPause);

static const char *StateToString(State aState);

using PrefixTimer = TimerMilliIn<RoutingManager, &RoutingManager::HandlePdPrefixManagerTimer>;
using StateCallback = Callback<PdCallback>;

bool mEnabled;
bool mIsRunning;
bool mEnabled; // Whether PdPrefixManager is enabled. (guards the overall PdPrefixManager functions)
bool mIsStarted; // Whether PdPrefixManager is started. (monitoring prefixes)
bool mIsPaused; // Whether PdPrefixManager is paused. (when there is another BR advertising another prefix)

uint32_t mNumPlatformPioProcessed;
uint32_t mNumPlatformRaReceived;
TimeMilli mLastPlatformRaTime;
Expand Down
9 changes: 5 additions & 4 deletions src/posix/platform/netif.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1095,7 +1095,8 @@ static void processReceive(otMessage *aMessage, void *aContext)
}
}

#if OPENTHREAD_CONFIG_NAT64_TRANSLATOR_ENABLE || OPENTHREAD_CONFIG_BORDER_ROUTING_DHCP6_PD_ENABLE
#if OPENTHREAD_CONFIG_NAT64_TRANSLATOR_ENABLE || \
(OPENTHREAD_CONFIG_BORDER_ROUTING_ENABLE && OPENTHREAD_CONFIG_BORDER_ROUTING_DHCP6_PD_ENABLE)
static constexpr uint8_t kIpVersion4 = 4;
static constexpr uint8_t kIpVersion6 = 6;

Expand All @@ -1111,7 +1112,7 @@ static uint8_t getIpVersion(const uint8_t *data)
}
#endif

#if OPENTHREAD_CONFIG_BORDER_ROUTING_DHCP6_PD_ENABLE
#if OPENTHREAD_CONFIG_BORDER_ROUTING_ENABLE && OPENTHREAD_CONFIG_BORDER_ROUTING_DHCP6_PD_ENABLE

/**
* Returns nullptr if data does not point to a valid ICMPv6 RA message.
Expand Down Expand Up @@ -1156,7 +1157,7 @@ static otError tryProcessIcmp6RaMessage(otInstance *aInstance, const uint8_t *da
exit:
return error;
}
#endif // OPENTHREAD_CONFIG_BORDER_ROUTING_DHCP6_PD_ENABLE
#endif // OPENTHREAD_CONFIG_BORDER_ROUTING_ENABLE && OPENTHREAD_CONFIG_BORDER_ROUTING_DHCP6_PD_ENABLE

#ifdef __linux__
/**
Expand Down Expand Up @@ -1222,7 +1223,7 @@ static void processTransmit(otInstance *aInstance)
}
#endif

#if OPENTHREAD_CONFIG_BORDER_ROUTING_DHCP6_PD_ENABLE
#if OPENTHREAD_CONFIG_BORDER_ROUTING_ENABLE && OPENTHREAD_CONFIG_BORDER_ROUTING_DHCP6_PD_ENABLE
if (tryProcessIcmp6RaMessage(aInstance, reinterpret_cast<uint8_t *>(&packet[offset]), rval) == OT_ERROR_NONE)
{
ExitNow();
Expand Down
124 changes: 124 additions & 0 deletions tests/scripts/thread-cert/border_router/test_dhcp6pd.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,124 @@
#!/usr/bin/env python3
#
# Copyright (c) 2024, The OpenThread Authors.
# All rights reserved.
#
# Redistribution and use in source and binary forms, with or without
# modification, are permitted provided that the following conditions are met:
# 1. Redistributions of source code must retain the above copyright
# notice, this list of conditions and the following disclaimer.
# 2. Redistributions in binary form must reproduce the above copyright
# notice, this list of conditions and the following disclaimer in the
# documentation and/or other materials provided with the distribution.
# 3. Neither the name of the copyright holder nor the
# names of its contributors may be used to endorse or promote products
# derived from this software without specific prior written permission.
#
# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
# AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
# IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
# ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE
# LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
# CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
# SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
# INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
# CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
# ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
# POSSIBILITY OF SUCH DAMAGE.
#
import ipaddress
import typing
import unittest

import config
import thread_cert

# Test description:
#
# This test verifies DHCP6 PD pauses requesting PD prefix when there is another BR advertising PD prefix.
#

LEADER = 1
ROUTER = 2


class TestDhcp6Pd(thread_cert.TestCase):
SUPPORT_NCP = False
USE_MESSAGE_FACTORY = False

TOPOLOGY = {
LEADER: {
'name': 'Leader',
'allowlist': [ROUTER],
'is_otbr': True,
'mode': 'rdn',
},
ROUTER: {
'name': 'Router',
'allowlist': [LEADER],
'is_otbr': True,
'mode': 'rdn',
},
}

def test(self):
leader = self.nodes[LEADER]
router = self.nodes[ROUTER]

#---------------------------------------------------------------
# Start the server & client devices.

# Case 1: Only 1 BR receives PD prefix

leader.start()
self.simulator.go(config.LEADER_STARTUP_DELAY)
self.assertEqual(leader.get_state(), 'leader')
leader.pd_set_enabled(True)

router.start()
self.simulator.go(config.ROUTER_STARTUP_DELAY)
self.assertEqual(router.get_state(), 'router')
router.pd_set_enabled(True)

leader.start_pd_radvd_service("2001:db8:abcd:1234::/64")
self.simulator.go(30)

self.assertSetEqual({leader.pd_state, router.pd_state}, {"running", "idle"})

self.assertEqual(leader.pd_get_prefix(), "2001:db8:abcd:1234::/64")

# Case 2: More than one BR receives PD prefix
# The BR with "smaller" PD prefix wins

# Clean up and ensure no prefix is currently published.
leader.pd_set_enabled(False)
router.pd_set_enabled(False)
self.simulator.go(30)

leader.start_pd_radvd_service("2001:db8:abcd:1234::/64")
router.start_pd_radvd_service("2001:db8:1234:abcd::/64")
leader.pd_set_enabled(True)
router.pd_set_enabled(True)
self.simulator.go(30)

self.assertSetEqual({leader.pd_state, router.pd_state}, {"running", "idle"})

# Case 3: When the other BR lost PD prefix, the remaining BR should try to request one.

if leader.pd_state == 'running':
br_to_stop = leader
br_to_continue = router
expected_prefix = "2001:db8:1234:abcd::/64"
else:
br_to_stop = router
br_to_continue = leader
expected_prefix = "2001:db8:abcd:1234::/64"
br_to_stop.pd_set_enabled(False)
self.simulator.go(30)

self.assertEqual(br_to_continue.pd_state, "running")
self.assertEqual(br_to_continue.pd_get_prefix(), expected_prefix)


if __name__ == '__main__':
unittest.main()
Loading

0 comments on commit 7cd179e

Please sign in to comment.