Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
From:   Song Liu <songliubraving@fb.com>
To:     Okash Khawaja <osk@fb.com>
CC:     Daniel Borkmann <daniel@iogearbox.net>, Martin Lau <kafai@fb.com>,
        "Alexei Starovoitov" <ast@kernel.org>, Yonghong Song <yhs@fb.com>,
        Quentin Monnet <quentin.monnet@netronome.com>,
        Jakub Kicinski <jakub.kicinski@netronome.com>,
        "David S. Miller" <davem@davemloft.net>,
        Networking <netdev@vger.kernel.org>,
        Kernel Team <Kernel-team@fb.com>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH bpf-next 1/3] bpf: btf: export btf types and name by
 offset from lib
Thread-Topic: [PATCH bpf-next 1/3] bpf: btf: export btf types and name by
 offset from lib
Thread-Index: AQHUCNbnxSXMFV+WbUWxh41B5ch9i6RpvaUAgAACYwCAAAnSgA==
Date:   Wed, 20 Jun 2018 23:24:00 +0000
Message-ID: <599DC237-386E-495A-B205-2C76A7AFEDFF@fb.com>
References: <20180620203051.223156973@fb.com>
 <20180620203702.996737905@fb.com>
 <AD663533-9EF3-47EE-A2BA-87E1938776BA@fb.com> <20180620224849.GA18488@w1t1fb>
In-Reply-To: <20180620224849.GA18488@w1t1fb>
Accept-Language: en-US
Content-Language: en-US
received-spf: None (protection.outlook.com: fb.com does not designate
 permitted sender hosts)
spamdiagnosticoutput: 1:99
spamdiagnosticmetadata: NSPM
Content-Type: text/plain; charset="us-ascii"
Content-ID: <F8B2845C035900449331B3793A03CCFC@namprd15.prod.outlook.com>
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-Network-Message-Id: 16bc9940-5590-4766-f529-08d5d704e74f
X-MS-Exchange-CrossTenant-originalarrivaltime: 20 Jun 2018 23:24:00.2260
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 8ae927fe-1255-47a7-a2af-5f3a069daaa2
X-MS-Exchange-Transport-CrossTenantHeadersStamped: MWHPR15MB1357
X-OriginatorOrg: fb.com
X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:,, definitions=2018-06-20_10:,,
 signatures=0
X-Proofpoint-Spam-Reason: safe
X-FB-Internal: Safe
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org


> On Jun 20, 2018, at 3:48 PM, Okash Khawaja <osk@fb.com> wrote:
>=20
> On Wed, Jun 20, 2018 at 11:40:19PM +0100, Song Liu wrote:
>>=20
>>=20
>>> On Jun 20, 2018, at 1:30 PM, Okash Khawaja <osk@fb.com> wrote:
>>>=20
>>> This patch introduces btf__resolve_type() function and exports two
>>> existing functions from libbpf. btf__resolve_type follows modifier
>>> types like const and typedef until it hits a type which actually takes
>>> up memory, and then returns it. This function follows similar pattern
>>> to btf__resolve_size but instead of computing size, it just returns
>>> the type.
>>>=20
>>> These  functions will be used in the followig patch which parses
>>> information inside array of `struct btf_type *`. btf_name_by_offset is
>>> used for printing variable names.
>>>=20
>>> Signed-off-by: Okash Khawaja <osk@fb.com>
>>> Acked-by: Martin KaFai Lau <kafai@fb.com>
>>>=20
>>> ---
>>> tools/lib/bpf/btf.c |   65 ++++++++++++++++++++++++++++++++++++--------=
--------
>>> tools/lib/bpf/btf.h |    3 ++
>>> 2 files changed, 48 insertions(+), 20 deletions(-)
>>>=20
>>> --- a/tools/lib/bpf/btf.c
>>> +++ b/tools/lib/bpf/btf.c
>>> @@ -17,6 +17,11 @@
>>>=20
>>> #define BTF_MAX_NR_TYPES 65535
>>>=20
>>> +#define IS_MODIFIER(k) (((k) =3D=3D BTF_KIND_TYPEDEF) || \
>>> +		((k) =3D=3D BTF_KIND_VOLATILE) || \
>>> +		((k) =3D=3D BTF_KIND_CONST) || \
>>> +		((k) =3D=3D BTF_KIND_RESTRICT))
>>> +
>>> static struct btf_type btf_void;
>>>=20
>>> struct btf {
>>> @@ -33,14 +38,6 @@ struct btf {
>>> 	int fd;
>>> };
>>>=20
>>> -static const char *btf_name_by_offset(const struct btf *btf, uint32_t =
offset)
>>> -{
>>> -	if (offset < btf->hdr->str_len)
>>> -		return &btf->strings[offset];
>>> -	else
>>> -		return NULL;
>>> -}
>>> -
>>> static int btf_add_type(struct btf *btf, struct btf_type *t)
>>> {
>>> 	if (btf->types_size - btf->nr_types < 2) {
>>> @@ -190,15 +187,6 @@ static int btf_parse_type_sec(struct btf
>>> 	return 0;
>>> }
>>>=20
>>> -static const struct btf_type *btf_type_by_id(const struct btf *btf,
>>> -					     uint32_t type_id)
>>> -{
>>> -	if (type_id > btf->nr_types)
>>> -		return NULL;
>>> -
>>> -	return btf->types[type_id];
>>> -}
>>=20
>> nit: Why do we need to move these two functions to later of the file?=20
> No real reason. It looked like this file was following a convention of
> keeping statics together. I'll put them back in place if no one objects.

I see. Let's keep this patch as-is then.=20

Thanks,
Song

>=20
>>=20
>> Other than this,
>>=20
>> Acked-by: Song Liu <songliubraving@fb.com>
> Thanks
>=20
>>=20
>>> -
>>> static bool btf_type_is_void(const struct btf_type *t)
>>> {
>>> 	return t =3D=3D &btf_void || BTF_INFO_KIND(t->info) =3D=3D BTF_KIND_FW=
D;
>>> @@ -234,7 +222,7 @@ int64_t btf__resolve_size(const struct b
>>> 	int64_t size =3D -1;
>>> 	int i;
>>>=20
>>> -	t =3D btf_type_by_id(btf, type_id);
>>> +	t =3D btf__type_by_id(btf, type_id);
>>> 	for (i =3D 0; i < MAX_RESOLVE_DEPTH && !btf_type_is_void_or_null(t);
>>> 	     i++) {
>>> 		size =3D btf_type_size(t);
>>> @@ -259,7 +247,7 @@ int64_t btf__resolve_size(const struct b
>>> 			return -EINVAL;
>>> 		}
>>>=20
>>> -		t =3D btf_type_by_id(btf, type_id);
>>> +		t =3D btf__type_by_id(btf, type_id);
>>> 	}
>>>=20
>>> 	if (size < 0)
>>> @@ -271,6 +259,26 @@ int64_t btf__resolve_size(const struct b
>>> 	return nelems * size;
>>> }
>>>=20
>>> +int32_t btf__resolve_type(const struct btf *btf, uint32_t type_id)
>>> +{
>>> +	const struct btf_type *t;
>>> +	int depth =3D 0;
>>> +
>>> +	t =3D btf__type_by_id(btf, type_id);
>>> +	while (depth < MAX_RESOLVE_DEPTH &&
>>> +			!btf_type_is_void_or_null(t) &&
>>> +			IS_MODIFIER(BTF_INFO_KIND(t->info))) {
>>> +		type_id =3D t->type;
>>> +		t =3D btf__type_by_id(btf, type_id);
>>> +		depth++;
>>> +	}
>>> +
>>> +	if (depth =3D=3D MAX_RESOLVE_DEPTH || btf_type_is_void_or_null(t))
>>> +		return -EINVAL;
>>> +
>>> +	return type_id;
>>> +}
>>> +
>>> int32_t btf__find_by_name(const struct btf *btf, const char *type_name)
>>> {
>>> 	uint32_t i;
>>> @@ -280,7 +288,7 @@ int32_t btf__find_by_name(const struct b
>>>=20
>>> 	for (i =3D 1; i <=3D btf->nr_types; i++) {
>>> 		const struct btf_type *t =3D btf->types[i];
>>> -		const char *name =3D btf_name_by_offset(btf, t->name_off);
>>> +		const char *name =3D btf__name_by_offset(btf, t->name_off);
>>>=20
>>> 		if (name && !strcmp(type_name, name))
>>> 			return i;
>>> @@ -371,3 +379,20 @@ int btf__fd(const struct btf *btf)
>>> {
>>> 	return btf->fd;
>>> }
>>> +
>>> +const char *btf__name_by_offset(const struct btf *btf, uint32_t offset=
)
>>> +{
>>> +	if (offset < btf->hdr->str_len)
>>> +		return &btf->strings[offset];
>>> +	else
>>> +		return NULL;
>>> +}
>>> +
>>> +const struct btf_type *btf__type_by_id(const struct btf *btf,
>>> +					     uint32_t type_id)
>>> +{
>>> +	if (type_id > btf->nr_types)
>>> +		return NULL;
>>> +
>>> +	return btf->types[type_id];
>>> +}
>>> --- a/tools/lib/bpf/btf.h
>>> +++ b/tools/lib/bpf/btf.h
>>> @@ -17,6 +17,9 @@ void btf__free(struct btf *btf);
>>> struct btf *btf__new(uint8_t *data, uint32_t size, btf_print_fn_t err_l=
og);
>>> int32_t btf__find_by_name(const struct btf *btf, const char *type_name)=
;
>>> int64_t btf__resolve_size(const struct btf *btf, uint32_t type_id);
>>> +int32_t btf__resolve_type(const struct btf *btf, uint32_t type_id);
>>> int btf__fd(const struct btf *btf);
>>> +const char *btf__name_by_offset(const struct btf *btf, uint32_t offset=
);
>>> +const struct btf_type *btf__type_by_id(const struct btf *btf, uint32_t=
 type_id);
>>>=20
>>> #endif