Date: Mon, 21 Nov 2011 18:28:10 +1100 From: Lawrence Stewart <lstewart@freebsd.org> To: Julien Ridoux <jrid@cubinlab.ee.unimelb.edu.au> Cc: svn-src-head@FreeBSD.org, svn-src-all@FreeBSD.org, src-committers@FreeBSD.org, Ben Kaduk <minimarmot@gmail.com> Subject: Re: svn commit: r227778 - head/sys/net Message-ID: <4EC9FD8A.5040401@freebsd.org> In-Reply-To: <648D11A8-3636-49E5-BF20-83E4EA87242C@cubinlab.ee.unimelb.edu.au> References: <201111210417.pAL4HOdi023556@svn.freebsd.org> <CAK2BMK4DP=japDufnbMUgqMgmL7rRye4wMrwqzHePyreNwiu-Q@mail.gmail.com> <4EC9E408.9000304@freebsd.org> <648D11A8-3636-49E5-BF20-83E4EA87242C@cubinlab.ee.unimelb.edu.au>
next in thread | previous in thread | raw e-mail | index | archive | help
On 11/21/11 17:18, Julien Ridoux wrote: > > On 21/11/2011, at 4:39 PM, Lawrence Stewart wrote: > >> On 11/21/11 16:12, Ben Kaduk wrote: >>> On Sun, Nov 20, 2011 at 11:17 PM, Lawrence >>> Stewart<lstewart@freebsd.org> wrote: >>>> Author: lstewart Date: Mon Nov 21 04:17:24 2011 New Revision: >>>> 227778 URL: http://svn.freebsd.org/changeset/base/227778 >>>> >>>> Log: - When feed-forward clock support is compiled in, change >>>> the BPF header to contain both a regular timestamp obtained >>>> from the system clock and the current feed-forward ffcounter >>>> value. This enables new possibilities including >>> >>> Is it really necessary to make the ABI dependent on a kernel >>> configuration option? This causes all sorts of headaches if >>> loadable modules ever want to use that ABI, something that we >>> just ran into with vm_page_t and friends and had a long thread on >>> -current about. >> >> Fair question. Julien, if pcap and other consumers will happily >> ignore the new ffcount_stamp member in the bpf header, is there any >> reason to conditionally add the ffcounter into the header struct? > > It is a valid question indeed. The feedback I have received so far > was to not have the feed-forward clock support be a default kernel > configuration option. What follows is based on this assumption. > > The commit (r227747) introduces sysctl that are conditioned by the > same "FFCLOCK" kernel configuration option. If a loadable module > tests for the presence of this sysctl, it will know if the > ffcount_stamp member is available. Is it too much of a hack? > > Alternatively, if the ffcounter is added to the bpf header > unconditionally, the ffcount_stamp member can be set to 0. Loadable > modules will then see a consistent ABI but will retrieve a > meaningless value. > > I am not sure which option makes more sense, any preference? If I understand the issues correctly, I think the appropriate path forward is to remove the conditional change to the bpf header and have ffcount_stamp become a permanent member of the struct. We'll just leave the member uninitialised in the !FFCLOCK case. This change will make the patch un-MFCable, but I think that's ok. As to the issue of how a kernel module would detect if it's being loaded into a FFCLOCK enabled kernel, why wouldn't we expect modules to "#include opt_ffclock.h" and conditionally compile code based on FFCLOCK being defined? Is there a use case for run-time (as opposed to compile-time) module detection of feed-forward clock capabilities? Cheers, Lawrence
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?4EC9FD8A.5040401>