Hi Ori,
Thanks for your comments.
I will update the commit logs as well as the documents.

BR. Bing

> -----Original Message-----
> From: Ori Kam <or...@nvidia.com>
> Sent: Sunday, October 4, 2020 5:40 PM
> To: Bing Zhao <bi...@nvidia.com>; NBU-Contact-Thomas Monjalon
> <tho...@monjalon.net>; ferruh.yi...@intel.com;
> arybche...@solarflare.com; m...@ashroe.eu; nhor...@tuxdriver.com;
> bernard.iremon...@intel.com; beilei.x...@intel.com;
> wenzhuo...@intel.com
> Cc: dev@dpdk.org
> Subject: RE: [PATCH 4/4] app/testpmd: change hairpin queues setup
> 
> Hi Bing,
> 
> PSB,
> 
> Best,
> Ori
> 
> > -----Original Message-----
> > From: Bing Zhao <bi...@nvidia.com>
> > Sent: Thursday, October 1, 2020 3:26 AM
> > Subject: [PATCH 4/4] app/testpmd: change hairpin queues setup
> >
> > A new parameter `hairpin-mode` is introduced to the testpmd
> command
> > line. Bitmask value is used to provide more flexible configuration.
> >
> > Bit 0 in the LSB indicates the hairpin will use the loop mode. The
> > previous port RX queue will be connected to the current port TX
> queue.
> > Bit 1 in the LSB indicates the hairpin will use pair port mode.
> The
> > even index port will be paired with the next odd index port. If
> the
> > total number of probed port is odd, then the last one will be
> paired
> > to itself.
> > If this byte is zero, then each port will be paired to itself.
> > Bit 0 takes a higher priority in the checking.
> >
> > Bit 4 in the second bytes indicate if the hairpin will use
> explicit TX
> > flow mode.
> >
> I think some basic example will be great here.
> 
> > If not set, default value zero will be used and the behavior will
> try
> > to get align with the previous single port mode. If the ports
> belong
> > to different vendors' NICs, it is suggested to use the `self`
> hairpin
> > mode only.
> >
> > Since hairpin configures the hardware resources, the port mask of
> > packets forwarding engine will not be used here.
> >
> > Signed-off-by: Bing Zhao <bi...@nvidia.com>
> > ---
> >  app/test-pmd/parameters.c | 15 +++++++++++
> >  app/test-pmd/testpmd.c    | 68
> > ++++++++++++++++++++++++++++++++++++++++++++---
> >  app/test-pmd/testpmd.h    |  2 ++
> >  3 files changed, 81 insertions(+), 4 deletions(-)
> >
> > diff --git a/app/test-pmd/parameters.c b/app/test-pmd/parameters.c
> > index 1ead595..991029d 100644
> > --- a/app/test-pmd/parameters.c
> > +++ b/app/test-pmd/parameters.c
> > @@ -221,6 +221,9 @@ usage(char* progname)
> >            "enabled\n");
> >     printf("  --record-core-cycles: enable measurement of CPU
> cycles.\n");
> >     printf("  --record-burst-stats: enable display of RX and TX
> > bursts.\n");
> > +   printf("  --hairpin-mode=0xXX: bitmask set the hairpin port
> mode.\n "
> > +          "    0x10 - explicit tx rule, 0x02 - hairpin ports
> paired\n"
> > +          "    0x01 - hairpin ports loop, 0x00 - hairpin port
> self\n");
> >  }
> >
> >  #ifdef RTE_LIBRTE_CMDLINE
> > @@ -644,6 +647,7 @@ launch_args_parse(int argc, char** argv)
> >             { "rxd",                        1, 0, 0 },
> >             { "txd",                        1, 0, 0 },
> >             { "hairpinq",                   1, 0, 0 },
> > +           { "hairpin-mode",               1, 0, 0 },
> >             { "burst",                      1, 0, 0 },
> >             { "mbcache",                    1, 0, 0 },
> >             { "txpt",                       1, 0, 0 },
> > @@ -1111,6 +1115,17 @@ launch_args_parse(int argc, char** argv)
> >                             rte_exit(EXIT_FAILURE, "Either rx or tx
> queues should "
> >                                             "be non-zero\n");
> >                     }
> > +                   if (!strcmp(lgopts[opt_idx].name, "hairpin-mode"))
> {
> > +                           char *end = NULL;
> > +                           unsigned int n;
> > +
> > +                           errno = 0;
> > +                           n = strtoul(optarg, &end, 0);
> > +                           if (errno != 0 || end == optarg)
> > +                                   rte_exit(EXIT_FAILURE, "hairpin mode
> > invalid\n");
> > +                           else
> > +                                   hairpin_mode = (uint16_t)n;
> > +                   }
> >                     if (!strcmp(lgopts[opt_idx].name, "burst")) {
> >                             n = atoi(optarg);
> >                             if (n == 0) {
> > diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c index
> > fe6450c..c604045 100644
> > --- a/app/test-pmd/testpmd.c
> > +++ b/app/test-pmd/testpmd.c
> > @@ -367,6 +367,9 @@ bool setup_on_probe_event = true;
> >  /* Clear ptypes on port initialization. */  uint8_t clear_ptypes
> =
> > true;
> >
> > +/* Hairpin ports configuration mode. */ uint16_t hairpin_mode;
> > +
> >  /* Pretty printing of ethdev events */  static const char * const
> > eth_event_desc[] = {
> >     [RTE_ETH_EVENT_UNKNOWN] = "unknown", @@ -2345,7 +2348,7 @@
> > port_is_started(portid_t port_id)
> >
> >  /* Configure the Rx and Tx hairpin queues for the selected port.
> */
> > static int -setup_hairpin_queues(portid_t pi)
> > +setup_hairpin_queues(portid_t pi, portid_t p_pi, uint16_t cnt_pi)
> >  {
> >     queueid_t qi;
> >     struct rte_eth_hairpin_conf hairpin_conf = { @@ -2354,10
> +2357,48 @@
> > setup_hairpin_queues(portid_t pi)
> >     int i;
> >     int diag;
> >     struct rte_port *port = &ports[pi];
> > +   uint16_t peer_rx_port = pi;
> > +   uint16_t peer_tx_port = pi;
> > +   uint32_t manual = 1;
> > +   uint32_t tx_exp = hairpin_mode & 0x10;
> > +
> > +   if (!(hairpin_mode & 0xf)) {
> > +           peer_rx_port = pi;
> > +           peer_tx_port = pi;
> > +           manual = 0;
> > +   } else if (hairpin_mode & 0x1) {
> > +           peer_tx_port = rte_eth_find_next_owned_by(pi + 1,
> > +
> > RTE_ETH_DEV_NO_OWNER);
> > +           if (peer_tx_port >= RTE_MAX_ETHPORTS)
> > +                   peer_tx_port = rte_eth_find_next_owned_by(0,
> > +                                           RTE_ETH_DEV_NO_OWNER);
> > +           if (p_pi != RTE_MAX_ETHPORTS) {
> > +                   peer_rx_port = p_pi;
> > +           } else {
> > +                   uint16_t next_pi;
> > +
> > +                   RTE_ETH_FOREACH_DEV(next_pi)
> > +                           peer_rx_port = next_pi;
> > +           }
> > +           manual = 1;
> > +   } else if (hairpin_mode & 0x2) {
> > +           if (cnt_pi & 0x1) {
> > +                   peer_rx_port = p_pi;
> > +           } else {
> > +                   peer_rx_port = rte_eth_find_next_owned_by(pi + 1,
> > +                                           RTE_ETH_DEV_NO_OWNER);
> > +                   if (peer_rx_port >= RTE_MAX_ETHPORTS)
> > +                           peer_rx_port = pi;
> > +           }
> > +           peer_tx_port = peer_rx_port;
> > +           manual = 1;
> > +   }
> >
> >     for (qi = nb_txq, i = 0; qi < nb_hairpinq + nb_txq; qi++) {
> > -           hairpin_conf.peers[0].port = pi;
> > +           hairpin_conf.peers[0].port = peer_rx_port;
> >             hairpin_conf.peers[0].queue = i + nb_rxq;
> > +           hairpin_conf.manual_bind = !!manual;
> > +           hairpin_conf.tx_explicit = !!tx_exp;
> >             diag = rte_eth_tx_hairpin_queue_setup
> >                     (pi, qi, nb_txd, &hairpin_conf);
> >             i++;
> > @@ -2377,8 +2418,10 @@ setup_hairpin_queues(portid_t pi)
> >             return -1;
> >     }
> >     for (qi = nb_rxq, i = 0; qi < nb_hairpinq + nb_rxq; qi++) {
> > -           hairpin_conf.peers[0].port = pi;
> > +           hairpin_conf.peers[0].port = peer_tx_port;
> >             hairpin_conf.peers[0].queue = i + nb_txq;
> > +           hairpin_conf.manual_bind = !!manual;
> > +           hairpin_conf.tx_explicit = !!tx_exp;
> >             diag = rte_eth_rx_hairpin_queue_setup
> >                     (pi, qi, nb_rxd, &hairpin_conf);
> >             i++;
> > @@ -2405,6 +2448,8 @@ start_port(portid_t pid)  {
> >     int diag, need_check_link_status = -1;
> >     portid_t pi;
> > +   portid_t p_pi = RTE_MAX_ETHPORTS;
> > +   uint16_t cnt_pi = 0;
> >     queueid_t qi;
> >     struct rte_port *port;
> >     struct rte_ether_addr mac_addr;
> > @@ -2544,8 +2589,10 @@ start_port(portid_t pid)
> >                             return -1;
> >                     }
> >                     /* setup hairpin queues */
> > -                   if (setup_hairpin_queues(pi) != 0)
> > +                   if (setup_hairpin_queues(pi, p_pi, cnt_pi) != 0)
> >                             return -1;
> > +                   p_pi = pi;
> > +                   cnt_pi++;
> >             }
> >             configure_rxtx_dump_callbacks(verbose_level);
> >             if (clear_ptypes) {
> > @@ -3775,6 +3822,19 @@ main(int argc, char** argv)
> >             rte_exit(EXIT_FAILURE, "Start ports failed\n");
> >
> >     /* set all ports to promiscuous mode by default */
> > +   if (hairpin_mode & 0x3) {
> > +           RTE_ETH_FOREACH_DEV(port_id) {
> > +                   ret = rte_eth_hairpin_bind(port_id,
> > RTE_MAX_ETHPORTS);
> > +                   if (ret != 0) {
> > +                           RTE_LOG(ERR, EAL, "Error during binding "
> > +                                   "hairpin tx port %u: %s",
> > +                                   port_id, rte_strerror(-ret));
> > +                           return -1;
> > +                   }
> > +           }
> > +   }
> > +
> > +   /* set all ports to promiscuous mode by default */
> >     RTE_ETH_FOREACH_DEV(port_id) {
> >             ret = rte_eth_promiscuous_enable(port_id);
> >             if (ret != 0)
> > diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h index
> > c7e7e41..29ede20 100644
> > --- a/app/test-pmd/testpmd.h
> > +++ b/app/test-pmd/testpmd.h
> > @@ -398,6 +398,8 @@ extern uint32_t param_total_num_mbufs;
> >
> >  extern uint16_t stats_period;
> >
> > +extern uint16_t hairpin_mode;
> > +
> >  #ifdef RTE_LIBRTE_LATENCY_STATS
> >  extern uint8_t latencystats_enabled;
> >  extern lcoreid_t latencystats_lcore_id;
> > --
> > 2.5.5

Reply via email to