Netdev List
 help / color / mirror / Atom feed
* [PATCH v5 bpf-next 02/14] libbpf: convert libbpf code to use new btf helpers
From: Andrii Nakryiko @ 2019-08-07 20:48 UTC (permalink / raw)
  To: bpf, netdev, ast, daniel, yhs
  Cc: andrii.nakryiko, kernel-team, Andrii Nakryiko
In-Reply-To: <20190807204843.513594-1-andriin@fb.com>

Simplify code by relying on newly added BTF helper functions.

Signed-off-by: Andrii Nakryiko <andriin@fb.com>
---
 tools/lib/bpf/btf.c      | 181 ++++++++++++++++++---------------------
 tools/lib/bpf/btf_dump.c | 138 ++++++++++-------------------
 tools/lib/bpf/libbpf.c   |  60 +++++++------
 3 files changed, 158 insertions(+), 221 deletions(-)

diff --git a/tools/lib/bpf/btf.c b/tools/lib/bpf/btf.c
index 467224feb43b..1cd4e5d67158 100644
--- a/tools/lib/bpf/btf.c
+++ b/tools/lib/bpf/btf.c
@@ -19,13 +19,6 @@
 #define BTF_MAX_NR_TYPES 0x7fffffff
 #define BTF_MAX_STR_OFFSET 0x7fffffff
 
-#define IS_MODIFIER(k) (((k) == BTF_KIND_TYPEDEF) || \
-		((k) == BTF_KIND_VOLATILE) || \
-		((k) == BTF_KIND_CONST) || \
-		((k) == BTF_KIND_RESTRICT))
-
-#define IS_VAR(k) ((k) == BTF_KIND_VAR)
-
 static struct btf_type btf_void;
 
 struct btf {
@@ -192,9 +185,9 @@ static int btf_parse_str_sec(struct btf *btf)
 static int btf_type_size(struct btf_type *t)
 {
 	int base_size = sizeof(struct btf_type);
-	__u16 vlen = BTF_INFO_VLEN(t->info);
+	__u16 vlen = btf_vlen(t);
 
-	switch (BTF_INFO_KIND(t->info)) {
+	switch (btf_kind(t)) {
 	case BTF_KIND_FWD:
 	case BTF_KIND_CONST:
 	case BTF_KIND_VOLATILE:
@@ -219,7 +212,7 @@ static int btf_type_size(struct btf_type *t)
 	case BTF_KIND_DATASEC:
 		return base_size + vlen * sizeof(struct btf_var_secinfo);
 	default:
-		pr_debug("Unsupported BTF_KIND:%u\n", BTF_INFO_KIND(t->info));
+		pr_debug("Unsupported BTF_KIND:%u\n", btf_kind(t));
 		return -EINVAL;
 	}
 }
@@ -263,7 +256,7 @@ const struct btf_type *btf__type_by_id(const struct btf *btf, __u32 type_id)
 
 static bool btf_type_is_void(const struct btf_type *t)
 {
-	return t == &btf_void || BTF_INFO_KIND(t->info) == BTF_KIND_FWD;
+	return t == &btf_void || btf_is_fwd(t);
 }
 
 static bool btf_type_is_void_or_null(const struct btf_type *t)
@@ -284,7 +277,7 @@ __s64 btf__resolve_size(const struct btf *btf, __u32 type_id)
 	t = btf__type_by_id(btf, type_id);
 	for (i = 0; i < MAX_RESOLVE_DEPTH && !btf_type_is_void_or_null(t);
 	     i++) {
-		switch (BTF_INFO_KIND(t->info)) {
+		switch (btf_kind(t)) {
 		case BTF_KIND_INT:
 		case BTF_KIND_STRUCT:
 		case BTF_KIND_UNION:
@@ -303,7 +296,7 @@ __s64 btf__resolve_size(const struct btf *btf, __u32 type_id)
 			type_id = t->type;
 			break;
 		case BTF_KIND_ARRAY:
-			array = (const struct btf_array *)(t + 1);
+			array = btf_array(t);
 			if (nelems && array->nelems > UINT32_MAX / nelems)
 				return -E2BIG;
 			nelems *= array->nelems;
@@ -334,8 +327,7 @@ int btf__resolve_type(const struct btf *btf, __u32 type_id)
 	t = 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)) ||
-		IS_VAR(BTF_INFO_KIND(t->info)))) {
+	       (btf_is_mod(t) || btf_is_typedef(t) || btf_is_var(t))) {
 		type_id = t->type;
 		t = btf__type_by_id(btf, type_id);
 		depth++;
@@ -554,11 +546,11 @@ static int compare_vsi_off(const void *_a, const void *_b)
 static int btf_fixup_datasec(struct bpf_object *obj, struct btf *btf,
 			     struct btf_type *t)
 {
-	__u32 size = 0, off = 0, i, vars = BTF_INFO_VLEN(t->info);
+	__u32 size = 0, off = 0, i, vars = btf_vlen(t);
 	const char *name = btf__name_by_offset(btf, t->name_off);
 	const struct btf_type *t_var;
 	struct btf_var_secinfo *vsi;
-	struct btf_var *var;
+	const struct btf_var *var;
 	int ret;
 
 	if (!name) {
@@ -574,12 +566,11 @@ static int btf_fixup_datasec(struct bpf_object *obj, struct btf *btf,
 
 	t->size = size;
 
-	for (i = 0, vsi = (struct btf_var_secinfo *)(t + 1);
-	     i < vars; i++, vsi++) {
+	for (i = 0, vsi = btf_var_secinfos(t); i < vars; i++, vsi++) {
 		t_var = btf__type_by_id(btf, vsi->type);
-		var = (struct btf_var *)(t_var + 1);
+		var = btf_var(t_var);
 
-		if (BTF_INFO_KIND(t_var->info) != BTF_KIND_VAR) {
+		if (!btf_is_var(t_var)) {
 			pr_debug("Non-VAR type seen in section %s\n", name);
 			return -EINVAL;
 		}
@@ -595,7 +586,8 @@ static int btf_fixup_datasec(struct bpf_object *obj, struct btf *btf,
 
 		ret = bpf_object__variable_offset(obj, name, &off);
 		if (ret) {
-			pr_debug("No offset found in symbol table for VAR %s\n", name);
+			pr_debug("No offset found in symbol table for VAR %s\n",
+				 name);
 			return -ENOENT;
 		}
 
@@ -619,7 +611,7 @@ int btf__finalize_data(struct bpf_object *obj, struct btf *btf)
 		 * is section size and global variable offset. We use
 		 * the info from the ELF itself for this purpose.
 		 */
-		if (BTF_INFO_KIND(t->info) == BTF_KIND_DATASEC) {
+		if (btf_is_datasec(t)) {
 			err = btf_fixup_datasec(obj, btf, t);
 			if (err)
 				break;
@@ -774,14 +766,13 @@ int btf__get_map_kv_tids(const struct btf *btf, const char *map_name,
 		return -EINVAL;
 	}
 
-	if (BTF_INFO_KIND(container_type->info) != BTF_KIND_STRUCT ||
-	    BTF_INFO_VLEN(container_type->info) < 2) {
+	if (!btf_is_struct(container_type) || btf_vlen(container_type) < 2) {
 		pr_warning("map:%s container_name:%s is an invalid container struct\n",
 			   map_name, container_name);
 		return -EINVAL;
 	}
 
-	key = (struct btf_member *)(container_type + 1);
+	key = btf_members(container_type);
 	value = key + 1;
 
 	key_size = btf__resolve_size(btf, key->type);
@@ -1440,10 +1431,9 @@ static struct btf_dedup *btf_dedup_new(struct btf *btf, struct btf_ext *btf_ext,
 	d->map[0] = 0;
 	for (i = 1; i <= btf->nr_types; i++) {
 		struct btf_type *t = d->btf->types[i];
-		__u16 kind = BTF_INFO_KIND(t->info);
 
 		/* VAR and DATASEC are never deduped and are self-canonical */
-		if (kind == BTF_KIND_VAR || kind == BTF_KIND_DATASEC)
+		if (btf_is_var(t) || btf_is_datasec(t))
 			d->map[i] = i;
 		else
 			d->map[i] = BTF_UNPROCESSED_ID;
@@ -1484,11 +1474,11 @@ static int btf_for_each_str_off(struct btf_dedup *d, str_off_fn_t fn, void *ctx)
 		if (r)
 			return r;
 
-		switch (BTF_INFO_KIND(t->info)) {
+		switch (btf_kind(t)) {
 		case BTF_KIND_STRUCT:
 		case BTF_KIND_UNION: {
-			struct btf_member *m = (struct btf_member *)(t + 1);
-			__u16 vlen = BTF_INFO_VLEN(t->info);
+			struct btf_member *m = btf_members(t);
+			__u16 vlen = btf_vlen(t);
 
 			for (j = 0; j < vlen; j++) {
 				r = fn(&m->name_off, ctx);
@@ -1499,8 +1489,8 @@ static int btf_for_each_str_off(struct btf_dedup *d, str_off_fn_t fn, void *ctx)
 			break;
 		}
 		case BTF_KIND_ENUM: {
-			struct btf_enum *m = (struct btf_enum *)(t + 1);
-			__u16 vlen = BTF_INFO_VLEN(t->info);
+			struct btf_enum *m = btf_enum(t);
+			__u16 vlen = btf_vlen(t);
 
 			for (j = 0; j < vlen; j++) {
 				r = fn(&m->name_off, ctx);
@@ -1511,8 +1501,8 @@ static int btf_for_each_str_off(struct btf_dedup *d, str_off_fn_t fn, void *ctx)
 			break;
 		}
 		case BTF_KIND_FUNC_PROTO: {
-			struct btf_param *m = (struct btf_param *)(t + 1);
-			__u16 vlen = BTF_INFO_VLEN(t->info);
+			struct btf_param *m = btf_params(t);
+			__u16 vlen = btf_vlen(t);
 
 			for (j = 0; j < vlen; j++) {
 				r = fn(&m->name_off, ctx);
@@ -1801,16 +1791,16 @@ static long btf_hash_enum(struct btf_type *t)
 /* Check structural equality of two ENUMs. */
 static bool btf_equal_enum(struct btf_type *t1, struct btf_type *t2)
 {
-	struct btf_enum *m1, *m2;
+	const struct btf_enum *m1, *m2;
 	__u16 vlen;
 	int i;
 
 	if (!btf_equal_common(t1, t2))
 		return false;
 
-	vlen = BTF_INFO_VLEN(t1->info);
-	m1 = (struct btf_enum *)(t1 + 1);
-	m2 = (struct btf_enum *)(t2 + 1);
+	vlen = btf_vlen(t1);
+	m1 = btf_enum(t1);
+	m2 = btf_enum(t2);
 	for (i = 0; i < vlen; i++) {
 		if (m1->name_off != m2->name_off || m1->val != m2->val)
 			return false;
@@ -1822,8 +1812,7 @@ static bool btf_equal_enum(struct btf_type *t1, struct btf_type *t2)
 
 static inline bool btf_is_enum_fwd(struct btf_type *t)
 {
-	return BTF_INFO_KIND(t->info) == BTF_KIND_ENUM &&
-	       BTF_INFO_VLEN(t->info) == 0;
+	return btf_is_enum(t) && btf_vlen(t) == 0;
 }
 
 static bool btf_compat_enum(struct btf_type *t1, struct btf_type *t2)
@@ -1843,8 +1832,8 @@ static bool btf_compat_enum(struct btf_type *t1, struct btf_type *t2)
  */
 static long btf_hash_struct(struct btf_type *t)
 {
-	struct btf_member *member = (struct btf_member *)(t + 1);
-	__u32 vlen = BTF_INFO_VLEN(t->info);
+	const struct btf_member *member = btf_members(t);
+	__u32 vlen = btf_vlen(t);
 	long h = btf_hash_common(t);
 	int i;
 
@@ -1864,16 +1853,16 @@ static long btf_hash_struct(struct btf_type *t)
  */
 static bool btf_shallow_equal_struct(struct btf_type *t1, struct btf_type *t2)
 {
-	struct btf_member *m1, *m2;
+	const struct btf_member *m1, *m2;
 	__u16 vlen;
 	int i;
 
 	if (!btf_equal_common(t1, t2))
 		return false;
 
-	vlen = BTF_INFO_VLEN(t1->info);
-	m1 = (struct btf_member *)(t1 + 1);
-	m2 = (struct btf_member *)(t2 + 1);
+	vlen = btf_vlen(t1);
+	m1 = btf_members(t1);
+	m2 = btf_members(t2);
 	for (i = 0; i < vlen; i++) {
 		if (m1->name_off != m2->name_off || m1->offset != m2->offset)
 			return false;
@@ -1890,7 +1879,7 @@ static bool btf_shallow_equal_struct(struct btf_type *t1, struct btf_type *t2)
  */
 static long btf_hash_array(struct btf_type *t)
 {
-	struct btf_array *info = (struct btf_array *)(t + 1);
+	const struct btf_array *info = btf_array(t);
 	long h = btf_hash_common(t);
 
 	h = hash_combine(h, info->type);
@@ -1908,13 +1897,13 @@ static long btf_hash_array(struct btf_type *t)
  */
 static bool btf_equal_array(struct btf_type *t1, struct btf_type *t2)
 {
-	struct btf_array *info1, *info2;
+	const struct btf_array *info1, *info2;
 
 	if (!btf_equal_common(t1, t2))
 		return false;
 
-	info1 = (struct btf_array *)(t1 + 1);
-	info2 = (struct btf_array *)(t2 + 1);
+	info1 = btf_array(t1);
+	info2 = btf_array(t2);
 	return info1->type == info2->type &&
 	       info1->index_type == info2->index_type &&
 	       info1->nelems == info2->nelems;
@@ -1927,14 +1916,10 @@ static bool btf_equal_array(struct btf_type *t1, struct btf_type *t2)
  */
 static bool btf_compat_array(struct btf_type *t1, struct btf_type *t2)
 {
-	struct btf_array *info1, *info2;
-
 	if (!btf_equal_common(t1, t2))
 		return false;
 
-	info1 = (struct btf_array *)(t1 + 1);
-	info2 = (struct btf_array *)(t2 + 1);
-	return info1->nelems == info2->nelems;
+	return btf_array(t1)->nelems == btf_array(t2)->nelems;
 }
 
 /*
@@ -1944,8 +1929,8 @@ static bool btf_compat_array(struct btf_type *t1, struct btf_type *t2)
  */
 static long btf_hash_fnproto(struct btf_type *t)
 {
-	struct btf_param *member = (struct btf_param *)(t + 1);
-	__u16 vlen = BTF_INFO_VLEN(t->info);
+	const struct btf_param *member = btf_params(t);
+	__u16 vlen = btf_vlen(t);
 	long h = btf_hash_common(t);
 	int i;
 
@@ -1966,16 +1951,16 @@ static long btf_hash_fnproto(struct btf_type *t)
  */
 static bool btf_equal_fnproto(struct btf_type *t1, struct btf_type *t2)
 {
-	struct btf_param *m1, *m2;
+	const struct btf_param *m1, *m2;
 	__u16 vlen;
 	int i;
 
 	if (!btf_equal_common(t1, t2))
 		return false;
 
-	vlen = BTF_INFO_VLEN(t1->info);
-	m1 = (struct btf_param *)(t1 + 1);
-	m2 = (struct btf_param *)(t2 + 1);
+	vlen = btf_vlen(t1);
+	m1 = btf_params(t1);
+	m2 = btf_params(t2);
 	for (i = 0; i < vlen; i++) {
 		if (m1->name_off != m2->name_off || m1->type != m2->type)
 			return false;
@@ -1992,7 +1977,7 @@ static bool btf_equal_fnproto(struct btf_type *t1, struct btf_type *t2)
  */
 static bool btf_compat_fnproto(struct btf_type *t1, struct btf_type *t2)
 {
-	struct btf_param *m1, *m2;
+	const struct btf_param *m1, *m2;
 	__u16 vlen;
 	int i;
 
@@ -2000,9 +1985,9 @@ static bool btf_compat_fnproto(struct btf_type *t1, struct btf_type *t2)
 	if (t1->name_off != t2->name_off || t1->info != t2->info)
 		return false;
 
-	vlen = BTF_INFO_VLEN(t1->info);
-	m1 = (struct btf_param *)(t1 + 1);
-	m2 = (struct btf_param *)(t2 + 1);
+	vlen = btf_vlen(t1);
+	m1 = btf_params(t1);
+	m2 = btf_params(t2);
 	for (i = 0; i < vlen; i++) {
 		if (m1->name_off != m2->name_off)
 			return false;
@@ -2028,7 +2013,7 @@ static int btf_dedup_prim_type(struct btf_dedup *d, __u32 type_id)
 	__u32 cand_id;
 	long h;
 
-	switch (BTF_INFO_KIND(t->info)) {
+	switch (btf_kind(t)) {
 	case BTF_KIND_CONST:
 	case BTF_KIND_VOLATILE:
 	case BTF_KIND_RESTRICT:
@@ -2141,13 +2126,13 @@ static uint32_t resolve_fwd_id(struct btf_dedup *d, uint32_t type_id)
 {
 	__u32 orig_type_id = type_id;
 
-	if (BTF_INFO_KIND(d->btf->types[type_id]->info) != BTF_KIND_FWD)
+	if (!btf_is_fwd(d->btf->types[type_id]))
 		return type_id;
 
 	while (is_type_mapped(d, type_id) && d->map[type_id] != type_id)
 		type_id = d->map[type_id];
 
-	if (BTF_INFO_KIND(d->btf->types[type_id]->info) != BTF_KIND_FWD)
+	if (!btf_is_fwd(d->btf->types[type_id]))
 		return type_id;
 
 	return orig_type_id;
@@ -2156,7 +2141,7 @@ static uint32_t resolve_fwd_id(struct btf_dedup *d, uint32_t type_id)
 
 static inline __u16 btf_fwd_kind(struct btf_type *t)
 {
-	return BTF_INFO_KFLAG(t->info) ? BTF_KIND_UNION : BTF_KIND_STRUCT;
+	return btf_kflag(t) ? BTF_KIND_UNION : BTF_KIND_STRUCT;
 }
 
 /*
@@ -2277,8 +2262,8 @@ static int btf_dedup_is_equiv(struct btf_dedup *d, __u32 cand_id,
 
 	cand_type = d->btf->types[cand_id];
 	canon_type = d->btf->types[canon_id];
-	cand_kind = BTF_INFO_KIND(cand_type->info);
-	canon_kind = BTF_INFO_KIND(canon_type->info);
+	cand_kind = btf_kind(cand_type);
+	canon_kind = btf_kind(canon_type);
 
 	if (cand_type->name_off != canon_type->name_off)
 		return 0;
@@ -2327,12 +2312,12 @@ static int btf_dedup_is_equiv(struct btf_dedup *d, __u32 cand_id,
 		return btf_dedup_is_equiv(d, cand_type->type, canon_type->type);
 
 	case BTF_KIND_ARRAY: {
-		struct btf_array *cand_arr, *canon_arr;
+		const struct btf_array *cand_arr, *canon_arr;
 
 		if (!btf_compat_array(cand_type, canon_type))
 			return 0;
-		cand_arr = (struct btf_array *)(cand_type + 1);
-		canon_arr = (struct btf_array *)(canon_type + 1);
+		cand_arr = btf_array(cand_type);
+		canon_arr = btf_array(canon_type);
 		eq = btf_dedup_is_equiv(d,
 			cand_arr->index_type, canon_arr->index_type);
 		if (eq <= 0)
@@ -2342,14 +2327,14 @@ static int btf_dedup_is_equiv(struct btf_dedup *d, __u32 cand_id,
 
 	case BTF_KIND_STRUCT:
 	case BTF_KIND_UNION: {
-		struct btf_member *cand_m, *canon_m;
+		const struct btf_member *cand_m, *canon_m;
 		__u16 vlen;
 
 		if (!btf_shallow_equal_struct(cand_type, canon_type))
 			return 0;
-		vlen = BTF_INFO_VLEN(cand_type->info);
-		cand_m = (struct btf_member *)(cand_type + 1);
-		canon_m = (struct btf_member *)(canon_type + 1);
+		vlen = btf_vlen(cand_type);
+		cand_m = btf_members(cand_type);
+		canon_m = btf_members(canon_type);
 		for (i = 0; i < vlen; i++) {
 			eq = btf_dedup_is_equiv(d, cand_m->type, canon_m->type);
 			if (eq <= 0)
@@ -2362,7 +2347,7 @@ static int btf_dedup_is_equiv(struct btf_dedup *d, __u32 cand_id,
 	}
 
 	case BTF_KIND_FUNC_PROTO: {
-		struct btf_param *cand_p, *canon_p;
+		const struct btf_param *cand_p, *canon_p;
 		__u16 vlen;
 
 		if (!btf_compat_fnproto(cand_type, canon_type))
@@ -2370,9 +2355,9 @@ static int btf_dedup_is_equiv(struct btf_dedup *d, __u32 cand_id,
 		eq = btf_dedup_is_equiv(d, cand_type->type, canon_type->type);
 		if (eq <= 0)
 			return eq;
-		vlen = BTF_INFO_VLEN(cand_type->info);
-		cand_p = (struct btf_param *)(cand_type + 1);
-		canon_p = (struct btf_param *)(canon_type + 1);
+		vlen = btf_vlen(cand_type);
+		cand_p = btf_params(cand_type);
+		canon_p = btf_params(canon_type);
 		for (i = 0; i < vlen; i++) {
 			eq = btf_dedup_is_equiv(d, cand_p->type, canon_p->type);
 			if (eq <= 0)
@@ -2427,8 +2412,8 @@ static void btf_dedup_merge_hypot_map(struct btf_dedup *d)
 		targ_type_id = d->hypot_map[cand_type_id];
 		t_id = resolve_type_id(d, targ_type_id);
 		c_id = resolve_type_id(d, cand_type_id);
-		t_kind = BTF_INFO_KIND(d->btf->types[t_id]->info);
-		c_kind = BTF_INFO_KIND(d->btf->types[c_id]->info);
+		t_kind = btf_kind(d->btf->types[t_id]);
+		c_kind = btf_kind(d->btf->types[c_id]);
 		/*
 		 * Resolve FWD into STRUCT/UNION.
 		 * It's ok to resolve FWD into STRUCT/UNION that's not yet
@@ -2497,7 +2482,7 @@ static int btf_dedup_struct_type(struct btf_dedup *d, __u32 type_id)
 		return 0;
 
 	t = d->btf->types[type_id];
-	kind = BTF_INFO_KIND(t->info);
+	kind = btf_kind(t);
 
 	if (kind != BTF_KIND_STRUCT && kind != BTF_KIND_UNION)
 		return 0;
@@ -2592,7 +2577,7 @@ static int btf_dedup_ref_type(struct btf_dedup *d, __u32 type_id)
 	t = d->btf->types[type_id];
 	d->map[type_id] = BTF_IN_PROGRESS_ID;
 
-	switch (BTF_INFO_KIND(t->info)) {
+	switch (btf_kind(t)) {
 	case BTF_KIND_CONST:
 	case BTF_KIND_VOLATILE:
 	case BTF_KIND_RESTRICT:
@@ -2616,7 +2601,7 @@ static int btf_dedup_ref_type(struct btf_dedup *d, __u32 type_id)
 		break;
 
 	case BTF_KIND_ARRAY: {
-		struct btf_array *info = (struct btf_array *)(t + 1);
+		struct btf_array *info = btf_array(t);
 
 		ref_type_id = btf_dedup_ref_type(d, info->type);
 		if (ref_type_id < 0)
@@ -2650,8 +2635,8 @@ static int btf_dedup_ref_type(struct btf_dedup *d, __u32 type_id)
 			return ref_type_id;
 		t->type = ref_type_id;
 
-		vlen = BTF_INFO_VLEN(t->info);
-		param = (struct btf_param *)(t + 1);
+		vlen = btf_vlen(t);
+		param = btf_params(t);
 		for (i = 0; i < vlen; i++) {
 			ref_type_id = btf_dedup_ref_type(d, param->type);
 			if (ref_type_id < 0)
@@ -2791,7 +2776,7 @@ static int btf_dedup_remap_type(struct btf_dedup *d, __u32 type_id)
 	struct btf_type *t = d->btf->types[type_id];
 	int i, r;
 
-	switch (BTF_INFO_KIND(t->info)) {
+	switch (btf_kind(t)) {
 	case BTF_KIND_INT:
 	case BTF_KIND_ENUM:
 		break;
@@ -2811,7 +2796,7 @@ static int btf_dedup_remap_type(struct btf_dedup *d, __u32 type_id)
 		break;
 
 	case BTF_KIND_ARRAY: {
-		struct btf_array *arr_info = (struct btf_array *)(t + 1);
+		struct btf_array *arr_info = btf_array(t);
 
 		r = btf_dedup_remap_type_id(d, arr_info->type);
 		if (r < 0)
@@ -2826,8 +2811,8 @@ static int btf_dedup_remap_type(struct btf_dedup *d, __u32 type_id)
 
 	case BTF_KIND_STRUCT:
 	case BTF_KIND_UNION: {
-		struct btf_member *member = (struct btf_member *)(t + 1);
-		__u16 vlen = BTF_INFO_VLEN(t->info);
+		struct btf_member *member = btf_members(t);
+		__u16 vlen = btf_vlen(t);
 
 		for (i = 0; i < vlen; i++) {
 			r = btf_dedup_remap_type_id(d, member->type);
@@ -2840,8 +2825,8 @@ static int btf_dedup_remap_type(struct btf_dedup *d, __u32 type_id)
 	}
 
 	case BTF_KIND_FUNC_PROTO: {
-		struct btf_param *param = (struct btf_param *)(t + 1);
-		__u16 vlen = BTF_INFO_VLEN(t->info);
+		struct btf_param *param = btf_params(t);
+		__u16 vlen = btf_vlen(t);
 
 		r = btf_dedup_remap_type_id(d, t->type);
 		if (r < 0)
@@ -2859,8 +2844,8 @@ static int btf_dedup_remap_type(struct btf_dedup *d, __u32 type_id)
 	}
 
 	case BTF_KIND_DATASEC: {
-		struct btf_var_secinfo *var = (struct btf_var_secinfo *)(t + 1);
-		__u16 vlen = BTF_INFO_VLEN(t->info);
+		struct btf_var_secinfo *var = btf_var_secinfos(t);
+		__u16 vlen = btf_vlen(t);
 
 		for (i = 0; i < vlen; i++) {
 			r = btf_dedup_remap_type_id(d, var->type);
diff --git a/tools/lib/bpf/btf_dump.c b/tools/lib/bpf/btf_dump.c
index 7065bb5b2752..715967762312 100644
--- a/tools/lib/bpf/btf_dump.c
+++ b/tools/lib/bpf/btf_dump.c
@@ -100,21 +100,6 @@ static bool str_equal_fn(const void *a, const void *b, void *ctx)
 	return strcmp(a, b) == 0;
 }
 
-static __u16 btf_kind_of(const struct btf_type *t)
-{
-	return BTF_INFO_KIND(t->info);
-}
-
-static __u16 btf_vlen_of(const struct btf_type *t)
-{
-	return BTF_INFO_VLEN(t->info);
-}
-
-static bool btf_kflag_of(const struct btf_type *t)
-{
-	return BTF_INFO_KFLAG(t->info);
-}
-
 static const char *btf_name_of(const struct btf_dump *d, __u32 name_off)
 {
 	return btf__name_by_offset(d->btf, name_off);
@@ -349,7 +334,7 @@ static int btf_dump_order_type(struct btf_dump *d, __u32 id, bool through_ptr)
 	 */
 	struct btf_dump_type_aux_state *tstate = &d->type_states[id];
 	const struct btf_type *t;
-	__u16 kind, vlen;
+	__u16 vlen;
 	int err, i;
 
 	/* return true, letting typedefs know that it's ok to be emitted */
@@ -357,18 +342,16 @@ static int btf_dump_order_type(struct btf_dump *d, __u32 id, bool through_ptr)
 		return 1;
 
 	t = btf__type_by_id(d->btf, id);
-	kind = btf_kind_of(t);
 
 	if (tstate->order_state == ORDERING) {
 		/* type loop, but resolvable through fwd declaration */
-		if ((kind == BTF_KIND_STRUCT || kind == BTF_KIND_UNION) &&
-		    through_ptr && t->name_off != 0)
+		if (btf_is_composite(t) && through_ptr && t->name_off != 0)
 			return 0;
 		pr_warning("unsatisfiable type cycle, id:[%u]\n", id);
 		return -ELOOP;
 	}
 
-	switch (kind) {
+	switch (btf_kind(t)) {
 	case BTF_KIND_INT:
 		tstate->order_state = ORDERED;
 		return 0;
@@ -378,14 +361,12 @@ static int btf_dump_order_type(struct btf_dump *d, __u32 id, bool through_ptr)
 		tstate->order_state = ORDERED;
 		return err;
 
-	case BTF_KIND_ARRAY: {
-		const struct btf_array *a = (void *)(t + 1);
+	case BTF_KIND_ARRAY:
+		return btf_dump_order_type(d, btf_array(t)->type, through_ptr);
 
-		return btf_dump_order_type(d, a->type, through_ptr);
-	}
 	case BTF_KIND_STRUCT:
 	case BTF_KIND_UNION: {
-		const struct btf_member *m = (void *)(t + 1);
+		const struct btf_member *m = btf_members(t);
 		/*
 		 * struct/union is part of strong link, only if it's embedded
 		 * (so no ptr in a path) or it's anonymous (so has to be
@@ -396,7 +377,7 @@ static int btf_dump_order_type(struct btf_dump *d, __u32 id, bool through_ptr)
 
 		tstate->order_state = ORDERING;
 
-		vlen = btf_vlen_of(t);
+		vlen = btf_vlen(t);
 		for (i = 0; i < vlen; i++, m++) {
 			err = btf_dump_order_type(d, m->type, false);
 			if (err < 0)
@@ -447,7 +428,7 @@ static int btf_dump_order_type(struct btf_dump *d, __u32 id, bool through_ptr)
 		return btf_dump_order_type(d, t->type, through_ptr);
 
 	case BTF_KIND_FUNC_PROTO: {
-		const struct btf_param *p = (void *)(t + 1);
+		const struct btf_param *p = btf_params(t);
 		bool is_strong;
 
 		err = btf_dump_order_type(d, t->type, through_ptr);
@@ -455,7 +436,7 @@ static int btf_dump_order_type(struct btf_dump *d, __u32 id, bool through_ptr)
 			return err;
 		is_strong = err > 0;
 
-		vlen = btf_vlen_of(t);
+		vlen = btf_vlen(t);
 		for (i = 0; i < vlen; i++, p++) {
 			err = btf_dump_order_type(d, p->type, through_ptr);
 			if (err < 0)
@@ -553,7 +534,7 @@ static void btf_dump_emit_type(struct btf_dump *d, __u32 id, __u32 cont_id)
 		return;
 
 	t = btf__type_by_id(d->btf, id);
-	kind = btf_kind_of(t);
+	kind = btf_kind(t);
 
 	if (top_level_def && t->name_off == 0) {
 		pr_warning("unexpected nameless definition, id:[%u]\n", id);
@@ -618,12 +599,9 @@ static void btf_dump_emit_type(struct btf_dump *d, __u32 id, __u32 cont_id)
 	case BTF_KIND_RESTRICT:
 		btf_dump_emit_type(d, t->type, cont_id);
 		break;
-	case BTF_KIND_ARRAY: {
-		const struct btf_array *a = (void *)(t + 1);
-
-		btf_dump_emit_type(d, a->type, cont_id);
+	case BTF_KIND_ARRAY:
+		btf_dump_emit_type(d, btf_array(t)->type, cont_id);
 		break;
-	}
 	case BTF_KIND_FWD:
 		btf_dump_emit_fwd_def(d, id, t);
 		btf_dump_printf(d, ";\n\n");
@@ -656,8 +634,8 @@ static void btf_dump_emit_type(struct btf_dump *d, __u32 id, __u32 cont_id)
 		 * applicable
 		 */
 		if (top_level_def || t->name_off == 0) {
-			const struct btf_member *m = (void *)(t + 1);
-			__u16 vlen = btf_vlen_of(t);
+			const struct btf_member *m = btf_members(t);
+			__u16 vlen = btf_vlen(t);
 			int i, new_cont_id;
 
 			new_cont_id = t->name_off == 0 ? cont_id : id;
@@ -678,8 +656,8 @@ static void btf_dump_emit_type(struct btf_dump *d, __u32 id, __u32 cont_id)
 		}
 		break;
 	case BTF_KIND_FUNC_PROTO: {
-		const struct btf_param *p = (void *)(t + 1);
-		__u16 vlen = btf_vlen_of(t);
+		const struct btf_param *p = btf_params(t);
+		__u16 vlen = btf_vlen(t);
 		int i;
 
 		btf_dump_emit_type(d, t->type, cont_id);
@@ -696,7 +674,7 @@ static void btf_dump_emit_type(struct btf_dump *d, __u32 id, __u32 cont_id)
 static int btf_align_of(const struct btf *btf, __u32 id)
 {
 	const struct btf_type *t = btf__type_by_id(btf, id);
-	__u16 kind = btf_kind_of(t);
+	__u16 kind = btf_kind(t);
 
 	switch (kind) {
 	case BTF_KIND_INT:
@@ -709,15 +687,12 @@ static int btf_align_of(const struct btf *btf, __u32 id)
 	case BTF_KIND_CONST:
 	case BTF_KIND_RESTRICT:
 		return btf_align_of(btf, t->type);
-	case BTF_KIND_ARRAY: {
-		const struct btf_array *a = (void *)(t + 1);
-
-		return btf_align_of(btf, a->type);
-	}
+	case BTF_KIND_ARRAY:
+		return btf_align_of(btf, btf_array(t)->type);
 	case BTF_KIND_STRUCT:
 	case BTF_KIND_UNION: {
-		const struct btf_member *m = (void *)(t + 1);
-		__u16 vlen = btf_vlen_of(t);
+		const struct btf_member *m = btf_members(t);
+		__u16 vlen = btf_vlen(t);
 		int i, align = 1;
 
 		for (i = 0; i < vlen; i++, m++)
@@ -726,7 +701,7 @@ static int btf_align_of(const struct btf *btf, __u32 id)
 		return align;
 	}
 	default:
-		pr_warning("unsupported BTF_KIND:%u\n", btf_kind_of(t));
+		pr_warning("unsupported BTF_KIND:%u\n", btf_kind(t));
 		return 1;
 	}
 }
@@ -737,20 +712,18 @@ static bool btf_is_struct_packed(const struct btf *btf, __u32 id,
 	const struct btf_member *m;
 	int align, i, bit_sz;
 	__u16 vlen;
-	bool kflag;
 
 	align = btf_align_of(btf, id);
 	/* size of a non-packed struct has to be a multiple of its alignment*/
 	if (t->size % align)
 		return true;
 
-	m = (void *)(t + 1);
-	kflag = btf_kflag_of(t);
-	vlen = btf_vlen_of(t);
+	m = btf_members(t);
+	vlen = btf_vlen(t);
 	/* all non-bitfield fields have to be naturally aligned */
 	for (i = 0; i < vlen; i++, m++) {
 		align = btf_align_of(btf, m->type);
-		bit_sz = kflag ? BTF_MEMBER_BITFIELD_SIZE(m->offset) : 0;
+		bit_sz = btf_member_bitfield_size(t, i);
 		if (bit_sz == 0 && m->offset % (8 * align) != 0)
 			return true;
 	}
@@ -807,7 +780,7 @@ static void btf_dump_emit_struct_fwd(struct btf_dump *d, __u32 id,
 				     const struct btf_type *t)
 {
 	btf_dump_printf(d, "%s %s",
-			btf_kind_of(t) == BTF_KIND_STRUCT ? "struct" : "union",
+			btf_is_struct(t) ? "struct" : "union",
 			btf_dump_type_name(d, id));
 }
 
@@ -816,12 +789,11 @@ static void btf_dump_emit_struct_def(struct btf_dump *d,
 				     const struct btf_type *t,
 				     int lvl)
 {
-	const struct btf_member *m = (void *)(t + 1);
-	bool kflag = btf_kflag_of(t), is_struct;
+	const struct btf_member *m = btf_members(t);
+	bool is_struct = btf_is_struct(t);
 	int align, i, packed, off = 0;
-	__u16 vlen = btf_vlen_of(t);
+	__u16 vlen = btf_vlen(t);
 
-	is_struct = btf_kind_of(t) == BTF_KIND_STRUCT;
 	packed = is_struct ? btf_is_struct_packed(d->btf, id, t) : 0;
 	align = packed ? 1 : btf_align_of(d->btf, id);
 
@@ -835,8 +807,8 @@ static void btf_dump_emit_struct_def(struct btf_dump *d,
 		int m_off, m_sz;
 
 		fname = btf_name_of(d, m->name_off);
-		m_sz = kflag ? BTF_MEMBER_BITFIELD_SIZE(m->offset) : 0;
-		m_off = kflag ? BTF_MEMBER_BIT_OFFSET(m->offset) : m->offset;
+		m_sz = btf_member_bitfield_size(t, i);
+		m_off = btf_member_bit_offset(t, i);
 		align = packed ? 1 : btf_align_of(d->btf, m->type);
 
 		btf_dump_emit_bit_padding(d, off, m_off, m_sz, align, lvl + 1);
@@ -870,8 +842,8 @@ static void btf_dump_emit_enum_def(struct btf_dump *d, __u32 id,
 				   const struct btf_type *t,
 				   int lvl)
 {
-	const struct btf_enum *v = (void *)(t+1);
-	__u16 vlen = btf_vlen_of(t);
+	const struct btf_enum *v = btf_enum(t);
+	__u16 vlen = btf_vlen(t);
 	const char *name;
 	size_t dup_cnt;
 	int i;
@@ -905,7 +877,7 @@ static void btf_dump_emit_fwd_def(struct btf_dump *d, __u32 id,
 {
 	const char *name = btf_dump_type_name(d, id);
 
-	if (btf_kflag_of(t))
+	if (btf_kflag(t))
 		btf_dump_printf(d, "union %s", name);
 	else
 		btf_dump_printf(d, "struct %s", name);
@@ -987,7 +959,6 @@ static void btf_dump_emit_type_decl(struct btf_dump *d, __u32 id,
 	struct id_stack decl_stack;
 	const struct btf_type *t;
 	int err, stack_start;
-	__u16 kind;
 
 	stack_start = d->decl_stack_cnt;
 	for (;;) {
@@ -1008,8 +979,7 @@ static void btf_dump_emit_type_decl(struct btf_dump *d, __u32 id,
 			break;
 
 		t = btf__type_by_id(d->btf, id);
-		kind = btf_kind_of(t);
-		switch (kind) {
+		switch (btf_kind(t)) {
 		case BTF_KIND_PTR:
 		case BTF_KIND_VOLATILE:
 		case BTF_KIND_CONST:
@@ -1017,12 +987,9 @@ static void btf_dump_emit_type_decl(struct btf_dump *d, __u32 id,
 		case BTF_KIND_FUNC_PROTO:
 			id = t->type;
 			break;
-		case BTF_KIND_ARRAY: {
-			const struct btf_array *a = (void *)(t + 1);
-
-			id = a->type;
+		case BTF_KIND_ARRAY:
+			id = btf_array(t)->type;
 			break;
-		}
 		case BTF_KIND_INT:
 		case BTF_KIND_ENUM:
 		case BTF_KIND_FWD:
@@ -1032,7 +999,7 @@ static void btf_dump_emit_type_decl(struct btf_dump *d, __u32 id,
 			goto done;
 		default:
 			pr_warning("unexpected type in decl chain, kind:%u, id:[%u]\n",
-				   kind, id);
+				   btf_kind(t), id);
 			goto done;
 		}
 	}
@@ -1070,7 +1037,7 @@ static void btf_dump_emit_mods(struct btf_dump *d, struct id_stack *decl_stack)
 		id = decl_stack->ids[decl_stack->cnt - 1];
 		t = btf__type_by_id(d->btf, id);
 
-		switch (btf_kind_of(t)) {
+		switch (btf_kind(t)) {
 		case BTF_KIND_VOLATILE:
 			btf_dump_printf(d, "volatile ");
 			break;
@@ -1087,20 +1054,6 @@ static void btf_dump_emit_mods(struct btf_dump *d, struct id_stack *decl_stack)
 	}
 }
 
-static bool btf_is_mod_kind(const struct btf *btf, __u32 id)
-{
-	const struct btf_type *t = btf__type_by_id(btf, id);
-
-	switch (btf_kind_of(t)) {
-	case BTF_KIND_VOLATILE:
-	case BTF_KIND_CONST:
-	case BTF_KIND_RESTRICT:
-		return true;
-	default:
-		return false;
-	}
-}
-
 static void btf_dump_emit_name(const struct btf_dump *d,
 			       const char *name, bool last_was_ptr)
 {
@@ -1139,7 +1092,7 @@ static void btf_dump_emit_type_chain(struct btf_dump *d,
 		}
 
 		t = btf__type_by_id(d->btf, id);
-		kind = btf_kind_of(t);
+		kind = btf_kind(t);
 
 		switch (kind) {
 		case BTF_KIND_INT:
@@ -1185,7 +1138,7 @@ static void btf_dump_emit_type_chain(struct btf_dump *d,
 			btf_dump_printf(d, " restrict");
 			break;
 		case BTF_KIND_ARRAY: {
-			const struct btf_array *a = (void *)(t + 1);
+			const struct btf_array *a = btf_array(t);
 			const struct btf_type *next_t;
 			__u32 next_id;
 			bool multidim;
@@ -1201,7 +1154,8 @@ static void btf_dump_emit_type_chain(struct btf_dump *d,
 			 */
 			while (decls->cnt) {
 				next_id = decls->ids[decls->cnt - 1];
-				if (btf_is_mod_kind(d->btf, next_id))
+				next_t = btf__type_by_id(d->btf, next_id);
+				if (btf_is_mod(next_t))
 					decls->cnt--;
 				else
 					break;
@@ -1214,7 +1168,7 @@ static void btf_dump_emit_type_chain(struct btf_dump *d,
 			}
 
 			next_t = btf__type_by_id(d->btf, next_id);
-			multidim = btf_kind_of(next_t) == BTF_KIND_ARRAY;
+			multidim = btf_is_array(next_t);
 			/* we need space if we have named non-pointer */
 			if (fname[0] && !last_was_ptr)
 				btf_dump_printf(d, " ");
@@ -1228,8 +1182,8 @@ static void btf_dump_emit_type_chain(struct btf_dump *d,
 			return;
 		}
 		case BTF_KIND_FUNC_PROTO: {
-			const struct btf_param *p = (void *)(t + 1);
-			__u16 vlen = btf_vlen_of(t);
+			const struct btf_param *p = btf_params(t);
+			__u16 vlen = btf_vlen(t);
 			int i;
 
 			btf_dump_emit_mods(d, decls);
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index ead915aec349..8fc62b6b1cd6 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -1049,9 +1049,9 @@ static bool get_map_field_int(const char *map_name, const struct btf *btf,
 	const struct btf_array *arr_info;
 	const struct btf_type *arr_t;
 
-	if (BTF_INFO_KIND(t->info) != BTF_KIND_PTR) {
+	if (!btf_is_ptr(t)) {
 		pr_warning("map '%s': attr '%s': expected PTR, got %u.\n",
-			   map_name, name, BTF_INFO_KIND(t->info));
+			   map_name, name, btf_kind(t));
 		return false;
 	}
 
@@ -1061,12 +1061,12 @@ static bool get_map_field_int(const char *map_name, const struct btf *btf,
 			   map_name, name, t->type);
 		return false;
 	}
-	if (BTF_INFO_KIND(arr_t->info) != BTF_KIND_ARRAY) {
+	if (!btf_is_array(arr_t)) {
 		pr_warning("map '%s': attr '%s': expected ARRAY, got %u.\n",
-			   map_name, name, BTF_INFO_KIND(arr_t->info));
+			   map_name, name, btf_kind(arr_t));
 		return false;
 	}
-	arr_info = (const void *)(arr_t + 1);
+	arr_info = btf_array(arr_t);
 	*res = arr_info->nelems;
 	return true;
 }
@@ -1084,11 +1084,11 @@ static int bpf_object__init_user_btf_map(struct bpf_object *obj,
 	struct bpf_map *map;
 	int vlen, i;
 
-	vi = (const struct btf_var_secinfo *)(const void *)(sec + 1) + var_idx;
+	vi = btf_var_secinfos(sec) + var_idx;
 	var = btf__type_by_id(obj->btf, vi->type);
-	var_extra = (const void *)(var + 1);
+	var_extra = btf_var(var);
 	map_name = btf__name_by_offset(obj->btf, var->name_off);
-	vlen = BTF_INFO_VLEN(var->info);
+	vlen = btf_vlen(var);
 
 	if (map_name == NULL || map_name[0] == '\0') {
 		pr_warning("map #%d: empty name.\n", var_idx);
@@ -1098,9 +1098,9 @@ static int bpf_object__init_user_btf_map(struct bpf_object *obj,
 		pr_warning("map '%s' BTF data is corrupted.\n", map_name);
 		return -EINVAL;
 	}
-	if (BTF_INFO_KIND(var->info) != BTF_KIND_VAR) {
+	if (!btf_is_var(var)) {
 		pr_warning("map '%s': unexpected var kind %u.\n",
-			   map_name, BTF_INFO_KIND(var->info));
+			   map_name, btf_kind(var));
 		return -EINVAL;
 	}
 	if (var_extra->linkage != BTF_VAR_GLOBAL_ALLOCATED &&
@@ -1111,9 +1111,9 @@ static int bpf_object__init_user_btf_map(struct bpf_object *obj,
 	}
 
 	def = skip_mods_and_typedefs(obj->btf, var->type);
-	if (BTF_INFO_KIND(def->info) != BTF_KIND_STRUCT) {
+	if (!btf_is_struct(def)) {
 		pr_warning("map '%s': unexpected def kind %u.\n",
-			   map_name, BTF_INFO_KIND(var->info));
+			   map_name, btf_kind(var));
 		return -EINVAL;
 	}
 	if (def->size > vi->size) {
@@ -1136,8 +1136,8 @@ static int bpf_object__init_user_btf_map(struct bpf_object *obj,
 	pr_debug("map '%s': at sec_idx %d, offset %zu.\n",
 		 map_name, map->sec_idx, map->sec_offset);
 
-	vlen = BTF_INFO_VLEN(def->info);
-	m = (const void *)(def + 1);
+	vlen = btf_vlen(def);
+	m = btf_members(def);
 	for (i = 0; i < vlen; i++, m++) {
 		const char *name = btf__name_by_offset(obj->btf, m->name_off);
 
@@ -1187,9 +1187,9 @@ static int bpf_object__init_user_btf_map(struct bpf_object *obj,
 					   map_name, m->type);
 				return -EINVAL;
 			}
-			if (BTF_INFO_KIND(t->info) != BTF_KIND_PTR) {
+			if (!btf_is_ptr(t)) {
 				pr_warning("map '%s': key spec is not PTR: %u.\n",
-					   map_name, BTF_INFO_KIND(t->info));
+					   map_name, btf_kind(t));
 				return -EINVAL;
 			}
 			sz = btf__resolve_size(obj->btf, t->type);
@@ -1230,9 +1230,9 @@ static int bpf_object__init_user_btf_map(struct bpf_object *obj,
 					   map_name, m->type);
 				return -EINVAL;
 			}
-			if (BTF_INFO_KIND(t->info) != BTF_KIND_PTR) {
+			if (!btf_is_ptr(t)) {
 				pr_warning("map '%s': value spec is not PTR: %u.\n",
-					   map_name, BTF_INFO_KIND(t->info));
+					   map_name, btf_kind(t));
 				return -EINVAL;
 			}
 			sz = btf__resolve_size(obj->btf, t->type);
@@ -1293,7 +1293,7 @@ static int bpf_object__init_user_btf_maps(struct bpf_object *obj, bool strict)
 	nr_types = btf__get_nr_types(obj->btf);
 	for (i = 1; i <= nr_types; i++) {
 		t = btf__type_by_id(obj->btf, i);
-		if (BTF_INFO_KIND(t->info) != BTF_KIND_DATASEC)
+		if (!btf_is_datasec(t))
 			continue;
 		name = btf__name_by_offset(obj->btf, t->name_off);
 		if (strcmp(name, MAPS_ELF_SEC) == 0) {
@@ -1307,7 +1307,7 @@ static int bpf_object__init_user_btf_maps(struct bpf_object *obj, bool strict)
 		return -ENOENT;
 	}
 
-	vlen = BTF_INFO_VLEN(sec->info);
+	vlen = btf_vlen(sec);
 	for (i = 0; i < vlen; i++) {
 		err = bpf_object__init_user_btf_map(obj, sec, i,
 						    obj->efile.btf_maps_shndx,
@@ -1368,24 +1368,22 @@ static void bpf_object__sanitize_btf(struct bpf_object *obj)
 	struct btf *btf = obj->btf;
 	struct btf_type *t;
 	int i, j, vlen;
-	__u16 kind;
 
 	if (!obj->btf || (has_func && has_datasec))
 		return;
 
 	for (i = 1; i <= btf__get_nr_types(btf); i++) {
 		t = (struct btf_type *)btf__type_by_id(btf, i);
-		kind = BTF_INFO_KIND(t->info);
 
-		if (!has_datasec && kind == BTF_KIND_VAR) {
+		if (!has_datasec && btf_is_var(t)) {
 			/* replace VAR with INT */
 			t->info = BTF_INFO_ENC(BTF_KIND_INT, 0, 0);
 			t->size = sizeof(int);
-			*(int *)(t+1) = BTF_INT_ENC(0, 0, 32);
-		} else if (!has_datasec && kind == BTF_KIND_DATASEC) {
+			*(int *)(t + 1) = BTF_INT_ENC(0, 0, 32);
+		} else if (!has_datasec && btf_is_datasec(t)) {
 			/* replace DATASEC with STRUCT */
-			struct btf_var_secinfo *v = (void *)(t + 1);
-			struct btf_member *m = (void *)(t + 1);
+			const struct btf_var_secinfo *v = btf_var_secinfos(t);
+			struct btf_member *m = btf_members(t);
 			struct btf_type *vt;
 			char *name;
 
@@ -1396,7 +1394,7 @@ static void bpf_object__sanitize_btf(struct bpf_object *obj)
 				name++;
 			}
 
-			vlen = BTF_INFO_VLEN(t->info);
+			vlen = btf_vlen(t);
 			t->info = BTF_INFO_ENC(BTF_KIND_STRUCT, 0, vlen);
 			for (j = 0; j < vlen; j++, v++, m++) {
 				/* order of field assignments is important */
@@ -1406,12 +1404,12 @@ static void bpf_object__sanitize_btf(struct bpf_object *obj)
 				vt = (void *)btf__type_by_id(btf, v->type);
 				m->name_off = vt->name_off;
 			}
-		} else if (!has_func && kind == BTF_KIND_FUNC_PROTO) {
+		} else if (!has_func && btf_is_func_proto(t)) {
 			/* replace FUNC_PROTO with ENUM */
-			vlen = BTF_INFO_VLEN(t->info);
+			vlen = btf_vlen(t);
 			t->info = BTF_INFO_ENC(BTF_KIND_ENUM, 0, vlen);
 			t->size = sizeof(__u32); /* kernel enforced */
-		} else if (!has_func && kind == BTF_KIND_FUNC) {
+		} else if (!has_func && btf_is_func(t)) {
 			/* replace FUNC with TYPEDEF */
 			t->info = BTF_INFO_ENC(BTF_KIND_TYPEDEF, 0, 0);
 		}
-- 
2.17.1


^ permalink raw reply related

* [PATCH v5 bpf-next 01/14] libbpf: add helpers for working with BTF types
From: Andrii Nakryiko @ 2019-08-07 20:48 UTC (permalink / raw)
  To: bpf, netdev, ast, daniel, yhs
  Cc: andrii.nakryiko, kernel-team, Andrii Nakryiko
In-Reply-To: <20190807204843.513594-1-andriin@fb.com>

Add lots of frequently used helpers that simplify working with BTF
types.

Signed-off-by: Andrii Nakryiko <andriin@fb.com>
---
 tools/lib/bpf/btf.h | 176 ++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 176 insertions(+)

diff --git a/tools/lib/bpf/btf.h b/tools/lib/bpf/btf.h
index 88a52ae56fc6..037679f0dec8 100644
--- a/tools/lib/bpf/btf.h
+++ b/tools/lib/bpf/btf.h
@@ -5,6 +5,7 @@
 #define __LIBBPF_BTF_H
 
 #include <stdarg.h>
+#include <linux/btf.h>
 #include <linux/types.h>
 
 #ifdef __cplusplus
@@ -120,6 +121,181 @@ LIBBPF_API void btf_dump__free(struct btf_dump *d);
 
 LIBBPF_API int btf_dump__dump_type(struct btf_dump *d, __u32 id);
 
+/*
+ * A set of helpers for easier BTF types handling
+ */
+static inline __u16 btf_kind(const struct btf_type *t)
+{
+	return BTF_INFO_KIND(t->info);
+}
+
+static inline __u16 btf_vlen(const struct btf_type *t)
+{
+	return BTF_INFO_VLEN(t->info);
+}
+
+static inline bool btf_kflag(const struct btf_type *t)
+{
+	return BTF_INFO_KFLAG(t->info);
+}
+
+static inline bool btf_is_int(const struct btf_type *t)
+{
+	return btf_kind(t) == BTF_KIND_INT;
+}
+
+static inline bool btf_is_ptr(const struct btf_type *t)
+{
+	return btf_kind(t) == BTF_KIND_PTR;
+}
+
+static inline bool btf_is_array(const struct btf_type *t)
+{
+	return btf_kind(t) == BTF_KIND_ARRAY;
+}
+
+static inline bool btf_is_struct(const struct btf_type *t)
+{
+	return btf_kind(t) == BTF_KIND_STRUCT;
+}
+
+static inline bool btf_is_union(const struct btf_type *t)
+{
+	return btf_kind(t) == BTF_KIND_UNION;
+}
+
+static inline bool btf_is_composite(const struct btf_type *t)
+{
+	__u16 kind = btf_kind(t);
+
+	return kind == BTF_KIND_STRUCT || kind == BTF_KIND_UNION;
+}
+
+static inline bool btf_is_enum(const struct btf_type *t)
+{
+	return btf_kind(t) == BTF_KIND_ENUM;
+}
+
+static inline bool btf_is_fwd(const struct btf_type *t)
+{
+	return btf_kind(t) == BTF_KIND_FWD;
+}
+
+static inline bool btf_is_typedef(const struct btf_type *t)
+{
+	return btf_kind(t) == BTF_KIND_TYPEDEF;
+}
+
+static inline bool btf_is_volatile(const struct btf_type *t)
+{
+	return btf_kind(t) == BTF_KIND_VOLATILE;
+}
+
+static inline bool btf_is_const(const struct btf_type *t)
+{
+	return btf_kind(t) == BTF_KIND_CONST;
+}
+
+static inline bool btf_is_restrict(const struct btf_type *t)
+{
+	return btf_kind(t) == BTF_KIND_RESTRICT;
+}
+
+static inline bool btf_is_mod(const struct btf_type *t)
+{
+	__u16 kind = btf_kind(t);
+
+	return kind == BTF_KIND_VOLATILE ||
+	       kind == BTF_KIND_CONST ||
+	       kind == BTF_KIND_RESTRICT;
+}
+
+static inline bool btf_is_func(const struct btf_type *t)
+{
+	return btf_kind(t) == BTF_KIND_FUNC;
+}
+
+static inline bool btf_is_func_proto(const struct btf_type *t)
+{
+	return btf_kind(t) == BTF_KIND_FUNC_PROTO;
+}
+
+static inline bool btf_is_var(const struct btf_type *t)
+{
+	return btf_kind(t) == BTF_KIND_VAR;
+}
+
+static inline bool btf_is_datasec(const struct btf_type *t)
+{
+	return btf_kind(t) == BTF_KIND_DATASEC;
+}
+
+static inline __u8 btf_int_encoding(const struct btf_type *t)
+{
+	return BTF_INT_ENCODING(*(__u32 *)(t + 1));
+}
+
+static inline __u8 btf_int_offset(const struct btf_type *t)
+{
+	return BTF_INT_OFFSET(*(__u32 *)(t + 1));
+}
+
+static inline __u8 btf_int_bits(const struct btf_type *t)
+{
+	return BTF_INT_BITS(*(__u32 *)(t + 1));
+}
+
+static inline struct btf_array *btf_array(const struct btf_type *t)
+{
+	return (struct btf_array *)(t + 1);
+}
+
+static inline struct btf_enum *btf_enum(const struct btf_type *t)
+{
+	return (struct btf_enum *)(t + 1);
+}
+
+static inline struct btf_member *btf_members(const struct btf_type *t)
+{
+	return (struct btf_member *)(t + 1);
+}
+
+/* get bit offset of a member with specified index */
+static inline __u32 btf_member_bit_offset(const struct btf_type *t,
+					  __u32 member_idx)
+{
+	const struct btf_member *m = btf_members(t) + member_idx;
+	bool kflag = btf_kflag(t);
+
+	return kflag ? BTF_MEMBER_BIT_OFFSET(m->offset) : m->offset;
+}
+/* get bitfield size of a member, assuming t is BTF_KIND_STRUCT or
+ * BTF_KIND_UNION. If member is not a bitfield, zero is returned. */
+static inline __u32 btf_member_bitfield_size(const struct btf_type *t,
+					     __u32 member_idx)
+{
+	const struct btf_member *m = btf_members(t) + member_idx;
+	bool kflag = btf_kflag(t);
+
+	return kflag ? BTF_MEMBER_BITFIELD_SIZE(m->offset) : 0;
+}
+
+static inline struct btf_param *btf_params(const struct btf_type *t)
+{
+	return (struct btf_param *)(t + 1);
+}
+
+static inline struct btf_var *btf_var(const struct btf_type *t)
+{
+	return (struct btf_var *)(t + 1);
+}
+
+static inline struct btf_var_secinfo *
+btf_var_secinfos(const struct btf_type *t)
+{
+	return (struct btf_var_secinfo *)(t + 1);
+}
+
 #ifdef __cplusplus
 } /* extern "C" */
 #endif
-- 
2.17.1


^ permalink raw reply related

* [PATCH v5 bpf-next 03/14] libbpf: add .BTF.ext offset relocation section loading
From: Andrii Nakryiko @ 2019-08-07 20:48 UTC (permalink / raw)
  To: bpf, netdev, ast, daniel, yhs
  Cc: andrii.nakryiko, kernel-team, Andrii Nakryiko
In-Reply-To: <20190807204843.513594-1-andriin@fb.com>

Add support for BPF CO-RE offset relocations. Add section/record
iteration macros for .BTF.ext. These macro are useful for iterating over
each .BTF.ext record, either for dumping out contents or later for BPF
CO-RE relocation handling.

To enable other parts of libbpf to work with .BTF.ext contents, moved
a bunch of type definitions into libbpf_internal.h.

Signed-off-by: Andrii Nakryiko <andriin@fb.com>
Acked-by: Song Liu <songliubraving@fb.com>
---
 tools/lib/bpf/btf.c             |  69 ++++++++-------------
 tools/lib/bpf/btf.h             |   4 ++
 tools/lib/bpf/libbpf_internal.h | 105 ++++++++++++++++++++++++++++++++
 3 files changed, 136 insertions(+), 42 deletions(-)

diff --git a/tools/lib/bpf/btf.c b/tools/lib/bpf/btf.c
index 1cd4e5d67158..aacb7608f02d 100644
--- a/tools/lib/bpf/btf.c
+++ b/tools/lib/bpf/btf.c
@@ -35,47 +35,6 @@ struct btf {
 	int fd;
 };
 
-struct btf_ext_info {
-	/*
-	 * info points to the individual info section (e.g. func_info and
-	 * line_info) from the .BTF.ext. It does not include the __u32 rec_size.
-	 */
-	void *info;
-	__u32 rec_size;
-	__u32 len;
-};
-
-struct btf_ext {
-	union {
-		struct btf_ext_header *hdr;
-		void *data;
-	};
-	struct btf_ext_info func_info;
-	struct btf_ext_info line_info;
-	__u32 data_size;
-};
-
-struct btf_ext_info_sec {
-	__u32	sec_name_off;
-	__u32	num_info;
-	/* Followed by num_info * record_size number of bytes */
-	__u8	data[0];
-};
-
-/* The minimum bpf_func_info checked by the loader */
-struct bpf_func_info_min {
-	__u32   insn_off;
-	__u32   type_id;
-};
-
-/* The minimum bpf_line_info checked by the loader */
-struct bpf_line_info_min {
-	__u32	insn_off;
-	__u32	file_name_off;
-	__u32	line_off;
-	__u32	line_col;
-};
-
 static inline __u64 ptr_to_u64(const void *ptr)
 {
 	return (__u64) (unsigned long) ptr;
@@ -822,6 +781,9 @@ static int btf_ext_setup_info(struct btf_ext *btf_ext,
 	/* The start of the info sec (including the __u32 record_size). */
 	void *info;
 
+	if (ext_sec->len == 0)
+		return 0;
+
 	if (ext_sec->off & 0x03) {
 		pr_debug(".BTF.ext %s section is not aligned to 4 bytes\n",
 		     ext_sec->desc);
@@ -925,11 +887,24 @@ static int btf_ext_setup_line_info(struct btf_ext *btf_ext)
 	return btf_ext_setup_info(btf_ext, &param);
 }
 
+static int btf_ext_setup_offset_reloc(struct btf_ext *btf_ext)
+{
+	struct btf_ext_sec_setup_param param = {
+		.off = btf_ext->hdr->offset_reloc_off,
+		.len = btf_ext->hdr->offset_reloc_len,
+		.min_rec_size = sizeof(struct bpf_offset_reloc),
+		.ext_info = &btf_ext->offset_reloc_info,
+		.desc = "offset_reloc",
+	};
+
+	return btf_ext_setup_info(btf_ext, &param);
+}
+
 static int btf_ext_parse_hdr(__u8 *data, __u32 data_size)
 {
 	const struct btf_ext_header *hdr = (struct btf_ext_header *)data;
 
-	if (data_size < offsetof(struct btf_ext_header, func_info_off) ||
+	if (data_size < offsetofend(struct btf_ext_header, hdr_len) ||
 	    data_size < hdr->hdr_len) {
 		pr_debug("BTF.ext header not found");
 		return -EINVAL;
@@ -987,6 +962,9 @@ struct btf_ext *btf_ext__new(__u8 *data, __u32 size)
 	}
 	memcpy(btf_ext->data, data, size);
 
+	if (btf_ext->hdr->hdr_len <
+	    offsetofend(struct btf_ext_header, line_info_len))
+		goto done;
 	err = btf_ext_setup_func_info(btf_ext);
 	if (err)
 		goto done;
@@ -995,6 +973,13 @@ struct btf_ext *btf_ext__new(__u8 *data, __u32 size)
 	if (err)
 		goto done;
 
+	if (btf_ext->hdr->hdr_len <
+	    offsetofend(struct btf_ext_header, offset_reloc_len))
+		goto done;
+	err = btf_ext_setup_offset_reloc(btf_ext);
+	if (err)
+		goto done;
+
 done:
 	if (err) {
 		btf_ext__free(btf_ext);
diff --git a/tools/lib/bpf/btf.h b/tools/lib/bpf/btf.h
index 037679f0dec8..d18ac0b4185c 100644
--- a/tools/lib/bpf/btf.h
+++ b/tools/lib/bpf/btf.h
@@ -58,6 +58,10 @@ struct btf_ext_header {
 	__u32	func_info_len;
 	__u32	line_info_off;
 	__u32	line_info_len;
+
+	/* optional part of .BTF.ext header */
+	__u32	offset_reloc_off;
+	__u32	offset_reloc_len;
 };
 
 LIBBPF_API void btf__free(struct btf *btf);
diff --git a/tools/lib/bpf/libbpf_internal.h b/tools/lib/bpf/libbpf_internal.h
index 2ac29bd36226..2e83a34f8c79 100644
--- a/tools/lib/bpf/libbpf_internal.h
+++ b/tools/lib/bpf/libbpf_internal.h
@@ -29,6 +29,10 @@
 #ifndef max
 # define max(x, y) ((x) < (y) ? (y) : (x))
 #endif
+#ifndef offsetofend
+# define offsetofend(TYPE, FIELD) \
+	(offsetof(TYPE, FIELD) + sizeof(((TYPE *)0)->FIELD))
+#endif
 
 extern void libbpf_print(enum libbpf_print_level level,
 			 const char *format, ...)
@@ -46,4 +50,105 @@ do {				\
 int libbpf__load_raw_btf(const char *raw_types, size_t types_len,
 			 const char *str_sec, size_t str_len);
 
+struct btf_ext_info {
+	/*
+	 * info points to the individual info section (e.g. func_info and
+	 * line_info) from the .BTF.ext. It does not include the __u32 rec_size.
+	 */
+	void *info;
+	__u32 rec_size;
+	__u32 len;
+};
+
+#define for_each_btf_ext_sec(seg, sec)					\
+	for (sec = (seg)->info;						\
+	     (void *)sec < (seg)->info + (seg)->len;			\
+	     sec = (void *)sec + sizeof(struct btf_ext_info_sec) +	\
+		   (seg)->rec_size * sec->num_info)
+
+#define for_each_btf_ext_rec(seg, sec, i, rec)				\
+	for (i = 0, rec = (void *)&(sec)->data;				\
+	     i < (sec)->num_info;					\
+	     i++, rec = (void *)rec + (seg)->rec_size)
+
+struct btf_ext {
+	union {
+		struct btf_ext_header *hdr;
+		void *data;
+	};
+	struct btf_ext_info func_info;
+	struct btf_ext_info line_info;
+	struct btf_ext_info offset_reloc_info;
+	__u32 data_size;
+};
+
+struct btf_ext_info_sec {
+	__u32	sec_name_off;
+	__u32	num_info;
+	/* Followed by num_info * record_size number of bytes */
+	__u8	data[0];
+};
+
+/* The minimum bpf_func_info checked by the loader */
+struct bpf_func_info_min {
+	__u32   insn_off;
+	__u32   type_id;
+};
+
+/* The minimum bpf_line_info checked by the loader */
+struct bpf_line_info_min {
+	__u32	insn_off;
+	__u32	file_name_off;
+	__u32	line_off;
+	__u32	line_col;
+};
+
+/* The minimum bpf_offset_reloc checked by the loader
+ *
+ * Offset relocation captures the following data:
+ * - insn_off - instruction offset (in bytes) within a BPF program that needs
+ *   its insn->imm field to be relocated with actual offset;
+ * - type_id - BTF type ID of the "root" (containing) entity of a relocatable
+ *   offset;
+ * - access_str_off - offset into corresponding .BTF string section. String
+ *   itself encodes an accessed field using a sequence of field and array
+ *   indicies, separated by colon (:). It's conceptually very close to LLVM's
+ *   getelementptr ([0]) instruction's arguments for identifying offset to 
+ *   a field.
+ *
+ * Example to provide a better feel.
+ *
+ *   struct sample {
+ *       int a;
+ *       struct {
+ *           int b[10];
+ *       };
+ *   };
+ *
+ *   struct sample *s = ...;
+ *   int x = &s->a;     // encoded as "0:0" (a is field #0)
+ *   int y = &s->b[5];  // encoded as "0:1:0:5" (anon struct is field #1, 
+ *                      // b is field #0 inside anon struct, accessing elem #5)
+ *   int z = &s[10]->b; // encoded as "10:1" (ptr is used as an array)
+ *
+ * type_id for all relocs in this example  will capture BTF type id of
+ * `struct sample`.
+ *
+ * Such relocation is emitted when using __builtin_preserve_access_index()
+ * Clang built-in, passing expression that captures field address, e.g.:
+ *
+ * bpf_probe_read(&dst, sizeof(dst),
+ *		  __builtin_preserve_access_index(&src->a.b.c));
+ *
+ * In this case Clang will emit offset relocation recording necessary data to
+ * be able to find offset of embedded `a.b.c` field within `src` struct.
+ *
+ *   [0] https://llvm.org/docs/LangRef.html#getelementptr-instruction
+ */
+struct bpf_offset_reloc {
+	__u32   insn_off;
+	__u32   type_id;
+	__u32   access_str_off;
+};
+
 #endif /* __LIBBPF_LIBBPF_INTERNAL_H */
-- 
2.17.1


^ permalink raw reply related

* Re: [v3,1/4] tools: bpftool: add net attach command to attach XDP on interface
From: Jakub Kicinski @ 2019-08-07 20:42 UTC (permalink / raw)
  To: Daniel T. Lee; +Cc: Daniel Borkmann, Alexei Starovoitov, netdev
In-Reply-To: <20190807022509.4214-2-danieltimlee@gmail.com>

On Wed,  7 Aug 2019 11:25:06 +0900, Daniel T. Lee wrote:
> By this commit, using `bpftool net attach`, user can attach XDP prog on
> interface. New type of enum 'net_attach_type' has been made, as stated at
> cover-letter, the meaning of 'attach' is, prog will be attached on interface.
> 
> With 'overwrite' option at argument, attached XDP program could be replaced.
> Added new helper 'net_parse_dev' to parse the network device at argument.
> 
> BPF prog will be attached through libbpf 'bpf_set_link_xdp_fd'.
> 
> Signed-off-by: Daniel T. Lee <danieltimlee@gmail.com>
> ---
>  tools/bpf/bpftool/net.c | 141 ++++++++++++++++++++++++++++++++++++----
>  1 file changed, 130 insertions(+), 11 deletions(-)
> 
> diff --git a/tools/bpf/bpftool/net.c b/tools/bpf/bpftool/net.c
> index 67e99c56bc88..c05a3fac5cac 100644
> --- a/tools/bpf/bpftool/net.c
> +++ b/tools/bpf/bpftool/net.c
> @@ -55,6 +55,35 @@ struct bpf_attach_info {
>  	__u32 flow_dissector_id;
>  };
>  
> +enum net_attach_type {
> +	NET_ATTACH_TYPE_XDP,
> +	NET_ATTACH_TYPE_XDP_GENERIC,
> +	NET_ATTACH_TYPE_XDP_DRIVER,
> +	NET_ATTACH_TYPE_XDP_OFFLOAD,
> +};
> +
> +static const char * const attach_type_strings[] = {
> +	[NET_ATTACH_TYPE_XDP]		= "xdp",
> +	[NET_ATTACH_TYPE_XDP_GENERIC]	= "xdpgeneric",
> +	[NET_ATTACH_TYPE_XDP_DRIVER]	= "xdpdrv",
> +	[NET_ATTACH_TYPE_XDP_OFFLOAD]	= "xdpoffload",
> +};
> +
> +const size_t max_net_attach_type = ARRAY_SIZE(attach_type_strings);

Nit: in practice max_.._type is num_types - 1, so perhaps rename this
to num_.. or such?

> +static enum net_attach_type parse_attach_type(const char *str)
> +{
> +	enum net_attach_type type;
> +
> +	for (type = 0; type < max_net_attach_type; type++) {
> +		if (attach_type_strings[type] &&
> +		   is_prefix(str, attach_type_strings[type]))

                   ^
this is misaligned by one space

Please try checkpatch with the --strict option to catch these.

> +			return type;
> +	}
> +
> +	return max_net_attach_type;
> +}
> +
>  static int dump_link_nlmsg(void *cookie, void *msg, struct nlattr **tb)
>  {
>  	struct bpf_netdev_t *netinfo = cookie;
> @@ -223,6 +252,97 @@ static int query_flow_dissector(struct bpf_attach_info *attach_info)
>  	return 0;
>  }
>  
> +static int net_parse_dev(int *argc, char ***argv)
> +{
> +	int ifindex;
> +
> +	if (is_prefix(**argv, "dev")) {
> +		NEXT_ARGP();
> +
> +		ifindex = if_nametoindex(**argv);
> +		if (!ifindex)
> +			p_err("invalid devname %s", **argv);
> +
> +		NEXT_ARGP();
> +	} else {
> +		p_err("expected 'dev', got: '%s'?", **argv);
> +		return -1;
> +	}
> +
> +	return ifindex;
> +}
> +
> +static int do_attach_detach_xdp(int progfd, enum net_attach_type attach_type,
> +				int ifindex, bool overwrite)
> +{
> +	__u32 flags = 0;
> +
> +	if (!overwrite)
> +		flags = XDP_FLAGS_UPDATE_IF_NOEXIST;
> +	if (attach_type == NET_ATTACH_TYPE_XDP_GENERIC)
> +		flags |= XDP_FLAGS_SKB_MODE;
> +	if (attach_type == NET_ATTACH_TYPE_XDP_DRIVER)
> +		flags |= XDP_FLAGS_DRV_MODE;
> +	if (attach_type == NET_ATTACH_TYPE_XDP_OFFLOAD)
> +		flags |= XDP_FLAGS_HW_MODE;
> +
> +	return bpf_set_link_xdp_fd(ifindex, progfd, flags);
> +}
> +
> +static int do_attach(int argc, char **argv)
> +{
> +	enum net_attach_type attach_type;
> +	int progfd, ifindex, err = 0;
> +	bool overwrite = false;
> +
> +	/* parse attach args */
> +	if (!REQ_ARGS(5))
> +		return -EINVAL;
> +
> +	attach_type = parse_attach_type(*argv);
> +	if (attach_type == max_net_attach_type) {
> +		p_err("invalid net attach/detach type");

worth adding the type to the error message so that user know which part
of command line was wrong:

	p_err("invalid net attach/detach type '%s'", *argv);

> +		return -EINVAL;
> +	}
> +
> +	NEXT_ARG();

nit: the new line should be before NEXT_ARG(), IOV NEXT_ARG() belongs
to the code which consumed the argument

> +	progfd = prog_parse_fd(&argc, &argv);
> +	if (progfd < 0)
> +		return -EINVAL;
> +
> +	ifindex = net_parse_dev(&argc, &argv);
> +	if (ifindex < 1) {
> +		close(progfd);
> +		return -EINVAL;
> +	}
> +
> +	if (argc) {
> +		if (is_prefix(*argv, "overwrite")) {
> +			overwrite = true;
> +		} else {
> +			p_err("expected 'overwrite', got: '%s'?", *argv);
> +			close(progfd);
> +			return -EINVAL;
> +		}
> +	}
> +
> +	/* attach xdp prog */
> +	if (is_prefix("xdp", attach_type_strings[attach_type]))

I'm still unclear on why this if is needed

> +		err = do_attach_detach_xdp(progfd, attach_type, ifindex,
> +					   overwrite);
> +
> +	if (err < 0) {
> +		p_err("interface %s attach failed",
> +		      attach_type_strings[attach_type]);

Please add the error string, like:

		p_err("interface %s attach failed: %s",
		      attach_type_strings[attach_type], strerror(errno));


> +		return err;
> +	}
> +
> +	if (json_output)
> +		jsonw_null(json_wtr);
> +
> +	return 0;
> +}
> +
>  static int do_show(int argc, char **argv)
>  {
>  	struct bpf_attach_info attach_info = {};
> @@ -231,17 +351,10 @@ static int do_show(int argc, char **argv)
>  	unsigned int nl_pid;
>  	char err_buf[256];
>  
> -	if (argc == 2) {
> -		if (strcmp(argv[0], "dev") != 0)
> -			usage();
> -		filter_idx = if_nametoindex(argv[1]);
> -		if (filter_idx == 0) {
> -			fprintf(stderr, "invalid dev name %s\n", argv[1]);
> -			return -1;
> -		}
> -	} else if (argc != 0) {
> +	if (argc == 2)
> +		filter_idx = net_parse_dev(&argc, &argv);

You should check filter_idx is not negative here, no?

> +	else if (argc != 0)
>  		usage();
> -	}
>  
>  	ret = query_flow_dissector(&attach_info);
>  	if (ret)

^ permalink raw reply

* Re: [v3,2/4] tools: bpftool: add net detach command to detach XDP on interface
From: Maciej Fijalkowski @ 2019-08-07 20:40 UTC (permalink / raw)
  To: Y Song
  Cc: Daniel T. Lee, Daniel Borkmann, Alexei Starovoitov, netdev,
	jakub.kicinski
In-Reply-To: <CAH3MdRX2SYj+79+L_FJtxMQZfPQDtYDFEbgH6VGAKMYnBXU4Vw@mail.gmail.com>

On Wed, 7 Aug 2019 13:12:17 -0700
Y Song <ys114321@gmail.com> wrote:

> On Wed, Aug 7, 2019 at 11:30 AM Maciej Fijalkowski
> <maciejromanfijalkowski@gmail.com> wrote:
> >
> > On Wed, 7 Aug 2019 10:02:04 -0700
> > Y Song <ys114321@gmail.com> wrote:
> >  
> > > On Tue, Aug 6, 2019 at 7:25 PM Daniel T. Lee <danieltimlee@gmail.com> wrote:  
> > > >
> > > > By this commit, using `bpftool net detach`, the attached XDP prog can
> > > > be detached. Detaching the BPF prog will be done through libbpf
> > > > 'bpf_set_link_xdp_fd' with the progfd set to -1.
> > > >
> > > > Signed-off-by: Daniel T. Lee <danieltimlee@gmail.com>
> > > > ---
> > > >  tools/bpf/bpftool/net.c | 42 ++++++++++++++++++++++++++++++++++++++++-
> > > >  1 file changed, 41 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/tools/bpf/bpftool/net.c b/tools/bpf/bpftool/net.c
> > > > index c05a3fac5cac..7be96acb08e0 100644
> > > > --- a/tools/bpf/bpftool/net.c
> > > > +++ b/tools/bpf/bpftool/net.c
> > > > @@ -343,6 +343,43 @@ static int do_attach(int argc, char **argv)
> > > >         return 0;
> > > >  }
> > > >
> > > > +static int do_detach(int argc, char **argv)
> > > > +{
> > > > +       enum net_attach_type attach_type;
> > > > +       int progfd, ifindex, err = 0;
> > > > +
> > > > +       /* parse detach args */
> > > > +       if (!REQ_ARGS(3))
> > > > +               return -EINVAL;
> > > > +
> > > > +       attach_type = parse_attach_type(*argv);
> > > > +       if (attach_type == max_net_attach_type) {
> > > > +               p_err("invalid net attach/detach type");
> > > > +               return -EINVAL;
> > > > +       }
> > > > +
> > > > +       NEXT_ARG();
> > > > +       ifindex = net_parse_dev(&argc, &argv);
> > > > +       if (ifindex < 1)
> > > > +               return -EINVAL;
> > > > +
> > > > +       /* detach xdp prog */
> > > > +       progfd = -1;
> > > > +       if (is_prefix("xdp", attach_type_strings[attach_type]))
> > > > +               err = do_attach_detach_xdp(progfd, attach_type, ifindex, NULL);  
> > >
> > > I found an issue here. This is probably related to do_attach_detach_xdp.
> > >
> > > -bash-4.4$ sudo ./bpftool net attach x pinned /sys/fs/bpf/xdp_example dev v1
> > > -bash-4.4$ sudo ./bpftool net
> > > xdp:
> > > v1(4) driver id 1172
> > >
> > > tc:
> > > eth0(2) clsact/ingress fbflow_icmp id 29 act []
> > > eth0(2) clsact/egress cls_fg_dscp_section id 27 act []
> > > eth0(2) clsact/egress fbflow_egress id 28
> > > eth0(2) clsact/egress fbflow_sslwall_egress id 35
> > >
> > > flow_dissector:
> > >
> > > -bash-4.4$ sudo ./bpftool net detach x dev v2  
> >
> > Shouldn't this be v1 as dev?  
> 
> I am testing a scenario where with wrong devname
> we did not return an error.

Ah ok. In this scenario if driver has a native xdp support we would be invoking
its ndo_bpf even if there's no prog currently attached and it wouldn't return
error value.

Looking at dev_xdp_uninstall, setting driver's prog to NULL is being done only
when prog is attached. Maybe we should consider querying the driver in
dev_change_xdp_fd regardless of passed fd value? E.g. don't query only when
prog >= 0.

I don't recall whether this was brought up previously.

CCing Jakub so we have one thread.

Maciej

> 
> Yes, if dev "v1", it works as expected.
> 
> >  
> > > -bash-4.4$ sudo ./bpftool net
> > > xdp:
> > > v1(4) driver id 1172
> > >
> > > tc:
> > > eth0(2) clsact/ingress fbflow_icmp id 29 act []
> > > eth0(2) clsact/egress cls_fg_dscp_section id 27 act []
> > > eth0(2) clsact/egress fbflow_egress id 28
> > > eth0(2) clsact/egress fbflow_sslwall_egress id 35
> > >
> > > flow_dissector:
> > >
> > > -bash-4.4$
> > >
> > > Basically detaching may fail due to wrong dev name or wrong type, etc.
> > > But the tool did not return an error. Is this expected?
> > > This may be related to this funciton "bpf_set_link_xdp_fd()".
> > > So this patch itself should be okay.
> > >  
> > > > +
> > > > +       if (err < 0) {
> > > > +               p_err("interface %s detach failed",
> > > > +                     attach_type_strings[attach_type]);
> > > > +               return err;
> > > > +       }
> > > > +
> > > > +       if (json_output)
> > > > +               jsonw_null(json_wtr);
> > > > +
> > > > +       return 0;
> > > > +}
> > > > +
> > > >  static int do_show(int argc, char **argv)
> > > >  {
> > > >         struct bpf_attach_info attach_info = {};
> > > > @@ -419,6 +456,7 @@ static int do_help(int argc, char **argv)
> > > >         fprintf(stderr,
> > > >                 "Usage: %s %s { show | list } [dev <devname>]\n"
> > > >                 "       %s %s attach ATTACH_TYPE PROG dev <devname> [ overwrite ]\n"
> > > > +               "       %s %s detach ATTACH_TYPE dev <devname>\n"
> > > >                 "       %s %s help\n"
> > > >                 "\n"
> > > >                 "       " HELP_SPEC_PROGRAM "\n"
> > > > @@ -429,7 +467,8 @@ static int do_help(int argc, char **argv)
> > > >                 "      to dump program attachments. For program types\n"
> > > >                 "      sk_{filter,skb,msg,reuseport} and lwt/seg6, please\n"
> > > >                 "      consult iproute2.\n",
> > > > -               bin_name, argv[-2], bin_name, argv[-2], bin_name, argv[-2]);
> > > > +               bin_name, argv[-2], bin_name, argv[-2], bin_name, argv[-2],
> > > > +               bin_name, argv[-2]);
> > > >
> > > >         return 0;
> > > >  }
> > > > @@ -438,6 +477,7 @@ static const struct cmd cmds[] = {
> > > >         { "show",       do_show },
> > > >         { "list",       do_show },
> > > >         { "attach",     do_attach },
> > > > +       { "detach",     do_detach },
> > > >         { "help",       do_help },
> > > >         { 0 }
> > > >  };
> > > > --
> > > > 2.20.1
> > > >  
> >  


^ permalink raw reply

* Re: memory leak in internal_dev_create
From: Pravin Shelar @ 2019-08-07 20:32 UTC (permalink / raw)
  To: Hillf Danton
  Cc: syzbot, David S. Miller, ovs dev, linux-kernel,
	Linux Kernel Network Developers, syzkaller-bugs
In-Reply-To: <20190806115932.3044-1-hdanton@sina.com>

On Tue, Aug 6, 2019 at 5:00 AM Hillf Danton <hdanton@sina.com> wrote:
>
>
> On Tue, 06 Aug 2019 01:58:05 -0700
> > Hello,
> >
> > syzbot found the following crash on:
> >

...
> > BUG: memory leak
> > unreferenced object 0xffff8881228ca500 (size 128):
> >    comm "syz-executor032", pid 7015, jiffies 4294944622 (age 7.880s)
> >    hex dump (first 32 bytes):
> >      00 f0 27 18 81 88 ff ff 80 ac 8c 22 81 88 ff ff  ..'........"....
> >      40 b7 23 17 81 88 ff ff 00 00 00 00 00 00 00 00  @.#.............
> >    backtrace:
> >      [<000000000eb78212>] kmemleak_alloc_recursive  include/linux/kmemleak.h:43 [inline]
> >      [<000000000eb78212>] slab_post_alloc_hook mm/slab.h:522 [inline]
> >      [<000000000eb78212>] slab_alloc mm/slab.c:3319 [inline]
> >      [<000000000eb78212>] kmem_cache_alloc_trace+0x145/0x2c0 mm/slab.c:3548
> >      [<00000000006ea6c6>] kmalloc include/linux/slab.h:552 [inline]
> >      [<00000000006ea6c6>] kzalloc include/linux/slab.h:748 [inline]
> >      [<00000000006ea6c6>] ovs_vport_alloc+0x37/0xf0  net/openvswitch/vport.c:130
> >      [<00000000f9a04a7d>] internal_dev_create+0x24/0x1d0  net/openvswitch/vport-internal_dev.c:164
> >      [<0000000056ee7c13>] ovs_vport_add+0x81/0x190  net/openvswitch/vport.c:199
> >      [<000000005434efc7>] new_vport+0x19/0x80 net/openvswitch/datapath.c:194
> >      [<00000000b7b253f1>] ovs_dp_cmd_new+0x22f/0x410  net/openvswitch/datapath.c:1614
> >      [<00000000e0988518>] genl_family_rcv_msg+0x2ab/0x5b0  net/netlink/genetlink.c:629
> >      [<00000000d0cc9347>] genl_rcv_msg+0x54/0x9c net/netlink/genetlink.c:654
> >      [<000000006694b647>] netlink_rcv_skb+0x61/0x170  net/netlink/af_netlink.c:2477
> >      [<0000000088381f37>] genl_rcv+0x29/0x40 net/netlink/genetlink.c:665
> >      [<00000000dad42a47>] netlink_unicast_kernel  net/netlink/af_netlink.c:1302 [inline]
> >      [<00000000dad42a47>] netlink_unicast+0x1ec/0x2d0  net/netlink/af_netlink.c:1328
> >      [<0000000067e6b079>] netlink_sendmsg+0x270/0x480  net/netlink/af_netlink.c:1917
> >      [<00000000aab08a47>] sock_sendmsg_nosec net/socket.c:637 [inline]
> >      [<00000000aab08a47>] sock_sendmsg+0x54/0x70 net/socket.c:657
> >      [<000000004cb7c11d>] ___sys_sendmsg+0x393/0x3c0 net/socket.c:2311
> >      [<00000000c4901c63>] __sys_sendmsg+0x80/0xf0 net/socket.c:2356
> >      [<00000000c10abb2d>] __do_sys_sendmsg net/socket.c:2365 [inline]
> >      [<00000000c10abb2d>] __se_sys_sendmsg net/socket.c:2363 [inline]
> >      [<00000000c10abb2d>] __x64_sys_sendmsg+0x23/0x30 net/socket.c:2363
>
>
> Always free vport manually unless register_netdevice() succeeds.
>
> --- a/net/openvswitch/vport-internal_dev.c
> +++ b/net/openvswitch/vport-internal_dev.c
> @@ -137,7 +137,7 @@ static void do_setup(struct net_device *
>         netdev->priv_flags |= IFF_LIVE_ADDR_CHANGE | IFF_OPENVSWITCH |
>                               IFF_NO_QUEUE;
>         netdev->needs_free_netdev = true;
> -       netdev->priv_destructor = internal_dev_destructor;
> +       netdev->priv_destructor = NULL;
>         netdev->ethtool_ops = &internal_dev_ethtool_ops;
>         netdev->rtnl_link_ops = &internal_dev_link_ops;
>
> @@ -159,7 +159,6 @@ static struct vport *internal_dev_create
>         struct internal_dev *internal_dev;
>         struct net_device *dev;
>         int err;
> -       bool free_vport = true;
>
>         vport = ovs_vport_alloc(0, &ovs_internal_vport_ops, parms);
>         if (IS_ERR(vport)) {
> @@ -190,10 +189,9 @@ static struct vport *internal_dev_create
>
>         rtnl_lock();
>         err = register_netdevice(vport->dev);
> -       if (err) {
> -               free_vport = false;
> +       if (err)
>                 goto error_unlock;
> -       }
> +       vport->dev->priv_destructor = internal_dev_destructor;
>
I am not sure why have you moved this assignment out of do_setup().

Otherwise patch looks good to me.

Thanks.
>         dev_set_promiscuity(vport->dev, 1);
>         rtnl_unlock();
> @@ -207,8 +205,7 @@ error_unlock:
>  error_free_netdev:
>         free_netdev(dev);
>  error_free_vport:
> -       if (free_vport)
> -               ovs_vport_free(vport);
> +       ovs_vport_free(vport);
>  error:
>         return ERR_PTR(err);
>  }
> --
>

^ permalink raw reply

* Re: [v3,2/4] tools: bpftool: add net detach command to detach XDP on interface
From: Y Song @ 2019-08-07 20:12 UTC (permalink / raw)
  To: Maciej Fijalkowski
  Cc: Daniel T. Lee, Daniel Borkmann, Alexei Starovoitov, netdev
In-Reply-To: <20190807203041.000020a8@gmail.com>

On Wed, Aug 7, 2019 at 11:30 AM Maciej Fijalkowski
<maciejromanfijalkowski@gmail.com> wrote:
>
> On Wed, 7 Aug 2019 10:02:04 -0700
> Y Song <ys114321@gmail.com> wrote:
>
> > On Tue, Aug 6, 2019 at 7:25 PM Daniel T. Lee <danieltimlee@gmail.com> wrote:
> > >
> > > By this commit, using `bpftool net detach`, the attached XDP prog can
> > > be detached. Detaching the BPF prog will be done through libbpf
> > > 'bpf_set_link_xdp_fd' with the progfd set to -1.
> > >
> > > Signed-off-by: Daniel T. Lee <danieltimlee@gmail.com>
> > > ---
> > >  tools/bpf/bpftool/net.c | 42 ++++++++++++++++++++++++++++++++++++++++-
> > >  1 file changed, 41 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/tools/bpf/bpftool/net.c b/tools/bpf/bpftool/net.c
> > > index c05a3fac5cac..7be96acb08e0 100644
> > > --- a/tools/bpf/bpftool/net.c
> > > +++ b/tools/bpf/bpftool/net.c
> > > @@ -343,6 +343,43 @@ static int do_attach(int argc, char **argv)
> > >         return 0;
> > >  }
> > >
> > > +static int do_detach(int argc, char **argv)
> > > +{
> > > +       enum net_attach_type attach_type;
> > > +       int progfd, ifindex, err = 0;
> > > +
> > > +       /* parse detach args */
> > > +       if (!REQ_ARGS(3))
> > > +               return -EINVAL;
> > > +
> > > +       attach_type = parse_attach_type(*argv);
> > > +       if (attach_type == max_net_attach_type) {
> > > +               p_err("invalid net attach/detach type");
> > > +               return -EINVAL;
> > > +       }
> > > +
> > > +       NEXT_ARG();
> > > +       ifindex = net_parse_dev(&argc, &argv);
> > > +       if (ifindex < 1)
> > > +               return -EINVAL;
> > > +
> > > +       /* detach xdp prog */
> > > +       progfd = -1;
> > > +       if (is_prefix("xdp", attach_type_strings[attach_type]))
> > > +               err = do_attach_detach_xdp(progfd, attach_type, ifindex, NULL);
> >
> > I found an issue here. This is probably related to do_attach_detach_xdp.
> >
> > -bash-4.4$ sudo ./bpftool net attach x pinned /sys/fs/bpf/xdp_example dev v1
> > -bash-4.4$ sudo ./bpftool net
> > xdp:
> > v1(4) driver id 1172
> >
> > tc:
> > eth0(2) clsact/ingress fbflow_icmp id 29 act []
> > eth0(2) clsact/egress cls_fg_dscp_section id 27 act []
> > eth0(2) clsact/egress fbflow_egress id 28
> > eth0(2) clsact/egress fbflow_sslwall_egress id 35
> >
> > flow_dissector:
> >
> > -bash-4.4$ sudo ./bpftool net detach x dev v2
>
> Shouldn't this be v1 as dev?

I am testing a scenario where with wrong devname
we did not return an error.

Yes, if dev "v1", it works as expected.

>
> > -bash-4.4$ sudo ./bpftool net
> > xdp:
> > v1(4) driver id 1172
> >
> > tc:
> > eth0(2) clsact/ingress fbflow_icmp id 29 act []
> > eth0(2) clsact/egress cls_fg_dscp_section id 27 act []
> > eth0(2) clsact/egress fbflow_egress id 28
> > eth0(2) clsact/egress fbflow_sslwall_egress id 35
> >
> > flow_dissector:
> >
> > -bash-4.4$
> >
> > Basically detaching may fail due to wrong dev name or wrong type, etc.
> > But the tool did not return an error. Is this expected?
> > This may be related to this funciton "bpf_set_link_xdp_fd()".
> > So this patch itself should be okay.
> >
> > > +
> > > +       if (err < 0) {
> > > +               p_err("interface %s detach failed",
> > > +                     attach_type_strings[attach_type]);
> > > +               return err;
> > > +       }
> > > +
> > > +       if (json_output)
> > > +               jsonw_null(json_wtr);
> > > +
> > > +       return 0;
> > > +}
> > > +
> > >  static int do_show(int argc, char **argv)
> > >  {
> > >         struct bpf_attach_info attach_info = {};
> > > @@ -419,6 +456,7 @@ static int do_help(int argc, char **argv)
> > >         fprintf(stderr,
> > >                 "Usage: %s %s { show | list } [dev <devname>]\n"
> > >                 "       %s %s attach ATTACH_TYPE PROG dev <devname> [ overwrite ]\n"
> > > +               "       %s %s detach ATTACH_TYPE dev <devname>\n"
> > >                 "       %s %s help\n"
> > >                 "\n"
> > >                 "       " HELP_SPEC_PROGRAM "\n"
> > > @@ -429,7 +467,8 @@ static int do_help(int argc, char **argv)
> > >                 "      to dump program attachments. For program types\n"
> > >                 "      sk_{filter,skb,msg,reuseport} and lwt/seg6, please\n"
> > >                 "      consult iproute2.\n",
> > > -               bin_name, argv[-2], bin_name, argv[-2], bin_name, argv[-2]);
> > > +               bin_name, argv[-2], bin_name, argv[-2], bin_name, argv[-2],
> > > +               bin_name, argv[-2]);
> > >
> > >         return 0;
> > >  }
> > > @@ -438,6 +477,7 @@ static const struct cmd cmds[] = {
> > >         { "show",       do_show },
> > >         { "list",       do_show },
> > >         { "attach",     do_attach },
> > > +       { "detach",     do_detach },
> > >         { "help",       do_help },
> > >         { 0 }
> > >  };
> > > --
> > > 2.20.1
> > >
>

^ permalink raw reply

* Re: [PATCH v4 bpf-next 02/14] libbpf: convert libbpf code to use new btf helpers
From: Andrii Nakryiko @ 2019-08-07 20:10 UTC (permalink / raw)
  To: Alexei Starovoitov
  Cc: Alexei Starovoitov, Andrii Nakryiko, bpf, Networking,
	Daniel Borkmann, Yonghong Song, Kernel Team
In-Reply-To: <151d13d1-e894-56cc-4690-4661c8afc65e@fb.com>

On Wed, Aug 7, 2019 at 1:01 PM Alexei Starovoitov <ast@fb.com> wrote:
>
> On 8/7/19 12:59 PM, Andrii Nakryiko wrote:
> > On Wed, Aug 7, 2019 at 12:30 PM Alexei Starovoitov
> > <alexei.starovoitov@gmail.com> wrote:
> >>
> >> On Tue, Aug 06, 2019 at 10:37:54PM -0700, Andrii Nakryiko wrote:
> >>> Simplify code by relying on newly added BTF helper functions.
> >>>
> >>> Signed-off-by: Andrii Nakryiko <andriin@fb.com>
> >> ..
> >>>
> >>> -     for (i = 0, vsi = (struct btf_var_secinfo *)(t + 1);
> >>> -          i < vars; i++, vsi++) {
> >>> +     for (i = 0, vsi = (void *)btf_var_secinfos(t); i < vars; i++, vsi++) {
> >>
> >>> +                     struct btf_member *m = (void *)btf_members(t);
> >> ...
> >>>                case BTF_KIND_ENUM: {
> >>> -                     struct btf_enum *m = (struct btf_enum *)(t + 1);
> >>> -                     __u16 vlen = BTF_INFO_VLEN(t->info);
> >>> +                     struct btf_enum *m = (void *)btf_enum(t);
> >>> +                     __u16 vlen = btf_vlen(t);
> >> ...
> >>>                case BTF_KIND_FUNC_PROTO: {
> >>> -                     struct btf_param *m = (struct btf_param *)(t + 1);
> >>> -                     __u16 vlen = BTF_INFO_VLEN(t->info);
> >>> +                     struct btf_param *m = (void *)btf_params(t);
> >>> +                     __u16 vlen = btf_vlen(t);
> >>
> >> So all of these 'void *' type hacks are only to drop const-ness ?
> >
> > Yes.
> >
> >> May be the helpers shouldn't be taking const then?
> >>
> >
> > Probably not, because then we'll have much wider-spread problem of
> > casting const pointers into non-const when passing btf_type into
> > helpers.
> > I think const as a default is the right choice, because normally BTF
> > is immutable and btf__type_by_id is returning const pointer, etc.
> > That's typical and expected use-case. btf_dedup and BTF sanitization +
> > datasec size setting pieces are an exception that have to modify BTF
> > types in place before passing it to user.
> >
> > So realistically I think we can just leave it as (void *), or I can do
> > explicit non-const type casts, or we can just not use helpers for
> > mutable cases. Do you have a preference?
>
> Hmm. Take a const into the helper and drop it there?
> I'd like to avoid all these 'void *'.

Well, I guess it's a way to do this as well. Given it's C, I guess
it's acceptable to be so free about constness. I'll update patches.

^ permalink raw reply

* Re: [PATCH v4 bpf-next 02/14] libbpf: convert libbpf code to use new btf helpers
From: Alexei Starovoitov @ 2019-08-07 20:01 UTC (permalink / raw)
  To: Andrii Nakryiko, Alexei Starovoitov
  Cc: Andrii Nakryiko, bpf, Networking, Daniel Borkmann, Yonghong Song,
	Kernel Team
In-Reply-To: <CAEf4BzahxLWRVNcNWpba7_7CbbQgN8k0RU8Ya1XCK8j4rPQ0NQ@mail.gmail.com>

On 8/7/19 12:59 PM, Andrii Nakryiko wrote:
> On Wed, Aug 7, 2019 at 12:30 PM Alexei Starovoitov
> <alexei.starovoitov@gmail.com> wrote:
>>
>> On Tue, Aug 06, 2019 at 10:37:54PM -0700, Andrii Nakryiko wrote:
>>> Simplify code by relying on newly added BTF helper functions.
>>>
>>> Signed-off-by: Andrii Nakryiko <andriin@fb.com>
>> ..
>>>
>>> -     for (i = 0, vsi = (struct btf_var_secinfo *)(t + 1);
>>> -          i < vars; i++, vsi++) {
>>> +     for (i = 0, vsi = (void *)btf_var_secinfos(t); i < vars; i++, vsi++) {
>>
>>> +                     struct btf_member *m = (void *)btf_members(t);
>> ...
>>>                case BTF_KIND_ENUM: {
>>> -                     struct btf_enum *m = (struct btf_enum *)(t + 1);
>>> -                     __u16 vlen = BTF_INFO_VLEN(t->info);
>>> +                     struct btf_enum *m = (void *)btf_enum(t);
>>> +                     __u16 vlen = btf_vlen(t);
>> ...
>>>                case BTF_KIND_FUNC_PROTO: {
>>> -                     struct btf_param *m = (struct btf_param *)(t + 1);
>>> -                     __u16 vlen = BTF_INFO_VLEN(t->info);
>>> +                     struct btf_param *m = (void *)btf_params(t);
>>> +                     __u16 vlen = btf_vlen(t);
>>
>> So all of these 'void *' type hacks are only to drop const-ness ?
> 
> Yes.
> 
>> May be the helpers shouldn't be taking const then?
>>
> 
> Probably not, because then we'll have much wider-spread problem of
> casting const pointers into non-const when passing btf_type into
> helpers.
> I think const as a default is the right choice, because normally BTF
> is immutable and btf__type_by_id is returning const pointer, etc.
> That's typical and expected use-case. btf_dedup and BTF sanitization +
> datasec size setting pieces are an exception that have to modify BTF
> types in place before passing it to user.
> 
> So realistically I think we can just leave it as (void *), or I can do
> explicit non-const type casts, or we can just not use helpers for
> mutable cases. Do you have a preference?

Hmm. Take a const into the helper and drop it there?
I'd like to avoid all these 'void *'.

^ permalink raw reply

* Re: [PATCH v4 bpf-next 02/14] libbpf: convert libbpf code to use new btf helpers
From: Andrii Nakryiko @ 2019-08-07 19:59 UTC (permalink / raw)
  To: Alexei Starovoitov
  Cc: Andrii Nakryiko, bpf, Networking, Alexei Starovoitov,
	Daniel Borkmann, Yonghong Song, Kernel Team
In-Reply-To: <20190807193011.g2zuaapc2uvvr4h6@ast-mbp>

On Wed, Aug 7, 2019 at 12:30 PM Alexei Starovoitov
<alexei.starovoitov@gmail.com> wrote:
>
> On Tue, Aug 06, 2019 at 10:37:54PM -0700, Andrii Nakryiko wrote:
> > Simplify code by relying on newly added BTF helper functions.
> >
> > Signed-off-by: Andrii Nakryiko <andriin@fb.com>
> ..
> >
> > -     for (i = 0, vsi = (struct btf_var_secinfo *)(t + 1);
> > -          i < vars; i++, vsi++) {
> > +     for (i = 0, vsi = (void *)btf_var_secinfos(t); i < vars; i++, vsi++) {
>
> > +                     struct btf_member *m = (void *)btf_members(t);
> ...
> >               case BTF_KIND_ENUM: {
> > -                     struct btf_enum *m = (struct btf_enum *)(t + 1);
> > -                     __u16 vlen = BTF_INFO_VLEN(t->info);
> > +                     struct btf_enum *m = (void *)btf_enum(t);
> > +                     __u16 vlen = btf_vlen(t);
> ...
> >               case BTF_KIND_FUNC_PROTO: {
> > -                     struct btf_param *m = (struct btf_param *)(t + 1);
> > -                     __u16 vlen = BTF_INFO_VLEN(t->info);
> > +                     struct btf_param *m = (void *)btf_params(t);
> > +                     __u16 vlen = btf_vlen(t);
>
> So all of these 'void *' type hacks are only to drop const-ness ?

Yes.

> May be the helpers shouldn't be taking const then?
>

Probably not, because then we'll have much wider-spread problem of
casting const pointers into non-const when passing btf_type into
helpers.
I think const as a default is the right choice, because normally BTF
is immutable and btf__type_by_id is returning const pointer, etc.
That's typical and expected use-case. btf_dedup and BTF sanitization +
datasec size setting pieces are an exception that have to modify BTF
types in place before passing it to user.

So realistically I think we can just leave it as (void *), or I can do
explicit non-const type casts, or we can just not use helpers for
mutable cases. Do you have a preference?

^ permalink raw reply

* [PATCH net 2/2] tc-testing: updated skbedit action tests with batch create/delete
From: Roman Mashak @ 2019-08-07 19:57 UTC (permalink / raw)
  To: davem; +Cc: netdev, kernel, jhs, xiyou.wangcong, jiri, Roman Mashak
In-Reply-To: <1565207849-11442-1-git-send-email-mrv@mojatatu.com>

Update TDC tests with cases varifying ability of TC to install or delete
batches of skbedit actions.

Signed-off-by: Roman Mashak <mrv@mojatatu.com>
---
 .../tc-testing/tc-tests/actions/skbedit.json       | 47 ++++++++++++++++++++++
 1 file changed, 47 insertions(+)

diff --git a/tools/testing/selftests/tc-testing/tc-tests/actions/skbedit.json b/tools/testing/selftests/tc-testing/tc-tests/actions/skbedit.json
index bf5ebf59c2d4..9cdd2e31ac2c 100644
--- a/tools/testing/selftests/tc-testing/tc-tests/actions/skbedit.json
+++ b/tools/testing/selftests/tc-testing/tc-tests/actions/skbedit.json
@@ -670,5 +670,52 @@
         "teardown": [
             "$TC actions flush action skbedit"
         ]
+    },
+    {
+        "id": "630c",
+        "name": "Add batch of 32 skbedit actions with all parameters and cookie",
+        "category": [
+            "actions",
+            "skbedit"
+        ],
+        "setup": [
+            [
+                "$TC actions flush action skbedit",
+                0,
+                1,
+                255
+            ]
+        ],
+        "cmdUnderTest": "bash -c \"for i in \\`seq 1 32\\`; do cmd=\\\"action skbedit queue_mapping 2 priority 10 mark 7/0xaabbccdd ptype host inheritdsfield index \\$i cookie aabbccddeeff112233445566778800a1 \\\"; args=\"\\$args\\$cmd\"; done && $TC actions add \\$args\"",
+        "expExitCode": "0",
+        "verifyCmd": "$TC actions list action skbedit",
+        "matchPattern": "^[ \t]+index [0-9]+ ref",
+        "matchCount": "32",
+        "teardown": [
+            "$TC actions flush action skbedit"
+        ]
+    },
+    {
+        "id": "706d",
+        "name": "Delete batch of 32 skbedit actions with all parameters",
+        "category": [
+            "actions",
+            "skbedit"
+        ],
+        "setup": [
+            [
+                "$TC actions flush action skbedit",
+                0,
+                1,
+                255
+            ],
+            "bash -c \"for i in \\`seq 1 32\\`; do cmd=\\\"action skbedit queue_mapping 2 priority 10 mark 7/0xaabbccdd ptype host inheritdsfield index \\$i \\\"; args=\\\"\\$args\\$cmd\\\"; done && $TC actions add \\$args\""
+        ],
+        "cmdUnderTest": "bash -c \"for i in \\`seq 1 32\\`; do cmd=\\\"action skbedit index \\$i \\\"; args=\"\\$args\\$cmd\"; done && $TC actions del \\$args\"",
+        "expExitCode": "0",
+        "verifyCmd": "$TC actions list action skbedit",
+        "matchPattern": "^[ \t]+index [0-9]+ ref",
+        "matchCount": "0",
+        "teardown": []
     }
 ]
-- 
2.7.4


^ permalink raw reply related

* [PATCH net 1/2] net sched: update skbedit action for batched events operations
From: Roman Mashak @ 2019-08-07 19:57 UTC (permalink / raw)
  To: davem; +Cc: netdev, kernel, jhs, xiyou.wangcong, jiri, Roman Mashak
In-Reply-To: <1565207849-11442-1-git-send-email-mrv@mojatatu.com>

Add get_fill_size() routine used to calculate the action size
when building a batch of events.

Fixes: ca9b0e27e ("pkt_action: add new action skbedit")
Signed-off-by: Roman Mashak <mrv@mojatatu.com>
---
 net/sched/act_skbedit.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/net/sched/act_skbedit.c b/net/sched/act_skbedit.c
index b100870f02a6..37dced00b63d 100644
--- a/net/sched/act_skbedit.c
+++ b/net/sched/act_skbedit.c
@@ -307,6 +307,17 @@ static int tcf_skbedit_search(struct net *net, struct tc_action **a, u32 index)
 	return tcf_idr_search(tn, a, index);
 }
 
+static size_t tcf_skbedit_get_fill_size(const struct tc_action *act)
+{
+	return nla_total_size(sizeof(struct tc_skbedit))
+		+ nla_total_size(sizeof(u32)) /* TCA_SKBEDIT_PRIORITY */
+		+ nla_total_size(sizeof(u16)) /* TCA_SKBEDIT_QUEUE_MAPPING */
+		+ nla_total_size(sizeof(u32)) /* TCA_SKBEDIT_MARK */
+		+ nla_total_size(sizeof(u16)) /* TCA_SKBEDIT_PTYPE */
+		+ nla_total_size(sizeof(u32)) /* TCA_SKBEDIT_MASK */
+		+ nla_total_size_64bit(sizeof(u64)); /* TCA_SKBEDIT_FLAGS */
+}
+
 static struct tc_action_ops act_skbedit_ops = {
 	.kind		=	"skbedit",
 	.id		=	TCA_ID_SKBEDIT,
@@ -316,6 +327,7 @@ static struct tc_action_ops act_skbedit_ops = {
 	.init		=	tcf_skbedit_init,
 	.cleanup	=	tcf_skbedit_cleanup,
 	.walk		=	tcf_skbedit_walker,
+	.get_fill_size	=	tcf_skbedit_get_fill_size,
 	.lookup		=	tcf_skbedit_search,
 	.size		=	sizeof(struct tcf_skbedit),
 };
-- 
2.7.4


^ permalink raw reply related

* [PATCH net 0/2] Fix batched event generation for skbedit action
From: Roman Mashak @ 2019-08-07 19:57 UTC (permalink / raw)
  To: davem; +Cc: netdev, kernel, jhs, xiyou.wangcong, jiri, Roman Mashak

When adding or deleting a batch of entries, the kernel sends up to
TCA_ACT_MAX_PRIO (defined to 32 in kernel) entries in an event to user
space. However it does not consider that the action sizes may vary and
require different skb sizes.

For example, consider the following script adding 32 entries with all
supported skbedit parameters and cookie (in order to maximize netlink
messages size):

% cat tc-batch.sh
TC="sudo /mnt/iproute2.git/tc/tc"

$TC actions flush action skbedit
for i in `seq 1 $1`;
do
   cmd="action skbedit queue_mapping 2 priority 10 mark 7/0xaabbccdd \
               ptype host inheritdsfield \
               index $i cookie aabbccddeeff112233445566778800a1 "
   args=$args$cmd
done
$TC actions add $args
%
% ./tc-batch.sh 32
Error: Failed to fill netlink attributes while adding TC action.
We have an error talking to the kernel
%

patch 1 adds callback in tc_action_ops of skbedit action, which calculates
the action size, and passes size to tcf_add_notify()/tcf_del_notify().

patch 2 updates the TDC test suite with relevant skbedit test cases.

Roman Mashak (2):
  net sched: update skbedit action for batched events operations
  tc-testing: updated skbedit action tests with batch create/delete

 net/sched/act_skbedit.c                            | 12 ++++++
 .../tc-testing/tc-tests/actions/skbedit.json       | 47 ++++++++++++++++++++++
 2 files changed, 59 insertions(+)

-- 
2.7.4


^ permalink raw reply

* Re: [PATCH v4 bpf-next 01/14] libbpf: add helpers for working with BTF types
From: Andrii Nakryiko @ 2019-08-07 19:54 UTC (permalink / raw)
  To: Alexei Starovoitov
  Cc: Andrii Nakryiko, bpf, Networking, Alexei Starovoitov,
	Daniel Borkmann, Yonghong Song, Kernel Team
In-Reply-To: <20190807193110.p5flmxojmdjdg4dj@ast-mbp>

On Wed, Aug 7, 2019 at 12:31 PM Alexei Starovoitov
<alexei.starovoitov@gmail.com> wrote:
>
> On Tue, Aug 06, 2019 at 10:37:53PM -0700, Andrii Nakryiko wrote:
> > Add lots of frequently used helpers that simplify working with BTF
> > types.
> >
> > Signed-off-by: Andrii Nakryiko <andriin@fb.com>
> ..
> > +/* get bitfield size of a member, assuming t is BTF_KIND_STRUCT or
> > + * BTF_KIND_UNION. If member is not a bitfield, zero is returned. */
>
> Invalid comment style.
>

oops, fixing

^ permalink raw reply

* Re: [PATCH net v2] net/tls: prevent skb_orphan() from leaking TLS plain text with offload
From: Willem de Bruijn @ 2019-08-07 19:54 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: David Miller, Network Development, davejwatson, borisp, aviadye,
	John Fastabend, Daniel Borkmann, Eric Dumazet, Alexei Starovoitov,
	oss-drivers
In-Reply-To: <CA+FuTSeR1QqAZVTLQ-mJ8iHi+h+ghbrGyT6TWATTecQSbQP6sA@mail.gmail.com>

On Wed, Aug 7, 2019 at 2:47 PM Willem de Bruijn
<willemdebruijn.kernel@gmail.com> wrote:
>
> On Wed, Aug 7, 2019 at 2:01 PM Jakub Kicinski
> <jakub.kicinski@netronome.com> wrote:
> >
> > On Wed, 7 Aug 2019 12:59:00 -0400, Willem de Bruijn wrote:
> > > On Wed, Aug 7, 2019 at 2:06 AM Jakub Kicinski wrote:
> > > > diff --git a/net/core/sock.c b/net/core/sock.c
> > > > index d57b0cc995a0..0f9619b0892f 100644
> > > > --- a/net/core/sock.c
> > > > +++ b/net/core/sock.c
> > > > @@ -1992,6 +1992,20 @@ void skb_set_owner_w(struct sk_buff *skb, struct sock *sk)
> > > >  }
> > > >  EXPORT_SYMBOL(skb_set_owner_w);
> > > >
> > > > +static bool can_skb_orphan_partial(const struct sk_buff *skb)
> > > > +{
> > > > +#ifdef CONFIG_TLS_DEVICE
> > > > +       /* Drivers depend on in-order delivery for crypto offload,
> > > > +        * partial orphan breaks out-of-order-OK logic.
> > > > +        */
> > > > +       if (skb->decrypted)
> > > > +               return false;
> > > > +#endif
> > > > +       return (IS_ENABLED(CONFIG_INET) &&
> > > > +               skb->destructor == tcp_wfree) ||
> > >
> > > Please add parentheses around IS_ENABLED(CONFIG_INET) &&
> > > skb->destructor == tcp_wfree
> >
> > Mm.. there are parenthesis around them, maybe I'm being slow,
> > could you show me how?
>
> I mean
>
>     return (skb->destructor == sock_wfree ||
>                (IS_ENABLED(CONFIG_INET) && skb->destructor == tcp_wfree))
>
> In other words, (a || (b && c)) instead of (a || b && c). Though the
> existing code also eschews the extra parentheses.

No, ignore that last bit. It uses #ifdef, of course.

^ permalink raw reply

* Why is this WiFi booster so popular across the World?
From:  Horace Harrington @ 2019-08-07 19:24 UTC (permalink / raw)
  To: netdev

Internet providers make big money by overcharging you for faster internet lines. Could this little device really be the one solution that we've all been waiting for?

Frequency: 2.4Ghz
Wireless Rate: 300Mbps
Interface: 1 10/100Mbps WAN/LAN RJ45 Ports

Amazing new technology find out here! http://bit.ly/asdf411gt

------------------------
If you'd prefer not to receive future emails, Unsubscribe Here http://bit.ly/itr4fgy
11041 Santa Monica Blvd. #301 Los Angeles, CA 90025


^ permalink raw reply

* [PATCH net-next] r8169: allocate rx buffers using alloc_pages_node
From: Heiner Kallweit @ 2019-08-07 19:38 UTC (permalink / raw)
  To: Realtek linux nic maintainers, David Miller; +Cc: netdev@vger.kernel.org

We allocate 16kb per rx buffer, so we can avoid some overhead by using
alloc_pages_node directly instead of bothering kmalloc_node. Due to
this change buffers are page-aligned now, therefore the alignment check
can be removed.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/net/ethernet/realtek/r8169_main.c | 45 ++++++++++-------------
 1 file changed, 19 insertions(+), 26 deletions(-)

diff --git a/drivers/net/ethernet/realtek/r8169_main.c b/drivers/net/ethernet/realtek/r8169_main.c
index 3c7af6669..4f0b32280 100644
--- a/drivers/net/ethernet/realtek/r8169_main.c
+++ b/drivers/net/ethernet/realtek/r8169_main.c
@@ -642,7 +642,7 @@ struct rtl8169_private {
 	struct RxDesc *RxDescArray;	/* 256-aligned Rx descriptor ring */
 	dma_addr_t TxPhyAddr;
 	dma_addr_t RxPhyAddr;
-	void *Rx_databuff[NUM_RX_DESC];	/* Rx data buffers */
+	struct page *Rx_databuff[NUM_RX_DESC];	/* Rx data buffers */
 	struct ring_info tx_skb[NUM_TX_DESC];	/* Tx data buffers */
 	u16 cp_cmd;
 	u16 irq_mask;
@@ -5261,12 +5261,13 @@ static inline void rtl8169_make_unusable_by_asic(struct RxDesc *desc)
 }
 
 static void rtl8169_free_rx_databuff(struct rtl8169_private *tp,
-				     void **data_buff, struct RxDesc *desc)
+				     struct page **data_buff,
+				     struct RxDesc *desc)
 {
-	dma_unmap_single(tp_to_dev(tp), le64_to_cpu(desc->addr),
-			 R8169_RX_BUF_SIZE, DMA_FROM_DEVICE);
+	dma_unmap_page(tp_to_dev(tp), le64_to_cpu(desc->addr),
+		       R8169_RX_BUF_SIZE, DMA_FROM_DEVICE);
 
-	kfree(*data_buff);
+	__free_pages(*data_buff, get_order(R8169_RX_BUF_SIZE));
 	*data_buff = NULL;
 	rtl8169_make_unusable_by_asic(desc);
 }
@@ -5281,38 +5282,30 @@ static inline void rtl8169_mark_to_asic(struct RxDesc *desc)
 	desc->opts1 = cpu_to_le32(DescOwn | eor | R8169_RX_BUF_SIZE);
 }
 
-static struct sk_buff *rtl8169_alloc_rx_data(struct rtl8169_private *tp,
-					     struct RxDesc *desc)
+static struct page *rtl8169_alloc_rx_data(struct rtl8169_private *tp,
+					  struct RxDesc *desc)
 {
-	void *data;
-	dma_addr_t mapping;
 	struct device *d = tp_to_dev(tp);
 	int node = dev_to_node(d);
+	dma_addr_t mapping;
+	struct page *data;
 
-	data = kmalloc_node(R8169_RX_BUF_SIZE, GFP_KERNEL, node);
+	data = alloc_pages_node(node, GFP_KERNEL, get_order(R8169_RX_BUF_SIZE));
 	if (!data)
 		return NULL;
 
-	/* Memory should be properly aligned, but better check. */
-	if (!IS_ALIGNED((unsigned long)data, 8)) {
-		netdev_err_once(tp->dev, "RX buffer not 8-byte-aligned\n");
-		goto err_out;
-	}
-
-	mapping = dma_map_single(d, data, R8169_RX_BUF_SIZE, DMA_FROM_DEVICE);
+	mapping = dma_map_page(d, data, 0, R8169_RX_BUF_SIZE, DMA_FROM_DEVICE);
 	if (unlikely(dma_mapping_error(d, mapping))) {
 		if (net_ratelimit())
 			netif_err(tp, drv, tp->dev, "Failed to map RX DMA!\n");
-		goto err_out;
+		__free_pages(data, get_order(R8169_RX_BUF_SIZE));
+		return NULL;
 	}
 
 	desc->addr = cpu_to_le64(mapping);
 	rtl8169_mark_to_asic(desc);
-	return data;
 
-err_out:
-	kfree(data);
-	return NULL;
+	return data;
 }
 
 static void rtl8169_rx_clear(struct rtl8169_private *tp)
@@ -5337,7 +5330,7 @@ static int rtl8169_rx_fill(struct rtl8169_private *tp)
 	unsigned int i;
 
 	for (i = 0; i < NUM_RX_DESC; i++) {
-		void *data;
+		struct page *data;
 
 		data = rtl8169_alloc_rx_data(tp, tp->RxDescArray + i);
 		if (!data) {
@@ -5892,6 +5885,7 @@ static int rtl_rx(struct net_device *dev, struct rtl8169_private *tp, u32 budget
 
 	for (rx_left = min(budget, NUM_RX_DESC); rx_left > 0; rx_left--, cur_rx++) {
 		unsigned int entry = cur_rx % NUM_RX_DESC;
+		const void *rx_buf = page_address(tp->Rx_databuff[entry]);
 		struct RxDesc *desc = tp->RxDescArray + entry;
 		u32 status;
 
@@ -5946,9 +5940,8 @@ static int rtl_rx(struct net_device *dev, struct rtl8169_private *tp, u32 budget
 				goto release_descriptor;
 			}
 
-			prefetch(tp->Rx_databuff[entry]);
-			skb_copy_to_linear_data(skb, tp->Rx_databuff[entry],
-						pkt_size);
+			prefetch(rx_buf);
+			skb_copy_to_linear_data(skb, rx_buf, pkt_size);
 			skb->tail += pkt_size;
 			skb->len = pkt_size;
 
-- 
2.22.0


^ permalink raw reply related

* Re: [PATCH net] netdevsim: Restore per-network namespace accounting for fib entries
From: David Ahern @ 2019-08-07 19:31 UTC (permalink / raw)
  To: Jiri Pirko; +Cc: Jakub Kicinski, David Ahern, davem, netdev
In-Reply-To: <20190807130739.GA2201@nanopsycho>

On 8/7/19 7:07 AM, Jiri Pirko wrote:
> 
> Yeah. I believe it was a mistake to add it in the first place. Abuses
> netdevsim for something it is not. I'm fine to use devlink the way you
> want to after we conclude 2), but outside netdevsim.
> 
> Again, netdevsim is there for config api testing purposes. If things
> got broken, it is not that bit deal. I broke the way it is
> instantiated significantly for example (iplink->sysfs).
>

netdevsim was created as a way of testing hardware focused kernel APIs
and code paths without hardware. yes?

The devlink api was added to netdevsim to test fib notifiers failing by
handlers. The notifiers were added for mlxsw - a hardware driver - as a
way to get notifications of fib changes.

The easiest way to test the error paths was to code limits to fib
entries very similar to what mlxsw implements with its 'devlink
resource' implementation. ie., to implement 'devlink resource' for a
software emulated device. netdevsim was the most logical place for it.

See the commit logs starting at:

commit b349e0b5ec5d7be57ac243fb08ae8b994c928165
Merge: 6e2135ce54b7 37923ed6b8ce
Author: David S. Miller <davem@davemloft.net>
Date:   Thu Mar 29 14:10:31 2018 -0400

    Merge branch 'net-Allow-FIB-notifiers-to-fail-add-and-replace'

    David Ahern says:

    ====================
    net: Allow FIB notifiers to fail add and replace


Everything about that implementation is using the s/w device to test
code paths targeted at hardware offloads.

^ permalink raw reply

* Re: [PATCH v4 bpf-next 01/14] libbpf: add helpers for working with BTF types
From: Alexei Starovoitov @ 2019-08-07 19:31 UTC (permalink / raw)
  To: Andrii Nakryiko
  Cc: bpf, netdev, ast, daniel, yhs, andrii.nakryiko, kernel-team
In-Reply-To: <20190807053806.1534571-2-andriin@fb.com>

On Tue, Aug 06, 2019 at 10:37:53PM -0700, Andrii Nakryiko wrote:
> Add lots of frequently used helpers that simplify working with BTF
> types.
> 
> Signed-off-by: Andrii Nakryiko <andriin@fb.com>
..
> +/* get bitfield size of a member, assuming t is BTF_KIND_STRUCT or
> + * BTF_KIND_UNION. If member is not a bitfield, zero is returned. */

Invalid comment style.


^ permalink raw reply

* Re: [PATCH v4 bpf-next 02/14] libbpf: convert libbpf code to use new btf helpers
From: Alexei Starovoitov @ 2019-08-07 19:30 UTC (permalink / raw)
  To: Andrii Nakryiko
  Cc: bpf, netdev, ast, daniel, yhs, andrii.nakryiko, kernel-team
In-Reply-To: <20190807053806.1534571-3-andriin@fb.com>

On Tue, Aug 06, 2019 at 10:37:54PM -0700, Andrii Nakryiko wrote:
> Simplify code by relying on newly added BTF helper functions.
> 
> Signed-off-by: Andrii Nakryiko <andriin@fb.com>
..
>  
> -	for (i = 0, vsi = (struct btf_var_secinfo *)(t + 1);
> -	     i < vars; i++, vsi++) {
> +	for (i = 0, vsi = (void *)btf_var_secinfos(t); i < vars; i++, vsi++) {

> +			struct btf_member *m = (void *)btf_members(t);
...
>  		case BTF_KIND_ENUM: {
> -			struct btf_enum *m = (struct btf_enum *)(t + 1);
> -			__u16 vlen = BTF_INFO_VLEN(t->info);
> +			struct btf_enum *m = (void *)btf_enum(t);
> +			__u16 vlen = btf_vlen(t);
...
>  		case BTF_KIND_FUNC_PROTO: {
> -			struct btf_param *m = (struct btf_param *)(t + 1);
> -			__u16 vlen = BTF_INFO_VLEN(t->info);
> +			struct btf_param *m = (void *)btf_params(t);
> +			__u16 vlen = btf_vlen(t);

So all of these 'void *' type hacks are only to drop const-ness ?
May be the helpers shouldn't be taking const then?


^ permalink raw reply

* Re: [PATCH net-next v4 2/2] net: phy: broadcom: add 1000Base-X support for BCM54616S
From: Heiner Kallweit @ 2019-08-07 19:18 UTC (permalink / raw)
  To: Tao Ren, Andrew Lunn, Florian Fainelli, David S . Miller,
	Arun Parameswaran, Justin Chen, Vladimir Oltean,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
	openbmc@lists.ozlabs.org
In-Reply-To: <fe0d39ea-91f3-0cac-f13b-3d46ea1748a3@fb.com>

On 06.08.2019 23:42, Tao Ren wrote:
> Hi Andrew / Heiner / Vladimir,
> 
> On 8/6/19 2:09 PM, Tao Ren wrote:
>> The BCM54616S PHY cannot work properly in RGMII->1000Base-KX mode (for
>> example, on Facebook CMM BMC platform), mainly because genphy functions
>> are designed for copper links, and 1000Base-X (clause 37) auto negotiation
>> needs to be handled differently.
>>
>> This patch enables 1000Base-X support for BCM54616S by customizing 3
>> driver callbacks:
>>
>>   - probe: probe callback detects PHY's operation mode based on
>>     INTERF_SEL[1:0] pins and 1000X/100FX selection bit in SerDES 100-FX
>>     Control register.
>>
>>   - config_aneg: bcm54616s_config_aneg_1000bx function is added for auto
>>     negotiation in 1000Base-X mode.
>>
>>   - read_status: BCM54616S and BCM5482 PHY share the same read_status
>>     callback which manually set link speed and duplex mode in 1000Base-X
>>     mode.
>>
>> Signed-off-by: Tao Ren <taoren@fb.com>
> 
> I customized config_aneg function for BCM54616S 1000Base-X mode and link-down issue is also fixed: the patch is tested on Facebook CMM and Minipack BMC and everything looks normal. Please kindly review when you have bandwidth and let me know if you have further suggestions.
> 
> BTW, I would be happy to help if we decide to add a set of genphy functions for clause 37, although that may mean I need more help/guidance from you :-)

You want to have standard clause 37 aneg and this should be generic in phylib.
I hacked together a first version that is compile-tested only:
https://patchwork.ozlabs.org/patch/1143631/
It supports fixed mode too.

It doesn't support half duplex mode because phylib doesn't know 1000BaseX HD yet.
Not sure whether half duplex mode is used at all in reality.

You could test the new core functions in your own config_aneg and read_status
callback implementations.

> 
> 
> Cheers,
> 
> Tao
> 
Heiner

^ permalink raw reply

* Re: [v3,2/4] tools: bpftool: add net detach command to detach XDP on interface
From: Jakub Kicinski @ 2019-08-07 19:14 UTC (permalink / raw)
  To: Y Song; +Cc: Daniel T. Lee, Daniel Borkmann, Alexei Starovoitov, netdev
In-Reply-To: <CAH3MdRW4LgdLoqSpLsWUOwjnNhJA1sodHqSD2Z14JY6aHMaKxg@mail.gmail.com>

On Wed, 7 Aug 2019 10:02:04 -0700, Y Song wrote:
> -bash-4.4$ sudo ./bpftool net detach x dev v2
> -bash-4.4$ sudo ./bpftool net
> xdp:
> v1(4) driver id 1172
> 
> tc:
> eth0(2) clsact/ingress fbflow_icmp id 29 act []
> eth0(2) clsact/egress cls_fg_dscp_section id 27 act []
> eth0(2) clsact/egress fbflow_egress id 28
> eth0(2) clsact/egress fbflow_sslwall_egress id 35
> 
> flow_dissector:
> 
> -bash-4.4$
> 
> Basically detaching may fail due to wrong dev name or wrong type, etc.
> But the tool did not return an error. Is this expected?
> This may be related to this funciton "bpf_set_link_xdp_fd()".
> So this patch itself should be okay.

Yup, I'm pretty sure kernel doesn't return errors on unset if there 
is no program on the interface.

^ permalink raw reply

* [PATCH RFC] net: phy: add support for clause 37 auto-negotiation
From: Heiner Kallweit @ 2019-08-07 19:11 UTC (permalink / raw)
  To: Andrew Lunn, Florian Fainelli, David Miller; +Cc: netdev@vger.kernel.org

This patch adds support for clause 37 1000Base-X auto-negotiation.
It's compile-tested only as I don't have fiber equipment.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/net/phy/phy_device.c | 139 +++++++++++++++++++++++++++++++++++
 include/linux/phy.h          |   5 ++
 2 files changed, 144 insertions(+)

diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index 7ddd91df9..f24e3e7e5 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -1617,6 +1617,40 @@ static int genphy_config_advert(struct phy_device *phydev)
 	return changed;
 }
 
+/**
+ * genphy_c37_config_advert - sanitize and advertise auto-negotiation parameters
+ * @phydev: target phy_device struct
+ *
+ * Description: Writes MII_ADVERTISE with the appropriate values,
+ *   after sanitizing the values to make sure we only advertise
+ *   what is supported.  Returns < 0 on error, 0 if the PHY's advertisement
+ *   hasn't changed, and > 0 if it has changed. This function is intended
+ *   for Clause 37 1000Base-X mode.
+ */
+static int genphy_c37_config_advert(struct phy_device *phydev)
+{
+	u16 adv = 0;
+
+	/* Only allow advertising what this PHY supports */
+	linkmode_and(phydev->advertising, phydev->advertising,
+		     phydev->supported);
+
+	if (linkmode_test_bit(ETHTOOL_LINK_MODE_1000baseX_Full_BIT,
+			      phydev->advertising))
+		adv |= ADVERTISE_1000XFULL;
+	if (linkmode_test_bit(ETHTOOL_LINK_MODE_Pause_BIT,
+			      phydev->advertising))
+		adv |= ADVERTISE_1000XPAUSE;
+	if (linkmode_test_bit(ETHTOOL_LINK_MODE_Asym_Pause_BIT,
+			      phydev->advertising))
+		adv |= ADVERTISE_1000XPSE_ASYM;
+
+	return phy_modify_changed(phydev, MII_ADVERTISE,
+				  ADVERTISE_1000XFULL | ADVERTISE_1000XPAUSE |
+				  ADVERTISE_1000XHALF | ADVERTISE_1000XPSE_ASYM,
+				  adv);
+}
+
 /**
  * genphy_config_eee_advert - disable unwanted eee mode advertisement
  * @phydev: target phy_device struct
@@ -1726,6 +1760,54 @@ int genphy_config_aneg(struct phy_device *phydev)
 }
 EXPORT_SYMBOL(genphy_config_aneg);
 
+/**
+ * genphy_c37_config_aneg - restart auto-negotiation or write BMCR
+ * @phydev: target phy_device struct
+ *
+ * Description: If auto-negotiation is enabled, we configure the
+ *   advertising, and then restart auto-negotiation.  If it is not
+ *   enabled, then we write the BMCR. This function is intended
+ *   for use with Clause 37 1000Base-X mode.
+ */
+int genphy_c37_config_aneg(struct phy_device *phydev)
+{
+	int err, changed;
+
+	if (AUTONEG_ENABLE != phydev->autoneg)
+		return genphy_setup_forced(phydev);
+
+	err = phy_modify(phydev, MII_BMCR, BMCR_SPEED1000 | BMCR_SPEED100,
+			 BMCR_SPEED1000);
+	if (err)
+		return err;
+
+	changed = genphy_c37_config_advert(phydev);
+	if (changed < 0) /* error */
+		return changed;
+
+	if (!changed) {
+		/* Advertisement hasn't changed, but maybe aneg was never on to
+		 * begin with?  Or maybe phy was isolated?
+		 */
+		int ctl = phy_read(phydev, MII_BMCR);
+
+		if (ctl < 0)
+			return ctl;
+
+		if (!(ctl & BMCR_ANENABLE) || (ctl & BMCR_ISOLATE))
+			changed = 1; /* do restart aneg */
+	}
+
+	/* Only restart aneg if we are advertising something different
+	 * than we were before.
+	 */
+	if (changed > 0)
+		return genphy_restart_aneg(phydev);
+
+	return 0;
+}
+EXPORT_SYMBOL(genphy_c37_config_aneg);
+
 /**
  * genphy_aneg_done - return auto-negotiation status
  * @phydev: target phy_device struct
@@ -1864,6 +1946,63 @@ int genphy_read_status(struct phy_device *phydev)
 }
 EXPORT_SYMBOL(genphy_read_status);
 
+/**
+ * genphy_c37_read_status - check the link status and update current link state
+ * @phydev: target phy_device struct
+ *
+ * Description: Check the link, then figure out the current state
+ *   by comparing what we advertise with what the link partner
+ *   advertises. This function is for Clause 37 1000Base-X mode.
+ */
+int genphy_c37_read_status(struct phy_device *phydev)
+{
+	int lpa, err, old_link = phydev->link;
+
+	/* Update the link, but return if there was an error */
+	err = genphy_update_link(phydev);
+	if (err)
+		return err;
+
+	/* why bother the PHY if nothing can have changed */
+	if (phydev->autoneg == AUTONEG_ENABLE && old_link && phydev->link)
+		return 0;
+
+	phydev->duplex = DUPLEX_UNKNOWN;
+	phydev->pause = 0;
+	phydev->asym_pause = 0;
+
+	if (phydev->autoneg == AUTONEG_ENABLE && phydev->autoneg_complete) {
+		lpa = phy_read(phydev, MII_LPA);
+		if (lpa < 0)
+			return lpa;
+
+		linkmode_mod_bit(ETHTOOL_LINK_MODE_Autoneg_BIT,
+				 phydev->lp_advertising, lpa & LPA_LPACK);
+		linkmode_mod_bit(ETHTOOL_LINK_MODE_1000baseX_Full_BIT,
+				 phydev->lp_advertising, lpa & LPA_1000XFULL);
+		linkmode_mod_bit(ETHTOOL_LINK_MODE_Pause_BIT,
+				 phydev->lp_advertising, lpa & LPA_1000XPAUSE);
+		linkmode_mod_bit(ETHTOOL_LINK_MODE_Asym_Pause_BIT,
+				 phydev->lp_advertising,
+				 lpa & LPA_1000XPAUSE_ASYM);
+
+		phy_resolve_aneg_linkmode(phydev);
+	} else if (phydev->autoneg == AUTONEG_DISABLE) {
+		int bmcr = phy_read(phydev, MII_BMCR);
+
+		if (bmcr < 0)
+			return bmcr;
+
+		if (bmcr & BMCR_FULLDPLX)
+			phydev->duplex = DUPLEX_FULL;
+		else
+			phydev->duplex = DUPLEX_HALF;
+	}
+
+	return 0;
+}
+EXPORT_SYMBOL(genphy_c37_read_status);
+
 /**
  * genphy_soft_reset - software reset the PHY via BMCR_RESET bit
  * @phydev: target phy_device struct
diff --git a/include/linux/phy.h b/include/linux/phy.h
index 462b90b73..36cbdfe84 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -1077,6 +1077,11 @@ int genphy_suspend(struct phy_device *phydev);
 int genphy_resume(struct phy_device *phydev);
 int genphy_loopback(struct phy_device *phydev, bool enable);
 int genphy_soft_reset(struct phy_device *phydev);
+
+/* Clause 37 */
+int genphy_c37_aneg_done(struct phy_device *phydev);
+int genphy_c37_read_status(struct phy_device *phydev);
+
 static inline int genphy_no_soft_reset(struct phy_device *phydev)
 {
 	return 0;
-- 
2.22.0


^ permalink raw reply related

* Re: [PATCH iproute2-next v2] ip tunnel: add json output
From: David Ahern @ 2019-08-07 19:08 UTC (permalink / raw)
  To: Andrea Claudi, netdev; +Cc: stephen, dsahern
In-Reply-To: <a7a161117f3a68e5a0cea008a8ca7e80b42bf2fa.1564766777.git.aclaudi@redhat.com>

On 8/2/19 11:38 AM, Andrea Claudi wrote:
> Add json support on iptunnel and ip6tunnel.
> The plain text output format should remain the same.
> 
> Signed-off-by: Andrea Claudi <aclaudi@redhat.com>
> ---
> Changes since v1:
>  * Use print_color_* for ifname and ip addresses;
>  * Use print_null() instead of print_bool() where appropriate;
>  * Reduce indentation level on tnl_print_gre_flags()
> ---
>  ip/ip6tunnel.c | 66 ++++++++++++++++++++++++---------------
>  ip/iptunnel.c  | 84 ++++++++++++++++++++++++++++++++------------------
>  ip/tunnel.c    | 37 +++++++++++++++++-----
>  3 files changed, 125 insertions(+), 62 deletions(-)
> 

applied to iproute2-next. Thanks



^ permalink raw reply

* Re: [PATCH iproute2-next] rdma: Add driver QP type string
From: David Ahern @ 2019-08-07 19:07 UTC (permalink / raw)
  To: Gal Pressman, Stephen Hemminger; +Cc: netdev, linux-rdma, Leon Romanovsky
In-Reply-To: <20190804080756.58364-1-galpress@amazon.com>

On 8/4/19 2:07 AM, Gal Pressman wrote:
> RDMA resource tracker now tracks driver QPs as well, add driver QP type
> string to qp_types_to_str function.
> 
> Signed-off-by: Gal Pressman <galpress@amazon.com>
> ---
>  rdma/res.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 

applied to iproute2-next. Thanks



^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox