Merge branch 'dev/vdm-msg-type-support'

Signed-off-by: Jeremy Kerr <jk@codeconstruct.com.au>

Author: jk-ozlabs

CHANGELOG.md

@@ -17,6 +17,15 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
 
 1. Fixed `mctp-bench` compile on musl libc
 
+### Changed
+
+1. `mctpd`'s `RegisterTypeSupport` dbus method will no longer allow the
+   VDM messaging types (0x7e and 0x7f) to be registered, as these do not
+   make sense without a corresponding VDM type registration.
+
+   Registering a VDM type will automatically include the corresponding VDM type
+   in the Get Message Type Support response.
+
 ### Added
 
 1. `mctpd` now implements the Get Vendor Define Message Support control protocol

docs/mctpd.md

@@ -50,6 +50,10 @@ If the message type is already registered, then dbus call will fail.
  - `<versions>` Versions supported for this message type formatted as uint32
    integers as specified in DSP0236
 
+The `msg_type` value must be a valid message type, and cannot be one of the
+VDM types (0x7e or 0x7f). Those VDM types are handled by the
+`RegisterVDMTypeSupport` method instead.
+
 De-registration is automatic - the specified types (and versions) are registered
 for as long as the dbus sender remains attached to the message bus, and are
 unregistered on disconnect.
@@ -72,6 +76,9 @@ If the VDM type is already registered, then dbus call will fail.
    - For IANA format: 32-bit unsigned integer (`u`)
  - `<command set type>` Command set type (16-bit unsigned integer) as defined by the vendor
 
+Registering a VDM type will cause the corresponding VDM type value (0x7e or
+0x7f) to be returned in mctpd's Get Message Type Support response.
+
 De-registration is automatic - the specified VDM types are registered for as
 long as the dbus sender remains attached to the message bus, and are
 removed when the sender disconnects.

src/mctpd.c

@@ -61,6 +61,8 @@ static const char *conf_file_default = MCTPD_CONF_FILE_DEFAULT;
 
 static const mctp_eid_t eid_alloc_min = 0x08;
 static const mctp_eid_t eid_alloc_max = 0xfe;
+static const uint8_t MCTP_TYPE_VENDOR_PCIE = 0x7e;
+static const uint8_t MCTP_TYPE_VENDOR_IANA = 0x7f;
 
 // arbitrary sanity
 static size_t MAX_PEER_SIZE = 1000000;
@@ -981,6 +983,7 @@ static int handle_control_get_message_type_support(
 {
 	struct mctp_ctrl_resp_get_msg_type_support *resp = NULL;
 	struct mctp_ctrl_cmd_get_msg_type_support *req = NULL;
+	bool pcie_support = false, iana_support = false;
 	size_t i, resp_len, type_count;
 	uint8_t *resp_buf, *msg_types;
 	int rc;
@@ -992,6 +995,15 @@ static int handle_control_get_message_type_support(
 
 	req = (void *)buf;
 	type_count = ctx->num_supported_msg_types;
+
+	for (i = 0; i < ctx->num_supported_vdm_types; i++) {
+		pcie_support |= ctx->supported_vdm_types[i].format ==
+				VID_FORMAT_PCIE;
+		iana_support |= ctx->supported_vdm_types[i].format ==
+				VID_FORMAT_IANA;
+	}
+	type_count += (pcie_support + iana_support);
+
 	// Allocate extra space for the message types
 	resp_len = sizeof(*resp) + type_count;
 	resp_buf = malloc(resp_len);
@@ -1004,13 +1016,19 @@ static int handle_control_get_message_type_support(
 	mctp_ctrl_msg_hdr_init_resp(&resp->ctrl_hdr, req->ctrl_hdr);
 	resp->completion_code = MCTP_CTRL_CC_SUCCESS;
 
-	resp->msg_type_count = type_count;
 	// Append message types after msg_type_count
 	msg_types = (uint8_t *)(resp + 1);
-	for (i = 0; i < type_count; i++) {
+	for (i = 0; i < ctx->num_supported_msg_types; i++) {
 		msg_types[i] = ctx->supported_msg_types[i].msg_type;
 	}
+	if (pcie_support) {
+		msg_types[i++] = MCTP_TYPE_VENDOR_PCIE;
+	}
+	if (iana_support) {
+		msg_types[i++] = MCTP_TYPE_VENDOR_IANA;
+	}
 
+	resp->msg_type_count = type_count;
 	rc = reply_message(ctx, sd, resp, resp_len, addr);
 	free(resp_buf);
 
@@ -3587,6 +3605,10 @@ static int method_register_type_support(sd_bus_message *call, void *data,
 	rc = sd_bus_message_read(call, "y", &msg_type);
 	if (rc < 0)
 		goto err;
+	if (msg_type == 0 || msg_type >= MCTP_TYPE_VENDOR_PCIE) {
+		return sd_bus_error_setf(berr, SD_BUS_ERROR_INVALID_ARGS,
+					 "Invalid message type %d", msg_type);
+	}
 	rc = sd_bus_message_read_array(call, 'u', (const void **)&versions,
 				       &versions_len);
 	if (rc < 0)

tests/test_mctpd.py

@@ -1269,6 +1269,14 @@ async def test_get_message_types(dbus, mctpd, routed_ep):
     mctp = await mctpd_mctp_base_iface_obj(dbus)
     await mctp.call_register_type_support(5, [0xF1F2F3F4])
 
+    # Verify invalid msg type causes error
+    with pytest.raises(asyncdbus.errors.DBusError) as ex:
+        await mctp.call_register_type_support(0x0, [0xF1F2F3F4])
+    assert str(ex.value) == "Invalid message type 0"
+    with pytest.raises(asyncdbus.errors.DBusError) as ex:
+        await mctp.call_register_type_support(0x7e, [0xF1F2F3F4])
+    assert str(ex.value) == "Invalid message type 126"
+
     # Verify get message type response includes spdm
     cmd = MCTPControlCommand(True, 0, 0x05, bytes([0x00]))
     rsp = await ep.send_control(mctpd.network.mctp_socket, cmd)
@@ -1297,6 +1305,11 @@ async def test_register_vdm_type_support_pcie_only(dbus, mctpd, routed_ep):
     v_type = asyncdbus.Variant('q', 0xABCD)
     await mctp.call_register_vdm_type_support(0x00, v_type, 0x0001)
 
+    # Verify Get Message Type Support response includes PCI VDM type
+    cmd = MCTPControlCommand(True, 0, 0x05, bytes([0x00]))
+    rsp = await ep.send_control(mctpd.network.mctp_socket, cmd)
+    assert rsp.hex(' ') == '00 05 00 02 00 7e'
+
     # Verify PCIe VDM (selector 0)
     cmd = MCTPControlCommand(True, 0, 0x06, bytes([0x00]))
     rsp = await ep.send_control(mctpd.network.mctp_socket, cmd)
@@ -1316,11 +1329,46 @@ async def test_register_vdm_type_support_iana_only(dbus, mctpd, routed_ep):
     v_type = asyncdbus.Variant('u', 0x1234ABCD)
     await mctp.call_register_vdm_type_support(0x01, v_type, 0x5678)
 
+    # Verify Get Message Type Support response includes IANA VDM type
+    cmd = MCTPControlCommand(True, 0, 0x05, bytes([0x00]))
+    rsp = await ep.send_control(mctpd.network.mctp_socket, cmd)
+    assert rsp.hex(' ') == '00 05 00 02 00 7f'
+
     # Verify IANA VDM (selector 0)
     cmd = MCTPControlCommand(True, 0, 0x06, bytes([0x00]))
     rsp = await ep.send_control(mctpd.network.mctp_socket, cmd)
     assert rsp.hex(' ') == '00 06 00 ff 01 12 34 ab cd 56 78'
 
+""" Test RegisterVDMTypeSupport when both IANA and PCI types are registered """
+async def test_register_vdm_type_support_both(dbus, mctpd, routed_ep):
+    ep = routed_ep
+    mctp = await mctpd_mctp_base_iface_obj(dbus)
+
+    # Register IANA VDM: format=0x01, VID=0x1234ABCD, command_set=0x5678
+    v_type = asyncdbus.Variant('u', 0x1234ABCD)
+    await mctp.call_register_vdm_type_support(0x01, v_type, 0x5678)
+
+    # Register PCI VDM: format=0x00, VID=0xABCD, command_set=0x5678
+    v_type = asyncdbus.Variant('q', 0xABCD)
+    await mctp.call_register_vdm_type_support(0x00, v_type, 0x5678)
+
+    # Verify Get Message Type Support response includes both VDM types
+    cmd = MCTPControlCommand(True, 0, 0x05, bytes([0x00]))
+    rsp = await ep.send_control(mctpd.network.mctp_socket, cmd)
+    assert rsp.hex(' ') == '00 05 00 03 00 7e 7f'
+
+    # we assume ordering of IANA vs PCI here, but current mctpd will
+    # preserve that.
+    # Verify IANA VDM (selector 0)
+    cmd = MCTPControlCommand(True, 0, 0x06, bytes([0x00]))
+    rsp = await ep.send_control(mctpd.network.mctp_socket, cmd)
+    assert rsp.hex(' ') == '00 06 00 01 01 12 34 ab cd 56 78'
+
+    # Verify PCI VDM (selector 1)
+    cmd = MCTPControlCommand(True, 0, 0x06, bytes([0x01]))
+    rsp = await ep.send_control(mctpd.network.mctp_socket, cmd)
+    assert rsp.hex(' ') == '00 06 00 ff 00 ab cd 56 78'
+
 """ Test RegisterVDMTypeSupport with dbus disconnect """
 async def test_register_vdm_type_support_dbus_disconnect(mctpd, routed_ep):
     ep = routed_ep
@@ -1333,14 +1381,24 @@ async def test_register_vdm_type_support_dbus_disconnect(mctpd, routed_ep):
     async with asyncdbus.MessageBus().connect() as temp_bus:
         mctp = await mctpd_mctp_base_iface_obj(temp_bus)
 
-        # Register PCIe VDM: format=0x00, VID=0xABCD, command_set=0x0001
+        # Register PCIe VDM: format=0x00, VID=0xABCD, command_set=1 and 2
         v_type = asyncdbus.Variant('q', 0xABCD)
         await mctp.call_register_vdm_type_support(0x00, v_type, 0x0001)
+        await mctp.call_register_vdm_type_support(0x00, v_type, 0x0002)
 
         # Verify PCIe VDM (selector 0)
         cmd = MCTPControlCommand(True, 0, 0x06, bytes([0x00]))
         rsp = await ep.send_control(mctpd.network.mctp_socket, cmd)
-        assert rsp.hex(' ') == '00 06 00 ff 00 ab cd 00 01'
+        assert rsp.hex(' ') == '00 06 00 01 00 ab cd 00 01'
+        # Verify PCIe VDM (selector 1)
+        cmd = MCTPControlCommand(True, 0, 0x06, bytes([0x01]))
+        rsp = await ep.send_control(mctpd.network.mctp_socket, cmd)
+        assert rsp.hex(' ') == '00 06 00 ff 00 ab cd 00 02'
+
+        # Verify GetMsgType includes VDM
+        cmd = MCTPControlCommand(True, 0, 0x05)
+        rsp = await ep.send_control(mctpd.network.mctp_socket, cmd)
+        assert rsp.hex(' ') == '00 05 00 02 00 7e'
 
     # Give mctpd a moment to process the disconnection
     await trio.sleep(0.1)
@@ -1350,6 +1408,11 @@ async def test_register_vdm_type_support_dbus_disconnect(mctpd, routed_ep):
     rsp = await ep.send_control(mctpd.network.mctp_socket, cmd)
     assert rsp.hex(' ') == '00 06 02'  # Should be error again
 
+    # Verify GetMsgType has only control command
+    cmd = MCTPControlCommand(True, 0, 0x05)
+    rsp = await ep.send_control(mctpd.network.mctp_socket, cmd)
+    assert rsp.hex(' ') == '00 05 00 01 00'
+
 """ Test RegisterVDMTypeSupport error handling """
 async def test_register_vdm_type_support_errors(dbus, mctpd):
     mctp = await mctpd_mctp_base_iface_obj(dbus)