bpf.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] samples, bpf: Add duration option D for sampleip
@ 2022-11-16  6:46 Kang Minchul
  2022-11-17 23:26 ` Andrii Nakryiko
  0 siblings, 1 reply; 4+ messages in thread
From: Kang Minchul @ 2022-11-16  6:46 UTC (permalink / raw)
  To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko
  Cc: Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend,
	KP Singh, bpf, linux-kernel, Kang Minchul

Although sampleip program can handle three options,
(-F for frequency, duration, and -h for help)
currently there is no independent option for duration.

This patch adds option -D for duration like below:

$ sudo ./samples/bpf/sampleip -h
USAGE: sampleip [-F freq] [-D duration]
       -F freq       # sample frequency (Hertz), default 99
       -D duration   # sampling duration (seconds), default 5

$ sudo ./samples/bpf/sampleip -F 120
Sampling at 120 Hertz for 5 seconds. Ctrl-C also ends.
ADDR                KSYM                          COUNT
...

$ sudo ./samples/bpf/sampleip -D 7
Sampling at 99 Hertz for 7 seconds. Ctrl-C also ends.
ADDR                KSYM                          COUNT
...

$ sudo ./samples/bpf/sampleip -F 120 -D 7
Sampling at 120 Hertz for 7 seconds. Ctrl-C also ends.
ADDR                KSYM                          COUNT
...

Signed-off-by: Kang Minchul <tegongkang@gmail.com>
---
 samples/bpf/sampleip_user.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/samples/bpf/sampleip_user.c b/samples/bpf/sampleip_user.c
index 921c505bb567..ce6aadd496e1 100644
--- a/samples/bpf/sampleip_user.c
+++ b/samples/bpf/sampleip_user.c
@@ -28,9 +28,9 @@ static int nr_cpus;
 
 static void usage(void)
 {
-	printf("USAGE: sampleip [-F freq] [duration]\n");
-	printf("       -F freq    # sample frequency (Hertz), default 99\n");
-	printf("       duration   # sampling duration (seconds), default 5\n");
+	printf("USAGE: sampleip [-F freq] [-D duration]\n");
+	printf("       -F freq       # sample frequency (Hertz), default 99\n");
+	printf("       -D duration   # sampling duration (seconds), default 5\n");
 }
 
 static int sampling_start(int freq, struct bpf_program *prog,
@@ -145,19 +145,20 @@ int main(int argc, char **argv)
 	char filename[256];
 
 	/* process arguments */
-	while ((opt = getopt(argc, argv, "F:h")) != -1) {
+	while ((opt = getopt(argc, argv, "F:D:h")) != -1) {
 		switch (opt) {
 		case 'F':
 			freq = atoi(optarg);
 			break;
+		case 'D':
+			secs = atoi(optarg);
+			break;
 		case 'h':
 		default:
 			usage();
 			return 0;
 		}
 	}
-	if (argc - optind == 1)
-		secs = atoi(argv[optind]);
 	if (freq == 0 || secs == 0) {
 		usage();
 		return 1;
-- 
2.34.1


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

* Re: [PATCH] samples, bpf: Add duration option D for sampleip
  2022-11-16  6:46 [PATCH] samples, bpf: Add duration option D for sampleip Kang Minchul
@ 2022-11-17 23:26 ` Andrii Nakryiko
  2022-11-19 16:05   ` Kang Minchul
  0 siblings, 1 reply; 4+ messages in thread
From: Andrii Nakryiko @ 2022-11-17 23:26 UTC (permalink / raw)
  To: Kang Minchul
  Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
	Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend,
	KP Singh, bpf, linux-kernel

On Tue, Nov 15, 2022 at 10:46 PM Kang Minchul <tegongkang@gmail.com> wrote:
>
> Although sampleip program can handle three options,
> (-F for frequency, duration, and -h for help)
> currently there is no independent option for duration.

Because it's positional argument, which is very clearly documented by
usage(). What's wrong with that and why do you want to change this?

>
> This patch adds option -D for duration like below:
>
> $ sudo ./samples/bpf/sampleip -h
> USAGE: sampleip [-F freq] [-D duration]
>        -F freq       # sample frequency (Hertz), default 99
>        -D duration   # sampling duration (seconds), default 5
>
> $ sudo ./samples/bpf/sampleip -F 120
> Sampling at 120 Hertz for 5 seconds. Ctrl-C also ends.
> ADDR                KSYM                          COUNT
> ...
>
> $ sudo ./samples/bpf/sampleip -D 7
> Sampling at 99 Hertz for 7 seconds. Ctrl-C also ends.
> ADDR                KSYM                          COUNT
> ...
>
> $ sudo ./samples/bpf/sampleip -F 120 -D 7
> Sampling at 120 Hertz for 7 seconds. Ctrl-C also ends.
> ADDR                KSYM                          COUNT
> ...
>
> Signed-off-by: Kang Minchul <tegongkang@gmail.com>
> ---
>  samples/bpf/sampleip_user.c | 13 +++++++------
>  1 file changed, 7 insertions(+), 6 deletions(-)
>
> diff --git a/samples/bpf/sampleip_user.c b/samples/bpf/sampleip_user.c
> index 921c505bb567..ce6aadd496e1 100644
> --- a/samples/bpf/sampleip_user.c
> +++ b/samples/bpf/sampleip_user.c
> @@ -28,9 +28,9 @@ static int nr_cpus;
>
>  static void usage(void)
>  {
> -       printf("USAGE: sampleip [-F freq] [duration]\n");
> -       printf("       -F freq    # sample frequency (Hertz), default 99\n");
> -       printf("       duration   # sampling duration (seconds), default 5\n");
> +       printf("USAGE: sampleip [-F freq] [-D duration]\n");
> +       printf("       -F freq       # sample frequency (Hertz), default 99\n");
> +       printf("       -D duration   # sampling duration (seconds), default 5\n");
>  }
>
>  static int sampling_start(int freq, struct bpf_program *prog,
> @@ -145,19 +145,20 @@ int main(int argc, char **argv)
>         char filename[256];
>
>         /* process arguments */
> -       while ((opt = getopt(argc, argv, "F:h")) != -1) {
> +       while ((opt = getopt(argc, argv, "F:D:h")) != -1) {
>                 switch (opt) {
>                 case 'F':
>                         freq = atoi(optarg);
>                         break;
> +               case 'D':
> +                       secs = atoi(optarg);
> +                       break;
>                 case 'h':
>                 default:
>                         usage();
>                         return 0;
>                 }
>         }
> -       if (argc - optind == 1)
> -               secs = atoi(argv[optind]);
>         if (freq == 0 || secs == 0) {
>                 usage();
>                 return 1;
> --
> 2.34.1
>

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

* Re: [PATCH] samples, bpf: Add duration option D for sampleip
  2022-11-17 23:26 ` Andrii Nakryiko
@ 2022-11-19 16:05   ` Kang Minchul
  2022-11-23 21:38     ` Andrii Nakryiko
  0 siblings, 1 reply; 4+ messages in thread
From: Kang Minchul @ 2022-11-19 16:05 UTC (permalink / raw)
  To: Andrii Nakryiko
  Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
	Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend,
	KP Singh, bpf, linux-kernel

Thanks for your reply.

2022년 11월 18일 (금) 오전 8:26, Andrii Nakryiko <andrii.nakryiko@gmail.com>님이 작성:
>
> On Tue, Nov 15, 2022 at 10:46 PM Kang Minchul <tegongkang@gmail.com> wrote:
> >
> > Although sampleip program can handle three options,
> > (-F for frequency, duration, and -h for help)
> > currently there is no independent option for duration.
>
> Because it's positional argument, which is very clearly documented by
> usage(). What's wrong with that and why do you want to change this?
Yes, but I'm not sure why only 'duration' should be a positional argument.

I don't think it is 'wrong', but I think it's better to treat
'duration' just as same as 'frequency' because
 frequency and duration are two independent things in this case.
(duration is not dependent on frequency)

So I thought making an option for duration like below

$ sudo ./samples/bpf/sampleip -F <Frequecny> -D <Duration>

is better than below.

$ sudo ./samples/bpf/sampleip -F <Frequecny> <Duration>

I am not insisting strongly on this, so if I have misunderstood something,
I'll respect the existing way.

Regards,

Kang Minchul
> >
> > This patch adds option -D for duration like below:
> >
> > $ sudo ./samples/bpf/sampleip -h
> > USAGE: sampleip [-F freq] [-D duration]
> >        -F freq       # sample frequency (Hertz), default 99
> >        -D duration   # sampling duration (seconds), default 5
> >
> > $ sudo ./samples/bpf/sampleip -F 120
> > Sampling at 120 Hertz for 5 seconds. Ctrl-C also ends.
> > ADDR                KSYM                          COUNT
> > ...
> >
> > $ sudo ./samples/bpf/sampleip -D 7
> > Sampling at 99 Hertz for 7 seconds. Ctrl-C also ends.
> > ADDR                KSYM                          COUNT
> > ...
> >
> > $ sudo ./samples/bpf/sampleip -F 120 -D 7
> > Sampling at 120 Hertz for 7 seconds. Ctrl-C also ends.
> > ADDR                KSYM                          COUNT
> > ...
> >
> > Signed-off-by: Kang Minchul <tegongkang@gmail.com>
> > ---
> >  samples/bpf/sampleip_user.c | 13 +++++++------
> >  1 file changed, 7 insertions(+), 6 deletions(-)
> >
> > diff --git a/samples/bpf/sampleip_user.c b/samples/bpf/sampleip_user.c
> > index 921c505bb567..ce6aadd496e1 100644
> > --- a/samples/bpf/sampleip_user.c
> > +++ b/samples/bpf/sampleip_user.c
> > @@ -28,9 +28,9 @@ static int nr_cpus;
> >
> >  static void usage(void)
> >  {
> > -       printf("USAGE: sampleip [-F freq] [duration]\n");
> > -       printf("       -F freq    # sample frequency (Hertz), default 99\n");
> > -       printf("       duration   # sampling duration (seconds), default 5\n");
> > +       printf("USAGE: sampleip [-F freq] [-D duration]\n");
> > +       printf("       -F freq       # sample frequency (Hertz), default 99\n");
> > +       printf("       -D duration   # sampling duration (seconds), default 5\n");
> >  }
> >
> >  static int sampling_start(int freq, struct bpf_program *prog,
> > @@ -145,19 +145,20 @@ int main(int argc, char **argv)
> >         char filename[256];
> >
> >         /* process arguments */
> > -       while ((opt = getopt(argc, argv, "F:h")) != -1) {
> > +       while ((opt = getopt(argc, argv, "F:D:h")) != -1) {
> >                 switch (opt) {
> >                 case 'F':
> >                         freq = atoi(optarg);
> >                         break;
> > +               case 'D':
> > +                       secs = atoi(optarg);
> > +                       break;
> >                 case 'h':
> >                 default:
> >                         usage();
> >                         return 0;
> >                 }
> >         }
> > -       if (argc - optind == 1)
> > -               secs = atoi(argv[optind]);
> >         if (freq == 0 || secs == 0) {
> >                 usage();
> >                 return 1;
> > --
> > 2.34.1
> >

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

* Re: [PATCH] samples, bpf: Add duration option D for sampleip
  2022-11-19 16:05   ` Kang Minchul
@ 2022-11-23 21:38     ` Andrii Nakryiko
  0 siblings, 0 replies; 4+ messages in thread
From: Andrii Nakryiko @ 2022-11-23 21:38 UTC (permalink / raw)
  To: Kang Minchul
  Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
	Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend,
	KP Singh, bpf, linux-kernel

On Sat, Nov 19, 2022 at 8:06 AM Kang Minchul <tegongkang@gmail.com> wrote:
>
> Thanks for your reply.
>
> 2022년 11월 18일 (금) 오전 8:26, Andrii Nakryiko <andrii.nakryiko@gmail.com>님이 작성:
> >
> > On Tue, Nov 15, 2022 at 10:46 PM Kang Minchul <tegongkang@gmail.com> wrote:
> > >
> > > Although sampleip program can handle three options,
> > > (-F for frequency, duration, and -h for help)
> > > currently there is no independent option for duration.
> >
> > Because it's positional argument, which is very clearly documented by
> > usage(). What's wrong with that and why do you want to change this?
> Yes, but I'm not sure why only 'duration' should be a positional argument.
>
> I don't think it is 'wrong', but I think it's better to treat
> 'duration' just as same as 'frequency' because
>  frequency and duration are two independent things in this case.
> (duration is not dependent on frequency)
>
> So I thought making an option for duration like below
>
> $ sudo ./samples/bpf/sampleip -F <Frequecny> -D <Duration>
>
> is better than below.
>
> $ sudo ./samples/bpf/sampleip -F <Frequecny> <Duration>
>
> I am not insisting strongly on this, so if I have misunderstood something,
> I'll respect the existing way.
>

I think it's just a common convention (e.g., iostat has the similar
approach). Let's keep it as is.


> Regards,
>
> Kang Minchul
> > >
> > > This patch adds option -D for duration like below:
> > >
> > > $ sudo ./samples/bpf/sampleip -h
> > > USAGE: sampleip [-F freq] [-D duration]
> > >        -F freq       # sample frequency (Hertz), default 99
> > >        -D duration   # sampling duration (seconds), default 5
> > >
> > > $ sudo ./samples/bpf/sampleip -F 120
> > > Sampling at 120 Hertz for 5 seconds. Ctrl-C also ends.
> > > ADDR                KSYM                          COUNT
> > > ...
> > >
> > > $ sudo ./samples/bpf/sampleip -D 7
> > > Sampling at 99 Hertz for 7 seconds. Ctrl-C also ends.
> > > ADDR                KSYM                          COUNT
> > > ...
> > >
> > > $ sudo ./samples/bpf/sampleip -F 120 -D 7
> > > Sampling at 120 Hertz for 7 seconds. Ctrl-C also ends.
> > > ADDR                KSYM                          COUNT
> > > ...
> > >
> > > Signed-off-by: Kang Minchul <tegongkang@gmail.com>
> > > ---
> > >  samples/bpf/sampleip_user.c | 13 +++++++------
> > >  1 file changed, 7 insertions(+), 6 deletions(-)
> > >
> > > diff --git a/samples/bpf/sampleip_user.c b/samples/bpf/sampleip_user.c
> > > index 921c505bb567..ce6aadd496e1 100644
> > > --- a/samples/bpf/sampleip_user.c
> > > +++ b/samples/bpf/sampleip_user.c
> > > @@ -28,9 +28,9 @@ static int nr_cpus;
> > >
> > >  static void usage(void)
> > >  {
> > > -       printf("USAGE: sampleip [-F freq] [duration]\n");
> > > -       printf("       -F freq    # sample frequency (Hertz), default 99\n");
> > > -       printf("       duration   # sampling duration (seconds), default 5\n");
> > > +       printf("USAGE: sampleip [-F freq] [-D duration]\n");
> > > +       printf("       -F freq       # sample frequency (Hertz), default 99\n");
> > > +       printf("       -D duration   # sampling duration (seconds), default 5\n");
> > >  }
> > >
> > >  static int sampling_start(int freq, struct bpf_program *prog,
> > > @@ -145,19 +145,20 @@ int main(int argc, char **argv)
> > >         char filename[256];
> > >
> > >         /* process arguments */
> > > -       while ((opt = getopt(argc, argv, "F:h")) != -1) {
> > > +       while ((opt = getopt(argc, argv, "F:D:h")) != -1) {
> > >                 switch (opt) {
> > >                 case 'F':
> > >                         freq = atoi(optarg);
> > >                         break;
> > > +               case 'D':
> > > +                       secs = atoi(optarg);
> > > +                       break;
> > >                 case 'h':
> > >                 default:
> > >                         usage();
> > >                         return 0;
> > >                 }
> > >         }
> > > -       if (argc - optind == 1)
> > > -               secs = atoi(argv[optind]);
> > >         if (freq == 0 || secs == 0) {
> > >                 usage();
> > >                 return 1;
> > > --
> > > 2.34.1
> > >

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

end of thread, other threads:[~2022-11-23 21:38 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-11-16  6:46 [PATCH] samples, bpf: Add duration option D for sampleip Kang Minchul
2022-11-17 23:26 ` Andrii Nakryiko
2022-11-19 16:05   ` Kang Minchul
2022-11-23 21:38     ` Andrii Nakryiko

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).