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

Support statically linked NativeAOT distribution #448

Open
wants to merge 16 commits into
base: master
Choose a base branch
from

Conversation

hez2010
Copy link

@hez2010 hez2010 commented Mar 1, 2023

  • Use SystemOperating APIs on .NET 6+. The NativeAOT compiler can remove all branches that don't fit target operating system, so Windows-specific APIs won't be linked on Linux platform.

@codecov-commenter
Copy link

codecov-commenter commented Mar 1, 2023

Codecov Report

Attention: Patch coverage is 97.61905% with 1 lines in your changes are missing coverage. Please review.

Project coverage is 78.67%. Comparing base (a9c2893) to head (191eb3e).

Files Patch % Lines
SharpPcap/Tunneling/WinTap/WinTapDriver.cs 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #448      +/-   ##
==========================================
- Coverage   82.14%   78.67%   -3.48%     
==========================================
  Files          51       51              
  Lines        2823     2856      +33     
  Branches      312      312              
==========================================
- Hits         2319     2247      -72     
- Misses        383      503     +120     
+ Partials      121      106      -15     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@kayoub5
Copy link
Collaborator

kayoub5 commented Mar 1, 2023

Use SystemOperating APIs on .NET 6+. The NativeAOT compiler can remove all branches that don't fit target operating system, so Windows-specific APIs won't be linked on Linux platform.

Are you experiencing a specific error with NativeAOT, or is this just an optimization?

Currently SharpPcap uses some runtime dynamic logic to detect so/dll filename, unfortunately there is no standard name that the code can be linked against in a cross platform way.
See https://github.com/dotpcap/sharppcap/blob/master/SharpPcap/LibPcap/LibPcapSafeNativeMethods.Resolver.cs

@hez2010
Copy link
Author

hez2010 commented Mar 1, 2023

there is no standard name

Actually .NET doesn't care about the prefix lib and the extension name in DllImport. A simple pcap should cover libpcap.so, libpcap.dylib and pcap.dll.

@hez2010
Copy link
Author

hez2010 commented Mar 1, 2023

Are you experiencing a specific error with NativeAOT, or is this just an optimization?

I encountered unresolved symbols error from the linker (because some Windows-specific P/Invoke doesn't get trimmed away on Linux) after enable static linking in NativeAOT. By using OperatingSystem APIs, the AOT compiler can just ignore the code path that doesn't belong to the target platform.

@kayoub5
Copy link
Collaborator

kayoub5 commented Mar 1, 2023

there is no standard name

Actually .NET doesn't care about the prefix lib and the extension name in DllImport. A simple pcap should cover libpcap.so, libpcap.dylib and pcap.dll.

In Ubuntu the so name is libpcap.so.0.8 instead of libpcap.so, depending on what apt package you install.

@kayoub5
Copy link
Collaborator

kayoub5 commented Mar 1, 2023

Are you experiencing a specific error with NativeAOT, or is this just an optimization?

I encountered unresolved symbols error from the linker (because some Windows-specific P/Invoke doesn't get trimmed away on Linux) after enable static linking in NativeAOT. By using OperatingSystem APIs, the AOT compiler can just ignore the code path that doesn't belong to the target platform.

What symbols did you get an error for?
It could be SetDllDirectory and not libpcap that's causing the issue

@hez2010
Copy link
Author

hez2010 commented Mar 1, 2023

In Ubuntu the so name is libpcap.so.0.8 instead of libpcap.so, depending on what apt package you install.

I think the package manager will create a symbolic link libpcap.so which is pointing to libpcap.so.0.8 automatically.

@hez2010
Copy link
Author

hez2010 commented Mar 1, 2023

What symbols did you get an error for?

The error message pointed to pcap_open, pcap_setbuff and pcap_setmintocopy explicitly.

@hez2010
Copy link
Author

hez2010 commented Mar 1, 2023

You can try to build https://github.com/hez2010/SysuSurf using dotnet publish -c Release -r linux-x64 /p:PublishAot=true, or dotnet publish -c Release -r linux-musl-x64 /p:PublishAot=true with .NET 8 preview 1 to verify whether it works.
And I've successfully built one using this PR version of SharpPcap, and uploaded the artifact to https://github.com/hez2010/SysuSurf/releases/download/v1.4/Linux_x64_distroless.zip. This binary can run on any x64 Linux without need of installing any dependency.

@kayoub5
Copy link
Collaborator

kayoub5 commented Mar 5, 2023

the idea itself is ok, but the pr breaks CI, especially remote pcap.

@hez2010
Copy link
Author

hez2010 commented Mar 6, 2023

Will try to figure out a way to use wpcap on Windows and pcap on Linux to fix the ci.

@hez2010
Copy link
Author

hez2010 commented Mar 6, 2023

the idea itself is ok, but the pr breaks CI, especially remote pcap.

Is RPCAP a Windows-only feature?

@kayoub5
Copy link
Collaborator

kayoub5 commented Mar 6, 2023

the idea itself is ok, but the pr breaks CI, especially remote pcap.

Is RPCAP a Windows-only feature?

it works in windows (winpcap), and in Linux (requires a configuration flag during build)
see

./configure --enable-remote --disable-universal

@kayoub5
Copy link
Collaborator

kayoub5 commented Mar 6, 2023

there is no standard name

Actually .NET doesn't care about the prefix lib and the extension name in DllImport. A simple pcap should cover libpcap.so, libpcap.dylib and pcap.dll.

the dll name is wpcap.dll and not pcap.dll in windows, so standard donet library resolution logic won't work.

@kayoub5
Copy link
Collaborator

kayoub5 commented Mar 6, 2023

Any reason you have to use <DirectPInvoke Include="pcap" /> ?
if removed, the code should still work without any changes to sharppcap.

@hez2010
Copy link
Author

hez2010 commented Mar 6, 2023

Any reason you have to use <DirectPInvoke Include="pcap" /> ? if removed, the code should still work without any changes to sharppcap.

Only with DirectPInvoke can we statically linked the library into the binary.

@hez2010
Copy link
Author

hez2010 commented Mar 6, 2023

After passing --enable-remote --disable-universal to ./configure, I'm able to build it with pcap_open without issue now, so I revert the change to pcap_open.

@hez2010
Copy link
Author

hez2010 commented Mar 6, 2023

Not sure why CI is still failing, seems like some unrelated timeout issue?

@kayoub5
Copy link
Collaborator

kayoub5 commented Mar 6, 2023

Only with DirectPInvoke can we statically linked the library into the binary.

Why do you want to statically link it in the first place?

@hez2010
Copy link
Author

hez2010 commented Mar 6, 2023

Why do you want to statically link it in the first place?

I want to run it on my router, which is hard to have all dependencies installed. So I would prefer to build a distroless binary which statically links everything and can run directly from scratch.

Co-authored-by: Ayoub Kaanich <[email protected]>
@kayoub5
Copy link
Collaborator

kayoub5 commented Mar 6, 2023

As far as I see, using OperatingSystem.IsWindows / OperatingSystem.IsLinux is only required in those files

SharpPcap/LibPcap/LibPcapSafeNativeMethods.cs
SharpPcap/LibPcap/LibPcapLiveDevice.cs

Other files should not affect the linker, and the ugly check is not needed.

@hez2010
Copy link
Author

hez2010 commented May 2, 2024

@kayoub5 It turns out that the NativeAOT toolchain doesn't care about the library name at all. So let's keep using wpcap as the default library name.

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