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

Minimal set of changes required to work with Windows eBPF project #62

Merged
merged 1 commit into from
May 12, 2021

Conversation

Alan-Jowett
Copy link
Collaborator

Minimal set of changes required to work with Windows eBPF project

Resolves: #61

Signed-off-by: Alan Jowett alanjo@microsoft.com

@coveralls
Copy link

coveralls commented Apr 28, 2021

Coverage Status

Coverage decreased (-0.5%) to 95.541% when pulling d2c7bdc on Alan-Jowett:master into 0014f29 on iovisor:master.

@jpsamaroo
Copy link

Is there a public repo for this Windows eBPF effort? It would be nice to have some context on what the downstream user for these changes will be.

@Alan-Jowett
Copy link
Collaborator Author

It was pointed out to me by @dthaler that there is a better way to work around some of the issues, which allowed me to reduce the churn even further.

  1. Add ubpf_translate that emits x64 machine code from ebpf byte code (with out changing page protections etc).
  2. Removed dependency on printf in ebpf_execute
  3. Perform correct register mapping / save / restore for Windows ABI.

vm/ubpf_jit_x86_64.c Outdated Show resolved Hide resolved
vm/ubpf_jit_x86_64.c Show resolved Hide resolved
vm/ubpf_jit_x86_64.c Outdated Show resolved Hide resolved
vm/ubpf_jit_x86_64.c Outdated Show resolved Hide resolved
vm/ubpf_jit_x86_64.c Outdated Show resolved Hide resolved
@Alan-Jowett
Copy link
Collaborator Author

Added code to suppress generation of call to fprintf on divide by zero.

@dthaler
Copy link
Collaborator

dthaler commented Apr 28, 2021

Added code to suppress generation of call to fprintf on divide by zero.

Why not just implement fprintf (e.g., as a no-op)?

@Alan-Jowett
Copy link
Collaborator Author

Why not just implement fprintf (e.g., as a no-op)?

The issue is that fprintf would need to be treated like a helper function. We would need to pass in the target address to ubpf during jit compilation so that it could emit code with the correct fprintf address in the target environment.

It seems simpler to just make the jitter emit:
"return -1" on integer overflow.

@Alan-Jowett
Copy link
Collaborator Author

Added an option to override what function is called when divide by zero occurs.

@Alan-Jowett
Copy link
Collaborator Author

@jpsamaroo Our project ebpf-for-windows is now public so you can see how we are using ubpf.

It would be great if you would consider the PR now so we can switch to using your repo.

@jpsamaroo
Copy link

Thanks for the link! Interesting project 🙂

It would be great if you would consider the PR now so we can switch to using your repo.

This isn't my repo, and I'm not currently actively maintaining my own fork since ubpf integration isn't one of my top priorities right now. This would be up to @pchaigno to consider for merge.

Copy link

@jpsamaroo jpsamaroo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FWIW, this LGTM!

vm/ubpf_vm.c Outdated Show resolved Hide resolved
vm/ubpf_vm.c Outdated Show resolved Hide resolved
Copy link
Collaborator

@rlane rlane left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very cool!

vm/inc/ubpf.h Outdated Show resolved Hide resolved
vm/inc/ubpf.h Outdated Show resolved Hide resolved
vm/inc/ubpf.h Show resolved Hide resolved
Allow caller to provide function to invoke on divide by zero and
other errors.

Signed-off-by: Alan Jowett <alanjo@microsoft.com>
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.

Ubpf doesn't compile or emit code correctly for Windows
5 participants