mctpd: add AssignBridgeStatic method by potinlai · Pull Request #148 · CodeConstruct/mctp

potinlai · 2026-03-19T02:04:43Z

Add AssignBridgeStatic method to assign static EID with bridge pool allocation.

Tested:

root@bmc:~# busctl call au.com.codeconstruct.MCTP1 /au/com/codeconstruct/mctp1/interfaces/mcu1u2u1u4u1 au.com.codeconstruct.MCTP.BusOwner1 AssignBridgeStatic ayyyy 0 30 5 31
yisb 30 1 "/au/com/codeconstruct/mctp1/networks/1/endpoints/30" true
root@bmc:~# busctl introspect au.com.codeconstruct.MCTP1 /au/com/codeconstruct/mctp1/networks/1/endpoints/30
NAME                                TYPE      SIGNATURE RESULT/VALUE                           FLAGS
au.com.codeconstruct.MCTP.Bridge1   interface -         -                                      -
.PoolEnd                            property  y         35                                     const
.PoolStart                          property  y         31                                     const
au.com.codeconstruct.MCTP.Endpoint1 interface -         -                                      -
.Recover                            method    -         -                                      -
.Remove                             method    -         -                                      -
.SetMTU                             method    u         -                                      -
.Connectivity                       property  s         "Available"                            emits-change
org.freedesktop.DBus.Introspectable interface -         -                                      -
.Introspect                         method    -         s                                      -
org.freedesktop.DBus.Peer           interface -         -                                      -
.GetMachineId                       method    -         s                                      -
.Ping                               method    -         -                                      -
org.freedesktop.DBus.Properties     interface -         -                                      -
.Get                                method    ss        v                                      -
.GetAll                             method    s         a{sv}                                  -
.Set                                method    ssv       -                                      -
.PropertiesChanged                  signal    sa{sv}as  -                                      -
xyz.openbmc_project.Common.UUID     interface -         -                                      -
.UUID                               property  s         "8b7e3d86-5fa5-298c-a9d6-a2d5d4baa6fa" const
xyz.openbmc_project.MCTP.Endpoint   interface -         -                                      -
.EID                                property  y         30                                     const
.NetworkId                          property  u         1                                      const
.SupportedMessageTypes              property  ay        4 1 5 126 127                          const
.VendorDefinedMessageTypes          property  a(yvq)    0                                      const

Signed-off-by: Potin Lai potin.lai@quantatw.com

jk-ozlabs · 2026-03-19T02:09:41Z

I'm going to need more context than that commit message. What is the aim here?

jk-ozlabs · 2026-03-19T02:25:27Z

+	if (static_eid) {
+		new_eid = static_eid;
+	} else {
 		new_eid = alloc.start;
+	}


You can't just redefine the allocated range like this, those EIDs at [static_eid, static_eid + alloc.extent] may not be available.

Hi @jk-ozlabs,
Thank you for your suggestion in Discord.
I have updated the change to introduce a new method called AssignBridgeStatic.

jk-ozlabs

A few things inline, and the commit message needs more explanation. Rather than a chunk of 'Tested' output, can you include some details about the use case and what is implemented here?

jk-ozlabs · 2026-03-27T00:32:52Z

+
+	peer->pool_size = pool_size;
+	if (peer->pool_size)
+		peer->pool_start = pool_start;


How are you ensuring that this does not conflict with existing allocations?

Also, check that pool_start == static_eid + 1 here, in order to be compliant with the spec.

jk-ozlabs · 2026-03-27T00:37:52Z

+		}
+
+		// Return existing record.
+		peer_path = path_from_peer(peer);


Is this valid here, and is the check sufficient? The existing peer may not be the bridge you expect...

jk-ozlabs · 2026-03-27T03:02:43Z

Also, could you add some test cases here? We want to check the successful call, but also conflicts with existing EID allocations.

Add AssignBridgeStatic method to assign static EID with bridge pool allocation. Tested: ``` root@bmc:~# busctl call au.com.codeconstruct.MCTP1 /au/com/codeconstruct/mctp1/interfaces/mcu1u2u1u4u1 au.com.codeconstruct.MCTP.BusOwner1 AssignBridgeStatic ayyyy 0 30 31 5 yisb 30 1 "/au/com/codeconstruct/mctp1/networks/1/endpoints/30" true root@bmc:~# busctl introspect au.com.codeconstruct.MCTP1 /au/com/codeconstruct/mctp1/networks/1/endpoints/30 NAME TYPE SIGNATURE RESULT/VALUE FLAGS au.com.codeconstruct.MCTP.Bridge1 interface - - - .PoolEnd property y 35 const .PoolStart property y 31 const au.com.codeconstruct.MCTP.Endpoint1 interface - - - .Recover method - - - .Remove method - - - .SetMTU method u - - .Connectivity property s "Available" emits-change org.freedesktop.DBus.Introspectable interface - - - .Introspect method - s - org.freedesktop.DBus.Peer interface - - - .GetMachineId method - s - .Ping method - - - org.freedesktop.DBus.Properties interface - - - .Get method ss v - .GetAll method s a{sv} - .Set method ssv - - .PropertiesChanged signal sa{sv}as - - xyz.openbmc_project.Common.UUID interface - - - .UUID property s "8b7e3d86-5fa5-298c-a9d6-a2d5d4baa6fa" const xyz.openbmc_project.MCTP.Endpoint interface - - - .EID property y 30 const .NetworkId property u 1 const .SupportedMessageTypes property ay 4 1 5 126 127 const .VendorDefinedMessageTypes property a(yvq) 0 const ``` Signed-off-by: Potin Lai <potin.lai@quantatw.com>

potinlai · 2026-04-20T16:38:45Z

Also, could you add some test cases here? We want to check the successful call, but also conflicts with existing EID allocations.

Hi @jk-ozlabs,
I’m still trying to understand how to run the test cases. I copied the test-mctpd binary to the BMC and executed it, but I encountered the following error:

root@bmc:~# /tmp/test-mctpd
test-mctpd: No MCTP_TEST_SOCK fd provided

Could you please advise on the correct way to run this test?

jk-ozlabs · 2026-04-21T01:33:37Z

The binary doesn't need to run on the BMC; the idea behind the test infrastructure is that the test-mctpd build does not have any dependencies on the system MCTP stack (and is designed to run where there is no MCTP capabilities on the system)

The Testing section in the README has some details, but you just run the tests locally using something like:

$ python3 -m venv env
$ ./env/bin/pip install -r tests/requirements.txt
$ export PATH=$PWD/env/bin:$PATH
$ meson setup obj
$ ninja -C obj test

faizana-nvidia · 2026-04-22T04:25:13Z

+```
+
+`<pool-start-EID>`: The starting EID for the range of EIDs being allocated.
+This EID must be contiguous with the bridge’s own EID.


Hello @jk-ozlabs,

Has there been any plan to consider allocation of pool with non contiguous pool start (w.r.t bridge's own EID)? may be while moving to v.14 DSP0236?
#71 (comment)
https://github.com/DMTF/PMCI-WG/issues/1540

It's something I would like to support, but no specific plans at the moment.

Note that this would only be possible after confirming the MCTP implementation version of the bridge device though - which may complicate the external API, since the success/failure of a non-contiguous allocation call would depend on the remote endpoint version. So, it's probably not a big win at the moment, as we already implement the contiguity restriction anyway.

Thank you for the clarification, I suppose this PR will give option to choose variable pool size allocation incase ask is to have pool less/equal to what bridge pool size is (response from SET ENDPOINT ID)

This does bring up a good point though - the new API (introduced by this change) would be somewhat difficult to use when we want to implement split pools, as the caller may not know in advance if the call is valid.

I see a couple of options:

We remove the pool-start argument, and let mctpd specify the pool start address; or

We allow a 0 value for pool-start, indicating that mctpd can put the pool range whereever it likes

The downside of (1) is that callers may require the pool range to be predefined, for whatever reason.

@potinlai can you provide some details on your need for the static EID allocation, perhaps? Do you need the pool range to be static, or only the bridge EID?

potinlai force-pushed the main branch from 5890115 to 415b574 Compare March 19, 2026 02:06

jk-ozlabs reviewed Mar 19, 2026

View reviewed changes

potinlai force-pushed the main branch from 415b574 to 76714d4 Compare March 26, 2026 11:47

potinlai changed the title ~~mctpd: allow bridge for assign static endpoint~~ mctpd: add AssignBridgeStatic method Mar 26, 2026

potinlai force-pushed the main branch from 76714d4 to 8fde5bb Compare March 26, 2026 12:22

jk-ozlabs requested changes Mar 27, 2026

View reviewed changes

potinlai force-pushed the main branch from 8fde5bb to 6f47f6e Compare April 20, 2026 16:13

faizana-nvidia reviewed Apr 22, 2026

View reviewed changes

Conversation

potinlai commented Mar 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jk-ozlabs commented Mar 19, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jk-ozlabs left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jk-ozlabs commented Mar 27, 2026

Uh oh!

potinlai commented Apr 20, 2026

Uh oh!

jk-ozlabs commented Apr 21, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

potinlai commented Mar 19, 2026 •

edited

Loading