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

x/net/bpf: support jump conditions on x #27814

Closed
arthurfabre opened this issue Sep 22, 2018 · 2 comments
Closed

x/net/bpf: support jump conditions on x #27814

arthurfabre opened this issue Sep 22, 2018 · 2 comments

Comments

@arthurfabre
Copy link

@arthurfabre arthurfabre commented Sep 22, 2018

Currently conditionals / jumps are represented by JumpIf. This doesn't support conditionals / jumps that use register x instead of the constant k encoded in the instruction.

I have added preliminary support for this in the form of a new JumpIfX instruction.

Questions:

  • Should JumpIf be renamed JumpIfConstant for consistency with ALUOpConstant & ALUOpX?
  • Do I need to add support for this to the vm or is adding it to disassembler enough?

cc @danderson & @mdlayher as you seem to be the maintainers

@gopherbot gopherbot added this to the Unreleased milestone Sep 22, 2018
arthurfabre added a commit to arthurfabre/net that referenced this issue Sep 23, 2018
Add JumpIfX instructions, which implements conditional jumps based on
RegA and RegX. This is in addition to the pre-existing JumpIf
instruction with uses RegA and K.

Simplify some of the parsing logic, and add a seperate helper for
checking for "fake" JumpIfs.

Add JumpIfX support to the BPF vm.

Fixes golang/go#27814
arthurfabre added a commit to arthurfabre/net that referenced this issue Sep 23, 2018
Add JumpIfX instructions, which implements conditional jumps based on
RegA and RegX. This is in addition to the pre-existing JumpIf
instruction with uses RegA and K.

These instructions are not mentionned in either the kernel's
'filter.txt' or the original BPF paper, but are supported
but tools like 'bpf_asm'.

Simplify some of the parsing logic, and add a seperate helper for
checking for "fake" JumpIfs.

Add JumpIfX support to the BPF vm.

Update testdata with JumpIfX instructions.

Fixes golang/go#27814
arthurfabre added a commit to arthurfabre/net that referenced this issue Sep 23, 2018
Add JumpIfX instructions, which implements conditional jumps based on
RegA and RegX. This is in addition to the pre-existing JumpIf
instruction with uses RegA and K.

These instructions are not mentioned in either the kernel's
'filter.txt' or the original BPF paper, but are supported
but tools like 'bpf_asm'.

Simplify some of the parsing logic, and add a separate helper for
checking for "fake" JumpIfs.

Add JumpIfX support to the BPF vm.

Update testdata with JumpIfX instructions, and add tests
for both the assembler/disassembler and vm.

Fixes golang/go#27814
arthurfabre added a commit to arthurfabre/net that referenced this issue Sep 23, 2018
Add JumpIfX instructions, which implements conditional jumps based on
RegA and RegX. This is in addition to the pre-existing JumpIf
instruction with uses RegA and K.

These instructions are not mentioned in either the kernel's
'filter.txt' or the original BPF paper, but are supported
by tools like 'bpf_asm'.

Simplify some of the parsing logic, and add a separate helper for
checking for "fake" JumpIfs.

Add JumpIfX support to the BPF vm.

Update testdata with JumpIfX instructions, and add tests
for both the assembler/disassembler and vm.

Fixes golang/go#27814
arthurfabre added a commit to arthurfabre/net that referenced this issue Sep 23, 2018
Add JumpIfX instructions, which implements conditional jumps using
RegA and RegX. This is in addition to the pre-existing JumpIf
instruction which use RegA and K.

These instructions are not mentioned in either the kernel's
'filter.txt' or the original BPF paper, but are supported
by tools like 'bpf_asm'.

Simplify some of the parsing logic, and add a separate helper for
checking for "fake" JumpIfs.

Add JumpIfX support to the BPF vm.

Update testdata with JumpIfX instructions, and add tests
for both the assembler/disassembler and vm.

Fixes golang/go#27814
arthurfabre added a commit to arthurfabre/net that referenced this issue Sep 23, 2018
Add a JumpIfX instruction which implements conditional jumps using
RegA and RegX. This is in addition to the pre-existing JumpIf
instruction which uses RegA and K.

This instruction / addressing mode is not mentioned in either the
kernel's 'filter.txt' or the original BPF paper, but are supported
by tools like 'bpf_asm'.

Simplify some of the parsing logic, and add a separate helper for
checking for "fake" JumpIfs.

Add JumpIfX support to the BPF vm.

Update testdata with JumpIfX instructions, and add tests
for both the assembler/disassembler and vm.

Fixes golang/go#27814
arthurfabre added a commit to arthurfabre/net that referenced this issue Sep 23, 2018
Add a JumpIfX instruction which implements conditional jumps using
RegA and RegX. This is in addition to the pre-existing JumpIf
instruction which uses RegA and K.

This instruction / addressing mode is not mentioned in either the
kernel's 'filter.txt' or the original BPF paper, but are supported
by tools like 'bpf_asm'.

Simplify some of the parsing logic, and add a separate helper for
checking for "fake" JumpIfs.

Add JumpIfX support to the BPF vm.

Update testdata with JumpIfX instructions, and add tests
for both the assembler/disassembler and vm.

Fixes golang/go#27814
@gopherbot
Copy link

@gopherbot gopherbot commented Sep 23, 2018

Change https://golang.org/cl/136895 mentions this issue: bpf: Support JumpIf on RegX instead of K

@danderson
Copy link

@danderson danderson commented Oct 1, 2018

Already reviewed your CL, but for reference...

I would prefer to not rename JumpIf, at least not until Go has generally available modules with good versioning. I don't want to break the API of every user of this package just for a little bit of consistency. Once Go modules are out of experimental state, we can look at maybe releasing a 2.0.0 of the module.

We want the VM to be able to execute any code that the asm/disasm knows how to handle. Your PR does include both, so that's a moot point now :).

Thank you for contributing the extra instruction! Mildly annoying that it's not documented (or did I just completely miss it out when I implemented the package originally??)

arthurfabre added a commit to arthurfabre/net that referenced this issue Oct 2, 2018
Add a JumpIfX instruction which implements conditional jumps using
RegA and RegX. This is in addition to the pre-existing JumpIf
instruction which uses RegA and K.

This instruction / addressing mode is not mentioned in either the
kernel's 'filter.txt' or the original BPF paper, but are supported
by tools like 'bpf_asm'.

Simplify some of the parsing logic, and add a separate helper for
checking for "fake" JumpIfs.

Add JumpIfX support to the BPF vm.

Update testdata with JumpIfX instructions, and add tests
for both the assembler/disassembler and vm.

Fixes golang/go#27814
arthurfabre added a commit to arthurfabre/net that referenced this issue Oct 10, 2018
Add a JumpIfX instruction which implements conditional jumps using
RegA and RegX. This is in addition to the pre-existing JumpIf
instruction which uses RegA and K.

This instruction / addressing mode is not mentioned in either the
kernel's 'filter.txt' or the original BPF paper, but are supported
by tools like 'bpf_asm'.

Simplify some of the parsing logic, and add a separate helper for
checking for "fake" JumpIfs.

Add JumpIfX support to the BPF vm.

Update testdata with JumpIfX instructions, and add tests
for both the assembler/disassembler and vm.

Fixes golang/go#27814
arthurfabre added a commit to arthurfabre/net that referenced this issue Oct 10, 2018
Add a JumpIfX instruction which implements conditional jumps using
RegA and RegX. This is in addition to the pre-existing JumpIf
instruction which uses RegA and K.

This instruction / addressing mode is not mentioned in either the
kernel's 'filter.txt' or the original BPF paper, but are supported
by tools like 'bpf_asm'.

Simplify some of the parsing logic, and add a separate helper for
checking for "fake" JumpIfs.

Add JumpIfX support to the BPF vm.

Update testdata with JumpIfX instructions, and add tests
for both the assembler/disassembler and vm.

Fixes golang/go#27814
arthurfabre added a commit to arthurfabre/net that referenced this issue Oct 10, 2018
Add a JumpIfX instruction which implements conditional jumps using
RegA and RegX. This is in addition to the pre-existing JumpIf
instruction which uses RegA and K.

This instruction / addressing mode is not mentioned in either the
kernel's 'filter.txt' or the original BPF paper, but are supported
by tools like 'bpf_asm'.

Simplify some of the parsing logic, and add a separate helper for
checking for "fake" JumpIfs.

Add JumpIfX support to the BPF vm.

Update testdata with JumpIfX instructions, and add tests
for both the assembler/disassembler and vm.

Fixes golang/go#27814
@bradfitz bradfitz changed the title x/net/bpf: Support jump conditions on x x/net/bpf: support jump conditions on x Oct 11, 2018
arthurfabre added a commit to arthurfabre/net that referenced this issue Oct 11, 2018
Add a JumpIfX instruction which implements conditional jumps using
RegA and RegX. This is in addition to the pre-existing JumpIf
instruction which uses RegA and K.

This instruction / addressing mode is not mentionned in the original BPF
paper, but is supported by tools like bpf_asm, and has recently been
added to the kernel's filter.txt.

Simplify some of the parsing logic, and add a separate helper for
checking for "fake" JumpIfs.

Add JumpIfX support to the BPF vm.

Update testdata with JumpIfX instructions, and add tests
for both the assembler/disassembler and vm.

Fixes golang/go#27814
@golang golang locked and limited conversation to collaborators Oct 11, 2019
NET12115 added a commit to NET12115/Golang-C-NET that referenced this issue Feb 28, 2022
Add a JumpIfX instruction which implements conditional jumps using
RegA and RegX. This is in addition to the pre-existing JumpIf
instruction which uses RegA and K.

This instruction / addressing mode is not mentionned in the original BPF
paper, but is supported by tools like bpf_asm, and has recently been
added to the kernel's filter.txt.

Simplify some of the parsing logic, and add a separate helper for
checking for "fake" JumpIfs.

Add JumpIfX support to the BPF vm.

Update testdata with JumpIfX instructions, and add tests
for both the assembler/disassembler and vm.

Fixes golang/go#27814

Change-Id: I0c3f6ac7eb5b4cd4d9c5af8784ee2e8d25195a0a
GitHub-Last-Rev: 39a88165b2d3253c37db4b0e303d862b60dc37c9
GitHub-Pull-Request: golang/net#20
Reviewed-on: https://go-review.googlesource.com/c/136895
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants