Page MenuHomeFreeBSD

Add dtrace network provider for UDP-Lite
ClosedPublic

Authored by tuexen on Jul 21 2018, 3:32 AM.

Details

Summary

Currently UDP-Lite packets fire UDP send/recv probes, which is incorrect. Implement a UDP-Lite network provider to handle them.

Test Plan

Use the two tests included in this patch.

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 21 2018, 3:32 AM
tuexen edited the summary of this revision. (Show Details)Jul 21 2018, 3:33 AM

@markj Please let me know your preferred commit split for the files in this review.

dteske added a comment.EditedJul 21 2018, 4:43 AM

Can you expand your patch to include the creation of a corresponding dwatch module? It's easy ...

cd cddl/usr.sbin/dwatch/libexec
sed -e 's/udp/&lite/g;s/length/coverage/g' udp > udplite
echo "$( awk '(line=$1)||1;NF==2&&sub(/udp/,"&lite");/^LINKS/&&gsub(/udp/,"&lite"){_[n++]=$0}/udplite-send/{for(x=0;x<n;)print _[x++]}' Makefile )" > Makefile

Can you expand your patch to include the creation of a corresponding dwatch module? It's easy ...

cd cddl/usr.sbin/dwatch/libexec
sed -e 's/udp/&lite/g;s/length/coverage/g' udp > udplite
echo "$( awk '(line=$1)||1;NF==2&&sub(/udp/,"&lite");/^LINKS/&&gsub(/udp/,"&lite"){_[n++]=$0}/udplite-send/{for(x=0;x<n;)print _[x++]}' Makefile )" > Makefile

Not that easy:

tuexen@head:~/head/cddl/usr.sbin/dwatch/libexec % echo "$( awk '(line=$1)||1;NF==2&&sub(/udp/,"&lite");/^LINKS/&&gsub(/udp/,"&lite"){_[n++]=$0}/udplite-send/{for(x=0;x<n;)print _[x++]}' Makefile )" > Makefile
Illegal variable name.

Can you expand your patch to include the creation of a corresponding dwatch module? It's easy ...

cd cddl/usr.sbin/dwatch/libexec
sed -e 's/udp/&lite/g;s/length/coverage/g' udp > udplite
echo "$( awk '(line=$1)||1;NF==2&&sub(/udp/,"&lite");/^LINKS/&&gsub(/udp/,"&lite"){_[n++]=$0}/udplite-send/{for(x=0;x<n;)print _[x++]}' Makefile )" > Makefile

Not that easy:

tuexen@head:~/head/cddl/usr.sbin/dwatch/libexec % echo "$( awk '(line=$1)||1;NF==2&&sub(/udp/,"&lite");/^LINKS/&&gsub(/udp/,"&lite"){_[n++]=$0}/udplite-send/{for(x=0;x<n;)print _[x++]}' Makefile )" > Makefile
Illegal variable name.

Bah, foiled by [t]csh. Use this:

sh -c 'echo "$( awk '\''(line=$1)||1;NF==2&&sub(/udp/,"&lite");/^LINKS/&&gsub(/udp/,"&lite"){_[n++]=$0}/udplite-send/{for(x=0;x<n;)print _[x++]}'\'' Makefile )" > Makefile'
tuexen updated this revision to Diff 45645.Jul 21 2018, 6:16 AM

Added dwatch script as provided by dteske@.

Can you expand your patch to include the creation of a corresponding dwatch module? It's easy ...

cd cddl/usr.sbin/dwatch/libexec
sed -e 's/udp/&lite/g;s/length/coverage/g' udp > udplite
echo "$( awk '(line=$1)||1;NF==2&&sub(/udp/,"&lite");/^LINKS/&&gsub(/udp/,"&lite"){_[n++]=$0}/udplite-send/{for(x=0;x<n;)print _[x++]}' Makefile )" > Makefile

Not that easy:

tuexen@head:~/head/cddl/usr.sbin/dwatch/libexec % echo "$( awk '(line=$1)||1;NF==2&&sub(/udp/,"&lite");/^LINKS/&&gsub(/udp/,"&lite"){_[n++]=$0}/udplite-send/{for(x=0;x<n;)print _[x++]}' Makefile )" > Makefile
Illegal variable name.

Bah, foiled by [t]csh. Use this:

sh -c 'echo "$( awk '\''(line=$1)||1;NF==2&&sub(/udp/,"&lite");/^LINKS/&&gsub(/udp/,"&lite"){_[n++]=$0}/udplite-send/{for(x=0;x<n;)print _[x++]}'\'' Makefile )" > Makefile'

Works. Thank you very much. dwatch script added.

markj added a comment.Jul 30 2018, 3:59 PM

Is there a strong reason for having a separate udplite.d and dtrace_udplite.4? The udp and udplite providers are very similar and I'd prefer not adding extra source files which are near duplicates of existing files. Aside from that the change seems fine to me.

tuexen added a comment.EditedJul 30 2018, 4:31 PM

UDP-Lite is a protocol different from UDP. So I just wanted to keep things reflecting that.. However, I can add the contents of udplite.d to udp.d.
I can also add the UDP-Lite stuff to the dtrace-udp man-page. We just should make sure that it shows up, when asking for man dtrace-udplite.

Please note that Solaris doesn't provide support for UDP-Lite in dtrace, since Solaris doesn't support UDP-Lite. That is why I tried to extrapolate how this would be done...

Let me know if you prefer the approach with merged files...

markj accepted this revision.Jul 30 2018, 5:02 PM

UDP-Lite is a protocol different from UDP. So I just wanted to keep things reflecting that.. However, I can add the contents of udplite.d to udp.d.

Right, but for the purpose of documenting and supporting two nearly identical providers, I think it's fine to merge things. The UDP and UDP-Lite protocol code is itself shared between the two.

I can also add the UDP-Lite stuff to the dtrace-udp man-page. We just should make sure that it shows up, when asking from dtrace-udplite.

We'd just need to add an MLINK from dtrace_udplite.4 to dtrace_udp.4. I think it's fine for the same man page to document two similar providers, even if there is no precedent for it.

Please note that Solaris doesn't provide support for UDP-Lite in dtrace, since Solaris doesn't support UDP-Lite. That is why I tried to extrapolate how this would be done...
Let me know if you prefer the approach with merged files...

I do, but will leave it up to you and won't insist on it since I don't work on this area much these days.

OK. Then I would like to keep the files separate. This way we don't bother people interested in UDP and not knowing about UDP-Lite with UDP-Lite. I guess these cover most users...

tuexen added a reviewer: rrs.Jul 30 2018, 6:14 PM
rrs accepted this revision.Jul 31 2018, 7:01 PM
This revision was not accepted when it landed; it landed in state Needs Review.Jul 31 2018, 10:56 PM
This revision was automatically updated to reflect the committed changes.