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 B7607A00C4; Thu, 6 Oct 2022 16:23:00 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 5E30842C67; Thu, 6 Oct 2022 16:23:00 +0200 (CEST) Received: from NAM02-DM3-obe.outbound.protection.outlook.com (mail-dm3nam02on2061.outbound.protection.outlook.com [40.107.95.61]) by mails.dpdk.org (Postfix) with ESMTP id A915D42C66 for ; Thu, 6 Oct 2022 16:22:58 +0200 (CEST) ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=lvmecB89KEwvgVIcGxrJpDpQit+WN3nYKyEK/ty3P/O79KnYA6Yye98ADE4yor5RamLUPuOnLmbJYqW2Lh+8TjX0TADo4K6t+1c5y7BH+jfKVvFMEuKSOMKz12akKmNk/1CTteDk29583fdggRS/zNYtHuHmRZ8qXMV4ipOjf7wrk22opVuYLWM0RxjpCFyNSnpGj3lqhWbbqq4xcNRq/SnmGHHrQxcn8+A7U1xOPLGPFd9Xs7UiKQDcaQIEk6WveKLmHQUNfEvgZpK3O7tvV9MHXth1SKnB51dtAxG3Zvu6AuYQVVBDobuWgADnzmNi+1DfydBlr4OrVev180mCvA== 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=QiJ58b1HQXX6/6V8UVIJfwyi/2HI7OLd1AUOeB8oQCs=; b=BEiz1IVi4lO00duG94n0MVOS3AyXUW1SIamDqFVwEL4ta1gGQMZ4G82kzUOKsxg5gcjnttZrbUgCHKzVfMTNBckd8IenAHHU635n/h8ah6YOR0vD86m1vMgboqufMv+gQdu2Pcv5OUElExhGiKngkMDUghUE2f2LPxpeqzSsbScS14SEyIxG8VTXtUPeySPecGUzZJV3WQrimOJZtme8D+7ZlpifRQf/uO2txNdi1CufOWT7K6Snz2Xco/gYBBSQZgKaLIvsSClyx9i0Mc9p8jPtGFDKsrFkqe6L7SkhZejitpqurRlu5/WDPiMdtdwe2C3XMffBCz/C/At6Jcjs5A== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=amd.com; dmarc=pass action=none header.from=amd.com; dkim=pass header.d=amd.com; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=amd.com; s=selector1; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=QiJ58b1HQXX6/6V8UVIJfwyi/2HI7OLd1AUOeB8oQCs=; b=KfWmfJ5YpaKmjicKZTX4YSF4orO/GIWgFPpSsIhkU0aEA8e8fVuDRq49cEMw9TRj+90Vwr+6Ou6jRcq5+fSYY3VsifPCUyh8ZgEEV1TVOMhMlSQk6e53eDDf2SH6744EGkPkeYTSJumzeKIBuJyH5eA2v+G171SqFDEyO4W+c1o= Authentication-Results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=amd.com; Received: from CH2PR12MB4294.namprd12.prod.outlook.com (2603:10b6:610:a9::11) by SJ0PR12MB6736.namprd12.prod.outlook.com (2603:10b6:a03:47a::18) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.5676.31; Thu, 6 Oct 2022 14:22:56 +0000 Received: from CH2PR12MB4294.namprd12.prod.outlook.com ([fe80::d07a:463f:6f93:337f]) by CH2PR12MB4294.namprd12.prod.outlook.com ([fe80::d07a:463f:6f93:337f%7]) with mapi id 15.20.5676.036; Thu, 6 Oct 2022 14:22:56 +0000 Message-ID: Date: Thu, 6 Oct 2022 15:22:51 +0100 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101 Thunderbird/102.3.1 Subject: Re: [PATCH v4 3/9] net/gve: add support for device initialization Content-Language: en-US To: Junfeng Guo , qi.z.zhang@intel.com, jingjing.wu@intel.com Cc: dev@dpdk.org, xiaoyun.li@intel.com, awogbemila@google.com, bruce.richardson@intel.com, xueqin.lin@intel.com, Haiyue Wang References: <20220923093829.3019525-2-junfeng.guo@intel.com> <20220927073255.1803892-1-junfeng.guo@intel.com> <20220927073255.1803892-4-junfeng.guo@intel.com> From: Ferruh Yigit In-Reply-To: <20220927073255.1803892-4-junfeng.guo@intel.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-ClientProxiedBy: LO4P265CA0034.GBRP265.PROD.OUTLOOK.COM (2603:10a6:600:2ae::8) To CH2PR12MB4294.namprd12.prod.outlook.com (2603:10b6:610:a9::11) MIME-Version: 1.0 X-MS-PublicTrafficType: Email X-MS-TrafficTypeDiagnostic: CH2PR12MB4294:EE_|SJ0PR12MB6736:EE_ X-MS-Office365-Filtering-Correlation-Id: f37bb927-5e01-451a-e266-08daa7a64367 X-MS-Exchange-SenderADCheck: 1 X-MS-Exchange-AntiSpam-Relay: 0 X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: TKST5OjNI4BZoPWDFpGf/+b2q3fWsRngqyoro8M63pUc35LK+7UD+aPTLLKXbpsYvt0FCnPhhjlEa8XvhbzXpVYsN8vLvsmkFar0KIgPwu5zh3kZb2QKCKvKvmZI2vt3s7BGokc7XbN6TmeTDv99fqoTmUF6enlzMDyPv4SwqdQl2G5hg92+AJJVkyjf7va2dN5cJ8rBRWftqRgGtZu425rIuQvcbhIG9LG9DKmwQp8d+ctxNh51jc6GjHcJf9HrEOIGh4mxtRoZEkX7+D8gi/GdvA7vydSPZFPUPrnGMcWwbZjvFo2bucnLOd/1qQMG+Ms9j6TNRouk52BvJdcYvWccta0TFQOj7KqNFKwesfto0Krf2O0phVxLM9ZQoL4j5GLRtpDyUptvmzST5HMkSsT7G9/G0m5QxFR5lQ5e4fJ7g2Elq4URJI3QIov3QXRa2ehm3UjnqrGWpPAvBdsPvvMdfa/cemS5cBQ97RpeYIKVeUFRdXTh3R4ahZWGfnvB1VHFzRA4UhOD8Em1hHPLsUB8vSYmnBJPYNmdTMv/4pq9MH963bI6iDf8EqT+LTCwGaTXuGtGa9owwo6/RraOxYzaeqdR0BpJelM/vLDJNhdDFs/AkUihnRzhFdRB2pj4s1AwVppLhdSrVPo9BtD12NwCgzaDCvev4TZx0d5pS3YEEjYtKRVVR2SCiVWnoFyGXTyMpROV1FapcQMolRzTTYZi6GBOnFsLl78d6Wvf8Oi/K2XHvISwFwRXt1VoRoIw6vqlvqyoWIjWNXY3XUrZDNskS91JTJf+57WBCnOilcf6FYk6QPFL5960UJws1zNXIE6arl6Q2FLwoF7x96ANrmTAXp8o1helRoY//wRHk10= X-Forefront-Antispam-Report: CIP:255.255.255.255; CTRY:; LANG:en; SCL:1; SRV:; IPV:NLI; SFV:NSPM; H:CH2PR12MB4294.namprd12.prod.outlook.com; PTR:; CAT:NONE; SFS:(13230022)(4636009)(39860400002)(376002)(396003)(136003)(366004)(346002)(451199015)(316002)(478600001)(26005)(4326008)(6666004)(53546011)(8936002)(44832011)(6512007)(8676002)(36756003)(41300700001)(5660300002)(6506007)(186003)(2616005)(2906002)(66946007)(66476007)(66556008)(31686004)(83380400001)(86362001)(31696002)(6486002)(966005)(38100700002)(45980500001)(43740500002); DIR:OUT; SFP:1101; X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1 X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?anZjclJaSUk3czJ4THRhOWZ4REM1Tk9Bbk5DVmpzWmdyOUFnTE5tWjdyenU3?= =?utf-8?B?UTB2VEh3WVF5K2RyRlRPVDVZWGdtZkRNUC93M3hjQURyQjYxdUN2bEpqZEl0?= =?utf-8?B?ZHB2Z1RVZk1BUHN4WUg4aEo3ZVBZK1J4K3ZMaWU3aStEbC9YTHdodm1RZStO?= =?utf-8?B?VGZtVURjOU9kTllBUlIvc2trdzBCRTB5U3pwVkRJKzdiVWltSFNVZU5DUVR4?= =?utf-8?B?UEsxL1VMZlJoQkVIaThmK1RDb2xNdWZleS92RCtEVloxbzMwWHFXaGFtWGJQ?= =?utf-8?B?MVNnZTNUMkpvME5tSU5kK0JvT08vd0UyaCtDUElSWFpxdldydkd4UUJMN1d2?= =?utf-8?B?bnowNW1MakxtWXAxbEVHL0x6N0F2NjlkN3JlbkorcS9FYWxmOXBsQTZkSFIr?= =?utf-8?B?cVUyYXhLcXlXUTc1Sk5XMFJqRjJtZ1pNcWlwMXgyOVB3Q09vOGY1NHRLdFlE?= =?utf-8?B?cXRWa21xKzQvODJ0MlV2dUJ6M2kwMlI1N2xUQjE3cktGOTRQa3NxWjFtS2Ny?= =?utf-8?B?MFF3UDRnbVlVUkpJUXE0U0hhalFvZDRZdk1jbUYrOFJqUlpoc2tkdUZydEVw?= =?utf-8?B?aElQSGtGaHRxQVBNbnVuaDkzUjZ0dW1JYm9NRU9udjFERXUvOHlTTXZRMEVQ?= =?utf-8?B?akNFL3dUc1dzOGlFUDMzRUpscEJxcXBweDlYNzdPREZEOXVWSG9MUUphR0Qr?= =?utf-8?B?WnVMRFhZRFgvYjUzMTdhSFlRYXNCbTBpWS9TZVRZNE83SjQveGpmMEJVVjN1?= =?utf-8?B?Y2o5ZHpaV1pndjdHZ05sb0FWSXFySklXN3g2OVF6OS9FT1cwTG9Banhsc290?= =?utf-8?B?bWoyVTNYaG41M0xVMm42LzNHcDR3Zm1hRllraUx3V2lmNXRQeHFlUkY2RUJV?= =?utf-8?B?S0w5eS9wK1ptNHQzOGk4eTZnYlVKNDdQYkZmSjZaM3hYbFcwcjhEdmdmQmgz?= =?utf-8?B?dGVmN0ExU2c1QXhaN0xDRjFzaG1STkpWWG0weTloTEdpdkZ4ZEdEU1F6RXpU?= =?utf-8?B?ZW1DSkhNYXIycW1pRnZKeGRwWm1qeHNvVWZCOUtYM0FIaFFZWDlhYmZKYlVw?= =?utf-8?B?U3RyUGtWQitnb3B5WUEzRkE2SWxRbWYvV2EvTlBIVW5xb3REKys0Y1JMb0xx?= =?utf-8?B?SzdKMUpNQ2NjaGdwQ2VYM05xUTdvNDA0M0ZudDFlWUt5WjVwaEZFQ3dKazRj?= =?utf-8?B?OElSOSttQ2lqOU45RklxZ1hiNXZZVG5wNmRycHNWZUhESVY5V0MzMGZZb2tk?= =?utf-8?B?WUI2dVdhay9LU0VzWU40ay9jeDZod3NMaXdZaERuQkJqKzJqYS8vVk9hNHVC?= =?utf-8?B?T1dOR0Y5Y0xEQ1A3N3h2OHlQaUFwNHdxbGRya0hRK3UybzJyUnowalpsOU5J?= =?utf-8?B?TU9pNDAyNndTWmVQaS83UjRLTjZReENYSVA3UWFndHpxVFMxYUVnQ2I0SURO?= =?utf-8?B?VXY5eVc4UjMvUVBwWmRZL1FFUlQ1WG51MG92YkZrQTh1UDNzd3cybmJIV3ph?= =?utf-8?B?NWJzQ3dBK1EydHBZRjIxL3RWeEx3NFBnTzVsdEtEZjBVR2NVTXZKNmgyRVBj?= =?utf-8?B?L1VhKzh4ZTRqTnlPMm9WbGFCdmZLdDFzaktIZFhKMmVCVWxEZ3NHUnBoOEwr?= =?utf-8?B?NGdMajVJY3ROWlo3bkViN2kzN1V3Nms4ek4wVnJTRytWNlpGMWpZajlXMjda?= =?utf-8?B?WURIdGtYMXVHT3VIanhiTGRJVU1LNEt5eXBRV1NNUlA4eVRuQlR5d1lnOXlV?= =?utf-8?B?bnlwL0djQ3YycTB3YU5nMjZvU1ZRTmVaN0FoU0hjVG9nTERVMHNQNytMYnYw?= =?utf-8?B?UWFhdlE3ZURZSFdoc2V2eVZGaE1nRTYvV3g4a1g5M3dUY0t2WW0wTHNtOWZp?= =?utf-8?B?LzB5Y1htQVE2cWtLc1RqdmdKQUlHS0c1VjgydmppTzFQRkRtWm1BWGloa0Nk?= =?utf-8?B?alVXNi9HQ2JkeUxPUVNYWjhtVHg1UVNUbkRrOHMvQjh6YlgydEVXdm1senZx?= =?utf-8?B?TU52QlFidXhDL2thSkxXdUd0RVQ0S2pIRWh1RzY3NWNqVldsNk16Nmo4SC9E?= =?utf-8?B?QzBTL3Vza1dqRFo4L1R6SFh6WjhnR2NwMVVXa3FqWTlrTmxvUFNrem1DbVZG?= =?utf-8?Q?vPa+/tPoNhhu91I8yejjtcyaq?= X-OriginatorOrg: amd.com X-MS-Exchange-CrossTenant-Network-Message-Id: f37bb927-5e01-451a-e266-08daa7a64367 X-MS-Exchange-CrossTenant-AuthSource: CH2PR12MB4294.namprd12.prod.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-OriginalArrivalTime: 06 Oct 2022 14:22:56.5824 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-CrossTenant-Id: 3dd8961f-e488-4e60-8e11-a82d994e183d X-MS-Exchange-CrossTenant-MailboxType: HOSTED X-MS-Exchange-CrossTenant-UserPrincipalName: yjkMTmL5Nf02vzrCscayOCXeqKXgQUEClMPOnfeF3nc4oSpQi+TC1k9XoDBneTN3 X-MS-Exchange-Transport-CrossTenantHeadersStamped: SJ0PR12MB6736 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 9/27/2022 8:32 AM, Junfeng Guo wrote: > > Support device init and the fowllowing devops: s/fowllowing/following/ > - dev_configure > - dev_start > - dev_stop > - dev_close At this stage most of above are empty functions and not implemented yet, instead can you document in the commit log that build system and device initialization is added? > > Signed-off-by: Haiyue Wang > Signed-off-by: Xiaoyun Li > Signed-off-by: Junfeng Guo <...> > --- /dev/null > +++ b/doc/guides/nics/gve.rst > @@ -0,0 +1,69 @@ > +.. SPDX-License-Identifier: BSD-3-Clause > + Copyright(C) 2022 Intel Corporation. > + > +GVE poll mode driver > +======================= > + > +The GVE PMD (**librte_net_gve**) provides poll mode driver support for > +Google Virtual Ethernet device. > + > +Please refer to https://cloud.google.com/compute/docs/networking/using-gvnic > +for the device description. > + This seems another virtual interface, similar to iavf/virtio/idpf ... Can you please briefly describe here the motivation to add yet another virtual interface, and again briefly describe cons/pros of the interface? > +The base code is under MIT license and based on GVE kernel driver v1.3.0. > +GVE base code files are: > + > +- gve_adminq.h > +- gve_adminq.c > +- gve_desc.h > +- gve_desc_dqo.h > +- gve_register.h > +- gve.h > + > +Please refer to https://github.com/GoogleCloudPlatform/compute-virtual-ethernet-linux/tree/v1.3.0/google/gve > +to find the original base code. > + > +GVE has 3 queue formats: > + > +- GQI_QPL - GQI with queue page list > +- GQI_RDA - GQI with raw DMA addressing > +- DQO_RDA - DQO with raw DMA addressing > + > +GQI_QPL queue format is queue page list mode. Driver needs to allocate > +memory and register this memory as a Queue Page List (QPL) in hardware > +(Google Hypervisor/GVE Backend) first. Each queue has its own QPL. > +Then Tx needs to copy packets to QPL memory and put this packet's offset > +in the QPL memory into hardware descriptors so that hardware can get the > +packets data. And Rx needs to read descriptors of offset in QPL to get > +QPL address and copy packets from the address to get real packets data. > + > +GQI_RDA queue format works like usual NICs that driver can put packets' > +physical address into hardware descriptors. > + > +DQO_RDA queue format has submission and completion queue pair for each > +Tx/Rx queue. And similar as GQI_RDA, driver can put packets' physical > +address into hardware descriptors. > + > +Please refer to https://www.kernel.org/doc/html/latest/networking/device_drivers/ethernet/google/gve.html > +to get more information about GVE queue formats. > + > +Features and Limitations > +------------------------ > + > +In this release, the GVE PMD provides the basic functionality of packet > +reception and transmission. > +Supported features of the GVE PMD are: > + > +- Multiple queues for TX and RX > +- Receiver Side Scaling (RSS) > +- TSO offload > +- Port hardware statistics > +- Link state information > +- TX multi-segments (Scatter TX) > +- Tx UDP/TCP/SCTP Checksum > + Can you build this list gradully, by adding relavent item in each patch that adds it? That way mapping with the code and documented feature becomes more obvious. <...> > +static int > +gve_dev_uninit(struct rte_eth_dev *eth_dev) > +{ > + struct gve_priv *priv = eth_dev->data->dev_private; > + > + eth_dev->data->mac_addrs = NULL; > + At this stage 'mac_addrs' is not freed, setting it to NULL prevents it to be freed. <...> > + > +static struct rte_pci_driver rte_gve_pmd = { > + .id_table = pci_id_gve_map, > + .drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_INTR_LSC, As far as I can see LSC interrupt is not supported, if that is correct should we drop the flag? <...> > + > +struct gve_priv { > + struct gve_irq_db *irq_dbs; /* array of num_ntfy_blks */ > + const struct rte_memzone *irq_dbs_mz; > + uint32_t mgmt_msix_idx; > + rte_be32_t *cnt_array; /* array of num_event_counters */ > + const struct rte_memzone *cnt_array_mz; > + > + uint16_t num_event_counters; > + uint16_t tx_desc_cnt; /* txq size */ > + uint16_t rx_desc_cnt; /* rxq size */ > + uint16_t tx_pages_per_qpl; /* tx buffer length */ > + uint16_t rx_data_slot_cnt; /* rx buffer length */ > + > + /* Only valid for DQO_RDA queue format */ > + uint16_t tx_compq_size; /* tx completion queue size */ > + uint16_t rx_bufq_size; /* rx buff queue size */ > + > + uint64_t max_registered_pages; > + uint64_t num_registered_pages; /* num pages registered with NIC */ > + uint16_t default_num_queues; /* default num queues to set up */ > + enum gve_queue_format queue_format; /* see enum gve_queue_format */ > + uint8_t enable_rsc; > + > + uint16_t max_nb_txq; > + uint16_t max_nb_rxq; > + uint32_t num_ntfy_blks; /* spilt between TX and RX so must be even */ > + > + struct gve_registers __iomem *reg_bar0; /* see gve_register.h */ > + rte_be32_t __iomem *db_bar2; /* "array" of doorbells */ > + struct rte_pci_device *pci_dev; > + > + /* Admin queue - see gve_adminq.h*/ > + union gve_adminq_command *adminq; > + struct gve_dma_mem adminq_dma_mem; > + uint32_t adminq_mask; /* masks prod_cnt to adminq size */ > + uint32_t adminq_prod_cnt; /* free-running count of AQ cmds executed */ > + uint32_t adminq_cmd_fail; /* free-running count of AQ cmds failed */ > + uint32_t adminq_timeouts; /* free-running count of AQ cmds timeouts */ > + /* free-running count of per AQ cmd executed */ > + uint32_t adminq_describe_device_cnt; > + uint32_t adminq_cfg_device_resources_cnt; > + uint32_t adminq_register_page_list_cnt; > + uint32_t adminq_unregister_page_list_cnt; > + uint32_t adminq_create_tx_queue_cnt; > + uint32_t adminq_create_rx_queue_cnt; > + uint32_t adminq_destroy_tx_queue_cnt; > + uint32_t adminq_destroy_rx_queue_cnt; > + uint32_t adminq_dcfg_device_resources_cnt; > + uint32_t adminq_set_driver_parameter_cnt; > + uint32_t adminq_report_stats_cnt; > + uint32_t adminq_report_link_speed_cnt; > + uint32_t adminq_get_ptype_map_cnt; > + > + volatile uint32_t state_flags; > + > + /* Gvnic device link speed from hypervisor. */ > + uint64_t link_speed; > + > + uint16_t max_mtu; > + struct rte_ether_addr dev_addr; /* mac address */ > + > + struct gve_queue_page_list *qpl; > + > + struct gve_tx_queue **txqs; > + struct gve_rx_queue **rxqs; > +}; > + Similar to previous comment, can you construct the headers by only adding used fields in that patch? When batch copied an existing struct, it is very easy to add unused code and very hard to detect it. So if you only add what you need, that becomes easy to be sure all fields are used. Also it makes more obvious which fields related to which feature. <...> > new file mode 100644 > index 0000000000..c2e0723b4c > --- /dev/null > +++ b/drivers/net/gve/version.map > @@ -0,0 +1,3 @@ > +DPDK_22 { > + local: *; > +}; it is 'DPDK_23' now, hopefully we will have an update to get rid of empty map files, feel free to review: https://patches.dpdk.org/project/dpdk/list/?series=25002