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 BBB9AA0C55; Mon, 6 Sep 2021 20:41:48 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 38C91410F8; Mon, 6 Sep 2021 20:41:48 +0200 (CEST) Received: from mga18.intel.com (mga18.intel.com [134.134.136.126]) by mails.dpdk.org (Postfix) with ESMTP id A5939410F0; Mon, 6 Sep 2021 20:41:46 +0200 (CEST) X-IronPort-AV: E=McAfee;i="6200,9189,10099"; a="207138954" X-IronPort-AV: E=Sophos;i="5.85,273,1624345200"; d="scan'208";a="207138954" Received: from fmsmga006.fm.intel.com ([10.253.24.20]) by orsmga106.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 06 Sep 2021 11:41:45 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.85,273,1624345200"; d="scan'208";a="691768273" Received: from fmsmsx601.amr.corp.intel.com ([10.18.126.81]) by fmsmga006.fm.intel.com with ESMTP; 06 Sep 2021 11:41:45 -0700 Received: from fmsmsx608.amr.corp.intel.com (10.18.126.88) by fmsmsx601.amr.corp.intel.com (10.18.126.81) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2242.12; Mon, 6 Sep 2021 11:41:44 -0700 Received: from fmsedg601.ED.cps.intel.com (10.1.192.135) by fmsmsx608.amr.corp.intel.com (10.18.126.88) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2242.12 via Frontend Transport; Mon, 6 Sep 2021 11:41:44 -0700 Received: from NAM10-DM6-obe.outbound.protection.outlook.com (104.47.58.109) by edgegateway.intel.com (192.55.55.70) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.1.2242.10; Mon, 6 Sep 2021 11:41:44 -0700 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=im/efMWfAUXmcpn950By+4YLXWwfFjf7Pk3PM/XKrn2YkzXC2cLkKPPp6e4JNNTjjGrtkUSN3NQYEzjpCN4DfFIWxCU3FtupGr0jfhadYBuh8qSiwc+f4LO2x2lhKMZRNFSm1hWiGrMjEWylM1KHBh0VWvY0ePU2IIWRzn4EiQbaB/pgajV1oPbwrzuaW2aNov4KbumW7dZ6ADPOYPylaVmTYXO3YzAXoQrJCS/QvcQFgrGHwzbN6eeQp5tF3ipdxuIFE06lDVM2kmhJbG7VfigsxxKlzMzxfk8xW587VaUCc7Zj2UZgACTyBUUM0XR5jNsMc8K1pO/ykKbuJgFS7g== 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; bh=+UwppZtMSe7CQTe7xqCXnneP+UxpJy1OBmqpolVvXYo=; b=RNC1wf4YlcSQLMNowNVNZNJNgJMNW2ANE2zSZ0rBc8DRGFGOkVfALCWU4hjjspxYangBgEvRqx3CouVgCLu+5j6jTIHazJZdd1hEuBwcTKpt6q27tcGG+HrBRYIROBUJjjLhPLct8UBSORmjagGikej30ojqBNCPXVq16Gybe8qKS6zFnRtMi5EU34GyUGJ8l3NFg5oVGCSC6ScCUrEC/ysnPRtpYDIT6GP4OUGtr2AsiFTQR9pE/B+/eculd9XTXFJ3K3f2rn6T3mMQqtRMJHgjYTSxKJ3YKgXDR2AN1U0defqSttzlqtgdxlTAkjQdiiLh5mH6ZBj10/Q6BU+o/w== 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 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=intel.onmicrosoft.com; s=selector2-intel-onmicrosoft-com; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=+UwppZtMSe7CQTe7xqCXnneP+UxpJy1OBmqpolVvXYo=; b=aAbnmPcHSWZwz4hFEhNQBqiUKWPX0k95XyEXxXYgN9sCPqFiNRXJ+cAjIV34OYYw/vopQX11Z1jQ8kHA4aiKq6U3rRLX8/oMlv7MC9SdT0PbL3F9LTmBgdgOKo6zmIEQDmxseWjw7qFuDa6F2Icp0kJKCfopfHGRI2H4nNcvaYs= Authentication-Results: dpdk.org; dkim=none (message not signed) header.d=none;dpdk.org; dmarc=none action=none header.from=intel.com; Received: from PH0PR11MB5000.namprd11.prod.outlook.com (2603:10b6:510:41::19) by PH0PR11MB4888.namprd11.prod.outlook.com (2603:10b6:510:32::22) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.4478.22; Mon, 6 Sep 2021 18:41:40 +0000 Received: from PH0PR11MB5000.namprd11.prod.outlook.com ([fe80::747b:3a08:d1ec:31fc]) by PH0PR11MB5000.namprd11.prod.outlook.com ([fe80::747b:3a08:d1ec:31fc%5]) with mapi id 15.20.4478.025; Mon, 6 Sep 2021 18:41:40 +0000 To: Konstantin Ananyev , CC: , , , , , References: <20210820162834.12544-1-konstantin.ananyev@intel.com> <20210820162834.12544-3-konstantin.ananyev@intel.com> From: Ferruh Yigit X-User: ferruhy Message-ID: Date: Mon, 6 Sep 2021 19:41:34 +0100 In-Reply-To: <20210820162834.12544-3-konstantin.ananyev@intel.com> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 8bit X-ClientProxiedBy: DB8PR06CA0038.eurprd06.prod.outlook.com (2603:10a6:10:120::12) To PH0PR11MB5000.namprd11.prod.outlook.com (2603:10b6:510:41::19) MIME-Version: 1.0 Received: from [192.168.0.206] (37.228.236.146) by DB8PR06CA0038.eurprd06.prod.outlook.com (2603:10a6:10:120::12) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.4478.17 via Frontend Transport; Mon, 6 Sep 2021 18:41:39 +0000 X-MS-PublicTrafficType: Email X-MS-Office365-Filtering-Correlation-Id: ad18439d-2ef2-45ec-d197-08d97165f765 X-MS-TrafficTypeDiagnostic: PH0PR11MB4888: X-LD-Processed: 46c98d88-e344-4ed4-8496-4ed7712e255d,ExtAddr X-MS-Exchange-Transport-Forked: True X-Microsoft-Antispam-PRVS: X-MS-Oob-TLC-OOBClassifiers: OLM:9508; X-MS-Exchange-SenderADCheck: 1 X-MS-Exchange-AntiSpam-Relay: 0 X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: hzjy2f682XPXQ8YZzzj9TvUdXDJ+yHgfWYeoh+yy4zDELdU40g86PIvpPpTrY6Nzs66BUMXuWq4wL2ZkosdLWLs3Wcbq6yWvezfpcirWXxOAVUdT4y4qqNMEOg7YcHmn1nFSnLqo3JZ9iMKRbD1y2VQ8m5s36YGMwLjjcfFFmVbNO/OByRVNzRZssRlLfNd2NpFrtfurTSVZKjFurFaj/CfILD5+Rb/DLG8+QSDfwwpZhsamvqZkIkiMN12rtVvlQh+Lj3aMyyuogPZrytLo1o5lUkEi579dGcmJ0dyUwpEtrtH5J5yBMAxRgNuGDgE0Lcnm2gUt+fDUCeqHR5Isi9CTMMVvyOVtIaj5U0V50MbbYzBlL8TTpRAkGJ7xh2jrfwNGw4iyuSNNkJW8VL6tsm8NvZF6fIWtRAndBA6Q+z0rbdl19jsbrW7jxp06lQ32YzeqatZOzHbstgOTn6YIjzjO4kcmYqhyBvymfNrsrRUSzw7yHJr2xFlJGXi3gJDIimseBe2RgoLARG8Xl8EdLZGu9Ci/Wg87rnC+Xq7ZEapbQCaGnTTnzBxu9me6BcUvPxCtgzJL5YPNWecGEdgWzH8gEOv7R58NNDEVPGSvye9nKAsUjyfdb6mZ4TV83q7PdMJuJr2EBWeUeou1B8zC+x5yytRBBHf39BvSGwSqV9SaeGY4C0MyE+Ylv1QpV1TpHq6WTz4jn3N975xEN1gP8hgzKvMZlkNhGoC6kOZoL+A= 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:(4636009)(366004)(376002)(346002)(136003)(39860400002)(396003)(478600001)(956004)(4326008)(31696002)(8936002)(2616005)(66556008)(66476007)(6486002)(186003)(86362001)(44832011)(26005)(6666004)(5660300002)(31686004)(66946007)(2906002)(8676002)(16576012)(316002)(38100700002)(53546011)(83380400001)(36756003)(83323001)(45980500001); DIR:OUT; SFP:1102; X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1 X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?aVV4S2xVdDVVdjNqV3M1MWJEQmlaVFlqQ1EyeGhCd3Q5Q1JYSVp2bGNOU201?= =?utf-8?B?anczbnBzU3dJOXQxNGxpMGd6NEtrYXhTWTJnQllmYmx2TTRSZzFyUEJoeU9V?= =?utf-8?B?NkR4VDJEdmFIVTJLbm01QktEWEF6dTErK2VGRHZYUitrWCtCYWYwZHdHbXJO?= =?utf-8?B?Q2FhT3VLR3lXNFB3cnhTanp2bDNYOHhNR1oxeXNZNTdaejBIa3BWd2QzakFT?= =?utf-8?B?VnQ5SjA3UW9KY2xGditycTlKU3d1dHR1cGNBcXBXcm93RVZnSDYvYi9seWtw?= =?utf-8?B?VG1wOFl6QURFQU1xWXozN0RGcmxMaTJxQVNQQ25VWW1INlE1dElabHIvTXlB?= =?utf-8?B?WDU3a2thUXRxWFFPcmk2NTg5enprdFBmZDdkQWNZeWJFK055RkdDWEMrYS9Q?= =?utf-8?B?Q21WcGdUeUR2MkNWTFAzR0VLZFdmTHpKc2p1K0U1WXo0YjlRVEFEUzZGUzlY?= =?utf-8?B?OVliSGZKd3lTd1QyVzNGNlJqK2xqRUlqN3V2ZmhzMjBsd0RPbHhkWEc5N2tF?= =?utf-8?B?NVlqTjFIeU1QZmh5M2FwU3ZYL3NWSFRqcGRKZ3FjakJFalpsTFk2bDBiaHp6?= =?utf-8?B?bnBUSSs2U1ZaU1lBTDE3WlRkaVhPcW1EZzg0QWhwRklPTE1pRHdCcXhpR3ZJ?= =?utf-8?B?THB1Y2xWbzVSUUtRUlZKTzQ5WGpRaldPbjRBM1hhczQ4ZVNmRWxFTHlIUm9w?= =?utf-8?B?VG1MSk8yaHhwZDlxS3NxQ3F6a2loK25PeXJTYVdIMFFNZVh3eS8ybm9xVlds?= =?utf-8?B?Mm53QlErb3NNN2Y1T0lpVmptVlJGbnl0WlIxSlVYWXFGQ05CODV1cEQvVnEw?= =?utf-8?B?L2RYU1lmdDFMSE92ZEs0SjZQOEZDWnVGK2kvYWFxMzNyaE5aU1dVMExCTTJi?= =?utf-8?B?VUZuS09nQ3Y0cTlwMVJ1RnhPSWNzbFZ4MS9BNjFDdTBoZURRRmR3QURPYTdu?= =?utf-8?B?azUxMk95OExQMWFad2t2Ym40djNqZDVJRjBZWCtLS0tJc2tUZjNNM2t3ZVVO?= =?utf-8?B?V2xqSW13VEJ1cm5wNFZQR2FOaHJCa2dJMTdlV0YxOFFxM1FFWFBJWVRzc0oy?= =?utf-8?B?M3QrK2FxOWNBUWNKZ3dKY3pxWkZFaUhtVVZpTVIvd3pybjBmZlpsREptNUI2?= =?utf-8?B?UE1raC81RTlReThHaTVmV1B0MXZ1N1JYUE5XV2NyOFJkZXJmem9jaWdiNjZG?= =?utf-8?B?VVpxS290QXlISnMrMVpWaUdzZ2kramkwVUk3Wk9uU056OEZOcUxMc2lXLzZo?= =?utf-8?B?NzZid3FsZ2NGRE9kTHFCVzB6ZUh6ZnlrdVZlNTRYQWVFK3VmU3FsWnFpeXNN?= =?utf-8?B?SjF5cjJrYUhsWGd6T20rdG13K3M0bEtrYzBwNW9KN0lGdnJLcUFXcXA3c0NQ?= =?utf-8?B?aVVBbGljb2I5Q1NpZHcvM25VNmhwOGFLbnA4S0FRNGlpRXI0enMrbEtqM2V1?= =?utf-8?B?Z1FYRDRPdUFkd1hCcjVSZjhOeHhZRE10cWkxRGZNWm9MM2FsV2dHclpDb3Qr?= =?utf-8?B?RkRCdnZhT3pES0tGME5xVTFSbWlyR1NjcnpmQlM1ZkdnaTNYME9xOUpLZjAz?= =?utf-8?B?TVRrV0ZOOENFYVRkb25vdE81WjZQZURlYnBRcFBvOTY4Y09NMnhUVFpTbGN1?= =?utf-8?B?NnlvN1M3UkM5UkVaMGRiMGhUZ3hoOTNqTjMzbm5QSlF1Yjl6cG95N25KU2ta?= =?utf-8?B?Tk53d0o3Ym9OTVUrOE1mVGRaSkV1ZDgrSFdKUXhsdEVYekQ5Zk00WEZNbWwy?= =?utf-8?Q?gU0GzkT4/B0Bpu6SUddE5WInm2KYnlFp9Gf8XnL?= X-MS-Exchange-CrossTenant-Network-Message-Id: ad18439d-2ef2-45ec-d197-08d97165f765 X-MS-Exchange-CrossTenant-AuthSource: PH0PR11MB5000.namprd11.prod.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-OriginalArrivalTime: 06 Sep 2021 18:41:40.7320 (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: VEWulYQPD6Hcx9CSTPe1qp31gfEc/JSJUzrP/EpmBUxMHkHvynU9jh2q4JVPDqA20KVFeXUZnPSoowdHdz6ibQ== X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH0PR11MB4888 X-OriginatorOrg: intel.com Subject: Re: [dpdk-dev] [RFC 2/7] eth: make drivers to use new API for Rx 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 Sender: "dev" On 8/20/2021 5:28 PM, Konstantin Ananyev wrote: > ethdev: > - make changes so drivers can start using new API for rx_pkt_burst(). > - provide helper functions/macros. > - remove rx_pkt_burst() from 'struct rte_eth_dev'. > drivers/net: > - adjust to new rx_burst API. > Overall this enables us hiding the ethdev internals, which is good. But it duplicates most of the datapath function (rx burst for this patch) per each PMD ops. I wonder if we can have the callbacks ('_rte_eth_rx_epilog()') as separate function, this still enables us to hide the structs. Of course additional function call will bring some overhead, but if we enabled callbacks and calling them per packet, do we really care about additional function call? > Signed-off-by: Konstantin Ananyev <...> > @@ -3229,7 +3289,7 @@ int > ice_rx_burst_mode_get(struct rte_eth_dev *dev, __rte_unused uint16_t queue_id, > struct rte_eth_burst_mode *mode) > { > - eth_rx_burst_t pkt_burst = dev->rx_pkt_burst; > + rte_eth_rx_burst_t pkt_burst = rte_eth_get_rx_burst(dev->data->port_id); Does it makes easier to orginanise the patchset to have a separate patch to switch first to 'rte_eth_get_rx_burst()' / 'rte_eth_set_rx_burst()' with old implementation ('dev->rx_pkt_burst' get/set), and later just change the 'rte_eth_get_rx_burst()' / 'rte_eth_set_rx_burst()' implementation when structure is updated. <...> > --- a/drivers/net/ice/ice_rxtx_vec_sse.c > +++ b/drivers/net/ice/ice_rxtx_vec_sse.c > @@ -587,13 +587,15 @@ _ice_recv_raw_pkts_vec(struct ice_rx_queue *rxq, struct rte_mbuf **rx_pkts, > * - nb_pkts > ICE_VPMD_RX_BURST, only scan ICE_VPMD_RX_BURST > * numbers of DD bits > */ > -uint16_t > +static inline uint16_t These functions eventually will be called via a function pointer, so is there a benefit to request them to 'inline', why not just 'static' ? <...> > +_RTE_ETH_RX_DEF(ice_recv_scattered_pkts_vec) > + This will duplicate most of the Rx burst function for each PMD Rx ops. <...> > + > +#define _RTE_ETH_FUNC(fn) _rte_eth_##fn > + Do we need this macro? The functions are still 'static', so they won't be visible to application and there won't be a namespace problem. Dropping and just use the original fucntion name may reduce the changes in the drivers. <...> > +__rte_experimental > +rte_eth_rx_burst_t rte_eth_get_rx_burst(uint16_t port_id); > + > +__rte_experimental > +int rte_eth_set_rx_burst(uint16_t port_id, rte_eth_rx_burst_t rxf); can s/__rte_experimental/__rte_internal/ <...> > + > +__rte_experimental > +rte_eth_rx_burst_t > +rte_eth_get_rx_burst(uint16_t port_id) > +{ > + if (port_id >= RTE_DIM(rte_eth_burst_api)) { > + rte_errno = EINVAL; > + return NULL; > + } > + return rte_eth_burst_api[port_id].rx_pkt_burst; > +} > + > +__rte_experimental > +int > +rte_eth_set_rx_burst(uint16_t port_id, rte_eth_rx_burst_t rxf) > +{ > + if (port_id >= RTE_DIM(rte_eth_burst_api)) > + return -EINVAL; > + > + rte_eth_burst_api[port_id].rx_pkt_burst = rxf; > + return 0; > +} Since these are internal functions for drivers, it can be easier for drivers to use directly with 'struct rte_eth_dev *eth_dev', instead of 'port_id'. So instead of APIs getting 'port_id' as parameter, they can get 'struct rte_eth_dev *eth_dev'? Drivers for sure will have 'eth_dev' references for their device. Overall, I think make sense for all public APIs to have handler ('port_id') as parameter, and all driver APIs to have 'eth_device' as paramter. <...> > @@ -4981,44 +4981,11 @@ static inline uint16_t > rte_eth_rx_burst(uint16_t port_id, uint16_t queue_id, > struct rte_mbuf **rx_pkts, const uint16_t nb_pkts) > { > - struct rte_eth_dev *dev = &rte_eth_devices[port_id]; > - uint16_t nb_rx; > - > -#ifdef RTE_ETHDEV_DEBUG_RX > - RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, 0); > - RTE_FUNC_PTR_OR_ERR_RET(*dev->rx_pkt_burst, 0); > - > - if (queue_id >= dev->data->nb_rx_queues) { > - RTE_ETHDEV_LOG(ERR, "Invalid RX queue_id=%u\n", queue_id); > + if (port_id >= RTE_MAX_ETHPORTS) As an API, it makes sense to validate the input. But not sure to add these checks as part of this set, as previous versions don't have it. Perhaps we can add them with separate patch and discussion. <...> > +++ b/lib/ethdev/version.map > @@ -249,6 +249,11 @@ EXPERIMENTAL { > rte_mtr_meter_policy_delete; > rte_mtr_meter_policy_update; > rte_mtr_meter_policy_validate; > + > + # added in 21.11 > + rte_eth_burst_api; > + rte_eth_get_rx_burst; > + rte_eth_set_rx_burst; I think these APIs intented to use only by drivers, so instead of experimental, they can be added as 'INTERNAL'.