Re: svn commit: r307082 - in head: . sys/amd64/conf sys/arm/conf sys/arm64/conf sys/conf sys/i386/conf sys/mips/conf sys/modules/cc sys/modules/khelp sys/netinet sys/netinet/tcp_stacks sys/pc98/conf s

[Jonathan Looney]

Thanks!

Yes, that was clearly an error on my part, and your patch is an obvious
solution. I committed it at r307336.

Thanks again!

Jonathan

On Fri, Oct 14, 2016 at 8:01 PM, Shawn Webb 
wrote:

> On Wed, Oct 12, 2016 at 02:16:42AM +, Jonathan T. Looney wrote:
> > Author: jtl
> > Date: Wed Oct 12 02:16:42 2016
> > New Revision: 307082
> > URL: https://svnweb.freebsd.org/changeset/base/307082
> >
> > Log:
> >   In the TCP stack, the hhook(9) framework provides hooks for kernel
> modules
> >   to add actions that run when a TCP frame is sent or received on a TCP
> >   session in the ESTABLISHED state. In the base tree, this functionality
> is
> >   only used for the h_ertt module, which is used by the cc_cdg, cc_chd,
> cc_hd,
> >   and cc_vegas congestion control modules.
> >
> >   Presently, we incur overhead to check for hooks each time a TCP frame
> is
> >   sent or received on an ESTABLISHED TCP session.
> >
> >   This change adds a new compile-time option (TCP_HHOOK) to determine
> whether
> >   to include the hhook(9) framework for TCP. To retain backwards
> >   compatibility, I added the TCP_HHOOK option to every configuration
> file that
> >   already defined "options INET". (Therefore, this patch introduces no
> >   functional change. In order to see a functional difference, you need to
> >   compile a custom kernel without the TCP_HHOOK option.) This change will
> >   allow users to easily exclude this functionality from their kernel,
> should
> >   they wish to do so.
> >
> >   Note that any users who use a custom kernel configuration and use one
> of the
> >   congestion control modules listed above will need to add the TCP_HHOOK
> >   option to their kernel configuration.
> >
> >   Reviewed by:rrs, lstewart, hiren (previous version), sjg
> (makefiles only)
> >   Sponsored by:   Netflix
> >   Differential Revision:  https://reviews.freebsd.org/D8185
>
> This commit breaks the build when VNET is enabled. Attached is a
> candidate patch to fix.
>
> If the patch doesn't make it to the list, I've pasted it here:
> http://ix.io/1wbE
>
> Thanks,
>
> --
> Shawn Webb
> Cofounder and Security Engineer
> HardenedBSD
>
> GPG Key ID:  0x6A84658F52456EEE
> GPG Key Fingerprint: 2ABA B6BD EF6A F486 BE89  3D9E 6A84 658F 5245 6EEE
>
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r307082 - in head: . sys/amd64/conf sys/arm/conf sys/arm64/conf sys/conf sys/i386/conf sys/mips/conf sys/modules/cc sys/modules/khelp sys/netinet sys/netinet/tcp_stacks sys/pc98/conf s

[Shawn Webb]

On Wed, Oct 12, 2016 at 02:16:42AM +, Jonathan T. Looney wrote:
> Author: jtl
> Date: Wed Oct 12 02:16:42 2016
> New Revision: 307082
> URL: https://svnweb.freebsd.org/changeset/base/307082
> 
> Log:
>   In the TCP stack, the hhook(9) framework provides hooks for kernel modules
>   to add actions that run when a TCP frame is sent or received on a TCP
>   session in the ESTABLISHED state. In the base tree, this functionality is
>   only used for the h_ertt module, which is used by the cc_cdg, cc_chd, cc_hd,
>   and cc_vegas congestion control modules.
>   
>   Presently, we incur overhead to check for hooks each time a TCP frame is
>   sent or received on an ESTABLISHED TCP session.
>   
>   This change adds a new compile-time option (TCP_HHOOK) to determine whether
>   to include the hhook(9) framework for TCP. To retain backwards
>   compatibility, I added the TCP_HHOOK option to every configuration file that
>   already defined "options INET". (Therefore, this patch introduces no
>   functional change. In order to see a functional difference, you need to
>   compile a custom kernel without the TCP_HHOOK option.) This change will
>   allow users to easily exclude this functionality from their kernel, should
>   they wish to do so.
>   
>   Note that any users who use a custom kernel configuration and use one of the
>   congestion control modules listed above will need to add the TCP_HHOOK
>   option to their kernel configuration.
>   
>   Reviewed by:rrs, lstewart, hiren (previous version), sjg (makefiles 
> only)
>   Sponsored by:   Netflix
>   Differential Revision:  https://reviews.freebsd.org/D8185

This commit breaks the build when VNET is enabled. Attached is a
candidate patch to fix.

If the patch doesn't make it to the list, I've pasted it here:
http://ix.io/1wbE

Thanks,

-- 
Shawn Webb
Cofounder and Security Engineer
HardenedBSD

GPG Key ID:  0x6A84658F52456EEE
GPG Key Fingerprint: 2ABA B6BD EF6A F486 BE89  3D9E 6A84 658F 5245 6EEE
diff --git a/sys/netinet/tcp_subr.c b/sys/netinet/tcp_subr.c
index b8c9ff0..e69c3d4 100644
--- a/sys/netinet/tcp_subr.c
+++ b/sys/netinet/tcp_subr.c
@@ -742,7 +742,10 @@ tcp_init(void)
 static void
 tcp_destroy(void *unused __unused)
 {
-   int error, n;
+   int n;
+#ifdef TCP_HHOOK
+   int error;
+#endif
 
/*
 * All our processes are gone, all our sockets should be cleaned


signature.asc
Description: PGP signature

Re: svn commit: r307082 - in head: . sys/amd64/conf sys/arm/conf sys/arm64/conf sys/conf sys/i386/conf sys/mips/conf sys/modules/cc sys/modules/khelp sys/netinet sys/netinet/tcp_stacks sys/pc98/conf s

[Jonathan Looney]

On Wed, Oct 12, 2016 at 5:44 AM, Slawa Olhovchenkov  wrote:

> Do you perform estimate of performane impact of this overhead?
>
>
Somewhat, but not precisely. It will depend on (at least) a few factors:

a. The hardware
   * How fast is your CPU? Your L1/L2/L3 cache?
   * How many CPUs?
   * How smart is the hardware prefetching?
   * How fast is the memory bus?
   * etc.
b. Whether VNET is enabled
   * If VNET is enabled, the hook lookup will require more instructions to
 find the correct list
c. Which TCP stack you are using
   * The hook lookup is implemented as an external function in the
 non-default stacks, so that requires the overhead of a function call
d. The probability that the number of hooks is in your CPU cache
   * This is a combination of the hardware and the exact workload on the
 device

On a test system running a fairly heavy load of TCP traffic using the
default TCP stack, without VNET, and without any TCP hooks installed, the
input/output hhook functions accounted for approximately .075% of the
"busy" CPU cycles during one of my measurements. That certainly is not a
large number, but it is large enough to be measurable. And, I think the
hardware I used for the measurement is tuned for high-performance, so
this may be close to the "best case" measurement.

I suspect that systems with a large number of short-lived sessions will
see a larger improvement from the deletion of khelp_init_osd() from the
session setup path. (I didn't measure this.)

I also suspect that systems with VNET will see a larger improvement.

I also suspect that systems with slower memory busses, smaller caches,
etc. will see a larger improvement.

But, this is very hard to measure precisely in a generic sense. The one
concrete thing we *can* measure is that the functions add some number
of instructions to the session setup, session teardown, input, and
output code paths. If you need those instructions to achieve the desired
functionality (in this case, probably a congestion-control algorithm),
then you probably want those instructions. If not, then you may want to
delete them. This was the impetus to add a kernel option to give you
the ability to decide whether you want to include the functionality.

I hope this answer helps explain some more about the change.

Jonathan
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r307082 - in head: . sys/amd64/conf sys/arm/conf sys/arm64/conf sys/conf sys/i386/conf sys/mips/conf sys/modules/cc sys/modules/khelp sys/netinet sys/netinet/tcp_stacks sys/pc98/conf s

[Slawa Olhovchenkov]

On Wed, Oct 12, 2016 at 02:16:42AM +, Jonathan T. Looney wrote:

> Author: jtl
> Date: Wed Oct 12 02:16:42 2016
> New Revision: 307082
> URL: https://svnweb.freebsd.org/changeset/base/307082
> 
> Log:
>   In the TCP stack, the hhook(9) framework provides hooks for kernel modules
>   to add actions that run when a TCP frame is sent or received on a TCP
>   session in the ESTABLISHED state. In the base tree, this functionality is
>   only used for the h_ertt module, which is used by the cc_cdg, cc_chd, cc_hd,
>   and cc_vegas congestion control modules.
>   
>   Presently, we incur overhead to check for hooks each time a TCP frame is
>   sent or received on an ESTABLISHED TCP session.

Do you perform estimate of performane impact of this overhead?
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"