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 115C05A027A for ; Thu, 3 Aug 2023 04:27:08 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=201602; t=1691029621; bh=DozycYWSFkMm/RDnXxNVtRZsk0hqLVo2U7N5M3QrzX4=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=LxEjMw4pIbbohDKqxPq7y1BtF1uy6sFUtA/SsjJFDrX0iXfEcFYpBquVYsMt007iM /qJuRg30r2lGysi51D1tvci0yatb3FU+o0Jhrja5OHYVHO8m0yNg4CL8coMxizJSlG rCalOUooYvJK6sgIVTHGzY3U4zLDcAIGPZWJRPJ0= Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4RGXmP55pBz4wy5; Thu, 3 Aug 2023 12:27:01 +1000 (AEST) Date: Thu, 3 Aug 2023 12:26:53 +1000 From: David Gibson To: Stefano Brivio Subject: Re: [PATCH 17/17] netlink: Propagate errors for "dup" operations Message-ID: References: <20230724060936.952659-1-david@gibson.dropbear.id.au> <20230724060936.952659-18-david@gibson.dropbear.id.au> <20230803004820.7b7967ac@elisabeth> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="hMHmgi8hi/EhV7/7" Content-Disposition: inline In-Reply-To: <20230803004820.7b7967ac@elisabeth> Message-ID-Hash: WACXHNRUEB5DWYMYLD4F4YJCBY7J37VL X-Message-ID-Hash: WACXHNRUEB5DWYMYLD4F4YJCBY7J37VL 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: passt-dev@passt.top 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: --hMHmgi8hi/EhV7/7 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Thu, Aug 03, 2023 at 12:48:20AM +0200, Stefano Brivio wrote: > On Mon, 24 Jul 2023 16:09:36 +1000 > David Gibson wrote: >=20 > > We now detect errors on netlink "set" operations while configuring the > > pasta namespace with --config-net. However in many cases rather than > > a simple "set" we use a more complex "dup" function to copy > > configuration from the host to the namespace. We're not yet properly > > detecting and reporting netlink errors for that case. > >=20 > > Change the "dup" operations to propagate netlink errors to their > > caller, pasta_ns_conf() and report them there. > >=20 > > Link: https://bugs.passt.top/show_bug.cgi?id=3D60 > >=20 > > Signed-off-by: David Gibson > > --- > > netlink.c | 40 ++++++++++++++++++++++++++++------------ > > netlink.h | 8 ++++---- > > pasta.c | 15 ++++++++------- > > 3 files changed, 40 insertions(+), 23 deletions(-) > >=20 > > diff --git a/netlink.c b/netlink.c > > index 9e72b16..cdc18c0 100644 > > --- a/netlink.c > > +++ b/netlink.c > > @@ -413,9 +413,11 @@ int nl_route_set_def(int s, unsigned int ifi, sa_f= amily_t af, void *gw) > > * @s_dst: Netlink socket in destination namespace > > * @ifi_dst: Interface index in destination namespace > > * @af: Address family > > + * > > + * Return: 0 on success, negative error code on failure > > */ > > -void nl_route_dup(int s_src, unsigned int ifi_src, > > - int s_dst, unsigned int ifi_dst, sa_family_t af) > > +int nl_route_dup(int s_src, unsigned int ifi_src, > > + int s_dst, unsigned int ifi_dst, sa_family_t af) > > { > > struct req_t { > > struct nlmsghdr nlh; > > @@ -477,9 +479,11 @@ void nl_route_dup(int s_src, unsigned int ifi_src, > > =20 > > if (extra) { > > err("netlink: Too many routes to duplicate"); > > - return; > > + return -E2BIG; >=20 > This is "Argument list too long", and... I don't have much better > ideas. I would instinctively use ENOSPC or ENOMEM in this case, but > both are slightly misleading in different ways, too. Right. My main reason for picking this is that it's an obscure error that won't be generated by the underlying netlink operations so it's unambiguous. I'm open to better ideas. > > } > > } > > + if (status < 0) > > + return status; > > =20 > > /* Routes might have dependencies between each other, and the > > * kernel processes RTM_NEWROUTE messages sequentially. For n > > @@ -494,15 +498,20 @@ void nl_route_dup(int s_src, unsigned int ifi_src, > > NLMSG_OK(nh, status); > > nh =3D NLMSG_NEXT(nh, status)) { > > uint16_t flags =3D nh->nlmsg_flags; > > + int rc; > > =20 > > if (nh->nlmsg_type !=3D RTM_NEWROUTE) > > continue; > > =20 > > - nl_do(s_dst, nh, RTM_NEWROUTE, > > - (flags & ~NLM_F_DUMP_FILTERED) | NLM_F_CREATE, > > - nh->nlmsg_len); > > + rc =3D nl_do(s_dst, nh, RTM_NEWROUTE, > > + (flags & ~NLM_F_DUMP_FILTERED) | NLM_F_CREATE, > > + nh->nlmsg_len); > > + if (rc < 0 && rc !=3D -ENETUNREACH && rc !=3D -EEXIST) > > + return rc; > > } > > } > > + > > + return 0; > > } > > =20 > > /** > > @@ -635,9 +644,11 @@ int nl_addr_set(int s, unsigned int ifi, sa_family= _t af, > > * @s_dst: Netlink socket in destination network namespace > > * @ifi_dst: Interface index in destination namespace > > * @af: Address family > > + * > > + * Return: 0 on success, negative error code on failure > > */ > > -void nl_addr_dup(int s_src, unsigned int ifi_src, > > - int s_dst, unsigned int ifi_dst, sa_family_t af) > > +int nl_addr_dup(int s_src, unsigned int ifi_src, > > + int s_dst, unsigned int ifi_dst, sa_family_t af) > > { > > struct req_t { > > struct nlmsghdr nlh; > > @@ -651,6 +662,7 @@ void nl_addr_dup(int s_src, unsigned int ifi_src, > > struct nlmsghdr *nh; > > ssize_t status; > > uint16_t seq; > > + int rc=3D 0; >=20 > Missing whitespace. Fixed. --=20 David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson --hMHmgi8hi/EhV7/7 Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAEBCAAdFiEEO+dNsU4E3yXUXRK2zQJF27ox2GcFAmTLEGcACgkQzQJF27ox 2GfvEg//aC1UajENrTXdWqwTTLgxzpYkoM6yOBC3+V5oN4Kfg/mLlhLaebjW10UF y5JEPXAT8wHZ7PqqpAaYGVbLjLNnzlmvesrP5AEesvoUltTRFPgiNfwZwcesFush d0RFSx+I4wk8tPDBd0Y89YmF9DAczDFU/yj81FFCCzNVsTd7MHLb7oRIXraPAfNo dqr2fkreKvBmIzg5QIgs3rc9WSgJ0VP0b7N7vm41DgKdWp2dj6mhtWtJZ9clKq6T kvkhGjx02kIjOUhPM6W43Cs2Ny3I+BIYDQijSOKBv4chkBI6oHyfROtqZl08UlPc mj0FMuH+FvrSF1MevxSERF+38uoHFj2OstSU5w5Sv0A0qHJhvG0pAKQ/Y4hJT6Jc 56yk+3bCsxnwrIY4g5Bs+RaKIqtF5nScooIRehuQhk7zkwPO8KpLuP7h3hGVRZhO qPIwDx/18+r2pjkgrt9gI30ZOnRFyQ3zf3a998IMS0WpE8yusxaTZQzf4B/M3YDU 6sPrmx/OT10ND6rEBGjOhk61naVfn5BdDQvDRUmdJwWnHAwfzVEgi/AbNlN9eark cnWOW9EzcXu2vOLkjCHM1hrlVP/QimkjIWeCiRLkVBkkqOZvsYg5pPVYJxhgjGqb KipJ6T+TxYxCw2BywdHWC+q75xO8LCE1yrCrOtVaegVSu07EOEA= =dXic -----END PGP SIGNATURE----- --hMHmgi8hi/EhV7/7--