2022-11-16 07:18:35

by Kang Minchul

Subject: [PATCH] samples, bpf: Add duration option D for sampleip

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 <[email protected]>
---
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

2022-11-18 00:02:54

by Andrii Nakryiko

Subject: Re: [PATCH] samples, bpf: Add duration option D for sampleip

On Tue, Nov 15, 2022 at 10:46 PM Kang Minchul <[email protected]> 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 <[email protected]>
> ---
> 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
>

2022-11-19 16:25:22

by Kang Minchul

Subject: Re: [PATCH] samples, bpf: Add duration option D for sampleip

Thanks for your reply.

2022년 11월 18일 (금) 오전 8:26, Andrii Nakryiko <[email protected]>님이 작성:
>
> On Tue, Nov 15, 2022 at 10:46 PM Kang Minchul <[email protected]> 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 <[email protected]>
> > ---
> > 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
> >

2022-11-23 22:01:48

by Andrii Nakryiko

Subject: Re: [PATCH] samples, bpf: Add duration option D for sampleip

On Sat, Nov 19, 2022 at 8:06 AM Kang Minchul <[email protected]> wrote:
>
> Thanks for your reply.
>
> 2022년 11월 18일 (금) 오전 8:26, Andrii Nakryiko <[email protected]>님이 작성:
> >
> > On Tue, Nov 15, 2022 at 10:46 PM Kang Minchul <[email protected]> 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 <[email protected]>
> > > ---
> > > 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
> > >