There's no point in letting a container perform duplicate address
detection as we'll silently discard neighbour solicitations with
unspecified source addresses anyway, without relaying them to anybody.
And we realised that it's not harmless, see the whole discussion around
https://github.com/containers/podman/pull/23561#discussion_r1711639663:
we can't communicate with the container right away because of that,
which is surely annoying for tests, but it could also be an issue for
use cases with …
[View More]very short-lived containers or namespaces.
Disabling DAD via procfs configuration would be simpler than all this,
but we don't own the namespace (unless we spawn a shell), so we
shouldn't mess up with procfs entries, assuming it's even possible.
Set the nodad attribute, and prevent DAD from being triggered before
on link up, before we can set that attribute.
v3:
- in 4/7, actually handle all the netlink responses for the case where
we change multiple addresses
v2:
- in 4/7, instead of doing the whole nl_routes_dup()-vendored dance
to keep addresses in a single buffer, send NLM_F_REPLACE requests
right away, but use nlmsg_send() instead of nl_do(), and check for
answers to our further requests later. Use warn() instead of die()
if we can't set nodad attributes
- in 5/7, make nl_addr_get_ll() get a pointer to struct in6_addr
instead of a generic void pointer, and warn(), don't die(), if
it fails
Stefano Brivio (7):
netlink: Fix typo in function comment for nl_addr_get()
netlink, pasta: Split MTU setting functionality out of nl_link_up()
netlink, pasta: Turn nl_link_up() into a generic function to set link
flags
netlink, pasta: Disable DAD for link-local addresses on namespace
interface
netlink, pasta: Fetch link-local address from namespace interface once
it's up
pasta: Disable neighbour solicitations on device up to prevent DAD
netlink: Fix typo in function comment for nl_addr_set()
netlink.c | 146 +++++++++++++++++++++++++++++++++++++++++++++++++-----
netlink.h | 6 ++-
pasta.c | 29 ++++++++++-
3 files changed, 166 insertions(+), 15 deletions(-)
--
2.43.0
[View Less]
There's no point in letting a container perform duplicate address
detection as we'll silently discard neighbour solicitations with
unspecified source addresses anyway, without relaying them to anybody.
And we realised that it's not harmless, see the whole discussion around
https://github.com/containers/podman/pull/23561#discussion_r1711639663:
we can't communicate with the container right away because of that,
which is surely annoying for tests, but it could also be an issue for
use cases with …
[View More]very short-lived containers or namespaces.
Disabling DAD via procfs configuration would be simpler than all this,
but we don't own the namespace (unless we spawn a shell), so we
shouldn't mess up with procfs entries, assuming it's even possible.
Set the nodad attribute, and prevent DAD from being triggered before
on link up, before we can set that attribute.
v2:
- in 4/7, instead of doing the whole nl_routes_dup()-vendored dance
to keep addresses in a single buffer, send NLM_F_REPLACE requests
right away, but use nlmsg_send() instead of nl_do(), and check for
answers to our further requests later. Use warn() instead of die()
if we can't set nodad attributes
- in 5/7, make nl_addr_get_ll() get a pointer to struct in6_addr
instead of a generic void pointer, and warn(), don't die(), if
it fails
Stefano Brivio (7):
netlink: Fix typo in function comment for nl_addr_get()
netlink, pasta: Split MTU setting functionality out of nl_link_up()
netlink, pasta: Turn nl_link_up() into a generic function to set link
flags
netlink, pasta: Disable DAD for link-local addresses on namespace
interface
netlink, pasta: Fetch link-local address from namespace interface once
it's up
pasta: Disable neighbour solicitations on device up to prevent DAD
netlink: Fix typo in function comment for nl_addr_set()
netlink.c | 144 +++++++++++++++++++++++++++++++++++++++++++++++++-----
netlink.h | 6 ++-
pasta.c | 29 ++++++++++-
3 files changed, 164 insertions(+), 15 deletions(-)
--
2.43.0
[View Less]
There's no point in letting a container perform duplicate address
detection as we'll silently discard neighbour solicitations with
unspecified source addresses anyway, without relaying them to anybody.
And we realised that it's not harmless, see the whole discussion around
https://github.com/containers/podman/pull/23561#discussion_r1711639663:
we can't communicate with the container right away because of that,
which is surely annoying for tests, but it could also be an issue for
use cases with …
[View More]very short-lived containers or namespaces.
Disabling DAD via procfs configuration would be simpler than all this,
but we don't own the namespace (unless we spawn a shell), so we
shouldn't mess up with procfs entries, assuming it's even possible.
Set the nodad attribute, and prevent DAD from being triggered before
on link up, before we can set that attribute.
Stefano Brivio (7):
netlink: Fix typo in function comment for nl_addr_get()
netlink, pasta: Split MTU setting functionality out of nl_link_up()
netlink, pasta: Turn nl_link_up() into a generic function to set link
flags
netlink, pasta: Disable DAD for link-local addresses on namespace
interface
netlink, pasta: Fetch link-local address from namespace interface once
it's up
pasta: Disable neighbour solicitations on device up to prevent DAD
netlink: Fix typo in function comment for nl_addr_set()
netlink.c | 186 ++++++++++++++++++++++++++++++++++++++++++++++++++----
netlink.h | 6 +-
pasta.c | 29 ++++++++-
3 files changed, 206 insertions(+), 15 deletions(-)
--
2.43.0
[View Less]
Here are fixes for a number of bugs I encountered while working toward
configurable host mapping. There are more coming, but I'm still
working on getting them ready.
David Gibson (3):
Correct inaccurate comments on ip[46]_ctx::addr
conf: Delay handling -D option until after addresses are configured
fwd: Rework inconsistencies in translation of inbound flows
conf.c | 84 +++++++++++++++++++++++++++++-------------------------
fwd.c | 88 ++++++++++++++++++++++++++++++++++++++++++++++++…
[View More]+--------
passt.h | 4 +--
3 files changed, 124 insertions(+), 52 deletions(-)
--
2.46.0
[View Less]
There are some places where we have addresses that are "ours" in the
sense that they're local to passt on at least one interface. But in
some cases it wasn't clear which addresses those were or how to use
them. Make a number of renames, cleanups and small fixes related to
that.
..and also an assortment of slightly related things that I encountered
along the way.
Note that 1/16 is an important fix for a bug I introduced in the last
series I sent. For the rest, apply as many as you're happy …
[View More]with and
I'll respin what's left as necessary.
David Gibson (16):
conf: Don't ignore -t and -u options after -D
treewide: Use "our address" instead of "forwarding address"
util: Helper for formatting MAC addresses
treewide: Rename MAC address fields for clarity
treewide: Use struct assignment instead of memcpy() for IP addresses
conf: Use array indices rather than pointers for DNS array slots
conf: More accurately count entries added in get_dns()
conf: Move DNS array bounds checks into add_dns[46]
conf: Move adding of a nameserver from resolv.conf into subfunction
conf: Correct setting of dns_match address in add_dns6()
conf: Treat --dns addresses as guest visible addresses
conf: Remove incorrect initialisation of addr_ll_seen
util: Correct sock_l4() binding for link local addresses
treewide: Change misleading 'addr_ll' name
Clarify which addresses in ip[46]_ctx are meaningful where
Initialise our_tap_ll to ip6.gw when suitable
arp.c | 4 +-
conf.c | 181 ++++++++++++++++++++++++++++---------------------
dhcp.c | 5 +-
dhcpv6.c | 21 +++---
flow.c | 74 ++++++++++----------
flow.h | 18 ++---
fwd.c | 70 +++++++++----------
icmp.c | 4 +-
ndp.c | 9 +--
passt.1 | 14 ++--
passt.c | 2 +-
passt.h | 22 +++---
pasta.c | 8 +--
tap.c | 12 ++--
tcp.c | 33 ++++-----
tcp_internal.h | 2 +-
udp.c | 12 ++--
util.c | 22 +++++-
util.h | 3 +
19 files changed, 285 insertions(+), 231 deletions(-)
--
2.46.0
[View Less]
Using a zero port on TCP or UDP is dubious, and we can't really deal with
forwarding such a flow within the constraints of the socket API. Hence
we ASSERT()ed that we had non-zero ports in flow_hash().
The intention was to make sure that the protocol code sanitizes such ports
before completing a flow entry. Unfortunately, flow_hash() is also called
on new packets to see if they have an existing flow, so the unsanitized
guest packet can crash passt with the assert.
Correct this by moving the …
[View More]assert from flow_hash() to flow_sidx_hash()
which is only used on entries already in the table, not on unsanitized
data.
Signed-off-by: David Gibson <david(a)gibson.dropbear.id.au>
---
flow.c | 18 ++++++++++--------
1 file changed, 10 insertions(+), 8 deletions(-)
diff --git a/flow.c b/flow.c
index 687e9fd0..93b687dc 100644
--- a/flow.c
+++ b/flow.c
@@ -561,12 +561,6 @@ static uint64_t flow_hash(const struct ctx *c, uint8_t proto, uint8_t pif,
{
struct siphash_state state = SIPHASH_INIT(c->hash_secret);
- /* For the hash table to work, we need complete endpoint information,
- * and at least a forwarding port.
- */
- ASSERT(pif != PIF_NONE && !inany_is_unspecified(&side->eaddr) &&
- side->eport != 0 && side->fport != 0);
-
inany_siphash_feed(&state, &side->faddr);
inany_siphash_feed(&state, &side->eaddr);
@@ -586,8 +580,16 @@ static uint64_t flow_hash(const struct ctx *c, uint8_t proto, uint8_t pif,
static uint64_t flow_sidx_hash(const struct ctx *c, flow_sidx_t sidx)
{
const struct flow_common *f = &flow_at_sidx(sidx)->f;
- return flow_hash(c, FLOW_PROTO(f),
- f->pif[sidx.sidei], &f->side[sidx.sidei]);
+ const struct flowside *side = &f->side[sidx.sidei];
+ uint8_t pif = f->pif[sidx.sidei];
+
+ /* For the hash table to work, entries must have complete endpoint
+ * information, and at least a forwarding port.
+ */
+ ASSERT(pif != PIF_NONE && !inany_is_unspecified(&side->eaddr) &&
+ side->eport != 0 && side->fport != 0);
+
+ return flow_hash(c, FLOW_PROTO(f), pif, side);
}
/**
--
2.46.0
[View Less]