| 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