Date: Fri, 8 Dec 2017 15:30:12 -0800
From: Jakub Kicinski <jakub.kicinski@netronome.com>
To: David Ahern <dsahern@gmail.com>
Cc: Quentin Monnet <quentin.monnet@netronome.com>,
        Roman Gushchin <guro@fb.com>, netdev@vger.kernel.org,
        linux-kernel@vger.kernel.org, kernel-team@fb.com, ast@kernel.org,
        daniel@iogearbox.net, kafai@fb.com
Subject: Re: [PATCH v2 net-next 4/4] bpftool: implement cgroup bpf
 operations
Message-ID: <20171208153012.0743ed43@cakuba.netronome.com>
In-Reply-To: <77e22817-4be2-ae4f-11b7-ebc4c51f39f3@gmail.com>
References: <20171207183909.16240-1-guro@fb.com>
        <20171207183909.16240-5-guro@fb.com>
        <d3cc2f85-f3ee-7771-c5b2-13ca6e80a910@netronome.com>
        <20171208141251.GA9458@castle>
        <0eed580a-804b-329e-7bfc-1dc5c09a1deb@netronome.com>
        <77e22817-4be2-ae4f-11b7-ebc4c51f39f3@gmail.com>
Organization: Netronome Systems, Ltd.
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Sender: linux-kernel-owner@vger.kernel.org
Content-Transfer-Encoding: 8bit
Content-Length: 1542
Lines: 29

On Fri, 8 Dec 2017 09:52:16 -0700, David Ahern wrote:
> On 12/8/17 8:39 AM, Quentin Monnet wrote:
> > I don't believe compatibility is an issue here, since the program and
> > its documentation come together (so they should stay in sync) and are
> > part of the kernel tree (so the tool should be compatible with the
> > kernel sources it comes with). My concern is that there is no way to
> > guess from the current description what the values for ATTACH_FLAG or
> > ATTACH_TYPE can be, without reading the source code of the program—which
> > is not exactly user-friendly.
> 
> The tool should be backward and forward compatible across kernel
> versions. Running a newer command on an older kernel should fail in a
> deterministic. While the tool is in the kernel tree for ease of
> development, that should not be confused with having a direct tie to any
> kernel version.
>
> I believe man pages do include kernel version descriptions in flags
> (e.g., man 7 socket -- flags are denoted with "since Linux x.y") which
> is one way to handle it with the usual caveat that vendors might have
> backported support to earlier kernels.

Let's see if I understand correctly.  We have a list of hard coded
strings which the tool will recognize (static const char * const
attach_type_strings[]).  We should put that list into the man page and
help so users know what values are possible.  And in the "verbose"
part of the man section mark each flag with kernel version it was
introduced in.

Roman, would you agree this is the best way forward?