[dpdk-dev] [PATCH v4 3/3] examples/l3fwd: Handle SIGINT and SIGTERM in l3fwd

2015-12-31 Thread Wang, Zhihong


> -Original Message-
> From: Stephen Hemminger [mailto:stephen at networkplumber.org]
> Sent: Thursday, December 31, 2015 10:09 AM
> To: Wang, Zhihong 
> Cc: Ananyev, Konstantin ; dev at dpdk.org; 
> Qiu,
> Michael 
> Subject: Re: [PATCH v4 3/3] examples/l3fwd: Handle SIGINT and SIGTERM in
> l3fwd
> 
> On Thu, 31 Dec 2015 01:44:20 +
> "Wang, Zhihong"  wrote:
> 
> > > > +#define PORT_IDLE 0
> > > > +#define PORT_INIT 1
> > > > +#define PORT_WORK 2
> > > > +#define PORT_STOP 3
> > > > +#define PORT_QUIT 4
> > >
> > > Seems ok, but over-complicated.
> > > I think all you need is just IDLE, INIT, QUIT.
> >
> > Yes for l2/l3fwd 3 states are enough.
> > I implement a full state machine so it can also serve as an example on how 
> > to
> do this in other cases, like where stop might be called before or during init.
> 
> These are examples, it is better to have as little code as necessary to get 
> the job
> done. That makes the example clearer.  Adding extra unnecessary complexity
> just makes it harder to understand.


Thanks for the suggestions!
I'll send the v5 combining your comments and Konstantin's together to make it 
simpler.


[dpdk-dev] [PATCH v4 3/3] examples/l3fwd: Handle SIGINT and SIGTERM in l3fwd

2015-12-31 Thread Wang, Zhihong
> > +#define PORT_IDLE 0
> > +#define PORT_INIT 1
> > +#define PORT_WORK 2
> > +#define PORT_STOP 3
> > +#define PORT_QUIT 4
> 
> Seems ok, but over-complicated.
> I think all you need is just IDLE, INIT, QUIT.

Yes for l2/l3fwd 3 states are enough.
I implement a full state machine so it can also serve as an example on how to 
do this in other cases, like where stop might be called before or during init.

> Konstantin




[dpdk-dev] [PATCH v4 3/3] examples/l3fwd: Handle SIGINT and SIGTERM in l3fwd

2015-12-30 Thread Stephen Hemminger
On Thu, 31 Dec 2015 01:44:20 +
"Wang, Zhihong"  wrote:

> > > +#define PORT_IDLE 0
> > > +#define PORT_INIT 1
> > > +#define PORT_WORK 2
> > > +#define PORT_STOP 3
> > > +#define PORT_QUIT 4  
> > 
> > Seems ok, but over-complicated.
> > I think all you need is just IDLE, INIT, QUIT.  
> 
> Yes for l2/l3fwd 3 states are enough.
> I implement a full state machine so it can also serve as an example on how to 
> do this in other cases, like where stop might be called before or during init.

These are examples, it is better to have as little code as necessary to get the
job done. That makes the example clearer.  Adding extra unnecessary complexity
just makes it harder to understand.


[dpdk-dev] [PATCH v4 3/3] examples/l3fwd: Handle SIGINT and SIGTERM in l3fwd

2015-12-30 Thread Ananyev, Konstantin


> -Original Message-
> From: Wang, Zhihong
> Sent: Tuesday, December 29, 2015 11:27 PM
> To: dev at dpdk.org
> Cc: Ananyev, Konstantin; stephen at networkplumber.org; Qiu, Michael; Wang, 
> Zhihong
> Subject: [PATCH v4 3/3] examples/l3fwd: Handle SIGINT and SIGTERM in l3fwd
> 
> Handle SIGINT and SIGTERM in l3fwd.
> 
> Signed-off-by: Zhihong Wang 
> Acked-by: Michael Qiu 
> ---
>  examples/l3fwd/main.c | 167 
> +++---
>  1 file changed, 145 insertions(+), 22 deletions(-)
> 
> diff --git a/examples/l3fwd/main.c b/examples/l3fwd/main.c
> index 5b0c2dd..f73d2a4 100644
> --- a/examples/l3fwd/main.c
> +++ b/examples/l3fwd/main.c
> @@ -41,6 +41,9 @@
>  #include 
>  #include 
>  #include 
> +#include 
> +#include 
> +#include 
> 
>  #include 
>  #include 
> @@ -75,6 +78,16 @@
>  #include 
>  #include 
> 
> +#define PORT_IDLE 0
> +#define PORT_INIT 1
> +#define PORT_WORK 2
> +#define PORT_STOP 3
> +#define PORT_QUIT 4

Seems ok, but over-complicated.
I think all you need is just IDLE, INIT, QUIT.
Konstantin

> +
> +static volatile uint32_t port_status;
> +static volatile bool force_quit;
> +static volatile int signo_quit;
> +
>  #define APP_LOOKUP_EXACT_MATCH  0
>  #define APP_LOOKUP_LPM  1
>  #define DO_RFC_1812_CHECKS
> @@ -1553,8 +1566,7 @@ main_loop(__attribute__((unused)) void *dummy)
>   portid, queueid);
>   }
> 
> - while (1) {
> -
> + while (!force_quit) {
>   cur_tsc = rte_rdtsc();
> 
>   /*
> @@ -2516,8 +2528,12 @@ check_all_ports_link_status(uint8_t port_num, uint32_t 
> port_mask)
>   printf("\nChecking link status");
>   fflush(stdout);
>   for (count = 0; count <= MAX_CHECK_TIME; count++) {
> + if (force_quit)
> + return;
>   all_ports_up = 1;
>   for (portid = 0; portid < port_num; portid++) {
> + if (force_quit)
> + return;
>   if ((port_mask & (1 << portid)) == 0)
>   continue;
>   memset(, 0, sizeof(link));
> @@ -2559,6 +2575,101 @@ check_all_ports_link_status(uint8_t port_num, 
> uint32_t port_mask)
>   }
>  }
> 
> +static uint8_t
> +start_ports(void)
> +{
> + unsigned portid, nb_ports, avail_ports;
> + int ret;
> +
> + if (rte_atomic32_cmpset(_status,
> + PORT_IDLE, PORT_INIT) == 0) {
> + printf("Ports not idle...\n");
> + return 0;
> + }
> +
> + nb_ports = rte_eth_dev_count();
> + avail_ports = 0;
> + for (portid = 0; portid < nb_ports; portid++) {
> + if ((enabled_port_mask & (1 << portid)) == 0)
> + continue;
> + avail_ports++;
> + printf("Starting port %d...", portid);
> + ret = rte_eth_dev_start(portid);
> + if (ret < 0)
> + rte_exit(EXIT_FAILURE,
> + "rte_eth_dev_start: err=%d, port=%d\n",
> + ret, portid);
> + /*
> +  * If enabled, put device in promiscuous mode.
> +  * This allows IO forwarding mode to forward packets
> +  * to itself through 2 cross-connected  ports of the
> +  * target machine.
> +  */
> + if (promiscuous_on)
> + rte_eth_promiscuous_enable(portid);
> + printf(" Done\n");
> + }
> +
> + if (avail_ports) {
> + if (rte_atomic32_cmpset(_status,
> + PORT_INIT, PORT_WORK) == 0)
> + printf("Set port state failed!\n");
> + } else {
> + if (rte_atomic32_cmpset(_status,
> + PORT_INIT, PORT_IDLE) == 0)
> + printf("Set port state failed!\n");
> + }
> +
> + return avail_ports;
> +}
> +
> +static void
> +stop_ports(void)
> +{
> + unsigned portid, nb_ports;
> +
> + if (rte_atomic32_cmpset(_status,
> + PORT_WORK, PORT_STOP) == 0) {
> + printf("Ports not started...\n");
> + return;
> + }
> +
> + nb_ports = rte_eth_dev_count();
> + for (portid = 0; portid < nb_ports; portid++) {
> + if ((enabled_port_mask & (1 << portid)) == 0)
> + continue;
> + printf("Stopping port %d...", portid);
> + rte_eth_dev_stop(portid);
> + rte_eth_dev_close(portid);
> + printf(" Done\n");
> + }
> +
> + if (rte_atomic32_cmpset(_status,
> + PORT_STOP, PORT_IDLE) == 0)
> + printf("Set port state failed!\n");
> +}
> +
> +static void
> +signal_handler(int signum)
> +{
> + if (signum == SIGINT || signum == SIGTERM) {
> + printf("\nSignal %d received, preparing to 

[dpdk-dev] [PATCH v4 3/3] examples/l3fwd: Handle SIGINT and SIGTERM in l3fwd

2015-12-29 Thread Zhihong Wang
Handle SIGINT and SIGTERM in l3fwd.

Signed-off-by: Zhihong Wang 
Acked-by: Michael Qiu 
---
 examples/l3fwd/main.c | 167 +++---
 1 file changed, 145 insertions(+), 22 deletions(-)

diff --git a/examples/l3fwd/main.c b/examples/l3fwd/main.c
index 5b0c2dd..f73d2a4 100644
--- a/examples/l3fwd/main.c
+++ b/examples/l3fwd/main.c
@@ -41,6 +41,9 @@
 #include 
 #include 
 #include 
+#include 
+#include 
+#include 

 #include 
 #include 
@@ -75,6 +78,16 @@
 #include 
 #include 

+#define PORT_IDLE 0
+#define PORT_INIT 1
+#define PORT_WORK 2
+#define PORT_STOP 3
+#define PORT_QUIT 4
+
+static volatile uint32_t port_status;
+static volatile bool force_quit;
+static volatile int signo_quit;
+
 #define APP_LOOKUP_EXACT_MATCH  0
 #define APP_LOOKUP_LPM  1
 #define DO_RFC_1812_CHECKS
@@ -1553,8 +1566,7 @@ main_loop(__attribute__((unused)) void *dummy)
portid, queueid);
}

-   while (1) {
-
+   while (!force_quit) {
cur_tsc = rte_rdtsc();

/*
@@ -2516,8 +2528,12 @@ check_all_ports_link_status(uint8_t port_num, uint32_t 
port_mask)
printf("\nChecking link status");
fflush(stdout);
for (count = 0; count <= MAX_CHECK_TIME; count++) {
+   if (force_quit)
+   return;
all_ports_up = 1;
for (portid = 0; portid < port_num; portid++) {
+   if (force_quit)
+   return;
if ((port_mask & (1 << portid)) == 0)
continue;
memset(, 0, sizeof(link));
@@ -2559,6 +2575,101 @@ check_all_ports_link_status(uint8_t port_num, uint32_t 
port_mask)
}
 }

+static uint8_t
+start_ports(void)
+{
+   unsigned portid, nb_ports, avail_ports;
+   int ret;
+
+   if (rte_atomic32_cmpset(_status,
+   PORT_IDLE, PORT_INIT) == 0) {
+   printf("Ports not idle...\n");
+   return 0;
+   }
+
+   nb_ports = rte_eth_dev_count();
+   avail_ports = 0;
+   for (portid = 0; portid < nb_ports; portid++) {
+   if ((enabled_port_mask & (1 << portid)) == 0)
+   continue;
+   avail_ports++;
+   printf("Starting port %d...", portid);
+   ret = rte_eth_dev_start(portid);
+   if (ret < 0)
+   rte_exit(EXIT_FAILURE,
+   "rte_eth_dev_start: err=%d, port=%d\n",
+   ret, portid);
+   /*
+* If enabled, put device in promiscuous mode.
+* This allows IO forwarding mode to forward packets
+* to itself through 2 cross-connected  ports of the
+* target machine.
+*/
+   if (promiscuous_on)
+   rte_eth_promiscuous_enable(portid);
+   printf(" Done\n");
+   }
+
+   if (avail_ports) {
+   if (rte_atomic32_cmpset(_status,
+   PORT_INIT, PORT_WORK) == 0)
+   printf("Set port state failed!\n");
+   } else {
+   if (rte_atomic32_cmpset(_status,
+   PORT_INIT, PORT_IDLE) == 0)
+   printf("Set port state failed!\n");
+   }
+
+   return avail_ports;
+}
+
+static void
+stop_ports(void)
+{
+   unsigned portid, nb_ports;
+
+   if (rte_atomic32_cmpset(_status,
+   PORT_WORK, PORT_STOP) == 0) {
+   printf("Ports not started...\n");
+   return;
+   }
+
+   nb_ports = rte_eth_dev_count();
+   for (portid = 0; portid < nb_ports; portid++) {
+   if ((enabled_port_mask & (1 << portid)) == 0)
+   continue;
+   printf("Stopping port %d...", portid);
+   rte_eth_dev_stop(portid);
+   rte_eth_dev_close(portid);
+   printf(" Done\n");
+   }
+
+   if (rte_atomic32_cmpset(_status,
+   PORT_STOP, PORT_IDLE) == 0)
+   printf("Set port state failed!\n");
+}
+
+static void
+signal_handler(int signum)
+{
+   if (signum == SIGINT || signum == SIGTERM) {
+   printf("\nSignal %d received, preparing to exit...\n",
+   signum);
+   if (rte_atomic32_cmpset(_status,
+   PORT_IDLE, PORT_QUIT) == 0) {
+   printf("Ports started already...\n");
+   signo_quit = signum;
+   force_quit = true;
+   } else {
+   printf("Ports not started yet...\n");
+   printf("Bye...\n");
+   /* exit with the expected status */
+