<div dir="ltr"><div dir="ltr"><br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Sun, Jan 7, 2024 at 10:50 PM Stephen Hemminger <<a href="mailto:stephen@networkplumber.org">stephen@networkplumber.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">On Sun, 7 Jan 2024 16:59:20 +0530<br>
Kumara Parameshwaran <<a href="mailto:kumaraparamesh92@gmail.com" target="_blank">kumaraparamesh92@gmail.com</a>> wrote:<br>
<br>
> + /* Return early if the TCP flags are not handled in GRO layer */<br>
> + if (tcp_hdr->tcp_flags & (~(VALID_GRO_TCP_FLAGS)))<br>
<br>
Nit, lots of extra paren here. Could be:<br>
if (tcp_hdr->tcp_flags & ~VALID_GRO_TCP_FLAGS)<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Done. <br></blockquote>
> + if (find == 1) {<br>
> + /*<br>
> + * Any packet with additional flags like PSH,FIN should be processed<br>
> + * and flushed immediately.<br>
> + * Hence marking the start time to 0, so that the packets will be flushed<br>
> + * immediately in timer mode.<br>
> + */<br>
> + if (tcp_hdr->tcp_flags & (RTE_TCP_ACK_FLAG | RTE_TCP_PSH_FLAG | RTE_TCP_FIN_FLAG)) {<br>
> + if (tcp_hdr->tcp_flags != RTE_TCP_ACK_FLAG)<br>
> + tbl->items[item_start_idx].start_time = 0;<br>
> + return process_tcp_item(pkt, tcp_hdr, tcp_dl, tbl->items,<br>
> + tbl->flows[i].start_index,<br>
> + &tbl->item_num, tbl->max_item_num,<br>
> + ip_id, is_atomic, start_time);<br>
> + } else {<br>
> + return -1;<br>
> + }<br>
> + }<br>
<br>
Reordering this conditional would keep code from being so indented.<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Doing this reordering as suggested by Jiyau since the find == 1 would be likely in most cases. <br></blockquote>
> - delete_tcp_item(tbl->items, item_idx, &tbl->item_num, INVALID_ARRAY_INDEX);<br>
> + delete_tcp_item(tbl->items, item_idx, &tbl->item_num,<br>
> + INVALID_ARRAY_INDEX);<br>
> return -1;<br>
<br>
This change is unnecessary, max line length in DPDK is 100 characters for readability.<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Done. <br></blockquote>
> return 0;<br>
> + } else {<br>
> + return -1;<br>
> }<br>
> <br>
> - return process_tcp_item(pkt, tcp_hdr, tcp_dl, tbl->items, tbl->flows[i].start_index,<br>
> - &tbl->item_num, tbl->max_item_num,<br>
> - ip_id, is_atomic, start_time);<br>
> + return -1;<br>
> }<br>
<br>
Since end of else and end of function both return -1, the else clause is unnecessary.<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Done. <br></blockquote>
</blockquote></div></div>