Add packet capture functionality and many more CLI improvements

[Douile]

Continuation of #162.

Adds packet capture functionality to the CLI without the need for external tools (like wireshark).

[cainthebest]

This PR is looking really good, I think setting some goals for this will mark the process toward this being ready.

• Documentation
  • Internal documentation for developers, explaining the source code.
  • Comprehensive end-user documentation for the CLI interface.
• Functionality
  • Working :)
  • Add support for multiple data interchange formats.
  • Design the library so that capture functionality is not included by default.
  • Implement custom socket wrapping in the capture module.
  • Achieve a high level of accuracy in emulating missing packets for pcap. (Known issue, added to docs)
  • Avoid the use of unsafe code practices.
• Optimizations
  • Use fat LTO
  • Remove debug symbols to streamline the binary.
  • Adjust code generation units to 1 for optimization.
  • Have a reasonable binary size (Currently ~2.3MB)
  • Look into specific compiler flags suitable for different architectures/OS. (Not planned in this PR)
• Release workflow
  • Make the release accessible as possible with GitHub release (Not planned in this PR)
  • Pre-build binaries for major architectures/OS.
  • Set up an automated job to attach binaries with checksums to each release event.

[Douile]

Remove debug symbols to streamline the binary.

I'm not sure that we should do this, debugging symbols are very useful if something goes wrong. Although maybe we could use split-debugging symbols. (stripping got the binary down to 2.5M from 5.3M on my release binary).

Look into specific compiler flags suitable for different architectures/OS.

I'm not sure this is necessary for this PR but more of a stretch goal.

Achieve a high level of accuracy in emulating missing packets for pcap.

Could you elaborate more on what we're currently missing (sorry I wrote this code a while ago).

Everything else LGTM! 🚀

[cainthebest]

I'm not sure that we should do this, debugging symbols are very useful if something goes wrong. Although maybe we could use split-debugging symbols. (stripping got the binary down to 2.5M from 5.3M on my release binary).

Im not that familiar on this usage in release but I think it worth looking into as its close to 50% of the size, we can always compile the build with them.

I'm not sure this is necessary for this PR but more of a stretch goal.

yeah that true, will remove from the list

Could you elaborate more on what we're currently missing (sorry I wrote this code a while ago).

Well you could say nothing is missing but currently the emulation isn't quite right. I don't think its a big issue as we can see that they are generated.

Was mentioned here #162 (comment)

Edit: I think we can mark this as a known issue on merge

[Douile]

Thanks for all your amazing work on this, it looks great. Let me know if there's anything I can help with to get this finished. I'm holding of pushing anything ATM because I don't want to conflict with any local changes you might have.

If we stay with the rust-build action we could add the MINIFY option to reduce the binary size. Otherwise strip or cargo-strip are great options.

It might also be worth providing a statically linked linux binary: it would be slightly larger in size but should work on most distros (stretch goal though that can be a different PR).

At the moment I'm looking into our CI testing, with all these new features added the hard-coded feature sets might not cover everything. There's a project called cargo-hack that allows testing power sets of features that I might use (this won't be in this PR but I will test it locally with that before we merge).

EDIT:

Well you could say nothing is missing but currently the emulation isn't quite right. I don't think its a big issue as we can see that they are generated.

Yeah I think that's a good option maybe we include that in the help string for the capture option as well. It's just the TCP headers aren't quite right and stuff like DNS isn't included. (Worth noting with this PR as is we can't capture HTTP traffic from #175).

Something like:

Captured traffic is not a one-to-one representation of what is sent over the internet, the packets contain the information that is directly sent by the gamedig package.

[cainthebest]

I'm holding of pushing anything ATM because I don't want to conflict with any local changes you might have.

Feel free to push, I have been testing the workflow from a different repo

[cainthebest]

I think this is now at the edge of the scope for this PR, we have a few things we can improve at a later date but we have made the goal of the functionality we set out. As this is quite a big change I have requested a review, pls feel free to nitpick

[CosminPerRam]

Huge works and stuff looks very clean, great job.
Just a few small bits and some question (:

[cainthebest]

Had another look over and LGTM, are you happy to merge this @CosminPerRam ?

[CosminPerRam]

Hell yeah! I'll let you guys do the honor (: