Page MenuHomeFreeBSD

Add missing dtrace receive and send probes for TCP
ClosedPublic

Authored by tuexen on Jul 20 2018, 8:07 PM.

Details

Summary

TCP was missing a couple of send and receive probes for dtrace, mostly in the syn-cache handling and the handling of the TIMEWAIT state. This patch fixes that.

Test Plan

Execute the tests from D16288.

Diff Detail

Repository
rS 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

tuexen created this revision.Jul 20 2018, 8:07 PM
tuexen added a reviewer: rrs.Jul 20 2018, 8:25 PM
markj accepted this revision.Jul 21 2018, 7:03 PM
markj added inline comments.
sys/netinet/tcp_timewait.c
458 ↗(On Diff #45626)

I would use an "else" here instead of adding an extra label and goto.

tuexen added inline comments.Jul 22 2018, 11:30 AM
sys/netinet/tcp_timewait.c
458 ↗(On Diff #45626)

So you would prefer:

	/*
	 * Acknowledge the segment if it has data or is not a duplicate ACK.
	 */
	if (thflags != TH_ACK || tlen != 0 ||
	    th->th_seq != tw->rcv_nxt || th->th_ack != tw->snd_nxt) {
		TCP_PROBE5(receive, NULL, NULL, m, NULL, th);
		tcp_twrespond(tw, TH_ACK);
	} else {
drop:
		TCP_PROBE5(receive, NULL, NULL, m, NULL, th);
	}
	INP_WUNLOCK(inp);
	m_freem(m);
	return (0);
}

I thought about it, but didn't like the drop label inside an else clause...
Do you really prefer it? If yes, I'll change. Please let me know.

tuexen retitled this revision from Add missing dtrace receive and send probes to Add missing dtrace receive and send probes for TCP.Jul 22 2018, 11:31 AM
markj added inline comments.Jul 22 2018, 4:20 PM
sys/netinet/tcp_timewait.c
458 ↗(On Diff #45626)

I don't prefer it strongly enough to insist upon it, especially since I don't work on TCP. :) The version here is fine with me.

rrs accepted this revision.Jul 30 2018, 12:00 PM
rrs added inline comments.
sys/netinet/tcp_timewait.c
458 ↗(On Diff #45626)

I like it with the label. The drop inside the else is not something
I would like to see.

This revision is now accepted and ready to land.Jul 30 2018, 12:00 PM
This revision was automatically updated to reflect the committed changes.