Re: [PATCH iproute2-next v2 4/8] dcb: app: modify dcb_app_parse_mapping_cb for dcb-rewr reuse

[Petr Machata <petrm@nvidia.com>]

Daniel Machon <daniel.machon@microchip.com> writes:

> When parsing APP table entries, priority and protocol is assigned from
> value and key, respectively. Rewrite requires it opposite.
>
> Adapt the existing dcb_app_parse_mapping_cb for this, by using callbacks
> for pushing app or rewr entries to the table.
>
> Signed-off-by: Daniel Machon <daniel.machon@microchip.com>
> ---
>  dcb/dcb.h     | 12 ++++++++++++
>  dcb/dcb_app.c | 23 ++++++++++++-----------
>  2 files changed, 24 insertions(+), 11 deletions(-)
>
> diff --git a/dcb/dcb.h b/dcb/dcb.h
> index 84ce95d5c1b2..b3bc30cd02c5 100644
> --- a/dcb/dcb.h
> +++ b/dcb/dcb.h
> @@ -62,7 +62,16 @@ struct dcb_app_table {
>  	int attr;
>  };
>  
> +struct dcb_app_parse_mapping {
> +	__u8 selector;
> +	struct dcb_app_table *tab;
> +	int (*push)(struct dcb_app_table *tab,
> +		    __u8 selector, __u32 key, __u64 value);
> +	int err;
> +};
> +
>  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);
> @@ -70,11 +79,14 @@ bool dcb_app_selector_validate(enum ieee_attrs_app type, __u8 selector);
>  bool dcb_app_pid_eq(const struct dcb_app *aa, const struct dcb_app *ab);
>  bool dcb_app_prio_eq(const struct dcb_app *aa, const struct dcb_app *ab);
>  
> +int dcb_app_table_push(struct dcb_app_table *tab, struct dcb_app *app);
>  void dcb_app_table_remove_replaced(struct dcb_app_table *a,
>  				   const struct dcb_app_table *b,
>  				   bool (*key_eq)(const struct dcb_app *aa,
>  						  const struct dcb_app *ab));
>  
> +void dcb_app_parse_mapping_cb(__u32 key, __u64 value, void *data);
> +
>  /* 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 4cd175a0623b..97cba658aa6b 100644
> --- a/dcb/dcb_app.c
> +++ b/dcb/dcb_app.c
> @@ -105,7 +105,7 @@ static void dcb_app_table_fini(struct dcb_app_table *tab)
>  	free(tab->apps);
>  }
>  
> -static int dcb_app_table_push(struct dcb_app_table *tab, struct dcb_app *app)
> +int dcb_app_table_push(struct dcb_app_table *tab, struct dcb_app *app)
>  {
>  	struct dcb_app *apps = realloc(tab->apps, (tab->n_apps + 1) * sizeof(*tab->apps));
>  
> @@ -231,25 +231,25 @@ static 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)
> +static int dcb_app_push(struct dcb_app_table *tab,
> +			__u8 selector, __u32 key, __u64 value)
>  {
> -	struct dcb_app_parse_mapping *pm = data;
>  	struct dcb_app app = {
> -		.selector = pm->selector,
> +		.selector = selector,
>  		.priority = value,
>  		.protocol = key,
>  	};
> +	return dcb_app_table_push(tab, &app);
> +}
> +
> +void dcb_app_parse_mapping_cb(__u32 key, __u64 value, void *data)
> +{
> +	struct dcb_app_parse_mapping *pm = data;
>  
>  	if (pm->err)
>  		return;
>  
> -	pm->err = dcb_app_table_push(pm->tab, &app);
> +	pm->err = pm->push(pm->tab, pm->selector, key, value);
>  }
>  
>  static int dcb_app_parse_mapping_ethtype_prio(__u32 key, char *value, void *data)
> @@ -663,6 +663,7 @@ static int dcb_cmd_app_parse_add_del(struct dcb *dcb, const char *dev,
>  {
>  	struct dcb_app_parse_mapping pm = {
>  		.tab = tab,
> +		.push = dcb_app_push,
>  	};
>  	int ret;

I think I misunderstood your code. Since you are adding new functions
for parsing the PRIO-DSCP and PRIO-PCP mappings, which have their own
dcb_parse_mapping() invocations, couldn't you just copy over the
dcb_app_parse_mapping_cb() from APP and adapt it to do the right thing
for REWR? Then the push callback is not even necessary
dcb_app_parse_mapping_cb() does not need to be public.