Hi
> -----Original Message-----
> From: Zhang, AlvinX <alvinx.zh...@intel.com>
> Sent: Saturday, September 18, 2021 11:07
> To: Li, Xiaoyun <xiaoyun...@intel.com>; Ananyev, Konstantin
> <konstantin.anan...@intel.com>
> Cc: dev@dpdk.org; Zhang, AlvinX <alvinx.zh...@intel.com>
> Subject: [PATCH v3 1/2] app/testpmd: update forward engine beginning
>
> For each forward engine, there may be some special conditions must be meet
met
> before the forwarding run.
is running / runs
>
> Adding checks for these conditions in configuring is not suitable, because one
> condition may rely on multiple configurations, and the conditions required by
> each forward engine is not general.
>
> The best solution is each forward engine has a callback to check whether these
> conditions are meet, and then testpmd can call the callback to determine
met
> whether the forwarding can be started.
>
> There was a void callback 'port_fwd_begin' in forward engine, it did some
> initialization for forwarding, this patch updates it's return type, then we
> can add
its return value then we can
> some checks in it to confirm whether the forwarding can be started. In
> addition,
> this patch puts the calling part of the callback up to before some forwarding
> related status being set.
this patch calls this callback before the forwarding stats is reset and then
launches the forwarding engine.
Also the cc stable issue I state in the 2nd patch.
But overall the patch looks good to me.
BRs
Xiaoyun
>
> Signed-off-by: Alvin Zhang <alvinx.zh...@intel.com>
> ---
> app/test-pmd/flowgen.c | 3 ++-
> app/test-pmd/ieee1588fwd.c | 3 ++-
> app/test-pmd/noisy_vnf.c | 4 +++-
> app/test-pmd/testpmd.c | 38 ++++++++++++++++++++++++++------------
> app/test-pmd/testpmd.h | 2 +-
> app/test-pmd/txonly.c | 3 ++-
> 6 files changed, 36 insertions(+), 17 deletions(-)
>
> diff --git a/app/test-pmd/flowgen.c b/app/test-pmd/flowgen.c index
> 0d3664a..83234fc 100644
> --- a/app/test-pmd/flowgen.c
> +++ b/app/test-pmd/flowgen.c
> @@ -201,10 +201,11 @@
> get_end_cycles(fs, start_tsc);
> }
>
> -static void
> +static int
> flowgen_begin(portid_t pi)
> {
> printf(" number of flows for port %u: %d\n", pi, nb_flows_flowgen);
> + return 0;
> }
>
> struct fwd_engine flow_gen_engine = {
> diff --git a/app/test-pmd/ieee1588fwd.c b/app/test-pmd/ieee1588fwd.c index
> 034f238..81624a7 100644
> --- a/app/test-pmd/ieee1588fwd.c
> +++ b/app/test-pmd/ieee1588fwd.c
> @@ -198,10 +198,11 @@ struct ptpv2_msg {
> port_ieee1588_tx_timestamp_check(fs->rx_port);
> }
>
> -static void
> +static int
> port_ieee1588_fwd_begin(portid_t pi)
> {
> rte_eth_timesync_enable(pi);
> + return 0;
> }
>
> static void
> diff --git a/app/test-pmd/noisy_vnf.c b/app/test-pmd/noisy_vnf.c index
> 382a4c2..e4434be 100644
> --- a/app/test-pmd/noisy_vnf.c
> +++ b/app/test-pmd/noisy_vnf.c
> @@ -231,7 +231,7 @@ struct noisy_config {
> rte_free(noisy_cfg[pi]);
> }
>
> -static void
> +static int
> noisy_fwd_begin(portid_t pi)
> {
> struct noisy_config *n;
> @@ -273,6 +273,8 @@ struct noisy_config {
> rte_exit(EXIT_FAILURE,
> "--noisy-lkup-memory-size must be > 0\n");
> }
> +
> + return 0;
> }
>
> struct fwd_engine noisy_vnf_engine = {
> diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c index
> 97ae52e..0345b2e 100644
> --- a/app/test-pmd/testpmd.c
> +++ b/app/test-pmd/testpmd.c
> @@ -2172,16 +2172,10 @@ struct extmem_param { static void
> launch_packet_forwarding(lcore_function_t *pkt_fwd_on_lcore) {
> - port_fwd_begin_t port_fwd_begin;
> unsigned int i;
> unsigned int lc_id;
> int diag;
>
> - port_fwd_begin = cur_fwd_config.fwd_eng->port_fwd_begin;
> - if (port_fwd_begin != NULL) {
> - for (i = 0; i < cur_fwd_config.nb_fwd_ports; i++)
> - (*port_fwd_begin)(fwd_ports_ids[i]);
> - }
> for (i = 0; i < cur_fwd_config.nb_fwd_lcores; i++) {
> lc_id = fwd_lcores_cpuids[i];
> if ((interactive == 0) || (lc_id != rte_lcore_id())) { @@
> -2227,10
> +2221,35 @@ struct extmem_param {
> fprintf(stderr, "Packet forwarding already started\n");
> return;
> }
> - test_done = 0;
>
> fwd_config_setup();
>
> + port_fwd_begin = cur_fwd_config.fwd_eng->port_fwd_begin;
> + if (port_fwd_begin != NULL) {
> + for (i = 0; i < cur_fwd_config.nb_fwd_ports; i++) {
> + if (port_fwd_begin(fwd_ports_ids[i])) {
> + fprintf(stderr,
> + "Packet forwarding not ready\n");
> + return;
> + }
> + }
> + }
> +
> + if (with_tx_first) {
> + port_fwd_begin = tx_only_engine.port_fwd_begin;
> + if (port_fwd_begin != NULL) {
> + for (i = 0; i < cur_fwd_config.nb_fwd_ports; i++) {
> + if (port_fwd_begin(fwd_ports_ids[i])) {
> + fprintf(stderr,
> + "Packet forwarding not
> ready\n");
> + return;
> + }
> + }
> + }
> + }
> +
> + test_done = 0;
> +
> if(!no_flush_rx)
> flush_fwd_rx_queues();
>
> @@ -2239,11 +2258,6 @@ struct extmem_param {
>
> fwd_stats_reset();
> if (with_tx_first) {
> - port_fwd_begin = tx_only_engine.port_fwd_begin;
> - if (port_fwd_begin != NULL) {
> - for (i = 0; i < cur_fwd_config.nb_fwd_ports; i++)
> - (*port_fwd_begin)(fwd_ports_ids[i]);
> - }
> while (with_tx_first--) {
> launch_packet_forwarding(
> run_one_txonly_burst_on_core);
> diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h index
> 5863b2f..e9d9db0 100644
> --- a/app/test-pmd/testpmd.h
> +++ b/app/test-pmd/testpmd.h
> @@ -268,7 +268,7 @@ struct fwd_lcore {
> * Forwards packets unchanged on the same port.
> * Check that sent IEEE1588 PTP packets are timestamped by the hardware.
> */
> -typedef void (*port_fwd_begin_t)(portid_t pi);
> +typedef int (*port_fwd_begin_t)(portid_t pi);
> typedef void (*port_fwd_end_t)(portid_t pi); typedef void
> (*packet_fwd_t)(struct fwd_stream *fs);
>
> diff --git a/app/test-pmd/txonly.c b/app/test-pmd/txonly.c index
> aed820f..386a4ff 100644
> --- a/app/test-pmd/txonly.c
> +++ b/app/test-pmd/txonly.c
> @@ -435,7 +435,7 @@
> get_end_cycles(fs, start_tsc);
> }
>
> -static void
> +static int
> tx_only_begin(portid_t pi)
> {
> uint16_t pkt_data_len;
> @@ -467,6 +467,7 @@
> timestamp_init_req++;
> /* Make sure all settings are visible on forwarding cores.*/
> rte_wmb();
> + return 0;
> }
>
> struct fwd_engine tx_only_engine = {
> --
> 1.8.3.1
