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.

Sign up for GitHub

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

Jump to bottom

Minimise Debug::fmt in XDE error handling #460

Closed
1 of 2 tasks
FelixMcFelix opened this issue Feb 15, 2024 · 0 comments · Fixed by #585
Closed
1 of 2 tasks

Minimise Debug::fmt in XDE error handling #460

FelixMcFelix opened this issue Feb 15, 2024 · 0 comments · Fixed by #585
Assignees
@FelixMcFelix
Labels
perf
Milestone
MVP

Comments

@FelixMcFelix
Copy link
Collaborator

FelixMcFelix commented Feb 15, 2024 •
edited
Loading

We currently have a few sets of culprits for calling format!() on errored packets:

  • opte::engine::dbg. There are places where we pass in a newly generated string to this function, however we gate the decision to print the string on a static mut switch. We need to move string formatting to fall after this check, otherwise we're always doing an expensive step.
  • Dtrace probes such as __dtrace_probe_bad__packet are making use of format!() to simplify error presentation, but these packets can potentially come from both the underlay and guest traffic.

Tasks
Beta Give feedback
@FelixMcFelix FelixMcFelix self-assigned this Feb 15, 2024
@FelixMcFelix FelixMcFelix changed the title opte: excise Debug::fmt from error handling opte: excise Debug::fmt from dtrace error handling Feb 15, 2024
@FelixMcFelix FelixMcFelix changed the title opte: excise Debug::fmt from dtrace error handling Excise Debug::fmt from XDE dtrace error handling Feb 15, 2024
@FelixMcFelix FelixMcFelix added this to the 7 milestone Feb 16, 2024
@FelixMcFelix FelixMcFelix changed the title Excise Debug::fmt from XDE dtrace error handling Minimise Debug::fmt in XDE error handling Feb 16, 2024
@rcgoodfellow rcgoodfellow modified the milestones: 7, 8 Mar 7, 2024
This was referenced Mar 12, 2024
Closed
Merged
@FelixMcFelix FelixMcFelix modified the milestones: 8, MVP Apr 23, 2024
FelixMcFelix added a commit that referenced this issue May 10, 2024
@FelixMcFelix
Use a repr(C) flow ID to reduce repeated per-packet conversion costs (
976f4dc 
#475)

This PR replaces `InnerFlowId` with an equivalent C-ABI friendly struct,
from which we can directly pass pointers to our dtrace SDTs. This has
proven necessary as we are reconstructing this many times per packet in
both the UFT slowpath and fastpath.

Additionally, this makes partial progress on #460 by using the new
`DError` machinery whenever a packet is `Ok(Modified | Hairpin |
Bypass)` to elide string formats on layer/port processing. The `Err(_)`
and `Ok(Drop{ .. })` cases remain open work, but are less common and in
those cases we have at least removed a superfluous realloc + memcpy on
the formatted string.

From local IGB<->IGB results and comparing 4e2ad7e against master in CI,
this seems like O(10%) reduction in packet latency *spent in XDE*, with
comparable bandwidth increase.

Closes #462.
@FelixMcFelix FelixMcFelix mentioned this issue Oct 30, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@FelixMcFelix FelixMcFelix

Labels
perf
Projects
None yet
Milestone
MVP
Development

Successfully merging a pull request may close this issue.

Datapath overhaul: zero-copy metadata with ingot, 'Compiled' UFTs oxidecomputer/opte
2 participants
@rcgoodfellow @FelixMcFelix