[dpdk-dev] [PATCH v1 2/8] bus: introduce opaque control framework

[Shreyansh Jain]

On Thursday 12 October 2017 01:48 PM, Gaetan Rivet wrote:
> New configuration elements are added to the buses. They make the ABI
> unstable and will continue to do so.
> 
> This new control scheme allows to add new bus operators without
> breaking the ABI and by only expanding the API.
> 
> This helps having more stability in core EAL subsystems, while allowing
> flexibility for future evolutions.
> 
> Signed-off-by: Gaetan Rivet <gaetan.rivet at 6wind.com>
> ---
>   lib/librte_eal/common/eal_common_bus.c  |  9 +++++++
>   lib/librte_eal/common/include/rte_bus.h | 46 +++++++++++++++++++++++++++++++++
>   2 files changed, 55 insertions(+)
> 
> diff --git a/lib/librte_eal/common/eal_common_bus.c b/lib/librte_eal/common/eal_common_bus.c
> index 3c66a02..65d7229 100644
> --- a/lib/librte_eal/common/eal_common_bus.c
> +++ b/lib/librte_eal/common/eal_common_bus.c
> @@ -42,6 +42,13 @@
>   struct rte_bus_list rte_bus_list =
>   	TAILQ_HEAD_INITIALIZER(rte_bus_list);
>   
> +static rte_bus_ctrl_t
> +rte_bus_default_ctrl(enum rte_bus_ctrl_op op __rte_unused,
> +		     enum rte_bus_ctrl_item item __rte_unused)
> +{
> +	return NULL;
> +}
> +
>   void
>   rte_bus_register(struct rte_bus *bus)
>   {
> @@ -53,6 +60,8 @@ rte_bus_register(struct rte_bus *bus)
>   	RTE_VERIFY(bus->find_device);
>   	/* Buses supporting driver plug also require unplug. */
>   	RTE_VERIFY(!bus->plug || bus->unplug);
> +	if (bus->ctrl == NULL)
> +		bus->ctrl = &rte_bus_default_ctrl;
>   
>   	TAILQ_INSERT_TAIL(&rte_bus_list, bus, next);
>   	RTE_LOG(DEBUG, EAL, "Registered [%s] bus.\n", bus->name);
> diff --git a/lib/librte_eal/common/include/rte_bus.h b/lib/librte_eal/common/include/rte_bus.h
> index 331d954..bd3c28e 100644
> --- a/lib/librte_eal/common/include/rte_bus.h
> +++ b/lib/librte_eal/common/include/rte_bus.h
> @@ -183,6 +183,51 @@ struct rte_bus_conf {
>   	enum rte_bus_probe_mode probe_mode; /**< Probe policy. */
>   };
>   
> +/**
> + * Bus configuration items.
> + */
> +enum rte_bus_ctrl_item {
> +	RTE_BUS_CTRL_PROBE_MODE = 0,
> +	RTE_BUS_CTRL_ITEM_MAX,
> +};

I am assuming that a driver implementation can take more than ITEM_MAX 
control knobs. It is opaque to the library. Are we on same page?

For example, a bus driver can implement:

rte_bus_XXX_ctrl_item {
	<Leaving space for allowing rte_bus.h implementations>
	RTE_BUS_XYZ_KNOB_1 = 100,
	RTE_BUS_XYZ_KNOB_2,
	RTE_BUS_XYZ_KNOB_3,
};

without the library knowing or restricting the API to RTE_BUS_CTRL_ITEM_MAX.

I see that in your code for PCI (Patch 5/8: pci_ctrl) you have 
restricted the control knob to RTE_BUS_CTRL_ITEM_MAX.
I hope that such restrictions would not float to library layer.

If we are on same page, should this be documented as a code comment 
somewhere?
if not, do you think what I am stating makes sense?

> +
> +/**
> + * Bus configuration operations.
> + */
> +enum rte_bus_ctrl_op {
> +	RTE_BUS_CTRL_GET = 0,
> +	RTE_BUS_CTRL_SET,
> +	RTE_BUS_CTRL_RESET,
> +	RTE_BUS_CTRL_OP_MAX,
> +};

Similarly, the driver implementation can choose to implement a operation 
which is not defined in the above structures. Obviously, the application 
is expected to know - it being a custom knob.

[...]