diff mbox series

[iproute2-next,v2,3/8] dcb: app: modify dcb_app_table_remove_replaced() for dcb-rewr reuse

Message ID 20230510-dcb-rewr-v2-3-9f38e688117e@microchip.com (mailing list archive)
State Superseded
Delegated to: David Ahern
Headers show
Series Introduce new dcb-rewr subcommand | expand

Checks

Context Check Description
netdev/tree_selection success Not a local patch

Commit Message

Daniel Machon May 25, 2023, 6:10 p.m. UTC
When doing a replace command, entries are checked against selector and
protocol. Rewrite requires the check to be against selector and
priority.

Adapt the existing dcb_app_table_remove_replace function for this, by
using callback functions for selector, pid and prio checks.

Signed-off-by: Daniel Machon <daniel.machon@microchip.com>
---
 dcb/dcb.h     | 14 ++++++++++++++
 dcb/dcb_app.c | 32 ++++++++++++++++++++------------
 2 files changed, 34 insertions(+), 12 deletions(-)

Comments

Petr Machata May 29, 2023, 5 p.m. UTC | #1
Daniel Machon <daniel.machon@microchip.com> writes:

> When doing a replace command, entries are checked against selector and
> protocol. Rewrite requires the check to be against selector and
> priority.
>
> Adapt the existing dcb_app_table_remove_replace function for this, by
> using callback functions for selector, pid and prio checks.
>
> Signed-off-by: Daniel Machon <daniel.machon@microchip.com>
> ---
>  dcb/dcb.h     | 14 ++++++++++++++
>  dcb/dcb_app.c | 32 ++++++++++++++++++++------------
>  2 files changed, 34 insertions(+), 12 deletions(-)
>
> diff --git a/dcb/dcb.h b/dcb/dcb.h
> index d40664f29dad..84ce95d5c1b2 100644
> --- a/dcb/dcb.h
> +++ b/dcb/dcb.h
> @@ -56,11 +56,25 @@ void dcb_print_array_kw(const __u8 *array, size_t array_size,
>  
>  /* dcb_app.c */
>  
> +struct dcb_app_table {
> +	struct dcb_app *apps;
> +	size_t n_apps;
> +	int attr;
> +};
> +
>  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);
>  
> +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);

This function isn't necessary until 5/8, that's when it should be added.

> +
> +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));
> +

This is conflating publication with prototype change. It would be best
to separate. I think the publication could be done at the point when you
actually need the function, i.e. 5/8 as well. I would keep here just the
prototype change.

>  /* 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 1e36541bec61..4cd175a0623b 100644
> --- a/dcb/dcb_app.c
> +++ b/dcb/dcb_app.c
> @@ -68,12 +68,6 @@ static void dcb_app_help(void)
>  	dcb_app_help_add();
>  }
>  
> -struct dcb_app_table {
> -	struct dcb_app *apps;
> -	size_t n_apps;
> -	int attr;
> -};
> -
>  enum ieee_attrs_app dcb_app_attr_type_get(__u8 selector)
>  {
>  	switch (selector) {
> @@ -153,8 +147,22 @@ 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)
> +bool dcb_app_pid_eq(const struct dcb_app *aa, const struct dcb_app *ab)
> +{
> +	return aa->selector == ab->selector &&
> +	       aa->protocol == ab->protocol;
> +}
> +
> +bool dcb_app_prio_eq(const struct dcb_app *aa, const struct dcb_app *ab)
> +{
> +	return aa->selector == ab->selector &&
> +	       aa->priority == ab->priority;
> +}
> +
> +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))
>  {
>  	size_t ia, ja;
>  	size_t ib;
> @@ -167,13 +175,13 @@ static void dcb_app_table_remove_replaced(struct dcb_app_table *a,
>  		for (ib = 0; ib < b->n_apps; ib++) {
>  			const struct dcb_app *ab = &b->apps[ib];
>  
> -			if (aa->selector == ab->selector &&
> -			    aa->protocol == ab->protocol)
> +			if (key_eq(aa, ab))
>  				present = true;
>  			else
>  				continue;
>  
> -			if (aa->priority == ab->priority) {
> +			if (aa->protocol == ab->protocol &&
> +			    aa->priority == ab->priority) {
>  				found = true;
>  				break;
>  			}
> @@ -891,7 +899,7 @@ static int dcb_cmd_app_replace(struct dcb *dcb, const char *dev, int argc, char
>  	}
>  
>  	/* Remove the obsolete entries. */
> -	dcb_app_table_remove_replaced(&orig, &tab);
> +	dcb_app_table_remove_replaced(&orig, &tab, dcb_app_pid_eq);
>  	ret = dcb_app_add_del(dcb, dev, DCB_CMD_IEEE_DEL, &orig, NULL);
>  	if (ret != 0) {
>  		fprintf(stderr, "Could not remove replaced APP entries\n");
Daniel Machon May 30, 2023, 8:03 a.m. UTC | #2
> > When doing a replace command, entries are checked against selector and
> > protocol. Rewrite requires the check to be against selector and
> > priority.
> >
> > Adapt the existing dcb_app_table_remove_replace function for this, by
> > using callback functions for selector, pid and prio checks.
> >
> > Signed-off-by: Daniel Machon <daniel.machon@microchip.com>
> > ---
> >  dcb/dcb.h     | 14 ++++++++++++++
> >  dcb/dcb_app.c | 32 ++++++++++++++++++++------------
> >  2 files changed, 34 insertions(+), 12 deletions(-)
> >
> > diff --git a/dcb/dcb.h b/dcb/dcb.h
> > index d40664f29dad..84ce95d5c1b2 100644
> > --- a/dcb/dcb.h
> > +++ b/dcb/dcb.h
> > @@ -56,11 +56,25 @@ void dcb_print_array_kw(const __u8 *array, size_t array_size,
> >
> >  /* dcb_app.c */
> >
> > +struct dcb_app_table {
> > +     struct dcb_app *apps;
> > +     size_t n_apps;
> > +     int attr;
> > +};
> > +
> >  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);
> >
> > +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);
> 
> This function isn't necessary until 5/8, that's when it should be added.
> 
> > +
> > +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));
> > +
> 
> This is conflating publication with prototype change. It would be best
> to separate. I think the publication could be done at the point when you
> actually need the function, i.e. 5/8 as well. I would keep here just the
> prototype change.
> 

Sounds OK to me. Will change in next v.
Petr Machata May 30, 2023, 8:29 p.m. UTC | #3
Petr Machata <petrm@nvidia.com> writes:

> Daniel Machon <daniel.machon@microchip.com> writes:
>
>> diff --git a/dcb/dcb.h b/dcb/dcb.h
>> index d40664f29dad..84ce95d5c1b2 100644
>> --- a/dcb/dcb.h
>> +++ b/dcb/dcb.h
>> @@ -56,11 +56,25 @@ void dcb_print_array_kw(const __u8 *array, size_t array_size,
>>  
>>  /* dcb_app.c */
>>  
>> +struct dcb_app_table {
>> +	struct dcb_app *apps;
>> +	size_t n_apps;
>> +	int attr;
>> +};
>> +
>>  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);
>>  
>> +bool dcb_app_pid_eq(const struct dcb_app *aa, const struct dcb_app *ab);

And I suspect this one does not need to be public at all?

>> +bool dcb_app_prio_eq(const struct dcb_app *aa, const struct dcb_app *ab);
>
> This function isn't necessary until 5/8, that's when it should be added.
diff mbox series

Patch

diff --git a/dcb/dcb.h b/dcb/dcb.h
index d40664f29dad..84ce95d5c1b2 100644
--- a/dcb/dcb.h
+++ b/dcb/dcb.h
@@ -56,11 +56,25 @@  void dcb_print_array_kw(const __u8 *array, size_t array_size,
 
 /* dcb_app.c */
 
+struct dcb_app_table {
+	struct dcb_app *apps;
+	size_t n_apps;
+	int attr;
+};
+
 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);
 
+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);
+
+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));
+
 /* 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 1e36541bec61..4cd175a0623b 100644
--- a/dcb/dcb_app.c
+++ b/dcb/dcb_app.c
@@ -68,12 +68,6 @@  static void dcb_app_help(void)
 	dcb_app_help_add();
 }
 
-struct dcb_app_table {
-	struct dcb_app *apps;
-	size_t n_apps;
-	int attr;
-};
-
 enum ieee_attrs_app dcb_app_attr_type_get(__u8 selector)
 {
 	switch (selector) {
@@ -153,8 +147,22 @@  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)
+bool dcb_app_pid_eq(const struct dcb_app *aa, const struct dcb_app *ab)
+{
+	return aa->selector == ab->selector &&
+	       aa->protocol == ab->protocol;
+}
+
+bool dcb_app_prio_eq(const struct dcb_app *aa, const struct dcb_app *ab)
+{
+	return aa->selector == ab->selector &&
+	       aa->priority == ab->priority;
+}
+
+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))
 {
 	size_t ia, ja;
 	size_t ib;
@@ -167,13 +175,13 @@  static void dcb_app_table_remove_replaced(struct dcb_app_table *a,
 		for (ib = 0; ib < b->n_apps; ib++) {
 			const struct dcb_app *ab = &b->apps[ib];
 
-			if (aa->selector == ab->selector &&
-			    aa->protocol == ab->protocol)
+			if (key_eq(aa, ab))
 				present = true;
 			else
 				continue;
 
-			if (aa->priority == ab->priority) {
+			if (aa->protocol == ab->protocol &&
+			    aa->priority == ab->priority) {
 				found = true;
 				break;
 			}
@@ -891,7 +899,7 @@  static int dcb_cmd_app_replace(struct dcb *dcb, const char *dev, int argc, char
 	}
 
 	/* Remove the obsolete entries. */
-	dcb_app_table_remove_replaced(&orig, &tab);
+	dcb_app_table_remove_replaced(&orig, &tab, dcb_app_pid_eq);
 	ret = dcb_app_add_del(dcb, dev, DCB_CMD_IEEE_DEL, &orig, NULL);
 	if (ret != 0) {
 		fprintf(stderr, "Could not remove replaced APP entries\n");