Skip to content

Push local packets directly to netif, stop using events#2711

Merged
robgjansen merged 1 commit intoshadow:mainfrom
robgjansen:local-packets
Feb 1, 2023
Merged

Push local packets directly to netif, stop using events#2711
robgjansen merged 1 commit intoshadow:mainfrom
robgjansen:local-packets

Conversation

@robgjansen
Copy link
Copy Markdown
Member

Local packets were previously "sent" to the netif by scheduling an event. This commit changes this behavior to instead just directly push local packets to the destination netif. This results in fewer events (it saves 1 event for every local packet) and should thus be more performant. It does change the order of operations a bit, so we expect non-deterministic results across this commit.

@robgjansen robgjansen added Type: Enhancement New functionality or improved design Component: Main Composing the core Shadow executable labels Feb 1, 2023
@robgjansen robgjansen self-assigned this Feb 1, 2023
@codecov
Copy link
Copy Markdown

codecov bot commented Feb 1, 2023

Codecov Report

Base: 68.07% // Head: 67.85% // Decreases project coverage by -0.22% ⚠️

Coverage data is based on head (7d53ee5) compared to base (b204df8).
Patch coverage: 100.00% of modified lines in pull request are covered.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2711      +/-   ##
==========================================
- Coverage   68.07%   67.85%   -0.22%     
==========================================
  Files         203      203              
  Lines       30515    30509       -6     
  Branches     5971     5970       -1     
==========================================
- Hits        20773    20703      -70     
- Misses       5032     5107      +75     
+ Partials     4710     4699      -11     
Flag Coverage Δ
tests 67.85% <100.00%> (-0.22%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
src/main/network/relay/mod.rs 83.52% <100.00%> (+0.01%) ⬆️
src/main/host/descriptor/eventfd.rs 39.39% <0.00%> (-26.27%) ⬇️
src/main/host/syscall/handler/fcntl.rs 45.07% <0.00%> (-15.50%) ⬇️
src/main/host/descriptor/descriptor_table.rs 62.83% <0.00%> (-10.62%) ⬇️
src/main/host/syscall/handler/unistd.rs 65.10% <0.00%> (-5.11%) ⬇️
src/main/host/syscall/type_formatting.rs 74.78% <0.00%> (-2.61%) ⬇️
src/lib/shmem/src/scmutex.rs 87.05% <0.00%> (-2.36%) ⬇️
src/main/host/syscall/handler/mod.rs 81.87% <0.00%> (-1.88%) ⬇️
src/main/core/manager.rs 63.41% <0.00%> (-1.34%) ⬇️
src/main/host/descriptor/mod.rs 67.88% <0.00%> (-0.41%) ⬇️
... and 9 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@robgjansen
Copy link
Copy Markdown
Member Author

TGen results are as expected. We don't see an performance improvement since the TGen benchmark is not sending packets over localhost, however the Tor benchmark does send localhost packets so we may see an improvement when that is next run.
https://github.com/shadow/benchmark-results/tree/master/tgen/2023-02-01-T19-13-56

Local packets were previously "sent" to the netif by scheduling an
event. This commit changes this behavior to instead just directly
push local packets to the destination netif. This results in fewer
events (it saves 1 event for every local packet) and should thus be
more performant. It does change the order of operations a bit, so
we expect non-deterministic results across this commit.
@robgjansen robgjansen enabled auto-merge (squash) February 1, 2023 22:38
@robgjansen robgjansen merged commit b4a1313 into shadow:main Feb 1, 2023
@robgjansen robgjansen deleted the local-packets branch February 1, 2023 22:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Component: Main Composing the core Shadow executable Type: Enhancement New functionality or improved design

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants