From: Shreyansh Jain <shreyansh.jain@nxp.com>
To: Gaetan Rivet <gaetan.rivet@6wind.com>
Cc: <dev@dpdk.org>
Subject: Re: [dpdk-dev] [PATCH v1 2/8] bus: introduce opaque control framework
Date: Mon, 11 Dec 2017 17:30:16 +0530 [thread overview]
Message-ID: <088de7d2-9bd4-4a49-44ba-9df9c52b72d1@nxp.com> (raw)
In-Reply-To: <91106540c460d22dd23a30dc2903d7e238ff9a3b.1507796085.git.gaetan.rivet@6wind.com>
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@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.
[...]
next prev parent reply other threads:[~2017-12-11 11:46 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-10-12 8:18 [dpdk-dev] [PATCH v1 0/8] Bus " Gaetan Rivet
2017-10-12 8:18 ` [dpdk-dev] [PATCH v1 1/8] bus: rename scan policy as probe policy Gaetan Rivet
2017-10-12 8:18 ` [dpdk-dev] [PATCH v1 2/8] bus: introduce opaque control framework Gaetan Rivet
2017-12-11 12:00 ` Shreyansh Jain [this message]
2017-12-11 12:43 ` Gaëtan Rivet
2017-12-11 13:36 ` Shreyansh Jain
2017-12-11 14:38 ` Gaëtan Rivet
2017-12-12 7:21 ` Shreyansh Jain
2017-10-12 8:18 ` [dpdk-dev] [PATCH v1 3/8] bus: remove probe mode configuration structure Gaetan Rivet
2017-10-12 8:18 ` [dpdk-dev] [PATCH v1 4/8] bus: add probe mode setter Gaetan Rivet
2017-12-11 12:39 ` Shreyansh Jain
2017-12-11 12:43 ` Shreyansh Jain
2017-10-12 8:18 ` [dpdk-dev] [PATCH v1 5/8] bus/pci: implement ctrl operator Gaetan Rivet
2017-10-12 8:18 ` [dpdk-dev] [PATCH v1 6/8] bus: add IOVA mode as a ctrl operation Gaetan Rivet
2017-10-12 8:18 ` [dpdk-dev] [PATCH v1 7/8] bus/pci: implement IOVA mode getter Gaetan Rivet
2017-10-12 8:18 ` [dpdk-dev] [PATCH v1 8/8] bus: remove redundant " Gaetan Rivet
2017-12-11 11:53 ` [dpdk-dev] [PATCH v1 0/8] Bus control framework Shreyansh Jain
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=088de7d2-9bd4-4a49-44ba-9df9c52b72d1@nxp.com \
--to=shreyansh.jain@nxp.com \
--cc=dev@dpdk.org \
--cc=gaetan.rivet@6wind.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).