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(-)
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:
--
2.34.1Daniel 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.
© 2016 - 2025 Red Hat, Inc.