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

Add UDP support to packetbeat's process monitor #7571

Merged
merged 2 commits into from
Jul 12, 2018

Conversation

adriansr
Copy link
Contributor

This updates the process monitor with UDP support.

Until now, it would lookup all ports as TCP. This could result in the wrong process being reported when two different processes used the same port number, one for TCP and one for UDP.

The Linux and Windows implementations have been updated to fetch information for UDP ports.

Closes #541

@@ -77,3 +78,22 @@ func _GetExtendedTcpTable(pTcpTable uintptr, pdwSize *uint32, bOrder bool, ulAf
}
return
}

func _GetExtendedUdpTable(pTcpTable uintptr, pdwSize *uint32, bOrder bool, ulAf uint32, tableClass uint32, reserved uint32) (code syscall.Errno, err error) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

func _GetExtendedUdpTable should be _GetExtendedUDPTable
func parameter pTcpTable should be pTCPTable

@@ -57,6 +57,7 @@ var (
modiphlpapi = windows.NewLazySystemDLL("iphlpapi.dll")

procGetExtendedTcpTable = modiphlpapi.NewProc("GetExtendedTcpTable")
procGetExtendedUdpTable = modiphlpapi.NewProc("GetExtendedUdpTable")

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

var procGetExtendedUdpTable should be procGetExtendedUDPTable

owningPID uint32
}

type UDP6RowOwnerPID struct {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

exported type UDP6RowOwnerPID should have comment or be unexported

@@ -51,6 +52,19 @@ type TCP6RowOwnerPID struct {
owningPID uint32
}

type UDPRowOwnerPID struct {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

exported type UDPRowOwnerPID should have comment or be unexported

@@ -24,6 +24,7 @@ import (
)

const (
UDP_TABLE_OWNER_PID = 1

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

don't use ALL_CAPS in Go names; use CamelCase
exported const UDP_TABLE_OWNER_PID should have comment (or a comment on this block) or be unexported

Copy link
Member

@andrewkroh andrewkroh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. I'm going to give it a try on Windows.

@andrewkroh
Copy link
Member

Related to this page: https://www.elastic.co/guide/en/beats/packetbeat/master/configuration-processes.html

It needs to be updated to mention UDP. And the note about Linux only should be expanded to include Windows.

adriansr added 2 commits July 12, 2018 18:17
This updates the process monitor with UDP support.

Until now, it would lookup all ports as TCP. This could result in the
wrong process being reported when two different processes used the same
port number, one for TCP and one for UDP.

The Linux and Windows implementations have been updated to fetch
information for UDP ports.

Closes elastic#541
- Windows support
- UDP support
- cmdline fields
@adriansr adriansr force-pushed the feature/pb/udp_procs branch from 82b3730 to 5a48b17 Compare July 12, 2018 16:34
@adriansr
Copy link
Contributor Author

Thanks @andrewkroh, I've added a new commit with updated docs.

or destination is a local process. The `cmdline` and/or `client_cmdline` fields
will be added to an event, when the server side or client side of the connection
belong to a local process, respectively. Additionally, you can specify a pattern
using the `cmdline_grep` option, to also name those processes. This will cause
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This totally helps clarify how it works for me.

@andrewkroh andrewkroh merged commit 9e768f3 into elastic:master Jul 12, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants