[v4] usertools/dpdk-setup.sh: fix dpdk-setup's behaviour on non-alphanumeric inputs
diff mbox series

Message ID 20200602073813.1484-1-sarosh.arif@emumba.com
State New
Delegated to: Thomas Monjalon
Headers show
Series
  • [v4] usertools/dpdk-setup.sh: fix dpdk-setup's behaviour on non-alphanumeric inputs
Related show

Checks

Context Check Description
ci/Intel-compilation success Compilation OK
ci/iol-testing warning Testing issues
ci/travis-robot success Travis build: passed
ci/iol-mellanox-Performance success Performance Testing PASS
ci/iol-nxp-Performance success Performance Testing PASS
ci/iol-intel-Performance success Performance Testing PASS
ci/checkpatch success coding style OK

Commit Message

Sarosh Arif June 2, 2020, 7:38 a.m. UTC 
Combine https://patches.dpdk.org/patch/67855/ with v2 of this patch to fix
the overall behaviour of dpdk-setup.sh on non-alphanumeric inputs.
Instead of crashing the script will prompt the user to re-enter the input 
in case of non-alphanumeric input. 

Bugzilla ID: 419
Signed-off-by: Sarosh Arif <sarosh.arif@emumba.com>
Signed-off-by: Muhammad Bilal <m.bilal@emumba.com>
---
v4: fix signoff
---
 usertools/dpdk-setup.sh | 37 ++++++++++++++++++++++++++-----------
 1 file changed, 26 insertions(+), 11 deletions(-)

Comments

Sarosh Arif July 23, 2020, 12:13 p.m. UTC | #1 
Since this script has no maintainer who is responsible for providing ACK or
NAK on the patches related to this script?

On Tue, Jun 2, 2020 at 12:39 PM Sarosh Arif <sarosh.arif@emumba.com> wrote:

> Combine https://patches.dpdk.org/patch/67855/ with v2 of this patch to fix
> the overall behaviour of dpdk-setup.sh on non-alphanumeric inputs.
> Instead of crashing the script will prompt the user to re-enter the input
> in case of non-alphanumeric input.
>
> Bugzilla ID: 419
> Signed-off-by: Sarosh Arif <sarosh.arif@emumba.com>
> Signed-off-by: Muhammad Bilal <m.bilal@emumba.com>
> ---
> v4: fix signoff
> ---
>  usertools/dpdk-setup.sh | 37 ++++++++++++++++++++++++++-----------
>  1 file changed, 26 insertions(+), 11 deletions(-)
>
> diff --git a/usertools/dpdk-setup.sh b/usertools/dpdk-setup.sh
> index e5bbe9fee..69cd1d2bd 100755
> --- a/usertools/dpdk-setup.sh
> +++ b/usertools/dpdk-setup.sh
> @@ -320,14 +320,18 @@ set_non_numa_pages()
>         echo "  enter '64' to reserve 64 * 2MB pages"
>         echo -n "Number of pages: "
>         read Pages
> +       numeric="^[[:digit:]]+$"
> +       PG_PATH="/sys/kernel/mm/hugepages/hugepages-${HUGEPGSZ}"
> +       if [[ $Pages =~ $numeric ]]; then
> +               echo "echo $Pages > $PG_PATH/nr_hugepages" > .echo_tmp
> +               echo "Reserving hugepages"
> +               sudo sh .echo_tmp
> +               rm -f .echo_tmp
> +               create_mnt_huge
> +       else
> +               echo "Please enter a numeric value"
> +       fi
>
> -       echo "echo $Pages >
> /sys/kernel/mm/hugepages/hugepages-${HUGEPGSZ}/nr_hugepages" > .echo_tmp
> -
> -       echo "Reserving hugepages"
> -       sudo sh .echo_tmp
> -       rm -f .echo_tmp
> -
> -       create_mnt_huge
>  }
>
>  #
> @@ -343,10 +347,16 @@ set_numa_pages()
>         echo "  enter '64' to reserve 64 * 2MB pages on each node"
>
>         echo > .echo_tmp
> +       numeric="^[[:digit:]]+$"
>         for d in /sys/devices/system/node/node? ; do
>                 node=$(basename $d)
>                 echo -n "Number of pages for $node: "
>                 read Pages
> +               while [[ ! "$Pages" =~ $numeric ]]; do
> +                       echo "Please enter a numeric value"
> +                       echo -n "Number of pages for $node: "
> +                       read Pages
> +               done
>                 echo "echo $Pages >
> $d/hugepages/hugepages-${HUGEPGSZ}/nr_hugepages" >> .echo_tmp
>         done
>         echo "Reserving hugepages"
> @@ -592,10 +602,15 @@ while [ "$QUIT" == "0" ]; do
>         echo "[$OPTION_NUM] Exit Script"
>         OPTIONS[$OPTION_NUM]="quit"
>         echo ""
> -       echo -n "Option: "
> -       read our_entry
> -       echo ""
> -       ${OPTIONS[our_entry]} ${our_entry}
> +       read -p "Option: " our_entry
> +       [ $? -eq 0 ] || exit 0
> +
> +       numeric="^[[:digit:]]+$"
> +       if [[ "$our_entry" =~ $numeric ]]; then
> +               ${OPTIONS[our_entry]} ${our_entry}
> +       else
> +               echo "Please enter a numeric value"
> +       fi
>
>         if [ "$QUIT" == "0" ] ; then
>                 echo
> --
> 2.17.1
>
>
Stephen Hemminger July 23, 2020, 5 p.m. UTC | #2 
On Thu, 23 Jul 2020 17:13:17 +0500
Sarosh Arif <sarosh.arif@emumba.com> wrote:

> Since this script has no maintainer who is responsible for providing ACK or
> NAK on the patches related to this script?
> 
> On Tue, Jun 2, 2020 at 12:39 PM Sarosh Arif <sarosh.arif@emumba.com> wrote:
> 
> > Combine https://patches.dpdk.org/patch/67855/ with v2 of this patch to fix
> > the overall behaviour of dpdk-setup.sh on non-alphanumeric inputs.
> > Instead of crashing the script will prompt the user to re-enter the input
> > in case of non-alphanumeric input.
> >
> > Bugzilla ID: 419
> > Signed-off-by: Sarosh Arif <sarosh.arif@emumba.com>
> > Signed-off-by: Muhammad Bilal <m.bilal@emumba.com>

If no maintainer is defined for a subtree, it should be responsiblity
of the core maintainers to handle it.
 

PS: dpdk-setup has out grown its welcome, and a better version with
a real UI is needed.
Thomas Monjalon July 24, 2020, 1:29 p.m. UTC | #3 
23/07/2020 19:00, Stephen Hemminger:
> On Thu, 23 Jul 2020 17:13:17 +0500
> Sarosh Arif <sarosh.arif@emumba.com> wrote:
> 
> > Since this script has no maintainer who is responsible for providing ACK or
> > NAK on the patches related to this script?
> > 
> > On Tue, Jun 2, 2020 at 12:39 PM Sarosh Arif <sarosh.arif@emumba.com> wrote:
> > 
> > > Combine https://patches.dpdk.org/patch/67855/ with v2 of this patch to fix
> > > the overall behaviour of dpdk-setup.sh on non-alphanumeric inputs.
> > > Instead of crashing the script will prompt the user to re-enter the input
> > > in case of non-alphanumeric input.
> > >
> > > Bugzilla ID: 419
> > > Signed-off-by: Sarosh Arif <sarosh.arif@emumba.com>
> > > Signed-off-by: Muhammad Bilal <m.bilal@emumba.com>
> 
> If no maintainer is defined for a subtree, it should be responsiblity
> of the core maintainers to handle it.
>  
> 
> PS: dpdk-setup has out grown its welcome, and a better version with
> a real UI is needed.

I don't like this script.
It works only for few cases.
I would like to remove it.
Jerin Jacob July 24, 2020, 2:05 p.m. UTC | #4 
On Fri, Jul 24, 2020 at 7:00 PM Thomas Monjalon <thomas@monjalon.net> wrote:
>
> 23/07/2020 19:00, Stephen Hemminger:
> > On Thu, 23 Jul 2020 17:13:17 +0500
> > Sarosh Arif <sarosh.arif@emumba.com> wrote:
> >
> > > Since this script has no maintainer who is responsible for providing ACK or
> > > NAK on the patches related to this script?
> > >
> > > On Tue, Jun 2, 2020 at 12:39 PM Sarosh Arif <sarosh.arif@emumba.com> wrote:
> > >
> > > > Combine https://patches.dpdk.org/patch/67855/ with v2 of this patch to fix
> > > > the overall behaviour of dpdk-setup.sh on non-alphanumeric inputs.
> > > > Instead of crashing the script will prompt the user to re-enter the input
> > > > in case of non-alphanumeric input.
> > > >
> > > > Bugzilla ID: 419
> > > > Signed-off-by: Sarosh Arif <sarosh.arif@emumba.com>
> > > > Signed-off-by: Muhammad Bilal <m.bilal@emumba.com>
> >
> > If no maintainer is defined for a subtree, it should be responsiblity
> > of the core maintainers to handle it.
> >
> >
> > PS: dpdk-setup has out grown its welcome, and a better version with
> > a real UI is needed.
>
> I don't like this script.
> It works only for few cases.
> I would like to remove it.

+1

>
>

Patch
diff mbox series 

diff --git a/usertools/dpdk-setup.sh b/usertools/dpdk-setup.sh
index e5bbe9fee..69cd1d2bd 100755
--- a/usertools/dpdk-setup.sh
+++ b/usertools/dpdk-setup.sh
@@ -320,14 +320,18 @@  set_non_numa_pages()
 	echo "  enter '64' to reserve 64 * 2MB pages"
 	echo -n "Number of pages: "
 	read Pages
+	numeric="^[[:digit:]]+$"
+	PG_PATH="/sys/kernel/mm/hugepages/hugepages-${HUGEPGSZ}"
+       if [[ $Pages =~ $numeric ]]; then
+		echo "echo $Pages > $PG_PATH/nr_hugepages" > .echo_tmp
+		echo "Reserving hugepages"
+		sudo sh .echo_tmp
+		rm -f .echo_tmp
+		create_mnt_huge
+	else
+		echo "Please enter a numeric value"
+	fi
 
-	echo "echo $Pages > /sys/kernel/mm/hugepages/hugepages-${HUGEPGSZ}/nr_hugepages" > .echo_tmp
-
-	echo "Reserving hugepages"
-	sudo sh .echo_tmp
-	rm -f .echo_tmp
-
-	create_mnt_huge
 }
 
 #
@@ -343,10 +347,16 @@  set_numa_pages()
 	echo "  enter '64' to reserve 64 * 2MB pages on each node"
 
 	echo > .echo_tmp
+	numeric="^[[:digit:]]+$"
 	for d in /sys/devices/system/node/node? ; do
 		node=$(basename $d)
 		echo -n "Number of pages for $node: "
 		read Pages
+		while [[ ! "$Pages" =~ $numeric ]]; do
+			echo "Please enter a numeric value"
+			echo -n "Number of pages for $node: "
+			read Pages
+		done
 		echo "echo $Pages > $d/hugepages/hugepages-${HUGEPGSZ}/nr_hugepages" >> .echo_tmp
 	done
 	echo "Reserving hugepages"
@@ -592,10 +602,15 @@  while [ "$QUIT" == "0" ]; do
 	echo "[$OPTION_NUM] Exit Script"
 	OPTIONS[$OPTION_NUM]="quit"
 	echo ""
-	echo -n "Option: "
-	read our_entry
-	echo ""
-	${OPTIONS[our_entry]} ${our_entry}
+	read -p "Option: " our_entry
+       [ $? -eq 0 ] || exit 0
+
+ 	numeric="^[[:digit:]]+$"
+       if [[ "$our_entry" =~ $numeric ]]; then
+     	        ${OPTIONS[our_entry]} ${our_entry}
+       else
+     	        echo "Please enter a numeric value"
+       fi
 
 	if [ "$QUIT" == "0" ] ; then
 		echo