Re: [PATCH v2] tcp: Store the owner connections for flags frames

[Yumei Huang <yuhuang@redhat.com>]

On Wed, Sep 10, 2025 at 6:50 PM Volker Diels-Grabsch <v@njh.eu> wrote:
>
> Dear Yumei,
>
> Thanks a lot for providing a proper fix for that issue.  Just a minor
> nitpick from my side:
>
> Yumei Huang wrote:
> > @@ -209,13 +209,14 @@ int tcp_buf_send_flag(const struct ctx *c, struct tcp_tap_conn *conn, int flags)
> >       if (ret <= 0)
> >               return ret;
> >
> > -     tcp_payload_used++;
> > +     tcp_frame_conns[tcp_payload_used++] = conn;
> >       l4len = optlen + sizeof(struct tcphdr);
> >       iov[TCP_IOV_PAYLOAD].iov_len = l4len;
> >       tcp_l2_buf_fill_headers(conn, iov, NULL, seq, false);
> >
> >       if (flags & DUP_ACK) {
> >               struct iovec *dup_iov = tcp_l2_iov[tcp_payload_used++];
> > +             tcp_frame_conns[tcp_payload_used - 1] = conn;
>
> I find it a bit strange to read that way, incrementing tcp_payload_used
> just to subtract one from it in the next step.  I, personally, would
> find it easier to read and to understand that way around:
>
>                 struct iovec *dup_iov = tcp_l2_iov[tcp_payload_used];
>                 tcp_frame_conns[tcp_payload_used++] = conn;

Good point. Sent v3. Thanks!

>
> But maybe it's just me.
>
>
> Best regards,
> Volker
>
> --
> .---<<<((()))>>>---.
> |      [[||]]      |
> '---<<<((()))>>>---'
>


--
Thanks,

Yumei Huang