Miscellaneous improvements to mctpd

src/mctp-netlink.c

@@ -1096,6 +1096,11 @@ int *mctp_nl_if_list(const mctp_nl *nl, size_t *ret_num_ifs)
 	return ifs;
 }
 
+bool mctp_nl_if_exists(const mctp_nl *nl, int ifindex)
+{
+	return entry_byindex(nl, ifindex) != NULL;
+}
+
 static int linkmap_add_entry(mctp_nl *nl, struct ifinfomsg *info,
 		const char *ifname, size_t ifname_len, uint32_t net,
 		bool up, uint32_t min_mtu, uint32_t max_mtu, size_t hwaddr_len)
@@ -1140,6 +1145,64 @@ static int linkmap_add_entry(mctp_nl *nl, struct ifinfomsg *info,
 	return 0;
 }
 
+/* Common parts of RTM_NEWADDR and RTM_DELADDR */
+struct mctp_addralter_msg {
+	struct nlmsghdr nh;
+	struct ifaddrmsg ifmsg;
+	struct rtattr rta;
+	uint8_t data[4];
+};
+static int fill_addralter_args(struct mctp_nl *nl,
+			       struct mctp_addralter_msg *msg,
+			       struct rtattr **prta, size_t *prta_len,
+			       mctp_eid_t eid, int ifindex)
+{
+	memset(msg, 0x0, sizeof(*msg));
+
+	msg->nh.nlmsg_flags = NLM_F_REQUEST | NLM_F_ACK;
+
+	msg->ifmsg.ifa_index = ifindex;
+	msg->ifmsg.ifa_family = AF_MCTP;
+
+	msg->rta.rta_type = IFA_LOCAL;
+	msg->rta.rta_len = RTA_LENGTH(sizeof(eid));
+	memcpy(RTA_DATA(&msg->rta), &eid, sizeof(eid));
+
+	msg->nh.nlmsg_len =
+		NLMSG_LENGTH(sizeof(msg->ifmsg)) + RTA_SPACE(sizeof(eid));
+
+	if (prta)
+		*prta = &msg->rta;
+	if (prta_len)
+		*prta_len = msg->rta.rta_len;
+
+	return 0;
+}
+
+int mctp_nl_addr(struct mctp_nl *nl, mctp_eid_t eid, int ifindex, int rtm_command)
+{
+	struct mctp_addralter_msg msg;
+	int rc;
+
+	rc = fill_addralter_args(nl, &msg, NULL, NULL, eid, ifindex);
+	if (rc)
+		return -1;
+
+	msg.nh.nlmsg_type = rtm_command;
+
+	return mctp_nl_send(nl, &msg.nh);
+}
+
+int mctp_nl_addr_add(struct mctp_nl *nl, mctp_eid_t eid, int ifindex)
+{
+	return mctp_nl_addr(nl, eid, ifindex, RTM_NEWADDR);
+}
+
+int mctp_nl_addr_del(struct mctp_nl *nl, mctp_eid_t eid, int ifindex)
+{
+	return mctp_nl_addr(nl, eid, ifindex, RTM_DELADDR);
+}
+
 /* Common parts of RTM_NEWROUTE and RTM_DELROUTE */
 struct mctp_rtalter_msg {
 	struct nlmsghdr		nh;
@@ -1151,19 +1214,12 @@ struct mctp_rtalter_msg {
 				];
 };
 static int fill_rtalter_args(struct mctp_nl *nl, struct mctp_rtalter_msg *msg,
-	struct rtattr **prta, size_t *prta_len,
-	mctp_eid_t eid, const char* linkstr)
+			     struct rtattr **prta, size_t *prta_len,
+			     mctp_eid_t eid, int ifindex)
 {
-	int ifindex;
 	struct rtattr *rta;
 	size_t rta_len;
 
-	ifindex = mctp_nl_ifindex_byname(nl, linkstr);
-	if (!ifindex) {
-		warnx("invalid device %s", linkstr);
-		return -1;
-	}
-
 	memset(msg, 0x0, sizeof(*msg));
 	msg->nh.nlmsg_flags = NLM_F_REQUEST | NLM_F_ACK;
 
@@ -1190,14 +1246,15 @@ static int fill_rtalter_args(struct mctp_nl *nl, struct mctp_rtalter_msg *msg,
 	return 0;
 }
 
-int mctp_nl_route_add(struct mctp_nl *nl, uint8_t eid, const char* ifname,
-		uint32_t mtu) {
+int mctp_nl_route_add(struct mctp_nl *nl, uint8_t eid, int ifindex,
+		      uint32_t mtu)
+{
 	struct mctp_rtalter_msg msg;
 	struct rtattr *rta;
 	size_t rta_len;
 	int rc;
 
-	rc = fill_rtalter_args(nl, &msg, &rta, &rta_len, eid, ifname);
+	rc = fill_rtalter_args(nl, &msg, &rta, &rta_len, eid, ifindex);
 	if (rc) {
 		return -1;
 	}
@@ -1223,20 +1280,18 @@ int mctp_nl_route_add(struct mctp_nl *nl, uint8_t eid, const char* ifname,
 	}
 
 	return mctp_nl_send(nl, &msg.nh);
-
 }
 
-int mctp_nl_route_del(struct mctp_nl *nl, uint8_t eid, const char* ifname)
+int mctp_nl_route_del(struct mctp_nl *nl, uint8_t eid, int ifindex)
 {
 	struct mctp_rtalter_msg msg;
 	int rc;
 
-	rc = fill_rtalter_args(nl, &msg, NULL, NULL, eid, ifname);
+	rc = fill_rtalter_args(nl, &msg, NULL, NULL, eid, ifindex);
 	if (rc) {
 		return rc;
 	}
 	msg.nh.nlmsg_type = RTM_DELROUTE;
 
 	return mctp_nl_send(nl, &msg.nh);
 }
-

src/mctp-netlink.h

@@ -78,6 +78,7 @@ void mctp_nl_linkmap_dump(const mctp_nl *nl);
 uint32_t *mctp_nl_net_list(const mctp_nl *nl, size_t *ret_num_nets);
 /* Returns an allocated list of ifindex, caller to free */
 int *mctp_nl_if_list(const mctp_nl *nl, size_t *ret_num_if);
+bool mctp_nl_if_exists(const mctp_nl *nl, int ifindex);
 
 /* Get/set userdata for a link. The userdata is attached to a link
  * with index @ifindex. Userdata will also be populated into
@@ -93,10 +94,15 @@ void *mctp_nl_get_link_userdata(const mctp_nl *nl, int ifindex);
 /* Returns NULL if the link does not exist */
 void *mctp_nl_get_link_userdata_byname(const mctp_nl *nl, const char *ifname);
 
+/* MCTP addr helper */
+int mctp_nl_addr(struct mctp_nl *nl, uint8_t eid, int ifindex, int rtm_command);
+int mctp_nl_addr_add(struct mctp_nl *nl, uint8_t eid, int ifindex);
+int mctp_nl_addr_del(struct mctp_nl *nl, uint8_t eid, int ifindex);
+
 /* MCTP route helper */
-int mctp_nl_route_add(struct mctp_nl *nl, uint8_t eid, const char* ifname,
-		uint32_t mtu);
-int mctp_nl_route_del(struct mctp_nl *nl, uint8_t eid, const char* ifname);
+int mctp_nl_route_add(struct mctp_nl *nl, uint8_t eid, int ifindex,
+		      uint32_t mtu);
+int mctp_nl_route_del(struct mctp_nl *nl, uint8_t eid, int ifindex);
 
 /* Helpers */
 

src/mctp.c

@@ -748,12 +748,6 @@ static int cmd_addr_addremove(struct ctx *ctx,
 	const char* cmdname, int rtm_command,
 	int argc, const char **argv)
 {
-	struct {
-		struct nlmsghdr		nh;
-		struct ifaddrmsg	ifmsg;
-		struct rtattr		rta;
-		uint8_t			data[4];
-	} msg = {0};
 	const char *eidstr, *linkstr;
 	uint32_t tmp;
 	uint8_t eid;
@@ -786,21 +780,7 @@ static int cmd_addr_addremove(struct ctx *ctx,
 	}
 	eid = tmp & 0xff;
 
-	msg.nh.nlmsg_type = rtm_command;
-	// request an error status since there's no other reply
-	msg.nh.nlmsg_flags = NLM_F_REQUEST | NLM_F_ACK;
-
-	msg.ifmsg.ifa_index = ifindex;
-	msg.ifmsg.ifa_family = AF_MCTP;
-
-	msg.rta.rta_type = IFA_LOCAL;
-	msg.rta.rta_len = RTA_LENGTH(sizeof(eid));
-	memcpy(RTA_DATA(&msg.rta), &eid, sizeof(eid));
-
-	msg.nh.nlmsg_len = NLMSG_LENGTH(sizeof(msg.ifmsg)) +
-			RTA_SPACE(sizeof(eid));
-
-	return mctp_nl_send(ctx->nl, &msg.nh);
+	return mctp_nl_addr(ctx->nl, eid, ifindex, rtm_command);
 }
 
 static int cmd_addr_add(struct ctx *ctx, int argc, const char **argv)
@@ -874,6 +854,7 @@ static int cmd_route_add(struct ctx *ctx, int argc, const char **argv)
 {
 	const char *eidstr = NULL, *linkstr = NULL, *mtustr = NULL;
 	uint32_t mtu = 0, eid = 0;
+	int ifindex = 0;
 	int rc = 0;
 
 	if (!(argc == 4 || argc == 6)) {
@@ -906,18 +887,24 @@ static int cmd_route_add(struct ctx *ctx, int argc, const char **argv)
 		warnx("Bad eid");
 		rc = -EINVAL;
 	}
+	ifindex = mctp_nl_ifindex_byname(ctx->nl, linkstr);
+	if (!ifindex) {
+		warnx("add: invalid device %s", linkstr);
+		rc = -EINVAL;
+	}
 	if (rc) {
 		warnx("add: invalid command line arguments");
 		return -1;
 	}
 
-	return mctp_nl_route_add(ctx->nl, eid, linkstr, mtu);
+	return mctp_nl_route_add(ctx->nl, eid, ifindex, mtu);
 }
 
 static int cmd_route_del(struct ctx *ctx, int argc, const char **argv)
 {
-	const char *linkstr = NULL, *eidstr = NULL;
+	const char *eidstr = NULL;
 	uint32_t tmp = 0;
+	int ifindex = 0;
 	uint8_t eid;
 	int rc = 0;
 
@@ -936,14 +923,18 @@ static int cmd_route_del(struct ctx *ctx, int argc, const char **argv)
 		warnx("Bad eid");
 		rc = -EINVAL;
 	}
+	ifindex = mctp_nl_ifindex_byname(ctx->nl, argv[3]);
+	if (!ifindex) {
+		warnx("del: invalid device %s", argv[3]);
+		rc = -EINVAL;
+	}
 	if (rc) {
 		warnx("del: invalid command line arguments");
 		return -1;
 	}
 	eid = tmp & 0xff;
-	linkstr = argv[3];
 
-	return mctp_nl_route_del(ctx->nl, eid, linkstr);
+	return mctp_nl_route_del(ctx->nl, eid, ifindex);
 }
 
 static int cmd_route(struct ctx *ctx, int argc, const char **argv)

src/mctpd.c

@@ -570,6 +570,29 @@ static int read_message(struct ctx *ctx, int sd, uint8_t **ret_buf, size_t *ret_
 	return rc;
 }
 
+/* Replies to a physical address */
+static int reply_message_phys(struct ctx *ctx, int sd, const void *resp, size_t resp_len,
+	const struct sockaddr_mctp_ext *addr)
+{
+	ssize_t len;
+	struct sockaddr_mctp_ext reply_addr = *addr;
+
+	reply_addr.smctp_base.smctp_tag &= ~MCTP_TAG_OWNER;
+
+	len = mctp_ops.mctp.sendto(sd, resp, resp_len, 0,
+				   (struct sockaddr *)&reply_addr,
+				   sizeof(reply_addr));
+	if (len < 0) {
+		return -errno;
+	}
+
+	if ((size_t)len != resp_len) {
+		bug_warn("short sendto %zd, expected %zu", len, resp_len);
+		return -EPROTO;
+	}
+	return 0;
+}
+
 /* Replies to a real EID, not physical addressing */
 static int reply_message(struct ctx *ctx, int sd, const void *resp, size_t resp_len,
 	const struct sockaddr_mctp_ext *addr)
@@ -694,7 +717,8 @@ static int handle_control_get_endpoint_id(struct ctx *ctx,
 	SET_ENDPOINT_ID_TYPE(resp->eid_type, 2);
 	// TODO: medium specific information
 
-	return reply_message(ctx, sd, resp, sizeof(*resp), addr);
+	// Get Endpoint ID is typically send and reply using physical addressing.
+	return reply_message_phys(ctx, sd, resp, sizeof(*resp), addr);
 }
 
 static int handle_control_get_endpoint_uuid(struct ctx *ctx,
@@ -1545,26 +1569,25 @@ static int change_peer_eid(struct peer *peer, mctp_eid_t new_eid)
 	return 0;
 }
 
-static int peer_set_mtu(struct ctx *ctx, struct peer *peer, uint32_t mtu) {
-	const char* ifname = NULL;
+static int peer_set_mtu(struct ctx *ctx, struct peer *peer, uint32_t mtu)
+{
 	int rc;
 
-	ifname = mctp_nl_if_byindex(ctx->nl, peer->phys.ifindex);
-	if (!ifname) {
+	if (!mctp_nl_if_exists(peer->ctx->nl, peer->phys.ifindex)) {
 		bug_warn("%s: no interface for ifindex %d",
 			__func__, peer->phys.ifindex);
 		return -EPROTO;
 	}
 
-	rc = mctp_nl_route_del(ctx->nl, peer->eid, ifname);
+	rc = mctp_nl_route_del(ctx->nl, peer->eid, peer->phys.ifindex);
 	if (rc < 0 && rc != -ENOENT) {
 		warnx("%s, Failed removing existing route for eid %d %s",
-			__func__,
-			peer->phys.ifindex, ifname);
+		      __func__, peer->phys.ifindex,
+		      mctp_nl_if_byindex(ctx->nl, peer->phys.ifindex));
 		// Continue regardless, route_add will likely fail with EEXIST
 	}
 
-	rc = mctp_nl_route_add(ctx->nl, peer->eid, ifname, mtu);
+	rc = mctp_nl_route_add(ctx->nl, peer->eid, peer->phys.ifindex, mtu);
 	if (rc >= 0) {
 		peer->mtu = mtu;
 	}
@@ -2254,19 +2277,17 @@ static int peer_neigh_update(struct peer *peer, uint16_t type)
 // type is RTM_NEWROUTE or RTM_DELROUTE
 static int peer_route_update(struct peer *peer, uint16_t type)
 {
-	const char * link;
-
-	link = mctp_nl_if_byindex(peer->ctx->nl, peer->phys.ifindex);
-	if (!link) {
+	if (!mctp_nl_if_exists(peer->ctx->nl, peer->phys.ifindex)) {
 		bug_warn("%s: Unknown ifindex %d", __func__, peer->phys.ifindex);
 		return -ENODEV;
 	}
 
 	if (type == RTM_NEWROUTE) {
-		return mctp_nl_route_add(peer->ctx->nl,
-			peer->eid, link, peer->mtu);
+		return mctp_nl_route_add(peer->ctx->nl, peer->eid,
+					 peer->phys.ifindex, peer->mtu);
 	} else if (type == RTM_DELROUTE) {
-		return mctp_nl_route_del(peer->ctx->nl, peer->eid, link);
+		return mctp_nl_route_del(peer->ctx->nl, peer->eid,
+					 peer->phys.ifindex);
 	}
 
 	bug_warn("%s: bad type %d", __func__, type);