<div dir="ltr"><div>Hi Jiayu,</div><div>Can you please suggest if the patch can be used ? When timestamps are disabled we would unnecessarily indulge pure TCP ack packets in the GRO layer. Or can we have a fix where if the TCP timestamp option is not present in the packet, do not process the packet, return immediately ? <br></div><div><br></div><div>Thanks,</div><div>Param<br></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Mon, Apr 25, 2022 at 11:36 PM kumaraparameshwaran rathinavel <<a href="mailto:kumaraparamesh92@gmail.com">kumaraparamesh92@gmail.com</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"><div dir="ltr"><div>Hi, <br></div><div><br></div><div>I would like you to review this patch and let me know what you think of it. <br></div><div><br></div><div>Thanks,</div><div>Kumara.<br></div><div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">---------- Forwarded message ---------<br>From: <b class="gmail_sendername" dir="auto">Kumara Parameshwaran</b> <span dir="auto"><<a href="mailto:kumaraparamesh92@gmail.com" target="_blank">kumaraparamesh92@gmail.com</a>></span><br>Date: Sun, Apr 3, 2022 at 5:20 PM<br>Subject: [PATCH v1] gro: bug fix in identifying 0 length tcp packets<br>To: <<a href="mailto:jiayu.hu@intel.com" target="_blank">jiayu.hu@intel.com</a>><br>Cc: <<a href="mailto:dev@dpdk.org" target="_blank">dev@dpdk.org</a>>, Kumara Parameshwaran <<a href="mailto:kumaraparamesh92@gmail.com" target="_blank">kumaraparamesh92@gmail.com</a>>, <<a href="mailto:stable@dpdk.org" target="_blank">stable@dpdk.org</a>>, Kumara Parameshwaran <<a href="mailto:kparameshwar@vmware.com" target="_blank">kparameshwar@vmware.com</a>><br></div><br><br>As the minimum Ethernet frame size is 64 bytes, a 0 length<br>
tcp payload without tcp options would be 54 bytes and hence<br>
there would be padding. So it would be incorrect to use the<br>
packet length to determine the tcp data length.<br>
<br>
Fixes: 1e4cf4d6d4fb ("gro: cleanup")<br>
Cc: <a href="mailto:stable@dpdk.org" target="_blank">stable@dpdk.org</a><br>
<br>
Signed-off-by: Kumara Parameshwaran <<a href="mailto:kparameshwar@vmware.com" target="_blank">kparameshwar@vmware.com</a>><br>
---<br>
v1:<br>
Do not use packet length to determine the tcp data length as <br>
the packet length could have padded bytes. This would lead <br>
to addition of 0 length tcp packets into the GRO layer when <br>
there ethernet fram is padded.<br>
lib/gro/gro_tcp4.c | 5 ++---<br>
1 file changed, 2 insertions(+), 3 deletions(-)<br>
<br>
diff --git a/lib/gro/gro_tcp4.c b/lib/gro/gro_tcp4.c<br>
index 7498c66..45e3f48 100644<br>
--- a/lib/gro/gro_tcp4.c<br>
+++ b/lib/gro/gro_tcp4.c<br>
@@ -198,7 +198,7 @@ gro_tcp4_reassemble(struct rte_mbuf *pkt,<br>
struct rte_tcp_hdr *tcp_hdr;<br>
uint32_t sent_seq;<br>
int32_t tcp_dl;<br>
- uint16_t ip_id, hdr_len, frag_off;<br>
+ uint16_t ip_id, frag_off;<br>
uint8_t is_atomic;<br>
<br>
struct tcp4_flow_key key;<br>
@@ -217,7 +217,6 @@ gro_tcp4_reassemble(struct rte_mbuf *pkt,<br>
eth_hdr = rte_pktmbuf_mtod(pkt, struct rte_ether_hdr *);<br>
ipv4_hdr = (struct rte_ipv4_hdr *)((char *)eth_hdr + pkt->l2_len);<br>
tcp_hdr = (struct rte_tcp_hdr *)((char *)ipv4_hdr + pkt->l3_len);<br>
- hdr_len = pkt->l2_len + pkt->l3_len + pkt->l4_len;<br>
<br>
/*<br>
* Don't process the packet which has FIN, SYN, RST, PSH, URG, ECE<br>
@@ -229,7 +228,7 @@ gro_tcp4_reassemble(struct rte_mbuf *pkt,<br>
* Don't process the packet whose payload length is less than or<br>
* equal to 0.<br>
*/<br>
- tcp_dl = pkt->pkt_len - hdr_len;<br>
+ tcp_dl = rte_be_to_cpu_16(ipv4_hdr->total_length) - (pkt->l3_len + pkt->l4_len);<br>
if (tcp_dl <= 0)<br>
return -1;<br>
<br>
-- <br>
2.7.4<br>
<br>
</div></div></div>
</blockquote></div>