From: Stefano Brivio <sbrivio@redhat.com>
To: passt-dev@passt.top
Subject: [PATCH 2/3] tcp, udp: Fix partial success return codes in {tcp,udp}_sock_init()
Date: Wed, 8 Mar 2023 13:33:47 +0100 [thread overview]
Message-ID: <20230308123348.2232214-3-sbrivio@redhat.com> (raw)
In-Reply-To: <20230308123348.2232214-1-sbrivio@redhat.com>
The comments say we should return 0 on partial success, and an error
code on complete failure. Rationale: if the user configures a port
forwarding, and we succeed to bind that port for IPv4 or IPv6 only,
that might actually be what the user intended.
Adjust the two functions to reflect the comments.
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
---
tcp.c | 21 +++++++++------------
udp.c | 30 ++++++++++++++----------------
2 files changed, 23 insertions(+), 28 deletions(-)
diff --git a/tcp.c b/tcp.c
index e209483..a29e387 100644
--- a/tcp.c
+++ b/tcp.c
@@ -2993,7 +2993,7 @@ static int tcp_sock_init_af(const struct ctx *c, int af, in_port_t port,
int tcp_sock_init(const struct ctx *c, sa_family_t af, const void *addr,
const char *ifname, in_port_t port)
{
- int ret = 0, af_ret;
+ int r4 = SOCKET_MAX + 1, r6 = SOCKET_MAX + 1;
if (af == AF_UNSPEC && c->ifi4 && c->ifi6)
/* Attempt to get a dual stack socket */
@@ -3001,19 +3001,16 @@ int tcp_sock_init(const struct ctx *c, sa_family_t af, const void *addr,
return 0;
/* Otherwise create a socket per IP version */
- if ((af == AF_INET || af == AF_UNSPEC) && c->ifi4) {
- af_ret = tcp_sock_init_af(c, AF_INET, port, addr, ifname);
- if (af_ret < 0)
- ret = af_ret;
- }
+ if ((af == AF_INET || af == AF_UNSPEC) && c->ifi4)
+ r4 = tcp_sock_init_af(c, AF_INET, port, addr, ifname);
- if ((af == AF_INET6 || af == AF_UNSPEC) && c->ifi6) {
- af_ret = tcp_sock_init_af(c, AF_INET6, port, addr, ifname);
- if (af_ret < 0)
- ret = af_ret;
- }
+ if ((af == AF_INET6 || af == AF_UNSPEC) && c->ifi6)
+ r6 = tcp_sock_init_af(c, AF_INET6, port, addr, ifname);
- return ret;
+ if (IN_INTERVAL(0, SOCKET_MAX, r4) || IN_INTERVAL(0, SOCKET_MAX, r6))
+ return 0;
+
+ return r4 < 0 ? r4 : r6;
}
/**
diff --git a/udp.c b/udp.c
index 41e0afd..a5f7537 100644
--- a/udp.c
+++ b/udp.c
@@ -983,7 +983,7 @@ int udp_sock_init(const struct ctx *c, int ns, sa_family_t af,
const void *addr, const char *ifname, in_port_t port)
{
union udp_epoll_ref uref = { .u32 = 0 };
- int s, ret = 0;
+ int s, r4 = SOCKET_MAX + 1, r6 = SOCKET_MAX + 1;
if (ns) {
uref.udp.port = (in_port_t)(port +
@@ -999,8 +999,8 @@ int udp_sock_init(const struct ctx *c, int ns, sa_family_t af,
uref.udp.orig = true;
if (!ns) {
- s = sock_l4(c, AF_INET, IPPROTO_UDP, addr, ifname,
- port, uref.u32);
+ r4 = s = sock_l4(c, AF_INET, IPPROTO_UDP, addr,
+ ifname, port, uref.u32);
udp_tap_map[V4][uref.udp.port].sock = s < 0 ? -1 : s;
udp_splice_init[V4][port].sock = s < 0 ? -1 : s;
@@ -1008,13 +1008,10 @@ int udp_sock_init(const struct ctx *c, int ns, sa_family_t af,
struct in_addr loopback = { htonl(INADDR_LOOPBACK) };
uref.udp.ns = true;
- s = sock_l4(c, AF_INET, IPPROTO_UDP, &loopback,
- ifname, port, uref.u32);
+ r4 = s = sock_l4(c, AF_INET, IPPROTO_UDP, &loopback,
+ ifname, port, uref.u32);
udp_splice_ns[V4][port].sock = s < 0 ? -1 : s;
}
-
- if (s < 0)
- ret = s;
}
if ((af == AF_INET6 || af == AF_UNSPEC) && c->ifi6) {
@@ -1023,24 +1020,25 @@ int udp_sock_init(const struct ctx *c, int ns, sa_family_t af,
uref.udp.orig = true;
if (!ns) {
- s = sock_l4(c, AF_INET6, IPPROTO_UDP, addr, ifname,
- port, uref.u32);
+ r6 = s = sock_l4(c, AF_INET6, IPPROTO_UDP, addr,
+ ifname, port, uref.u32);
udp_tap_map[V6][uref.udp.port].sock = s < 0 ? -1 : s;
udp_splice_init[V6][port].sock = s < 0 ? -1 : s;
} else {
uref.udp.ns = true;
- s = sock_l4(c, AF_INET6, IPPROTO_UDP, &in6addr_loopback,
- ifname, port, uref.u32);
+ r6 = s = sock_l4(c, AF_INET6, IPPROTO_UDP,
+ &in6addr_loopback,
+ ifname, port, uref.u32);
udp_splice_ns[V6][port].sock = s < 0 ? -1 : s;
}
-
- if (s < 0)
- ret = s;
}
- return ret;
+ if (IN_INTERVAL(0, SOCKET_MAX, r4) || IN_INTERVAL(0, SOCKET_MAX, r6))
+ return 0;
+
+ return r4 < 0 ? r4 : r6;
}
/**
--
@@ -983,7 +983,7 @@ int udp_sock_init(const struct ctx *c, int ns, sa_family_t af,
const void *addr, const char *ifname, in_port_t port)
{
union udp_epoll_ref uref = { .u32 = 0 };
- int s, ret = 0;
+ int s, r4 = SOCKET_MAX + 1, r6 = SOCKET_MAX + 1;
if (ns) {
uref.udp.port = (in_port_t)(port +
@@ -999,8 +999,8 @@ int udp_sock_init(const struct ctx *c, int ns, sa_family_t af,
uref.udp.orig = true;
if (!ns) {
- s = sock_l4(c, AF_INET, IPPROTO_UDP, addr, ifname,
- port, uref.u32);
+ r4 = s = sock_l4(c, AF_INET, IPPROTO_UDP, addr,
+ ifname, port, uref.u32);
udp_tap_map[V4][uref.udp.port].sock = s < 0 ? -1 : s;
udp_splice_init[V4][port].sock = s < 0 ? -1 : s;
@@ -1008,13 +1008,10 @@ int udp_sock_init(const struct ctx *c, int ns, sa_family_t af,
struct in_addr loopback = { htonl(INADDR_LOOPBACK) };
uref.udp.ns = true;
- s = sock_l4(c, AF_INET, IPPROTO_UDP, &loopback,
- ifname, port, uref.u32);
+ r4 = s = sock_l4(c, AF_INET, IPPROTO_UDP, &loopback,
+ ifname, port, uref.u32);
udp_splice_ns[V4][port].sock = s < 0 ? -1 : s;
}
-
- if (s < 0)
- ret = s;
}
if ((af == AF_INET6 || af == AF_UNSPEC) && c->ifi6) {
@@ -1023,24 +1020,25 @@ int udp_sock_init(const struct ctx *c, int ns, sa_family_t af,
uref.udp.orig = true;
if (!ns) {
- s = sock_l4(c, AF_INET6, IPPROTO_UDP, addr, ifname,
- port, uref.u32);
+ r6 = s = sock_l4(c, AF_INET6, IPPROTO_UDP, addr,
+ ifname, port, uref.u32);
udp_tap_map[V6][uref.udp.port].sock = s < 0 ? -1 : s;
udp_splice_init[V6][port].sock = s < 0 ? -1 : s;
} else {
uref.udp.ns = true;
- s = sock_l4(c, AF_INET6, IPPROTO_UDP, &in6addr_loopback,
- ifname, port, uref.u32);
+ r6 = s = sock_l4(c, AF_INET6, IPPROTO_UDP,
+ &in6addr_loopback,
+ ifname, port, uref.u32);
udp_splice_ns[V6][port].sock = s < 0 ? -1 : s;
}
-
- if (s < 0)
- ret = s;
}
- return ret;
+ if (IN_INTERVAL(0, SOCKET_MAX, r4) || IN_INTERVAL(0, SOCKET_MAX, r6))
+ return 0;
+
+ return r4 < 0 ? r4 : r6;
}
/**
--
2.39.2
next prev parent reply other threads:[~2023-03-08 12:33 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-03-08 12:33 [PATCH 0/3] Fail gracefully on too many open files Stefano Brivio
2023-03-08 12:33 ` [PATCH 1/3] tcp, udp, util: Pass socket creation errors all the way up Stefano Brivio
2023-03-08 22:16 ` David Gibson
2023-03-08 12:33 ` Stefano Brivio [this message]
2023-03-08 22:43 ` [PATCH 2/3] tcp, udp: Fix partial success return codes in {tcp,udp}_sock_init() David Gibson
2023-03-08 12:33 ` [PATCH 3/3] conf: Terminate on EMFILE or ENFILE on sockets for port mapping Stefano Brivio
2023-03-08 22:45 ` David Gibson
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20230308123348.2232214-3-sbrivio@redhat.com \
--to=sbrivio@redhat.com \
--cc=passt-dev@passt.top \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://passt.top/passt
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for IMAP folder(s).