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

Jump to bottom

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
Labels
FrozenDueToAge
Milestone
Unreleased

Comments

@arthurfabre
Copy link

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
@arthurfabre
bpf: Support JumpIf on RegX instead of K
58f6675 
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
@arthurfabre
bpf: Support JumpIf on RegX instead of K
c11c72e 
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
@arthurfabre
bpf: Support JumpIf on RegX instead of K
68a1bbf 
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
@arthurfabre
bpf: Support JumpIf on RegX instead of K
5f217e1 
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
@arthurfabre
bpf: Support JumpIf on RegX instead of K
feb4b30 
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
@arthurfabre
bpf: Support JumpIf on RegX instead of K
2a62e4f 
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
@arthurfabre
bpf: Support JumpIf on RegX instead of K
a2e9621 
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 arthurfabre mentioned this issue Sep 23, 2018
Closed
@gopherbot
Copy link

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

@danderson
Copy link
Contributor

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
@arthurfabre
bpf: Support JumpIf on RegX instead of K
0badf86 
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
@arthurfabre
bpf: Support JumpIf on RegX instead of K
b732816 
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
@arthurfabre
bpf: Support JumpIf on RegX instead of K
2163517 
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
@arthurfabre
bpf: Support JumpIf on RegX instead of K
f910e19 
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
@arthurfabre
bpf: support JumpIf on RegX instead of K
39a8816 
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
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge
Projects
None yet
Milestone
Unreleased
Development

No branches or pull requests
3 participants
@danderson @arthurfabre @gopherbot