Skip to content
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.

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

Support get_rounting_table_entries command in mctpd #11

Open
DelphineCCChiu opened this issue Aug 31, 2023 · 10 comments
Open

Support get_rounting_table_entries command in mctpd #11

DelphineCCChiu opened this issue Aug 31, 2023 · 10 comments

Comments

@DelphineCCChiu
Copy link

DelphineCCChiu commented Aug 31, 2023

This is our hardware architecture: BMC - (I2C) - BIC1 - (I2C) - BIC2.

The usage scenarios are as follows:

  1. The BMC communicates with the Bridge IC through PLDM/MCTP over I2C, and the BIC1 bridges the command to the subsequent Bridge IC (BIC2).
  2. When the BIC2 detects any board anomaly, it actively sends information back to the BMC via PLDM/MCTP over I2C.

The mctpd doesn't support allocate endpoint eids as define in section 12.10 DSP0236. The BMC issues this command to MCTP bridge, and bridge allocates eids to devices.

However, we plan to maintain static eids in devices at this time. We are implementing the get_routing_table_entries command as defined in section 12.12 DSP0236.

The sequence:

  1. The BMC get rounting table entries from MCTP bridge.
  2. The BMC adds EIDs to known table and the physical addresses of EIDs are set as address of MCTP bridge.
  3. The BMC can send command to those EIDs via MCTP bridge.

Once the implementation is complete, we are eager to contribute the code. Would like to know if anyone has opposition.
Thank you.

@DelphineCCChiu DelphineCCChiu closed this as not planned Won't fix, can't repro, duplicate, stale Aug 31, 2023
@khangng-ampere
Copy link

I think this issue is more about how we should support static routing entries configurations, so the issue title should reflect that rather than the proposed solution.

I think this should work ok in your situation, but I am not sure if this should be a part of mctpd:

  1. 8.17.2 DSP0236 says no static EIDs discovery, if I'm reading this correctly.

No mechanism is defined in the MCTP base specification for a bridge or bus owner to discover and incorporate a static EID into its routing information. Thus, a simple endpoint that is configured with a static EID shall also be used with a bus owner that is configured to support the static EIDs for the endpoint.

  1. If we do this anyway, would this play well with dynamic EIDs allocations? In other words, how do the BMC decide which EIDs belong to the static pool or the dynamic pool when assigning/allocating dynamic EIDs before the process you described hasn't occured yet?

I think the static EIDs assignments should happen at least before dynamic EIDs assignments. Which would mean BMC has no way to communicate with bridges yet, and BMC will have to configure it by itself, using some non-volatile configurations or manual routing entries adding.

@jk-ozlabs
Copy link
Member

Thanks for syncing up, @DelphineCCChiu !

This is our hardware architecture: BMC - (I2C) - BIC1 - (I2C) - BIC2.

Which are the bus owners in this diagram? Is the BMC the TMBO?

However, we plan to maintain static eids in devices at this time.

I have some rough plans to add static endpoint definitions for mctpd to consume on startup, that might help with this. As @khangng-ampere has indicated, we would do this before the dynamic addresses are assigned (and either define the dynamic EID pool accordingly, or at least ensure that those static EIDs are now reserved...)

We are implementing the get_routing_table_entries command as defined in section 12.12 DSP0236.

The sequence:

1. The BMC get rounting table entries from MCTP bridge.

2. The BMC adds EIDs to known table and the physical addresses of EIDs are set as address of MCTP bridge.

3. The BMC can send command to those EIDs via MCTP bridge.

OK, so the BMC is not the TMBO then?

@DelphineCCChiu
Copy link
Author

graph TB
BMC[BMC] --> BIC1[BIC1]
BMC[BMC] --> BIC2[BIC1]
BIC1[BIC1] --> BIC3[BIC2]
BIC1[BIC1] --> BIC4[BIC2]
BIC2[BIC1] --> BIC5[BIC2]
BIC2[BIC1] --> BIC6[BIC2]
Loading

The BMC is the TMBO. The sequence should be the BMC get eids from BIC1s, and then get rounting table entries from BIC1s.
Since the devices in the lowest hierarchy(BIC2) will be vary between different hardware configurations, we plan to get rounting table entries from the middle hierarchy(BIC1) instead of adding static endpoint definition in BMC.

We are making this command as a D-Bus method in mctpd, so we might need to call get endpointID and get rounting table entries before assigning dynamic EIDs to devices.

@khangng-ampere
Copy link

khangng-ampere commented Sep 7, 2023

Since the devices in the lowest hierarchy(BIC2) will be vary between different hardware configurations, we plan to get rounting table entries from the middle hierarchy(BIC1) instead of adding static endpoint definition in BMC.

This sounds like you're trying to avoid implementing dynamic EIDs, but inventing a different kind of dynamic EIDs in the process 😅 My impression of reading DSP0236 is that static EIDs are not intended to be used this way tho.

But anyway, I think it is specific to your use case, I cannot say much about that. I think it is fine if the implementation you plan to merge to mctpd only includes the .GetRoutingTableEntries method which gets and returns the peer's routing entries, but not the part where you add the entries to the TMBO routing table. That part should not be mctpd's responsibility, at least not yet. (If this is your original intention then I'm sorry for having misunderstood it)

@DelphineCCChiu
Copy link
Author

graph LR
BMC[BMC] --> |SLOT 1| BIC1[BIC1]
BMC[BMC] --> |SLOT 2| BIC2[BIC1]
BIC1[BIC1] --> BIC3[BIC2]
BIC1[BIC1] --> BIC4[BIC2]
BIC2[BIC1] --> BIC5[BIC2]
BIC2[BIC1] --> BIC6[BIC2]
BIC3[BIC2] --> device1[MCTP device1]
BIC3[BIC2] --> device2[MCTP device2]
BIC4[BIC2] --> device3[MCTP device3]
BIC5[BIC2] --> device4[MCTP device4]
BIC5[BIC2] --> device5[MCTP device5]
BIC6[BIC2] --> device6[MCTP device6]
Loading

Added more detail about our system.

We plan to maintain static EIDs because we have many slots plugged in many devices. If we use dynamic EIDs allocation, for example, the MCTP device1 will have different EIDs between different machines. It will be hard to debug in development.

We will have different system hardware configurations. e.g. config A has only one BIC2 with two MCTP devices connected, config B has two BIC2 like the architecture above. Furthermore, the MCTP devices will be hot-plugged. It will be difficult to identify which EID is belong to which device on different machines eventually. However, we still need to dynamic detect what devices are connecting to the system.

Do you have any suggestion which meets the specification and our use case?

@jk-ozlabs
Copy link
Member

I'm not at all opposed to having the Get Routing Table Entries command implemented, but we're getting close to the fringes of the spec with your use of static addressing there. For example, with the TMBO no longer responsible for allocating EIDS, how are you ensuring that the lowest-level static EIDs - known only to the BIC1 devices - are unique across the system?

When using the dynamic addressing model implied by the spec, each BIC1 would only have two routing table entries: one for the directly-connected BIC2 devices (it's possible that there is one entry for the BIC2 and one for the devices behind it, but that's more of an implementation detail). Each of those entries would describe a range of EIDs (ie: those behind the BIC2), and map them to a single physical address (ie. the i2c address of the BIC2). I'm not sure that that helps for your use case though?

It seems like tying the addressing / routing information to the inventory information is making things a little complex for your design there.

Is it possible to have entirely dynamic EIDs, but have the inventory information passed through some other mechanism? Or are you just looking for whether specific (pre-defined) devices are present or absent? If so, could you just check for the presence of those specific EIDs?

[It may be worthwhile separating these discussions into two issues: one for the Get Routing Table Entries implementation, and one for the inventory management structure...]

@DelphineCCChiu
Copy link
Author

We might consider having a static EIDs configuration in BMC. It contains the EIDs and physical addresses. (The configured EIDs should be reserved.) The BMC scans those devices (maybe use setup_added_peer) when mctpd startup. The present devices' EIDs will be published on D-Bus. We do need to have a re-scan mechanism, since the devices can be hot-plugged. We plan to re-scan all devices according to a GPIO event.

Do you think this is appropriate to implemented in mctpd?

@jk-ozlabs
Copy link
Member

Yes, I would like a mechanism to define static endpoints (which would then appear on d-bus). These could be optionally probed via a simple Get Endpoint ID (which ties into some plans for regular presence detection). But for now, just having those static endpoints/addresses represented would be great.

I was just thinking a simple config file for that, but open to other ideas.

@DelphineCCChiu
Copy link
Author

Hi @jk-ozlabs,
We are considering making the configuration integrated with entity-manager's configuration. In this way, the other services are able to know what devices/bridges the EIDs belong to.

For a mctp device connected to BMC:

{
    "Address": "0x33",
    "Bus": 2,
    "EndpointIds": [15],
    "Name": "mctp_device1",
    "Type": "mctp_device"
}

And for a mctp bridge connected to BMC:

{
    "Address": "0x33",
    "Bus": 1,
    "EndpointIds": [10, 11, 12],
    "Name": "mctp_bridge1",
    "Type": "mctp_device"
}

The mctpd can probe "Type": "mctp_device" for identifying mctp configuration.

Do you have any concern about this implementation?

@jk-ozlabs
Copy link
Member

I was originally thinking a separate (text-based?) address configuration for mctpd; this would define the dynamic address pools available for allocation, and any static assignements.

However, I'm OK with an EM-based approach too, given the ties into OpenBMC there. This would be the first instance where we include "outgoing" dbus interfaces from mctpd though, and we'd need to manage the dependencies appropriately.

@FighterNan: any thoughts on this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants