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 CDDAAA034C; Wed, 21 Dec 2022 16:37:14 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id AF24740A7F; Wed, 21 Dec 2022 16:37:14 +0100 (CET) Received: from NAM11-DM6-obe.outbound.protection.outlook.com (mail-dm6nam11on2058.outbound.protection.outlook.com [40.107.223.58]) by mails.dpdk.org (Postfix) with ESMTP id 77D5D40A7A for ; Wed, 21 Dec 2022 16:37:13 +0100 (CET) ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=TTxljx5pcZzggbWmNPIwyz3cDfzI69luuS/2N4G+YBtIRIScl3fxBONk+x+7uX2/Fj5vSooHdqfrndeEk2N4hi+o109Y9T5y3L4eNgSUOtNLZTVMBTI96sVN40WaoH8CJgP7o3yaIFiP7ddvjczZFP6C92nWe+nqPpsY9mpDTGzfGrECP1OnRhTFG64i9qU38UMTi/9ha0YRPXbZndtmyFU+bXpKd2dSw6HU8TO0XvYXEFNIGvzuACJGL4FnDzw2PglrhRgBkn4hiODRdHkm7IkA84xb2bbnuAKTs495fP++8cqgi9LDBN//bbBaQb5JIbiLrIszm4kFmQwHObvk1A== 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=V0wkGwwjtIuK8MMMo1CeCw1Qjz8HuQSNVxzCGB/F0I0=; b=k/uFELSDRXOUTAdhEN4TDPXQb9GuG57adr+Tk3AHk1UtAnOExF+nHcE7PCBcvbpsyPpX7DEhu8qciiIlM2loUagjYmgeEZg7kakLY2WdQTt2/40OBqO5nWmqhFyCuLBoQzroXS4I+dJ+gPm4BWC2j76HnM+VRhImhqVngXqw29AEhZiIBoCDszz2gyT300DPR2qcnEj4QAG8uLM4+FiLg5w6re9XSUJhOCfAjObFoQmvtTg57sy4M98RtWp7z9S2cJ01r7bg2LihVmlgujxHnjiav1P41DjVQRZ9xjy/yXVEujKAu7km6JWOOC6XfwbsG8Y3PDuUu8jFbWLSJ6GXTw== 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=V0wkGwwjtIuK8MMMo1CeCw1Qjz8HuQSNVxzCGB/F0I0=; b=bjhyoT+7hjQrxm0UMZ9UK9eAetq+bRhAhuLsLO5YjjkVFfT+FF5SHAn2Ih32PGpFYjKgZYflMgGPHI+Q38FUVl7NjpaVhG7ve7t60VI/m0XVc5ID5Xmt/PWlYsd7ZV3UVH8v8Agmj0Un0aF0nQVV0xUDfdDsZVPWFrICUbA0IOk= 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 DM6PR12MB4482.namprd12.prod.outlook.com (2603:10b6:5:2a8::23) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.5924.16; Wed, 21 Dec 2022 15:37:10 +0000 Received: from CH2PR12MB4294.namprd12.prod.outlook.com ([fe80::4807:1f44:5e04:e05a]) by CH2PR12MB4294.namprd12.prod.outlook.com ([fe80::4807:1f44:5e04:e05a%8]) with mapi id 15.20.5924.016; Wed, 21 Dec 2022 15:37:10 +0000 Message-ID: <01a50e41-6433-a855-25ea-50b94071411d@amd.com> Date: Wed, 21 Dec 2022 15:37:05 +0000 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101 Thunderbird/102.6.0 Content-Language: en-US To: Jesna K E , dev@dpdk.org Cc: Selwin.Sebastian@amd.com References: <20221221025202.31733-1-jesna.k.e@amd.com> From: Ferruh Yigit Subject: Re: [PATCH v1 1/2] net/axgbe: add multi-process support In-Reply-To: <20221221025202.31733-1-jesna.k.e@amd.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-ClientProxiedBy: LO4P123CA0108.GBRP123.PROD.OUTLOOK.COM (2603:10a6:600:191::23) To CH2PR12MB4294.namprd12.prod.outlook.com (2603:10b6:610:a9::11) MIME-Version: 1.0 X-MS-PublicTrafficType: Email X-MS-TrafficTypeDiagnostic: CH2PR12MB4294:EE_|DM6PR12MB4482:EE_ X-MS-Office365-Filtering-Correlation-Id: c6171142-8fcc-4443-5d2b-08dae3693959 X-MS-Exchange-SenderADCheck: 1 X-MS-Exchange-AntiSpam-Relay: 0 X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: Pxvxrl2GLFiCJBIgAGQJsbBON5oMD9aA/XYE8CYhjl5PN5fTIRuG9AaJweiWcX4eSqcnNqK1iQUPBeFv/fVThKopQm6SwO6VjTdGL0S+FJmz6yJA9w+BxxjxnddWhx3NonLWhevzzkvWN7aKwkycFBDeHikiEjfOMdu9cyu5rEg+nhgI2RBHjV//FInerOwAGzdJC7ekpzclzxMu1ub+3EMSImr2I9ZUt9m94d8AZh5UQ/jQs/+CJlp9OAM2iTa5JHldcAo2x2LFeLo3taKTbSG3OyX0yIaJucJO1imJq+LYrgEQQkKOZ49OB+6AHfXOeiqAd8cLV+lRVxdbVgchVzCr00Ju1dOGiys0DK1aevYixk3LDL10R+Acsb+i/q4Vsd+2f3/BxHzb9szLYkyJHCOBJSKIEmh8wT+9d2U5DelHK30eA5OXB5MqKcajLfchfjyvmuG/Efv6MVklX6Y9lHZReSqUcXCOlFfHwwZpveGTEN2+FYZcQ7M4rTRAi5yz+VmuoPDFg6SY2HOTiR3WISlfLdUELDU9ALQ2ZsiW+VE5CN5ZunfSdZ9NAybgEtjgCio7LcQduUYhkuQbRoox3f6aRsP346guSKyof8kfAlbVDVtZD092wlmHvrQGAA67SUoK2sK5kvez6hochsMIQSlCNR0+EjAzhw/vuU2KLxL4RNV+pyz6Fg/5ZaZKI4TJOCb2DsVe7TjkLpDeLwL+o8f2ysUyD0zoAxy9lINYopc= 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)(136003)(366004)(346002)(376002)(396003)(39860400002)(451199015)(44832011)(38100700002)(6486002)(478600001)(31686004)(2906002)(5660300002)(316002)(8936002)(86362001)(36756003)(31696002)(4326008)(66946007)(66556008)(66476007)(8676002)(26005)(83380400001)(2616005)(53546011)(186003)(41300700001)(6506007)(6666004)(6512007)(45980500001)(43740500002); DIR:OUT; SFP:1101; X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1 X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?N2RzOTBQRjFLcVpjTnU2enZmcW10cWx3Sm9vZ0FKK1NRbXlaK2gwQkg0ZUVk?= =?utf-8?B?dXpkdGFqNkdqVDlpR1RrQW0zUFhDSnMwVW5BTzJnY1ZGMHp5b0gycWRSTERG?= =?utf-8?B?YTU4dzl5b0RhWEFEVENPUFdETmR0cmIyWFBnS2JJcmhiNnptbXROaVlzamF5?= =?utf-8?B?MSt4Ukd1SUIyWmVocmJwYjhyRUxZRW9MZ2d0Zm95Z2hoZm5kN1FNMmJLNktI?= =?utf-8?B?UGZoYzdWY25uVkJTOHJVa3gxWVZVSXMzY0NZVk5HZE1MUnhyYmdGU2FQdE1N?= =?utf-8?B?WEMvRzZkbm1OZHcvUG9nSWhhck5VOWRvdng1eG1iOWdnR3NmNmZBelNQZi9u?= =?utf-8?B?bmt2T0duM3YzWkdQckc0Zy9rdlhuZmlaSGthQnhqZmtkYjc1cmNqTTBXUDJz?= =?utf-8?B?U2hUUUZGVUc5ZE1IUEVpdUpROWFJYVNrUGhvaDRVZ09LVUphb0o3ZmJBaFlF?= =?utf-8?B?MDhVZ21nSnRMdWxTL2hyaDgzSDNpVTN3UHp5MjJXd3FPQjdtbGRpSk92OVB3?= =?utf-8?B?bFFmUFV5SExGc3BVdXdXZ0VOK2xzLzdUcWtuV2Jlc3I1QWVrUGIrWHJrMDlM?= =?utf-8?B?VWRiR092SHFqV1k0QkQxU2xpdTZ6eGZqOWVRdzNsMUU2cDYzL2U4Wi93RkFu?= =?utf-8?B?RStuS2g2UVJIRzZyN2pZWlZ0ckM2UmhQV1RUNzFVVnVBQjc2Q25uank1TzhW?= =?utf-8?B?bk5nVDB0cEIwSzN0WUhHbFBETHZMb3RER3owTmtVa01zaDJpOW94TUpzaVJT?= =?utf-8?B?a3U3aUg2ZzdhT1FYWVJac0lxeDMwRW5TNzh6STZjcmlKbHBFOHkxZGNVeXNu?= =?utf-8?B?SzRZTE9rUHJJTjcxMG9oMm1lVy9qcDM1YzdHZkQ1dnJBbHMxVkhsc2IrNkNa?= =?utf-8?B?MDJVNWtINnhocHNjTUx6dVJkU2luNlpxQ1dpSjJuSk12QmQvTHdObWlnRzVJ?= =?utf-8?B?L1BhSnpuTmVXLzhhZkgzbk1IdzR0UEhsOGFORmE4OExwbm1SaVNkRzNVQ1Qz?= =?utf-8?B?SVpEQjdHSTZxeFMxbW9QUDQ5V0FPcWVoS3hMeUEvUGYxWWpnaUFYWmVGcEFP?= =?utf-8?B?Q3diZkR2UmRjODcwME9nbldMVTZkeXU5TTAxU2swWFBDeXd3L3dYaWVKZSsv?= =?utf-8?B?a0UzQnpWNDNJam9LVk1WdUtYYzBnYnRaVjBuTzRtQ0M1QXNXNXBkQ2E1YUU1?= =?utf-8?B?RGFqcmIxSmhFOTdFVThtTXlqRmo0eHdSK203c0RrNUFpbG1Iazd5WVFPdXZa?= =?utf-8?B?RHVxSFV4eXFCeXpNdXl3aEhjeHJnZk5IUjBvYkdYOGJ0NWZRTlBiZUNJQW1R?= =?utf-8?B?cFVib0o2MzFCZkZuU25RbVh0YVdVV3JnYzFKT3ByNVFrcjF0QXh2am9kUGwv?= =?utf-8?B?blJGd09EcHJydDl6NzYxY2JQaUhqSWV2OXlwVTl0d2dVeGt3bkVkaVlaMjRN?= =?utf-8?B?SkRFUnVwazlDT0xPbWM0V3NKd0h4U3VGMUpLZkVPeEp6Qm90aGxxblhXS0dS?= =?utf-8?B?dkhVdTBZb1JuK3FOMFVDQ2R3SS93NTFqdEl1dkkvem5GT08vbTR2M21XdEhq?= =?utf-8?B?dXBTZWNwTkRyWHhSaVkvZll5N0QxTzNucGxscG5kdG9KSDgzejNaZFJ5cHp3?= =?utf-8?B?c3YwQUJmQlQxQXZDZEgvSkpTRFBzK3hTUkxrQTIwZ0E2VUhUTkhoTXUzdFJB?= =?utf-8?B?VysrU2g3NUtxYzU4a2JyZHVSZWFtKzlkRnNiNE54NlBEZXJlb1l4OGVQcGc0?= =?utf-8?B?S0ZoZ01QUjVVTEI3Q0MvZ3ROczNiRk52aGtQcDd3Qis3TjlzQmdIYlZPYkVS?= =?utf-8?B?UW05OER2NEF6cWVoMkQvWnlBVmMrelNWelhjQVd1QStod1FoOXZ0K05EelZa?= =?utf-8?B?VHdPTmo1UXU1dEpvc2NnSkNLZ0hCYUIwN3JYZzBMVmNyTnd1N084Mm14Nk1N?= =?utf-8?B?NkJLcFZoQkNodEpaWkkzV21SN1Z0N052WXV5RUswOURpYTVWb2lrdlczRFJP?= =?utf-8?B?ZEtUOEs1eFIrbEk2VExoQmdqdno1U2RXaVJmbVJWSDU4YWRoWDgzSi9jajVE?= =?utf-8?B?WmxXeTBHSENLcm92a2xnUmNXL3VhSEhuZG12UFdaNXV6UVNvb252WWhsS1o2?= =?utf-8?Q?IJIAqjvlAFskVuvYOb+CObdjF?= X-OriginatorOrg: amd.com X-MS-Exchange-CrossTenant-Network-Message-Id: c6171142-8fcc-4443-5d2b-08dae3693959 X-MS-Exchange-CrossTenant-AuthSource: CH2PR12MB4294.namprd12.prod.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-OriginalArrivalTime: 21 Dec 2022 15:37:10.1762 (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: i2Zqd1l790Jq9MWuCCNZoEmYdTsiBs9EoGtG8hTrHaxp56gViQLofvQkzNPr5z+B X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM6PR12MB4482 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 12/21/2022 2:52 AM, Jesna K E wrote: > Added multi-process support for axgbe PMD > To achieve multi-process support separate out TX and RX function > inside the axgbe driver and call that from a secondary process > when it is attaching to an already-configured NIC > > Signed-off-by: Jesna K E > --- > doc/guides/nics/features/axgbe.ini | 1 + > drivers/net/axgbe/axgbe_ethdev.c | 65 ++++++++++++++++++++++-------- > drivers/net/axgbe/axgbe_rxtx.c | 11 ----- > drivers/net/axgbe/axgbe_rxtx.h | 7 +++- > 4 files changed, 55 insertions(+), 29 deletions(-) > Overall looks good, do you think does it worth to document this in release notes (doc/guides/rel_notes/release_23_03.rst)? > diff --git a/doc/guides/nics/features/axgbe.ini b/doc/guides/nics/features/axgbe.ini > index 821bb682d4..5e2d6498e5 100644 > --- a/doc/guides/nics/features/axgbe.ini > +++ b/doc/guides/nics/features/axgbe.ini > @@ -18,6 +18,7 @@ L3 checksum offload = Y > L4 checksum offload = Y > Basic stats = Y > FW version = Y > +Multiprocess aware = Y > Linux = Y > x86-32 = Y > x86-64 = Y > diff --git a/drivers/net/axgbe/axgbe_ethdev.c b/drivers/net/axgbe/axgbe_ethdev.c > index b071e4e460..4b3d655bd3 100644 > --- a/drivers/net/axgbe/axgbe_ethdev.c > +++ b/drivers/net/axgbe/axgbe_ethdev.c > @@ -353,8 +353,6 @@ axgbe_dev_start(struct rte_eth_dev *dev) > { > struct axgbe_port *pdata = dev->data->dev_private; > int ret; > - struct rte_eth_dev_data *dev_data = dev->data; > - uint16_t max_pkt_len; > > dev->dev_ops = &axgbe_eth_dev_ops; > > @@ -388,17 +386,8 @@ axgbe_dev_start(struct rte_eth_dev *dev) > rte_bit_relaxed_clear32(AXGBE_STOPPED, &pdata->dev_state); > rte_bit_relaxed_clear32(AXGBE_DOWN, &pdata->dev_state); > > - max_pkt_len = dev_data->mtu + RTE_ETHER_HDR_LEN + RTE_ETHER_CRC_LEN; > - if ((dev_data->dev_conf.rxmode.offloads & RTE_ETH_RX_OFFLOAD_SCATTER) || > - max_pkt_len > pdata->rx_buf_size) > - dev_data->scattered_rx = 1; > - > - /* Scatter Rx handling */ > - if (dev_data->scattered_rx) > - dev->rx_pkt_burst = ð_axgbe_recv_scattered_pkts; > - else > - dev->rx_pkt_burst = &axgbe_recv_pkts; > - > + axgbe_set_rx_function(dev); > + axgbe_set_tx_function(dev); > return 0; > } > > @@ -2145,6 +2134,46 @@ get_pci_rc_devid(void) > return (uint16_t)device_id; > } > > +/* Takes ethdev as parameter > + * Used in dev_start by primary process and then > + * in dev_init by secondary process when attaching to an existing ethdev. > + */ > +void > +axgbe_set_tx_function(struct rte_eth_dev *dev) > +{ > + struct axgbe_port *pdata = dev->data->dev_private; > + struct axgbe_tx_queue *txq = dev->data->tx_queues[0]; > + > + if (pdata->multi_segs_tx) > + dev->tx_pkt_burst = &axgbe_xmit_pkts_seg; > +#ifdef RTE_ARCH_X86 > + if (!txq->vector_disable && > + rte_vect_get_max_simd_bitwidth() >= RTE_VECT_SIMD_128) > + dev->tx_pkt_burst = &axgbe_xmit_pkts_vec; > +#else > + dev->tx_pkt_burst = &axgbe_xmit_pkts; > +#endif > +} > + > +void > +axgbe_set_rx_function(struct rte_eth_dev *dev) > +{ > + struct rte_eth_dev_data *dev_data = dev->data; > + uint16_t max_pkt_len; > + struct axgbe_port *pdata; > + > + pdata = dev->data->dev_private; > + max_pkt_len = dev_data->mtu + RTE_ETHER_HDR_LEN + RTE_ETHER_CRC_LEN; > + if ((dev_data->dev_conf.rxmode.offloads & RTE_ETH_RX_OFFLOAD_SCATTER) || > + max_pkt_len > pdata->rx_buf_size) > + dev_data->scattered_rx = 1; > + /* Scatter Rx handling */ > + if (dev_data->scattered_rx) > + dev->rx_pkt_burst = ð_axgbe_recv_scattered_pkts; > + else > + dev->rx_pkt_burst = &axgbe_recv_pkts; > +} > + > /* > * It returns 0 on success. > */ > @@ -2159,17 +2188,20 @@ eth_axgbe_dev_init(struct rte_eth_dev *eth_dev) > int ret; > > eth_dev->dev_ops = &axgbe_eth_dev_ops; > - > eth_dev->rx_descriptor_status = axgbe_dev_rx_descriptor_status; > eth_dev->tx_descriptor_status = axgbe_dev_tx_descriptor_status; > > + eth_dev->tx_pkt_burst = &axgbe_xmit_pkts; > + eth_dev->rx_pkt_burst = &axgbe_recv_pkts; > /* > * For secondary processes, we don't initialise any further as primary > * has already done this work. > */ > - if (rte_eal_process_type() != RTE_PROC_PRIMARY) > + if (rte_eal_process_type() != RTE_PROC_PRIMARY) { > + axgbe_set_tx_function(eth_dev); > + axgbe_set_rx_function(eth_dev); > return 0; > - What do you think to keep empty lines before and after secondary process related block to group it? > + } > eth_dev->data->dev_flags |= RTE_ETH_DEV_AUTOFILL_QUEUE_XSTATS; > > pdata = eth_dev->data->dev_private; > @@ -2177,7 +2209,6 @@ eth_axgbe_dev_init(struct rte_eth_dev *eth_dev) > rte_bit_relaxed_set32(AXGBE_DOWN, &pdata->dev_state); > rte_bit_relaxed_set32(AXGBE_STOPPED, &pdata->dev_state); > pdata->eth_dev = eth_dev; > - Please remove noise in the patch > pci_dev = RTE_DEV_TO_PCI(eth_dev->device); > pdata->pci_dev = pci_dev; > > diff --git a/drivers/net/axgbe/axgbe_rxtx.c b/drivers/net/axgbe/axgbe_rxtx.c > index 7cff79e030..9b283bd9d0 100644 > --- a/drivers/net/axgbe/axgbe_rxtx.c > +++ b/drivers/net/axgbe/axgbe_rxtx.c > @@ -629,17 +629,6 @@ int axgbe_dev_tx_queue_setup(struct rte_eth_dev *dev, uint16_t queue_idx, > RTE_ETH_TX_OFFLOAD_MULTI_SEGS)) > pdata->multi_segs_tx = true; > > - if (pdata->multi_segs_tx) > - dev->tx_pkt_burst = &axgbe_xmit_pkts_seg; > - else if (txq->vector_disable || > - rte_vect_get_max_simd_bitwidth() < RTE_VECT_SIMD_128) > - dev->tx_pkt_burst = &axgbe_xmit_pkts; > - else > -#ifdef RTE_ARCH_X86 > - dev->tx_pkt_burst = &axgbe_xmit_pkts_vec; > -#else > - dev->tx_pkt_burst = &axgbe_xmit_pkts; > -#endif > > return 0; > } > diff --git a/drivers/net/axgbe/axgbe_rxtx.h b/drivers/net/axgbe/axgbe_rxtx.h > index eeef908ceb..d5660f5c4b 100644 > --- a/drivers/net/axgbe/axgbe_rxtx.h > +++ b/drivers/net/axgbe/axgbe_rxtx.h > @@ -158,7 +158,12 @@ struct axgbe_tx_queue { > * RX/TX function prototypes > */ > > - > +/* Takes an ethdev and sets up the tx function to be used based on > + * the queue parameters. Used in dev_start by primary process and then > + * in dev_init by secondary process when attaching to an existing ethdev. Is the comment only for Tx, or for burst functions? Since comment refers only to Tx. > + */ > +void axgbe_set_tx_function(struct rte_eth_dev *dev); > +void axgbe_set_rx_function(struct rte_eth_dev *dev); You can put an empty line here to highlight comment is for these two functions. > void axgbe_dev_tx_queue_release(struct rte_eth_dev *dev, uint16_t queue_idx); > int axgbe_dev_tx_queue_setup(struct rte_eth_dev *dev, uint16_t tx_queue_id, > uint16_t nb_tx_desc, unsigned int socket_id,