Skip to content

mctpd: use same IID for retries in query_peer_properties #152

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
FreddieJheng-quanta wants to merge 1 commit into
base: main
Choose a base branch
from
Open

mctpd: use same IID for retries in query_peer_properties #152

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
39 changes: 25 additions & 14 deletions src/mctpd.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2540,20 +2540,21 @@ static int get_endpoint_peer(struct ctx *ctx, sd_bus_error *berr,
return 0;
}

static int query_get_peer_msgtypes(struct peer *peer)
static int query_get_peer_msgtypes(struct peer *peer, uint8_t iid)
{
struct sockaddr_mctp_ext addr;
struct mctp_ctrl_cmd_get_msg_type_support req;
struct mctp_ctrl_resp_get_msg_type_support *resp = NULL;
uint8_t *buf = NULL;
size_t buf_size, expect_size;
uint8_t iid;
int rc;

peer->num_message_types = 0;
free(peer->message_types);
peer->message_types = NULL;
iid = mctp_next_iid(peer->ctx);

warnx("%s: Sending Get Message Type Support to %s IID 0x%02x", __func__,
peer_tostr(peer), iid);

mctp_ctrl_msg_hdr_init_req(&req.ctrl_hdr, iid,
MCTP_CTRL_CMD_GET_MESSAGE_TYPE_SUPPORT);
Expand Down Expand Up @@ -2744,14 +2745,13 @@ static int query_get_peer_uuid_by_phys(struct ctx *ctx, const dest_phys *dest,
return rc;
}

static int query_get_peer_uuid(struct peer *peer)
static int query_get_peer_uuid(struct peer *peer, uint8_t iid)
{
struct sockaddr_mctp_ext addr;
struct mctp_ctrl_cmd_get_uuid req;
struct mctp_ctrl_resp_get_uuid *resp = NULL;
uint8_t *buf = NULL;
size_t buf_size;
uint8_t iid;
int rc;

if (peer->state != REMOTE) {
Expand All @@ -2760,7 +2760,8 @@ static int query_get_peer_uuid(struct peer *peer)
return -EPROTO;
}

iid = mctp_next_iid(peer->ctx);
warnx("%s: Sending Get Endpoint UUID to %s IID 0x%02x", __func__,
peer_tostr(peer), iid);

mctp_ctrl_msg_hdr_init_req(&req.ctrl_hdr, iid,
MCTP_CTRL_CMD_GET_ENDPOINT_UUID);
Expand Down Expand Up @@ -3133,20 +3134,26 @@ static int query_peer_properties(struct peer *peer)
{
const unsigned int max_retries = 4;
bool supports_vdm = false;
uint8_t iid;
int rc;

iid = mctp_next_iid(peer->ctx);
for (unsigned int i = 0; i < max_retries; i++) {
rc = query_get_peer_msgtypes(peer);
rc = query_get_peer_msgtypes(peer, iid);

// Success
if (rc == 0)
break;

// On timeout, retry
// On timeout, retry with same IID
if (rc == -ETIMEDOUT) {
if (peer->ctx->verbose)
warnx("Retrying to get endpoint types for %s. Attempt %u",
peer_tostr(peer), i + 1);
warnx("Retrying to get endpoint types for %s. Attempt %u IID 0x%02x",
peer_tostr(peer), i + 1, iid);
if (i + 1 == max_retries)
warnx("Get Message Type Support timed out for %s, "
"proceeding to Get UUID without response",
peer_tostr(peer));
rc = 0;
continue;
}
Expand Down Expand Up @@ -3194,18 +3201,22 @@ static int query_peer_properties(struct peer *peer)
}
}

iid = mctp_next_iid(peer->ctx);
for (unsigned int i = 0; i < max_retries; i++) {
rc = query_get_peer_uuid(peer);
rc = query_get_peer_uuid(peer, iid);

// Success
if (rc == 0)
break;

// On timeout, retry
// On timeout, retry with same IID
if (rc == -ETIMEDOUT) {
if (peer->ctx->verbose)
warnx("Retrying to get peer UUID for %s. Attempt %u",
peer_tostr(peer), i + 1);
warnx("Retrying to get peer UUID for %s. Attempt %u IID 0x%02x",
peer_tostr(peer), i + 1, iid);
if (i + 1 == max_retries)
warnx("Get Endpoint UUID timed out for %s",
peer_tostr(peer));
rc = 0;
continue;
}
Expand Down
48 changes: 48 additions & 0 deletions tests/test_mctpd.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1765,6 +1765,54 @@ async def handle_mctp_control(self, sock, addr, data):
assert res == 0


async def test_query_peer_properties_same_iid_on_retry(nursery, dbus, sysnet):
"""Verify that retries for query_peer_properties reuse the same IID.

Per DSP0237 Table 9, a retry is a retransmission of the same MCTP control
message and must use the same instance ID (IID) within MT4.
"""

class IIDTrackingEndpoint(Endpoint):
"""Drop the first Get Message Type Support request, record all IIDs seen."""

def __init__(self, *args, **kwargs):
super().__init__(*args, **kwargs)
self.seen_iids = []
self.drop_next = True

async def handle_mctp_control(self, sock, addr, data):
rq = data[0] & 0x80
opcode = data[1]
iid = data[0] & 0x1F
if rq and opcode == 0x05: # Get Message Type Support
self.seen_iids.append(iid)
if self.drop_next:
self.drop_next = False
return # simulate timeout
return await super().handle_mctp_control(sock, addr, data)

mctpd = MctpdWrapper(dbus, sysnet)
await mctpd.start_mctpd(nursery)

iface = mctpd.system.interfaces[0]
mctp = await mctpd_mctp_iface_obj(dbus, iface)

ep = IIDTrackingEndpoint(iface, bytes([0x1A]), eid=15, types=[0, 1, 2])
mctpd.network.add_endpoint(ep)

await mctp.call_setup_endpoint(ep.lladdr)

# Two Get Message Type Support requests: initial + one retry
assert len(ep.seen_iids) == 2
# Both must carry the same IID
assert ep.seen_iids[0] == ep.seen_iids[1], (
f"IID changed across retry: {ep.seen_iids[0]:#04x} -> {ep.seen_iids[1]:#04x}"
)

res = await mctpd.stop_mctpd()
assert res == 0


async def test_bridged_endpoint_poll(dbus, sysnet, nursery, autojump_clock):
"""Test that we use endpoint poll interval from the config and
that we discover bridged endpoints via polling
Expand Down
Loading