| Message ID | 20230116144853.2446315-3-daniel.machon@microchip.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | Introduce new DCB rewrite table | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Clearly marked for net-next |
| netdev/fixes_present | success | Fixes tag not required for -next series |
| netdev/subject_prefix | success | Link |
| netdev/cover_letter | success | Series has a cover letter |
| netdev/patch_count | success | Link |
| netdev/header_inline | success | No static functions without inline keyword in header files |
| netdev/build_32bit | success | Errors and warnings before: 2 this patch: 2 |
| netdev/cc_maintainers | success | CCed 8 of 8 maintainers |
| netdev/build_clang | success | Errors and warnings before: 1 this patch: 1 |
| netdev/module_param | success | Was 0 now: 0 |
| netdev/verify_signedoff | success | Signed-off-by tag matches author and committer |
| netdev/check_selftest | success | No net selftest shell script |
| netdev/verify_fixes | success | No Fixes tag |
| netdev/build_allmodconfig_warn | success | Errors and warnings before: 2 this patch: 2 |
| netdev/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 118 lines checked |
| netdev/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| netdev/source_inline | success | Was 0 now: 0 |
Daniel Machon <daniel.machon@microchip.com> writes: > In preparation for DCB rewrite. Add a new function for setting and > deleting both app and rewrite entries. Moving this into a separate > function reduces duplicate code, as both type of entries requires the > same set of checks. The function will now iterate through a configurable > nested attribute (app or rewrite attr), validate each attribute and call > the appropriate set- or delete function. > > Note that this function always checks for nla_len(attr_itr) < > sizeof(struct dcb_app), which was only done in dcbnl_ieee_set and not in > dcbnl_ieee_del prior to this patch. This means, that any userspace tool > that used to shove in data < sizeof(struct dcb_app) would now receive > -ERANGE. > > Signed-off-by: Daniel Machon <daniel.machon@microchip.com> Reviewed-by: Petr Machata <petrm@nvidia.com>
Petr Machata <petrm@nvidia.com> writes: > Daniel Machon <daniel.machon@microchip.com> writes: > >> In preparation for DCB rewrite. Add a new function for setting and >> deleting both app and rewrite entries. Moving this into a separate >> function reduces duplicate code, as both type of entries requires the >> same set of checks. The function will now iterate through a configurable >> nested attribute (app or rewrite attr), validate each attribute and call >> the appropriate set- or delete function. >> >> Note that this function always checks for nla_len(attr_itr) < >> sizeof(struct dcb_app), which was only done in dcbnl_ieee_set and not in >> dcbnl_ieee_del prior to this patch. This means, that any userspace tool >> that used to shove in data < sizeof(struct dcb_app) would now receive >> -ERANGE. >> >> Signed-off-by: Daniel Machon <daniel.machon@microchip.com> > > Reviewed-by: Petr Machata <petrm@nvidia.com> ... though, now that I found some issues in 3/6, if you would somehow reformat the ?: expression that's now awkwardly split to two unaligned lines, that would placate my OCD: + err = dcbnl_app_table_setdel(ieee[DCB_ATTR_IEEE_APP_TABLE], + netdev, ops->ieee_setapp ?: + dcb_ieee_setapp); (and the one other).
> Petr Machata <petrm@nvidia.com> writes: > > > Daniel Machon <daniel.machon@microchip.com> writes: > > > >> In preparation for DCB rewrite. Add a new function for setting and > >> deleting both app and rewrite entries. Moving this into a separate > >> function reduces duplicate code, as both type of entries requires the > >> same set of checks. The function will now iterate through a configurable > >> nested attribute (app or rewrite attr), validate each attribute and call > >> the appropriate set- or delete function. > >> > >> Note that this function always checks for nla_len(attr_itr) < > >> sizeof(struct dcb_app), which was only done in dcbnl_ieee_set and not in > >> dcbnl_ieee_del prior to this patch. This means, that any userspace tool > >> that used to shove in data < sizeof(struct dcb_app) would now receive > >> -ERANGE. > >> > >> Signed-off-by: Daniel Machon <daniel.machon@microchip.com> > > > > Reviewed-by: Petr Machata <petrm@nvidia.com> > > ... though, now that I found some issues in 3/6, if you would somehow > reformat the ?: expression that's now awkwardly split to two unaligned > lines, that would placate my OCD: > > + err = dcbnl_app_table_setdel(ieee[DCB_ATTR_IEEE_APP_TABLE], > + netdev, ops->ieee_setapp ?: > + dcb_ieee_setapp); Putting the expression on the same line will violate the 80 char limit. Does splitting it like that hurt anything - other than your OCD :-P At least checkpatch didn't complain. /Daniel > > (and the one other).
<Daniel.Machon@microchip.com> writes: > > Petr Machata <petrm@nvidia.com> writes: >> >> > Daniel Machon <daniel.machon@microchip.com> writes: >> > >> >> In preparation for DCB rewrite. Add a new function for setting and >> >> deleting both app and rewrite entries. Moving this into a separate >> >> function reduces duplicate code, as both type of entries requires the >> >> same set of checks. The function will now iterate through a configurable >> >> nested attribute (app or rewrite attr), validate each attribute and call >> >> the appropriate set- or delete function. >> >> >> >> Note that this function always checks for nla_len(attr_itr) < >> >> sizeof(struct dcb_app), which was only done in dcbnl_ieee_set and not in >> >> dcbnl_ieee_del prior to this patch. This means, that any userspace tool >> >> that used to shove in data < sizeof(struct dcb_app) would now receive >> >> -ERANGE. >> >> >> >> Signed-off-by: Daniel Machon <daniel.machon@microchip.com> >> > >> > Reviewed-by: Petr Machata <petrm@nvidia.com> >> >> ... though, now that I found some issues in 3/6, if you would somehow >> reformat the ?: expression that's now awkwardly split to two unaligned >> lines, that would placate my OCD: >> >> + err = dcbnl_app_table_setdel(ieee[DCB_ATTR_IEEE_APP_TABLE], >> + netdev, ops->ieee_setapp ?: >> + dcb_ieee_setapp); > > Putting the expression on the same line will violate the 80 char limit. > Does splitting it like that hurt anything - other than your OCD :-P At > least checkpatch didn't complain. Yeah, don't worry about it.
diff --git a/net/dcb/dcbnl.c b/net/dcb/dcbnl.c index a76bdf6f0198..cb5319c6afe6 100644 --- a/net/dcb/dcbnl.c +++ b/net/dcb/dcbnl.c @@ -1099,6 +1099,41 @@ 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_app_table_setdel(struct nlattr *attr, + struct net_device *netdev, + int (*setdel)(struct net_device *dev, + struct dcb_app *app)) +{ + struct dcb_app *app_data; + enum ieee_attrs_app type; + struct nlattr *attr_itr; + int rem, err; + + nla_for_each_nested(attr_itr, attr, rem) { + type = nla_type(attr_itr); + + if (!dcbnl_app_attr_type_validate(type)) + continue; + + if (nla_len(attr_itr) < sizeof(struct dcb_app)) + return -ERANGE; + + app_data = nla_data(attr_itr); + + if (!dcbnl_app_selector_validate(type, app_data->selector)) + return -EINVAL; + + err = setdel(netdev, app_data); + if (err) + return err; + } + + return 0; +} + /* 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 +1603,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_app_table_setdel(ieee[DCB_ATTR_IEEE_APP_TABLE], + netdev, ops->ieee_setapp ?: + dcb_ieee_setapp); + if (err) + goto err; } if (ieee[DCB_ATTR_DCB_APP_TRUST_TABLE]) { @@ -1684,31 +1694,11 @@ static int dcbnl_ieee_del(struct net_device *netdev, struct nlmsghdr *nlh, return err; 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; - - app_data = nla_data(attr); - - if (!dcbnl_app_selector_validate(type, - app_data->selector)) { - err = -EINVAL; - goto err; - } - - if (ops->ieee_delapp) - err = ops->ieee_delapp(netdev, app_data); - else - err = dcb_ieee_delapp(netdev, app_data); - if (err) - goto err; - } + err = dcbnl_app_table_setdel(ieee[DCB_ATTR_IEEE_APP_TABLE], + netdev, ops->ieee_delapp ?: + dcb_ieee_delapp); + if (err) + goto err; } err:
In preparation for DCB rewrite. Add a new function for setting and deleting both app and rewrite entries. Moving this into a separate function reduces duplicate code, as both type of entries requires the same set of checks. The function will now iterate through a configurable nested attribute (app or rewrite attr), validate each attribute and call the appropriate set- or delete function. Note that this function always checks for nla_len(attr_itr) < sizeof(struct dcb_app), which was only done in dcbnl_ieee_set and not in dcbnl_ieee_del prior to this patch. This means, that any userspace tool that used to shove in data < sizeof(struct dcb_app) would now receive -ERANGE. Signed-off-by: Daniel Machon <daniel.machon@microchip.com> --- net/dcb/dcbnl.c | 100 ++++++++++++++++++++++-------------------------- 1 file changed, 45 insertions(+), 55 deletions(-)