Skip to content

Mctp bridge support #71

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

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Mctp bridge support #71

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b274f0d
Add mctp ALLOCATE_ENDPOINT_ID message structure
faizana-nvidia Apr 22, 2025
3f9ec21
Add Bridge Endpoint Support
faizana-nvidia Apr 23, 2025
686395a
Add Allocate Endpoint ID support for MCTP Bridge
faizana-nvidia Apr 24, 2025
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
6 changes: 4 additions & 2 deletions docs/mctpd.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -123,16 +123,18 @@ Similar to SetupEndpoint, but will always assign an EID rather than querying for
existing ones. Will return `new = false` when an endpoint is already known to
`mctpd`.

#### `.AssignEndpointStatic`: `ayy` → `yisb`
#### `.AssignEndpointStatic`: `ayyy` → `yisb`
Copy link
Member

Choose a reason for hiding this comment

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

This is a breaking change in the dbus API, which we cannot do here without proper versioning updates.

However, I'm not sure that having the caller be responsible for the pool sizes makes the most sense. I had planned to handle this as part of the mctpd configuration, allowing pools to be defined there. I'm happy to hear arguments for this approach though - what drove this design element?

Copy link
Author

Choose a reason for hiding this comment

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

I feel like there's a misunderstanding, the new argument was to take in pool_start-eid while the pool size itself would have been filled based on whatever bridge mentions as its pool size.

About this breaking the existing dbus api, I totally agree with you on this, based on your suggestion I agree may be having a new dbus method for AllocateBridgeStatic would be more convenient and stable way of doing this.

But I do have few questions on the suggested signature
AllocateBridgeStatic(addr: ay, pool_start: y, pool_size: y)

Q1) Do we really need pool_size as an argument here? (I get that your next point on checking this with the response of Set Endpoint ID command, but I don't get if its necessary)

where the Set Endpoint ID response must match the expected pool size.
Q2) If we go with seperate dbus method for Allocating EIDs, we would need own bridge EID as an argument as well.
AllocateBridgeStatic(addr: ay, bridge_eid: y, pool_start: y)

But this would again lead us to split the method (Static and Dynamic) to cover dynamic EID allocation as well.
I presume this is what you meant from

(we would also want purely-dynamic pools to be allocated from SetupEndpoint and friends, but that would result in a dynamic pool allocation. This dynamic pool would be defined either by a toml config option, or via a new TMBO dbus interface. However, we can handle those later, I think)

i.e for dynamic pool allocation we would prefer SetupEndpoint and AssignEndpoint method? Please correct me if I misunderstood you.

Copy link
Member

Choose a reason for hiding this comment

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

Q2) If we go with seperate dbus method for Allocating EIDs, we would need own bridge EID as an argument as well.

Yes, good point. I had intended to include this, but missed it in the prototype.

Q1) Do we really need pool_size as an argument here?

Yes, I think so.

With the static strategy that you're proposing here, the general interaction model will be that some external process (say, OpenBMC's Entity Manager) is taking responsibility for controlling the EID space. With the current approach (of just specifying the start), there is split responsibility: the bridge itself now controls part of the allocation scheme. To provide a consistent approach to the allocations, we would need control of the pool size so that we're not at risk of creating an overlapping pool with some other (future) bridge pool allocation.

We probably have a choice about what to do in case those pool requests do not match (ie, the call argument vs. the Set Endpoint ID response) - either refusing the pool allocation entirely, or truncating the pool to the smaller of those values. I think the latter would be reasonable.

But this would again lead us to split the method (Static and Dynamic) to cover dynamic EID allocation as well.

We don't really need to split it - the existing calls (SetupEndpoint, AssignEndpoint and possibly AssignEndpointStatic) would just gain support for the pool allocation as well - depending on what the device requests from the Set Endpoint ID response. In those cases, if a pool is requested, it would be completely dynamic, and we would serve that from a per-whole-network EID pool (configurable, via the toml file).

How does that sound?

Copy link
Author

Choose a reason for hiding this comment

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

Yeah sounds feasible and reasonable to me (should have thought about case when ask won't be of utilizing full bridge size but a short chunk of it, giving better control to upper applications like dbus sensors etc.). So we'll have a poll from start eid to pool size asked, to check presence of device on the network. But in case we are going with dynamic eid allocation, since pool size will be relied on what whatever bridge has as capacity, we'll utilizing all the eids from the pool.

Whatever pool size we ask either it would be more than bridge capability in which case, it would assign only to its max capability thus truncating the value and us getting notified via response of Allocate endpoint ID. On contrary, if asks is less than max capability then bridge would do it since we'll be updating the request with required size.


Similar to AssignEndpoint, but takes an additional EID argument:

```
AssignEndpointStatic <hwaddr> <static-EID>
AssignEndpointStatic <hwaddr> <static-EID> <pool_start-EID>
```

to assign `<static-EID>` to the endpoint with hardware address `hwaddr`.

For MCTP Bridges `<pool_start-EID>` will be starting eid assigned for downstream devices. For non-bridge devices this argument value will be zero.

This call will fail if the endpoint already has an EID, and that EID is
different from `static-EID`, or if `static-EID` is already assigned to another
endpoint.
Expand Down
23 changes: 23 additions & 0 deletions src/mctp-control-spec.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,29 @@ struct mctp_ctrl_resp_resolve_endpoint_id {
// ... uint8_t physical_address[N]
} __attribute__((__packed__));

typedef enum {
alloc_eid = 0,
force_alloc = 1,
get_alloc_info = 2,
reserved = 3
} mctp_ctrl_cmd_alloc_eid_op;

struct mctp_ctrl_cmd_alloc_eid {
struct mctp_ctrl_msg_hdr ctrl_hdr;
mctp_ctrl_cmd_alloc_eid_op alloc_eid_op : 2;
uint8_t : 6;
Comment on lines +187 to +188
Copy link
Member

Choose a reason for hiding this comment

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

Bitfield members have an implementation-defined layout, so we cannot use them for on-the-wire formats.

I'd suggest using specifically-sized fields (ie. uint8_t here), and handling the mask/shift operations on parse.

(same for `mctp_ctrl_resp_alloc_eid below too)

Copy link
Author

Choose a reason for hiding this comment

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

Thank you for your comments, your ask makes sense to me, have tried to go via similar approach as done in mctp_ctrl_cmd_set_eid where bitfield was used. Nonetheless I will change this and rely more on bitmask/shift operation

uint8_t pool_size;
uint8_t start_eid;
} __attribute__((__packed__));

struct mctp_ctrl_resp_alloc_eid {
struct mctp_ctrl_msg_hdr ctrl_hdr;
uint8_t completion_code;
uint8_t status : 2;
uint8_t : 6;
uint8_t eid_pool_size;
uint8_t eid_set;
} __attribute__((__packed__));

#define MCTP_CTRL_HDR_MSG_TYPE 0
#define MCTP_CTRL_HDR_FLAG_REQUEST (1 << 7)
Expand Down
200 changes: 193 additions & 7 deletions src/mctpd.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@
#define CC_MCTP_DBUS_IFACE_INTERFACE "au.com.codeconstruct.MCTP.Interface1"
#define CC_MCTP_DBUS_NETWORK_INTERFACE "au.com.codeconstruct.MCTP.Network1"

#define BRIDGE_SETTLE_DELAY_SEC 4
// an arbitrary constant for use with sd_id128_get_machine_app_specific()
static const char* mctpd_appid = "67369c05-4b97-4b7e-be72-65cfd8639f10";

Expand Down Expand Up @@ -177,6 +178,11 @@ struct peer {
uint8_t endpoint_type;
uint8_t medium_spec;
} recovery;

// Pool size
uint8_t pool_size;
uint8_t pool_start;

};
typedef struct peer peer;

Expand Down Expand Up @@ -238,6 +244,7 @@ static int add_local_eid(ctx *ctx, int net, int eid);
static int del_local_eid(ctx *ctx, int net, int eid);
static int add_net(ctx *ctx, int net);
static int add_interface(ctx *ctx, int ifindex);
static int endpoint_allocate_eid(peer *peer);

mctp_eid_t local_addr(const ctx *ctx, int ifindex) {
mctp_eid_t *eids, ret = 0;
Expand Down Expand Up @@ -1313,7 +1320,7 @@ static int endpoint_query_phys(ctx *ctx, const dest_phys *dest,
}

/* returns -ECONNREFUSED if the endpoint returns failure. */
static int endpoint_send_set_endpoint_id(const peer *peer, mctp_eid_t *new_eid)
static int endpoint_send_set_endpoint_id(peer *peer, mctp_eid_t *new_eid)
{
struct sockaddr_mctp_ext addr;
struct mctp_ctrl_cmd_set_eid req = {0};
Expand Down Expand Up @@ -1360,9 +1367,11 @@ static int endpoint_send_set_endpoint_id(const peer *peer, mctp_eid_t *new_eid)

alloc = resp->status & 0x3;
if (alloc != 0) {
// TODO for bridges
warnx("%s requested allocation pool, unimplemented",
dest_phys_tostr(dest));
peer->pool_size = resp->eid_pool_size;
if (peer->ctx->verbose) {
warnx("%s requested allocation of pool size = %d",
dest_phys_tostr(dest), peer->pool_size);
}
}

rc = 0;
Expand Down Expand Up @@ -2100,6 +2109,16 @@ static int method_assign_endpoint(sd_bus_message *call, void *data, sd_bus_error
goto err;
dfree(peer_path);

/* MCTP Bridge Support */
/* Allocate Endopint EID while looking for pool start eid
* Update pool endpoints to dbus */

if(peer->pool_size > 0) {
/* new start eid will be assigned before MCTP Allocate eid control command */
peer->pool_start = peer->eid + 1;
rc = endpoint_allocate_eid(peer);
}

return sd_bus_reply_method_return(call, "yisb",
peer->eid, peer->net, peer_path, 1);
err:
Expand All @@ -2114,7 +2133,7 @@ static int method_assign_endpoint_static(sd_bus_message *call, void *data,
char *peer_path = NULL;
peer *peer = NULL;
ctx *ctx = data;
uint8_t eid;
uint8_t eid, start_eid;
int rc;

dest->ifindex = interface_call_to_ifindex_busowner(ctx, call);
Expand All @@ -2130,6 +2149,10 @@ static int method_assign_endpoint_static(sd_bus_message *call, void *data,
if (rc < 0)
goto err;

rc = sd_bus_message_read(call, "y", &start_eid);
if (rc < 0)
goto err;

rc = validate_dest_phys(ctx, dest);
if (rc < 0)
return sd_bus_error_setf(berr, SD_BUS_ERROR_INVALID_ARGS,
Expand Down Expand Up @@ -2173,6 +2196,14 @@ static int method_assign_endpoint_static(sd_bus_message *call, void *data,
}
dfree(peer_path);

/* MCTP Bridge Support */
/* Allocate Endopint EID
* Update pool endpoints to dbus */
if(peer->pool_size > 0) {
Copy link
Member

Choose a reason for hiding this comment

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

minor: formatting here should be:

    if (peer->pool_size > 0) {

(with the space after the if)

Copy link
Author

Choose a reason for hiding this comment

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

my bad, since we are on this, what clang-format are we following here? Any steps on how I could run it locally. Would come in handy.

Copy link
Member

Choose a reason for hiding this comment

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

We don't have a full .clang-format definition, but we're roughly doing kernel format here. If it helps, I can put an actual config together, but that then we'd need a lot of churn in applying it.

So, that's probably better something done just before release, to avoid to much merge collisions.

Or, if you're OK with a bit of churn, I can do that beforehand?

Copy link
Author

Choose a reason for hiding this comment

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

I'm sold for before release part :)

peer->pool_start = start_eid;
rc = endpoint_allocate_eid(peer);
}

return sd_bus_reply_method_return(call, "yisb",
peer->eid, peer->net, peer_path, 1);
err:
Expand Down Expand Up @@ -2852,9 +2883,10 @@ static const sd_bus_vtable bus_owner_vtable[] = {
0),

SD_BUS_METHOD_WITH_NAMES("AssignEndpointStatic",
"ayy",
"ayyy",
SD_BUS_PARAM(physaddr)
SD_BUS_PARAM(eid),
SD_BUS_PARAM(eid)
SD_BUS_PARAM(start_eid),
"yisb",
SD_BUS_PARAM(eid)
SD_BUS_PARAM(net)
Expand Down Expand Up @@ -4477,6 +4509,160 @@ static void setup_config_defaults(ctx *ctx)
ctx->default_role = ENDPOINT_ROLE_BUS_OWNER;
}

static int endpoint_send_allocate_endpoint_id(peer *peer,
mctp_eid_t eid_start, uint8_t eid_pool_size, mctp_ctrl_cmd_alloc_eid_op oper,
uint8_t *allocated_pool_size, mctp_eid_t *allocated_pool_start)
{
struct sockaddr_mctp_ext addr;
struct mctp_ctrl_cmd_alloc_eid req = {0};
struct mctp_ctrl_resp_alloc_eid *resp = NULL;
uint8_t *buf = NULL;
size_t buf_size;
uint8_t iid, stat;
int rc;

iid = mctp_next_iid(peer->ctx);
req.ctrl_hdr.rq_dgram_inst = RQDI_REQ | iid;
req.ctrl_hdr.command_code = MCTP_CTRL_CMD_ALLOCATE_ENDPOINT_IDS;
req.alloc_eid_op = alloc_eid;
req.pool_size = eid_pool_size;
req.start_eid = eid_start;
rc = endpoint_query_peer(peer, MCTP_CTRL_HDR_MSG_TYPE, &req, sizeof(req),
&buf, &buf_size, &addr);
if (rc < 0)
goto out;

rc = mctp_ctrl_validate_response(buf, buf_size, sizeof(*resp),
peer_tostr_short(peer), iid,
MCTP_CTRL_CMD_ALLOCATE_ENDPOINT_IDS);

if (rc)
goto out;

resp = (void *)buf;
if (!resp) {
warnx("%s Invalid response Buffer\n", __func__);
return -ENOMEM;
}

stat = resp->status & 0x03;
if (stat == 0x00) {
if(peer->ctx->verbose) {
fprintf(stderr, "%s Allocation Accepted \n", __func__);
}
}
else if (stat == 0x1) {
warnx("%s Allocation was rejected as it was Allocated by other bus \n", __func__);
}

*allocated_pool_size = resp->eid_pool_size;
*allocated_pool_start = resp->eid_set;
if(peer->ctx->verbose) {
fprintf(stderr, "%s Allocated size of %d, starting from EID %d\n", __func__,
resp->eid_pool_size, resp->eid_set);
}

return 0;
out:
free(buf);
return rc;
}
static int cb_populate_pool_eids(sd_event_source *s, uint64_t t, void* data)
{
peer *peer = data;
int net = peer->net;
struct peer *allocated_peer = NULL;
dest_phys dest = peer->phys;
int rc;

fprintf (stderr, "Bridge Time expired, set pool eids\n");
for (mctp_eid_t eid = peer->pool_start;
eid < peer->pool_start + peer->pool_size; eid++) {
rc = add_peer(peer->ctx, &dest, eid, net, &allocated_peer);
if (rc < 0) {
warnx("%s failed to find peer for allocated eid %d\n",
__func__, eid);
continue;
}

rc = setup_added_peer(allocated_peer);
if (rc < 0) {
warnx("%s failed to setup peer for allocated eid %d\n",
__func__, eid);
continue;
}
}

return 0;
}

static mctp_eid_t get_pool_start (peer *peer, mctp_eid_t eid_start, uint8_t pool_size)
{
uint8_t count = 0;
mctp_eid_t pool_start = eid_alloc_max;
net_det *n = lookup_net(peer->ctx, peer->net);

if (!n) {
warnx("BUG: Unknown net %d : failed to get pool start\n", peer->net);
return eid_alloc_max;
}

for (mctp_eid_t e = eid_start; e <= eid_alloc_max; e++) {
if (n->peeridx[e] == -1) {
if(pool_start == eid_alloc_max) {
pool_start = e;
}
count++;
if (count == pool_size) return pool_start;
} else {
pool_start = eid_alloc_max;
count = 0;
}
}

return eid_alloc_max;
}

static int endpoint_allocate_eid(peer* peer)
{
uint8_t allocated_pool_size = 0;
mctp_eid_t allocated_pool_start = 0;

/* Find pool sized contiguous unused eids to allocate on the bridge. */
peer->pool_start = get_pool_start(peer, peer->pool_start, peer->pool_size);
if(peer->pool_start == eid_alloc_max) {
warnx("%s failed to find contiguous EIDs of required size", __func__);
return 0;
} else {
if(peer->ctx->verbose)
fprintf(stderr, "%s Asking for contiguous EIDs for pool with start eid : %d\n", __func__, peer->pool_start);
}

int rc = endpoint_send_allocate_endpoint_id(peer, peer->pool_start, peer->pool_size,
alloc_eid, &allocated_pool_size, &allocated_pool_start);
if (rc) {
warnx("%s failed to allocate endpoints, returned %s %d\n",
__func__, strerror(-rc), rc);
} else {
peer->pool_size = allocated_pool_size;
peer->pool_start = allocated_pool_start;

//delay for settling MCTP Bridge after allocation of downstream eid
Copy link
Member

Choose a reason for hiding this comment

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

I would prefer that we issue the Allocate Endpoint IDs immediately. If the bridge needs some time to settle, we can implement that with a retry.

For your device, what do you see when you attempt to do the Allocate immediately? Does the command fail, or something else?

Copy link
Author

Choose a reason for hiding this comment

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

Let me highlight the issue better, it's not that Allocate Endpoint ID is getting delayed (we are sending it as part of sequence right after bridge eid is assigned), but post allocate endpoint id, referring to my setup, I see no response from the downstream EIDs, basically timeout for GET_MESSG_TYPE and GET_UUID mctp commands on downstream eids.

On topic of retry mechanism, in my setup based on observational data, find that it takes around 4 sec for all eids to be in proper state after receiving ALLOCATE_EID but this also depends on the bus topology and how many downstream endpoints are there.

That being said it could easily be something which could become a common issue for Bridge's owing to their network congestion, resource congestion etc.
A retry mechanism would then be needed as general part of design for messages like GET_MESSG_TYPE and GET_UUID, which is currently missing implementation.

Would like to explore any other suggestion beyond retry mechanism, if possible :)

Copy link
Member

Choose a reason for hiding this comment

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

Let me highlight the issue better, it's not that Allocate Endpoint ID is getting delayed (we are sending it as part of sequence right after bridge eid is assigned), but post allocate endpoint id, referring to my setup, I see no response from the downstream EIDs, basically timeout for GET_MESSG_TYPE and GET_UUID mctp commands on downstream eids.

ah, gotchya!

Yeah, this isn't a trivial problem, and I've been thinking of potential approaches for a while.

My current leaning is that once an endpoint pool is assigned, mctpd would perform some sort of polling to determine which of the pool's EIDs are active (ie, have been assigned to a device, and that device is responding).

We could potentially do this via a Get Routing Table Entries command to the bridge, but that's a bit of a mis-use of the command, and isn't necessarily returning the data we want (the bridge may have routing information to a device, but the device may not be present).

So, I think the most reliable approach is that the pool's EIDs are periodically polled using a Get Endpoint ID command - which is the spec-recommended method of detecting endpoint presence, and is what we use for the existing recovery process. We may be able to share some code with the recover infrastructure for this too.

We would schedule those polls when the pool is allocated, and then repeat until we detect a device - at which point we would perform full enumeration. If no device is ever present, mctpd would end up polling forever, but I don't think there's an alternative there.

Copy link

Choose a reason for hiding this comment

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

How about doing a GetRoutingTableEntries, the bridge should return ERROR_NOT_READY if it is still assigning EIDs downstream from the previous AllocateEndpointID. Once we get a successful response, we can start querying EIDs downstream for UUIDs and message types.

Copy link
Member

Choose a reason for hiding this comment

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

the bridge should return ERROR_NOT_READY if it is still assigning EIDs downstream from the previous AllocateEndpointID

that's not necessarily what the bridge will do though - there's no concept of the routing state being "complete" at any point (eg, for a downstream port that may have an enpoint hotplugged at some point in the future (or perhaps never!))

For that reason, I don't think we'll be able to avoid polling.

Copy link
Author

Choose a reason for hiding this comment

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

one downside of this I feel (if we don't go via GetRoutingTable implementation (have a change list ready but nevermind)) is we'll be ending up utilizing the eids from the pool (specially for Dynamic eid allocation based on bridge pool size) which is a bit concerning if the network becomes complex.

Take RoutingTable approach makes us definitive about what eids needs to be consumed from the pool (minimizing the utilization, based on available device downstream).

Copy link
Member

Choose a reason for hiding this comment

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

ending up utilizing the eids from the pool

Not sure I understand what you mean by "utilizing the eids" here, could you elaborate?

Take RoutingTable approach makes us definitive about what eids needs to be consumed from the pool

All allocated EIDs will need to be consumed, we have no choice about that.

It's what endpoints we try to interact with that may vary, depending on the approach.

My concern with using Get Routing Table Entries is that we're on the edges of what the spec describes this command for, whereas Get Endpoint ID is mentioned as a mechanism for this. An endpoint being described in Get Routing Table Entries does not mean it is available, so we would have to fall back to querying/polling it with Get Endpoint ID anyway.

Copy link
Author

Choose a reason for hiding this comment

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

Not sure I understand what you mean by "utilizing the eids" here, could you elaborate?

Sorry I should have explained better. May be I'm overthinking it here but here's what I meant

For instance, if we have a network comprising of multiple Bridge's where we have to assign all Dynamic Eids to the downstream, as per our current assumption, we'll have to keep aside all Eids which bridge needs based on its pool size (Bridge eid : 8, pool size :15 thus 9- 23 is consumed from our Dynamic Pool even though there's possibility of not having devices downstream for 9-23 Eids, since we carve eids from our maintained pool via (code below) and then add_peer(consumed here) later update setup_peer()

/* Find an unused EID */ for (e = eid_alloc_min; e <= eid_alloc_max; e++) { if (n->peeridx[e] == -1) { rc = add_peer(ctx, dest, e, net, &peer); if (rc < 0) return rc; break; } }
this could end up consuming EIDs for the device which may not be there.

Also to mention based on the allocation of EIDs needs to be contiguous, this would create another issue if there happens to be other Bridges which has been allocated EID statically which kind of makes find a contiguous set of EIDs for the downstream endpoints of Dynamic Allocated Bridge difficult and where need of next contiguous set of EIDs rises.

Spec : DSP0236 Version: 1.3.0

9.1.9 Consolidating routing table entries
MCTP requires that when an EID pool is allocated to a device, the range of EIDs is contiguous and
follows the EID for the bridge itself. Thus, a bridge can elect to consolidate routing table information into
one entry when it recognizes that it has received an EID or EID range that is contiguous with an existing
entry for the same physical address and bus. (The reason that EID allocation and routing information
updates are not done as one range using the same command is because of the possibility that a device
may have already received an allocation from a different bus owner.)

In such contingency with limited EIDs availability, keeping EIDs for non available device is what I fear. Though I agree with your point too Routing Table may not be the best approach to detect Endpoint presence in the network especially if the endpoint is hot pluggable.

My concern with using Get Routing Table Entries is that we're on the edges of what the spec describes this command for, whereas Get Endpoint ID is mentioned as a mechanism for this. An endpoint being described in Get Routing Table Entries does not mean it is available, so we would have to fall back to querying/polling it with Get Endpoint ID anyway.

But then again in such case Spec tells us about commands like Discovery Notify which should be initiated by the Bridge to notify any change in current network topology under the bridge. Please feel free to correct me in my assumptions :)

Copy link
Member

Choose a reason for hiding this comment

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

we'll have to keep aside all Eids which bridge needs based on its pool size (Bridge eid : 8, pool size :15 thus 9- 23 is consumed from our Dynamic Pool even though there's possibility of not having devices downstream for 9-23 Eids
[...]
this could end up consuming EIDs for the device which may not be there.

Yes, absolutely. If we have done an Allocate Endpoint IDs that gives a range of EIDs to a bridge, then mctpd no longer has ownership of any of those EIDs, and cannot hand out any subset of that range elsewhere.

In such contingency with limited EIDs availability, keeping EIDs for non available device is what I fear.

We have no choice about that - if we allocate a range to a bridge, the entire range belongs to the bridge, regardless of whether the EIDs in that range are then allocated to downstream devices, or whether those downstream devices are active.

But then again in such case Spec tells us about commands like Discovery Notify which should be initiated by the Bridge to notify any change in current network topology under the bridge.

That's not my interpretation of the spec for Discovery Notify; this is more to announce endpoint presence, and is only used on specific transport types (ie, those that do not have their own presence detection mechanism).

uint64_t now_usec, timer_usec;
// Get the current time in microseconds
if (sd_event_now(peer->ctx->event, CLOCK_MONOTONIC, &now_usec) < 0) {
warnx("Failed to get current time");
return -1;
}
timer_usec = now_usec + BRIDGE_SETTLE_DELAY_SEC * 1000000ULL;
rc = sd_event_add_time(peer->ctx->event, NULL,
CLOCK_MONOTONIC, timer_usec, 0,
cb_populate_pool_eids, peer);
}

return 0;
}

int main(int argc, char **argv)
{
int rc;
Expand Down
Loading