samples/bpf: xdp_redirect_cpu: Add mprog-disable to optstring.
Subsystems (4)
PATCH 1/1 samples/bpf: xdp_redirect_cpu: Add mprog-disable to optstring.
Commit ce4dade7f12a ("samples/bpf: xdp_redirect_cpu: Load a eBPF program on cpumap") added the following option, but missed adding it to optstring:
- mprog-disable: disable loading XDP program on cpumap entries
Add the missing option character.
--- samples/bpf/xdp_redirect_cpu_user.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
samples/bpf/xdp_redirect_cpu_user.c +1 -1
@@ -841,7 +841,7 @@ int main(int argc, char **argv)memset (cpu,0 , n_cpus *sizeof (int ));/* Parse commands line args */ -while ((opt =getopt_long (argc, argv,"hSd:s:p:q:c:xzFf:e:r:m:" , +while ((opt =getopt_long (argc, argv,"hSd:s:p:q:c:xzFf:e:r:m:n" , long_options, &longindex)) !=-1 ) {switch (opt) {case 'd' :
Signature · 1 line
1.8.3.1
Kumar Kartikeya Dwivedi
maintainerQuoted 6 lines · Matthew Cover
Commit
ce4dade7f12a("samples/bpf:xdp_redirect_cpu: Load a eBPF program on cpumap") added the following option, but missed adding it to optstring:
- mprog-disable: disable loading XDP program on cpumap entriesAdd the missing option character.
I made some changes in this area in [0], since the support was primarily to do redirection from the cpumap prog, so by default we don't install anything now and only do so if a redirection interface is specified (and use devmap instead). So this option won't be used anyway going forward (since we don't install a dummy XDP_PASS program anymore) if it gets accepted.
[0]: https://lore.kernel.org/bpf/[email protected]
PS: I can restore it again if this is something really used beyond redirecting to another device (i.e. with custom BPF programs). Any feedback would be helpful.
Quoted 20 lines
Fixes ce4dade7f12a ("samples/bpf: xdp_redirect_cpu: Load a eBPF program on cpumap")Signed-off-by Matthew Cover <[email protected]>--- samples/bpf/xdp_redirect_cpu_user.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)samples/bpf/xdp_redirect_cpu_user.c +1
-2@@ -841,7 +841,7 @@ int main(int argc, char **argv)memset (cpu,0 , n_cpus *sizeof (int ));/* Parse commands line args */ -while ((opt =getopt_long (argc, argv,"hSd:s:p:q:c:xzFf:e:r:m:" , +while ((opt =getopt_long (argc, argv,"hSd:s:p:q:c:xzFf:e:r:m:n" , long_options, &longindex)) !=-1 ) {switch (opt) {case 'd' : --
Quoted 2 lines
1.8.3.1
--
Kartikeya
Quoted 15 lines · Kumar Kartikeya Dwivedi
Quoted 6 lines · Matthew Cover
Commit
ce4dade7f12a("samples/bpf:xdp_redirect_cpu: Load a eBPF program on cpumap") added the following option, but missed adding it to optstring:
- mprog-disable: disable loading XDP program on cpumap entriesAdd the missing option character.
I made some changes in this area in [0], since the support was primarily to do redirection from the cpumap prog, so by default we don't install anything now and only do so if a redirection interface is specified (and use devmap instead). So this option won't be used anyway going forward (since we don't install a dummy
XDP_PASSprogram anymore) if it gets accepted.
Quoted 4 lines
[0]: https://lore.kernel.org/bpf/[email protected]
PS: I can restore it again if this is something really used beyond redirecting to another device (i.e. with custom BPF programs). Any feedback would be helpful.
Hey Kartikeya. I happened to be looking through this code to get a feel for using CPUMAP for custom steering (e.g. RSS on encapsulated packets) in XDP and noticed the missing option character. Figured it was worth doing a quick patch and test.
Unfortunately, I'm not able to say much on your changes as I'm still getting familiarized with this code. It looks like your submission is in need of a rebase; v3 has been marked "Changes Requested" in patchwork [0]. As I see things, It'd be good to get this fix in there for now, whether or not the code is removed later.
[0]:https://patchwork.kernel.org/project/netdevbpf/patch/[email protected]/
Quoted 26 lines
Quoted 21 lines
Fixes ce4dade7f12a ("samples/bpf: xdp_redirect_cpu: Load a eBPF program on cpumap")Signed-off-by Matthew Cover <[email protected]>--- samples/bpf/xdp_redirect_cpu_user.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)samples/bpf/xdp_redirect_cpu_user.c +1
-2@@ -841,7 +841,7 @@ int main(int argc, char **argv)memset (cpu,0 , n_cpus *sizeof (int ));/* Parse commands line args */ -while ((opt =getopt_long (argc, argv,"hSd:s:p:q:c:xzFf:e:r:m:" , +while ((opt =getopt_long (argc, argv,"hSd:s:p:q:c:xzFf:e:r:m:n" , long_options, &longindex)) !=-1 ) {switch (opt) {case 'd' : --Quoted 2 lines
1.8.3.1
--
Kartikeya
Quoted 26 lines · Matt Cover
Quoted 14 lines · Kumar Kartikeya Dwivedi
Quoted 5 lines · Matthew Cover
Commit
ce4dade7f12a("samples/bpf:xdp_redirect_cpu: Load a eBPF program on cpumap") added the following option, but missed adding it to optstring:
- mprog-disable: disable loading XDP program on cpumap entriesAdd the missing option character.
I made some changes in this area in [0], since the support was primarily to do redirection from the cpumap prog, so by default we don't install anything now and only do so if a redirection interface is specified (and use devmap instead). So this option won't be used anyway going forward (since we don't install a dummy
XDP_PASSprogram anymore) if it gets accepted.Quoted 4 lines
[0]: https://lore.kernel.org/bpf/[email protected]
PS: I can restore it again if this is something really used beyond redirecting to another device (i.e. with custom BPF programs). Any feedback would be helpful.
Hey Kartikeya. I happened to be looking through this code to get a feel for using CPUMAP for custom steering (e.g. RSS on encapsulated packets) in XDP and noticed the missing option character. Figured it was worth doing a quick patch and test.
Quoted 5 lines
Unfortunately, I'm not able to say much on your changes as I'm still getting familiarized with this code. It looks like your submission is in need of a rebase; v3 has been marked "Changes Requested" in patchwork [0]. As I see things, It'd be good to get this fix in there for now, whether or not the code is removed later.
Yeap, given this fix in here is tiny & valid either way, I took it in for now, and when Kartikeya's rework eventually drops the whole option, so be it. ;)
Applied, thanks!
This patch was applied to bpf/bpf-next.git (refs/heads/master):
Hello:
This patch was applied to bpf/bpf-next.git (refs/heads/master):
Quoted 10 lines · On Fri, 30 Jul 2021 17:56:32 -0700 you wrote:
Commit
ce4dade7f12a("samples/bpf:xdp_redirect_cpu: Load a eBPF program on cpumap") added the following option, but missed adding it to optstring:
- mprog-disable: disable loading XDP program on cpumap entriesAdd the missing option character.
Fixes ce4dade7f12a ("samples/bpf: xdp_redirect_cpu: Load a eBPF program on cpumap")Signed-off-by Matthew Cover <[email protected]>[...]
Here is the summary with links:
- [bpf-next] samples/bpf: xdp_redirect_cpu: Add mprog-disable to optstring.
https://git.kernel.org/bpf/bpf-next/c/34ad6d9d8c27
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html