samples, bpf: Add duration option D for sampleip

Message ID 20221116064631.16224-1-tegongkang@gmail.com
State New
Headers
Series samples, bpf: Add duration option D for sampleip |

Commit Message

Kang Minchul Nov. 16, 2022, 6:46 a.m. UTC
  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(-)
  

Comments

Andrii Nakryiko Nov. 17, 2022, 11:26 p.m. UTC | #1
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
>
  
Kang Minchul Nov. 19, 2022, 4:05 p.m. UTC | #2
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
> >
  
Andrii Nakryiko Nov. 23, 2022, 9:38 p.m. UTC | #3
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
> > >
  

Patch

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;