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 6230BA0577; Tue, 28 Jun 2022 18:22:16 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id F1573400D7; Tue, 28 Jun 2022 18:22:15 +0200 (CEST) Received: from linux.microsoft.com (linux.microsoft.com [13.77.154.182]) by mails.dpdk.org (Postfix) with ESMTP id C383340042 for ; Tue, 28 Jun 2022 18:22:14 +0200 (CEST) Received: by linux.microsoft.com (Postfix, from userid 1086) id B3A9D20CDF32; Tue, 28 Jun 2022 09:22:13 -0700 (PDT) DKIM-Filter: OpenDKIM Filter v2.11.0 linux.microsoft.com B3A9D20CDF32 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.microsoft.com; s=default; t=1656433333; bh=dWNd8hWsBDOBl1XdzT2vKEocgK5u9Xq0MGPIljSF+6A=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=m4ZSbcpzfkypzJuAkl1TEQdwmyVKiNtfMHggr0kKAVIS+itmhNyqvXMOY5tBDtNqL IOHZXPfH6Le+FAcQOrfqK/vpB+r5DpV7WqJmC02eQLzI99AqQ6PO7vZF1RnuYPlILb M7rmulqWFPXVxEiXffQE9roDeu/5Trk31zkMyYnI= Date: Tue, 28 Jun 2022 09:22:13 -0700 From: Tyler Retzlaff To: David Marchand Cc: dev@dpdk.org, thomas@monjalon.net, bruce.richardson@intel.com, kevin.laatz@intel.com, Parav Pandit , Xueming Li , Hemant Agrawal , Sachin Saxena , Rosen Xu , Stephen Hemminger , Long Li , Chas Williams , "Min Hu (Connor)" , Matan Azrad , Ray Kinsella , Ferruh Yigit , Andrew Rybchenko Subject: Re: [RFC PATCH 11/11] bus: hide bus object Message-ID: <20220628162213.GA17875@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net> References: <20220628144643.1213026-1-david.marchand@redhat.com> <20220628144643.1213026-12-david.marchand@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20220628144643.1213026-12-david.marchand@redhat.com> User-Agent: Mutt/1.5.21 (2010-09-15) 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 Tue, Jun 28, 2022 at 04:46:43PM +0200, David Marchand wrote: > Make rte_bus opaque for non internal users. > This will make extending this object possible without breaking the ABI. > > Introduce a new driver header and move rte_bus definition and helpers. > > Signed-off-by: David Marchand > --- ... snip ... > -struct rte_bus { > - RTE_TAILQ_ENTRY(rte_bus) next; /**< Next bus object in linked list */ > - const char *name; /**< Name of the bus */ > - rte_bus_scan_t scan; /**< Scan for devices attached to bus */ > - rte_bus_probe_t probe; /**< Probe devices on bus */ > - rte_bus_find_device_t find_device; /**< Find a device on the bus */ > - rte_bus_plug_t plug; /**< Probe single device for drivers */ > - rte_bus_unplug_t unplug; /**< Remove single device from driver */ > - rte_bus_parse_t parse; /**< Parse a device name */ > - rte_bus_devargs_parse_t devargs_parse; /**< Parse bus devargs */ > - rte_dev_dma_map_t dma_map; /**< DMA map for device in the bus */ > - rte_dev_dma_unmap_t dma_unmap; /**< DMA unmap for device in the bus */ > - struct rte_bus_conf conf; /**< Bus configuration */ > - rte_bus_get_iommu_class_t get_iommu_class; /**< Get iommu class */ > - rte_dev_iterate_t dev_iterate; /**< Device iterator. */ > - rte_bus_hot_unplug_handler_t hot_unplug_handler; > - /**< handle hot-unplug failure on the bus */ > - rte_bus_sigbus_handler_t sigbus_handler; > - /**< handle sigbus error on the bus */ > - > -}; since we're overhauling anyway we could follow suit with a number of the lessons from posix apis e.g. pthread and eliminate typed pointers for a little extra value. > +struct rte_bus; > +struct rte_device; to avoid people tripping over mishandling pointers in/out of the api surface taking the opaque object you could declare opaque handle for the api to operate on instead. it would force the use of a cast in the implementation but would avoid accidental void * of the wrong thing that got cached being passed in. if the cast was really undesirable just whack it under an inline / internal function. e.g. make the opaque object an explicit type. struct { uintptr_t opaque; } rte_bus_handle_t; // implementation rte_bus_handle_t rte_bus_find(rte_bus_handle_t start, rte_bus_cmp_t cmp, const void *data) { struct rte_bus *bus = (struct rte_bus *)start.opaque; // do bus things on bus return {.opaque = whatever}; } then you will get hard compile time failure if someone messes up argument passing. e.g. void *somerandomp = ...; ... rte_bus_find(somerandomp, ...); // compile fail i would actually suggest this wrapping in a struct / handle approach for any opaque object that is passed over the api surface. the change is bigger of course... it has various other advantages when/if we decide to make the bus/driver surface stable in abi which i understand is not a promise dpdk makes right now but still we might one day. anyway, just a suggestion. i like the series either way.