CodeConstruct / mctp

MCTP userspace tools
GNU General Public License v2.0
33 stars 19 forks source link

mctpd: `AllocateEndpoints` implementation #46

Open PeterHo-wiwynn opened 4 months ago

PeterHo-wiwynn commented 4 months ago

We have a system which has devices at lower bus hierarchy.

As descibed in DSP0236, Bus owners are responsible for allocating pools of EIDs to MCTP bridges that are lower in the bus hierarchy. This is done using the Allocate Endpoint IDs command..

The sequence will be like:

sequenceDiagram
    BMC->>MCTP_bridge: SetEndpoint
    MCTP_bridge->>BMC: Endpoint requires EID pool allocation.
    BMC->>MCTP_bridge: AllocateEndpoints
    MCTP_bridge ->>MCTP_devices: SetEndpoint

The BMC need to send Starting EID and Number of EIDs. I think we can send next available EID as the Starting EID when we call AssignStaticEndpoint. (e.g. AssignStaticEndpoint with 10, and we give 11 as Starting EID) However, the Number of EIDs is hard to define a specific number. Moreover, for the dynamic EID, it's difficult to choose which number we should send as the Starting EID. If EID 8 and 10 is occupied, should we send Starting EID 9 and 1 as the Number of EIDs?

We might need to clarify how we implement AllocateEndpoints in mctpd.

jk-ozlabs commented 4 months ago

All sounds good. We'll likely need some configuration to assign these pools of EIDs to the downstream bridges.

First impressions for this may involve @amboar 's work on the OpenBMC mctpreactor to control the configuration of these, which then sends the pool data to mctpd via dbus, on discovery of these bridge devices.

So, we'll need to define the appropriate place in the dbus hierarchy to represent the EID pool data.

jk-ozlabs commented 4 months ago

Although, given that we have the (requested) pool size as part of the Set Endpoint ID response, we can just allocate that directly from the existing dynamic EID pool, and not need any extra configuration. Would that work for you?

We could later add limits and pre-defined allocations from the dynamic pool if necessary, but that doesn't seem to be required at the moment.

PeterHo-wiwynn commented 4 months ago

Although, given that we have the (requested) pool size as part of the Set Endpoint ID response, we can just allocate that directly from the existing dynamic EID pool, and not need any extra configuration. Would that work for you?

Oh, I missed that part. This makes sense now. I think I can start implementing it, and will make a PR when it's done.

santoshpuranik commented 1 week ago

@jk-ozlabs , @PeterHo-wiwynn : At NVIDIA, we are also looking at enabling bridge support with mctpd. Wondering if there is work started on this already? I wanted to get some understanding on how that design/D-Bus API would look like.

As previous comments on this issue already point out, an endpoint can tell the TBO that it is a bridge and requires pool EID allocation as a part of the response to SetEndpointID.

In case the EID assignments on the platform are dynamic, mctpd can simply call AllocateEndpointIDs with the next available EID (and reserve that EID + pool size).

However, there are platforms that desire setting fixed EIDs as the TBO. For such a use case, do you think it is reasonable to modify the AssignEndpointStatic D-Bus API to take an additional parameter (pool EID start) in addition to the EID parameter it takes today? Alternatively, we could define a new D-Bus API for bridges?

Can this EID and pool start parameter be supplied as an entity-manager configuration to the MCTP reactor?

Further, after the EID pool is assigned to the bridge, mctpd should query for the routing table from the bridge (GetRoutingTableEntries) and use the responses to setup routes and host the downstream EIDs as D-Bus objects. Does this approach seem right?

We would be happy to make PRs to add this function if you agree with this design/once we decide on the right design.

amboar commented 1 week ago

mctpd should query for the routing table from the bridge (GetRoutingTableEntries) and use the responses to setup routes and host the downstream EIDs as D-Bus objects. Does this approach seem right?

My understanding is Get Routing Table Entries should not be required for network maintenance operations. Rather, it's a debugging tool provided by the spec (DSP0236). If a design requires its use for non-debugging purposes, my hunch is that the design needs some more consideration.

santoshpuranik commented 1 week ago

mctpd should query for the routing table from the bridge (GetRoutingTableEntries) and use the responses to setup routes and host the downstream EIDs as D-Bus objects. Does this approach seem right?

My understanding is Get Routing Table Entries should not be required for network maintenance operations. Rather, it's a debugging tool provided by the spec (DSP0236). If a design requires its use for non-debugging purposes, my hunch is that the design needs some more consideration.

Almost none of the MCTP control commands are marked "mandatory to generate" in Table 12 of the spec. :) So in that sense, yes, Get Routing Table Entries should not be required.

However, the spec. (https://www.dmtf.org/sites/default/files/standards/documents/DSP0236_1.3.1.pdf) does say this in section 12.12:

"This command can be used to request an MCTP bridge or bus owner to return data corresponding to its present routing table entries. This data is used to enable troubleshooting the configuration of routing tables and to enable software to draw a logical picture of the MCTP network"

Drawing a logical picture of the MCTP network is a function of the control daemon, I think.

As a more concrete use case, one of the fields in a routing table entry is the physical transport binding identifier (byte 4). For scenarios where an endpoint has multiple (possibly redundant) physical paths from a bridge (ex. Serial/I2C/I3C), this field can be used by applications to choose the "fastest" path.

amboar commented 1 week ago

Almost none of the MCTP control commands are marked "mandatory to generate" in Table 12 of the spec. :) So in that sense, yes, Get Routing Table Entries should not be required.

Mandatory / Optional wasn't what I was referring to, rather the description of the command itself (as you quoted).

Drawing a logical picture of the MCTP network is a function of the control daemon, I think.

Sure, but who's looking at the picture, and why? My understanding of the intent of the description is "not software, but humans".

As a more concrete use case, one of the fields in a routing table entry is the physical transport binding identifier (byte 4). For scenarios where an endpoint has multiple (possibly redundant) physical paths from a bridge (ex. Serial/I2C/I3C), this field can be used by applications to choose the "fastest" path.

This can be made with entirely local decisions at each node ("Which of the interfaces is the fastest interface for my local route entries through which the destination is reachable?"). I don't see that there's a reason to fetch another node's route table? However, the other way this is learned is via Query Hop as part of path resolution, and again, the response is generated with purely local reasoning based on the route table state at each Query Hop destination.