[PATCH v11] gro: fix reordering of packets in GRO layer
kumaraparameshwaran rathinavel
kumaraparamesh92 at gmail.com
Mon Jan 8 17:11:58 CET 2024
On Sun, Jan 7, 2024 at 10:50 PM Stephen Hemminger <
stephen at networkplumber.org> wrote:
> On Sun, 7 Jan 2024 16:59:20 +0530
> Kumara Parameshwaran <kumaraparamesh92 at 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.
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mails.dpdk.org/archives/dev/attachments/20240108/add0e3d0/attachment-0001.htm>
More information about the dev
mailing list