Skip to content

[dbus] add dbus API to set border agent Id #3136

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.

Sign up for GitHub

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

Draft
Irving-cl wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

[dbus] add dbus API to set border agent Id #3136

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions etc/cmake/options.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,11 @@ if (OTBR_BORDER_AGENT_MESHCOP_SERVICE)
target_compile_definitions(otbr-config INTERFACE OTBR_ENABLE_BORDER_AGENT_MESHCOP_SERVICE=1)
endif()

option(OTBR_BORDER_AGENT_ID "Enable Border Agent ID support" ON)

@librasungirl librasungirl Nov 21, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

is it necessary?

@Irving-cl Irving-cl Nov 21, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Since openthread has an option OT_BORDER_AGENT_ID to turn on/off Border Agent Id, I think it's necessary to add an option in ot-br-posix as well to avoid the case where OT_BORDER_AGENT_ID is OFF.

BTW, I'm thinking if border agent id is a MUST, why not removing the macro in openthread as well. Thoughts?

@librasungirl librasungirl Nov 21, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It's indeed MUST requirement per specification,

TXT record inclusion is REQUIRED. The ID MUST remain identical if the device reboots. It MUST NOT persist after a BR factory-reset. It MUST be computed using a random function to minimize the ID collision probability.

But the spec doesn't define who is to maintain it. OT core provides the option to help generate/set/store it and when OT is responsible for meshcop service, OPENTHREAD_CONFIG_BORDER_AGENT_MESHCOP_SERVICE_ENABLE, it will report error if it's not ON.

https://github.com/openthread/openthread/blob/main/src/core/meshcop/border_agent.hpp#L80

For BR, application can specify it via UpdateMeshCop UpdateMeshCopTxt or use this new dbus API you are introducing if it want to manage the id

if (OTBR_BORDER_AGENT_ID)
target_compile_definitions(otbr-config INTERFACE OTBR_ENABLE_BORDER_AGENT_ID=1)
endif()

option(OTBR_BACKBONE_ROUTER "Enable Backbone Router" ON)
if (OTBR_BACKBONE_ROUTER)
target_compile_definitions(otbr-config INTERFACE OTBR_ENABLE_BACKBONE_ROUTER=1)
Expand Down
5 changes: 5 additions & 0 deletions src/dbus/client/thread_api_dbus.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -893,6 +893,11 @@ ClientError ThreadApiDBus::SetBorderAgentEnabled(bool aEnabled)
return CallDBusMethodSync(OTBR_DBUS_SET_BORDER_AGENT_ENABLED_METHOD, std::tie(aEnabled));
}

ClientError ThreadApiDBus::SetBorderAgentId(const std::vector<uint8_t> &aBorderAgentId)
{
return SetProperty(OTBR_DBUS_PROPERTY_BORDER_AGENT_ID, aBorderAgentId);
}
Comment thread
Irving-cl marked this conversation as resolved.

ClientError ThreadApiDBus::UpdateVendorMeshCopTxtEntries(std::vector<TxtEntry> &aUpdate)
{
auto args = std::tie(aUpdate);
Expand Down
11 changes: 11 additions & 0 deletions src/dbus/client/thread_api_dbus.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -800,6 +800,17 @@ class ThreadApiDBus
*/
ClientError SetBorderAgentEnabled(bool aEnabled);

/**
* This method sets the 16 byte Border Agent Id.

@librasungirl librasungirl Nov 21, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

pls add notes as https://github.com/openthread/openthread/blob/main/include/openthread/border_agent.h#L340 and highlight this API should be used with awareness and caution.

*
* @param[in] aBorderAgentId The Border Agent Id.
*
* @retval ERROR_NONE Successfully performed the dbus function call
* @retval ERROR_DBUS dbus encode/decode error
* @retval ... OpenThread defined error value otherwise
*/
ClientError SetBorderAgentId(const std::vector<uint8_t> &aBorderAgentId);

/**
* This method sets multiple vendor-specific entries for the TXT record of the MeshCoP service.
*
Expand Down
25 changes: 25 additions & 0 deletions src/dbus/server/dbus_thread_object_rcp.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,10 @@ otbrError DBusThreadObjectRcp::Init(void)
std::bind(&DBusThreadObjectRcp::SetDnsUpstreamQueryState, this, _1));
RegisterSetPropertyHandler(OTBR_DBUS_THREAD_INTERFACE, OTBR_DBUS_PROPERTY_NAT64_CIDR,
std::bind(&DBusThreadObjectRcp::SetNat64Cidr, this, _1));
#if OTBR_ENABLE_BORDER_AGENT_ID
RegisterSetPropertyHandler(OTBR_DBUS_THREAD_INTERFACE, OTBR_DBUS_PROPERTY_BORDER_AGENT_ID,
std::bind(&DBusThreadObjectRcp::SetBorderAgentIdHandler, this, _1));
#endif
#if OTBR_ENABLE_EPSKC
RegisterSetPropertyHandler(OTBR_DBUS_THREAD_INTERFACE, OTBR_DBUS_PROPERTY_EPHEMERAL_KEY_ENABLED,
std::bind(&DBusThreadObjectRcp::SetEphemeralKeyEnabled, this, _1));
Expand Down Expand Up @@ -223,8 +227,10 @@ otbrError DBusThreadObjectRcp::Init(void)
std::bind(&DBusThreadObjectRcp::GetRloc16Handler, this, _1));
RegisterGetPropertyHandler(OTBR_DBUS_THREAD_INTERFACE, OTBR_DBUS_PROPERTY_EXTENDED_ADDRESS,
std::bind(&DBusThreadObjectRcp::GetExtendedAddressHandler, this, _1));
#if OTBR_ENABLE_BORDER_AGENT_ID
RegisterGetPropertyHandler(OTBR_DBUS_THREAD_INTERFACE, OTBR_DBUS_PROPERTY_BORDER_AGENT_ID,
std::bind(&DBusThreadObjectRcp::GetBorderAgentIdHandler, this, _1));
#endif
RegisterGetPropertyHandler(OTBR_DBUS_THREAD_INTERFACE, OTBR_DBUS_PROPERTY_ROUTER_ID,
std::bind(&DBusThreadObjectRcp::GetRouterIdHandler, this, _1));
RegisterGetPropertyHandler(OTBR_DBUS_THREAD_INTERFACE, OTBR_DBUS_PROPERTY_LEADER_DATA,
Expand Down Expand Up @@ -1308,6 +1314,23 @@ otError DBusThreadObjectRcp::SetRadioRegionHandler(DBusMessageIter &aIter)
return error;
}

#if OTBR_ENABLE_BORDER_AGENT_ID
otError DBusThreadObjectRcp::SetBorderAgentIdHandler(DBusMessageIter &aIter)
{
otError error = OT_ERROR_NONE;
std::array<uint8_t, OT_BORDER_AGENT_ID_LENGTH> data;
otBorderAgentId borderAgentId;

VerifyOrExit(DBusMessageExtractFromVariant(&aIter, data) == OTBR_ERROR_NONE, error = OT_ERROR_INVALID_ARGS);
std::copy(data.begin(), data.end(), borderAgentId.mId);

error = otBorderAgentSetId(mHost.GetInstance(), &borderAgentId);

@zhanglongxia zhanglongxia Nov 20, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Will this conflict with the #3132?

@Irving-cl Irving-cl Nov 20, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I didn't sync code so didn't see #3132 yesterday.

#3132 can work but it seems a little unnatural to me to set it in UpdateMeshCopTxtHandler because UpdateMeshCopTxtHandler is used to update the external fields of the meshcop txt records while borderAgentId is an internal field (though it can be set by an API). The fields set in UpdateMeshCopTxtHandler will be taken to integrate into the txt data that provided by OT. But borderAgentId shouldn't be handled like that here because it's already in the txt data provided by OT. And in current code, a workaround is taken to do so. So it's like, we avail this API UpdateMeshCopTxtHandler and call a OT API here.

So I prefer having a dedicated API to set border agent Id. This PR will ONLY conflict with #3132 when both APIs are called to set the border agent Id. If we:

  • Set all vendor fields and border agent Id in UpdateMeshCopTxtHandler, it's ok
  • Set all vendor fields except border agent Id in UpdateMeshCopTxtHandler and set border agent id using this dedicated API, it's also ok.

So we can have both PRs.

@zhanglongxia zhanglongxia Nov 20, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Based on the description of the function otBorderAgentSetVendorTxtData(), the vendor-specific Key-Value TXT data pairs use TXT keys starting with v. The function UpdateMeshCopTxtHandler() is used to process the vendor TXT data. The key id is not the vendor-specific Key. I'm fine with this PR.

@librasungirl librasungirl Nov 21, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I am not sure about it.

As from spec An ID to uniquely identify the device hosting the Border Agent among multiple such devices., if manufacture want to manage ID themselves, it might also be vendor specific data.

Overall it's fine to add a separate dbus API but it should be well documented as in OT, people should be aware of the requirement of id by spec

The ID MUST remain identical if the device reboots. It MUST NOT persist after a BR factory-reset. It MUST be computed using a random function to minimize the ID collision probability.


exit:
return error;
}
#endif

#if OTBR_ENABLE_BORDER_AGENT
void DBusThreadObjectRcp::SetBorderAgentEnabledHandler(DBusRequest &aRequest)
{
Expand Down Expand Up @@ -1664,6 +1687,7 @@ otError DBusThreadObjectRcp::GetEui64Handler(DBusMessageIter &aIter)
return error;
}

#if OTBR_ENABLE_BORDER_AGENT_ID
otError DBusThreadObjectRcp::GetBorderAgentIdHandler(DBusMessageIter &aIter)
{
otBorderAgentId id;
Expand All @@ -1679,6 +1703,7 @@ otError DBusThreadObjectRcp::GetBorderAgentIdHandler(DBusMessageIter &aIter)
exit:
return error;
}
#endif

otError DBusThreadObjectRcp::GetOtRcpVersionHandler(DBusMessageIter &aIter)
{
Expand Down
5 changes: 5 additions & 0 deletions src/dbus/server/dbus_thread_object_rcp.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,9 @@ class DBusThreadObjectRcp : public DBusObject
otError SetRadioRegionHandler(DBusMessageIter &aIter);
otError SetDnsUpstreamQueryState(DBusMessageIter &aIter);
otError SetNat64Cidr(DBusMessageIter &aIter);
#if OTBR_ENABLE_BORDER_AGENT_ID
otError SetBorderAgentIdHandler(DBusMessageIter &aIter);
#endif
#if OTBR_ENABLE_EPSKC
otError SetEphemeralKeyEnabled(DBusMessageIter &aIter);
#endif
Expand All @@ -133,7 +136,9 @@ class DBusThreadObjectRcp : public DBusObject
otError GetPanIdHandler(DBusMessageIter &aIter);
otError GetExtPanIdHandler(DBusMessageIter &aIter);
otError GetEui64Handler(DBusMessageIter &aIter);
#if OTBR_ENABLE_BORDER_AGENT_ID
otError GetBorderAgentIdHandler(DBusMessageIter &aIter);
#endif
otError GetChannelHandler(DBusMessageIter &aIter);
otError GetNetworkKeyHandler(DBusMessageIter &aIter);
otError GetCcaFailureRateHandler(DBusMessageIter &aIter);
Expand Down
12 changes: 12 additions & 0 deletions tests/dbus/test_dbus_client.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -287,6 +287,18 @@ void CheckBorderAgent(ThreadApiDBus *aApi)
{
TEST_ASSERT(aApi->SetBorderAgentEnabled(false) == OTBR_ERROR_NONE);
TEST_ASSERT(aApi->SetBorderAgentEnabled(true) == OTBR_ERROR_NONE);

#if OTBR_ENABLE_BORDER_AGENT_ID
std::vector<uint8_t> borderAgentId = {0x00, 0x11, 0x22, 0x33, 0x44, 0x55, 0x66, 0x77,
0x88, 0x99, 0xaa, 0xbb, 0xcc, 0xdd, 0xee, 0xff};
std::vector<uint8_t> invalidBorderAgentId = {0x00};
std::vector<uint8_t> responseBorderAgentId;

TEST_ASSERT(aApi->SetBorderAgentId(invalidBorderAgentId) != OTBR_ERROR_NONE);
TEST_ASSERT(aApi->SetBorderAgentId(borderAgentId) == OTBR_ERROR_NONE);
TEST_ASSERT(aApi->GetBorderAgentId(responseBorderAgentId) == OTBR_ERROR_NONE);
TEST_ASSERT(borderAgentId == responseBorderAgentId);
#endif
}

#if OTBR_ENABLE_TELEMETRY_DATA_API
Expand Down
2 changes: 1 addition & 1 deletion third_party/openthread/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ set(OT_BACKBONE_ROUTER ${OTBR_BACKBONE_ROUTER} CACHE STRING "Enable Backbone Rou
set(OT_BACKBONE_ROUTER_DUA_NDPROXYING ${OTBR_DUA_ROUTING} CACHE STRING "Configure DUA ND Proxy feature in OpenThread" FORCE)
set(OT_BORDER_AGENT ON CACHE STRING "enable border agent" FORCE)
set(OT_BORDER_AGENT_EPSKC ON CACHE STRING "enable border agent ephemeral PSKc" FORCE)
set(OT_BORDER_AGENT_ID ON CACHE STRING "enable border agent ID" FORCE)
set(OT_BORDER_AGENT_ID ${OTBR_BORDER_AGENT_ID} CACHE STRING "enable border agent ID" FORCE)
set(OT_BORDER_ROUTER ON CACHE STRING "enable border router feature" FORCE)
set(OT_BORDER_ROUTING ${OTBR_BORDER_ROUTING} CACHE STRING "enable border routing feature" FORCE)
set(OT_BORDER_ROUTING_COUNTERS ${OTBR_BORDER_ROUTING_COUNTERS} CACHE STRING "enable border routing counters feature" FORCE)
Expand Down
Loading