Shall I raise a PR with the change ?


On Wed, Oct 12, 2022 at 7:04 AM Jun Qiu <jun.qiu@jaguarmicro.com> wrote:

> Yes,  It's better to do it before the tcp_dl check.
>
>
>
>    1. /* trim the tail padding bytes */
>    2. ip_tlen = rte_be_to_cpu_16(ipv4_hdr->total_length);
>    3. *if* (pkt->pkt_len > (uint32_t)(ip_tlen + pkt->l2_len))
>    4.     rte_pktmbuf_trim(pkt, pkt->pkt_len - ip_tlen - pkt->l2_len);
>    5.
>    6. /*
>    7.  * Don't process the packet whose payload length is less than or
>    8.  * equal to 0.
>    9.  */
>    10. tcp_dl = pkt->pkt_len - hdr_len;
>    11. *if* (tcp_dl <= 0)
>    12.     *return* -1;
>
> *From:* kumaraparameshwaran rathinavel <kumaraparamesh92@gmail.com>
> *Sent:* Tuesday, October 11, 2022 1:48 PM
> *To:* dev@dpdk.org; David Marchand <david.marchand@redhat.com>; Hu, Jiayu
> <jiayu.hu@intel.com>; Jun Qiu <jun.qiu@jaguarmicro.com>
> *Subject:* Fwd: [PATCH] gro : fix pkt length when extra bytes are padded
> to ethernet frame
>
>
>
>
>
>
>
> On Tue, Oct 11, 2022 at 1:03 AM David Marchand <david.marchand@redhat.com>
> wrote:
>
> On Mon, Oct 10, 2022 at 7:51 PM Kumara Parameshwaran
> <kumaraparamesh92@gmail.com> wrote:
> >
> > From: Kumara Parameshwaran <kumaraparamesh92@gmail.com>
> >
> > When GRO packets are merged the packet length is used while
> > merging the adjacent packets. If the padded bytes are accounted
> > we would end up acking unsent TCP segments.
> >
> > Signed-off-by: Kumara Parameshwaran <kumaraparamesh92@gmail.com>
> > v1:
> >         If there is padding to the ethernet frame cases where timestamp
> is disabled
> >         the packet length should be validated with the total ip length
> as packet length
> >         is used in the GRO merging logic
>
> Please, can you test with current main branch?
> We recently merged a fix:
>
> https://git.dpdk.org/dpdk/commit/?id=b8a55871d5af6f5b8694b1cb5eacbc629734e403
>
> Thanks, David. I did look at the patch. This would fix the problem, but
> lets say for a case of an ACK packet where TCP data length would be zero
> and if the packet contains the padded bytes, this check should be done
> before the follwing check and not after this. @Hu, Jiayu
> <jiayu.hu@intel.com>  @Jun Qiu <jun.qiu@jaguarmicro.com>  please let me
> know your thoughts.
>
> /*
> * Don't process the packet whose payload length is less than or
> * equal to 0.
> */
> tcp_dl = pkt->pkt_len - hdr_len;
> if (tcp_dl <= 0)
> return -1;
>
> --
> David Marchand
>
>