From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752259AbdLGXBE (ORCPT ); Thu, 7 Dec 2017 18:01:04 -0500 Received: from mail-wr0-f193.google.com ([209.85.128.193]:34285 "EHLO mail-wr0-f193.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751043AbdLGXBC (ORCPT ); Thu, 7 Dec 2017 18:01:02 -0500 X-Google-Smtp-Source: AGs4zMbjpXHSLDqFcR3DdiHDPl8Bzq97NyAGgua8i3roFm84IR88HaS+Ub7WLmddPx3LmdE8EISk8FG+Gm2DX+Th0bg= MIME-Version: 1.0 In-Reply-To: <20171207142306.285be14c@cakuba.netronome.com> References: <20171207183909.16240-1-guro@fb.com> <20171207183909.16240-5-guro@fb.com> <20171207142306.285be14c@cakuba.netronome.com> From: Philippe Ombredanne Date: Fri, 8 Dec 2017 00:00:21 +0100 Message-ID: Subject: Re: [PATCH v2 net-next 4/4] bpftool: implement cgroup bpf operations To: Roman Gushchin Cc: Jakub Kicinski , netdev@vger.kernel.org, LKML , kernel-team@fb.com, ast@kernel.org, daniel@iogearbox.net, kafai@fb.com, Quentin Monnet , David Ahern Content-Type: text/plain; charset="UTF-8" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Roman, On Thu, Dec 7, 2017 at 11:23 PM, Jakub Kicinski wrote: > On Thu, 7 Dec 2017 18:39:09 +0000, Roman Gushchin wrote: >> This patch adds basic cgroup bpf operations to bpftool: >> cgroup list, attach and detach commands. >> >> Usage is described in the corresponding man pages, >> and examples are provided. >> >> Syntax: >> $ bpftool cgroup list CGROUP >> $ bpftool cgroup attach CGROUP ATTACH_TYPE PROG [ATTACH_FLAGS] >> $ bpftool cgroup detach CGROUP ATTACH_TYPE PROG >> >> Signed-off-by: Roman Gushchin > > Looks good, a few very minor nits/questions below. > >> diff --git a/tools/bpf/bpftool/cgroup.c b/tools/bpf/bpftool/cgroup.c >> new file mode 100644 >> index 000000000000..88d67f74313f >> --- /dev/null >> +++ b/tools/bpf/bpftool/cgroup.c >> @@ -0,0 +1,305 @@ >> +/* >> + * Copyright (C) 2017 Facebook >> + * >> + * This program is free software; you can redistribute it and/or >> + * modify it under the terms of the GNU General Public License >> + * as published by the Free Software Foundation; either version >> + * 2 of the License, or (at your option) any later version. >> + * >> + * Author: Roman Gushchin >> + */ Have you considered using the new SPDX ids instead? e.g. may be something like: // SPDX-License-Identifier: GPL-2.0+ // Copyright (C) 2017 Facebook // Author: Roman Gushchin Don't you love it with less boilerplate and a better code/comments ratio? BTW the comment style may surprise you here: this is a suggestion, but not just. Check the posts from Linus on this topic and Thomas's doc patches for the rationale. Thank you for your kind consideration! -- Cordially Philippe Ombredanne