From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 5989DA00C3; Mon, 17 Jan 2022 19:22:53 +0100 (CET) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id D0F914014F; Mon, 17 Jan 2022 19:22:52 +0100 (CET) Received: from mga07.intel.com (mga07.intel.com [134.134.136.100]) by mails.dpdk.org (Postfix) with ESMTP id 0848A40140 for ; Mon, 17 Jan 2022 19:22:50 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1642443771; x=1673979771; h=message-id:date:to:cc:references:from:subject: in-reply-to:content-transfer-encoding:mime-version; bh=sIRkmM6MrLS5hMXMLgobcef4a8pJteWXQePqigBK0m8=; b=J4fRb8QCGkMNqzw0/RKHIymF24XsxEqxuzL795dH2vWE/vWNn5wcTDY2 pBI8Pjv2xA+J4CKDTwDAQ7EUArIv6L/ukv34SNbPMcPJ5Egyva2dyd3cd CsEiX19Zug1WDXCsh2nW+Q0QkIYH1lcyUl2cOeM15ZNQVNyGIdKOmq9zt 4V+9TRjpSzbwcLNMEUb6j8rzhrfD4R0+rK58QVa4iYBUxYa7wA/u/0HSk H4F14qfmHwQeISES1ieavzpXQqo7uFz4xhnc6tkC9wRPO/uV/Fc75Rvsf 7K2PVVTcjs41w5jwUohK+Vd7qjsOvNIGfp1z+kgrY3+Z8qD2xZjX2+Ws/ w==; X-IronPort-AV: E=McAfee;i="6200,9189,10229"; a="308013369" X-IronPort-AV: E=Sophos;i="5.88,296,1635231600"; d="scan'208";a="308013369" Received: from fmsmga005.fm.intel.com ([10.253.24.32]) by orsmga105.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 17 Jan 2022 10:22:48 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.88,296,1635231600"; d="scan'208";a="766713619" Received: from fmsmsx603.amr.corp.intel.com ([10.18.126.83]) by fmsmga005.fm.intel.com with ESMTP; 17 Jan 2022 10:22:48 -0800 Received: from fmsmsx604.amr.corp.intel.com (10.18.126.84) by fmsmsx603.amr.corp.intel.com (10.18.126.83) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2308.20; Mon, 17 Jan 2022 10:22:48 -0800 Received: from fmsedg602.ED.cps.intel.com (10.1.192.136) by fmsmsx604.amr.corp.intel.com (10.18.126.84) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2308.20 via Frontend Transport; Mon, 17 Jan 2022 10:22:48 -0800 Received: from NAM12-MW2-obe.outbound.protection.outlook.com (104.47.66.48) by edgegateway.intel.com (192.55.55.71) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.1.2308.20; Mon, 17 Jan 2022 10:22:47 -0800 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=XF3d7i/G1MLwyPIBG/CEs4ovz2QIXaWpNuYKjA19mFvji+5FgATPF9hOzqJm7ZuT5wjgsJWkOhgro5TD96ylQk+eg5JMa2pItzT3ci43r05xCO1Df4x+NVtsvqI4j3Zk97M0LkNKwlDdfECWRxVXrqkHwgaZSGcevspg05yiiXQPzWcZb0ho1wWTeugAF+udbwK5HLTOErky+rJ3heepKgdHhzoKWOyZTTK2yR9x8SfMdA30EyPddNmZIV4R3/tp6XctTpYRX5lZgS03d/6r7hADvtuPxnQrR8U67ES+/+dYQH97uhy05K2kSpJ2wVs6U/9jglqVghHn4XLJMijalQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=EKdTbsVLf/VOziVSRSx1JtRnaXLHTTElpNSeWtPKWi0=; b=lMf/Kw8sYNSjRRLONHdTWa2e6G6STOgGH0Dg1JLJQj3Y4He45h7R1oUMOoZZToGV3WSaCd5q9HFH5INhn2TrMslhCItpoBFD7I2UDAi0lrWyFL47ETK38Cboaa44bmWU+VIHzH9OQPDR7YI82O47ceyNHLlNlcc3AR0giGgY2BbUjEIghTa8+W4B+wqD6yy6th7dlNfBc/vLkd98WZweMJhhe9L6+htwRH9bAQz087fHeR0Aha18E+CUDBqRkgjkSOCGkJ57VM279WaL8JtrOs9XtlLvqvgk5OsuyPcV/HE2l05pggpEHHmli0S70xr73V8q5gJqiLjesBqxxJIWhg== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=intel.com; dmarc=pass action=none header.from=intel.com; dkim=pass header.d=intel.com; arc=none Authentication-Results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=intel.com; Received: from PH0PR11MB5000.namprd11.prod.outlook.com (2603:10b6:510:41::19) by SA1PR11MB5780.namprd11.prod.outlook.com (2603:10b6:806:233::6) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.4888.11; Mon, 17 Jan 2022 18:22:46 +0000 Received: from PH0PR11MB5000.namprd11.prod.outlook.com ([fe80::5046:8550:928d:850e]) by PH0PR11MB5000.namprd11.prod.outlook.com ([fe80::5046:8550:928d:850e%7]) with mapi id 15.20.4888.014; Mon, 17 Jan 2022 18:22:46 +0000 Message-ID: <83fe377d-adfe-76ca-5bbb-4f02f8575380@intel.com> Date: Mon, 17 Jan 2022 18:22:40 +0000 Content-Language: en-US To: Kumara Parameshwaran , CC: , Kumara Parameshwaran , "Raslan Darawsheh" References: <20211126041515.96259-1-kumaraparamesh92@gmail.com> From: Ferruh Yigit Subject: Re: [PATCH] net/tap: Bug fix to populate fds in secondary process X-User: ferruhy In-Reply-To: <20211126041515.96259-1-kumaraparamesh92@gmail.com> Content-Type: text/plain; charset="UTF-8"; format=flowed Content-Transfer-Encoding: 7bit X-ClientProxiedBy: LO2P265CA0478.GBRP265.PROD.OUTLOOK.COM (2603:10a6:600:a2::34) To PH0PR11MB5000.namprd11.prod.outlook.com (2603:10b6:510:41::19) MIME-Version: 1.0 X-MS-PublicTrafficType: Email X-MS-Office365-Filtering-Correlation-Id: 9b157971-e144-42ad-b7b7-08d9d9e65c3f X-MS-TrafficTypeDiagnostic: SA1PR11MB5780:EE_ X-Microsoft-Antispam-PRVS: X-MS-Oob-TLC-OOBClassifiers: OLM:612; X-MS-Exchange-SenderADCheck: 1 X-MS-Exchange-AntiSpam-Relay: 0 X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: LhzdlBPuGQoegkEPX7Wzrr4lUD6gDx17MmKbh0HUkza3PAM5b85m8+TC9kQODrYbDa3UB7Yw+QzKVvCFaVawxXvfHV2h82qEeS6bZ2Qp8+w6aJQw3DgLacW1RHI+lLou5WVmeY3snR/91Inh00mLbihS5GmN2jCNdJG4oXp7NjQ83ygNjZQCP4eWj4Sn3Fgwm1r+KWhZBX45+dhKYR5X708VQrMHb8YBiLiXRULl0rOCjaEHuLd90WgR9uYn3+twUgM7OpaUgG+TsbyRt+0RLn3TTJzHLqR8GHeglYdJ/x7PBDT2weoAnMeBl9AOmE37cetVc1qvivEiHnolmYMYlLYhfv+UHWsfbRmDk+Ughcn2SzClraz/VqkzVpViXn2DtDb6+Rnu1bAULvsbdUBvgnEtNkVoxeYtjwW8lo4uKutjv3D9kd+mOMwNtc0wVqhvhJXFEnWymrWxxNu4rLvIq+kIaLP1G6NFvS/bRjmBr6Pxu1VttnBuz9QhWdEvW8DiJEwiQMu7+gIL648/jS3SaSs4pRL+pBFYNRBjweC9P/SB9/3X6hdx0UWG+3NiU4Gfdrvab1SVemzfjMEXFJe7SNeHEBOsQVrnQcOphsUI0/E98gWEVuXGXALHlQXpVa5ZwgoEofQpg1Kj9pLRjz9x5ic8jyjqNPxF1ZLvB9Oamo+//oah35VqiwQfLSMdFo0sJmpf5Ir3JKKEYvPQ67AR8w== X-Forefront-Antispam-Report: CIP:255.255.255.255; CTRY:; LANG:en; SCL:1; SRV:; IPV:NLI; SFV:NSPM; H:PH0PR11MB5000.namprd11.prod.outlook.com; PTR:; CAT:NONE; SFS:(366004)(4326008)(6512007)(186003)(8936002)(6506007)(36756003)(83380400001)(316002)(38100700002)(6486002)(2616005)(54906003)(6636002)(86362001)(26005)(8676002)(6666004)(31686004)(44832011)(2906002)(31696002)(508600001)(5660300002)(82960400001)(66946007)(66476007)(66556008)(53546011)(45980500001); DIR:OUT; SFP:1102; X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1 X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?M1ZGOHlzMklOQjNFUmNhSDBIWG9DM1NuQWZXcjYyUzJjNFduT0tQNE9oa01M?= =?utf-8?B?RkhaN0gyYlJPWHNtS0lMcWNPVGJ1UUYxQ2RiSHJYZitjVDVNMU94Y3JBN3pl?= =?utf-8?B?MlhqZnZmczBLcGY3aVlpSzY2WC94QnpEdERCWEtiNklrLzBFKytLWi9sRzhD?= =?utf-8?B?V2NZaUhmWHJ5aVNuYWcwOHBZVkptNEFsVTc5VlppcTFtY2JCQVJ0a2hEZ3JL?= =?utf-8?B?Zlh5LzdVaVIzUGE3cDQ3VkVCcGF3cC9UL095MDVQdE5aaStBeUhzL0xSbm8x?= =?utf-8?B?SytMZHhrZmp1STQxWWxrL1RmbnB3amJBOU11YmFYOU1vY1Z4KzZkbUY5S0M3?= =?utf-8?B?eWxPcktyZXpGSnUyM0ZSWHpOYlNYbjVsMGd3cndtK1RHMytwMU8xM1NmRlQ5?= =?utf-8?B?MzRDSDdjczJ6T3lhVkxSbzlvYytjbnRjZkJlbHArSDRPVGRDQ1FFR2dBTzdj?= =?utf-8?B?QXZhZHhaK3pwMUJiRnFsVXF2UXh2dklPNmhJajJQR245WFpLMHpIZmhSeENU?= =?utf-8?B?Y21QYUc2dGF6dzJRNnA4UUhjVEVraGd2ZVNTd3YxUHhVWlRtOC83Vk9OZ1hk?= =?utf-8?B?MFAxTUtqeXByVzVRSUVTUzFGUWRldDhhMFZFWGJOd2dZdUlhRkp1by9BUTJr?= =?utf-8?B?WHRMM2hUNVZtQ2dadGlteEVXYUpGQ0NpZGx4d2RFRFkzb3d0VjBsK3RWWXBz?= =?utf-8?B?Nm5hTHQvK3VnY3JWNWt1c0U5M2lRWDZmRlUxdktaaEptaDBXVjZCaFlMUVY2?= =?utf-8?B?KzVBNldDMU8vOS9vb1MvS0ZLVGF3amJPQkpmdkF6b2lpMEdXcjk1RGNsZ1lz?= =?utf-8?B?V1hsYkxEZmR3c2FyQTBuc2M5dG8wMUZoTGF6bmYvWVNEVVNjLzdTNFN0TG1j?= =?utf-8?B?VW1oMjNFQ0Z1bXUrVUtjaitaQXJwL2NZVGpISWNoZ3U5WDNlOGRPZVkvc3Z5?= =?utf-8?B?K2pCUnFIZUw1WkhNTC9LZytGTlRWbm96MklHNTVCUWhiYWhJYk04ZG0xY09Q?= =?utf-8?B?bTl2Z1dPSmoycWRlNk9WcmpmYkpYSnJxSzBGVnZsMWdUaDRRSVYzZXNuK2pK?= =?utf-8?B?M1BsVW1WLy9jNEV2SC9ubUJCZC9rYThJa1BkTFBIdldGVURUU2RWQU9lazEz?= =?utf-8?B?aTJNZ1J0SlNkRC91NzhTdnJpUVloaDQ0R1VMUFFxZUxJVi8wZzEzbGl2OURi?= =?utf-8?B?YTI2K09XTFoxLzFnS21qejFiNmFud25nRy9PWHhpTnJ1aTduTUZuMGxYdVRv?= =?utf-8?B?Q0tFOFQweHNLdnBia05wTUg1RTJ3UVpqTTNuSEpCbEVmU3gvYlUvUHJidUs5?= =?utf-8?B?cmpXNklXSzlJc3V1cmgvNDY0SVNtSkpnc1ZHNnNrd1NmMEo0L1Q0YnpKVVc4?= =?utf-8?B?R3VTUWJpZEV1Wm05MWg5a09Fd3VWYWgydG4xbUlIRG92L0tBNmJTdi9YRk9h?= =?utf-8?B?bUdvWUNQd1h2TG5SS0JHN1dMYXp3OUtWcDJkRFI0SjhXQXZ3SWlIdmF2dE1r?= =?utf-8?B?T1podmtPRVlabHpGVyt1VGFNOTlDZzFKYXovSmJlQTk1dHh0MzlLK1RQY0hS?= =?utf-8?B?L2FvVW5kMFoxcXBYYkV6ZHdkeEQzR1g0VlUrSmRpMEROTThlajdoK3ZWQzZ5?= =?utf-8?B?SXJsL0VnZlRKa1FMRXl4NGx6dVRoZDVvQklmMjRIVHN6ellMWFdrbXNQVUFv?= =?utf-8?B?SXFJb1RTUUdCdm83aE5jVlZZODlDcGlCWWcwcHZNQTFPTXdVRDlmcjJRMWJP?= =?utf-8?B?TWk1ZnRYVTIrbDBPU2FpaGlMUFZNWHlIazY0Z24vRUZ2K1lLQUQwL0ludVRo?= =?utf-8?B?K0dIMEQ0bjVrbmFDRlBTREtmeDY3MTJVUTFVSFEvNURqdzR2NWNZbWpuWDZy?= =?utf-8?B?K3hUUnZZVXRCU0w2N29ubUZkMWJkME82N1A3dUZSWHhDR3RNeFNjcWRoWnJl?= =?utf-8?B?c1E0endMNkxXT0ZhdW14aTJ2V0lRSld4R3RGOWZpb3VwUkswTjRaT2g5RXhI?= =?utf-8?B?RVpYMkg1UTJybWZGMjJPZFkvVGN4NElOM3ZlT2ZPalhIZGhiUElXOUFNZVB3?= =?utf-8?B?M3RSZENLeXpLNi9oYjQ1L3B1cEh3Tm9YcjZhcWVvUWRmZDI3a3JMb3Jkckpt?= =?utf-8?B?OFdNbXp5MzQ4Nzl0SlBwdDJ4dU1PRXhlZ2lXOEpMV1RvRlREeDVMOEg1d0wz?= =?utf-8?Q?0TFmQwGhA/TICLeUdLmS/aw=3D?= X-MS-Exchange-CrossTenant-Network-Message-Id: 9b157971-e144-42ad-b7b7-08d9d9e65c3f X-MS-Exchange-CrossTenant-AuthSource: PH0PR11MB5000.namprd11.prod.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-OriginalArrivalTime: 17 Jan 2022 18:22:46.6926 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d X-MS-Exchange-CrossTenant-MailboxType: HOSTED X-MS-Exchange-CrossTenant-UserPrincipalName: Dyvuksg1XpJL2+TatJky1lu0kwFykKTkqe1K6r6aS3O/jt8IH9lEHh+JKXg0Siq17SNPTOnuZGqML/Am2aW/cQ== X-MS-Exchange-Transport-CrossTenantHeadersStamped: SA1PR11MB5780 X-OriginatorOrg: intel.com X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org On 11/26/2021 4:15 AM, Kumara Parameshwaran wrote: > From: Kumara Parameshwaran > > When a tap device is hotplugged to primary process which in turn > adds the device to all secondary process, the secondary process > does a tap_mp_attach_queues, but the fds are not populated in > the primary during the probe they are populated during the queue_setup, > added a fix to sync the queues during rte_eth_dev_start > Hi Kumara, Original intention seems first running primary application, later secondary, so yes it doesn't looks like covering the hotplug case. I have a few questions below, can you please check? > Signed-off-by: Kumara Parameshwaran > --- > drivers/net/tap/rte_eth_tap.c | 80 +++++++++++++++++++++++++++++++++++ > 1 file changed, 80 insertions(+) > > diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c > index f1b48cae82..8e7915656b 100644 > --- a/drivers/net/tap/rte_eth_tap.c > +++ b/drivers/net/tap/rte_eth_tap.c > @@ -67,6 +67,7 @@ > > /* IPC key for queue fds sync */ > #define TAP_MP_KEY "tap_mp_sync_queues" > +#define TAP_MP_REQ_START_RXTX "tap_mp_req_start_rxtx" > > #define TAP_IOV_DEFAULT_MAX 1024 > > @@ -880,11 +881,51 @@ tap_link_set_up(struct rte_eth_dev *dev) > return tap_ioctl(pmd, SIOCSIFFLAGS, &ifr, 1, LOCAL_AND_REMOTE); > } > > +static int tap_mp_req_on_rxtx(struct rte_eth_dev *dev) > +{ > + struct rte_mp_msg msg; > + struct ipc_queues *request_param = (struct ipc_queues *)msg.param; > + int err; > + int fd_iterator = 0; > + struct pmd_process_private *process_private = dev->process_private; > + int i; > + > + memset(&msg, 0, sizeof(msg)); > + strcpy(msg.name, TAP_MP_REQ_START_RXTX); > + strlcpy(request_param->port_name, dev->data->name, > + sizeof(request_param->port_name)); Why one use 'strcpy' and other 'strlcpy', can you please unify both to 'strlcpy'? > + msg.len_param = sizeof(*request_param); > + for (i = 0; i < dev->data->nb_tx_queues; i++) { > + msg.fds[fd_iterator++] = process_private->txq_fds[i]; > + msg.num_fds++; > + request_param->txq_count++; > + } > + for (i = 0; i < dev->data->nb_rx_queues; i++) { > + msg.fds[fd_iterator++] = process_private->rxq_fds[i]; > + msg.num_fds++; > + request_param->rxq_count++; > + } > + > + RTE_ASSERT(request_param->txq_count == dev->data->nb_tx_queues); > + RTE_ASSERT(request_param->rxq_count == dev->data->nb_rx_queues); > + Are these assertions really useful? Asserts are good to verify the external assumptions, but for this case the values are all related to above logic already. > + err = rte_mp_sendmsg(&msg); > + if (err < 0) { > + TAP_LOG(ERR, "Failed to send start req to secondary %d", > + rte_errno); > + return -1; > + } > + > + return 0; > +} > + > static int > tap_dev_start(struct rte_eth_dev *dev) > { > int err, i; > > + tap_mp_req_on_rxtx(dev); > + As for as I understand your logic is primary sends the message to the secondar(y|ies), so what happens first secondary is started? What about secondary sends the message when they are started? Also above functions is called by both primary and secondary, what happens when it is called by secondary? And the logic is not clear, it can be good to add a process type check to clarify. > err = tap_intr_handle_set(dev, 1); > if (err) > return err; > @@ -901,6 +942,39 @@ tap_dev_start(struct rte_eth_dev *dev) > return err; > } > > +static int > +tap_mp_req_start_rxtx(const struct rte_mp_msg *request, __rte_unused const void *peer) > +{ > + struct rte_eth_dev *dev; > + int ret; > + uint16_t port_id; > + const struct ipc_queues *request_param = > + (const struct ipc_queues *)request->param; > + int fd_iterator; > + int queue; > + struct pmd_process_private *process_private; > + > + ret = rte_eth_dev_get_port_by_name(request_param->port_name, &port_id); > + if (ret) { > + TAP_LOG(ERR, "Failed to get port id for %s", > + request_param->port_name); > + return -1; > + } > + dev = &rte_eth_devices[port_id];> + process_private = dev->process_private; > + dev->data->nb_rx_queues = request_param->rxq_count; > + dev->data->nb_tx_queues = request_param->txq_count; > + fd_iterator = 0; > + RTE_LOG(DEBUG, PMD, "tap_attach rx_q:%d tx_q:%d\n", request_param->rxq_count, > + request_param->txq_count); > + for (queue = 0; queue < request_param->txq_count; queue++) > + process_private->txq_fds[queue] = request->fds[fd_iterator++]; > + for (queue = 0; queue < request_param->rxq_count; queue++) > + process_private->rxq_fds[queue] = request->fds[fd_iterator++]; > + > + return 0; > +} > + > /* This function gets called when the current port gets stopped. > */ > static int > @@ -2445,6 +2519,12 @@ rte_pmd_tap_probe(struct rte_vdev_device *dev) > ret = tap_mp_attach_queues(name, eth_dev); > if (ret != 0) > return -1; > + > + ret = rte_mp_action_register(TAP_MP_REQ_START_RXTX, tap_mp_req_start_rxtx); > + if (ret < 0 && rte_errno != ENOTSUP) > + TAP_LOG(ERR, "tap: Failed to register IPC callback: %s", > + strerror(rte_errno)); > + While this update fds in start logic is added, do we still need the 'tap_mp_attach_queues()'? Can't we remove the 'tap_mp_sync_queues()' after this change? And need to unregister the mp_action, possibly in the 'tap_dev_close()'?