Data corruption from #93 still relevant

[tolikheha]

Data corruption from #93 still relevant and situation worse than it looks.

        if (tlen <= sbspace(&so->so_rcv)) {
            if (th->th_seq == tp->rcv_nxt &&
                OFP_LIST_EMPTY(&tp->t_segq) &&
                TCPS_HAVEESTABLISHED(tp->t_state)) {
                if (DELAY_ACK(tp))
                    t_flags_or(tp->t_flags, TF_DELACK);
                else
                    t_flags_or(tp->t_flags, TF_ACKNOW);
                tp->rcv_nxt += tlen;
                thflags = th->th_flags & OFP_TH_FIN;
                TCPSTAT_INC(tcps_rcvpack);
                TCPSTAT_ADD(tcps_rcvbyte, tlen);
                ND6_HINT(tp);
                SOCKBUF_LOCK(&so->so_rcv);
                if (so->so_rcv.sb_state & SBS_CANTRCVMORE)
                    odp_packet_free(m);
                else
                    ofp_sbappendstream_locked(&so->so_rcv,
                                    m);
                /* NB: sorwakeup_locked() does an implicit unlock. */
                sorwakeup_locked(so);
            } else {
                /*
                 * XXX: Due to the header drop above "th" is
                 * theoretically invalid by now.  Fortunately
                 * m_adj() doesn't actually frees any mbufs
                 * when trimming from the head.
                 */
                thflags = ofp_tcp_reass(tp, th, &tlen, m);
                t_flags_or(tp->t_flags, TF_ACKNOW);
}

There are few moment:

First, ofp_sbappendstream_locked can drop packet. So tcp controll block shouldn't be updated before ofp_sbappendstream_locked called and return value checked. Currently it doesn't return anything, but int ofp_sockbuf_put_last can return -1 in case of error and why it not passed here is a big mistery.

Next, this implementation will call ofp_tcp_reass even if there are space only for 1 packet. But this function can try to add a lot more packets than 1. And it will try to do this with ofp_sbappendstream_locked, which will drop all that packets with succesfull updating of tcp controll block. So check of socket buffer should be added to ofp_tcp_reass too.

[rishdas]

Mailing list

The main communication channel for the project is our mailing list: https://list.openfastpath.org/mailman/listinfo/openfastpath

To post a message to all the list members you need to be a subscriber. Email: openfastpath@list.openfastpath.org

Patches

Patches shall be submitted via the mailinglist in diff format. Please keep patches small, atomic (ie isolated to one fix or addition) and well-explained. Consider that the project uses Linux kernel coding style, which is verified through checkpatch.pl.

[rishdas]

Hi Thank you for the patch but @OpenFastPath patches are accepted why mailing lists. Please use git format-patch and send using git send-email to send the patch for review on the mailing list.

[sovu]

Hi,

Anyone who wants to contribute to the OpenFastPath Foundation needs to sign the contribution agreement. Please fill and sign the Contribution License Agreement, then send a scanned PDF of it to me: http://www.openfastpath.org/downloads/OpenFastPath_ContributionAgreement.pdf

More at: http://www.openfastpath.org/index.php/about-us/

• BYLAWS AND POLICIES

Kind Regards, Sorin Vultureanu OFP maintainer

email: sorin.vultureanu@enea.com