[RFC PATCH 11/11] bus: hide bus object

Tyler Retzlaff roretzla at linux.microsoft.com
Tue Jun 28 18:22:13 CEST 2022


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 <david.marchand at redhat.com>
> ---

... 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.



More information about the dev mailing list