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 0E2CDA0571; Tue, 28 Jun 2022 19:07:14 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id DE62C400D7; Tue, 28 Jun 2022 19:07:13 +0200 (CEST) Received: from linux.microsoft.com (linux.microsoft.com [13.77.154.182]) by mails.dpdk.org (Postfix) with ESMTP id 2742B40042 for ; Tue, 28 Jun 2022 19:07:13 +0200 (CEST) Received: by linux.microsoft.com (Postfix, from userid 1086) id 491AD20CDF3E; Tue, 28 Jun 2022 10:07:12 -0700 (PDT) DKIM-Filter: OpenDKIM Filter v2.11.0 linux.microsoft.com 491AD20CDF3E DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.microsoft.com; s=default; t=1656436032; bh=KsG4SfkN11sv5i/+O4Z43m2d7jPyzr0VbyyaJ0oCavI=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=Yc4VI87Nk17wMiQ+95e3C7D9V3+S2irw0NkKoV1bKl//+y5ctFQwA1N29UFM3T3Fv DbfSz+jtXFLoeTNuuEao21WTg9ZP8OsD3pUDjfNLX62OxEjXxzv8Uaut4HFt1zdz8H LAl9nynhowEGe+ZPw7+8yw8qazatt/JvWdThtSAA= Date: Tue, 28 Jun 2022 10:07:12 -0700 From: Tyler Retzlaff To: Stephen Hemminger Cc: David Marchand , 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: <20220628170712.GC17875@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net> References: <20220628144643.1213026-1-david.marchand@redhat.com> <20220628144643.1213026-12-david.marchand@redhat.com> <20220628162213.GA17875@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net> <20220628092905.2da82a8c@hermes.local> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20220628092905.2da82a8c@hermes.local> 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 09:29:05AM -0700, Stephen Hemminger wrote: > On Tue, 28 Jun 2022 09:22:13 -0700 > Tyler Retzlaff wrote: > > > 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. > > I don't like that because it least to dangerous casts in the internal code. > Better to keep the the type of the object. As long as the API only passes > around an pointer to a struct, without exposing the contents of the struct; > it is safer and easier to debug. as i mentioned you can use an inline/internal function or even a macro to hide the cast, you could provide some additional integrity checks here if desired as a value add. the fact that you expose that it is a struct is an internal implementation detail, if truly opaque tomorrow you could convert it to a simple integer that indexes or enumerates something and prevents any meaningful interpretation in the application. when you say it is safer to debug i think you mean for dpdk devs not the application developer because unless the app developer does something really gross/dangerous casting they really can't "mishandle" the opaque object except to use one that isn't initialized at all which we can detect and handle internally in a general way. i will however concede there would be slightly more finger work when debugging dpdk itself since gdb / debugger doesn't automatically infer type so you end up having to tell gdb what is in the uintptr_t. anyway just drawing from experience in the driver frameworks we maintain in windows, i think one of our regrets is that we didn't do this from day 1 and subsequentl that we initially only used one opaque type instead of defining separate (not implicitly convertable) types to each opaque type.