Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

connections closed during eBPF initialisation are never removed #2700

Closed
rade opened this issue Jul 10, 2017 · 1 comment
Closed

connections closed during eBPF initialisation are never removed #2700

rade opened this issue Jul 10, 2017 · 1 comment
Assignees
Labels
accuracy Incorrect information is being shown to the user; usually a bug bug Broken end user or developer functionality; not working as the developers intended it
Milestone

Comments

@rade
Copy link
Member

rade commented Jul 10, 2017

In a similar vein to #2689...

Apply

diff --git a/probe/endpoint/connection_tracker.go b/probe/endpoint/connection_tracker.go
index 7d6f7ea2..9cd90bcc 100644
--- a/probe/endpoint/connection_tracker.go
+++ b/probe/endpoint/connection_tracker.go
@@ -2,6 +2,7 @@ package endpoint
 
 import (
        "strconv"
+       "time"
 
        log "github.com/Sirupsen/logrus"
        "github.com/weaveworks/scope/probe/endpoint/procspy"
@@ -170,6 +171,8 @@ func (t *connectionTracker) getInitialState() {
                }
        })
 
+       time.Sleep(10 * time.Second)
+
        t.ebpfTracker.feedInitialConnections(conns, seenTuples, processesWaitingInAccept, report.MakeHostNodeID(t.conf.HostID))
 }

and then

  1. run nc www.weave.works 80 in one window
  2. run scope launch in another window
  3. within 10 seconds, in the first window type GET /<RET><RET>, which should terminate the connection.

Now bring up the process topology. This will show a nc process connected to The Internet. Eventually the process will become bare except for the pid:
screenshot from 2017-07-10 22-38-32

That node and edge will never go away.

@rade rade added accuracy Incorrect information is being shown to the user; usually a bug bug Broken end user or developer functionality; not working as the developers intended it labels Jul 10, 2017
@rade
Copy link
Member Author

rade commented Jul 10, 2017

The problem here is that we ignore 'close' events until after we've fed all existing connections to the ebpf tracker. So any connections that get closed after we've obtained the initial connection list but before we've fed them to the ebpf tracker, are stashed away by the tracker as open connections and never removed, since the only thing removing them is a 'close' event, which we've missed.

@rade rade self-assigned this Jul 11, 2017
rade added a commit that referenced this issue Jul 11, 2017
...when initialising eBPF-based connection tracking.

Previously we were ignoring all eBPF events until we had gathered the
existing connections. That means we could a) miss connections created
during the gathering, and b) fail to forget connections that got
closed during the gathering.

The fix comprises the following changes:

1. pay attention to eBPF events immediately. That way we do not
miss anything.

2. remember connections for which we received a Close event during the
initalisation phase, and subsequently drop gathered existing
connections that match these. That way we do not erroneously consider
a gathered connection as open when it got closed since the gathering.

3. drop gathered existing connections which match connections detected
through eBPF events. The latter typically have more / current
metadata. In particular, PIDs can be missing from the former.

Fixes #2689.
Fixes #2700.
rade added a commit that referenced this issue Jul 11, 2017
...when initialising eBPF-based connection tracking.

Previously we were ignoring all eBPF events until we had gathered the
existing connections. That means we could a) miss connections created
during the gathering, and b) fail to forget connections that got
closed during the gathering.

The fix comprises the following changes:

1. pay attention to eBPF events immediately. That way we do not
miss anything.

2. remember connections for which we received a Close event during the
initalisation phase, and subsequently drop gathered existing
connections that match these. That way we do not erroneously consider
a gathered connection as open when it got closed since the gathering.

3. drop gathered existing connections which match connections detected
through eBPF events. The latter typically have more / current
metadata. In particular, PIDs can be missing from the former.

Fixes #2689.
Fixes #2700.
@rade rade modified the milestone: 1.6 Jul 12, 2017
rade added a commit that referenced this issue Jul 13, 2017
don't miss, or fail to forget, initial connections

Fixes #2689.
Fixes #2700.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accuracy Incorrect information is being shown to the user; usually a bug bug Broken end user or developer functionality; not working as the developers intended it
Projects
None yet
Development

No branches or pull requests

1 participant