Page MenuHomeFreeBSD

Add UDP receive probes
ClosedPublic

Authored by tuexen on Jun 28 2018, 2:28 PM.
Tags
None
Referenced Files
Unknown Object (File)
Thu, Oct 31, 10:53 PM
Unknown Object (File)
Sat, Oct 26, 7:55 AM
Unknown Object (File)
Sat, Oct 26, 7:55 AM
Unknown Object (File)
Sat, Oct 26, 7:55 AM
Unknown Object (File)
Sat, Oct 26, 7:55 AM
Unknown Object (File)
Sat, Oct 26, 7:55 AM
Unknown Object (File)
Sat, Oct 26, 7:54 AM
Unknown Object (File)
Sat, Oct 26, 7:45 AM
Subscribers

Details

Summary

When a UDP packet is received and there is no endpoint to consume it, no UDP receive probe is fired. This is fixed by this patch. The corresponding dtrace test has been updated to verify this.

When a UDP/IPv4 packet is received and the TTL is smaller than the minimum TTL required by the endpoint, no UDP receive probe is fired. This is different from the TCP/IPv4 case. To improve consistency, fix this also by this patch.

Test Plan

Use the attached dtrace script udpio.d from the udp_dtrace man page and use nc -u 127.0.0.1 1234 and nc -u ::1 1234 to generate UDP packets assuming there is not endpoint on port 1234. The dtrace script should show the packets as sent and received.

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

It would be nice to add a test for this to cddl/contrib/opensolaris/cmd/dtrace/test/tst/common/ip, but I don't think that should block the commit of this change. I'm happy to help with that if needed.

Thanks for pointing me to the tests. I wasn't aware of them. Is there any documentation available how to run and/or write them?

Adding tests makes sense since I'm also adding some probes to TCP and finally will add support for SCTP and UDPLite for which
I'll add then tests, too.

Thanks for pointing me to the tests. I wasn't aware pf them. Is there any documentation available how to run and/or write them?

Adding tests makes sense since I'm also adding some probes to TCP and finally will add support for SCTP and UDPLite for which
I'll add then tests, too.

For the time being you need to set WITH_DTRACE_TESTS=YES in src.conf. Then, they get built and installed from cddl/usr.sbin/dtrace/tests. After adding tests, you need to run cddl/usr.sbin/dtrace/tests/tools/genmakefiles.sh to update the makefiles.

The tests can be run using kyua. They get installed to /usr/tests/cddl/usr.sbin/dtrace/tests. Most of them depend on the shells/pdksh port.

Simple question: And how do I run these test using kyua?

Simple question: And how do I run these test using kyua?

As root (required for dtrace), just run "kyua test" in the subdirectory containing the tests you wish to run. To run all dtrace tests, run it from /usr/tests/cddl/usr.sbin/dtrace.

Update dtrace test for UDP to reflect this change. In particular, the reception of the UDP packet should be counted.

This looks ok to me. It would be nice to amend the udp dtrace man page to state under what conditions the receive probe may fire, but I think that can be done as a separate revision.

Add explicit text to the man page describing when the receive probe fires.

@markj I'll commit the changes to the dtrace scripts separately from the changes to the UDP source code. Should the change of the man page go with the first commit, with the second commit or be in a third commit?

@markj I'll commit the changes to the dtrace scripts separately from the changes to the UDP source code. Should the change of the man page go with the first commit, with the second commit or be in a third commit?

I think the man page change and kernel change can be committed together. Thanks!

share/man/man4/dtrace_udp.4
27 ↗(On Diff #45378)

I think the "th" suffix is supposed to be omitted.

share/man/man4/dtrace_udp.4
51 ↗(On Diff #45378)

I would like to request an Oxford comma after "invalid", followed by a line-break before "or" (e.g., below):

the length is invalid,
or the checksum is wrong.

Address comments related to the man-page.

share/man/man4/dtrace_udp.4
27 ↗(On Diff #45378)

Fixed.

51 ↗(On Diff #45378)

Fixed.

If it's OK, I'd like to run some tests with dwatch to make sure that I don't need to patch my code for any change in functionality. I don't suspect this to be the case, but just want to make sure. If you're fine to wait another day or two for my testing, I'll definitely stamp approval

@dteske Did you had a chance to test it?

@dteske Did you had a chance to test it?

Not yet. Working on it today. It's not just a 30-second test considering I have to recompile the kernel in the middle of the A/B test.

I have used both your attached udpio.d (taken from dtrace_udp(4) in the EXAMPLES section) as well as:

dwatch -X udp

in the base system to confirm the pre-patch state. A probe fires when nc in the given test case of:

nc -u 127.0.0.1 1234

sends the datagram but no probe fires for the receive.

Going further, I compared UDP to TCP. The command:

dwatch -t 'this->lport==1234 || this->rport==1234' -FX tcp tcp:::send,tcp:::receive

combined with the test:

nc 127.0.0.1 1234

shows that the TCP receive probe fires in the context of the kernel interrupt thread (intr, commonly pid 12).

After patch, I will expect dwatch to catch intr processing the datagram. Now I have to recompile my kernel for the next test.

cddl/contrib/opensolaris/cmd/dtrace/test/tst/common/ip/tst.ipv4localudp.ksh
48 ↗(On Diff #45388)

It looks like the original (pre-patch) code had a typo. Could you do us a favor and fix the typo while you're here? s/elicts/elicits/

cddl/contrib/opensolaris/cmd/dtrace/test/tst/common/ip/tst.ipv4localudp.ksh
48 ↗(On Diff #45388)

Have it in my local copy...

Aside from testing with what comes with FreeBSD base, I also wanted to make sure that this change would be handled well by my DTrace network visualizer (sysutils/dwatch-gource in ports).

dteske@vm11 ~ $ dwatch -t 'this->family=="udp"' -X gource-net-raw
INFO Sourcing gource-net-raw profile [found in /usr/local/libexec/dwatch]
INFO Watching 'tcp:::debug-user, tcp:::state-change, udp:::send, fbt::soreceive_dgram:entry' ...
INFO Setting test: this->family=="udp"
2018 Jul 17 09:02:24 1001.1001 nc[98845]: 1531843344 udp 127.0.0.1:62235 SEND 127.0.0.1:1234 12 bytes

Here, I expect that after the patch I will still not see the RECV (desired) because it is not processed by an application (my network visualizer wants to know the application that is receiving the datagram and since udp:::receive only fires in the context of execname == "intr", the gource-net-raw profile instsead watches fbt::soreceive_dgram:entry with a predicate that indicates it is talking to the same socket).

Testing was successful. Everything worked as expected. Cheers.

Worth mentioning, I tested on stable/11 (for MFC consideration).

This revision was not accepted when it landed; it landed in state Needs Review.Jul 20 2018, 3:32 PM
This revision was automatically updated to reflect the committed changes.