From nobody Tue Jul 20 08:52:38 2021 X-Original-To: freebsd-hackers@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id D21BB12A8B12 for ; Tue, 20 Jul 2021 08:52:39 +0000 (UTC) (envelope-from zec@fer.hr) Received: from EUR05-AM6-obe.outbound.protection.outlook.com (mail-am6eur05on20608.outbound.protection.outlook.com [IPv6:2a01:111:f400:7e1b::608]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client CN "mail.protection.outlook.com", Issuer "DigiCert Cloud Services CA-1" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4GTXXl3knYz4hX0; Tue, 20 Jul 2021 08:52:39 +0000 (UTC) (envelope-from zec@fer.hr) ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=IClbjk9rHXlmEOoDXxPuKQRYP/kN/7IOjf3XqfgbFqM5R1rPd1AwRH4ARcHT1QSWrnWBI2zOPUXKUODsdmhLfSTT26aLTBuyziRpburbGFmAsxuQ3ORCW5nYjZLJCAIDT4S/5HSfOKj7ScEWcOE7jxRp6bLrDfHgbV1eXPBDznCX/ifZqVIPIAxTSaWk618SPUv4h8xTvbtB6shUGPC4P1HpWtTBPqqr2xhJP+SC7/s9ML2JDKO8bMIQ/deZPFxzeQQ/yOokqxjjsk4/0U2oyHSLPit9bHppEkXoLbf62ovg3YevbBIbBxIe+AqgbIBQP4aILU+0KTSc4QeDjvzJfg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=OnFnnrc1BsC3h5Ohn/JTKWLdeCvNa9nAiCEbLrgo1dI=; b=Doxm4WX2Db87AawVvPvVNV0g1ebQQtKNGVcenfSEymup6zIBYk/iji5jaNqKDiz/FGseLLbPIMJH/zl6ZIX3eG4nUkGXWhiLf8cWk0dJIGHNPhrBzp9AQ9PWH7rhDgftJhMnumgdEN7fLNXcusrdEu9nJkAEXJlh4w+nDwN88wruA1RbETyFxADiKREmjd+RmzxCD/Ug39sSPusCQXUYLA2Zca8yxws9ki86dASmF/X/pFTCYyrtoBgXUVs5ttcXWSuZ7ovV6fp9NyaiaGBXqhoHfKJROC8GXAMS4HKjj5mCgUr0UTCvZohVPkjHOQmCwxBLlQNq6lyYjTVm64gEFQ== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=fer.hr; dmarc=pass action=none header.from=fer.hr; dkim=pass header.d=fer.hr; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ferhr.onmicrosoft.com; s=selector2-ferhr-onmicrosoft-com; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=OnFnnrc1BsC3h5Ohn/JTKWLdeCvNa9nAiCEbLrgo1dI=; b=fXwqjXclfkXpizHkzFjYEs3H0XfgRP7ef9FM7uaX9qm8S2jqQz8Ny1eiN+Tf8FHyz1hv6l4JOLqy0tOQcVAqaLLQmyowYfjaeguEDmcAenSphgxUjVR9BtQDjEKIvi+GIXsr7Bs6q1KSTvDCMiM3xVsmJl4YxwpDwnQ/k/GYurk= Received: from VE1PR08MB4783.eurprd08.prod.outlook.com (2603:10a6:802:a9::16) by VE1PR08MB5646.eurprd08.prod.outlook.com (2603:10a6:800:1a9::19) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.4331.21; Tue, 20 Jul 2021 08:52:35 +0000 Received: from VE1PR08MB4783.eurprd08.prod.outlook.com ([fe80::71e1:6a95:a4ff:c1ee]) by VE1PR08MB4783.eurprd08.prod.outlook.com ([fe80::71e1:6a95:a4ff:c1ee%5]) with mapi id 15.20.4331.034; Tue, 20 Jul 2021 08:52:35 +0000 Date: Tue, 20 Jul 2021 10:52:38 +0200 From: Marko Zec To: Daniel O'Connor via freebsd-hackers Cc: darius@dons.net.au, Michael Tuexen Subject: Re: git: a730d82378d3 - main - tcp: fix RACK and BBR when using VIMAGE enabled kernel Message-ID: <20210720105238.1341472c@x23> In-Reply-To: References: <202107192233.16JMX0k4044018@gitrepo.freebsd.org> X-Mailer: Claws Mail 3.17.8 (GTK+ 2.24.33; amd64-portbld-freebsd11.4) Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit X-ClientProxiedBy: ZR0P278CA0110.CHEP278.PROD.OUTLOOK.COM (2603:10a6:910:20::7) To VE1PR08MB4783.eurprd08.prod.outlook.com (2603:10a6:802:a9::16) List-Id: Technical discussions relating to FreeBSD List-Archive: https://lists.freebsd.org/archives/freebsd-hackers List-Help: List-Post: List-Subscribe: List-Unsubscribe: Sender: owner-freebsd-hackers@freebsd.org MIME-Version: 1.0 X-MS-Exchange-MessageSentRepresentingType: 1 Received: from x23 (161.53.19.9) by ZR0P278CA0110.CHEP278.PROD.OUTLOOK.COM (2603:10a6:910:20::7) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.4331.24 via Frontend Transport; Tue, 20 Jul 2021 08:52:35 +0000 X-MS-PublicTrafficType: Email X-MS-Office365-Filtering-Correlation-Id: 07006901-dbb9-4750-c1b5-08d94b5bb829 X-MS-TrafficTypeDiagnostic: VE1PR08MB5646: X-Microsoft-Antispam-PRVS: X-MS-Oob-TLC-OOBClassifiers: OLM:6108; X-MS-Exchange-SenderADCheck: 1 X-MS-Exchange-AntiSpam-Relay: 0 X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: PUk/x/IfwIstq7vl69Nu0KTCyD1rvwMMDO74dTH4nhugcLvwCrLCk8bjeYFmGYYHLUOXCVzGsIvkuXhlNhkTLh81nVqkPiNmAuO9xU/h5EUtuYpXqs7pc8PUn4DjQs4B/DiqYFf3YJfs/AemgPP8gFbP6ZHg25A3uGoPk1p5XhkdLbG/yLjYWduns7lL+MOct7JfK4360WlUEN0I+Lutk6s0f0bqdHSKZD+9diVzkP/Qm6jckbwlW3C55yHpwPuuE3hUUOoXv96H2GmJKHJ5VuDfvP+0Y7N4GRlM/0LmTpqtIUldt0El2N4WRAwsVL2WbWE3JvvoUg3a2FLm28ryByMSl3GCh9SyGLqCosttpkScDnt0Zxf42oNowUuDFXfSpb2ITc8KEgsgIwu2FWtxnQixlQYuxPwHZRPqLVIkVMa5Fq88VNr8XnwgZNeWKJYDxS6ZNEhKKGQ37OTJ1h4g68TLsE0aXo1IQhTxRM+rL0ncx3Zu1rtAlmv3ZqImaPl+65hMHID2QR6dSQZFQLOvBIQiJ0ZlLygceXXDLKnizrB9aI5hj+ZJQna7524Z5PJvfdzAgh1aYIN4OFBiVfkgI0SjSPnzwajbmUNgoUJMHbycoB3I4tHsssdVZtqVmwNTecFj4DAusxr2dp8xa7yDMv8hfJncpP5mG1lt7M2PaNP4+VcOXHrXEjA5IgJclsHa4/4nLr+YY846GHYIpzBIBXVuBUVPLHtyOG8ohoe4DydpP/WtVH+pj5vEOwWzu2bH+hfkVwyflEhsZ/i8uuQVGA== X-Forefront-Antispam-Report: CIP:255.255.255.255;CTRY:;LANG:en;SCL:1;SRV:;IPV:NLI;SFV:NSPM;H:VE1PR08MB4783.eurprd08.prod.outlook.com;PTR:;CAT:NONE;SFS:(136003)(396003)(366004)(346002)(376002)(39850400004)(53546011)(186003)(9576002)(9686003)(38100700002)(86362001)(55016002)(83380400001)(52116002)(26005)(2906002)(8936002)(66476007)(5660300002)(6916009)(4326008)(33716001)(66556008)(966005)(956004)(316002)(8676002)(66946007)(786003)(1076003)(38350700002)(6496006)(478600001);DIR:OUT;SFP:1101; X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1 X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?27QOtA4t8aKc5rzlgCieIwefl/kr21ZocTa0Kj0+8cNA5Ywkg1cZ0eavXayH?= =?us-ascii?Q?XcfdpuEQjBq7I4lIGgSi5qf6/vu+Ur5xiw3A+wri67iDi1UFINU4krlwsvrd?= =?us-ascii?Q?EFeC1R6NyrgSUQvZZDFDJmz9h21nTjKNDgRKXxDxPZZd5eMSQLv3NK41fAuD?= =?us-ascii?Q?7cNzdEOkaNiZ9YAh/TMvuZl2nKnuSbGGeZmqVJpw8SGoLRW8Z51gY/s7EXO9?= =?us-ascii?Q?sJvJTRVz0XoXmR1kvZHXhV4ik4JQGArd/1nXeKJkwAkmvIN78zrls5Sk+iuc?= =?us-ascii?Q?Y/rTNxrkDbpDLAIJJijLcpQEWR3/er5qovkd3hpdwVh1OOzYZ6UiCMExhoBq?= =?us-ascii?Q?+WpRxD53OO0hWyv7fOvEm8iVzdoLGxoRmLqf5VzTtPTi3F9Z0eqNu+67lSNE?= =?us-ascii?Q?WSi+n929vS7QZkogdShrnI9jOiiq7U3tudRz8HgG5mtx0aH+Qo/0uf0sl8rG?= =?us-ascii?Q?pOAwZl+7Xbg5n2RNMR0zdLrsup7P4glNFDh+bhIL4leQ1G5oxakSIlNNW3HF?= =?us-ascii?Q?+ly1jJmMOAncm+w2sYoG1kBbJJtfKXD1czY/JmOXNdHoAX39rBlJ98fMabfI?= =?us-ascii?Q?+S8ZVJwY1P3CMDl1vOcOH5nkF/MHYs6weOOIxVYIkvFfqJjSoQSSw5nSLXwX?= =?us-ascii?Q?5V3d4cTP092uAWGyJwnGWU1xyqInbzRfNMtskQGI4FluddQAsD7d5WKIjXiR?= =?us-ascii?Q?qQFsF+7ArR2U1cG9ONPpiZYKgqEDx1Ic+Jcouq0i3ZixkGoUZAVjI7JugSDG?= =?us-ascii?Q?szWspHSToMIUpBuxgYOaMaK4kFpAi97NcYxgwCYwGdfMA18EnqwR0Iiz0/QC?= =?us-ascii?Q?NmYdvBFKe1bLxJpeT4nX9u2JugxUr+VAg0+KuOcc/G81xz8IotVBPzCMXRrQ?= =?us-ascii?Q?iiUanGpim2NHKCll98leXNg/kfwDDskMlYmqLLyQI5MDlsNDwhWJxplht8oK?= =?us-ascii?Q?F7+vL80qgULHh9GeS+4hhMiPkILuuwF7COfFYCUzWCzN/JXPSV7HTF64CHuF?= =?us-ascii?Q?S+OsY/ILixT05vbieqDWF2Dze53rThFWyvAwF7Qm8XyV+sH5KM0aucUQuIZt?= =?us-ascii?Q?c40JAQM5FqgtykhsOPuUv68uqhOLgNAgB+han7lBrWxgpXTGIniKFho0bCka?= =?us-ascii?Q?xz/cNGfdvzR+RTosu6OFbCJY2DVGXxhn8xQ4bDdFJ0WYGUbqwI+zmyg6j9yz?= =?us-ascii?Q?DySorRu6CrOUG5SemV9ZGkhb/I3aLgwA3iD0JpbcoJL6V6wfVtb1XOhywV0N?= =?us-ascii?Q?uxv7wBAPInbALCWPObYyKM9m3zxBZWcbbZ0hzQd9ZneaKZ1guxNqZJmQUq4/?= =?us-ascii?Q?K7wGWCpU94Uzt2MeXyG+EGdA?= X-OriginatorOrg: fer.hr X-MS-Exchange-CrossTenant-Network-Message-Id: 07006901-dbb9-4750-c1b5-08d94b5bb829 X-MS-Exchange-CrossTenant-AuthSource: VE1PR08MB4783.eurprd08.prod.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-OriginalArrivalTime: 20 Jul 2021 08:52:35.5336 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-CrossTenant-Id: ca71eddc-cc7b-4e5b-95bd-55b658e696be X-MS-Exchange-CrossTenant-MailboxType: HOSTED X-MS-Exchange-CrossTenant-UserPrincipalName: oq+D2nweHpS1Z2QCXWfBCLVLoMISiUTVH1LkSAiMGSycsAUXbyv+BxrR12QnFg51 X-MS-Exchange-Transport-CrossTenantHeadersStamped: VE1PR08MB5646 X-Rspamd-Queue-Id: 4GTXXl3knYz4hX0 X-Spamd-Bar: ---- Authentication-Results: mx1.freebsd.org; none X-Spamd-Result: default: False [-4.00 / 15.00]; REPLY(-4.00)[] X-ThisMailContainsUnwantedMimeParts: N On Tue, 20 Jul 2021 10:29:30 +0930 Daniel O'Connor via freebsd-hackers wrote: > > On 20 Jul 2021, at 08:03, Michael Tuexen wrote: > > > > The branch main has been updated by tuexen: > > > > URL: > > https://cgit.FreeBSD.org/src/commit/?id=a730d82378d3cdf5356775ec0c23ad2ca40c5edb > > > > diff --git a/sys/netinet/tcp_stacks/rack_bbr_common.c > > b/sys/netinet/tcp_stacks/rack_bbr_common.c index > > baa267b43752..bf93359368f9 100644 --- > > a/sys/netinet/tcp_stacks/rack_bbr_common.c +++ > > b/sys/netinet/tcp_stacks/rack_bbr_common.c @@ -508,16 +508,18 @@ > > skip_vnet: m_freem(m); > > m = m_save; > > } > > - if (no_vn == 0) > > + if (no_vn == 0) { > > CURVNET_RESTORE(); > > + } > > INP_UNLOCK_ASSERT(inp); > > return(retval); > > } > > skipped_pkt: > > m = m_save; > > } > > - if (no_vn == 0) > > + if (no_vn == 0) { > > CURVNET_RESTORE(); > > + } > > return(retval); > > } > > Not to pick on this particular commit, but does anyone know why > CURVNET_RESTORE os not defined such that it doesn't require wrapping > with {}? True, the body of CURVNET_RESTORE() macro could be redefined so that it gets enclosed in a do {...} while (0) block, most probably without any ill side-effects. One reason it was never defined in such manner is that CURVNET_ macros were never intended to be invoked conditionally, which is also clearly documented in VNET(9) . The other reason is my clumsiness... > Looking through vnet.h I see that VNET_ASSERT, > VNET_GLOBAL_EVENTHANDLER_REGISTER_TAG, and > VNET_GLOBAL_EVENTHANDLER_REGISTER have a do { } while(0) wrapper but > CURVNET_SET_QUIET, CURVNET_SET_VERBOSE, CURVNET_RESTORE, > VNET_SYSINIT, and VNET_SYSUNINIT don't. CURVNET_SET macros cannot be wrapped in a separate block because they declare a local variable which must be visible / available to the corresponding CURVNET_RESTORE(s). In this particular case, i.e. inside ctf_process_inbound_raw(), the rule that CURVNET_SET() must not be conditionally called seems to be circumvented by a local hack, in an attempt to handle a case when VNET cannot be inferred from m->m_pkthdr->rcvif->if_vnet, when m == NULL. Given that a non-NULL struct socket *so seem to be available as an argument to ctf_process_inbound_raw(), perhaps it could be possible to remove the if (no_vn == 0) hack, and instead to unconditionally invoke CURVNET_SET(so->so_vnet) on entry to ctf_process_inbound_raw()? Marko