Received: by 2002:a05:6358:4e97:b0:b3:742d:4702 with SMTP id ce23csp107626rwb; Wed, 10 Aug 2022 20:33:57 -0700 (PDT) X-Google-Smtp-Source: AA6agR5Wygy5PHz5H8wsELxRlUJeru6RMY340njca6w3sw7woNAvqLB5tJ1hIsmUtrOyun65lwWN X-Received: by 2002:a17:907:67b0:b0:730:679d:ac80 with SMTP id qu48-20020a17090767b000b00730679dac80mr21789349ejc.566.1660188837051; Wed, 10 Aug 2022 20:33:57 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1660188837; cv=none; d=google.com; s=arc-20160816; b=OJcanT6+qXWhvhuj+w2Y5+dPzfkngNUTLX3lqiEYcEf8f0lIAVwfSWHKjXR7/m7AbI tkHycgNqdCFrRkmo2pMDBNnfIDK/L3BoBjKICZDratRGuSXDSL2biNvjXzlaJpbcvu6p iwjEcpR/H3Mw17h6FYUqf2OTV4r9L0EaWoIsXBP99xPa8FPAOodq1j6SWbYgi8X1jclC z6vIAVQHkxU4uuIcEVEIgZFKIwZB423gybROqkx3RlSxW6G8ORsAi85zx94YfBIuYOFy V0mPdWGd8sYpOhqlEMpxuNIcNwYRO1fdLcVb+3Vh1T7sSOa87D3ZHEKWl+M3ESDCayJ2 4hiQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:cc:to:subject:message-id:date:from:in-reply-to :references:mime-version:dkim-signature; bh=rmxwVf7t1PNS4e5hrhApn9dN42pWVfDKUXBNKpe7GPE=; b=oeNuGpNYFepyoFWFQSjMd3U6GcRN+VjvmP5h6O73iP171sdprDvQi4AG+ggV5B22EJ i2w7//TIMrhcZ/1YLTXjZPZQXcoaY/ZGJi3U8VoQS2E0xIFA0RF/IsXVcQsM/BPXapOD 0KnsFroiqND4AsmtwmzvUpqJZAN6euPWEbCioIWUldWvmxEfS72EvpESMSZrkp7pnuGx oURR4eYFEZOPRvDy5eoy+ueV9g6z6VwlXdUriwF04gscHIcuzCCwCwGZsUtPeeTDIzJH AXUCqHyBmzDaocbdQqFRWj+ky5sQ1puwN7uiEI2oFmRMQsChBohBKHTrHK2lScVO6ndJ hVBg== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@google.com header.s=20210112 header.b=OGIBZRTW; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=REJECT sp=REJECT dis=NONE) header.from=google.com Return-Path: Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id he20-20020a1709073d9400b0073317fc9d6fsi2214412ejc.917.2022.08.10.20.33.31; Wed, 10 Aug 2022 20:33:57 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; dkim=pass header.i=@google.com header.s=20210112 header.b=OGIBZRTW; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=REJECT sp=REJECT dis=NONE) header.from=google.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S233943AbiHKDLE (ORCPT + 99 others); Wed, 10 Aug 2022 23:11:04 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:60036 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S233939AbiHKDLA (ORCPT ); Wed, 10 Aug 2022 23:11:00 -0400 Received: from mail-qt1-x830.google.com (mail-qt1-x830.google.com [IPv6:2607:f8b0:4864:20::830]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id E3F49E75 for ; Wed, 10 Aug 2022 20:10:58 -0700 (PDT) Received: by mail-qt1-x830.google.com with SMTP id x5so4999682qtv.9 for ; Wed, 10 Aug 2022 20:10:58 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc; bh=rmxwVf7t1PNS4e5hrhApn9dN42pWVfDKUXBNKpe7GPE=; b=OGIBZRTW1fN4NXGw414nvGJRbKag3aHq5Ziu4qj/DGZNt+s5Td7HX3QR6lPSb6HADc Datb70J9zakU4jgz+wRg4rl/IL4md+mq9H5HvaQpBOs3NQRm8bGlH75ErXDOvcIO6khE i/82PC2UyjnyduP6ecqJZpxSlRs2h+HRWC3QWdUSm3H0q5TRmo6jonEialdxv7o+UsJB k0Gpp6P5KZ2AjuOSl20G3N3QYh7HHV2UI8Xr7wAFsIWsHT4qicKzKPqa/cgWbNygpWoW 5Newq4k5GZxGmuf8H0VKL3dOlxtYdNQANQ3AdU5I/0oigyJ5bIPJclNfjuRFFPM9aTQS mafA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc; bh=rmxwVf7t1PNS4e5hrhApn9dN42pWVfDKUXBNKpe7GPE=; b=3xLNh6/7TtMEQ1m4fS0/iDKKgWVUtnPUe6XK9xUupCQ2/59cl+hf65yzYtnkdu8zRK bXMuUmFX+/URVYlHty4m/+ZEHGP+UJnWOosD99Qw/4L1cKMMaq7g0FeHy9ptuznejN28 UNe/djr+VBldkEmrWX6J3gFb7q0RPIe+AhNqB25LqI5i0jRWfWTE5lAVFfqabvwB2vMj /aExdUtPH53YM3q/yqz329dsHAd9Kiw3FDs1S1cWCrr9kuU8cKEe4EMzsiV0ya2+Z8Rg 7EB98ikjOn80aWpa27heD+/hR4QLcMOtNdB3hqgIS9fiHmP5owOjb1CBTj08/veaYNri gWPg== X-Gm-Message-State: ACgBeo3VSdlH0HJ+Z8s8vYpL4kNPzi0bXk9IJZUQFIN1nqPgQR7vFaGb jXrXxG3QsNqTapqeLabDW6hxJLict/u8tJRZamAUgg== X-Received: by 2002:ac8:57d2:0:b0:343:6785:a2e5 with SMTP id w18-20020ac857d2000000b003436785a2e5mr314856qta.299.1660187457913; Wed, 10 Aug 2022 20:10:57 -0700 (PDT) MIME-Version: 1.0 References: <20220805214821.1058337-1-haoluo@google.com> <20220805214821.1058337-5-haoluo@google.com> <20220809162325.hwgvys5n3rivuz7a@MacBook-Pro-3.local.dhcp.thefacebook.com> In-Reply-To: From: Hao Luo Date: Wed, 10 Aug 2022 20:10:47 -0700 Message-ID: Subject: Re: [PATCH bpf-next v7 4/8] bpf: Introduce cgroup iter To: Alexei Starovoitov Cc: Andrii Nakryiko , linux-kernel@vger.kernel.org, bpf@vger.kernel.org, cgroups@vger.kernel.org, netdev@vger.kernel.org, Alexei Starovoitov , Andrii Nakryiko , Daniel Borkmann , Martin KaFai Lau , Song Liu , Yonghong Song , Tejun Heo , Zefan Li , KP Singh , Johannes Weiner , Michal Hocko , Benjamin Tissoires , John Fastabend , Michal Koutny , Roman Gushchin , David Rientjes , Stanislav Fomichev , Shakeel Butt , Yosry Ahmed Content-Type: text/plain; charset="UTF-8" X-Spam-Status: No, score=-17.6 required=5.0 tests=BAYES_00,DKIMWL_WL_MED, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF, ENV_AND_HDR_SPF_MATCH,RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_PASS, T_SCC_BODY_TEXT_LINE,USER_IN_DEF_DKIM_WL,USER_IN_DEF_SPF_WL autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, Aug 9, 2022 at 11:38 AM Hao Luo wrote: > > On Tue, Aug 9, 2022 at 9:23 AM Alexei Starovoitov > wrote: > > > > On Mon, Aug 08, 2022 at 05:56:57PM -0700, Hao Luo wrote: > > > On Mon, Aug 8, 2022 at 5:19 PM Andrii Nakryiko > > > wrote: > > > > > > > > On Fri, Aug 5, 2022 at 2:49 PM Hao Luo wrote: > > > > > > > > > > Cgroup_iter is a type of bpf_iter. It walks over cgroups in four modes: > > > > > > > > > > - walking a cgroup's descendants in pre-order. > > > > > - walking a cgroup's descendants in post-order. > > > > > - walking a cgroup's ancestors. > > > > > - process only the given cgroup. > > > > > > [...] > > > > > diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h > > > > > index 59a217ca2dfd..4d758b2e70d6 100644 > > > > > --- a/include/uapi/linux/bpf.h > > > > > +++ b/include/uapi/linux/bpf.h > > > > > @@ -87,10 +87,37 @@ struct bpf_cgroup_storage_key { > > > > > __u32 attach_type; /* program attach type (enum bpf_attach_type) */ > > > > > }; > > > > > > > > > > +enum bpf_iter_order { > > > > > + BPF_ITER_ORDER_DEFAULT = 0, /* default order. */ > > > > > > > > why is this default order necessary? It just adds confusion (I had to > > > > look up source code to know what is default order). I might have > > > > missed some discussion, so if there is some very good reason, then > > > > please document this in commit message. But I'd rather not do some > > > > magical default order instead. We can set 0 to mean invalid and error > > > > out, or just do SELF as the very first value (and if user forgot to > > > > specify more fancy mode, they hopefully will quickly discover this in > > > > their testing). > > > > > > > > > > PRE/POST/UP are tree-specific orders. SELF applies on all iters and > > > yields only a single object. How does task_iter express a non-self > > > order? By non-self, I mean something like "I don't care about the > > > order, just scan _all_ the objects". And this "don't care" order, IMO, > > > may be the common case. I don't think everyone cares about walking > > > order for tasks. The DEFAULT is intentionally put at the first value, > > > so that if users don't care about order, they don't have to specify > > > this field. > > > > > > If that sounds valid, maybe using "UNSPEC" instead of "DEFAULT" is better? > > > > I agree with Andrii. > > This: > > + if (order == BPF_ITER_ORDER_DEFAULT) > > + order = BPF_ITER_DESCENDANTS_PRE; > > > > looks like an arbitrary choice. > > imo > > BPF_ITER_DESCENDANTS_PRE = 0, > > would have been more obvious. No need to dig into definition of "default". > > > > UNSPEC = 0 > > is fine too if we want user to always be conscious about the order > > and the kernel will error if that field is not initialized. > > That would be my preference, since it will match the rest of uapi/bpf.h > > > > Sounds good. In the next version, will use > > enum bpf_iter_order { > BPF_ITER_ORDER_UNSPEC = 0, > BPF_ITER_SELF_ONLY, /* process only a single object. */ > BPF_ITER_DESCENDANTS_PRE, /* walk descendants in pre-order. */ > BPF_ITER_DESCENDANTS_POST, /* walk descendants in post-order. */ > BPF_ITER_ANCESTORS_UP, /* walk ancestors upward. */ > }; > Sigh, I find that having UNSPEC=0 and erroring out when seeing UNSPEC doesn't work. Basically, if we have a non-iter prog and a cgroup_iter prog written in the same source file, I can't use bpf_object__attach_skeleton to attach them. Because the default prog_attach_fn for iter initializes `order` to 0 (that is, UNSPEC), which is going to be rejected by the kernel. In order to make bpf_object__attach_skeleton work on cgroup_iter, I think I need to use the following enum bpf_iter_order { BPF_ITER_DESCENDANTS_PRE, /* walk descendants in pre-order. */ BPF_ITER_DESCENDANTS_POST, /* walk descendants in post-order. */ BPF_ITER_ANCESTORS_UP, /* walk ancestors upward. */ BPF_ITER_SELF_ONLY, /* process only a single object. */ }; So that when calling bpf_object__attach_skeleton() on cgroup_iter, a link can be generated and the generated link defaults to pre-order walk on the whole hierarchy. Is there a better solution? > and explicitly list the values acceptable by cgroup_iter, error out if > UNSPEC is detected. > > Also, following Andrii's comments, will change BPF_ITER_SELF to > BPF_ITER_SELF_ONLY, which does seem a little bit explicit in > comparison. > > > I applied the first 3 patches to ease respin. > > Thanks! This helps! > > > Thanks!