RE: git: 5e248c23d995 - main - tcp: retain some CC signals outside of kernel scope

Go to: [ bottom of page ] [ top of archives ] [ this month ]
From: Scheffenegger, Richard <Richard.Scheffenegger_at_netapp.com>
Date: Sat, 24 Feb 2024 20:46:14 UTC
IMHO, these CC signals shouldn't be exposed outside of the kernel in the first place - but I didn't account for the fact that libstats is using them for some reason; I presume the source file living in sys/netinet would mean its consuming this in kernel scope, not as a library outside the scope of kernel.

In order to revive successful builds, I restored the previous scope of these values; I need to look what tcp_stats.c (libstat) is actually doing with these, and why.

One (internal ) change, which should be benign in the stats framework is that there used to be numerical identical CC signal identifiers, which are now unique. But that would certainly impact an old library on a newer kernel...

Sorry for not having spotted this prior to the commit and full world build - and the messy commit message.


Richard Scheffenegger


-----Original Message-----
From: John Baldwin <jhb@FreeBSD.org> 
Sent: Samstag, 24. Februar 2024 21:38
To: Richard Scheffenegger <rscheff@FreeBSD.org>; src-committers@FreeBSD.org; dev-commits-src-all@FreeBSD.org; dev-commits-src-main@FreeBSD.org
Subject: Re: git: 5e248c23d995 - main - tcp: retain some CC signals outside of kernel scope

EXTERNAL EMAIL - USE CAUTION when clicking links or attachments




On 2/24/24 12:02 PM, Richard Scheffenegger wrote:
> The branch main has been updated by rscheff:
>
> URL: 
> https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fcgit
> .freebsd.org%2Fsrc%2Fcommit%2F%3Fid%3D5e248c23d995a059d24f4784d5a256cd
> dd42e557&data=05%7C02%7CRichard.Scheffenegger%40netapp.com%7C6a08b69e9
> 9bb4faddc5308dc35788398%7C4b0911a0929b4715944bc03745165b3a%7C0%7C0%7C6
> 38444038934175414%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoi
> V2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C0%7C%7C%7C&sdata=IB7d4IsTVwJ9
> oMFofIL%2FokrEffs7mrvnLJ4NbTuuaj0%3D&reserved=0
>
> commit 5e248c23d995a059d24f4784d5a256cddd42e557
> Author:     Richard Scheffenegger <rscheff@FreeBSD.org>
> AuthorDate: 2024-02-24 20:01:54 +0000
> Commit:     Richard Scheffenegger <rscheff@FreeBSD.org>
> CommitDate: 2024-02-24 20:01:54 +0000
>
>      tcp: retain some CC signals outside of kernel scope
>
>      Summary: fix build error after 
> f74352fbcf15341accaf5a92240871f98323215d
>
>      Reviewers: #transport!
>
>      Subscribers: imp, melifaro, glebius
>
>      Differential Revision: 
> https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Frevi
> ews.freebsd.org%2FD44066&data=05%7C02%7CRichard.Scheffenegger%40netapp
> .com%7C6a08b69e99bb4faddc5308dc35788398%7C4b0911a0929b4715944bc0374516
> 5b3a%7C0%7C0%7C638444038934183348%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4w
> LjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C0%7C%7C%7C&sd
> ata=OXtukmrid5DSFwVPCxA0sZ85GjX1C%2BBAnu%2BTAVvTrbY%3D&reserved=0
> ---
>   sys/netinet/cc/cc.h | 4 +++-
>   1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/sys/netinet/cc/cc.h b/sys/netinet/cc/cc.h index 
> 5b2cb58a24a0..aac0825e5fe1 100644
> --- a/sys/netinet/cc/cc.h
> +++ b/sys/netinet/cc/cc.h
> @@ -121,12 +121,15 @@ struct cc_var {
>   #define CCF_HYSTART_CAN_SH_CWND     0x0800  /* Can hystart when going CSS -> CA slam the cwnd */
>   #define CCF_HYSTART_CONS_SSTH       0x1000  /* Should hystart use the more conservative ssthresh */
>
> +#endif /* defined(_KERNEL) || defined(_WANT_TCPCB) */
>   typedef enum {
> +#if defined(_KERNEL) || defined(_WANT_TCPCB)
>       /* ACK types passed to the ack_received() hook. */
>       CC_ACK =        0x0001, /* Regular in sequence ACK. */
>       CC_DUPACK =     0x0002, /* Duplicate ACK. */
>       CC_PARTIALACK = 0x0004, /* Not yet. */
>       CC_SACK =       0x0008, /* Not yet. */
> +#endif /* defined(_KERNEL) || defined(_WANT_TCPCB) */
>       /* Congestion signal types passed to the cong_signal() hook. */
>       CC_ECN =        0x0100, /* ECN marked packet received. */
>       CC_RTO =        0x0200, /* RTO fired. */
> @@ -138,7 +141,6 @@ typedef enum {
>        */
>       CC_SIGPRIVMASK = 0xFF000000     /* Mask to check if sig is private. */
>   } ccsignal_t;
> -#endif /* defined(_KERNEL) || defined(_WANT_TCPCB) */

Is there a good reason to not just expose all of the enum values always?

--
John Baldwin