Hi Stephen, Really appreciate the detailed review! Please see comments below.
> > +static int force_quit = -1; > > +static int signo_quit = -1; > > These need to be volatile otherwise you risk compiler optimizing away your > checks. Yes. Don't wanna take chances here. > > Also, don't use -1/0 just use 0/1 for boolean or better yet the definition in > <stdbool.h> of bool and true/false. > That way the code can read much nicer. -1 when forwarding not started yet. Can add a "static bool fwd_started;" to represent this to make it clearer. > > > #define RTE_LOGTYPE_L2FWD RTE_LOGTYPE_USER1 > > > > #define NB_MBUF 8192 > > @@ -284,6 +289,8 @@ l2fwd_main_loop(void) > > } > > > > while (1) { > > + if (unlikely(force_quit != 0)) > > + break; > > Please maske this a proper while loop instead. Exactly. > > while (!force_quit) { > > > > > cur_tsc = rte_rdtsc(); > > > > @@ -534,6 +541,45 @@ check_all_ports_link_status(uint8_t port_num, > uint32_t port_mask) > > } > > } > > > > +static void > > +stop_ports(void) > > +{ > > + unsigned portid, nb_ports; > > + > > + nb_ports = rte_eth_dev_count(); > > + for (portid = 0; portid < nb_ports; portid++) { > > + if ((l2fwd_enabled_port_mask & (1 << portid)) == 0) { > > + continue; > > + } > > No need for {} here. > > > + printf("Stopping port %d...", portid); > > + rte_eth_dev_stop(portid); > > + rte_eth_dev_close(portid); > > + printf(" Done\n"); > > + } > > +} > > + > > +static void > > +signal_handler(__rte_unused int signum) { > > + if (signum == SIGINT || signum == SIGTERM) { > > signum is used, dont give __rte_unused attribute. > > > > > /* launch per-lcore init on every lcore */ > > + force_quit = 0; > > What is gained by having tri-value here. Just initialize it as false. As stated above. > > > > rte_eal_mp_remote_launch(l2fwd_launch_one_lcore, NULL, > CALL_MASTER); > > RTE_LCORE_FOREACH_SLAVE(lcore_id) { > > if (rte_eal_wait_lcore(lcore_id) < 0) > > return -1; > > } > > > > + printf("Stopping forwarding... Done\n"); > > + /* stop ports */ > > + stop_ports(); > > + printf("Bye...\n"); > > + /* inform if there's a caller */ > > + if (force_quit != 0) { > > + signal(signo_quit, SIG_DFL); > > + kill(getpid(), signo_quit); > > The kill should not be needed. The purpose is to make the program exit with the killed status. > > It would be good if examples cleaned up allocations, that way they could be > used > with valgrind for validation of drivers, etc.