mctpd: allow bridge allocation with SetupEndpoint

Currently, we only allow bridge pool allocation through an
AssignEndpoint call, as that is guaranteed to involve a Set Endpoint ID
command, required to start the pool allocation process.

LearnEndpoint is intended to never modify endpoint state, so we keep that
as-is.

SetupEndpoint has always been a convenience method, intended to do a
LearnEndpoint if possible, or fall back to AssignEndpoint if not.
Because of this, we are not making any assurances about preserving state
with SetupEndpoint.

With the new bridge support, the current distinction between
AssignEndpoint (which will allocate a bridge pool) and SetupEndpoint
(which will not) is a potential point of confusion. Instead, allow
bridge allocations through SetupEndpoint.

We have a new conditional path here: we try the LearnEndpoint (ie, a Get
Endpoint ID, to see if we can use that EID) first, but add a new check
to determine if this is a bridge EID type. Is so, we force the fallback
to Set Endpoint ID, which will allow a bridge EID allocation too.

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

Author: jk-ozlabs

src/mctp-control-spec.h

@@ -279,6 +279,28 @@ struct mctp_ctrl_resp_allocate_eids {
 #define MCTP_SET_EID_ACCEPTED 0x0
 #define MCTP_SET_EID_REJECTED 0x1
 
+/* MCTP Get Endpoint ID request and response fields
+ * See DSP0236 v1.3.0 Table 15.
+ */
+#define MCTP_GET_EID_EP_TYPE_SHIFT 4
+#define MCTP_GET_EID_EP_TYPE_MASK 0x03
+#define GET_MCTP_GET_EID_EP_TYPE(field) \
+	(((field) >> MCTP_GET_EID_EP_TYPE_SHIFT) & MCTP_GET_EID_EP_TYPE_MASK)
+#define MCTP_GET_EID_EP_TYPE_EP 0
+#define MCTP_GET_EID_EP_TYPE_BRIDGE 1
+
+#define MCTP_GET_EID_EID_TYPE_SHIFT 0
+#define MCTP_GET_EID_EID_TYPE_MASK 0x03
+#define GET_MCTP_GET_EID_EID_TYPE(field) \
+	(((field) >> MCTP_GET_EID_EID_TYPE_SHIFT) & MCTP_GET_EID_EID_TYPE_MASK)
+#define MCTP_GET_EID_EID_TYPE_DYNAMIC 0
+/* Static EID is supported, may or may not match current */
+#define MCTP_GET_EID_EID_TYPE_STATIC 1
+/* Current eid is the same as static */
+#define MCTP_GET_EID_EID_TYPE_STATIC_SAME 2
+/* Current eid is different from static */
+#define MCTP_GET_EID_EID_TYPE_STATIC_DIFFERENT 3
+
 #define MCTP_EID_ALLOCATION_STATUS_SHIFT 0x0
 #define MCTP_EID_ALLOCATION_STATUS_MASK 0x3
 #define SET_MCTP_EID_ALLOCATION_STATUS(status)        \

src/mctpd.c

@@ -2383,15 +2383,27 @@ static int method_setup_endpoint(sd_bus_message *call, void *data,
 	/* Get Endpoint ID */
 	rc = get_endpoint_peer(ctx, berr, dest, &peer, NULL);
 	if (rc >= 0 && peer) {
-		if (ctx->verbose)
+		uint8_t ep_type = GET_MCTP_GET_EID_EP_TYPE(peer->endpoint_type);
+		if (ep_type == MCTP_GET_EID_EP_TYPE_BRIDGE) {
 			fprintf(stderr,
-				"%s returning from get_endpoint_peer %s\n",
-				__func__, peer_tostr(peer));
-		peer_path = path_from_peer(peer);
-		if (!peer_path)
-			goto err;
-		return sd_bus_reply_method_return(call, "yisb", peer->eid,
-						  peer->net, peer_path, 0);
+				"SetupEndpoint, eid %d: Get EID "
+				"response indicated a bridge with existing "
+				"EID, reassigning\n",
+				peer->eid);
+			remove_peer(peer);
+			peer = NULL;
+		} else {
+			if (ctx->verbose)
+				fprintf(stderr,
+					"%s returning from get_endpoint_peer %s\n",
+					__func__, peer_tostr(peer));
+			peer_path = path_from_peer(peer);
+			if (!peer_path)
+				goto err;
+			return sd_bus_reply_method_return(call, "yisb",
+							  peer->eid, peer->net,
+							  peer_path, 0);
+		}
 	} else if (rc == -EEXIST) {
 		// EEXISTS is OK, we will assign a new eid instead.
 	} else if (rc < 0) {
@@ -2400,10 +2412,13 @@ static int method_setup_endpoint(sd_bus_message *call, void *data,
 	}
 
 	/* Set Endpoint ID */
-	rc = endpoint_assign_eid(ctx, berr, dest, &peer, 0, false);
+	rc = endpoint_assign_eid(ctx, berr, dest, &peer, 0, true);
 	if (rc < 0)
 		goto err;
 
+	if (peer->pool_size)
+		endpoint_allocate_eids(peer);
+
 	peer_path = path_from_peer(peer);
 	if (!peer_path)
 		goto err;
@@ -2582,6 +2597,14 @@ static int method_learn_endpoint(sd_bus_message *call, void *data,
 	if (!peer)
 		return sd_bus_reply_method_return(call, "yisb", 0, 0, "", 0);
 
+	uint8_t ep_type = GET_MCTP_GET_EID_EP_TYPE(peer->endpoint_type);
+	if (ep_type == MCTP_GET_EID_EP_TYPE_BRIDGE) {
+		warnx("LearnEndpoint, eid %d: Get EID response "
+		      "indicated a bridge with existing EID, "
+		      "but no pool is assignable",
+		      peer->eid);
+	}
+
 	peer_path = path_from_peer(peer);
 	if (!peer_path)
 		goto err;

tests/mctpenv/__init__.py

@@ -361,7 +361,10 @@ async def handle_mctp_control(self, sock, addr, data):
 
             elif opcode == 2:
                 # Get Endpoint ID
-                data = bytes(hdr + [0x00, self.eid, 0x00, 0x00])
+                ep_type = 0
+                if len(self.bridged_eps) > 0:
+                    ep_type = 0x1 << 4
+                data = bytes(hdr + [0x00, self.eid, ep_type, 0x00])
                 await sock.send(raddr, data)
 
             elif opcode == 3:

tests/test_mctpd.py

@@ -1042,6 +1042,33 @@ async def test_bridge_ep_conflict_setup(dbus, mctpd):
         (eid, _, _, _) = await mctp.call_setup_endpoint(dev.lladdr)
         assert eid not in pool_range
 
+""" Test that mctpd will reassign a bridge endpoints (pre-configured) EID
+if necessary to satisfy the bridge pool allocation"""
+async def test_bridge_setup_reassign(dbus, mctpd):
+    iface = mctpd.system.interfaces[0]
+    mctp = await mctpd_mctp_iface_obj(dbus, iface)
+
+    # ep: regular endpoint, will conflict with a bridge pool
+    ep = mctpd.network.endpoints[0]
+    static_eid = 10
+    (eid, _, _, _) = await mctp.call_assign_endpoint_static(
+        ep.lladdr,
+        static_eid
+    )
+
+    assert eid == static_eid
+
+    # br: our bridge
+    conflict_eid = 9
+    br = Endpoint(iface, bytes([ep.lladdr[0] + 1]), eid=conflict_eid)
+    br.add_bridged_ep(Endpoint(iface, bytes()))
+    mctpd.network.add_endpoint(br)
+
+    (eid, _, _, _) = await mctp.call_setup_endpoint(br.lladdr)
+    assert eid != conflict_eid
+    assert br.allocated_pool is not None
+    assert br.allocated_pool[0] == eid + 1
+
 """ Test that we truncate the requested pool size to
     the max_pool_size config """
 async def test_assign_dynamic_eid_limited_pool(nursery, dbus, sysnet):