wait for datapath device to be ready #2117
Conversation
so that when the router container restarts after a reboot it waits for the proxy to create the datapath device. Fixes #2113
|
I've encountered hangs in random tests on CircleCI with this branch. Eventually I managed to reproduce that in a local test run and get a stack trace: So the fastdp code is stuck waiting for a response to a netlink request. Note that something else is stuck too: a That is due to the atrocious termination handling in that code. Our code calls What i don't understand is why that causes the fastdp netlink connection to block. @dpw any clues? |
|
Both @dpw and i have managed to reproduce this with a simple in <20 iterations. I then ran the same after applying the following netlink patch: diff --git a/link_linux.go b/link_linux.go
index 3aa9124..62e385d 100644
--- a/link_linux.go
+++ b/link_linux.go
@@ -844,7 +844,11 @@ func LinkSubscribe(ch chan<- LinkUpdate, done <-chan struct
if err != nil {
return
}
- ch <- LinkUpdate{IfInfomsg: *ifmsg, Link: link}
+ select {
+ case ch <- LinkUpdate{IfInfomsg: *ifmsg, Link: l
+ case <-done:
+ return
+ }
}
}
}()Alas that too stopped after ~40 iterations, this time with Note that this is free of any vishvanada/netlink. |
|
I have hacked NB: Closing the fd does not appear to terminate the nl socket Once I disabled the nl reading code in LinkSubscribe altogether, everything worked fine. But that's obviously not something we can do for real. |
|
Because The Linux Journal says
just for the fun of it I made the nl_pid unique in vishvanada and go-odp with diff --git a/nl/nl_linux.go b/nl/nl_linux.go
index e3afb5c..a1e4b0c 100644
--- a/nl/nl_linux.go
+++ b/nl/nl_linux.go
@@ -5,6 +5,7 @@ import (
"bytes"
"encoding/binary"
"fmt"
+ "math/rand"
"net"
"sync/atomic"
"syscall"
@@ -308,6 +309,7 @@ func Subscribe(protocol int, groups ...uint) (*NetlinkSocket
fd: fd,
}
s.lsa.Family = syscall.AF_NETLINK
+ s.lsa.Pid = rand.Uint32()
for _, g := range groups {
s.lsa.Groups |= (1 << (g - 1))
and diff --git a/odp/netlink.go b/odp/netlink.go
index d89fcad..adb4458 100644
--- a/odp/netlink.go
+++ b/odp/netlink.go
@@ -2,6 +2,7 @@ package odp
import (
"fmt"
+ "math/rand"
"reflect"
"sync/atomic"
"syscall"
@@ -39,7 +40,7 @@ func OpenNetlinkSocket(protocol int) (*NetlinkSocket, error) {
return nil, err
}
- addr := syscall.SockaddrNetlink{Family: syscall.AF_NETLINK}
+ addr := syscall.SockaddrNetlink{Family: syscall.AF_NETLINK, Pid: rand.Ui
if err := syscall.Bind(fd, &addr); err != nil {
return nil, err
}But it is still getting stuck in the same place as before. |
|
Here is the simplest fix that "solves" the problem: diff --git a/net/if.go b/net/if.go
index f107433..d73c1a9 100644
--- a/net/if.go
+++ b/net/if.go
@@ -19,9 +19,7 @@ func EnsureInterface(ifaceName string) (*net.Interface, error)
func ensureInterface(ifaceName string) (*net.Interface, error) {
ch := make(chan netlink.LinkUpdate)
- done := make(chan struct{})
- defer close(done)
- if err := netlink.LinkSubscribe(ch, done); err != nil {
+ if err := netlink.LinkSubscribe(ch, nil); err != nil {
return nil, err
}
// check for currently-existing interface after subscribing, to avoid rai.e. simply don't close the netlink socket. Something really weird is going on when that socket is closed. As I mentioned above, the recvfrom is firmly stuck, even after closure. I have confirmed that by literally making the recvfrom syscall and then closing the socket from another goroutine. |
|
This stripped down version of vishvanada's LinkSubscribe... func LinkSubscribe(ch chan<- LinkUpdate, _ <-chan struct{}) error {
s, err := nl.Subscribe(syscall.NETLINK_ROUTE, syscall.RTNLGRP_LINK)
if err != nil {
return err<
8000
/span>
}
fd := s.GetFd()
s.Close()
rb := make([]byte, syscall.Getpagesize())
// done := make(chan struct{})
go func() {
for {
_, _, err = syscall.Recvfrom(fd, rb, 0)
if err != nil {
// close(done)
return
}
}
}()
// <-done
return nil
}causes the subsequent call to the stdlib's I guess the problem might occur if the fd gets reused before the goroutine hits the first |
|
I've confirmed with a C test program that closing a netlink socket does not cause a This is clearly not what the So my guess is that the issues (both the interactions with go-odp and perhaps the http one too) result from the following sequence of events:
This is consistent with what I saw when looking at the go-odp hang. Is it consistent with the rest of the evidence? If this is correct, I think we can come up with a fix for |
Yes, that is correct. There are a few other permutations. In particular, the subscription go routine may never even have gotten as far as the first recvfrom before the fd gets closed and re-used. There is no way for us to tell whether the goroutine has reached the first recvfrom. The only approach I can think of that might work is to follow your suggestion from yesterday to somehow inject data that causes the recvfrom to proceed after we have closed the socket. i.e. the sequence would be
|
|
The slightly less gross alternative is for the main thread to signal the subscription goroutine when we want it to terminate, and for that goroutine to close the socket when it does. The only downside is that the fd remains in use until some data is received. |
It's probably hard to achieve 2 after closing the socket in 1.
Yes, think this is the starting point. Then, as a refinement, the |
Because that goroutine is not expecting that and hence failing to decode it? |
|
btw, there is a bunch of similar code in vishvanada, i.e. different functions to subscribe to different types of events. They are all broken in a similar way, so if we are contemplating submitting a patch, we really ought to fix them all. |
|
I have opened #2120 for the fd re-use issue and in this PR added a workaround, namely not closing the subscription. |
| defer close(done) | ||
| if err := netlink.LinkSubscribe(ch, done); err != nil { | ||
| // NB: We do not supply (and eventually close) a 'done' channel | ||
| // here since that can cause incorrect file descriptor re-usage. See |
This comment was marked as abuse.
This comment was marked as abuse.
Sorry, something went wrong.
This comment was marked as abuse.
This comment was marked as abuse.
Sorry, something went wrong.
Closing a subscription can cause the associated fd to be re-used while the subscription goroutine is also still trying to use it. Which can have disastrous consequences. So as a work around we don't close subscriptions. This means we leak an fd, some buffer space, and goroutine. The latter will eventually block in a channel send of a subscription event, since nothing is left reading ont the other end. So at least it won't be consuming cycles.
so that when the router container restarts after a reboot it waits for the proxy to create the datapath device, rather than dying.
Fixes #2113.