| 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: <87lehf5gu1.fsf@nvidia.com>
Date: Tue, 23 May 2023 13:18:40 +0200
From: Petr Machata <petrm@...dia.com>
To: Daniel Machon <daniel.machon@...rochip.com>
CC: <netdev@...r.kernel.org>, <dsahern@...nel.org>,
<stephen@...workplumber.org>, <petrm@...dia.com>,
<UNGLinuxDriver@...rochip.com>
Subject: Re: [PATCH iproute2-next 1/9] dcb: app: expose dcb-app functions in
new header
Daniel Machon <daniel.machon@...rochip.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@...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.
> /* 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
Powered by blists - more mailing lists