Messages in this thread | | | From | <> | Subject | Re: [PATCH net-next 2/6] net: dcb: add new common function for set/del of app/rewr entries | Date | Mon, 16 Jan 2023 13:00:10 +0000 |
| |
> > Signed-off-by: Daniel Machon <daniel.machon@microchip.com> > > --- > > net/dcb/dcbnl.c | 104 +++++++++++++++++++++++------------------------- > > 1 file changed, 49 insertions(+), 55 deletions(-) > > > > diff --git a/net/dcb/dcbnl.c b/net/dcb/dcbnl.c > > index a76bdf6f0198..6d19564e19a8 100644 > > --- a/net/dcb/dcbnl.c > > +++ b/net/dcb/dcbnl.c > > @@ -1099,6 +1099,45 @@ static int dcbnl_getapptrust(struct net_device *netdev, struct sk_buff *skb) > > return err; > > } > > > > +/* Set or delete APP table or rewrite table entries. The APP struct is validated > > + * and the appropriate callback function is called. > > + */ > > +static int dcbnl_apprewr_setdel(struct nlattr *attr, struct net_device *netdev, > > + int (*setdel)(struct net_device *dev, > > + struct dcb_app *app), > > + int (*ops_setdel)(struct net_device *dev, > > + struct dcb_app *app)) > > The name makes it look like it's rewrite-specific. Maybe make it > _app_table_? Given both DCB app and DCB rewrite use app table as the > database... Dunno. Not a big deal.
Yeah, the other shared funcs are called _app_* too, so lets stick with that naming. Agreed.
> > /* Handle IEEE 802.1Qaz/802.1Qau/802.1Qbb GET commands. */ > > static int dcbnl_ieee_fill(struct sk_buff *skb, struct net_device *netdev) > > { > > @@ -1568,36 +1607,11 @@ static int dcbnl_ieee_set(struct net_device *netdev, struct nlmsghdr *nlh, > > } > > > > if (ieee[DCB_ATTR_IEEE_APP_TABLE]) { > > - struct nlattr *attr; > > - int rem; > > - > > - nla_for_each_nested(attr, ieee[DCB_ATTR_IEEE_APP_TABLE], rem) { > > - enum ieee_attrs_app type = nla_type(attr); > > - struct dcb_app *app_data; > > - > > - if (!dcbnl_app_attr_type_validate(type)) > > - continue; > > - > > - if (nla_len(attr) < sizeof(struct dcb_app)) { > > - err = -ERANGE; > > - goto err; > > - } > > - > > - app_data = nla_data(attr); > > - > > - if (!dcbnl_app_selector_validate(type, > > - app_data->selector)) { > > - err = -EINVAL; > > - goto err; > > - } > > - > > - if (ops->ieee_setapp) > > - err = ops->ieee_setapp(netdev, app_data); > > - else > > - err = dcb_ieee_setapp(netdev, app_data); > > - if (err) > > - goto err; > > - } > > + err = dcbnl_apprewr_setdel(ieee[DCB_ATTR_IEEE_APP_TABLE], > > + netdev, dcb_ieee_setapp, > > + ops->ieee_setapp); > > This could pre-resolve the callback to use and pass one pointer: > > err = dcbnl_apprewr_setdel(ieee[DCB_ATTR_IEEE_APP_TABLE], netdev, > ops->ieee_setapp ?: dcb_ieee_setapp);
Agreed.
Will make the suggested changes in v2 - thank you Petr.
/Daniel
| |