From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from gandalf.ozlabs.org (mail.ozlabs.org [IPv6:2404:9400:2221:ea00::3]) by passt.top (Postfix) with ESMTPS id 4436D5A027C for ; Mon, 24 Jul 2023 08:09:47 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=201602; t=1690178979; bh=i+mp8m8P1qWp2hDh9SPNt7YEvH95bkjvCEVWD/rmsGY=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=BE+HzHDykNBF/LhvsJXP/VMoNT7LWCwvVMg453LuFs7QdLWa8Q6DoZP20sQiZ+5qv Wf8vqv9UjYMH6EiCJmJiuSHBx58v91yXWKJmql2+Mz4LtstQJTu46k1Vihw0Yn9A48 LK2JnKK3aedCesCwWctHgSF/+uEgceAOXimbsncg= Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4R8V9v56NMz4wyk; Mon, 24 Jul 2023 16:09:39 +1000 (AEST) From: David Gibson To: Stefano Brivio , passt-dev@passt.top Subject: [PATCH 10/17] netlink: Add nl_do() helper for simple operations with error checking Date: Mon, 24 Jul 2023 16:09:29 +1000 Message-ID: <20230724060936.952659-11-david@gibson.dropbear.id.au> X-Mailer: git-send-email 2.41.0 In-Reply-To: <20230724060936.952659-1-david@gibson.dropbear.id.au> References: <20230724060936.952659-1-david@gibson.dropbear.id.au> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Message-ID-Hash: Y4DZVDPRBATYGE77HUREGBJKFG4AJBQL X-Message-ID-Hash: Y4DZVDPRBATYGE77HUREGBJKFG4AJBQL X-MailFrom: dgibson@gandalf.ozlabs.org X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; emergency; loop; banned-address; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header CC: David Gibson X-Mailman-Version: 3.3.8 Precedence: list List-Id: Development discussion and patches for passt Archived-At: Archived-At: List-Archive: List-Archive: List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: So far we never checked for errors reported on netlink operations via NLMSG_ERROR messages. This has led to several subtle and tricky to debug situations which would have been obvious if we knew that certain netlink operations had failed. Introduce a nl_do() helper that performs netlink "do" operations (that is making a single change without retreiving complex information) with much more thorough error checking. As well as returning an error code if we get an NLMSG_ERROR message, we also check for unexpected behaviour in several places. That way if we've made a mistake in our assumptions about how netlink works it should result in a clear error rather than some subtle misbehaviour. We update those calls to nl_req() that can use the new wrapper to do so. We will extend those to better handle errors in future. We don't touch non-"do" operations for now, those are a bit trickier. Link: https://bugs.passt.top/show_bug.cgi?id=60 Signed-off-by: David Gibson --- netlink.c | 59 ++++++++++++++++++++++++++++++++++++++++++++----------- 1 file changed, 47 insertions(+), 12 deletions(-) diff --git a/netlink.c b/netlink.c index 3170344..cdd65c0 100644 --- a/netlink.c +++ b/netlink.c @@ -148,6 +148,47 @@ static ssize_t nl_req(int s, char *buf, void *req, return n; } +/** + * nl_do() - Send netlink "do" request, and wait for acknowledgement + * @s: Netlink socket + * @req: Request (will fill netlink header) + * @type: Request type + * @flags: Extra request flags (NLM_F_REQUEST and NLM_F_ACK assumed) + * @len: Request length + * + * Return: 0 on success, negative error code on error + */ +static int nl_do(int s, void *req, uint16_t type, uint16_t flags, ssize_t len) +{ + struct nlmsghdr *nh; + char buf[NLBUFSIZ]; + uint16_t seq; + ssize_t n; + + n = nl_req(s, buf, req, type, flags, len); + seq = ((struct nlmsghdr *)req)->nlmsg_seq; + + for (nh = (struct nlmsghdr *)buf; + NLMSG_OK(nh, n); nh = NLMSG_NEXT(nh, n)) { + struct nlmsgerr *errmsg; + + if (nh->nlmsg_seq != seq) + die("netlink: Unexpected response sequence number"); + + switch (nh->nlmsg_type) { + case NLMSG_DONE: + return 0; + case NLMSG_ERROR: + errmsg = (struct nlmsgerr *)NLMSG_DATA(nh); + return errmsg->error; + default: + warn("netlink: Unexpected response message"); + } + } + + die("netlink: Missing acknowledgement of request"); +} + /** * nl_get_ext_if() - Get interface index supporting IP version being probed * @s: Netlink socket @@ -289,7 +330,6 @@ void nl_route_set_def(int s, unsigned int ifi, sa_family_t af, void *gw) .rta.rta_len = RTA_LENGTH(sizeof(unsigned int)), .ifi = ifi, }; - char buf[NLBUFSIZ]; ssize_t len; if (af == AF_INET6) { @@ -316,7 +356,7 @@ void nl_route_set_def(int s, unsigned int ifi, sa_family_t af, void *gw) req.set.r4.rta_gw.rta_len = rta_len; } - nl_req(s, buf, &req, RTM_NEWROUTE, NLM_F_CREATE | NLM_F_EXCL, len); + nl_do(s, &req, RTM_NEWROUTE, NLM_F_CREATE | NLM_F_EXCL, len); } /** @@ -386,12 +426,11 @@ void nl_route_dup(int s_src, unsigned int ifi_src, NLMSG_OK(nh, n) && nh->nlmsg_type != NLMSG_DONE; nh = NLMSG_NEXT(nh, n)) { uint16_t flags = nh->nlmsg_flags; - char resp[NLBUFSIZ]; if (nh->nlmsg_type != RTM_NEWROUTE) continue; - nl_req(s_dst, resp, nh, RTM_NEWROUTE, + nl_do(s_dst, nh, RTM_NEWROUTE, (flags & ~NLM_F_DUMP_FILTERED) | NLM_F_CREATE, nh->nlmsg_len); } @@ -490,7 +529,6 @@ void nl_addr_set(int s, unsigned int ifi, sa_family_t af, .ifa.ifa_prefixlen = prefix_len, .ifa.ifa_scope = RT_SCOPE_UNIVERSE, }; - char buf[NLBUFSIZ]; ssize_t len; if (af == AF_INET6) { @@ -519,7 +557,7 @@ void nl_addr_set(int s, unsigned int ifi, sa_family_t af, req.set.a4.rta_a.rta_type = IFA_ADDRESS; } - nl_req(s, buf, &req, RTM_NEWADDR, NLM_F_CREATE | NLM_F_EXCL, len); + nl_do(s, &req, RTM_NEWADDR, NLM_F_CREATE | NLM_F_EXCL, len); } /** @@ -551,7 +589,6 @@ void nl_addr_dup(int s_src, unsigned int ifi_src, NLMSG_OK(nh, n) && nh->nlmsg_type != NLMSG_DONE; nh = NLMSG_NEXT(nh, n)) { struct ifaddrmsg *ifa; - char resp[NLBUFSIZ]; struct rtattr *rta; size_t na; @@ -572,7 +609,7 @@ void nl_addr_dup(int s_src, unsigned int ifi_src, rta->rta_type = IFA_UNSPEC; } - nl_req(s_dst, resp, nh, RTM_NEWADDR, + nl_do(s_dst, nh, RTM_NEWADDR, (nh->nlmsg_flags & ~NLM_F_DUMP_FILTERED) | NLM_F_CREATE, nh->nlmsg_len); } @@ -640,11 +677,10 @@ void nl_link_set_mac(int s, unsigned int ifi, void *mac) .rta.rta_type = IFLA_ADDRESS, .rta.rta_len = RTA_LENGTH(ETH_ALEN), }; - char buf[NLBUFSIZ]; memcpy(req.mac, mac, ETH_ALEN); - nl_req(s, buf, &req, RTM_NEWLINK, 0, sizeof(req)); + nl_do(s, &req, RTM_NEWLINK, 0, sizeof(req)); } /** @@ -670,11 +706,10 @@ void nl_link_up(int s, unsigned int ifi, int mtu) .mtu = mtu, }; ssize_t len = sizeof(req); - char buf[NLBUFSIZ]; if (!mtu) /* Shorten request to drop MTU attribute */ len = offsetof(struct req_t, rta); - nl_req(s, buf, &req, RTM_NEWLINK, 0, len); + nl_do(s, &req, RTM_NEWLINK, 0, len); } -- 2.41.0