Re: [dpdk-dev] [PATCH v4 02/15] ethdev: add support for hairpin queue

[Thomas Monjalon <thomas@monjalon.net>]

17/10/2019 17:32, Ori Kam:
> --- a/lib/librte_ethdev/rte_ethdev.h
> +++ b/lib/librte_ethdev/rte_ethdev.h
>  /**
> + * @warning
> + * @b EXPERIMENTAL: this API may change, or be removed, without prior notice
> + *
> + * A structure used to return the hairpin capabilities that are supported.
> + */
> +struct rte_eth_hairpin_cap {
> +	uint16_t max_n_queues;
> +	/**< The max number of hairpin queues (different bindings). */
> +	uint16_t max_rx_2_tx;
> +	/**< Max number of Rx queues to be connected to one Tx queue. */
> +	uint16_t max_tx_2_rx;
> +	/**< Max number of Tx queues to be connected to one Rx queue. */
> +	uint16_t max_nb_desc; /**< The max num of descriptors. */
> +};

I think you can switch to "comment-first style" for this struct.


> +#define RTE_ETH_MAX_HAIRPIN_PEERS 32

Usually I think such define is in the build config.
Any other opinion?


> +/**
> + * @warning
> + * @b EXPERIMENTAL: this API may change, or be removed, without prior notice
> + *
> + * A structure used to hold hairpin peer data.
> + */
> +struct rte_eth_hairpin_peer {
> +	uint16_t port; /**< Peer port. */
> +	uint16_t queue; /**< Peer queue. */
> +};

It may be the right place to give more words about what is a peer,
can we have multiple peers, etc.


> +/**
> + * @warning
> + * @b EXPERIMENTAL: this API may change, or be removed, without prior notice
> + *
> + * A structure used to configure hairpin binding.
> + */
> +struct rte_eth_hairpin_conf {
> +	uint16_t peer_n; /**< The number of peers. */

In general, I don't like one-letter abbreviations.
Is peer_count better?

> +	struct rte_eth_hairpin_peer peers[RTE_ETH_MAX_HAIRPIN_PEERS];
> +};