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

Use connection directions from conntrack where possible #967

Merged
merged 1 commit into from
Feb 23, 2016

Conversation

paulbellamy
Copy link
Contributor

Fixes #539

Needs in-depth review from @tomwilkie and/or @peterbourgon

toAddr, toPort = f.Original.Layer3.DstIP, uint16(f.Original.Layer4.DstPort)
tuple = []string{
fmt.Sprintf("%s:%d", fromAddr, fromPort),
fmt.Sprintf("%s:%d", toAddr, toPort),

This comment was marked as abuse.

This comment was marked as abuse.

This comment was marked as abuse.

seenTuples[strings.Join(tuple, " ")] = fourTuple{fromAddr, toAddr, fromPort, toPort}
// TODO(paulbellamy): Won't adding this from conntrack, and adding the
// edge again here result in double-counting? Also, if we've seen the
// same conn twice from proc (if both ends are on the same host).

This comment was marked as abuse.

@tomwilkie
Copy link
Contributor

Also, the TestSpyNoProcesses and 340_process_edge_across_host_2_test are probably related.

@tomwilkie tomwilkie assigned paulbellamy and unassigned tomwilkie Feb 18, 2016
@paulbellamy paulbellamy force-pushed the 539-connection-directions branch from 1d9e3ac to 9b280e3 Compare February 19, 2016 15:27
@paulbellamy paulbellamy assigned tomwilkie and unassigned paulbellamy Feb 19, 2016
remotePort = conn.RemotePort
localAddr = conn.LocalAddress.String()
remoteAddr = conn.RemoteAddress.String()
tuple = fourTuple{conn.LocalAddress.String(), conn.RemoteAddress.String(), conn.LocalPort, conn.RemotePort}

This comment was marked as abuse.

@tomwilkie
Copy link
Contributor

A bit nit picky (sorry!) but otherwise LGTM. Please squash.

@tomwilkie tomwilkie assigned paulbellamy and unassigned tomwilkie Feb 22, 2016
@paulbellamy paulbellamy force-pushed the 539-connection-directions branch 4 times, most recently from c72fe08 to c217acb Compare February 22, 2016 15:44
@paulbellamy paulbellamy assigned tomwilkie and unassigned paulbellamy Feb 22, 2016
* Remove report.EdgeMetadata.MaxConnCountTCP, as we don't display it anywhere
* Remove hostname metadata from local end of connection. We should be using the hostnodeid
@tomwilkie tomwilkie force-pushed the 539-connection-directions branch from c217acb to 6cef1b1 Compare February 23, 2016 14:18
@tomwilkie
Copy link
Contributor

LGTM, assuming tests now pass.

tomwilkie added a commit that referenced this pull request Feb 23, 2016
Use connection directions from conntrack where possible
@tomwilkie tomwilkie merged commit df2c21e into master Feb 23, 2016
@tomwilkie tomwilkie deleted the 539-connection-directions branch February 23, 2016 14:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants