From: Daniel Machon <daniel.machon@microchip.com> Date: 2022-09-08 11:56:28
This patch series adds support for offloading PCP-based queue
classification and introduces a new APPTRUST extension attribute to the
8021Qaz APP managed object. Prior to implemenation, it has been
discussed on the netdev mailing list here:
https://lore.kernel.org/netdev/Yv9VO1DYAxNduw6A@DEN-LT-70577/
In summary: there currently exist no conveinent way to offload per-port
PCP-based queue classification to hardware. Similarly, there is no way
to indicate the notion of trust for APP table selectors. This patch series
addresses both topics.
PCP based queue classification:
8021Q standardizes the Priority Code Point table (see 6.9.3 of
IEEE Std 802.1Q-2018). This patch series makes it possible, to
offload the PCP classification to said table. The new PCP
selector is not a standard part of the APP managed object,
therefore it has been assigned a value of 255 to avoid any
clashes with future DCB standard extensions.
Selector trust:
ASIC's often has the notion of trust DSCP and trust PCP. This
new object makes it possible to specify a trust order of app
selectors, which drivers can then react on.
Patch #1 introduces a new PCP selector to the APP object, which makes it
possible to encode PCP and DEI in the app triplet and offload it to the
PCP table of the ASIC.
Patch #2 Introduces the new extension attribute APPTRUST, which is a
list of trusted app selectors. Lower indexes has higher precedence.
selectors are stored consecutively, starting from index zero. With a
maximum number of 255 unique selectors, the list has the same maximum
size.
The userspace part of this will be posted in a separate patch series.
Daniel Machon (2):
net: dcb: add new pcp selector to app object
net: dcb: add new apptrust attribute
include/net/dcbnl.h | 2 ++
include/uapi/linux/dcbnl.h | 15 +++++++++++++++
net/dcb/dcbnl.c | 17 +++++++++++++++++
3 files changed, 34 insertions(+)
--
2.34.1
From: Daniel Machon <daniel.machon@microchip.com> Date: 2022-09-08 11:56:29
Add new PCP selector for the 8021Qaz APP managed object.
The purpose of adding the PCP selector, is to be able to offload
PCP-based queue classification to the 8021Q Priority Code Point table,
see 6.9.3 of IEEE Std 802.1Q-2018.
PCP and DEI is encoded in the protocol field as 8*dei+pcp, so that a
mapping of PCP 2 and DEI 1 to priority 3 is encoded as {255, 10, 3}.
While PCP is not a standard 8021Qaz selector, it seems very convenient
to add it to the APP object, as this is where similar priority mapping
is handled, and it perfectly fits the {selector, protocol, priority}
triplet.
Signed-off-by: Daniel Machon <daniel.machon@microchip.com>
---
include/uapi/linux/dcbnl.h | 1 +
1 file changed, 1 insertion(+)
From: Daniel Machon <daniel.machon@microchip.com> Date: 2022-09-08 11:56:36
Add a new apptrust extension attribute to the 8021Qaz APP managed
object.
The new attribute is meant to allow drivers, whose hw supports the
notion of trust, to be able to set whether a particular app selector is
to be trusted - and also the order of precedence of selectors.
A new structure ieee_apptrust has been created, which contains an array
of selectors, where lower indexes has higher precedence.
Signed-off-by: Daniel Machon <daniel.machon@microchip.com>
---
include/net/dcbnl.h | 2 ++
include/uapi/linux/dcbnl.h | 14 ++++++++++++++
net/dcb/dcbnl.c | 17 +++++++++++++++++
3 files changed, 33 insertions(+)
@@ -248,6 +248,19 @@ struct dcb_app {__u16protocol;};+#define IEEE_8021QAZ_APP_SEL_MAX 255++/* This structure contains trust order extension to the IEEE 802.1Qaz APP+*managedobject.+*+*@order:containstrustorderingofselectorvaluesfortheIEEE802.1Qaz+*APPmanagedobject.Lowerindexeshashighertrust.+*/+structieee_apptrust{+__u8num;+__u8order[IEEE_8021QAZ_APP_SEL_MAX];+};+/***structdcb_peer_app_info-APPfeatureinformationsentbythepeer*
@@ -1133,6 +1134,14 @@ static int dcbnl_ieee_fill(struct sk_buff *skb, struct net_device *netdev)spin_unlock_bh(&dcb_lock);nla_nest_end(skb,app);+if(ops->ieee_getapptrust){+structieee_apptrusttrust;+memset(&trust,0,sizeof(trust));+err=ops->ieee_getapptrust(netdev,&trust);+if(!err&&nla_put(skb,DCB_ATTR_IEEE_APP_TRUST,sizeof(trust),&trust))+return-EMSGSIZE;+}+/* get peer info if available */if(ops->ieee_peer_getets){structieee_etsets;
From: Vladimir Oltean <vladimir.oltean@nxp.com> Date: 2022-09-09 12:29:56
Hi Daniel,
On Thu, Sep 08, 2022 at 02:04:42PM +0200, Daniel Machon wrote:
Add a new apptrust extension attribute to the 8021Qaz APP managed
object.
The new attribute is meant to allow drivers, whose hw supports the
notion of trust, to be able to set whether a particular app selector is
to be trusted - and also the order of precedence of selectors.
A new structure ieee_apptrust has been created, which contains an array
of selectors, where lower indexes has higher precedence.
Signed-off-by: Daniel Machon <daniel.machon@microchip.com>
---
Let's say I have a switch which only looks at VLAN PCP/DEI if the bridge
vlan_filtering setting is enabled (otherwise, the switch is completely
VLAN unaware, including for QoS purposes).
Would it be ok to report through ieee_getapptrust() that the PCP
selector is trusted when under a vlan_filtering bridge, not trusted when
not under a vlan_filtering bridge, and deny changes to ieee_setapptrust()
for the PCP selector? I see the return value is not cached anywhere
within the kernel, just passed to the user.
Den Fri, Sep 09, 2022 at 12:29:50PM +0000 skrev Vladimir Oltean:
EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
Hi Daniel,
On Thu, Sep 08, 2022 at 02:04:42PM +0200, Daniel Machon wrote:
quoted
Add a new apptrust extension attribute to the 8021Qaz APP managed
object.
The new attribute is meant to allow drivers, whose hw supports the
notion of trust, to be able to set whether a particular app selector is
to be trusted - and also the order of precedence of selectors.
A new structure ieee_apptrust has been created, which contains an array
of selectors, where lower indexes has higher precedence.
Signed-off-by: Daniel Machon <daniel.machon@microchip.com>
---
Let's say I have a switch which only looks at VLAN PCP/DEI if the bridge
vlan_filtering setting is enabled (otherwise, the switch is completely
VLAN unaware, including for QoS purposes).
Would it be ok to report through ieee_getapptrust() that the PCP
selector is trusted when under a vlan_filtering bridge, not trusted when
not under a vlan_filtering bridge, and deny changes to ieee_setapptrust()
for the PCP selector? I see the return value is not cached anywhere
within the kernel, just passed to the user.
There *might* be a distinction between enabled and trusted, disabled and not-trusted.
For instance, sparx5 switch has this distinction (at least for dscp) - but really that is
hw dependent. Therefore, in your particular case, with the vlan_filtering on/off,
yes that would be OK IMO. Any concerns?
This patch merely provides the means for drivers to implement a user-specified trust
order and report it back to the user, just like with many of the other dcb attributes
(maxrate, buffer etc.).
From: Petr Machata <petrm@nvidia.com> Date: 2022-09-12 16:15:02
Daniel Machon [off-list ref] writes:
quoted hunk
Add a new apptrust extension attribute to the 8021Qaz APP managed
object.
The new attribute is meant to allow drivers, whose hw supports the
notion of trust, to be able to set whether a particular app selector is
to be trusted - and also the order of precedence of selectors.
A new structure ieee_apptrust has been created, which contains an array
of selectors, where lower indexes has higher precedence.
Signed-off-by: Daniel Machon <daniel.machon@microchip.com>
---
include/net/dcbnl.h | 2 ++
include/uapi/linux/dcbnl.h | 14 ++++++++++++++
net/dcb/dcbnl.c | 17 +++++++++++++++++
3 files changed, 33 insertions(+)
@@ -248,6 +248,19 @@ struct dcb_app {__u16protocol;};+#define IEEE_8021QAZ_APP_SEL_MAX 255++/* This structure contains trust order extension to the IEEE 802.1Qaz APP+*managedobject.+*+*@order:containstrustorderingofselectorvaluesfortheIEEE802.1Qaz+*APPmanagedobject.Lowerindexeshashighertrust.+*/+structieee_apptrust{
Trust level setting is not standard, so this should be something like
dcbnl_apptrust.
Ditto for DCB_ATTR_IEEE_APP_TRUST below. I believe DCB_ATTR_DCB_BUFFER
is in the DCB namespace for that reason, and the trust level artifacts
should be as well. Likewise with the DCB ops callbacks, which should
IMHO be dcbnl_get/setapptrust.
+ __u8 num;
Is this supposed to be a count of the "order" items?
If yes, I'd call it "count", because then it's clear the value is not
just a number, but actually a number of items.
+ __u8 order[IEEE_8021QAZ_APP_SEL_MAX];
Should be +1 IMHO, in case the whole u8 selector space is allocated. (In
particular, there's no guarantee that IEEE won't ever allocate the
selector 0.)
But of course this will never get anywhere close to that. We will end up
passing maybe one, two entries. So the UAPI seems excessive in how it
hands around this large array.
I wonder if it would be better to make the DCB_ATTR_IEEE_APP_TABLE
payload be an array of bytes, each byte a selector? Or something similar
to DCB_ATTR_IEEE_APP_TABLE / DCB_ATTR_IEEE_APP, a nest and an array of
payload attributes?
quoted hunk
+};
+
/**
* struct dcb_peer_app_info - APP feature information sent by the peer
*
I believe checkpatch warns if there's no empty line between the variable
declaration block and the rest of the code.
Maybe it's just because this is an RFC, but for the final submission
please make sure you run checkpatch.pl --max-line-length=80. The
80-character limit is not really a hard requirement anymore, but it's
still strongly preferred in net.
quoted hunk
+ memset(&trust, 0, sizeof(trust));
+ err = ops->ieee_getapptrust(netdev, &trust);
+ if (!err && nla_put(skb, DCB_ATTR_IEEE_APP_TRUST, sizeof(trust), &trust))
+ return -EMSGSIZE;
+ }
+
/* get peer info if available */
if (ops->ieee_peer_getets) {
struct ieee_ets ets;
@@ -1513,6 +1522,14 @@ static int dcbnl_ieee_set(struct net_device *netdev, struct nlmsghdr *nlh, } }+ if (ieee[DCB_ATTR_IEEE_APP_TRUST] && ops->ieee_setapptrust) {
Hmm, weird that none of the set requests are bounced if there's no set
callback. That way the request appears to succeed but doesn't actually
set anything. I find this very strange.
Drivers that do not even have any DCB ops give -EOPNOTSUPP as expected.
I think lack of a particular op should do this as well. We can't change
this for the existing ones, but IMHO the new OPs should be like that.
Besides invoking the OP, this should validate the payload. E.g. no
driver is supposed to accept trust policies that contain invalid
selectors. Pretty sure there's no value in repeated entries either.
From: Petr Machata <petrm@nvidia.com> Date: 2022-09-12 16:26:28
Daniel Machon [off-list ref] writes:
quoted hunk
Add new PCP selector for the 8021Qaz APP managed object.
The purpose of adding the PCP selector, is to be able to offload
PCP-based queue classification to the 8021Q Priority Code Point table,
see 6.9.3 of IEEE Std 802.1Q-2018.
PCP and DEI is encoded in the protocol field as 8*dei+pcp, so that a
mapping of PCP 2 and DEI 1 to priority 3 is encoded as {255, 10, 3}.
While PCP is not a standard 8021Qaz selector, it seems very convenient
to add it to the APP object, as this is where similar priority mapping
is handled, and it perfectly fits the {selector, protocol, priority}
triplet.
Signed-off-by: Daniel Machon <daniel.machon@microchip.com>
---
include/uapi/linux/dcbnl.h | 1 +
1 file changed, 1 insertion(+)
I'm thinking how to further isolate this from the IEEE standard values.
I think it would be better to pass the non-standard APP contributions in
a different attribute. IIUIC, this is how the APP table is passed:
DCB_ATTR_IEEE_APP_TABLE {
DCB_ATTR_IEEE_APP {
struct dcb_app { ... };
}
DCB_ATTR_IEEE_APP {
struct dcb_app { ... };
}
}
Well, instead, the non-standard stuff could be passed in a different
attribute:
DCB_ATTR_IEEE_APP_TABLE {
DCB_ATTR_IEEE_APP {
struct dcb_app { ... }; // standard contribution to APP table
}
DCB_ATTR_DCB_APP {
struct dcb_app { ... }; // non-standard contribution
}
}
The new selector could still stay as 255. This will allow us to keep the
internal bookkeeping simple for the likely case that 255 never becomes a
valid IEEE selector. But if it ever does, the UAPI can stay the same,
just the internals will need to be updated.
From: Petr Machata <petrm@nvidia.com> Date: 2022-09-12 16:34:15
[off-list ref] writes:
Den Fri, Sep 09, 2022 at 12:29:50PM +0000 skrev Vladimir Oltean:
quoted
Let's say I have a switch which only looks at VLAN PCP/DEI if the bridge
vlan_filtering setting is enabled (otherwise, the switch is completely
VLAN unaware, including for QoS purposes).
Would it be ok to report through ieee_getapptrust() that the PCP
selector is trusted when under a vlan_filtering bridge, not trusted when
not under a vlan_filtering bridge, and deny changes to ieee_setapptrust()
for the PCP selector? I see the return value is not cached anywhere
within the kernel, just passed to the user.
Therefore, in your particular case, with the vlan_filtering on/off,
yes that would be OK IMO. Any concerns?
Yeah, it would make sense to me. With the 802.1q bridge, the reported
trust level would be [PCP], with 802.1d it would be [].
As a service to the user, I would accept set requests that just reassert
the only valid configuration, but otherwise it sounds OK to me.
From: Vladimir Oltean <vladimir.oltean@nxp.com> Date: 2022-09-12 17:16:22
On Mon, Sep 12, 2022 at 06:30:08PM +0200, Petr Machata wrote:
[off-list ref] writes:
quoted
Den Fri, Sep 09, 2022 at 12:29:50PM +0000 skrev Vladimir Oltean:
quoted
Let's say I have a switch which only looks at VLAN PCP/DEI if the bridge
vlan_filtering setting is enabled (otherwise, the switch is completely
VLAN unaware, including for QoS purposes).
Would it be ok to report through ieee_getapptrust() that the PCP
selector is trusted when under a vlan_filtering bridge, not trusted when
not under a vlan_filtering bridge, and deny changes to ieee_setapptrust()
for the PCP selector? I see the return value is not cached anywhere
within the kernel, just passed to the user.
Therefore, in your particular case, with the vlan_filtering on/off,
yes that would be OK IMO. Any concerns?
Yeah, it would make sense to me. With the 802.1q bridge, the reported
trust level would be [PCP], with 802.1d it would be [].
As a service to the user, I would accept set requests that just reassert
the only valid configuration, but otherwise it sounds OK to me.
The purpose of adding the PCP selector, is to be able to offload
PCP-based queue classification to the 8021Q Priority Code Point table,
see 6.9.3 of IEEE Std 802.1Q-2018.
PCP and DEI is encoded in the protocol field as 8*dei+pcp, so that a
mapping of PCP 2 and DEI 1 to priority 3 is encoded as {255, 10, 3}.
While PCP is not a standard 8021Qaz selector, it seems very convenient
to add it to the APP object, as this is where similar priority mapping
is handled, and it perfectly fits the {selector, protocol, priority}
triplet.
Signed-off-by: Daniel Machon <daniel.machon@microchip.com>
---
include/uapi/linux/dcbnl.h | 1 +
1 file changed, 1 insertion(+)
I'm thinking how to further isolate this from the IEEE standard values.
I think it would be better to pass the non-standard APP contributions in
a different attribute. IIUIC, this is how the APP table is passed:
DCB_ATTR_IEEE_APP_TABLE {
DCB_ATTR_IEEE_APP {
struct dcb_app { ... };
}
DCB_ATTR_IEEE_APP {
struct dcb_app { ... };
}
}
Well, instead, the non-standard stuff could be passed in a different
attribute:
DCB_ATTR_IEEE_APP_TABLE {
DCB_ATTR_IEEE_APP {
struct dcb_app { ... }; // standard contribution to APP table
}
DCB_ATTR_DCB_APP {
struct dcb_app { ... }; // non-standard contribution
}
}
The new selector could still stay as 255. This will allow us to keep the
internal bookkeeping simple for the likely case that 255 never becomes a
valid IEEE selector. But if it ever does, the UAPI can stay the same,
just the internals will need to be updated.
I get your sentiment, but it seems a little far-fetched to me. The
trade-off will be extra code, in trade for something that IMO very likely
will not happen. Like you said earlier - how many selectors could one
possibly prioritize on?
Add a new apptrust extension attribute to the 8021Qaz APP managed
object.
The new attribute is meant to allow drivers, whose hw supports the
notion of trust, to be able to set whether a particular app selector is
to be trusted - and also the order of precedence of selectors.
A new structure ieee_apptrust has been created, which contains an array
of selectors, where lower indexes has higher precedence.
Signed-off-by: Daniel Machon <daniel.machon@microchip.com>
---
include/net/dcbnl.h | 2 ++
include/uapi/linux/dcbnl.h | 14 ++++++++++++++
net/dcb/dcbnl.c | 17 +++++++++++++++++
3 files changed, 33 insertions(+)
@@ -248,6 +248,19 @@ struct dcb_app {__u16protocol;};+#define IEEE_8021QAZ_APP_SEL_MAX 255++/* This structure contains trust order extension to the IEEE 802.1Qaz APP+*managedobject.+*+*@order:containstrustorderingofselectorvaluesfortheIEEE802.1Qaz+*APPmanagedobject.Lowerindexeshashighertrust.+*/+structieee_apptrust{
Trust level setting is not standard, so this should be something like
dcbnl_apptrust.
Ack.
Ditto for DCB_ATTR_IEEE_APP_TRUST below. I believe DCB_ATTR_DCB_BUFFER
is in the DCB namespace for that reason, and the trust level artifacts
should be as well. Likewise with the DCB ops callbacks, which should
IMHO be dcbnl_get/setapptrust.
quoted
+ __u8 num;
Is this supposed to be a count of the "order" items?
If yes, I'd call it "count", because then it's clear the value is not
just a number, but actually a number of items.
Yes, this is the number of selector-occupied indexes. I dont have any strongs
feelings on num vs count - we can go with count.
quoted
+ __u8 order[IEEE_8021QAZ_APP_SEL_MAX];
Should be +1 IMHO, in case the whole u8 selector space is allocated. (In
particular, there's no guarantee that IEEE won't ever allocate the
selector 0.)
Ack.
But of course this will never get anywhere close to that. We will end up
passing maybe one, two entries. So the UAPI seems excessive in how it
hands around this large array.
I wonder if it would be better to make the DCB_ATTR_IEEE_APP_TABLE
payload be an array of bytes, each byte a selector? Or something similar
to DCB_ATTR_IEEE_APP_TABLE / DCB_ATTR_IEEE_APP, a nest and an array of
payload attributes?
Hmm. It might seem excessive, but a quick few thoughts on your proposed solution:
- We need more code to define and parse the new DCB_ATTR_IEEE_APP_TRUST_TABLE /
DCB_ATTR_IEEE_APP_TRUST attributes.
- If the selectors are passed individually to the driver, we need a
dcbnl_delapptrust(), because now, the driver have to add and del from the
driver maintained array. You could of course accumulate selectors in an array
before passing them to the driver, but then why go away from the array in the
first place.
I kind of like the 'set' nature more than the 'add' 'del' nature. What do you think?
quoted
+};
+
/**
* struct dcb_peer_app_info - APP feature information sent by the peer
*
I believe checkpatch warns if there's no empty line between the variable
declaration block and the rest of the code.
It does give a warning. No empty lines on any of the other declarations though,
but lets indeed add one here.
Maybe it's just because this is an RFC, but for the final submission
please make sure you run checkpatch.pl --max-line-length=80. The
80-character limit is not really a hard requirement anymore, but it's
still strongly preferred in net.
quoted
+ memset(&trust, 0, sizeof(trust));
+ err = ops->ieee_getapptrust(netdev, &trust);
+ if (!err && nla_put(skb, DCB_ATTR_IEEE_APP_TRUST, sizeof(trust), &trust))
+ return -EMSGSIZE;
+ }
+
/* get peer info if available */
if (ops->ieee_peer_getets) {
struct ieee_ets ets;
@@ -1513,6 +1522,14 @@ static int dcbnl_ieee_set(struct net_device *netdev, struct nlmsghdr *nlh, } }+ if (ieee[DCB_ATTR_IEEE_APP_TRUST] && ops->ieee_setapptrust) {
Hmm, weird that none of the set requests are bounced if there's no set
callback. That way the request appears to succeed but doesn't actually
set anything. I find this very strange.
Drivers that do not even have any DCB ops give -EOPNOTSUPP as expected.
I think lack of a particular op should do this as well. We can't change
this for the existing ones, but IMHO the new OPs should be like that.
Besides invoking the OP, this should validate the payload. E.g. no
driver is supposed to accept trust policies that contain invalid
selectors. Pretty sure there's no value in repeated entries either.
Validation (bogus input and unique selectors) is done in userspace (dcb-apptrust).
From: Petr Machata <petrm@nvidia.com> Date: 2022-09-13 12:51:26
[off-list ref] writes:
Petr Machata [off-list ref] writes:
quoted
But of course this will never get anywhere close to that. We will end up
passing maybe one, two entries. So the UAPI seems excessive in how it
hands around this large array.
I wonder if it would be better to make the DCB_ATTR_IEEE_APP_TABLE
payload be an array of bytes, each byte a selector? Or something similar
to DCB_ATTR_IEEE_APP_TABLE / DCB_ATTR_IEEE_APP, a nest and an array of
payload attributes?
Hmm. It might seem excessive, but a quick few thoughts on your proposed solution:
- We need more code to define and parse the new DCB_ATTR_IEEE_APP_TRUST_TABLE /
DCB_ATTR_IEEE_APP_TRUST attributes.
Yes, a bit. But it's not too bad IMHO. Am I forgetting something here?
u8 selectors[256];
int nselectors;
int rem;
nla_for_each_nested(attr, ieee[DCB_ATTR_DCB_APP_TRUST_TABLE], rem) {
if (nla_type(attr) != DCB_ATTR_DCB_APP_TRUST ||
nla_len(attr) != 1 ||
nselectors >= sizeof(selectors)) {
err = -EINVAL;
goto err;
}
selectors[nselectors++] = nla_get_u8(attr);
}
... and you have reconstructed the array.
- If the selectors are passed individually to the driver, we need a
dcbnl_delapptrust(), because now, the driver have to add and del from the
driver maintained array. You could of course accumulate selectors in an array
before passing them to the driver, but then why go away from the array in the
first place.
I have no problem with using an array for the in-kernel API. There it's
easy to change. UAPI can't ever change.
Besides invoking the OP, this should validate the payload. E.g. no
driver is supposed to accept trust policies that contain invalid
selectors. Pretty sure there's no value in repeated entries either.
Validation (bogus input and unique selectors) is done in userspace (dcb-apptrust).
Using iproute2 dcb is not mandatory, the UAPI is client-agnostic. The
kernel needs to bounce bogons as well. Otherwise they will become part
of the UAPI with the meaning "this doesn't do anything".
And yeah, drivers will validate supported configurations. But still the
requests that go to the driver should already be sanitized, so that the
driver code doesn't need to worry about this.
But of course this will never get anywhere close to that. We will end up
passing maybe one, two entries. So the UAPI seems excessive in how it
hands around this large array.
I wonder if it would be better to make the DCB_ATTR_IEEE_APP_TABLE
payload be an array of bytes, each byte a selector? Or something similar
to DCB_ATTR_IEEE_APP_TABLE / DCB_ATTR_IEEE_APP, a nest and an array of
payload attributes?
Hmm. It might seem excessive, but a quick few thoughts on your proposed solution:
- We need more code to define and parse the new DCB_ATTR_IEEE_APP_TRUST_TABLE /
DCB_ATTR_IEEE_APP_TRUST attributes.
Yes, a bit. But it's not too bad IMHO. Am I forgetting something here?
u8 selectors[256];
int nselectors;
int rem;
nla_for_each_nested(attr, ieee[DCB_ATTR_DCB_APP_TRUST_TABLE], rem) {
if (nla_type(attr) != DCB_ATTR_DCB_APP_TRUST ||
nla_len(attr) != 1 ||
nselectors >= sizeof(selectors)) {
err = -EINVAL;
goto err;
}
selectors[nselectors++] = nla_get_u8(attr);
}
... and you have reconstructed the array.
LGTM.
quoted
- If the selectors are passed individually to the driver, we need a
dcbnl_delapptrust(), because now, the driver have to add and del from the
driver maintained array. You could of course accumulate selectors in an array
before passing them to the driver, but then why go away from the array in the
first place.
I have no problem with using an array for the in-kernel API. There it's
easy to change. UAPI can't ever change.
Besides invoking the OP, this should validate the payload. E.g. no
driver is supposed to accept trust policies that contain invalid
selectors. Pretty sure there's no value in repeated entries either.
Validation (bogus input and unique selectors) is done in userspace (dcb-apptrust).
Using iproute2 dcb is not mandatory, the UAPI is client-agnostic. The
kernel needs to bounce bogons as well. Otherwise they will become part
of the UAPI with the meaning "this doesn't do anything".
And yeah, drivers will validate supported configurations. But still the
requests that go to the driver should already be sanitized, so that the
driver code doesn't need to worry about this.
Good point.
Will prepare a v2 with suggested changes.