Again, just some quick comments after a glimpse.

On Sat, Jul 01, 2017 at 07:08:42PM +0800, Jiayu Hu wrote:
> +     for (i = 0; i < nb_pkts; i++) {
> +             if (RTE_ETH_IS_IPV4_HDR(pkts[i]->packet_type) &&
> +                             (pkts[i]->packet_type & RTE_PTYPE_L4_TCP)) {
> +                     ret = gro_tcp4_reassemble(pkts[i],
> +                                     &tcp_tbl,
> +                                     param->max_packet_size,
> +                                     current_time);
> +                     if (ret > 0)
> +                             /* merge successfully */
> +                             nb_after_gro--;
> +                     else if (ret < 0)
> +                             unprocess_pkts[unprocess_num++] =
> +                                     pkts[i];

Even it's just one statement, if the statement is spawned more than
one line, including the comment, the {} should be used.

Section 1.6.2. Control Statements and Loops of:
http://dpdk.org/doc/guides/contributing/coding_style.html

> +             } else
> +                     unprocess_pkts[unprocess_num++] =
> +                             pkts[i];

Besides, why breaking it to two lines, judging that it can be fit into
one line smaller than 80 chars.

> +     }
> +
> +     /* re-arrange GROed packets */
> +     if (nb_after_gro < nb_pkts) {
> +             i = gro_tcp4_tbl_timeout_flush(&tcp_tbl, 0,
> +                             pkts, nb_pkts);
> +             if (unprocess_num > 0)
> +                     memcpy(&pkts[i], unprocess_pkts,
> +                                     sizeof(struct rte_mbuf *) *
> +                                     unprocess_num);

Ditto.

> +void *gro_tcp4_tbl_create(uint16_t socket_id,
> +             uint16_t max_flow_num,
> +             uint16_t max_item_per_flow)
> +{
> +     size_t size;
> +     uint32_t entries_num;
> +     struct gro_tcp4_tbl *tbl;
> +
> +     entries_num = max_flow_num * max_item_per_flow;
> +     entries_num = entries_num > GRO_TCP4_TBL_MAX_ITEM_NUM ?
> +             GRO_TCP4_TBL_MAX_ITEM_NUM : entries_num;
> +
> +     if (entries_num == 0)
> +             return NULL;
> +
> +     tbl = (struct gro_tcp4_tbl *)rte_zmalloc_socket(
> +                     __func__,
> +                     sizeof(struct gro_tcp4_tbl),
> +                     RTE_CACHE_LINE_SIZE,
> +                     socket_id);

Again, the cast (from void *) is unnessary and should be dropped.

> +     memcpy(&(tbl->keys[key_idx].key),
> +                     &key, sizeof(struct tcp4_key));

Again, I believe they two can be fit into one single line.

        --yliu

Reply via email to