Haven't looked at the details yet, and below are some quick comments
after a glimpse.
On Sat, Jul 01, 2017 at 07:08:41PM +0800, Jiayu Hu wrote:
...
> +void *rte_gro_tbl_create(const
> + const struct rte_gro_param *param)
The DPDK style is:
void *
rte_gro_tbl_destroy(...)
Also you should revisit all other functions, as I have seen quite many
coding style issues like this.
> +{
> + gro_tbl_create_fn create_tbl_fn;
> + gro_tbl_destroy_fn destroy_tbl_fn;
> + struct gro_tbl *gro_tbl;
> + uint64_t gro_type_flag = 0;
> + uint8_t i, j;
> +
> + gro_tbl = rte_zmalloc_socket(__func__,
> + sizeof(struct gro_tbl),
> + RTE_CACHE_LINE_SIZE,
> + param->socket_id);
> + if (gro_tbl == NULL)
> + return NULL;
> + gro_tbl->max_packet_size = param->max_packet_size;
> + gro_tbl->max_timeout_cycles = param->max_timeout_cycles;
> + gro_tbl->desired_gro_types = param->desired_gro_types;
> +
> + for (i = 0; i < RTE_GRO_TYPE_MAX_NUM; i++) {
> + gro_type_flag = 1 << i;
> +
> + if ((param->desired_gro_types & gro_type_flag) == 0)
> + continue;
> + create_tbl_fn = tbl_create_functions[i];
> + if (create_tbl_fn == NULL)
> + continue;
> +
> + gro_tbl->tbls[i] = create_tbl_fn(
> + param->socket_id,
> + param->max_flow_num,
> + param->max_item_per_flow);
> + if (gro_tbl->tbls[i] == NULL) {
> + /* destroy all allocated tables */
> + for (j = 0; j < i; j++) {
> + gro_type_flag = 1 << j;
> + if ((param->desired_gro_types & gro_type_flag)
> == 0)
> + continue;
> + destroy_tbl_fn = tbl_destroy_functions[j];
> + if (destroy_tbl_fn)
> + destroy_tbl_fn(gro_tbl->tbls[j]);
> + }
> + rte_free(gro_tbl);
> + return NULL;
The typical way to handle this is to re-use rte_gro_tbl_destroy() as
much as possible. This saves duplicate code.
> + }
> + }
> + return gro_tbl;
> +}
> +
> +void rte_gro_tbl_destroy(void *tbl)
> +{
> + gro_tbl_destroy_fn destroy_tbl_fn;
> + struct gro_tbl *gro_tbl = (struct gro_tbl *)tbl;
The cast (from void *) is unnecessary and can be dropped.
...
> +/**
> + * the max packets number that rte_gro_reassemble_burst can
> + * process in each invocation.
> + */
> +#define RTE_GRO_MAX_BURST_ITEM_NUM 128UL
> +
> +/* max number of supported GRO types */
> +#define RTE_GRO_TYPE_MAX_NUM 64
> +#define RTE_GRO_TYPE_SUPPORT_NUM 0 /**< current supported GRO num */
The reason we need use comment style of "/**< ... */" is because this
is what the doc generator (doxygen) recognizes. If not doing this, your
comment won't be displayed at the generated doc page (for example,
http://dpdk.org/doc/api/rte__ethdev_8h.html#ade7de72f6c0f8102d01a0b3438856900).
The format, as far as I known, could be:
/**< here is a comment */
#define A_MACRO x
Or the one you did for RTE_GRO_TYPE_SUPPORT_NUM: put it at the end
of the line.
That being said, the comments for RTE_GRO_MAX_BURST_ITEM_NUM and
RTE_GRO_TYPE_MAX_NUM should be changed. Again, you should revisit
other places.
> +
> +
> +struct rte_gro_param {
> + uint64_t desired_gro_types; /**< desired GRO types */
> + uint32_t max_packet_size; /**< max length of merged packets */
> + uint16_t max_flow_num; /**< max flow number */
> + uint16_t max_item_per_flow; /**< max packet number per flow */
> +
> + /* socket index where the Ethernet port connects to */
Ditto.
...
> +++ b/lib/librte_gro/rte_gro_version.map
> @@ -0,0 +1,12 @@
> +DPDK_17.08 {
> + global:
> +
> + rte_gro_tbl_create;
> + rte_gro_tbl_destroy;
> + rte_gro_reassemble_burst;
> + rte_gro_reassemble;
> + rte_gro_timeout_flush;
> + rte_gro_tbl_item_num;
The undocumented habit is to list them in alpha order.
--yliu
