-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Move non-windows stuff to pcap_unix; Convert windows to implicit linking #568
Conversation
This add support for npcap on windows. Windows now supports both npcap and winpcap at the same time. If both winpcap and npcap are installed at the same time, npcap is preferred. Building on windows now doesn't need neither cgo nor npcap/winpcap sdks. This fixes google#500
On windows network device names for libpcap and net don't match :(
Finally I can remove this bookmark ;) Thank you very much @notti !! |
Happy to help. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
notti, this is truly phenomenal work. Thanks a ton for working on this!
Display which wpcap.dll didn't contain the functions from mustLoad
Looks great! Merging in now... |
This add support for npcap on windows.
Windows now supports both npcap and winpcap at the same time. If both
winpcap and npcap are installed at the same time, npcap is preferred.
Building on windows now doesn't need neither cgo nor npcap/winpcap sdks.
The way this works is that all the non-windows stuff is now in
pcap_unix.go
, while all the windows stuff is inpcap_windows.go
. Those two files provide a go-only internal api topcap.go
which to the outside looks the same as before (except for the small change mentioned below).The new windows code now doesn't use cgo (this is a pain to use on windows) and uses implicit linking (I couldn't get delay loading to work with cgo). => neither cgo nor npcap/winpcap sdk is needed. syscall.Syscall is used for all the calls and the
wpcap.dll
is loaded ininit()
. Since both npcap and winpcap provide awpcap.dll
with libpcap api - no distinctions between the two are necessary (except for the not supported api functions in older libpcap versions as before). The only place where npcap is used, is an additional ddlload path that is added before loading the actualwpcap.dll
.Gains: Now the pcap tests work on windows (only with npcap - with winpcap some of the tests fail due to outdatet libpcap (e.g. no pcapng support, ...))
API of pcap.go is the same as before except for one small change:
gopacket/pcap/pcap.go
Line 795 in a3c4a85
The function signature of this one seems to be wrong and slipped through the cracks (all related functions (e.g.,
DatalinkValToName
) acceptint
as value and notC.int
Additionally, I think
C.*
-types should never be exposed and this seems like a mistake.If this is bad since it might break something, I can change it back.
Ad performance: the new version has an additional function call in
getNextBufPtrLocked
sincepcapNextPacketEx
can't be inlined. But the newReadPacketData
is faster since it doesn't useC.GoBytes
any longer (which is a call into C code! - I think this function is supposed to be used from within C and not go...)Testing with the gopacket_benchmark was inconclusive.
Ad windows: Right now there is no support to select winpcap if both npcap and winpcap are installed... I don't know if there would be a good way to do that.