linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH bpf-next] samples/bpf: xdp_redirect_cpu: Add mprog-disable to optstring.
@ 2021-07-31  0:56 Matthew Cover
  2021-07-31 15:25 ` Kumar Kartikeya Dwivedi
  2021-08-04 22:50 ` patchwork-bot+netdevbpf
  0 siblings, 2 replies; 5+ messages in thread
From: Matthew Cover @ 2021-07-31  0:56 UTC (permalink / raw)
  To: Alexei Starovoitov, Daniel Borkmann, David S. Miller,
	Jakub Kicinski, Jesper Dangaard Brouer, John Fastabend,
	Andrii Nakryiko, Martin KaFai Lau, Song Liu, Yonghong Song,
	KP Singh, Lorenzo Bianconi, Matthew Cover, netdev, bpf,
	linux-kernel

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.

Fixes: ce4dade7f12a ("samples/bpf: xdp_redirect_cpu: Load a eBPF program on cpumap")
Signed-off-by: Matthew Cover <matthew.cover@stackpath.com>
---
 samples/bpf/xdp_redirect_cpu_user.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/samples/bpf/xdp_redirect_cpu_user.c b/samples/bpf/xdp_redirect_cpu_user.c
index d3ecdc1..9e225c9 100644
--- a/samples/bpf/xdp_redirect_cpu_user.c
+++ b/samples/bpf/xdp_redirect_cpu_user.c
@@ -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':
-- 
1.8.3.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH bpf-next] samples/bpf: xdp_redirect_cpu: Add mprog-disable to optstring.
  2021-07-31  0:56 [PATCH bpf-next] samples/bpf: xdp_redirect_cpu: Add mprog-disable to optstring Matthew Cover
@ 2021-07-31 15:25 ` Kumar Kartikeya Dwivedi
  2021-08-04 17:57   ` Matt Cover
  2021-08-04 22:50 ` patchwork-bot+netdevbpf
  1 sibling, 1 reply; 5+ messages in thread
From: Kumar Kartikeya Dwivedi @ 2021-07-31 15:25 UTC (permalink / raw)
  To: Matthew Cover
  Cc: Alexei Starovoitov, Daniel Borkmann, David S. Miller,
	Jakub Kicinski, Jesper Dangaard Brouer, John Fastabend,
	Andrii Nakryiko, Martin KaFai Lau, Song Liu, Yonghong Song,
	KP Singh, Lorenzo Bianconi, Matthew Cover, netdev, bpf,
	linux-kernel

On Sat, Jul 31, 2021 at 06:26:32AM IST, Matthew Cover 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 entries
>
> Add 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/20210728165552.435050-1-memxor@gmail.com

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.

> Fixes: ce4dade7f12a ("samples/bpf: xdp_redirect_cpu: Load a eBPF program on cpumap")
> Signed-off-by: Matthew Cover <matthew.cover@stackpath.com>
> ---
>  samples/bpf/xdp_redirect_cpu_user.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/samples/bpf/xdp_redirect_cpu_user.c b/samples/bpf/xdp_redirect_cpu_user.c
> index d3ecdc1..9e225c9 100644
> --- a/samples/bpf/xdp_redirect_cpu_user.c
> +++ b/samples/bpf/xdp_redirect_cpu_user.c
> @@ -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':
> --
> 1.8.3.1
>

--
Kartikeya

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH bpf-next] samples/bpf: xdp_redirect_cpu: Add mprog-disable to optstring.
  2021-07-31 15:25 ` Kumar Kartikeya Dwivedi
@ 2021-08-04 17:57   ` Matt Cover
  2021-08-04 22:50     ` Daniel Borkmann
  0 siblings, 1 reply; 5+ messages in thread
From: Matt Cover @ 2021-08-04 17:57 UTC (permalink / raw)
  To: Kumar Kartikeya Dwivedi
  Cc: Alexei Starovoitov, Daniel Borkmann, David S. Miller,
	Jakub Kicinski, Jesper Dangaard Brouer, John Fastabend,
	Andrii Nakryiko, Martin KaFai Lau, Song Liu, Yonghong Song,
	KP Singh, Lorenzo Bianconi, Matthew Cover, netdev, bpf, LKML

On Sat, Jul 31, 2021 at 8:25 AM Kumar Kartikeya Dwivedi
<memxor@gmail.com> wrote:
>
> On Sat, Jul 31, 2021 at 06:26:32AM IST, Matthew Cover 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 entries
> >
> > Add 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/20210728165552.435050-1-memxor@gmail.com
>
> 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/20210728165552.435050-9-memxor@gmail.com/

>
> > Fixes: ce4dade7f12a ("samples/bpf: xdp_redirect_cpu: Load a eBPF program on cpumap")
> > Signed-off-by: Matthew Cover <matthew.cover@stackpath.com>
> > ---
> >  samples/bpf/xdp_redirect_cpu_user.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/samples/bpf/xdp_redirect_cpu_user.c b/samples/bpf/xdp_redirect_cpu_user.c
> > index d3ecdc1..9e225c9 100644
> > --- a/samples/bpf/xdp_redirect_cpu_user.c
> > +++ b/samples/bpf/xdp_redirect_cpu_user.c
> > @@ -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':
> > --
> > 1.8.3.1
> >
>
> --
> Kartikeya

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH bpf-next] samples/bpf: xdp_redirect_cpu: Add mprog-disable to optstring.
  2021-07-31  0:56 [PATCH bpf-next] samples/bpf: xdp_redirect_cpu: Add mprog-disable to optstring Matthew Cover
  2021-07-31 15:25 ` Kumar Kartikeya Dwivedi
@ 2021-08-04 22:50 ` patchwork-bot+netdevbpf
  1 sibling, 0 replies; 5+ messages in thread
From: patchwork-bot+netdevbpf @ 2021-08-04 22:50 UTC (permalink / raw)
  To: Matt Cover
  Cc: ast, daniel, davem, jakub.kicinski, hawk, john.fastabend, andrii,
	kafai, songliubraving, yhs, kpsingh, lorenzo, matthew.cover,
	netdev, bpf, linux-kernel

Hello:

This patch was applied to bpf/bpf-next.git (refs/heads/master):

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 entries
> 
> Add the missing option character.
> 
> Fixes: ce4dade7f12a ("samples/bpf: xdp_redirect_cpu: Load a eBPF program on cpumap")
> Signed-off-by: Matthew Cover <matthew.cover@stackpath.com>
> 
> [...]

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



^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH bpf-next] samples/bpf: xdp_redirect_cpu: Add mprog-disable to optstring.
  2021-08-04 17:57   ` Matt Cover
@ 2021-08-04 22:50     ` Daniel Borkmann
  0 siblings, 0 replies; 5+ messages in thread
From: Daniel Borkmann @ 2021-08-04 22:50 UTC (permalink / raw)
  To: Matt Cover, Kumar Kartikeya Dwivedi
  Cc: Alexei Starovoitov, David S. Miller, Jakub Kicinski,
	Jesper Dangaard Brouer, John Fastabend, Andrii Nakryiko,
	Martin KaFai Lau, Song Liu, Yonghong Song, KP Singh,
	Lorenzo Bianconi, Matthew Cover, netdev, bpf, LKML

On 8/4/21 7:57 PM, Matt Cover wrote:
> On Sat, Jul 31, 2021 at 8:25 AM Kumar Kartikeya Dwivedi
> <memxor@gmail.com> wrote:
>>
>> On Sat, Jul 31, 2021 at 06:26:32AM IST, Matthew Cover 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 entries
>>>
>>> Add 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/20210728165552.435050-1-memxor@gmail.com
>>
>> 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.

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!

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2021-08-04 22:50 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-31  0:56 [PATCH bpf-next] samples/bpf: xdp_redirect_cpu: Add mprog-disable to optstring Matthew Cover
2021-07-31 15:25 ` Kumar Kartikeya Dwivedi
2021-08-04 17:57   ` Matt Cover
2021-08-04 22:50     ` Daniel Borkmann
2021-08-04 22:50 ` patchwork-bot+netdevbpf

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).