| Message ID | 20240416143718.2857981-2-alan.maguire@oracle.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Delegated to: | BPF |
| Headers | show |
| Series | pahole: support nonstandard btf_features | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Not a local patch |
On Tue, Apr 16, 2024 at 7:38 AM Alan Maguire <alan.maguire@oracle.com> wrote: > > Specifying --btf_features=all enables all supported BTF features. > However there are some features that are non-standard, so we should > support a way to use them in --btf_features but not participate in > the set of features enabled by "--btf_features=all". As part of this, > also support all used in a list of --btf_features, i.e. > > --btf_features=all,nonstandard_feature > > Signed-off-by: Alan Maguire <alan.maguire@oracle.com> > --- > man-pages/pahole.1 | 2 +- > pahole.c | 36 +++++++++++++++++++++++------------- > 2 files changed, 24 insertions(+), 14 deletions(-) > > diff --git a/man-pages/pahole.1 b/man-pages/pahole.1 > index 2be165d..2c08e97 100644 > --- a/man-pages/pahole.1 > +++ b/man-pages/pahole.1 > @@ -290,7 +290,7 @@ Allow using all the BTF features supported by pahole. > > .TP > .B \-\-btf_features=FEATURE_LIST > -Encode BTF using the specified feature list, or specify 'all' for all features supported. This option can be used as an alternative to unsing multiple BTF-related options. Supported features are > +Encode BTF using the specified feature list, or specify 'all' for all standard features supported. This option can be used as an alternative to unsing multiple BTF-related options. Supported standard features are > > .nf > encode_force Ignore invalid symbols when encoding BTF; for example > diff --git a/pahole.c b/pahole.c > index 77772bb..890ef81 100644 > --- a/pahole.c > +++ b/pahole.c > @@ -1266,23 +1266,26 @@ ARGP_PROGRAM_VERSION_HOOK_DEF = dwarves_print_version; > * BTF encoding apply; we encode type/decl tags, do not encode > * floats, etc. This ensures backwards compatibility. > */ > -#define BTF_FEATURE(name, alias, default_value) \ > - { #name, #alias, &conf_load.alias, default_value } > +#define BTF_FEATURE(name, alias, default_value, enable_for_all) \ > + { #name, #alias, &conf_load.alias, default_value, enable_for_all } > > struct btf_feature { > const char *name; > const char *option_alias; > bool *conf_value; > bool default_value; > + bool enable_for_all; /* some nonstandard features may not > + * be enabled for --btf_features=all > + */ > } btf_features[] = { > - BTF_FEATURE(encode_force, btf_encode_force, false), > - BTF_FEATURE(var, skip_encoding_btf_vars, true), > - BTF_FEATURE(float, btf_gen_floats, false), > - BTF_FEATURE(decl_tag, skip_encoding_btf_decl_tag, true), > - BTF_FEATURE(type_tag, skip_encoding_btf_type_tag, true), > - BTF_FEATURE(enum64, skip_encoding_btf_enum64, true), > - BTF_FEATURE(optimized_func, btf_gen_optimized, false), > - BTF_FEATURE(consistent_func, skip_encoding_btf_inconsistent_proto, false), > + BTF_FEATURE(encode_force, btf_encode_force, false, true), > + BTF_FEATURE(var, skip_encoding_btf_vars, true, true), > + BTF_FEATURE(float, btf_gen_floats, false, true), > + BTF_FEATURE(decl_tag, skip_encoding_btf_decl_tag, true, true), > + BTF_FEATURE(type_tag, skip_encoding_btf_type_tag, true, true), > + BTF_FEATURE(enum64, skip_encoding_btf_enum64, true, true), > + BTF_FEATURE(optimized_func, btf_gen_optimized, false, true), > + BTF_FEATURE(consistent_func, skip_encoding_btf_inconsistent_proto, false, true), > }; maybe keep those special features in a separate array instead? it is kind of weird to see --btf_features=all,and_then_some, maybe it makes sense to have --btf_extras or something for those? > > #define BTF_MAX_FEATURE_STR 1024 > @@ -1350,8 +1353,10 @@ static void parse_btf_features(const char *features, bool strict) > if (strcmp(features, "all") == 0) { > int i; > > - for (i = 0; i < ARRAY_SIZE(btf_features); i++) > - enable_btf_feature(&btf_features[i]); > + for (i = 0; i < ARRAY_SIZE(btf_features); i++) { > + if (btf_features[i].enable_for_all) > + enable_btf_feature(&btf_features[i]); > + } > return; > } > > @@ -1361,7 +1366,12 @@ static void parse_btf_features(const char *features, bool strict) > struct btf_feature *feature = find_btf_feature(feature_name); > > if (!feature) { > - if (strict) { > + /* --btf_features=all,nonstandard_feature should be > + * allowed. > + */ > + if (strcmp(feature_name, "all") == 0) { > + parse_btf_features(feature_name, strict); > + } else if (strict) { > fprintf(stderr, "Feature '%s' in '%s' is not supported. Supported BTF features are:\n", > feature_name, features); > show_supported_btf_features(stderr); > -- > 2.39.3 > >
On 19/04/2024 01:06, Andrii Nakryiko wrote: > On Tue, Apr 16, 2024 at 7:38 AM Alan Maguire <alan.maguire@oracle.com> wrote: >> >> Specifying --btf_features=all enables all supported BTF features. >> However there are some features that are non-standard, so we should >> support a way to use them in --btf_features but not participate in >> the set of features enabled by "--btf_features=all". As part of this, >> also support all used in a list of --btf_features, i.e. >> >> --btf_features=all,nonstandard_feature >> >> Signed-off-by: Alan Maguire <alan.maguire@oracle.com> >> --- >> man-pages/pahole.1 | 2 +- >> pahole.c | 36 +++++++++++++++++++++++------------- >> 2 files changed, 24 insertions(+), 14 deletions(-) >> >> diff --git a/man-pages/pahole.1 b/man-pages/pahole.1 >> index 2be165d..2c08e97 100644 >> --- a/man-pages/pahole.1 >> +++ b/man-pages/pahole.1 >> @@ -290,7 +290,7 @@ Allow using all the BTF features supported by pahole. >> >> .TP >> .B \-\-btf_features=FEATURE_LIST >> -Encode BTF using the specified feature list, or specify 'all' for all features supported. This option can be used as an alternative to unsing multiple BTF-related options. Supported features are >> +Encode BTF using the specified feature list, or specify 'all' for all standard features supported. This option can be used as an alternative to unsing multiple BTF-related options. Supported standard features are >> >> .nf >> encode_force Ignore invalid symbols when encoding BTF; for example >> diff --git a/pahole.c b/pahole.c >> index 77772bb..890ef81 100644 >> --- a/pahole.c >> +++ b/pahole.c >> @@ -1266,23 +1266,26 @@ ARGP_PROGRAM_VERSION_HOOK_DEF = dwarves_print_version; >> * BTF encoding apply; we encode type/decl tags, do not encode >> * floats, etc. This ensures backwards compatibility. >> */ >> -#define BTF_FEATURE(name, alias, default_value) \ >> - { #name, #alias, &conf_load.alias, default_value } >> +#define BTF_FEATURE(name, alias, default_value, enable_for_all) \ >> + { #name, #alias, &conf_load.alias, default_value, enable_for_all } >> >> struct btf_feature { >> const char *name; >> const char *option_alias; >> bool *conf_value; >> bool default_value; >> + bool enable_for_all; /* some nonstandard features may not >> + * be enabled for --btf_features=all >> + */ >> } btf_features[] = { >> - BTF_FEATURE(encode_force, btf_encode_force, false), >> - BTF_FEATURE(var, skip_encoding_btf_vars, true), >> - BTF_FEATURE(float, btf_gen_floats, false), >> - BTF_FEATURE(decl_tag, skip_encoding_btf_decl_tag, true), >> - BTF_FEATURE(type_tag, skip_encoding_btf_type_tag, true), >> - BTF_FEATURE(enum64, skip_encoding_btf_enum64, true), >> - BTF_FEATURE(optimized_func, btf_gen_optimized, false), >> - BTF_FEATURE(consistent_func, skip_encoding_btf_inconsistent_proto, false), >> + BTF_FEATURE(encode_force, btf_encode_force, false, true), >> + BTF_FEATURE(var, skip_encoding_btf_vars, true, true), >> + BTF_FEATURE(float, btf_gen_floats, false, true), >> + BTF_FEATURE(decl_tag, skip_encoding_btf_decl_tag, true, true), >> + BTF_FEATURE(type_tag, skip_encoding_btf_type_tag, true, true), >> + BTF_FEATURE(enum64, skip_encoding_btf_enum64, true, true), >> + BTF_FEATURE(optimized_func, btf_gen_optimized, false, true), >> + BTF_FEATURE(consistent_func, skip_encoding_btf_inconsistent_proto, false, true), >> }; > > maybe keep those special features in a separate array instead? > yeah, maybe that or have BTF_STANDARD_FEATURE() and BTF_EXTRA_FEATURE() macros to clarify the difference. > it is kind of weird to see --btf_features=all,and_then_some, maybe it > makes sense to have --btf_extras or something for those? > yeah, the "all" is a problem. Arnaldo and I talked a bit about using "default" or "standard" instead of "all"; the problem with default is that there's already a notion of default BTF values - these are the ones we get without specifying any --btf_features or other BTF-related flags. Having an "extras" feature option (drawing from a separate option array) as you suggest might be a cleaner way to make this distinction. What do others think? >> >> #define BTF_MAX_FEATURE_STR 1024 >> @@ -1350,8 +1353,10 @@ static void parse_btf_features(const char *features, bool strict) >> if (strcmp(features, "all") == 0) { >> int i; >> >> - for (i = 0; i < ARRAY_SIZE(btf_features); i++) >> - enable_btf_feature(&btf_features[i]); >> + for (i = 0; i < ARRAY_SIZE(btf_features); i++) { >> + if (btf_features[i].enable_for_all) >> + enable_btf_feature(&btf_features[i]); >> + } >> return; >> } >> >> @@ -1361,7 +1366,12 @@ static void parse_btf_features(const char *features, bool strict) >> struct btf_feature *feature = find_btf_feature(feature_name); >> >> if (!feature) { >> - if (strict) { >> + /* --btf_features=all,nonstandard_feature should be >> + * allowed. >> + */ >> + if (strcmp(feature_name, "all") == 0) { >> + parse_btf_features(feature_name, strict); >> + } else if (strict) { >> fprintf(stderr, "Feature '%s' in '%s' is not supported. Supported BTF features are:\n", >> feature_name, features); >> show_supported_btf_features(stderr); >> -- >> 2.39.3 >> >>
diff --git a/man-pages/pahole.1 b/man-pages/pahole.1 index 2be165d..2c08e97 100644 --- a/man-pages/pahole.1 +++ b/man-pages/pahole.1 @@ -290,7 +290,7 @@ Allow using all the BTF features supported by pahole. .TP .B \-\-btf_features=FEATURE_LIST -Encode BTF using the specified feature list, or specify 'all' for all features supported. This option can be used as an alternative to unsing multiple BTF-related options. Supported features are +Encode BTF using the specified feature list, or specify 'all' for all standard features supported. This option can be used as an alternative to unsing multiple BTF-related options. Supported standard features are .nf encode_force Ignore invalid symbols when encoding BTF; for example diff --git a/pahole.c b/pahole.c index 77772bb..890ef81 100644 --- a/pahole.c +++ b/pahole.c @@ -1266,23 +1266,26 @@ ARGP_PROGRAM_VERSION_HOOK_DEF = dwarves_print_version; * BTF encoding apply; we encode type/decl tags, do not encode * floats, etc. This ensures backwards compatibility. */ -#define BTF_FEATURE(name, alias, default_value) \ - { #name, #alias, &conf_load.alias, default_value } +#define BTF_FEATURE(name, alias, default_value, enable_for_all) \ + { #name, #alias, &conf_load.alias, default_value, enable_for_all } struct btf_feature { const char *name; const char *option_alias; bool *conf_value; bool default_value; + bool enable_for_all; /* some nonstandard features may not + * be enabled for --btf_features=all + */ } btf_features[] = { - BTF_FEATURE(encode_force, btf_encode_force, false), - BTF_FEATURE(var, skip_encoding_btf_vars, true), - BTF_FEATURE(float, btf_gen_floats, false), - BTF_FEATURE(decl_tag, skip_encoding_btf_decl_tag, true), - BTF_FEATURE(type_tag, skip_encoding_btf_type_tag, true), - BTF_FEATURE(enum64, skip_encoding_btf_enum64, true), - BTF_FEATURE(optimized_func, btf_gen_optimized, false), - BTF_FEATURE(consistent_func, skip_encoding_btf_inconsistent_proto, false), + BTF_FEATURE(encode_force, btf_encode_force, false, true), + BTF_FEATURE(var, skip_encoding_btf_vars, true, true), + BTF_FEATURE(float, btf_gen_floats, false, true), + BTF_FEATURE(decl_tag, skip_encoding_btf_decl_tag, true, true), + BTF_FEATURE(type_tag, skip_encoding_btf_type_tag, true, true), + BTF_FEATURE(enum64, skip_encoding_btf_enum64, true, true), + BTF_FEATURE(optimized_func, btf_gen_optimized, false, true), + BTF_FEATURE(consistent_func, skip_encoding_btf_inconsistent_proto, false, true), }; #define BTF_MAX_FEATURE_STR 1024 @@ -1350,8 +1353,10 @@ static void parse_btf_features(const char *features, bool strict) if (strcmp(features, "all") == 0) { int i; - for (i = 0; i < ARRAY_SIZE(btf_features); i++) - enable_btf_feature(&btf_features[i]); + for (i = 0; i < ARRAY_SIZE(btf_features); i++) { + if (btf_features[i].enable_for_all) + enable_btf_feature(&btf_features[i]); + } return; } @@ -1361,7 +1366,12 @@ static void parse_btf_features(const char *features, bool strict) struct btf_feature *feature = find_btf_feature(feature_name); if (!feature) { - if (strict) { + /* --btf_features=all,nonstandard_feature should be + * allowed. + */ + if (strcmp(feature_name, "all") == 0) { + parse_btf_features(feature_name, strict); + } else if (strict) { fprintf(stderr, "Feature '%s' in '%s' is not supported. Supported BTF features are:\n", feature_name, features); show_supported_btf_features(stderr);
Specifying --btf_features=all enables all supported BTF features. However there are some features that are non-standard, so we should support a way to use them in --btf_features but not participate in the set of features enabled by "--btf_features=all". As part of this, also support all used in a list of --btf_features, i.e. --btf_features=all,nonstandard_feature Signed-off-by: Alan Maguire <alan.maguire@oracle.com> --- man-pages/pahole.1 | 2 +- pahole.c | 36 +++++++++++++++++++++++------------- 2 files changed, 24 insertions(+), 14 deletions(-)