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 <passt-dev@passt.top>; 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 <david@gibson.dropbear.id.au>
To: Stefano Brivio <sbrivio@redhat.com>
Subject: Re: [PATCH 17/17] netlink: Propagate errors for "dup" operations
Message-ID: <ZMsQbTR6Dby49DW6@zatzit>
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 <passt-dev.passt.top>
Archived-At: <https://archives.passt.top/passt-dev/ZMsQbTR6Dby49DW6@zatzit/>
Archived-At: <https://passt.top/hyperkitty/list/passt-dev@passt.top/message/WACXHNRUEB5DWYMYLD4F4YJCBY7J37VL/>
List-Archive: <https://archives.passt.top/passt-dev/>
List-Archive: <https://passt.top/hyperkitty/list/passt-dev@passt.top/>
List-Help: <mailto:passt-dev-request@passt.top?subject=help>
List-Owner: <mailto:passt-dev-owner@passt.top>
List-Post: <mailto:passt-dev@passt.top>
List-Subscribe: <mailto:passt-dev-join@passt.top>
List-Unsubscribe: <mailto:passt-dev-leave@passt.top>
--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 <david@gibson.dropbear.id.au> 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 <david@gibson.dropbear.id.au>
> > ---
> > 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--