Date: Wed, 27 Jan 2016 17:11:47 +1100 From: Lawrence Stewart <lstewart@freebsd.org> To: Gleb Smirnoff <glebius@FreeBSD.org> Cc: src-committers@FreeBSD.org, svn-src-all@FreeBSD.org, svn-src-head@FreeBSD.org Subject: Re: svn commit: r294535 - in head/sys/netinet: . cc tcp_stacks Message-ID: <56A85FA3.50403@freebsd.org> In-Reply-To: <20160124080720.GE1004@FreeBSD.org> References: <201601212234.u0LMYpKT009948@repo.freebsd.org> <56A1D6B2.1010406@freebsd.org> <20160122181816.GB1004@FreeBSD.org> <56A31B78.2090100@freebsd.org> <20160124080720.GE1004@FreeBSD.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On 01/24/16 19:07, Gleb Smirnoff wrote: > Lawrence, > > On Sat, Jan 23, 2016 at 05:19:36PM +1100, Lawrence Stewart wrote: > L> > The problem is that cc.h (or tcp_cc.h) is already depening on many > L> > TCP types. So, the structures defined inside are not agnostic, including > L> > tcp headers before cc.h is required. > L> > L> Not in any significant way that tightly couples the API to TCP from > L> consumers' perspective. > L> > L> > The old cc.h used to include tcp.h implicitly, which is a bad style. > L> > > L> > Since many developers sorted netinet/* includes in a .c file using > L> > sort(1), that lead to cc.h always come before actual tcp includes, > L> > hiding the real requirement for tcp.h in a .c file. > L> > L> To provide some more context, I considered that choice to be a lesser > L> evil at the time. Linux had set the defacto standard macro naming and > L> location so even though I would have put the CC related defines in cc.h > L> given the choice, I wanted third-party software which checked for > L> modular CC the Linux way to work on FreeBSD. The alphabetical ordering > L> of includes is what prompted me to include tcp.h in cc.h so that cc.h > L> could happily sit at the top of the list. I probably could have just > L> moved cc.h to the cc subdir, in which case it would always come > L> logically after the TCP includes as <netinet/cc/cc.h> - perhaps that's > L> the right solution. > > Well, if the intent was to provide some sort of compatibility with Linux, > then my rename is even more valid! Look what left in the cc.h in the > Netflix source (which is kinda forward of FreeBSD): almost the whole > file is under _KERNEL. Only the tcp.h pollution left for userland. > > If Linux has cc.h and we want to put same definitions in cc.h, then > we should start new cc.h and keep this one renamed. Because if we > provided that one, we won't provide any compatibility, but would > just pollute with tcp.h. They do not have cc.h, they stuffed things into tcp.h. When I say I was trying to keep compat with Linux, I meant that I wanted to leave TCP_CA_NAME_MAX in tcp.h rather than move it to cc.h where it logically belongs and is required. This has no bearing on the naming of our cc.h file, but is the reason why I included tcp.h in cc.h, bad as that may be. > I am fine with netinet/cc/cc.h. If you agree I can run the rename. I think this is the right option. Please go ahead and move tcp_cc.h to netinet/cc/cc.h and update ifdef guard, src includes and man page accordingly. Cheers, Lawrence
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?56A85FA3.50403>