On Sun,  7 Jan 2024 16:59:20 +0530
Kumara Parameshwaran <kumaraparamesh92@gmail.com> wrote:

> +     /* Return early if the TCP flags are not handled in GRO layer */
> +     if (tcp_hdr->tcp_flags & (~(VALID_GRO_TCP_FLAGS)))

Nit, lots of extra paren here. Could be:
        if (tcp_hdr->tcp_flags & ~VALID_GRO_TCP_FLAGS)

Done.

> +     if (find == 1) {
> +             /*
> +              * Any packet with additional flags like PSH,FIN should be processed
> +              * and flushed immediately.
> +              * Hence marking the start time to 0, so that the packets will be flushed
> +              * immediately in timer mode.
> +              */
> +             if (tcp_hdr->tcp_flags & (RTE_TCP_ACK_FLAG | RTE_TCP_PSH_FLAG | RTE_TCP_FIN_FLAG)) {
> +                     if (tcp_hdr->tcp_flags != RTE_TCP_ACK_FLAG)
> +                             tbl->items[item_start_idx].start_time = 0;
> +                     return process_tcp_item(pkt, tcp_hdr, tcp_dl, tbl->items,
> +                                             tbl->flows[i].start_index,
> +                                             &tbl->item_num, tbl->max_item_num,
> +                                             ip_id, is_atomic, start_time);
> +             } else {
> +                     return -1;
> +             }
> +     }

Reordering this conditional would keep code from being so indented.

Doing this reordering as suggested by Jiyau since the find == 1 would be likely in most cases.

> -                     delete_tcp_item(tbl->items, item_idx, &tbl->item_num, INVALID_ARRAY_INDEX);
> +                     delete_tcp_item(tbl->items, item_idx, &tbl->item_num,
> +                             INVALID_ARRAY_INDEX);
>                       return -1;

This change is unnecessary, max line length in DPDK is 100 characters for readability.

Done.

>               return 0;
> +     } else {
> +             return -1;
>       }
> 
> -     return process_tcp_item(pkt, tcp_hdr, tcp_dl, tbl->items, tbl->flows[i].start_index,
> -                                             &tbl->item_num, tbl->max_item_num,
> -                                             ip_id, is_atomic, start_time);
> +     return -1;
>  }

Since end of else and end of function both return -1, the else clause is unnecessary.

Done.