Date: Fri, 14 Jul 2023 07:20:57 -0600 From: Warner Losh <imp@bsdimp.com> To: Konstantin Belousov <kostikbel@gmail.com> Cc: tuexen@freebsd.org, src-committers@freebsd.org, dev-commits-src-all@freebsd.org, dev-commits-src-main@freebsd.org Subject: Re: git: be78a31188c5 - main - tcp: fix build issue for some cc modules Message-ID: <CANCZdfrhoRZJDV74u-9ziLFLMZ90vu41fyhL5P7GsboS9UrotA@mail.gmail.com> In-Reply-To: <ZLE2oRW3hEM5R4fv@kib.kiev.ua> References: <202307132031.36DKVYIK019476@gitrepo.freebsd.org> <ZLEwLRprzR0gSBmH@kib.kiev.ua> <0C3C0CAD-523A-4A77-8942-4316DB49EF57@freebsd.org> <ZLE2oRW3hEM5R4fv@kib.kiev.ua>
next in thread | previous in thread | raw e-mail | index | archive | help
--000000000000c4d4d00600725192 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Fri, Jul 14, 2023 at 5:51=E2=80=AFAM Konstantin Belousov <kostikbel@gmai= l.com> wrote: > On Fri, Jul 14, 2023 at 01:28:54PM +0200, tuexen@freebsd.org wrote: > > > On 14. Jul 2023, at 13:23, Konstantin Belousov <kostikbel@gmail.com> > wrote: > > > > > > On Thu, Jul 13, 2023 at 08:31:34PM +0000, Michael Tuexen wrote: > > >> The branch main has been updated by tuexen: > > >> > > >> URL: > https://cgit.FreeBSD.org/src/commit/?id=3Dbe78a31188c530c93700396ecfdb560= 4a8f22fff > > >> > > >> commit be78a31188c530c93700396ecfdb5604a8f22fff > > >> Author: Michael Tuexen <tuexen@FreeBSD.org> > > >> AuthorDate: 2023-07-13 16:56:25 +0000 > > >> Commit: Michael Tuexen <tuexen@FreeBSD.org> > > >> CommitDate: 2023-07-13 16:56:25 +0000 > > >> > > >> tcp: fix build issue for some cc modules > > >> > > >> The TCP_HHOOK option was moved from opt_inet.h to opt_global.h in > > >> > https://cgit.FreeBSD.org/src/commit/?id=3De68b3792440cac248347afe08ba5881= a00ba6523 > > >> The corresponding changes in two Makefiles were missed, which > resulted > > >> in not building cc_cdg, cc_chd, cc_hd, and cc_vegas anymore. > > >> > > >> Reported by: void@f-m.fm > > >> Reviewed by: rrs, rscheff > > >> Sponsored by: Netflix, Inc. > > >> Differential Revision: https://reviews.freebsd.org/D41010 > > >> --- > > >> sys/modules/cc/Makefile | 6 +++--- > > >> sys/modules/khelp/Makefile | 6 +++--- > > >> 2 files changed, 6 insertions(+), 6 deletions(-) > > >> > > >> diff --git a/sys/modules/cc/Makefile b/sys/modules/cc/Makefile > > >> index 3f7110024722..b595cc204481 100644 > > >> --- a/sys/modules/cc/Makefile > > >> +++ b/sys/modules/cc/Makefile > > >> @@ -8,9 +8,9 @@ SUBDIR=3D cc_newreno \ > > >> > > >> # Do we have the TCP_HHOOK symbol defined? If not, there is no point > in > > >> # building these modules by default. > > >> -# We will default to building these modules unless $OPT_INET is > defined > > >> -# and does not contain the TCP_HHOOK option. > > >> -.if defined(ALL_MODULES) || ${OPT_INET:UTCP_HHOOK:MTCP_HHOOK} !=3D = "" > > >> +# We will default to building these modules if $OPT_GLOBAL does > contain > > >> +# the TCP_HHOOK option. > > >> +.if defined(ALL_MODULES) || ${OPT_GLOBAL:UTCP_HHOOK:MTCP_HHOOK} != =3D "" > > >> SUBDIR+=3D \ > > >> cc_cdg \ > > >> cc_chd \ > > >> diff --git a/sys/modules/khelp/Makefile b/sys/modules/khelp/Makefile > > >> index 256d8838c573..c01d61541062 100644 > > >> --- a/sys/modules/khelp/Makefile > > >> +++ b/sys/modules/khelp/Makefile > > >> @@ -4,9 +4,9 @@ SUBDIR=3D > > >> > > >> # Do we have the TCP_HHOOK symbol defined? If not, there is no point > in > > >> # building this modules by default. > > >> -# We will default to building this module unless $OPT_INET is defin= ed > > >> -# and does not contain the TCP_HHOOK option. > > >> -.if defined(ALL_MODULES) || ${OPT_INET:UTCP_HHOOK:MTCP_HHOOK} !=3D = "" > > >> +# We will default to building this module if $OPT_GLOBAL does conta= in > > >> +# the TCP_HHOOK option. > > >> +.if defined(ALL_MODULES) || ${OPT_GLOBAL:UTCP_HHOOK:MTCP_HHOOK} != =3D "" > I don't see how this could possibly work reliably. OPT_GLOBAL isn't set for all builds, and I'm not sure it's set for this submake given how we set them up= . > > >> SUBDIR+=3D h_ertt > > >> .endif > > >> > > > It seems modules are actually broken for some configurations. > > Some problems are known and being worked on. > > > > Could you share your kernel conf file and tell us, in which directory > > you are running the make command? > > My config is attached. I do 'make -C sys/amd64/compile/X' for this > specific > run. > I believe this is due to the interface between the kernel and the modules changing based on kernel config, which is something we've traditionally avoided. Removing the #ifdef for the 'osd' member in tcp_var.h git diff diff --git a/sys/netinet/tcp_var.h b/sys/netinet/tcp_var.h index 587998331fbf..f6d3e2b9fa85 100644 --- a/sys/netinet/tcp_var.h +++ b/sys/netinet/tcp_var.h @@ -482,9 +482,7 @@ struct tcpcb { struct mbufq t_inpkts; /* List of saved input packets. */ struct mbufq t_outpkts; /* List of saved output packets. */ #endif -#ifdef TCP_HHOOK struct osd t_osd; /* storage for Khelp module data */ -#endif uint8_t _t_logpoint; /* Used when a BB log points is enabled */ #ifdef TCP_REQUEST_TRK /* Response tracking addons. */ fixes the problem, at least as far as building goes. The t_osd member is trivial in size, and not worth the savings that having different KBIs based on options causes (though there's other ifdefs that likely should be rethought, and t_osd likely should be moved before all the optional bits). Warner --000000000000c4d4d00600725192 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable <div dir=3D"ltr"><div dir=3D"ltr"><br></div><br><div class=3D"gmail_quote">= <div dir=3D"ltr" class=3D"gmail_attr">On Fri, Jul 14, 2023 at 5:51=E2=80=AF= AM Konstantin Belousov <<a href=3D"mailto:kostikbel@gmail.com">kostikbel= @gmail.com</a>> wrote:<br></div><blockquote class=3D"gmail_quote" style= =3D"margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding= -left:1ex">On Fri, Jul 14, 2023 at 01:28:54PM +0200, <a href=3D"mailto:tuex= en@freebsd.org" target=3D"_blank">tuexen@freebsd.org</a> wrote:<br> > > On 14. Jul 2023, at 13:23, Konstantin Belousov <<a href=3D"mai= lto:kostikbel@gmail.com" target=3D"_blank">kostikbel@gmail.com</a>> wrot= e:<br> > > <br> > > On Thu, Jul 13, 2023 at 08:31:34PM +0000, Michael Tuexen wrote:<b= r> > >> The branch main has been updated by tuexen:<br> > >> <br> > >> URL: <a href=3D"https://cgit.FreeBSD.org/src/commit/?id=3Dbe7= 8a31188c530c93700396ecfdb5604a8f22fff" rel=3D"noreferrer" target=3D"_blank"= >https://cgit.FreeBSD.org/src/commit/?id=3Dbe78a31188c530c93700396ecfdb5604= a8f22fff</a><br> > >> <br> > >> commit be78a31188c530c93700396ecfdb5604a8f22fff<br> > >> Author:=C2=A0 =C2=A0 =C2=A0Michael Tuexen <tuexen@FreeBSD.= org><br> > >> AuthorDate: 2023-07-13 16:56:25 +0000<br> > >> Commit:=C2=A0 =C2=A0 =C2=A0Michael Tuexen <tuexen@FreeBSD.= org><br> > >> CommitDate: 2023-07-13 16:56:25 +0000<br> > >> <br> > >>=C2=A0 =C2=A0tcp: fix build issue for some cc modules<br> > >> <br> > >>=C2=A0 =C2=A0The TCP_HHOOK option was moved from opt_inet.h to= opt_global.h in<br> > >>=C2=A0 =C2=A0<a href=3D"https://cgit.FreeBSD.org/src/commit/?i= d=3De68b3792440cac248347afe08ba5881a00ba6523" rel=3D"noreferrer" target=3D"= _blank">https://cgit.FreeBSD.org/src/commit/?id=3De68b3792440cac248347afe08= ba5881a00ba6523</a><br> > >>=C2=A0 =C2=A0The corresponding changes in two Makefiles were m= issed, which resulted<br> > >>=C2=A0 =C2=A0in not building cc_cdg, cc_chd, cc_hd, and cc_veg= as anymore.<br> > >> <br> > >>=C2=A0 =C2=A0Reported by:=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 <a href=3D"mailto:void@f-m.fm" target=3D"_blank">void@f-m.fm</a><br> > >>=C2=A0 =C2=A0Reviewed by:=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 rrs, rscheff<br> > >>=C2=A0 =C2=A0Sponsored by:=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0Netflix, Inc.<br> > >>=C2=A0 =C2=A0Differential Revision:=C2=A0 <a href=3D"https://r= eviews.freebsd.org/D41010" rel=3D"noreferrer" target=3D"_blank">https://rev= iews.freebsd.org/D41010</a><br> > >> ---<br> > >> sys/modules/cc/Makefile=C2=A0 =C2=A0 | 6 +++---<br> > >> sys/modules/khelp/Makefile | 6 +++---<br> > >> 2 files changed, 6 insertions(+), 6 deletions(-)<br> > >> <br> > >> diff --git a/sys/modules/cc/Makefile b/sys/modules/cc/Makefil= e<br> > >> index 3f7110024722..b595cc204481 100644<br> > >> --- a/sys/modules/cc/Makefile<br> > >> +++ b/sys/modules/cc/Makefile<br> > >> @@ -8,9 +8,9 @@ SUBDIR=3D cc_newreno \<br> > >> <br> > >> # Do we have the TCP_HHOOK symbol defined? If not, there is n= o point in<br> > >> # building these modules by default.<br> > >> -# We will default to building these modules unless $OPT_INET= is defined<br> > >> -# and does not contain the TCP_HHOOK option.<br> > >> -.if defined(ALL_MODULES) || ${OPT_INET:UTCP_HHOOK:MTCP_HHOOK= } !=3D ""<br> > >> +# We will default to building these modules if $OPT_GLOBAL d= oes contain<br> > >> +# the TCP_HHOOK option.<br> > >> +.if defined(ALL_MODULES) || ${OPT_GLOBAL:UTCP_HHOOK:MTCP_HHO= OK} !=3D ""<br> > >> SUBDIR+=3D \<br> > >> cc_cdg \<br> > >> cc_chd \<br> > >> diff --git a/sys/modules/khelp/Makefile b/sys/modules/khelp/M= akefile<br> > >> index 256d8838c573..c01d61541062 100644<br> > >> --- a/sys/modules/khelp/Makefile<br> > >> +++ b/sys/modules/khelp/Makefile<br> > >> @@ -4,9 +4,9 @@ SUBDIR=3D<br> > >> <br> > >> # Do we have the TCP_HHOOK symbol defined? If not, there is n= o point in<br> > >> # building this modules by default.<br> > >> -# We will default to building this module unless $OPT_INET i= s defined<br> > >> -# and does not contain the TCP_HHOOK option.<br> > >> -.if defined(ALL_MODULES) || ${OPT_INET:UTCP_HHOOK:MTCP_HHOOK= } !=3D ""<br> > >> +# We will default to building this module if $OPT_GLOBAL doe= s contain<br> > >> +# the TCP_HHOOK option.<br> > >> +.if defined(ALL_MODULES) || ${OPT_GLOBAL:UTCP_HHOOK:MTCP_HHO= OK} !=3D ""<br></blockquote><div><br></div><div>I don't see h= ow this could possibly work reliably. OPT_GLOBAL isn't set for all</div= ><div>builds, and I'm not sure it's set for this submake given how = we set them up.<br></div><div>=C2=A0</div><blockquote class=3D"gmail_quote"= style=3D"margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);p= adding-left:1ex"> > >> SUBDIR+=3D h_ertt<br> > >> .endif<br> > >> <br> > > It seems modules are actually broken for some configurations.<br> > Some problems are known and being worked on.<br> > <br> > Could you share your kernel conf file and tell us, in which directory<= br> > you are running the make command?<br> <br> My config is attached.=C2=A0 I do 'make -C sys/amd64/compile/X' for= this specific<br> run.<br></blockquote><div><br></div><div>I believe this is due to the inter= face between the kernel and the modules changing</div><div>based on kernel = config, which is something we've traditionally avoided. Removing the</d= iv><div>#ifdef for the 'osd' member in tcp_var.h</div><div><br></di= v><div>git diff<br>diff --git a/sys/netinet/tcp_var.h b/sys/netinet/tcp_var= .h<br>index 587998331fbf..f6d3e2b9fa85 100644<br>--- a/sys/netinet/tcp_var.= h<br>+++ b/sys/netinet/tcp_var.h<br>@@ -482,9 +482,7 @@ struct tcpcb {<br>= =C2=A0 =C2=A0 =C2=A0 =C2=A0 struct mbufq t_inpkts; =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0/* List of saved input packets. */<br>=C2=A0 =C2=A0 =C2=A0 =C2=A0= struct mbufq t_outpkts; =C2=A0 =C2=A0 =C2=A0 =C2=A0 /* List of saved outpu= t packets. */<br>=C2=A0#endif<br>-#ifdef TCP_HHOOK<br>=C2=A0 =C2=A0 =C2=A0 = =C2=A0 struct osd =C2=A0 =C2=A0 =C2=A0t_osd; =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0/* storage for Khelp module data */<br>-#endif<br>=C2=A0 =C2=A0 =C2= =A0 =C2=A0 uint8_t _t_logpoint; =C2=A0 =C2=A0/* Used when a BB log points i= s enabled */<br>=C2=A0#ifdef TCP_REQUEST_TRK<br>=C2=A0 =C2=A0 =C2=A0 =C2=A0= /* Response tracking addons. */<br></div><div><br></div><div>fixes the pro= blem, at least as far as building goes. The t_osd member</div><div>is trivi= al in size, and not worth the savings that having different KBIs</div><div>= based on options causes (though there's other ifdefs that likely should= </div><div>be rethought, and t_osd likely should be moved before all the op= tional</div><div>bits).</div><div><br></div><div>Warner<br></div></div></di= v> --000000000000c4d4d00600725192--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CANCZdfrhoRZJDV74u-9ziLFLMZ90vu41fyhL5P7GsboS9UrotA>