Page MenuHomeFreeBSD

cxgbe tom: Permit rcv_nxt mismatches on FIN for iSCSI connections on T6.
ClosedPublic

Authored by jhb on Jun 22 2021, 11:13 PM.

Details

Summary

The remote peer might send a FIN in the middle of a burst of data
PDUs. In the case of T6 with data PDU completion moderation, the
driver would not have seen these PDUs since the final PDU in the burst
was never received resulting in a stale rcv_nxt when the FIN is
received.

While here, invert the logic in the condition to be more readable and
always set tp->rcv_nxt from the sequence number in the CPL. This sets
the proper value of rcv_nxt for FINs on connections with data received
but not reported via a CPL (e.g. a partial iSCSI PDU burst interrupted
by a FIN).

Reported by: Jithesh Arakkan @ Chelsio

Diff Detail

Repository
R10 FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

jhb requested review of this revision.Jun 22 2021, 11:13 PM
sys/dev/cxgbe/tom/t4_cpl_io.c
1316

My only question here is if you want this conditional on T6 as the mismatches should only occur on T6 and later due to completion moderation. T5 should in theory still always have a valid rcv_nxt.

np added inline comments.
sys/dev/cxgbe/tom/t4_cpl_io.c
1316

Yes, let's keep it as tight as possible. These seq# checks have caught legitimate problems in the past.

This revision is now accepted and ready to land.Jun 22 2021, 11:22 PM

Mmm, adding T6 to this started to make it unwieldy, so I flipped the condition to put the the KASSERT in the else clause with an empty if body, but I also noticed that technically we would report the wrong rcv_nxt value for the FIN when we actually had the proper value in the CPL, so I've further refined this to do the check before updating tp->rcv_nxt, and to just update rcv_nxt to the value in the CPL. I'll do some testing before updating the version here.

jhb planned changes to this revision.Jul 9 2021, 7:37 PM
jhb retitled this revision from cxgbe tom: Permit rcv_nxt mismatches on FIN for iSCSI connections. to cxgbe tom: Permit rcv_nxt mismatches on FIN for iSCSI connections on T6..Jul 19 2021, 10:12 PM
jhb edited the summary of this revision. (Show Details)
  • Restrict check to T6.
  • Invert condition in check.
  • Always use seq from CPL as seq # of FIN.

I only did light testing of this with iSCSI, but I tested both "plain" TCP offload as well as iSCSI offload connections.

This revision is now accepted and ready to land.Aug 2 2021, 12:02 AM