lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Message-ID: <ZG2xBsorejY7v5l1@DEN-LT-70577> Date: Wed, 24 May 2023 06:39:03 +0000 From: <Daniel.Machon@...rochip.com> To: <petrm@...dia.com> CC: <netdev@...r.kernel.org>, <dsahern@...nel.org>, <stephen@...workplumber.org>, <UNGLinuxDriver@...rochip.com> Subject: Re: [PATCH iproute2-next 1/9] dcb: app: expose dcb-app functions in new header > > 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@...rochip.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 >
Powered by blists - more mailing lists