All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jesper Dangaard Brouer <brouer@redhat.com>
To: "Daniel T. Lee" <danieltimlee@gmail.com>
Cc: "David S . Miller" <davem@davemloft.net>,
	netdev@vger.kernel.org, brouer@redhat.com,
	"Toke Høiland-Jørgensen" <toke@redhat.com>
Subject: Re: [v3 2/3] samples: pktgen: add helper functions for IP(v4/v6) CIDR parsing
Date: Mon, 16 Sep 2019 08:53:17 +0200	[thread overview]
Message-ID: <20190916085317.02e4d985@carbon> (raw)
In-Reply-To: <20190914151353.18054-2-danieltimlee@gmail.com>

On Sun, 15 Sep 2019 00:13:52 +0900
"Daniel T. Lee" <danieltimlee@gmail.com> wrote:

> This commit adds CIDR parsing and IP validate helper function to parse
> single IP or range of IP with CIDR. (e.g. 198.18.0.0/15)
> 
> Helpers will be used in prior to set target address in samples/pktgen.
> 
> Signed-off-by: Daniel T. Lee <danieltimlee@gmail.com>
> ---
> Changes since v3:
>  * Set errexit option to stop script execution on error
> 
>  samples/pktgen/functions.sh | 124 ++++++++++++++++++++++++++++++++++++
>  1 file changed, 124 insertions(+)
> 
> diff --git a/samples/pktgen/functions.sh b/samples/pktgen/functions.sh
> index 4af4046d71be..87ae61701904 100644
> --- a/samples/pktgen/functions.sh
> +++ b/samples/pktgen/functions.sh
> @@ -5,6 +5,8 @@
>  # Author: Jesper Dangaaard Brouer
>  # License: GPL
>  
> +set -o errexit

Unfortunately, this breaks the scripts.

The function proc_cmd are designed to grep after "Result: OK:" which
might fail, and your patch/change makes the script stop immediately.
We actually want to continue, and output what command that failed (and
also grep again after "Result:" to provide the kernel reason).

Even if you somehow "fix" function proc_cmd, then we in general want to
catch different error situations by looking at status $?, and output
meaning full errors via calling err() function.  IHMO as minimum with
errexit you need a 'trap' function that can help/inform the user of
what went wrong.

-- 
Best regards,
  Jesper Dangaard Brouer
  MSc.CS, Principal Kernel Engineer at Red Hat
  LinkedIn: http://www.linkedin.com/in/brouer

  reply	other threads:[~2019-09-16  6:53 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-09-14 15:13 [v3 1/3] samples: pktgen: make variable consistent with option Daniel T. Lee
2019-09-14 15:13 ` [v3 2/3] samples: pktgen: add helper functions for IP(v4/v6) CIDR parsing Daniel T. Lee
2019-09-16  6:53   ` Jesper Dangaard Brouer [this message]
2019-09-16 12:52     ` Toke Høiland-Jørgensen
2019-09-20  3:04       ` Daniel T. Lee
2019-09-14 15:13 ` [v3 3/3] samples: pktgen: allow to specify destination IP range (CIDR) Daniel T. Lee

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190916085317.02e4d985@carbon \
    --to=brouer@redhat.com \
    --cc=danieltimlee@gmail.com \
    --cc=davem@davemloft.net \
    --cc=netdev@vger.kernel.org \
    --cc=toke@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.