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

don't exclude NATed connections in mapping to processes #2978

Merged
merged 1 commit into from
Dec 15, 2017
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
don't exclude NATed connections in mapping to processes
We used to ignore source endpoints that are associated with multiple
destination endpoints, which is a partial workaround for our inability
to correctly represent two connections from the same source ip/port
but different processes, or the same destination ip/port but different
processes. See #2665.

However, that condition is too coarse. In particular, we end up
ignoring endpoints that are connected to NATed destinations, since the
latter are represented by two (or more) endpoints.

The change here corrects that.
  • Loading branch information
rade committed Dec 15, 2017
commit 20138b9218940a2c781b919a0dfa624c5aeefc96
34 changes: 29 additions & 5 deletions render/process.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package render

import (
"github.com/weaveworks/scope/probe/docker"
"github.com/weaveworks/scope/probe/endpoint"
"github.com/weaveworks/scope/probe/process"
"github.com/weaveworks/scope/report"
)
Expand Down Expand Up @@ -98,11 +99,7 @@ func (e endpoints2Processes) Render(rpt report.Report) Nodes {
if !ok {
continue
}

if len(n.Adjacency) > 1 {
// We cannot be sure that the pid is associated with all the
// connections. It is better to drop such an endpoint than
// risk rendering bogus connections.
if hasMoreThanOneConnection(n, endpoints.Nodes) {
continue
}

Expand All @@ -124,6 +121,33 @@ func (e endpoints2Processes) Render(rpt report.Report) Nodes {
return ret.result()
}

// When there is more than one connection originating from a source
// endpoint, we cannot be sure that its pid is associated with all of
// them, since the source endpoint may have been re-used by a
// different process. See #2665. It is better to drop such an endpoint
// than risk rendering bogus connections. Aliased connections - when
// all the remote endpoints represent the same logical endpoint, due
// to NATing - are fine though.
func hasMoreThanOneConnection(n report.Node, endpoints report.Nodes) bool {
if len(n.Adjacency) < 2 {
return false
}
firstRealEndpointID := ""
for _, endpointID := range n.Adjacency {
if ep, ok := endpoints[endpointID]; ok {
if copyID, _, ok := ep.Latest.LookupEntry(endpoint.CopyOf); ok {
endpointID = copyID
}
}
if firstRealEndpointID == "" {
firstRealEndpointID = endpointID
} else if firstRealEndpointID != endpointID {
return true
}
}
return false
}

// processes2Names maps process Nodes to Nodes for each process name.
func processes2Names(processes Nodes) Nodes {
ret := newJoinResults()
Expand Down