The most pressing problem fixed here is that spliced connections with
a remapped destination port would be attempted on the wrong side. This
is an old issue as indicated by the Fixes: tag.
Patch 1/3 restores sanity in comments before we attempt to fix the
issue, patch 2/3 fixes the actual issue, and patch 3/3 introduces
a minor rework on top to fix another issue, where if the user
explicitly configures a loopback address in a port binding we would
still create a non-spliced socket stealing the …
[View More]related connections.
Stefano Brivio (3):
tcp, tcp_splice: Adjust comments to current meaning of inbound and
outbound
tcp, tcp_splice: Fix port remapping for inbound, spliced connections
tcp: Don't create 'tap' socket for ports that are bound to loopback
only
tcp.c | 187 ++++++++++++++++++++++++++++++++-------------------
tcp_splice.c | 20 ++++--
util.h | 3 +
3 files changed, 134 insertions(+), 76 deletions(-)
--
2.35.1
[View Less]
Signed-off-by: Stefano Brivio <sbrivio(a)redhat.com>
---
conf.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/conf.c b/conf.c
index c2634fc..b1b7c63 100644
--- a/conf.c
+++ b/conf.c
@@ -693,14 +693,14 @@ static void usage(const char *name)
info( " default: use search list from /etc/resolv.conf");
if (strstr(name, "pasta"))
- info(" --dhcp-dns: \tPass DNS list via DHCP/DHCPv6/NDP");
+ info(" --dhcp-dns \tPass DNS list via DHCP/DHCPv6/NDP");
…
[View More]else
- info(" --no-dhcp-dns: No DNS list in DHCP/DHCPv6/NDP");
+ info(" --no-dhcp-dns No DNS list in DHCP/DHCPv6/NDP");
if (strstr(name, "pasta"))
- info(" --dhcp-search: Pass list via DHCP/DHCPv6/NDP");
+ info(" --dhcp-search Pass list via DHCP/DHCPv6/NDP");
else
- info(" --no-dhcp-search: No list in DHCP/DHCPv6/NDP");
+ info(" --no-dhcp-search No list in DHCP/DHCPv6/NDP");
info( " --dns-forward ADDR Forward DNS queries sent to ADDR");
info( " can be specified zero to two times (for IPv4 and IPv6)");
--
2.35.1
[View Less]
With default options, when we pass --config-net, the IPv6 address is
actually going to be recycled from the init namespace, so it is in
fact duplicated, but duplicate address detection has no way to find
out.
With a different configured address, that's not the case, but anyway
duplicate address detection will be unable to see this.
In both cases, we're wasting time for nothing.
Pass the IFA_F_NODAD flag as we configure globally scoped IPv6
addresses via netlink.
Signed-off-by: Stefano …
[View More]Brivio <sbrivio(a)redhat.com>
---
netlink.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/netlink.c b/netlink.c
index 9719e91..6e5a96b 100644
--- a/netlink.c
+++ b/netlink.c
@@ -343,6 +343,9 @@ void nl_addr(int ns, unsigned int ifi, sa_family_t af,
if (af == AF_INET6) {
size_t rta_len = RTA_LENGTH(sizeof(req.set.a6.l));
+ /* By default, strictly speaking, it's duplicated */
+ req.ifa.ifa_flags = IFA_F_NODAD;
+
req.nlh.nlmsg_len = offsetof(struct req_t, set.a6)
+ sizeof(req.set.a6);
--
2.35.1
[View Less]
Patches 1/7 to 4/7 add support for logging to a file, via
-l/--log-file, with mandatory size limit and rotation.
Patches 5/7 and 7/7 fix two minor details that came up while
implementing the feature itself.
Patch 6/7 adds a --version option with a version string generated
by the Makefile using git, if available, and includes the version
string in the log header.
v3:
- Further changes for 4/7 and 6/7 reported in change messages
v2:
- Drop patch adding new test from the series (8/8), …
[View More]subject to
further discussion
- Changes for 4/7 and 6/7 reported in change messages
Stefano Brivio (7):
Move logging functions to a new file, log.c
conf: Drop duplicate, diverging optstring assignments
passt.h: Include netinet/if_ether.h before struct ctx declaration
log, conf: Add support for logging to file
log: Add missing function comment for trace_init()
conf, log, Makefile: Add versioning information
util: Check return value of lseek() while reading bound ports from
procfs
Makefile | 22 ++-
README.md | 2 +-
conf.c | 74 ++++++--
contrib/fedora/passt.spec | 1 +
dhcp.c | 1 +
dhcpv6.c | 1 +
icmp.c | 1 +
isolation.c | 1 +
log.c | 369 ++++++++++++++++++++++++++++++++++++++
log.h | 32 ++++
ndp.c | 1 +
netlink.c | 1 +
packet.c | 1 +
passt.1 | 18 +-
passt.c | 2 +
passt.h | 2 +
pasta.c | 1 +
pcap.c | 1 +
tap.c | 1 +
tcp.c | 1 +
tcp_splice.c | 1 +
udp.c | 1 +
util.c | 131 +-------------
util.h | 22 +--
24 files changed, 525 insertions(+), 163 deletions(-)
create mode 100644 log.c
create mode 100644 log.h
--
2.35.1
[View Less]
First off, as we swap endianness for source ports in
udp_fill_data_v{4,6}(), we want host endianness, not network
endianness. It doesn't actually matter if we use htons() or ntohs()
here, but the current version is confusing.
In the IPv4 path, when we remap DNS answers, we already swapped the
endianness as needed for the source port: don't swap it again,
otherwise we'll not map DNS answers for IPv4.
In the IPv6 path, when we remap DNS answers, we want to check that
they came from our upstream …
[View More]DNS server, not the one configured via
--dns-forward (which doesn't even need to exist for this
functionality to work).
---
udp.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/udp.c b/udp.c
index cac9c65..4b201d3 100644
--- a/udp.c
+++ b/udp.c
@@ -678,7 +678,7 @@ static void udp_sock_fill_data_v4(const struct ctx *c, int n,
b->iph.tot_len = htons(ip_len);
src = ntohl(b->s_in.sin_addr.s_addr);
- src_port = htons(b->s_in.sin_port);
+ src_port = ntohs(b->s_in.sin_port);
if (src >> IN_CLASSA_NSHIFT == IN_LOOPBACKNET ||
src == INADDR_ANY || src == ntohl(c->ip4.addr_seen)) {
@@ -693,7 +693,7 @@ static void udp_sock_fill_data_v4(const struct ctx *c, int n,
bitmap_set(udp_act[V4][UDP_ACT_TAP], src_port);
} else if (c->ip4.dns_fwd &&
- src == ntohl(c->ip4.dns[0]) && ntohs(src_port) == 53) {
+ src == htonl(c->ip4.dns[0]) && src_port == 53) {
b->iph.saddr = c->ip4.dns_fwd;
} else {
b->iph.saddr = b->s_in.sin_addr.s_addr;
@@ -795,7 +795,7 @@ static void udp_sock_fill_data_v6(const struct ctx *c, int n,
bitmap_set(udp_act[V6][UDP_ACT_TAP], src_port);
} else if (!IN6_IS_ADDR_UNSPECIFIED(&c->ip6.dns_fwd) &&
- IN6_ARE_ADDR_EQUAL(src, &c->ip6.dns_fwd) && src_port == 53) {
+ IN6_ARE_ADDR_EQUAL(src, &c->ip6.dns[0]) && src_port == 53) {
b->ip6h.daddr = c->ip6.addr_seen;
b->ip6h.saddr = c->ip6.dns_fwd;
} else {
--
2.35.1
[View Less]
An n-sized pool, or a pool with n entries, doesn't include index n,
only up to n - 1.
I'm not entirely sure this sanity check actually covers any
practical case, but I spotted this while debugging a hang in
tap4_handler() (possibly due to malformed sequence entries from
qemu).
Signed-off-by: Stefano Brivio <sbrivio(a)redhat.com>
---
packet.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/packet.c b/packet.c
index 3f82e84..d1ff998 100644
--- a/packet.c
+++ b/packet.…
[View More]c
@@ -87,7 +87,7 @@ void packet_add_do(struct pool *p, size_t len, const char *start,
void *packet_get_do(const struct pool *p, size_t index, size_t offset,
size_t len, size_t *left, const char *func, int line)
{
- if (index > p->size || index > p->count) {
+ if (index >= p->size || index >= p->count) {
if (func) {
trace("packet %lu from pool size: %lu, count: %lu, "
"%s:%i", index, p->size, p->count, func, line);
--
2.35.1
[View Less]
This is a minor optimisation possibility I spotted while trying to
debug a hang in tap4_handler(): if we run out of space for packet
sequences, it's fine to add packets to an existing per-sequence pool.
We should check the count of packet sequences only once we realise
that we actually need a new packet sequence.
Signed-off-by: Stefano Brivio <sbrivio(a)redhat.com>
---
tap.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/tap.c b/tap.c
index 78de42c..…
[View More]77e513c 100644
--- a/tap.c
+++ b/tap.c
@@ -410,6 +410,9 @@ resume:
if (seq && L4_MATCH(iph, uh, seq) && seq->p.count < TAP_SEQS)
goto append;
+ if (seq_count == TAP_SEQS)
+ break; /* Resume after flushing if i < in->count */
+
for (seq = tap4_l4 + seq_count - 1; seq >= tap4_l4; seq--) {
if (L4_MATCH(iph, uh, seq)) {
if (seq->p.count >= TAP_SEQS)
@@ -429,9 +432,6 @@ resume:
append:
packet_add((struct pool *)&seq->p, l4_len, l4h);
-
- if (seq_count == TAP_SEQS)
- break; /* Resume after flushing if i < count */
}
for (j = 0, seq = tap4_l4; j < seq_count; j++, seq++) {
@@ -572,6 +572,9 @@ resume:
seq->p.count < TAP_SEQS)
goto append;
+ if (seq_count == TAP_SEQS)
+ break; /* Resume after flushing if i < in->count */
+
for (seq = tap6_l4 + seq_count - 1; seq >= tap6_l4; seq--) {
if (L4_MATCH(ip6h, proto, uh, seq)) {
if (seq->p.count >= TAP_SEQS)
@@ -591,9 +594,6 @@ resume:
append:
packet_add((struct pool *)&seq->p, l4_len, l4h);
-
- if (seq_count == TAP_SEQS)
- break; /* Resume after flushing if i < count */
}
for (j = 0, seq = tap6_l4; j < seq_count; j++, seq++) {
--
2.35.1
[View Less]
Hi,
Archives powered by public-inbox for passt-dev and passt-user are
now available at:
https://archives.passt.top/
Patch hunk headers link to code blobs (coderepo). They are also linked
from the Mailman interface.
--
Stefano