[v2] app/pdump: add exit_with_primary option support.
Checks
Commit Message
When primary app exits, the residual running pdump will stop
the primary app to restart. Add an exit_with_primary option
to make pdump exit with primary.
Suggested-by: Varghese, Vipin <vipin.varghese@intel.com>
Suggested-by: Burakov, Anatoly <anatoly.burakov@intel.com>
Signed-off-by: Suanming.Mou <mousuanming@huawei.com>
---
app/pdump/main.c | 26 ++++++++++++++++++++++++++
1 file changed, 26 insertions(+)
Comments
On 28-Apr-19 5:58 AM, Suanming.Mou wrote:
> When primary app exits, the residual running pdump will stop
> the primary app to restart. Add an exit_with_primary option
> to make pdump exit with primary.
>
> Suggested-by: Varghese, Vipin <vipin.varghese@intel.com>
> Suggested-by: Burakov, Anatoly <anatoly.burakov@intel.com>
> Signed-off-by: Suanming.Mou <mousuanming@huawei.com>
> ---
> app/pdump/main.c | 26 ++++++++++++++++++++++++++
> 1 file changed, 26 insertions(+)
>
> diff --git a/app/pdump/main.c b/app/pdump/main.c
> index 3d20854..3909f15 100644
> --- a/app/pdump/main.c
> +++ b/app/pdump/main.c
> @@ -26,11 +26,14 @@
> #include <rte_ring.h>
> #include <rte_string_fns.h>
> #include <rte_pdump.h>
> +#include <rte_alarm.h>
>
> #define CMD_LINE_OPT_PDUMP "pdump"
> #define CMD_LINE_OPT_PDUMP_NUM 256
> #define CMD_LINE_OPT_MULTI "multi"
> #define CMD_LINE_OPT_MULTI_NUM 257
> +#define CMD_LINE_OPT_EXIT_WP "exit_with_primary"
> +#define CMD_LINE_OPT_EXIT_WP_NUM 258
Unrelated to this patch, but seems very flaky and prone to error. How
about replacing this stuff with enum-based automatic value assignment,
like in lib/librte_eal/common/eal_options.h ?
> #define PDUMP_PORT_ARG "port"
> #define PDUMP_PCI_ARG "device_id"
> #define PDUMP_QUEUE_ARG "queue"
> @@ -65,6 +68,7 @@
> #define SIZE 256
> #define BURST_SIZE 32
> #define NUM_VDEVS 2
> +#define MONITOR_INTERVEL (500 * 1000)
I believe it should be INTERVAL
>
> /* true if x is a power of 2 */
> #define POWEROF2(x) ((((x)-1) & (x)) == 0)
> @@ -143,12 +147,14 @@ struct parse_val {
> static struct rte_eth_conf port_conf_default;
> static volatile uint8_t quit_signal;
> static uint8_t multiple_core_capture;
> +static uint8_t exit_with_primary;
>
<snip>
>
> @@ -403,6 +410,9 @@ struct parse_val {
> case CMD_LINE_OPT_MULTI_NUM:
> multiple_core_capture = 1;
> break;
> + case CMD_LINE_OPT_EXIT_WP_NUM:
> + exit_with_primary = 1;
> + break;
Any particular reason why it is not made the default?
On 2019/4/29 17:14, Burakov, Anatoly wrote:
> On 28-Apr-19 5:58 AM, Suanming.Mou wrote:
>> When primary app exits, the residual running pdump will stop
>> the primary app to restart. Add an exit_with_primary option
>> to make pdump exit with primary.
>>
>> Suggested-by: Varghese, Vipin <vipin.varghese@intel.com>
>> Suggested-by: Burakov, Anatoly <anatoly.burakov@intel.com>
>> Signed-off-by: Suanming.Mou <mousuanming@huawei.com>
>> ---
>> app/pdump/main.c | 26 ++++++++++++++++++++++++++
>> 1 file changed, 26 insertions(+)
>>
>> diff --git a/app/pdump/main.c b/app/pdump/main.c
>> index 3d20854..3909f15 100644
>> --- a/app/pdump/main.c
>> +++ b/app/pdump/main.c
>> @@ -26,11 +26,14 @@
>> #include <rte_ring.h>
>> #include <rte_string_fns.h>
>> #include <rte_pdump.h>
>> +#include <rte_alarm.h>
>> #define CMD_LINE_OPT_PDUMP "pdump"
>> #define CMD_LINE_OPT_PDUMP_NUM 256
>> #define CMD_LINE_OPT_MULTI "multi"
>> #define CMD_LINE_OPT_MULTI_NUM 257
>> +#define CMD_LINE_OPT_EXIT_WP "exit_with_primary"
>> +#define CMD_LINE_OPT_EXIT_WP_NUM 258
>
> Unrelated to this patch, but seems very flaky and prone to error. How
> about replacing this stuff with enum-based automatic value assignment,
> like in lib/librte_eal/common/eal_options.h ?
:)
>
>> #define PDUMP_PORT_ARG "port"
>> #define PDUMP_PCI_ARG "device_id"
>> #define PDUMP_QUEUE_ARG "queue"
>> @@ -65,6 +68,7 @@
>> #define SIZE 256
>> #define BURST_SIZE 32
>> #define NUM_VDEVS 2
>> +#define MONITOR_INTERVEL (500 * 1000)
>
> I believe it should be INTERVAL
Ah, yes, sorry for the typo.
>
>> /* true if x is a power of 2 */
>> #define POWEROF2(x) ((((x)-1) & (x)) == 0)
>> @@ -143,12 +147,14 @@ struct parse_val {
>> static struct rte_eth_conf port_conf_default;
>> static volatile uint8_t quit_signal;
>> static uint8_t multiple_core_capture;
>> +static uint8_t exit_with_primary;
>
> <snip>
Could you please help to confirm that the 'snip' here mean we should
delete the 'exit_with_primary' code?
>
>> @@ -403,6 +410,9 @@ struct parse_val {
>> case CMD_LINE_OPT_MULTI_NUM:
>> multiple_core_capture = 1;
>> break;
>> + case CMD_LINE_OPT_EXIT_WP_NUM:
>> + exit_with_primary = 1;
>> + break;
>
> Any particular reason why it is not made the default?
It's OK to make it default. How about Varghese ?
Thank you for the review.
On 29-Apr-19 10:43 AM, Suanming.Mou wrote:
>
<snip> :)
>>> /* true if x is a power of 2 */
>>> #define POWEROF2(x) ((((x)-1) & (x)) == 0)
>>> @@ -143,12 +147,14 @@ struct parse_val {
>>> static struct rte_eth_conf port_conf_default;
>>> static volatile uint8_t quit_signal;
>>> static uint8_t multiple_core_capture;
>>> +static uint8_t exit_with_primary;
>>
>> <snip>
> Could you please help to confirm that the 'snip' here mean we should
> delete the 'exit_with_primary' code?
No, "snip" means i'm skipping irrelevant sections, just to keep my reply
shorter and to the point :)
@@ -26,11 +26,14 @@
#include <rte_ring.h>
#include <rte_string_fns.h>
#include <rte_pdump.h>
+#include <rte_alarm.h>
#define CMD_LINE_OPT_PDUMP "pdump"
#define CMD_LINE_OPT_PDUMP_NUM 256
#define CMD_LINE_OPT_MULTI "multi"
#define CMD_LINE_OPT_MULTI_NUM 257
+#define CMD_LINE_OPT_EXIT_WP "exit_with_primary"
+#define CMD_LINE_OPT_EXIT_WP_NUM 258
#define PDUMP_PORT_ARG "port"
#define PDUMP_PCI_ARG "device_id"
#define PDUMP_QUEUE_ARG "queue"
@@ -65,6 +68,7 @@
#define SIZE 256
#define BURST_SIZE 32
#define NUM_VDEVS 2
+#define MONITOR_INTERVEL (500 * 1000)
/* true if x is a power of 2 */
#define POWEROF2(x) ((((x)-1) & (x)) == 0)
@@ -143,12 +147,14 @@ struct parse_val {
static struct rte_eth_conf port_conf_default;
static volatile uint8_t quit_signal;
static uint8_t multiple_core_capture;
+static uint8_t exit_with_primary;
/**< display usage */
static void
pdump_usage(const char *prgname)
{
printf("usage: %s [EAL options]"
+ " --["CMD_LINE_OPT_EXIT_WP"]"
" --["CMD_LINE_OPT_MULTI"]\n"
" --"CMD_LINE_OPT_PDUMP" "
"'(port=<port id> | device_id=<pci id or vdev name>),"
@@ -383,6 +389,7 @@ struct parse_val {
static struct option long_option[] = {
{CMD_LINE_OPT_PDUMP, 1, 0, CMD_LINE_OPT_PDUMP_NUM},
{CMD_LINE_OPT_MULTI, 0, 0, CMD_LINE_OPT_MULTI_NUM},
+ {CMD_LINE_OPT_EXIT_WP, 0, 0, CMD_LINE_OPT_EXIT_WP_NUM},
{NULL, 0, 0, 0}
};
@@ -403,6 +410,9 @@ struct parse_val {
case CMD_LINE_OPT_MULTI_NUM:
multiple_core_capture = 1;
break;
+ case CMD_LINE_OPT_EXIT_WP_NUM:
+ exit_with_primary = 1;
+ break;
default:
pdump_usage(prgname);
return -1;
@@ -864,12 +874,28 @@ struct parse_val {
return 0;
}
+static void monitor_primary(void *arg __rte_unused)
+{
+ if (quit_signal)
+ return;
+
+ if (rte_eal_primary_proc_alive(NULL))
+ rte_eal_alarm_set(MONITOR_INTERVEL, monitor_primary, NULL);
+ else
+ quit_signal = 1;
+
+ return;
+}
+
static inline void
dump_packets(void)
{
int i;
uint32_t lcore_id = 0;
+ if (exit_with_primary)
+ rte_eal_alarm_set(MONITOR_INTERVEL, monitor_primary, NULL);
+
if (!multiple_core_capture) {
printf(" core (%u), capture for (%d) tuples\n",
rte_lcore_id(), num_tuples);