[net-next,v2,2/6] net: dcb: add new common function for set/del of app/rewr entries
Commit Message
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(-)
Comments
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.
@@ -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: