[PATCH net-next V2] net/dcb: Validate bandwidth for non-ETS traffic classes

[PATCH net-next V2] net/dcb: Validate bandwidth for non-ETS traffic classes

[Tariq Toukan]

From: Alexei Lazar <alazar@nvidia.com>

The IEEE 802.1Qaz standard defines that bandwidth allocation percentages
only apply to ETS traffic classes. Reject ETS configurations that
specify non-zero bandwidth for non-ETS traffic classes.

Placing this check in dcbnl_ieee_set() enforces the constraint uniformly
for all drivers rather than relying on each driver to validate it
independently.

Drop the equivalent check from NFP's dcb_ets_check(). Keep ETS
bandwidth-sum validation unchanged.

Signed-off-by: Alexei Lazar <alazar@nvidia.com>
Reviewed-by: Carolina Jubran <cjubran@nvidia.com>
Signed-off-by: Tariq Toukan <tariqt@nvidia.com>
---
 drivers/net/ethernet/netronome/nfp/nic/dcb.c | 3 ---
 net/dcb/dcbnl.c                              | 9 +++++++++
 2 files changed, 9 insertions(+), 3 deletions(-)

V2:
- Generalized per Jakub's suggestion.

V1:
https://lore.kernel.org/all/20260622112925.624795-3-tariqt@nvidia.com/

diff --git a/drivers/net/ethernet/netronome/nfp/nic/dcb.c b/drivers/net/ethernet/netronome/nfp/nic/dcb.c
index bb498ac6bd7d..91fbfa3708fc 100644
--- a/drivers/net/ethernet/netronome/nfp/nic/dcb.c
+++ b/drivers/net/ethernet/netronome/nfp/nic/dcb.c
@@ -218,9 +218,6 @@ static int dcb_ets_check(struct net_device *dev, struct ieee_ets *ets)
 		if (ets->tc_tsa[i] == IEEE_8021QAZ_TSA_ETS) {
 			ets_exists = true;
 			sum += ets->tc_tx_bw[i];
-		} else if (ets->tc_tx_bw[i]) {
-			nfp_warn(app->cpp, "ETS BW for strict/vendor TC must be 0.");
-			return -EINVAL;
 		}
 	}
 
diff --git a/net/dcb/dcbnl.c b/net/dcb/dcbnl.c
index 35164bfa563d..ab71f5312876 100644
--- a/net/dcb/dcbnl.c
+++ b/net/dcb/dcbnl.c
@@ -1578,6 +1578,15 @@ static int dcbnl_ieee_set(struct net_device *netdev, struct nlmsghdr *nlh,
 
 	if (ieee[DCB_ATTR_IEEE_ETS] && ops->ieee_setets) {
 		struct ieee_ets *ets = nla_data(ieee[DCB_ATTR_IEEE_ETS]);
+
+		/* IEEE 802.1Qaz: BW allocation applies only to ETS TCs */
+		for (int i = 0; i < IEEE_8021QAZ_MAX_TCS; i++) {
+			if (ets->tc_tsa[i] != IEEE_8021QAZ_TSA_ETS &&
+			    ets->tc_tx_bw[i]) {
+				err = -EINVAL;
+				goto err;
+			}
+		}
 		err = ops->ieee_setets(netdev, ets);
 		if (err)
 			goto err;

base-commit: d8674294aefef02266c4d47ad10131f1bffbe534
-- 
2.44.0

Re: [PATCH net-next V2] net/dcb: Validate bandwidth for non-ETS traffic classes

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider.

Critical: 0 · High: 0 · Medium: 1 (1) · Low: 0

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [Medium] Round-trip mismatch: the kernel now rejects an ETS config that
  ice itself reports.

Pre-existing issues:
- [Medium] This problem was not introduced by this patch, but NFP's ETS
  path uses unchecked user-supplied prio_tc values as indexes into the…

--

net/dcb: Validate bandwidth for non-ETS traffic classes

This commit moves the check that rejects non-zero tc_tx_bw for non-ETS
traffic classes from the NFP driver into dcbnl_ieee_set(). Every driver
now gets this check. NFP's ETS bandwidth-sum check is left as it is.

> diff --git a/drivers/net/ethernet/netronome/nfp/nic/dcb.c b/drivers/net/ethernet/netronome/nfp/nic/dcb.c
> index bb498ac6bd7d6..91fbfa3708fc8 100644
> --- a/drivers/net/ethernet/netronome/nfp/nic/dcb.c
> +++ b/drivers/net/ethernet/netronome/nfp/nic/dcb.c
> @@ -218,9 +218,6 @@ static int dcb_ets_check(struct net_device *dev, struct ieee_ets *ets)
>  		if (ets->tc_tsa[i] == IEEE_8021QAZ_TSA_ETS) {
>  			ets_exists = true;
>  			sum += ets->tc_tx_bw[i];
> -		} else if (ets->tc_tx_bw[i]) {
> -			nfp_warn(app->cpp, "ETS BW for strict/vendor TC must be 0.");
> -			return -EINVAL;
>  		}
>  	}

[Severity: Medium]
This problem was not introduced by this patch, but NFP's ETS path still
does not check the user-supplied prio_tc values. Can they index past the
end of the 8-entry dcb->tc2idx[] array?

Neither dcbnl_ieee_set() nor the DCB_ATTR_IEEE_ETS policy checks prio_tc.
After this patch, dcb_ets_check() only checks the bandwidth sum. An
all-strict config with all tc_tx_bw set to 0 and prio_tc[0] = 255 passes
both checks. nfp_nic_dcbnl_ieee_setets() then stores the value as is:

drivers/net/ethernet/netronome/nfp/nic/dcb.c:nfp_nic_dcbnl_ieee_setets() {
    ...
		dcb->prio2tc[i] = ets->prio_tc[i];
    ...
}

nfp_nic_fill_ets() later uses it as an index:

drivers/net/ethernet/netronome/nfp/nic/dcb.c:nfp_nic_fill_ets() {
    ...
		writeb(dcb->tc2idx[dcb->prio2tc[i]],
		       dcb->dcbcfg_tbl + dcb->cfg_offset + NFP_DCB_DATA_OFF_PCP2IDX + i);
    ...
}

This reads tc2idx[255], which is well past the end of the kzalloc'd
nfp_app_nic_private. The byte read goes into the device's PCP2IDX table.
update_dscp_maxrate() has the same index pattern:

		writeb(dcb->tc2idx[dcb->prio2tc[dcb->dscp2prio[i]]],

So does the setapp path, which looks up dcb->tc2idx[dcb->prio2tc[prio]].
Should prio_tc[i] < IEEE_8021QAZ_MAX_TCS be enforced somewhere on this
path?

> diff --git a/net/dcb/dcbnl.c b/net/dcb/dcbnl.c
> index 35164bfa563d1..ab71f53128760 100644
> --- a/net/dcb/dcbnl.c
> +++ b/net/dcb/dcbnl.c
> @@ -1578,6 +1578,15 @@ static int dcbnl_ieee_set(struct net_device *netdev, struct nlmsghdr *nlh,
>  
>  	if (ieee[DCB_ATTR_IEEE_ETS] && ops->ieee_setets) {
>  		struct ieee_ets *ets = nla_data(ieee[DCB_ATTR_IEEE_ETS]);
> +
> +		/* IEEE 802.1Qaz: BW allocation applies only to ETS TCs */
> +		for (int i = 0; i < IEEE_8021QAZ_MAX_TCS; i++) {
> +			if (ets->tc_tsa[i] != IEEE_8021QAZ_TSA_ETS &&
> +			    ets->tc_tx_bw[i]) {
> +				err = -EINVAL;
> +				goto err;
> +			}
> +		}

[Severity: Medium]
Can this reject an ETS object that a driver itself reported back to user
space?

On ice in host-managed IEEE mode, take an all-strict config with all
tc_tx_bw set to 0, or one with every priority mapped to a strict TC0.
That config passes this new check. ice_dcbnl_setets() then gives TC0 100%
bandwidth without looking at tsatable[0]:

drivers/net/ethernet/intel/ice/ice_dcb_nl.c:ice_dcbnl_setets() {
    ...
	if (!bwcfg)
		new_cfg->etscfg.tcbwtable[0] = 100;
    ...
}

ice_dcb_bwchk() does the same when num_tc == 1 or total_bw == 0:

drivers/net/ethernet/intel/ice/ice_dcb_lib.c:ice_dcb_bwchk() {
    ...
	if (num_tc == 1) {
		etscfg->tcbwtable[0] = ICE_TC_MAX_BW;
		return 0;
	}
    ...
	if (!total_bw) {
		etscfg->tcbwtable[0] = ICE_TC_MAX_BW;
    ...
}

ice_pf_dcb_cfg() copies new_cfg into local_dcbx_cfg. ice_dcbnl_getets()
then copies tcbwtable and tsatable into struct ieee_ets as they are. So
both GET replies and the RTM_SETDCB notification from dcbnl_ieee_notify()
report tc_tsa[0] = STRICT and tc_tx_bw[0] = 100.

If that object is sent back to DCB_CMD_IEEE_SET, it now fails here with
-EINVAL. Before this patch the same round trip worked on ice.

Read-modify-write users such as iproute2's dcb ets set read the current
ETS object before applying a change. Wouldn't a change to an unrelated
field like prio-tc or willing now fail, unless the user also clears the
strict TC's bandwidth each time? Save and restore of the reported object
would fail the same way.

Should drivers like ice be changed to stop reporting bandwidth for
non-ETS TCs before the core enforces this rule?

>  		err = ops->ieee_setets(netdev, ets);
>  		if (err)
>  			goto err;

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261011083929.739261-1-tariqt%40nvidia.com