feat(network): Use ip= parameter for configuring networking - #2638
feat(network): Use ip= parameter for configuring networking#2638Gelbpunkt wants to merge 3 commits into
Conversation
|
@jounathaen and @sarahspberrypi are okay with the concept. Is this also ok for you, @stlankes? |
57eb68d to
af2377c
Compare
af2377c to
76d38c0
Compare
There was a problem hiding this comment.
Benchmark Results
Details
| Benchmark | Current: f77d4aa | Previous: 2e23902 | Performance Ratio |
|---|---|---|---|
| startup_benchmark Build Time | 95.46 s |
80.34 s |
1.19 ❗ |
| startup_benchmark File Size | 0.78 MB |
0.80 MB |
0.98 ❗ |
| Startup Time - 1 core | 0.78 s (±0.02 s) |
0.75 s (±0.02 s) |
1.04 |
| Startup Time - 2 cores | 0.79 s (±0.03 s) |
0.74 s (±0.02 s) |
1.07 ❗ |
| Startup Time - 4 cores | 0.79 s (±0.02 s) |
0.74 s (±0.02 s) |
1.06 ❗ |
| multithreaded_benchmark Build Time | 96.42 s |
82.11 s |
1.17 ❗ |
| multithreaded_benchmark File Size | 0.88 MB |
0.86 MB |
1.03 ❗ |
| Multithreaded Pi Efficiency - 2 Threads | 67.85 % (±5.30 %) |
85.89 % (±6.61 %) |
0.79 ❗ |
| Multithreaded Pi Efficiency - 4 Threads | 41.00 % (±2.21 %) |
43.43 % (±2.56 %) |
0.94 |
| Multithreaded Pi Efficiency - 8 Threads | 20.08 % (±1.62 %) |
25.76 % (±1.53 %) |
0.78 ❗ |
| micro_benchmarks Build Time | 212.40 s |
80.40 s |
2.64 ❗ |
| micro_benchmarks File Size | 0.89 MB |
0.86 MB |
1.03 ❗ |
| Scheduling time - 1 thread | 192.59 ticks (±26.79 ticks) |
62.65 ticks (±4.06 ticks) |
3.07 ❗ |
| Scheduling time - 2 threads | 112.63 ticks (±17.46 ticks) |
34.08 ticks (±4.10 ticks) |
3.30 ❗ |
| Micro - Time for syscall (getpid) | 9.86 ticks (±4.87 ticks) |
3.45 ticks (±0.58 ticks) |
2.86 ❗ |
| Memcpy speed - (built_in) block size 4096 | 56454.85 MByte/s (±40568.62 MByte/s) |
82448.38 MByte/s (±56997.13 MByte/s) |
0.68 |
| Memcpy speed - (built_in) block size 1048576 | 14635.47 MByte/s (±12275.76 MByte/s) |
30585.98 MByte/s (±24707.84 MByte/s) |
0.48 |
| Memcpy speed - (built_in) block size 16777216 | 13197.83 MByte/s (±11001.29 MByte/s) |
26340.06 MByte/s (±21720.96 MByte/s) |
0.50 |
| Memset speed - (built_in) block size 4096 | 56808.53 MByte/s (±40813.70 MByte/s) |
82292.76 MByte/s (±56891.50 MByte/s) |
0.69 |
| Memset speed - (built_in) block size 1048576 | 14877.38 MByte/s (±12379.28 MByte/s) |
31323.85 MByte/s (±25145.86 MByte/s) |
0.47 |
| Memset speed - (built_in) block size 16777216 | 13483.40 MByte/s (±11134.03 MByte/s) |
27104.68 MByte/s (±22209.94 MByte/s) |
0.50 |
| Memcpy speed - (rust) block size 4096 | 53590.22 MByte/s (±39069.37 MByte/s) |
74097.96 MByte/s (±51811.44 MByte/s) |
0.72 |
| Memcpy speed - (rust) block size 1048576 | 14639.34 MByte/s (±12179.75 MByte/s) |
30361.60 MByte/s (±24602.37 MByte/s) |
0.48 |
| Memcpy speed - (rust) block size 16777216 | 12175.06 MByte/s (±10071.56 MByte/s) |
27625.34 MByte/s (±22806.88 MByte/s) |
0.44 |
| Memset speed - (rust) block size 4096 | 54098.51 MByte/s (±39427.29 MByte/s) |
74373.47 MByte/s (±51976.48 MByte/s) |
0.73 |
| Memset speed - (rust) block size 1048576 | 14931.85 MByte/s (±12306.99 MByte/s) |
31110.89 MByte/s (±25033.24 MByte/s) |
0.48 |
| Memset speed - (rust) block size 16777216 | 12419.56 MByte/s (±10184.19 MByte/s) |
28386.93 MByte/s (±23265.03 MByte/s) |
0.44 |
| alloc_benchmarks Build Time | 209.94 s |
74.76 s |
2.81 ❗ |
| alloc_benchmarks File Size | 0.86 MB |
0.87 MB |
0.98 ❗ |
| Allocations - Allocation success | 91.35 % |
91.31 % |
1.00 ❗ |
| Allocations - Deallocation success | 100.00 % |
100.00 % |
1 |
| Allocations - Pre-fail Allocations | 61.55 % |
61.44 % |
1.00 ❗ |
| Allocations - Average Allocation time | 20940.51 Ticks (±1350.46 Ticks) |
5860.58 Ticks (±98.43 Ticks) |
3.57 ❗ |
| Allocations - Average Allocation time (no fail) | 21908.42 Ticks (±1708.51 Ticks) |
6554.81 Ticks (±92.86 Ticks) |
3.34 ❗ |
| Allocations - Average Deallocation time | 6708.21 Ticks (±2325.82 Ticks) |
1805.01 Ticks (±250.35 Ticks) |
3.72 ❗ |
| mutex_benchmark Build Time | 216.17 s |
79.82 s |
2.71 ❗ |
| mutex_benchmark File Size | 0.89 MB |
0.86 MB |
1.03 ❗ |
| Mutex Stress Test Average Time per Iteration - 1 Threads | 35.94 ns (±7.30 ns) |
12.10 ns (±0.41 ns) |
2.97 ❗ |
| Mutex Stress Test Average Time per Iteration - 2 Threads | 33.32 ns (±8.62 ns) |
40.26 ns (±1.68 ns) |
0.83 |
This comment was automatically generated by workflow using github-action-benchmark.
mkroening
left a comment
There was a problem hiding this comment.
Thanks for taking care of this! :)
Does the format also work well with IPv6 addresses? I assume we would do something like ip=[2001:db8::8a2e:370:7334]:[2001:db8::8a2e:370:7334], right?
| "-ip" => { | ||
| let ip = expect_arg(words.next(), word.as_str()); | ||
| env_vars.insert(String::from("HERMIT_IP"), ip); | ||
| } | ||
| "-mask" => { | ||
| let mask = expect_arg(words.next(), word.as_str()); | ||
| env_vars.insert(String::from("HERMIT_MASK"), mask); | ||
| } | ||
| "-gateway" => { | ||
| let gateway = expect_arg(words.next(), word.as_str()); | ||
| env_vars.insert(String::from("HERMIT_GATEWAY"), gateway); | ||
| } |
There was a problem hiding this comment.
Could you add a warning that these are no longer supported and ip= should be used instead? Same thing for the corresponding hermit_var!(...)s, which are commonly set at compile time at the moment.
There was a problem hiding this comment.
Done, wasn't so sure about the warning messages but I hope this is fine
| /// This is heavily inspired by the Linux kernel's parameter of the same name: | ||
| /// <https://docs.kernel.org/admin-guide/nfs/nfsroot.html#kernel-command-line> | ||
| #[derive(Clone, Copy, Debug, Default)] | ||
| pub struct IpConfiguration { |
There was a problem hiding this comment.
Nit: Move the most important type to the top of the file and the helpers down to help readers with discoverability.
There was a problem hiding this comment.
I think I ordered them in a sensible order now, please take a look :)
76d38c0 to
2c8e3c8
Compare
…MASK Setting just HERMIT_IP=10.0.5.3/24 is much more convenient than HERMIT_IP=10.0.5.3 and HERMIT_MASK=255.255.255.0 and lets us avoid variable hell once we end up adding proper IPv6 support.
This is heavily inspired by Linux' ip= parameter, see https://docs.kernel.org/admin-guide/nfs/nfsroot.html#kernel-command-line The facilities are currently unused and will be replacing HERMIT_IP and HERMIT_GATEWAY in a followup commit.
2c8e3c8 to
f77d4aa
Compare
Yes, this is roughly what I had in mind. It will work just fine with IPv6 addresses once we do a tiny bit of modifications (since this currently hardcodes IPv4 types, but that's a trivial change later). I believe |
This is a breaking change and replaces
HERMIT_IP,HERMIT_MASK,HERMIT_GATEWAY,HERMIT_DNS1andHERMIT_DNS2with a single command-line parameter,ip=, which is heavily inspired by Linux.The kernel will fall back to DHCP if it is enabled, otherwise it will now also no longer fall back to "magic" IPs, instead it will leave the interface unconfigured. Instead, only xtask contains the default static IP configuration that used to reside there and in the kernel.
This is in preparation for adding IPv6 support, which we decided against introducing new variables for. The same mechanism introduced here can be used to support IPv6 with minimal changes.