Re: [PATCH net-next 1/9] bpf: add BPF_J{LT,LE,SLT,SLE} instructions
On 08/09/2017 11:26 PM, David Miller wrote: From: Daniel BorkmannDate: Wed, 09 Aug 2017 22:32:34 +0200 For the case of cilium, we are not in control of the kernel, by the way, we run a few probes that are small BPF insns snippets that test the kernel for presence of certain features (e.g. helper, verifier, maps) and enable/disable them accordingly later in the code generation. On the user space side, we're indeed a bit more flexible and have no such restriction. Plan is for LLVM as one of the frontends that generate byte code (ply, for example, can probe the kernel directly for its code generation) to have i) a target specific option to offer a possibility to explicitly enable the extension by the user (as we have with -m target specific extensions today for various cpu insns), and ii) have the kernel check for presence of the extensions and enable it transparently when the user selects more aggressive options such as -march=native in a bpf target context, so we can select the underlying features transparently. I should have made that more clear earlier, sorry about that. I think this explanation needs to be in either your header posting or the commit message of patch #1. Thanks :) Ok, sure, I'll do a v2 with that included. Thanks!
Re: [PATCH net-next 1/9] bpf: add BPF_J{LT,LE,SLT,SLE} instructions
From: Daniel BorkmannDate: Wed, 09 Aug 2017 22:32:34 +0200 > For the case of cilium, we are not in control of the kernel, by > the way, we run a few probes that are small BPF insns snippets > that test the kernel for presence of certain features (e.g. helper, > verifier, maps) and enable/disable them accordingly later in the > code generation. On the user space side, we're indeed a bit more > flexible and have no such restriction. > > Plan is for LLVM as one of the frontends that generate byte code > (ply, for example, can probe the kernel directly for its code > generation) to have i) a target specific option to offer a > possibility to explicitly enable the extension by the user (as we > have with -m target specific extensions today for various cpu > insns), and ii) have the kernel check for presence of the extensions > and enable it transparently when the user selects more aggressive > options such as -march=native in a bpf target context, so we can > select the underlying features transparently. I should have made > that more clear earlier, sorry about that. I think this explanation needs to be in either your header posting or the commit message of patch #1. Thanks :)
Re: [PATCH net-next 1/9] bpf: add BPF_J{LT,LE,SLT,SLE} instructions
On 08/09/2017 08:01 PM, David Miller wrote: From: Daniel BorkmannDate: Wed, 09 Aug 2017 19:00:58 +0200 On 08/09/2017 06:55 PM, David Miller wrote: From: Daniel Borkmann Date: Wed, 9 Aug 2017 12:23:53 +0200 [1] https://github.com/borkmann/llvm/tree/bpf-insns How is this "backwards compatible"? If someone takes a new LLVM and tries to load those programs into an older kernel they will be rejected. There appears to be no effort to make things work cleanly in that situation at all. No, that was just the patch I used for LLVM to enable the insns, so not the final one that will be submitted there officially where we have a switch to enable/disable this functionality. So how does this switch work and how are people expected to use this switch? What is the default value and is it ever expected to change in the future? Yeah this seems hella awesome to get cilium programs smaller and faster in a restricted environment where you control the running kernel and everything. For the case of cilium, we are not in control of the kernel, by the way, we run a few probes that are small BPF insns snippets that test the kernel for presence of certain features (e.g. helper, verifier, maps) and enable/disable them accordingly later in the code generation. On the user space side, we're indeed a bit more flexible and have no such restriction. Plan is for LLVM as one of the frontends that generate byte code (ply, for example, can probe the kernel directly for its code generation) to have i) a target specific option to offer a possibility to explicitly enable the extension by the user (as we have with -m target specific extensions today for various cpu insns), and ii) have the kernel check for presence of the extensions and enable it transparently when the user selects more aggressive options such as -march=native in a bpf target context, so we can select the underlying features transparently. I should have made that more clear earlier, sorry about that. Thanks, Daniel
Re: [PATCH net-next 1/9] bpf: add BPF_J{LT,LE,SLT,SLE} instructions
From: Daniel BorkmannDate: Wed, 09 Aug 2017 19:00:58 +0200 > On 08/09/2017 06:55 PM, David Miller wrote: >> From: Daniel Borkmann >> Date: Wed, 9 Aug 2017 12:23:53 +0200 >> >>>[1] https://github.com/borkmann/llvm/tree/bpf-insns >> >> How is this "backwards compatible"? >> >> If someone takes a new LLVM and tries to load those programs >> into an older kernel they will be rejected. >> >> There appears to be no effort to make things work cleanly in >> that situation at all. > > No, that was just the patch I used for LLVM to enable the > insns, so not the final one that will be submitted there > officially where we have a switch to enable/disable this > functionality. So how does this switch work and how are people expected to use this switch? What is the default value and is it ever expected to change in the future? Yeah this seems hella awesome to get cilium programs smaller and faster in a restricted environment where you control the running kernel and everything. But from a review perspective one should be completely uninterested in that. So, generally speaking how does this all pan out? I seriously doubt the wisdom of this. You'll have to convince me of the long term viability and whether we ever get to the point where everyone can benefit from this. :)
Re: [PATCH net-next 1/9] bpf: add BPF_J{LT,LE,SLT,SLE} instructions
On 08/09/2017 07:00 PM, Daniel Borkmann wrote: On 08/09/2017 06:55 PM, David Miller wrote: From: Daniel BorkmannDate: Wed, 9 Aug 2017 12:23:53 +0200 [1] https://github.com/borkmann/llvm/tree/bpf-insns How is this "backwards compatible"? If someone takes a new LLVM and tries to load those programs into an older kernel they will be rejected. There appears to be no effort to make things work cleanly in that situation at all. No, that was just the patch I used for LLVM to enable the insns, so not the final one that will be submitted there officially where we have a switch to enable/disable this functionality. (In other words the one I used for doing the measurements.)
Re: [PATCH net-next 1/9] bpf: add BPF_J{LT,LE,SLT,SLE} instructions
On 08/09/2017 06:55 PM, David Miller wrote: From: Daniel BorkmannDate: Wed, 9 Aug 2017 12:23:53 +0200 [1] https://github.com/borkmann/llvm/tree/bpf-insns How is this "backwards compatible"? If someone takes a new LLVM and tries to load those programs into an older kernel they will be rejected. There appears to be no effort to make things work cleanly in that situation at all. No, that was just the patch I used for LLVM to enable the insns, so not the final one that will be submitted there officially where we have a switch to enable/disable this functionality.
Re: [PATCH net-next 1/9] bpf: add BPF_J{LT,LE,SLT,SLE} instructions
From: Daniel BorkmannDate: Wed, 9 Aug 2017 12:23:53 +0200 > [1] https://github.com/borkmann/llvm/tree/bpf-insns How is this "backwards compatible"? If someone takes a new LLVM and tries to load those programs into an older kernel they will be rejected. There appears to be no effort to make things work cleanly in that situation at all.