From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id 39867A04AD; Fri, 1 May 2020 16:00:44 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 2A0DB1DA63; Fri, 1 May 2020 16:00:30 +0200 (CEST) Received: from mga07.intel.com (mga07.intel.com [134.134.136.100]) by dpdk.org (Postfix) with ESMTP id A49D61DA62 for ; Fri, 1 May 2020 16:00:28 +0200 (CEST) IronPort-SDR: pvnCEgRJycwCN4dUdGfzlzbMMCYAt/6kSq97/wv8Xunmg6eGsVBFEJm5sDkRUw82tyc97KmwgM XCnOO8FGAJuQ== X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga002.jf.intel.com ([10.7.209.21]) by orsmga105.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 01 May 2020 07:00:27 -0700 IronPort-SDR: kR8FawwonIlRcF1RgOOZIFCtfKAuGlR2XSH2le5w0KzJkgFdZbalbHVbbh3n+NK+n1Qau8iLhr ZWK6jXxaNF7g== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.73,339,1583222400"; d="scan'208";a="276829245" Received: from fmsmsx108.amr.corp.intel.com ([10.18.124.206]) by orsmga002.jf.intel.com with ESMTP; 01 May 2020 07:00:27 -0700 Received: from fmsmsx121.amr.corp.intel.com (10.18.125.36) by FMSMSX108.amr.corp.intel.com (10.18.124.206) with Microsoft SMTP Server (TLS) id 14.3.439.0; Fri, 1 May 2020 07:00:27 -0700 Received: from bgsmsx151.gar.corp.intel.com (10.224.48.42) by fmsmsx121.amr.corp.intel.com (10.18.125.36) with Microsoft SMTP Server (TLS) id 14.3.439.0; Fri, 1 May 2020 07:00:27 -0700 Received: from bgsmsx101.gar.corp.intel.com ([169.254.1.75]) by BGSMSX151.gar.corp.intel.com ([169.254.3.182]) with mapi id 14.03.0439.000; Fri, 1 May 2020 19:30:23 +0530 From: "Varghese, Vipin" To: "pbhagavatula@marvell.com" , "jerinj@marvell.com" , "thomas@monjalon.net" , "Mcnamara, John" , "Kovacevic, Marko" , Ori Kam , "Richardson, Bruce" , "Nicolau, Radu" , Akhil Goyal , "Kantecki, Tomasz" , Sunil Kumar Kori CC: "aostruszka@marvell.com" , "dev@dpdk.org" , Vamsi Attunuru Thread-Topic: [dpdk-dev] [PATCH v4] examples/l2fwd: add cmdline option for forwarding port info Thread-Index: AQHWHMIrtjuhS3dMC0Ocl5Sv97Y/xaiTRbQg Date: Fri, 1 May 2020 14:00:22 +0000 Message-ID: <4C9E0AB70F954A408CC4ADDBF0F8FA7D4D4BB66C@BGSMSX101.gar.corp.intel.com> References: <20200427075944.1314-1-pbhagavatula@marvell.com> <20200427183118.3315-1-pbhagavatula@marvell.com> In-Reply-To: <20200427183118.3315-1-pbhagavatula@marvell.com> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: dlp-product: dlpe-windows dlp-version: 11.2.0.6 dlp-reaction: no-action x-originating-ip: [10.223.10.10] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Subject: Re: [dpdk-dev] [PATCH v4] examples/l2fwd: add cmdline option for forwarding port info X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" Hi Vamsi & Pavan, I like this idea, couple of queries snipped > +static int > +check_port_pair_config(void) > +{ > + uint32_t port_pair_config_mask =3D 0; > + uint32_t port_pair_mask =3D 0; > + uint16_t index, i, portid; > + > + for (index =3D 0; index < nb_port_pair_params; index++) { > + port_pair_mask =3D 0; > + > + for (i =3D 0; i < NUM_PORTS; i++) { > + portid =3D port_pair_params[index].port[i]; > + if ((l2fwd_enabled_port_mask & (1 << portid)) =3D=3D 0) { > + printf("port %u is not enabled in port > mask\n", > + portid); > + return -1; > + } > + if (!rte_eth_dev_is_valid_port(portid)) { > + printf("port %u is not present on the > board\n", > + portid); > + return -1; > + } > + Should we check & warn the user if=20 1. port speed mismatch 2. on different NUMA 3. port pairs are physical and vdev like tap, and KNI (performance). > + port_pair_mask |=3D 1 << portid; > + } > + snipped >=20 > + if (port_pair_params !=3D NULL) { > + if (check_port_pair_config() < 0) > + rte_exit(EXIT_FAILURE, "Invalid port pair config\n"); > + } > + > /* check port mask to possible port mask */ > if (l2fwd_enabled_port_mask & ~((1 << nb_ports) - 1)) > rte_exit(EXIT_FAILURE, "Invalid portmask; possible (0x%x)\n", > @@ -565,26 +689,35 @@ main(int argc, char **argv) > l2fwd_dst_ports[portid] =3D 0; > last_port =3D 0; >=20 Should not the check_port_pair be after this? If the port is not enabled in= port_mask will you skip that pair? or skip RX-TX from that port? > - /* > - * Each logical core is assigned a dedicated TX queue on each port. > - */ > - RTE_ETH_FOREACH_DEV(portid) { > - /* skip ports that are not enabled */ > - if ((l2fwd_enabled_port_mask & (1 << portid)) =3D=3D 0) > - continue; > + /* populate destination port details */ > + if (port_pair_params !=3D NULL) { > + uint16_t idx, p; >=20 > - if (nb_ports_in_mask % 2) { > - l2fwd_dst_ports[portid] =3D last_port; > - l2fwd_dst_ports[last_port] =3D portid; > + for (idx =3D 0; idx < (nb_port_pair_params << 1); idx++) { > + p =3D idx & 1; > + portid =3D port_pair_params[idx >> 1].port[p]; > + l2fwd_dst_ports[portid] =3D > + port_pair_params[idx >> 1].port[p ^ 1]; > } > - else > - last_port =3D portid; > + } else { > + RTE_ETH_FOREACH_DEV(portid) { > + /* skip ports that are not enabled */ > + if ((l2fwd_enabled_port_mask & (1 << portid)) =3D=3D 0) > + continue; >=20 > - nb_ports_in_mask++; > - } > - if (nb_ports_in_mask % 2) { > - printf("Notice: odd number of ports in portmask.\n"); > - l2fwd_dst_ports[last_port] =3D last_port; > + if (nb_ports_in_mask % 2) { > + l2fwd_dst_ports[portid] =3D last_port; > + l2fwd_dst_ports[last_port] =3D portid; > + } else { > + last_port =3D portid; > + } > + > + nb_ports_in_mask++; > + } > + if (nb_ports_in_mask % 2) { > + printf("Notice: odd number of ports in portmask.\n"); > + l2fwd_dst_ports[last_port] =3D last_port; > + } > } As mentioned above there can ports in mask which might be disabled for port= pair. Should not that be skipped rather than setting last port rx-tx loopb= ack? >=20 > rx_lcore_id =3D 0; > @@ -613,7 +746,8 @@ main(int argc, char **argv) >=20 > qconf->rx_port_list[qconf->n_rx_port] =3D portid; > qconf->n_rx_port++; > - printf("Lcore %u: RX port %u\n", rx_lcore_id, portid); > + printf("Lcore %u: RX port %u TX port %u\n", rx_lcore_id, > + portid, l2fwd_dst_ports[portid]); > } >=20 > nb_mbufs =3D RTE_MAX(nb_ports * (nb_rxd + nb_txd + > MAX_PKT_BURST + > -- > 2.17.1