Wait for a route to ff02::1 before sending NAs#49392
Conversation
robmry
added
area/networking
robmry
force-pushed
the
wait_for_mcast_route
branch
from
f0740ac to
ad9aa69
Compare
robmry
force-pushed
the
wait_for_mcast_route
branch
from
ad9aa69 to
c825bc6
Compare
Commit aa3a23d (Temporary debug for unsolicited NA) added code to check for a route from ::1 to ff02::1 if a neighbour advertisement send failed ... hoping to show that the route appeared fairly quickly after the first attempt. The failure didn't happen in CI, perhaps other changes in that PR changed the timing enough for it not to be an issue. Or, perhaps the change in commit 9a6e96f (Before sending ARPs/NAs, check the bridge is ready) was enough to solve the issue. But, in case it's a timing issue we're just no longer seeing on the CI hosts - check the route exists before trying to send an NA, and wait for a short time if not. Signed-off-by: Rob Murray <[email protected]>
robmry
force-pushed
the
wait_for_mcast_route
branch
from
c825bc6 to
d4fa252
Compare
Thanks Shaun ... but this PR was/is still draft, work-in-progress. |
| const pollInterval = 10 * time.Millisecond | ||
| const maxWait = 200 * time.Millisecond | ||
| for range int64(maxWait / pollInterval) { |
There was a problem hiding this comment.
Wondering if something like a ticker and/or ctx, cancel := context.WithTimeout(200 * time.Millisecond) would be something to consider?
I'm horrible at tickers though, as they always confuse me, but I think @vvoland is better at them 😂 🙈
There was a problem hiding this comment.
It's happening in the foreground, so I'm not sure that'd help? I used the same logic in waitForBridgePort, added a few days ago.
| ctx, span := otel.Tracer("").Start(ctx, "libnetwork.osl.waitForMcastRoute") | ||
| defer span.End() | ||
|
|
||
| const pollInterval = 10 * time.Millisecond |
There was a problem hiding this comment.
if we want to be optimistically re-trying, perhaps some exponential / increasing timeout would work? 10 milliseconds feels long if this is in the hot path, or is this code only ran on startup, not for every network connect / container start?
There was a problem hiding this comment.
It's every network connect - so it'll happen once per initial network connection, then for every subsequent network-connect.
The normal case should be that the route's already ready before the first check. So, no delay apart from the netlink request/response.
But, if the route isn't ready, without a delay the network-connect will just fail. So, waiting for a short time is ok, and waiting for a longer time (still milliseconds) if it's not going to work probably isn't really worse than failing sooner. I think, we might as well poll quickly for that whole time, to get going as soon as possible if it does spring in to life.
(If the interface doesn't appear to come "up" in waitForIfUpped, the delay is 5s. That's probably too long really, but all it can do is fail if the interface doesn't start, and we've no data on how long these things take on slow/busy hosts.)
thaJeztah
left a comment
thaJeztah
left a comment
There was a problem hiding this comment.
LGTM, but perhaps could use a revisit for some of the things
|
FYI: This broke our high availability container setup. |
Hi @uweber - thank you for letting us know ... so, we should just log instead in this case. I'll take a look. I've raised this as #49593 |
4 participants
- What I did
Commit aa3a23d (Temporary debug for unsolicited NA) added code to check for a route from ::1 to ff02::1 if a neighbour advertisement send failed ... hoping to show that the route appeared fairly quickly after the first attempt.
The failure didn't happen in CI, perhaps other changes in that PR changed the timing enough for it not to be an issue. Or, perhaps the change in commit 9a6e96f (Before sending ARPs/NAs, check the bridge is ready) was enough to solve the issue.
But, in case it's a timing issue we're just no longer seeing on the CI hosts - try to check the route exists before trying to send an NA, and wait for a short time if not.
- How I did it
After
waitForIfUppedandwaitForBridgePort, if there's an IPv6 address to advertise -waitForMcastRouteas well.Unfortunately, for reasons I don't yet understand and can't repro locally, in CI the netlink call
RouteGetWithOptionscall sometimes returnsEMSGSIZE. Once it does, it's persistent (the retries don't help). I think that normally means too many attributes were added to a request/response - but, in this case, the request is always the same, the response should have zero or one routes and, even if it had more, they should be split into multiple responses.But ... the NA send appears to work anyway (at-least, there's no
write ip ::1->ff02::1: sendmsg: network is unreachableerror).So,
waitForMcastRouteis just best-effort. If it seesEMSGSIZE, it doesn't delay things any further. If the route is still not set up after max retries, it just carries on.Another option would be to just delete the route check and see if the network-unreachable error crops up in the wild. But, I think this approach probably gives us the best chance of avoiding issues in 28.0. There's still a lot to understand though - why wasn't the route set up when the original errors were seen, and why the
EMSGSIZEerrors? The route from ::1 shouldn't really be necessary, the NA just needs to be fired out of the new interface. But, Go'sx/net/ipv6code appears to need it - perhaps there's a better way to implement the send.- How to verify it
Tricky, because I wasn't able to repro the
write ip ::1->ff02::1: sendmsg: network is unreachableorEMSGSIZEerrors locally, or confirm the cause of the original net-unreachable error with extra debug under CI.But, nothing extra is logged in a normal run - the route exists before the first check. And, by nobbling the code to loop until the timeout (and from hitting the
EMSGIZEerrors) - checked that the timeout worked.- Human readable description for the release notes