| Message ID | 20230510-dcb-rewr-v1-1-83adc1f93356@microchip.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | David Ahern |
| Headers | show |
| Series | Introduce new dcb-rewr subcommand | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Not a local patch |
Daniel Machon <daniel.machon@microchip.com> writes: > Add new headerfile dcb-app.h that exposes the functions required later > by dcb-rewr. The new dcb-rewr implementation will reuse much of the > existing dcb-app code. > > I thought this called for a separate header file, instead of polluting > the existing dcb.h file. > > Signed-off-by: Daniel Machon <daniel.machon@microchip.com> > --- > dcb/dcb.h | 9 ++------- > dcb/dcb_app.c | 54 ++++++++++++++++++------------------------------------ > dcb/dcb_app.h | 55 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ > 3 files changed, 75 insertions(+), 43 deletions(-) > > diff --git a/dcb/dcb.h b/dcb/dcb.h > index d40664f29dad..4c8a4aa25e0c 100644 > --- a/dcb/dcb.h > +++ b/dcb/dcb.h > @@ -6,6 +6,8 @@ > #include <stdbool.h> > #include <stddef.h> > > +#include "dcb_app.h" > + > /* dcb.c */ > > struct dcb { > @@ -54,13 +56,6 @@ void dcb_print_array_on_off(const __u8 *array, size_t size); > void dcb_print_array_kw(const __u8 *array, size_t array_size, > const char *const kw[], size_t kw_size); > > -/* dcb_app.c */ > - > -int dcb_cmd_app(struct dcb *dcb, int argc, char **argv); > -enum ieee_attrs_app dcb_app_attr_type_get(__u8 selector); > -bool dcb_app_attr_type_validate(enum ieee_attrs_app type); > -bool dcb_app_selector_validate(enum ieee_attrs_app type, __u8 selector); > - Why the move to a dedicated header? dcb.h ends up being the only client and everybody consumes the prototypes through that file anyway. I don't fine it necessary. > /* dcb_apptrust.c */ > > int dcb_cmd_apptrust(struct dcb *dcb, int argc, char **argv); > diff --git a/dcb/dcb_app.c b/dcb/dcb_app.c > index eeb78e70f63f..df339babd8e6 100644 > --- a/dcb/dcb_app.c > +++ b/dcb/dcb_app.c > @@ -10,8 +10,6 @@ > #include "utils.h" > #include "rt_names.h" > > -#define DCB_APP_PCP_MAX 15 > - > static const char *const pcp_names[DCB_APP_PCP_MAX + 1] = { > "0nd", "1nd", "2nd", "3nd", "4nd", "5nd", "6nd", "7nd", > "0de", "1de", "2de", "3de", "4de", "5de", "6de", "7de" > @@ -22,6 +20,7 @@ static const char *const ieee_attrs_app_names[__DCB_ATTR_IEEE_APP_MAX] = { > [DCB_ATTR_DCB_APP] = "DCB_ATTR_DCB_APP" > }; > > + This looks like a leftover. > static void dcb_app_help_add(void) > { > fprintf(stderr, > @@ -68,11 +67,6 @@ static void dcb_app_help(void) > dcb_app_help_add(); > } > > -struct dcb_app_table { > - struct dcb_app *apps; > - size_t n_apps; > -}; > - > enum ieee_attrs_app dcb_app_attr_type_get(__u8 selector) > { > switch (selector) { > @@ -105,7 +99,7 @@ bool dcb_app_selector_validate(enum ieee_attrs_app type, __u8 selector) > return dcb_app_attr_type_get(selector) == type; > } > > -static void dcb_app_table_fini(struct dcb_app_table *tab) > +void dcb_app_table_fini(struct dcb_app_table *tab) > { > free(tab->apps); > } > @@ -124,8 +118,8 @@ static int dcb_app_table_push(struct dcb_app_table *tab, struct dcb_app *app) > return 0; > } > > -static void dcb_app_table_remove_existing(struct dcb_app_table *a, > - const struct dcb_app_table *b) > +void dcb_app_table_remove_existing(struct dcb_app_table *a, > + const struct dcb_app_table *b) > { > size_t ia, ja; > size_t ib; > @@ -152,8 +146,8 @@ static void dcb_app_table_remove_existing(struct dcb_app_table *a, > a->n_apps = ja; > } > > -static void dcb_app_table_remove_replaced(struct dcb_app_table *a, > - const struct dcb_app_table *b) > +void dcb_app_table_remove_replaced(struct dcb_app_table *a, > + const struct dcb_app_table *b) > { > size_t ia, ja; > size_t ib; > @@ -189,8 +183,7 @@ static void dcb_app_table_remove_replaced(struct dcb_app_table *a, > a->n_apps = ja; > } > > -static int dcb_app_table_copy(struct dcb_app_table *a, > - const struct dcb_app_table *b) > +int dcb_app_table_copy(struct dcb_app_table *a, const struct dcb_app_table *b) > { > size_t i; > int ret; > @@ -217,18 +210,12 @@ static int dcb_app_cmp_cb(const void *a, const void *b) > return dcb_app_cmp(a, b); > } > > -static void dcb_app_table_sort(struct dcb_app_table *tab) > +void dcb_app_table_sort(struct dcb_app_table *tab) > { > qsort(tab->apps, tab->n_apps, sizeof(*tab->apps), dcb_app_cmp_cb); > } > > -struct dcb_app_parse_mapping { > - __u8 selector; > - struct dcb_app_table *tab; > - int err; > -}; > - > -static void dcb_app_parse_mapping_cb(__u32 key, __u64 value, void *data) > +void dcb_app_parse_mapping_cb(__u32 key, __u64 value, void *data) > { > struct dcb_app_parse_mapping *pm = data; > struct dcb_app app = { > @@ -260,7 +247,7 @@ static int dcb_app_parse_mapping_ethtype_prio(__u32 key, char *value, void *data > dcb_app_parse_mapping_cb, data); > } > > -static int dcb_app_parse_pcp(__u32 *key, const char *arg) > +int dcb_app_parse_pcp(__u32 *key, const char *arg) > { > int i; > > @@ -286,7 +273,7 @@ static int dcb_app_parse_mapping_pcp_prio(__u32 key, char *value, void *data) > dcb_app_parse_mapping_cb, data); > } > > -static int dcb_app_parse_dscp(__u32 *key, const char *arg) > +int dcb_app_parse_dscp(__u32 *key, const char *arg) > { > if (parse_mapping_num_all(key, arg) == 0) > return 0; > @@ -377,12 +364,12 @@ static bool dcb_app_is_default(const struct dcb_app *app) > app->protocol == 0; > } > > -static bool dcb_app_is_dscp(const struct dcb_app *app) > +bool dcb_app_is_dscp(const struct dcb_app *app) > { > return app->selector == IEEE_8021QAZ_APP_SEL_DSCP; > } > > -static bool dcb_app_is_pcp(const struct dcb_app *app) > +bool dcb_app_is_pcp(const struct dcb_app *app) > { > return app->selector == DCB_APP_SEL_PCP; > } > @@ -402,7 +389,7 @@ static bool dcb_app_is_port(const struct dcb_app *app) > return app->selector == IEEE_8021QAZ_APP_SEL_ANY; > } > > -static int dcb_app_print_key_dec(__u16 protocol) > +int dcb_app_print_key_dec(__u16 protocol) > { > return print_uint(PRINT_ANY, NULL, "%d:", protocol); > } > @@ -412,7 +399,7 @@ static int dcb_app_print_key_hex(__u16 protocol) > return print_uint(PRINT_ANY, NULL, "%x:", protocol); > } > > -static int dcb_app_print_key_dscp(__u16 protocol) > +int dcb_app_print_key_dscp(__u16 protocol) > { > const char *name = rtnl_dsfield_get_name(protocol << 2); > > @@ -422,7 +409,7 @@ static int dcb_app_print_key_dscp(__u16 protocol) > return print_uint(PRINT_ANY, NULL, "%d:", protocol); > } > > -static int dcb_app_print_key_pcp(__u16 protocol) > +int dcb_app_print_key_pcp(__u16 protocol) > { > /* Print in numerical form, if protocol value is out-of-range */ > if (protocol > DCB_APP_PCP_MAX) > @@ -577,7 +564,7 @@ static int dcb_app_get_table_attr_cb(const struct nlattr *attr, void *data) > return MNL_CB_OK; > } > > -static int dcb_app_get(struct dcb *dcb, const char *dev, struct dcb_app_table *tab) > +int dcb_app_get(struct dcb *dcb, const char *dev, struct dcb_app_table *tab) > { > uint16_t payload_len; > void *payload; > @@ -594,11 +581,6 @@ static int dcb_app_get(struct dcb *dcb, const char *dev, struct dcb_app_table *t > return 0; > } > > -struct dcb_app_add_del { > - const struct dcb_app_table *tab; > - bool (*filter)(const struct dcb_app *app); > -}; > - This structure is a protocol between dcb_app_add_del() and dcb_app_add_del_cb(). I don't think your patchset uses it elsewhere, so it can be kept private. > static int dcb_app_add_del_cb(struct dcb *dcb, struct nlmsghdr *nlh, void *data) > { > struct dcb_app_add_del *add_del = data; > @@ -620,7 +602,7 @@ static int dcb_app_add_del_cb(struct dcb *dcb, struct nlmsghdr *nlh, void *data) > return 0; > } > > -static int dcb_app_add_del(struct dcb *dcb, const char *dev, int command, > +int dcb_app_add_del(struct dcb *dcb, const char *dev, int command, > const struct dcb_app_table *tab, > bool (*filter)(const struct dcb_app *)) This has wrong indentation. > { > diff --git a/dcb/dcb_app.h b/dcb/dcb_app.h > new file mode 100644 > index 000000000000..8e7b010dcf75 > --- /dev/null > +++ b/dcb/dcb_app.h > @@ -0,0 +1,55 @@ > +/* SPDX-License-Identifier: GPL-2.0 */ > +#ifndef __DCB_APP_H_ > +#define __DCB_APP_H_ > + > +struct dcb; > + > +struct dcb_app_table { > + struct dcb_app *apps; > + size_t n_apps; > +}; > + > +struct dcb_app_add_del { > + const struct dcb_app_table *tab; > + bool (*filter)(const struct dcb_app *app); > +}; > + > +struct dcb_app_parse_mapping { > + __u8 selector; > + struct dcb_app_table *tab; > + int err; > +}; > + > +#define DCB_APP_PCP_MAX 15 > + > +int dcb_cmd_app(struct dcb *dcb, int argc, char **argv); > + > +int dcb_app_get(struct dcb *dcb, const char *dev, struct dcb_app_table *tab); > +int dcb_app_add_del(struct dcb *dcb, const char *dev, int command, > + const struct dcb_app_table *tab, > + bool (*filter)(const struct dcb_app *)); > + > +void dcb_app_table_sort(struct dcb_app_table *tab); > +void dcb_app_table_fini(struct dcb_app_table *tab); > +int dcb_app_table_copy(struct dcb_app_table *a, const struct dcb_app_table *b); > +void dcb_app_table_remove_replaced(struct dcb_app_table *a, > + const struct dcb_app_table *b); > +void dcb_app_table_remove_existing(struct dcb_app_table *a, > + const struct dcb_app_table *b); > + > +bool dcb_app_is_pcp(const struct dcb_app *app); > +bool dcb_app_is_dscp(const struct dcb_app *app); > + > +int dcb_app_print_key_dec(__u16 protocol); > +int dcb_app_print_key_dscp(__u16 protocol); > +int dcb_app_print_key_pcp(__u16 protocol); > + > +int dcb_app_parse_pcp(__u32 *key, const char *arg); > +int dcb_app_parse_dscp(__u32 *key, const char *arg); > +void dcb_app_parse_mapping_cb(__u32 key, __u64 value, void *data); > + > +bool dcb_app_selector_validate(enum ieee_attrs_app type, __u8 selector); > +bool dcb_app_attr_type_validate(enum ieee_attrs_app type); > +enum ieee_attrs_app dcb_app_attr_type_get(__u8 selector); > + > +#endif
> > Add new headerfile dcb-app.h that exposes the functions required later > > by dcb-rewr. The new dcb-rewr implementation will reuse much of the > > existing dcb-app code. > > > > I thought this called for a separate header file, instead of polluting > > the existing dcb.h file. > > > > Signed-off-by: Daniel Machon <daniel.machon@microchip.com> > > --- > > dcb/dcb.h | 9 ++------- > > dcb/dcb_app.c | 54 ++++++++++++++++++------------------------------------ > > dcb/dcb_app.h | 55 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ > > 3 files changed, 75 insertions(+), 43 deletions(-) > > > > diff --git a/dcb/dcb.h b/dcb/dcb.h > > index d40664f29dad..4c8a4aa25e0c 100644 > > --- a/dcb/dcb.h > > +++ b/dcb/dcb.h > > @@ -6,6 +6,8 @@ > > #include <stdbool.h> > > #include <stddef.h> > > > > +#include "dcb_app.h" > > + > > /* dcb.c */ > > > > struct dcb { > > @@ -54,13 +56,6 @@ void dcb_print_array_on_off(const __u8 *array, size_t size); > > void dcb_print_array_kw(const __u8 *array, size_t array_size, > > const char *const kw[], size_t kw_size); > > > > -/* dcb_app.c */ > > - > > -int dcb_cmd_app(struct dcb *dcb, int argc, char **argv); > > -enum ieee_attrs_app dcb_app_attr_type_get(__u8 selector); > > -bool dcb_app_attr_type_validate(enum ieee_attrs_app type); > > -bool dcb_app_selector_validate(enum ieee_attrs_app type, __u8 selector); > > - > > Why the move to a dedicated header? dcb.h ends up being the only client > and everybody consumes the prototypes through that file anyway. I don't > fine it necessary. I did try to rationalize that a bit in the commit description. I thought the amount of exposed app functions ended up polutting the dcb header. Maybe it is not that bad - can move them back in the next v. > > > /* dcb_apptrust.c */ > > > > int dcb_cmd_apptrust(struct dcb *dcb, int argc, char **argv); > > diff --git a/dcb/dcb_app.c b/dcb/dcb_app.c > > index eeb78e70f63f..df339babd8e6 100644 > > --- a/dcb/dcb_app.c > > +++ b/dcb/dcb_app.c > > @@ -10,8 +10,6 @@ > > #include "utils.h" > > #include "rt_names.h" > > > > -#define DCB_APP_PCP_MAX 15 > > - > > static const char *const pcp_names[DCB_APP_PCP_MAX + 1] = { > > "0nd", "1nd", "2nd", "3nd", "4nd", "5nd", "6nd", "7nd", > > "0de", "1de", "2de", "3de", "4de", "5de", "6de", "7de" > > @@ -22,6 +20,7 @@ static const char *const ieee_attrs_app_names[__DCB_ATTR_IEEE_APP_MAX] = { > > [DCB_ATTR_DCB_APP] = "DCB_ATTR_DCB_APP" > > }; > > > > + > > This looks like a leftover. > > > static void dcb_app_help_add(void) > > { > > fprintf(stderr, > > @@ -68,11 +67,6 @@ static void dcb_app_help(void) > > dcb_app_help_add(); > > } > > > > -struct dcb_app_table { > > - struct dcb_app *apps; > > - size_t n_apps; > > -}; > > - > > enum ieee_attrs_app dcb_app_attr_type_get(__u8 selector) > > { > > switch (selector) { > > @@ -105,7 +99,7 @@ bool dcb_app_selector_validate(enum ieee_attrs_app type, __u8 selector) > > return dcb_app_attr_type_get(selector) == type; > > } > > > > -static void dcb_app_table_fini(struct dcb_app_table *tab) > > +void dcb_app_table_fini(struct dcb_app_table *tab) > > { > > free(tab->apps); > > } > > @@ -124,8 +118,8 @@ static int dcb_app_table_push(struct dcb_app_table *tab, struct dcb_app *app) > > return 0; > > } > > > > -static void dcb_app_table_remove_existing(struct dcb_app_table *a, > > - const struct dcb_app_table *b) > > +void dcb_app_table_remove_existing(struct dcb_app_table *a, > > + const struct dcb_app_table *b) > > { > > size_t ia, ja; > > size_t ib; > > @@ -152,8 +146,8 @@ static void dcb_app_table_remove_existing(struct dcb_app_table *a, > > a->n_apps = ja; > > } > > > > -static void dcb_app_table_remove_replaced(struct dcb_app_table *a, > > - const struct dcb_app_table *b) > > +void dcb_app_table_remove_replaced(struct dcb_app_table *a, > > + const struct dcb_app_table *b) > > { > > size_t ia, ja; > > size_t ib; > > @@ -189,8 +183,7 @@ static void dcb_app_table_remove_replaced(struct dcb_app_table *a, > > a->n_apps = ja; > > } > > > > -static int dcb_app_table_copy(struct dcb_app_table *a, > > - const struct dcb_app_table *b) > > +int dcb_app_table_copy(struct dcb_app_table *a, const struct dcb_app_table *b) > > { > > size_t i; > > int ret; > > @@ -217,18 +210,12 @@ static int dcb_app_cmp_cb(const void *a, const void *b) > > return dcb_app_cmp(a, b); > > } > > > > -static void dcb_app_table_sort(struct dcb_app_table *tab) > > +void dcb_app_table_sort(struct dcb_app_table *tab) > > { > > qsort(tab->apps, tab->n_apps, sizeof(*tab->apps), dcb_app_cmp_cb); > > } > > > > -struct dcb_app_parse_mapping { > > - __u8 selector; > > - struct dcb_app_table *tab; > > - int err; > > -}; > > - > > -static void dcb_app_parse_mapping_cb(__u32 key, __u64 value, void *data) > > +void dcb_app_parse_mapping_cb(__u32 key, __u64 value, void *data) > > { > > struct dcb_app_parse_mapping *pm = data; > > struct dcb_app app = { > > @@ -260,7 +247,7 @@ static int dcb_app_parse_mapping_ethtype_prio(__u32 key, char *value, void *data > > dcb_app_parse_mapping_cb, data); > > } > > > > -static int dcb_app_parse_pcp(__u32 *key, const char *arg) > > +int dcb_app_parse_pcp(__u32 *key, const char *arg) > > { > > int i; > > > > @@ -286,7 +273,7 @@ static int dcb_app_parse_mapping_pcp_prio(__u32 key, char *value, void *data) > > dcb_app_parse_mapping_cb, data); > > } > > > > -static int dcb_app_parse_dscp(__u32 *key, const char *arg) > > +int dcb_app_parse_dscp(__u32 *key, const char *arg) > > { > > if (parse_mapping_num_all(key, arg) == 0) > > return 0; > > @@ -377,12 +364,12 @@ static bool dcb_app_is_default(const struct dcb_app *app) > > app->protocol == 0; > > } > > > > -static bool dcb_app_is_dscp(const struct dcb_app *app) > > +bool dcb_app_is_dscp(const struct dcb_app *app) > > { > > return app->selector == IEEE_8021QAZ_APP_SEL_DSCP; > > } > > > > -static bool dcb_app_is_pcp(const struct dcb_app *app) > > +bool dcb_app_is_pcp(const struct dcb_app *app) > > { > > return app->selector == DCB_APP_SEL_PCP; > > } > > @@ -402,7 +389,7 @@ static bool dcb_app_is_port(const struct dcb_app *app) > > return app->selector == IEEE_8021QAZ_APP_SEL_ANY; > > } > > > > -static int dcb_app_print_key_dec(__u16 protocol) > > +int dcb_app_print_key_dec(__u16 protocol) > > { > > return print_uint(PRINT_ANY, NULL, "%d:", protocol); > > } > > @@ -412,7 +399,7 @@ static int dcb_app_print_key_hex(__u16 protocol) > > return print_uint(PRINT_ANY, NULL, "%x:", protocol); > > } > > > > -static int dcb_app_print_key_dscp(__u16 protocol) > > +int dcb_app_print_key_dscp(__u16 protocol) > > { > > const char *name = rtnl_dsfield_get_name(protocol << 2); > > > > @@ -422,7 +409,7 @@ static int dcb_app_print_key_dscp(__u16 protocol) > > return print_uint(PRINT_ANY, NULL, "%d:", protocol); > > } > > > > -static int dcb_app_print_key_pcp(__u16 protocol) > > +int dcb_app_print_key_pcp(__u16 protocol) > > { > > /* Print in numerical form, if protocol value is out-of-range */ > > if (protocol > DCB_APP_PCP_MAX) > > @@ -577,7 +564,7 @@ static int dcb_app_get_table_attr_cb(const struct nlattr *attr, void *data) > > return MNL_CB_OK; > > } > > > > -static int dcb_app_get(struct dcb *dcb, const char *dev, struct dcb_app_table *tab) > > +int dcb_app_get(struct dcb *dcb, const char *dev, struct dcb_app_table *tab) > > { > > uint16_t payload_len; > > void *payload; > > @@ -594,11 +581,6 @@ static int dcb_app_get(struct dcb *dcb, const char *dev, struct dcb_app_table *t > > return 0; > > } > > > > -struct dcb_app_add_del { > > - const struct dcb_app_table *tab; > > - bool (*filter)(const struct dcb_app *app); > > -}; > > - > > This structure is a protocol between dcb_app_add_del() and > dcb_app_add_del_cb(). I don't think your patchset uses it elsewhere, so > it can be kept private. Yep. You are right. > > > static int dcb_app_add_del_cb(struct dcb *dcb, struct nlmsghdr *nlh, void *data) > > { > > struct dcb_app_add_del *add_del = data; > > @@ -620,7 +602,7 @@ static int dcb_app_add_del_cb(struct dcb *dcb, struct nlmsghdr *nlh, void *data) > > return 0; > > } > > > > -static int dcb_app_add_del(struct dcb *dcb, const char *dev, int command, > > +int dcb_app_add_del(struct dcb *dcb, const char *dev, int command, > > const struct dcb_app_table *tab, > > bool (*filter)(const struct dcb_app *)) > > This has wrong indentation. Ouch. Will fix in next v. > > > { > > diff --git a/dcb/dcb_app.h b/dcb/dcb_app.h > > new file mode 100644 > > index 000000000000..8e7b010dcf75 > > --- /dev/null > > +++ b/dcb/dcb_app.h > > @@ -0,0 +1,55 @@ > > +/* SPDX-License-Identifier: GPL-2.0 */ > > +#ifndef __DCB_APP_H_ > > +#define __DCB_APP_H_ > > + > > +struct dcb; > > + > > +struct dcb_app_table { > > + struct dcb_app *apps; > > + size_t n_apps; > > +}; > > + > > +struct dcb_app_add_del { > > + const struct dcb_app_table *tab; > > + bool (*filter)(const struct dcb_app *app); > > +}; > > + > > +struct dcb_app_parse_mapping { > > + __u8 selector; > > + struct dcb_app_table *tab; > > + int err; > > +}; > > + > > +#define DCB_APP_PCP_MAX 15 > > + > > +int dcb_cmd_app(struct dcb *dcb, int argc, char **argv); > > + > > +int dcb_app_get(struct dcb *dcb, const char *dev, struct dcb_app_table *tab); > > +int dcb_app_add_del(struct dcb *dcb, const char *dev, int command, > > + const struct dcb_app_table *tab, > > + bool (*filter)(const struct dcb_app *)); > > + > > +void dcb_app_table_sort(struct dcb_app_table *tab); > > +void dcb_app_table_fini(struct dcb_app_table *tab); > > +int dcb_app_table_copy(struct dcb_app_table *a, const struct dcb_app_table *b); > > +void dcb_app_table_remove_replaced(struct dcb_app_table *a, > > + const struct dcb_app_table *b); > > +void dcb_app_table_remove_existing(struct dcb_app_table *a, > > + const struct dcb_app_table *b); > > + > > +bool dcb_app_is_pcp(const struct dcb_app *app); > > +bool dcb_app_is_dscp(const struct dcb_app *app); > > + > > +int dcb_app_print_key_dec(__u16 protocol); > > +int dcb_app_print_key_dscp(__u16 protocol); > > +int dcb_app_print_key_pcp(__u16 protocol); > > + > > +int dcb_app_parse_pcp(__u32 *key, const char *arg); > > +int dcb_app_parse_dscp(__u32 *key, const char *arg); > > +void dcb_app_parse_mapping_cb(__u32 key, __u64 value, void *data); > > + > > +bool dcb_app_selector_validate(enum ieee_attrs_app type, __u8 selector); > > +bool dcb_app_attr_type_validate(enum ieee_attrs_app type); > > +enum ieee_attrs_app dcb_app_attr_type_get(__u8 selector); > > + > > +#endif >
<Daniel.Machon@microchip.com> writes: >> > -/* dcb_app.c */ >> > - >> > -int dcb_cmd_app(struct dcb *dcb, int argc, char **argv); >> > -enum ieee_attrs_app dcb_app_attr_type_get(__u8 selector); >> > -bool dcb_app_attr_type_validate(enum ieee_attrs_app type); >> > -bool dcb_app_selector_validate(enum ieee_attrs_app type, __u8 selector); >> > - >> >> Why the move to a dedicated header? dcb.h ends up being the only client >> and everybody consumes the prototypes through that file anyway. I don't >> fine it necessary. > > I did try to rationalize that a bit in the commit description. I thought > the amount of exposed app functions ended up polutting the dcb header. I think it's not too bad. The dcb.c section of the header is similarly large as the app section will be. Even with all the stuff that you publish, the header is still, what, 150 lines maybe? I find that the fragmentation isn't necessary and the current setup is just super simple. > Maybe it is not that bad - can move them back in the next v.
diff --git a/dcb/dcb.h b/dcb/dcb.h index d40664f29dad..4c8a4aa25e0c 100644 --- a/dcb/dcb.h +++ b/dcb/dcb.h @@ -6,6 +6,8 @@ #include <stdbool.h> #include <stddef.h> +#include "dcb_app.h" + /* dcb.c */ struct dcb { @@ -54,13 +56,6 @@ void dcb_print_array_on_off(const __u8 *array, size_t size); void dcb_print_array_kw(const __u8 *array, size_t array_size, const char *const kw[], size_t kw_size); -/* dcb_app.c */ - -int dcb_cmd_app(struct dcb *dcb, int argc, char **argv); -enum ieee_attrs_app dcb_app_attr_type_get(__u8 selector); -bool dcb_app_attr_type_validate(enum ieee_attrs_app type); -bool dcb_app_selector_validate(enum ieee_attrs_app type, __u8 selector); - /* dcb_apptrust.c */ int dcb_cmd_apptrust(struct dcb *dcb, int argc, char **argv); diff --git a/dcb/dcb_app.c b/dcb/dcb_app.c index eeb78e70f63f..df339babd8e6 100644 --- a/dcb/dcb_app.c +++ b/dcb/dcb_app.c @@ -10,8 +10,6 @@ #include "utils.h" #include "rt_names.h" -#define DCB_APP_PCP_MAX 15 - static const char *const pcp_names[DCB_APP_PCP_MAX + 1] = { "0nd", "1nd", "2nd", "3nd", "4nd", "5nd", "6nd", "7nd", "0de", "1de", "2de", "3de", "4de", "5de", "6de", "7de" @@ -22,6 +20,7 @@ static const char *const ieee_attrs_app_names[__DCB_ATTR_IEEE_APP_MAX] = { [DCB_ATTR_DCB_APP] = "DCB_ATTR_DCB_APP" }; + static void dcb_app_help_add(void) { fprintf(stderr, @@ -68,11 +67,6 @@ static void dcb_app_help(void) dcb_app_help_add(); } -struct dcb_app_table { - struct dcb_app *apps; - size_t n_apps; -}; - enum ieee_attrs_app dcb_app_attr_type_get(__u8 selector) { switch (selector) { @@ -105,7 +99,7 @@ bool dcb_app_selector_validate(enum ieee_attrs_app type, __u8 selector) return dcb_app_attr_type_get(selector) == type; } -static void dcb_app_table_fini(struct dcb_app_table *tab) +void dcb_app_table_fini(struct dcb_app_table *tab) { free(tab->apps); } @@ -124,8 +118,8 @@ static int dcb_app_table_push(struct dcb_app_table *tab, struct dcb_app *app) return 0; } -static void dcb_app_table_remove_existing(struct dcb_app_table *a, - const struct dcb_app_table *b) +void dcb_app_table_remove_existing(struct dcb_app_table *a, + const struct dcb_app_table *b) { size_t ia, ja; size_t ib; @@ -152,8 +146,8 @@ static void dcb_app_table_remove_existing(struct dcb_app_table *a, a->n_apps = ja; } -static void dcb_app_table_remove_replaced(struct dcb_app_table *a, - const struct dcb_app_table *b) +void dcb_app_table_remove_replaced(struct dcb_app_table *a, + const struct dcb_app_table *b) { size_t ia, ja; size_t ib; @@ -189,8 +183,7 @@ static void dcb_app_table_remove_replaced(struct dcb_app_table *a, a->n_apps = ja; } -static int dcb_app_table_copy(struct dcb_app_table *a, - const struct dcb_app_table *b) +int dcb_app_table_copy(struct dcb_app_table *a, const struct dcb_app_table *b) { size_t i; int ret; @@ -217,18 +210,12 @@ static int dcb_app_cmp_cb(const void *a, const void *b) return dcb_app_cmp(a, b); } -static void dcb_app_table_sort(struct dcb_app_table *tab) +void dcb_app_table_sort(struct dcb_app_table *tab) { qsort(tab->apps, tab->n_apps, sizeof(*tab->apps), dcb_app_cmp_cb); } -struct dcb_app_parse_mapping { - __u8 selector; - struct dcb_app_table *tab; - int err; -}; - -static void dcb_app_parse_mapping_cb(__u32 key, __u64 value, void *data) +void dcb_app_parse_mapping_cb(__u32 key, __u64 value, void *data) { struct dcb_app_parse_mapping *pm = data; struct dcb_app app = { @@ -260,7 +247,7 @@ static int dcb_app_parse_mapping_ethtype_prio(__u32 key, char *value, void *data dcb_app_parse_mapping_cb, data); } -static int dcb_app_parse_pcp(__u32 *key, const char *arg) +int dcb_app_parse_pcp(__u32 *key, const char *arg) { int i; @@ -286,7 +273,7 @@ static int dcb_app_parse_mapping_pcp_prio(__u32 key, char *value, void *data) dcb_app_parse_mapping_cb, data); } -static int dcb_app_parse_dscp(__u32 *key, const char *arg) +int dcb_app_parse_dscp(__u32 *key, const char *arg) { if (parse_mapping_num_all(key, arg) == 0) return 0; @@ -377,12 +364,12 @@ static bool dcb_app_is_default(const struct dcb_app *app) app->protocol == 0; } -static bool dcb_app_is_dscp(const struct dcb_app *app) +bool dcb_app_is_dscp(const struct dcb_app *app) { return app->selector == IEEE_8021QAZ_APP_SEL_DSCP; } -static bool dcb_app_is_pcp(const struct dcb_app *app) +bool dcb_app_is_pcp(const struct dcb_app *app) { return app->selector == DCB_APP_SEL_PCP; } @@ -402,7 +389,7 @@ static bool dcb_app_is_port(const struct dcb_app *app) return app->selector == IEEE_8021QAZ_APP_SEL_ANY; } -static int dcb_app_print_key_dec(__u16 protocol) +int dcb_app_print_key_dec(__u16 protocol) { return print_uint(PRINT_ANY, NULL, "%d:", protocol); } @@ -412,7 +399,7 @@ static int dcb_app_print_key_hex(__u16 protocol) return print_uint(PRINT_ANY, NULL, "%x:", protocol); } -static int dcb_app_print_key_dscp(__u16 protocol) +int dcb_app_print_key_dscp(__u16 protocol) { const char *name = rtnl_dsfield_get_name(protocol << 2); @@ -422,7 +409,7 @@ static int dcb_app_print_key_dscp(__u16 protocol) return print_uint(PRINT_ANY, NULL, "%d:", protocol); } -static int dcb_app_print_key_pcp(__u16 protocol) +int dcb_app_print_key_pcp(__u16 protocol) { /* Print in numerical form, if protocol value is out-of-range */ if (protocol > DCB_APP_PCP_MAX) @@ -577,7 +564,7 @@ static int dcb_app_get_table_attr_cb(const struct nlattr *attr, void *data) return MNL_CB_OK; } -static int dcb_app_get(struct dcb *dcb, const char *dev, struct dcb_app_table *tab) +int dcb_app_get(struct dcb *dcb, const char *dev, struct dcb_app_table *tab) { uint16_t payload_len; void *payload; @@ -594,11 +581,6 @@ static int dcb_app_get(struct dcb *dcb, const char *dev, struct dcb_app_table *t return 0; } -struct dcb_app_add_del { - const struct dcb_app_table *tab; - bool (*filter)(const struct dcb_app *app); -}; - static int dcb_app_add_del_cb(struct dcb *dcb, struct nlmsghdr *nlh, void *data) { struct dcb_app_add_del *add_del = data; @@ -620,7 +602,7 @@ static int dcb_app_add_del_cb(struct dcb *dcb, struct nlmsghdr *nlh, void *data) return 0; } -static int dcb_app_add_del(struct dcb *dcb, const char *dev, int command, +int dcb_app_add_del(struct dcb *dcb, const char *dev, int command, const struct dcb_app_table *tab, bool (*filter)(const struct dcb_app *)) { diff --git a/dcb/dcb_app.h b/dcb/dcb_app.h new file mode 100644 index 000000000000..8e7b010dcf75 --- /dev/null +++ b/dcb/dcb_app.h @@ -0,0 +1,55 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +#ifndef __DCB_APP_H_ +#define __DCB_APP_H_ + +struct dcb; + +struct dcb_app_table { + struct dcb_app *apps; + size_t n_apps; +}; + +struct dcb_app_add_del { + const struct dcb_app_table *tab; + bool (*filter)(const struct dcb_app *app); +}; + +struct dcb_app_parse_mapping { + __u8 selector; + struct dcb_app_table *tab; + int err; +}; + +#define DCB_APP_PCP_MAX 15 + +int dcb_cmd_app(struct dcb *dcb, int argc, char **argv); + +int dcb_app_get(struct dcb *dcb, const char *dev, struct dcb_app_table *tab); +int dcb_app_add_del(struct dcb *dcb, const char *dev, int command, + const struct dcb_app_table *tab, + bool (*filter)(const struct dcb_app *)); + +void dcb_app_table_sort(struct dcb_app_table *tab); +void dcb_app_table_fini(struct dcb_app_table *tab); +int dcb_app_table_copy(struct dcb_app_table *a, const struct dcb_app_table *b); +void dcb_app_table_remove_replaced(struct dcb_app_table *a, + const struct dcb_app_table *b); +void dcb_app_table_remove_existing(struct dcb_app_table *a, + const struct dcb_app_table *b); + +bool dcb_app_is_pcp(const struct dcb_app *app); +bool dcb_app_is_dscp(const struct dcb_app *app); + +int dcb_app_print_key_dec(__u16 protocol); +int dcb_app_print_key_dscp(__u16 protocol); +int dcb_app_print_key_pcp(__u16 protocol); + +int dcb_app_parse_pcp(__u32 *key, const char *arg); +int dcb_app_parse_dscp(__u32 *key, const char *arg); +void dcb_app_parse_mapping_cb(__u32 key, __u64 value, void *data); + +bool dcb_app_selector_validate(enum ieee_attrs_app type, __u8 selector); +bool dcb_app_attr_type_validate(enum ieee_attrs_app type); +enum ieee_attrs_app dcb_app_attr_type_get(__u8 selector); + +#endif
Add new headerfile dcb-app.h that exposes the functions required later by dcb-rewr. The new dcb-rewr implementation will reuse much of the existing dcb-app code. I thought this called for a separate header file, instead of polluting the existing dcb.h file. Signed-off-by: Daniel Machon <daniel.machon@microchip.com> --- dcb/dcb.h | 9 ++------- dcb/dcb_app.c | 54 ++++++++++++++++++------------------------------------ dcb/dcb_app.h | 55 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 75 insertions(+), 43 deletions(-)