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=-15.3 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,NICE_REPLY_A,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED, USER_AGENT_SANE_1 autolearn=unavailable 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 A6376C433E0 for ; Mon, 11 Jan 2021 21:02:27 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 5EF8622D2A for ; Mon, 11 Jan 2021 21:02:27 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1732738AbhAKVCL (ORCPT ); Mon, 11 Jan 2021 16:02:11 -0500 Received: from www62.your-server.de ([213.133.104.62]:41410 "EHLO www62.your-server.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1732725AbhAKVCE (ORCPT ); Mon, 11 Jan 2021 16:02:04 -0500 Received: from sslproxy06.your-server.de ([78.46.172.3]) by www62.your-server.de with esmtpsa (TLSv1.3:TLS_AES_256_GCM_SHA384:256) (Exim 4.92.3) (envelope-from ) id 1kz4JT-0002MR-Hb; Mon, 11 Jan 2021 22:01:19 +0100 Received: from [85.7.101.30] (helo=pc-9.home) by sslproxy06.your-server.de with esmtpsa (TLSv1.3:TLS_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1kz4JT-0002bd-Ci; Mon, 11 Jan 2021 22:01:19 +0100 Subject: Re: [PATCH bpf-next 2/2] bpf: extend bind v4/v6 selftests for mark/prio/bindtoifindex To: Yonghong Song , ast@kernel.org Cc: bpf@vger.kernel.org, netdev@vger.kernel.org References: <9dbbf51e7f6868b3e9c8610a8d49b4493fb1b50f.1610381606.git.daniel@iogearbox.net> <299c73acafd2c20d52624debb8a1e0019d85e6dd.1610381606.git.daniel@iogearbox.net> <1cf3b794-6b84-e6a4-bed3-6b72c480eafa@fb.com> From: Daniel Borkmann Message-ID: <1ba684dd-1fd8-7e71-4798-6abcfbb44eda@iogearbox.net> Date: Mon, 11 Jan 2021 22:01:18 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.7.2 MIME-Version: 1.0 In-Reply-To: <1cf3b794-6b84-e6a4-bed3-6b72c480eafa@fb.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 8bit X-Authenticated-Sender: daniel@iogearbox.net X-Virus-Scanned: Clear (ClamAV 0.102.4/26046/Mon Jan 11 13:34:14 2021) Precedence: bulk List-ID: X-Mailing-List: bpf@vger.kernel.org On 1/11/21 9:15 PM, Yonghong Song wrote: > On 1/11/21 8:17 AM, Daniel Borkmann wrote: >> Extend existing cgroup bind4/bind6 tests to add coverage for setting and >> retrieving SO_MARK, SO_PRIORITY and SO_BINDTOIFINDEX at the bind hook. >> >> Signed-off-by: Daniel Borkmann > > Ack with a minor comments below. > > Acked-by: Yonghong Song > >> --- >>   .../testing/selftests/bpf/progs/bind4_prog.c  | 41 +++++++++++++++++-- >>   .../testing/selftests/bpf/progs/bind6_prog.c  | 41 +++++++++++++++++-- >>   2 files changed, 74 insertions(+), 8 deletions(-) >> >> diff --git a/tools/testing/selftests/bpf/progs/bind4_prog.c b/tools/testing/selftests/bpf/progs/bind4_prog.c >> index c6520f21f5f5..4479ac27b1d3 100644 >> --- a/tools/testing/selftests/bpf/progs/bind4_prog.c >> +++ b/tools/testing/selftests/bpf/progs/bind4_prog.c >> @@ -29,18 +29,47 @@ static __inline int bind_to_device(struct bpf_sock_addr *ctx) >>       char veth2[IFNAMSIZ] = "test_sock_addr2"; >>       char missing[IFNAMSIZ] = "nonexistent_dev"; >>       char del_bind[IFNAMSIZ] = ""; >> +    int veth1_idx, veth2_idx; >>       if (bpf_setsockopt(ctx, SOL_SOCKET, SO_BINDTODEVICE, >> -                &veth1, sizeof(veth1))) >> +               &veth1, sizeof(veth1))) >> +        return 1; >> +    if (bpf_getsockopt(ctx, SOL_SOCKET, SO_BINDTOIFINDEX, >> +               &veth1_idx, sizeof(veth1_idx)) || !veth1_idx) >>           return 1; >>       if (bpf_setsockopt(ctx, SOL_SOCKET, SO_BINDTODEVICE, >> -                &veth2, sizeof(veth2))) >> +               &veth2, sizeof(veth2))) >> +        return 1; >> +    if (bpf_getsockopt(ctx, SOL_SOCKET, SO_BINDTOIFINDEX, >> +               &veth2_idx, sizeof(veth2_idx)) || !veth2_idx || >> +        veth1_idx == veth2_idx) >>           return 1; >>       if (bpf_setsockopt(ctx, SOL_SOCKET, SO_BINDTODEVICE, >> -                &missing, sizeof(missing)) != -ENODEV) >> +               &missing, sizeof(missing)) != -ENODEV) >> +        return 1; >> +    if (bpf_setsockopt(ctx, SOL_SOCKET, SO_BINDTOIFINDEX, >> +               &veth1_idx, sizeof(veth1_idx))) >>           return 1; >>       if (bpf_setsockopt(ctx, SOL_SOCKET, SO_BINDTODEVICE, >> -                &del_bind, sizeof(del_bind))) >> +               &del_bind, sizeof(del_bind))) >> +        return 1; >> + >> +    return 0; >> +} >> + >> +static __inline int misc_opts(struct bpf_sock_addr *ctx, int opt) >> +{ >> +    int old, tmp, new = 0xeb9f; >> + >> +    if (bpf_getsockopt(ctx, SOL_SOCKET, opt, &old, sizeof(old)) || >> +        old == new) >> +        return 1; > > Here, we assume old never equals to new. it would be good to add > a comment to explicitly state this is true. Maybe in the future > somebody will try to add more misc_opts which might have conflict > here. I thought it's obvious, but yes I can add a comment. > Alternatively, you could pass in "new" values > from user space with global variables for each option, > but that may be an overkill. Agree, that's overkill. Thanks, Daniel