From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-3.9 required=3.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI, SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 58950C352A3 for ; Mon, 10 Feb 2020 11:52:41 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 30AD420733 for ; Mon, 10 Feb 2020 11:52:41 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (1024-bit key) header.d=cloudflare.com header.i=@cloudflare.com header.b="ZfE2dAIo" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727429AbgBJLwl (ORCPT ); Mon, 10 Feb 2020 06:52:41 -0500 Received: from mail-wm1-f67.google.com ([209.85.128.67]:38051 "EHLO mail-wm1-f67.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727029AbgBJLwk (ORCPT ); Mon, 10 Feb 2020 06:52:40 -0500 Received: by mail-wm1-f67.google.com with SMTP id a9so10247759wmj.3 for ; Mon, 10 Feb 2020 03:52:39 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=cloudflare.com; s=google; h=references:user-agent:from:to:cc:subject:in-reply-to:date :message-id:mime-version:content-transfer-encoding; bh=v0jsfdDUzAOrCN8MLFPhyS4TShYPDB3NuAvx8HFlXUo=; b=ZfE2dAIoWQCCKQbXg/IGCuHB6GFkuDWFSAKaF5Rd8iG28tRFhT6YarG+OyIF5d/bW3 hI4lfzfJaNWpYDLzVj8+1QYCgOkQ3TjWPVeDT/Gb0RPiiPDGeBcSkTiGjXyA2e2RtDhJ UuDQvKqLBc0zpechLd4ToaV5fnpnTa8KNcgmw= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:references:user-agent:from:to:cc:subject :in-reply-to:date:message-id:mime-version:content-transfer-encoding; bh=v0jsfdDUzAOrCN8MLFPhyS4TShYPDB3NuAvx8HFlXUo=; b=iDCG4L1vGiWOIIfdt553ry8PS3LAUd67D+UqZwHBVO1gmaI1K7pUcw0MLkBddMD0cL 7G8QEmxPW2Ebkv+ITwNYv1YQE0XnSDUTOChIRcGIj3BpDdq5zCvlzVdlRMkD9NcLade2 Siz/Lmk8uFEorLWzNlPYyBIaLp8EFdsMYvjJSRiKyy0iRDQIWaQCJ3VBbxmX6pTjXg11 HRYGxgtNTp0+JR1xFoJItn1BmvDCXdpseI5XzUzGIZ1zH95wGG4QQPFGtFgB/+p0L8RM DtcdQ4Bx220YUoYaZRy4f7pyQ90/7+OS9vzckw47JhiNvEQbuR35UtguwZlck5ViNAcz cKMw== X-Gm-Message-State: APjAAAXrTWLXTEEj45g6OD/DpYVllchzwZm/CTnz9y83TdUWOhuH3x5q niayYSn+JJxFis/4cQMe9aYzaA== X-Google-Smtp-Source: APXvYqy4WMchAPxTBVzuTtOmfOnSGwG4sYdP4mxAzL+6LjS3N56N26Hb0nKO0rvmB/8D4K4Gydyx2Q== X-Received: by 2002:a1c:6189:: with SMTP id v131mr14964365wmb.185.1581335558374; Mon, 10 Feb 2020 03:52:38 -0800 (PST) Received: from cloudflare.com ([88.157.168.82]) by smtp.gmail.com with ESMTPSA id i204sm244164wma.44.2020.02.10.03.52.37 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 10 Feb 2020 03:52:37 -0800 (PST) References: <20200206111652.694507-1-jakub@cloudflare.com> <20200206111652.694507-4-jakub@cloudflare.com> <87eev3aidz.fsf@cloudflare.com> <5e40d43715474_2a9a2abf5f7f85c025@john-XPS-13-9370.notmuch> User-agent: mu4e 1.1.0; emacs 26.3 From: Jakub Sitnicki To: Alexei Starovoitov , John Fastabend Cc: Daniel Borkmann , bpf , Network Development , kernel-team Subject: Re: [PATCH bpf 3/3] selftests/bpf: Test freeing sockmap/sockhash with a socket in it In-reply-to: <5e40d43715474_2a9a2abf5f7f85c025@john-XPS-13-9370.notmuch> Date: Mon, 10 Feb 2020 11:52:36 +0000 Message-ID: <87blq6accb.fsf@cloudflare.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Sender: bpf-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: bpf@vger.kernel.org On Mon, Feb 10, 2020 at 03:55 AM GMT, John Fastabend wrote: > Jakub Sitnicki wrote: >> On Sun, Feb 09, 2020 at 03:41 AM CET, Alexei Starovoitov wrote: >> > On Thu, Feb 6, 2020 at 3:28 AM Jakub Sitnicki w= rote: >> >> >> >> Commit 7e81a3530206 ("bpf: Sockmap, ensure sock lock held during tear >> >> down") introduced sleeping issues inside RCU critical sections and wh= ile >> >> holding a spinlock on sockmap/sockhash tear-down. There has to be at = least >> >> one socket in the map for the problem to surface. >> >> >> >> This adds a test that triggers the warnings for broken locking rules.= Not a >> >> fix per se, but rather tooling to verify the accompanying fixes. Run = on a >> >> VM with 1 vCPU to reproduce the warnings. >> >> >> >> Fixes: 7e81a3530206 ("bpf: Sockmap, ensure sock lock held during tear= down") >> >> Signed-off-by: Jakub Sitnicki >> > >> > selftests/bpf no longer builds for me. >> > make >> > BINARY test_maps >> > TEST-OBJ [test_progs] sockmap_basic.test.o >> > /data/users/ast/net/tools/testing/selftests/bpf/prog_tests/sockmap_bas= ic.c: >> > In function =E2=80=98connected_socket_v4=E2=80=99: >> > /data/users/ast/net/tools/testing/selftests/bpf/prog_tests/sockmap_bas= ic.c:20:11: >> > error: =E2=80=98TCP_REPAIR_ON=E2=80=99 undeclared (first use in this f= unction); did >> > you mean =E2=80=98TCP_REPAIR=E2=80=99? >> > 20 | repair =3D TCP_REPAIR_ON; >> > | ^~~~~~~~~~~~~ >> > | TCP_REPAIR >> > /data/users/ast/net/tools/testing/selftests/bpf/prog_tests/sockmap_bas= ic.c:20:11: >> > note: each undeclared identifier is reported only once for each >> > function it appears in >> > /data/users/ast/net/tools/testing/selftests/bpf/prog_tests/sockmap_bas= ic.c:29:11: >> > error: =E2=80=98TCP_REPAIR_OFF_NO_WP=E2=80=99 undeclared (first use in= this function); >> > did you mean =E2=80=98TCP_REPAIR_OPTIONS=E2=80=99? >> > 29 | repair =3D TCP_REPAIR_OFF_NO_WP; >> > | ^~~~~~~~~~~~~~~~~~~~ >> > | TCP_REPAIR_OPTIONS >> > >> > Clearly /usr/include/linux/tcp.h is too old. >> > Suggestions? >> >> Sorry for the inconvenience. I see that tcp.h header is missing under >> linux/tools/include/uapi/. > > How about we just add the couple defines needed to sockmap_basic.c I don't > see a need to pull in all of tcp.h just for a couple defines that wont > change anyways. Looking back at how this happened. test_progs.h pulls in netinet/tcp.h: # 19 "/home/jkbs/src/linux/tools/testing/selftests/bpf/test_progs.h" 2 # 1 "/usr/include/netinet/tcp.h" 1 3 4 # 92 "/usr/include/netinet/tcp.h" 3 4 A glibc header, which gained TCP_REPAIR_* constants in 2.29 [0]: $ git describe --contains 5cd7dbdea13eb302620491ef44837b17e9d39c5a glibc-2.29~510 Pulling in linux/tcp.h would conflict with struct definitions in netinet/tcp.h. So redefining the possibly missing constants, like John suggests, is the right way out. I'm not sure, though, how to protect against such mistakes in the future. Any ideas? [0] https://sourceware.org/git/?p=3Dglibc.git;a=3Dcommit;h=3D5cd7dbdea13eb3= 02620491ef44837b17e9d39c5a > >> >> I have been building against my distro kernel headers, completely >> unaware of this. This is an oversight on my side. >> >> Can I ask for a revert? I'm traveling today with limited ability to >> post patches. > > I don't think we need a full revert. > >> >> I can resubmit the test with the missing header for bpf-next once it >> reopens. > > If you are traveling I'll post a patch with the defines. Thanks, again.