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 1966445B12; Fri, 11 Oct 2024 14:54:14 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 95B6C402A1; Fri, 11 Oct 2024 14:54:13 +0200 (CEST) Received: from frasgout.his.huawei.com (frasgout.his.huawei.com [185.176.79.56]) by mails.dpdk.org (Postfix) with ESMTP id 7EFD84028B for ; Fri, 11 Oct 2024 14:54:12 +0200 (CEST) Received: from mail.maildlp.com (unknown [172.18.186.231]) by frasgout.his.huawei.com (SkyGuard) with ESMTP id 4XQ64r1FYpz6K5rP; Fri, 11 Oct 2024 20:53:48 +0800 (CST) Received: from frapeml100007.china.huawei.com (unknown [7.182.85.133]) by mail.maildlp.com (Postfix) with ESMTPS id 9E9981401DC; Fri, 11 Oct 2024 20:54:11 +0800 (CST) Received: from frapeml500007.china.huawei.com (7.182.85.172) by frapeml100007.china.huawei.com (7.182.85.133) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.1.2507.39; Fri, 11 Oct 2024 14:54:11 +0200 Received: from frapeml500007.china.huawei.com ([7.182.85.172]) by frapeml500007.china.huawei.com ([7.182.85.172]) with mapi id 15.01.2507.039; Fri, 11 Oct 2024 14:54:11 +0200 From: Konstantin Ananyev To: Dariusz Sosnowski , Thomas Monjalon , Ferruh Yigit , Andrew Rybchenko CC: "dev@dpdk.org" Subject: RE: [PATCH v2 2/4] ethdev: add get restore flags driver callback Thread-Topic: [PATCH v2 2/4] ethdev: add get restore flags driver callback Thread-Index: AQHbG8DLmiQPOxbaRkyY7vXRFtM097KBf+Ew Date: Fri, 11 Oct 2024 12:54:11 +0000 Message-ID: <36888a4e9c074e06a822ca84a2eda09b@huawei.com> References: <20241011092103.181145-1-dsosnowski@nvidia.com> <20241011093351.187191-1-dsosnowski@nvidia.com> <20241011093351.187191-3-dsosnowski@nvidia.com> In-Reply-To: <20241011093351.187191-3-dsosnowski@nvidia.com> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.206.138.42] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 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 > Before this patch, ethdev layer assumed that all drivers require that > it has to forcefully restore: >=20 > - MAC addresses > - promiscuous mode setting > - all multicast mode setting >=20 > upon rte_eth_dev_start(). >=20 > This patch introduces a new callback to eth_dev_ops - > get_restore_flags(). > Drivers implementing this callback can explicitly enable/disable > certain parts of config restore procedure. >=20 > In order to minimize the changes to all the drivers and > preserve the current behavior, it is assumed that > if this callback is not defined, all configuration should be restored. >=20 > Signed-off-by: Dariusz Sosnowski LGTM, just few nits/suggestions, pls see below. Series-Acked-by: Konstantin Ananyev > --- > lib/ethdev/ethdev_driver.c | 11 +++++++ > lib/ethdev/ethdev_driver.h | 64 ++++++++++++++++++++++++++++++++++++++ > lib/ethdev/version.map | 1 + > 3 files changed, 76 insertions(+) >=20 > diff --git a/lib/ethdev/ethdev_driver.c b/lib/ethdev/ethdev_driver.c > index c335a25a82..f78f9fb5c1 100644 > --- a/lib/ethdev/ethdev_driver.c > +++ b/lib/ethdev/ethdev_driver.c > @@ -958,3 +958,14 @@ rte_eth_switch_domain_free(uint16_t domain_id) >=20 > return 0; > } > + > +void > +rte_eth_get_restore_flags(struct rte_eth_dev *dev, > + enum rte_eth_dev_operation op, > + uint32_t *flags) I would go with uint64_t for flags (to avoid limitations in future). Also, If it is void anyway, might be just return flags? i.e: uint64_t +rte_eth_get_restore_flags(struct rte_eth_dev *dev, enum rte_eth_= dev_operation op); Another thing - do we need to do update docs? PMD or PG sections, release notes? > +{ > + if (dev->dev_ops->get_restore_flags !=3D NULL) > + dev->dev_ops->get_restore_flags(dev, op, flags); > + else > + *flags =3D RTE_ETH_RESTORE_ALL; > +} > diff --git a/lib/ethdev/ethdev_driver.h b/lib/ethdev/ethdev_driver.h > index ae00ead865..8ac5328521 100644 > --- a/lib/ethdev/ethdev_driver.h > +++ b/lib/ethdev/ethdev_driver.h > @@ -1235,6 +1235,47 @@ typedef int (*eth_count_aggr_ports_t)(struct rte_e= th_dev *dev); > typedef int (*eth_map_aggr_tx_affinity_t)(struct rte_eth_dev *dev, uint1= 6_t tx_queue_id, > uint8_t affinity); >=20 > +/** > + * @internal > + * Defines types of operations which can be executed by the application. > + */ > +enum rte_eth_dev_operation { > + RTE_ETH_START, > +}; > + > +/**@{@name Restore flags > + * Flags returned by get_restore_flags() callback. > + * They indicate to ethdev layer which configuration is required to be r= estored. > + */ > +/** If set, ethdev layer will forcefully restore default and any other a= dded MAC addresses. */ > +#define RTE_ETH_RESTORE_MAC_ADDR RTE_BIT32(0) > +/** If set, ethdev layer will forcefully restore current promiscuous mod= e setting. */ > +#define RTE_ETH_RESTORE_PROMISC RTE_BIT32(1) > +/** If set, ethdev layer will forcefully restore current all multicast m= ode setting. */ > +#define RTE_ETH_RESTORE_ALLMULTI RTE_BIT32(2) > +/**@}*/ > + > +/** All configuration which can be restored by ethdev layer. */ > +#define RTE_ETH_RESTORE_ALL (RTE_ETH_RESTORE_MAC_ADDR | \ > + RTE_ETH_RESTORE_PROMISC | \ > + RTE_ETH_RESTORE_ALLMULTI) > + > +/** > + * @internal > + * Fetch from the driver what kind of configuration must be restored by = ethdev layer, > + * after certain operations are performed by the application (such as rt= e_eth_dev_start()). > + * > + * @param dev > + * Port (ethdev) handle. > + * @param op > + * Type of operation executed by the application. > + * @param flags > + * Flags indicating what configuration must be restored by ethdev laye= r. > + */ > +typedef void (*eth_get_restore_flags_t)(struct rte_eth_dev *dev, > + enum rte_eth_dev_operation op, > + uint32_t *flags); > + > /** > * @internal A structure containing the functions exported by an Etherne= t driver. > */ > @@ -1474,6 +1515,9 @@ struct eth_dev_ops { > eth_count_aggr_ports_t count_aggr_ports; > /** Map a Tx queue with an aggregated port of the DPDK port */ > eth_map_aggr_tx_affinity_t map_aggr_tx_affinity; > + > + /** Get configuration which ethdev should restore */ > + eth_get_restore_flags_t get_restore_flags; > }; >=20 > /** > @@ -2131,6 +2175,26 @@ struct rte_eth_fdir_conf { > struct rte_eth_fdir_flex_conf flex_conf; > }; >=20 > +/** > + * @internal > + * Fetch from the driver what kind of configuration must be restored by = ethdev layer, > + * using get_restore_flags() callback. > + * > + * If callback is not defined, it is assumed that all supported configur= ation must be restored. > + * > + * @param dev > + * Port (ethdev) handle. > + * @param op > + * Type of operation executed by the application. > + * @param flags > + * Flags indicating what configuration must be restored by ethdev laye= r. > + */ > +__rte_internal For new function we probably need 'rte_experimental'... But from other side - it is internal. Not sure what are rules here for such case. > +void > +rte_eth_get_restore_flags(struct rte_eth_dev *dev, > + enum rte_eth_dev_operation op, > + uint32_t *flags); > + > #ifdef __cplusplus > } > #endif > diff --git a/lib/ethdev/version.map b/lib/ethdev/version.map > index 1669055ca5..fa0469e602 100644 > --- a/lib/ethdev/version.map > +++ b/lib/ethdev/version.map > @@ -358,4 +358,5 @@ INTERNAL { > rte_eth_switch_domain_alloc; > rte_eth_switch_domain_free; > rte_flow_fp_default_ops; > + rte_eth_get_restore_flags; > }; > -- > 2.39.5