Move non-windows stuff to pcap_unix; Convert windows to implicit linking#568
Merged
gconnell merged 10 commits intogoogle:masterfrom Jan 4, 2019
Merged
Move non-windows stuff to pcap_unix; Convert windows to implicit linking#568gconnell merged 10 commits intogoogle:masterfrom
gconnell merged 10 commits intogoogle:masterfrom
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 :(
Contributor
|
Finally I can remove this bookmark ;) Thank you very much @notti !! |
Contributor
Author
|
Happy to help. |
This was referenced Nov 30, 2018
This was referenced Jan 3, 2019
gconnell
reviewed
Jan 3, 2019
Contributor
gconnell
left a comment
There was a problem hiding this comment.
notti, this is truly phenomenal work. Thanks a ton for working on this!
Display which wpcap.dll didn't contain the functions from mustLoad
Contributor
|
Looks great! Merging in now... |
This was referenced Jan 4, 2019
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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.gowhich 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.dllis loaded ininit(). Since both npcap and winpcap provide awpcap.dllwith 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) acceptintas value and notC.intAdditionally, 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
getNextBufPtrLockedsincepcapNextPacketExcan't be inlined. But the newReadPacketDatais faster since it doesn't useC.GoBytesany 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.