Re: [PATCH net-next v1 7/8] net: dsa: realtek: rtl8365mb: offload DCB apptrust
flat view
From: netdev-bot+sashiko@kernel.org
Date: 2026-10-06 13:43:27
Also in:
lkml
Thank you for your contribution! Sashiko AI review found more severe issue(s) in other patches of this series. This mail is only an FYI, in case you want to address the nit picks below when you respin. Low priority / nit picks for your consideration: - [Low] At this commit, rtl8365mb_port_set_apptrust() accepts IEEE_8021QAZ_APP_SEL_DSCP (listed in rtl8365mb_apptrust_map[]). - [Low] If a register access fails partway through rtl8365mb_port_set_apptrust(), the switch-wide trusted decision table… -- net: dsa: realtek: rtl8365mb: offload DCB apptrust This commit wires up port_get_apptrust/port_set_apptrust for rtl8365mb. It programs a second, switch-wide "trusted" priority-decision table with PCP and/or DSCP weights and steers ports to it with a per-port select bit. Precedence is fixed to PCP > DSCP, and a request that conflicts with the selectors another port already uses is refused with -EBUSY.
quoted hunk ↗ jump to hunk
diff --git a/drivers/net/dsa/realtek/rtl8365mb_dcb.c b/drivers/net/dsa/realtek/rtl8365mb_dcb.c index f48e95ef3947a..4afcae6e85a1e 100644 --- a/drivers/net/dsa/realtek/rtl8365mb_dcb.c +++ b/drivers/net/dsa/realtek/rtl8365mb_dcb.c
[ ... ]
quoted hunk ↗ jump to hunk
@@ -84,6 +88,24 @@ static_assert(RTL8365MB_NUM_IPMS == IEEE8021Q_TT_MAX); #define RTL8365MB_QOS_WEIGHT_UNTRUSTED 0 #define RTL8365MB_QOS_WEIGHT_PORT 1 +/* apptrust selectors this driver supports, in descending precedence. Each + * entry binds a dcbnl selector to the priority-decision source it enables, so + * this ordered table is the one place the fixed precedence lives. + */ +static const struct rtl8365mb_apptrust_map { + u8 sel; /* dcbnl apptrust selector */ + u8 src; /* priority-decision source it enables */ +} rtl8365mb_apptrust_map[] = { + { DCB_APP_SEL_PCP, RTL8365MB_QOS_PRIDEC_1Q }, + { IEEE_8021QAZ_APP_SEL_DSCP, RTL8365MB_QOS_PRIDEC_DSCP }, +};
[Severity: Low] Is DSCP trust usable yet at this point in the series? rtl8365mb_port_set_apptrust() accepts IEEE_8021QAZ_APP_SEL_DSCP. It writes a DSCP weight of 2 into the trusted table, which outranks the port default weight of 1. However, rtl8365mb_dcb_init() only programs the queue map, the PCP remap and the decision weights. The DSCP-to-internal-priority table keeps whatever the chip reset or the bootloader left there. rtl8365mb_switch_ops also has no port_get/add/del_dscp_prio ops yet. After "dcb apptrust set dev swpN order dscp", wouldn't IP traffic on that port be classified by a DSCP table that was never programmed, and that dcbnl has no way to change? The next patch in the series, "net: dsa: realtek: rtl8365mb: offload DCB DSCP-to-priority", seems to fix this. It adds rtl8365mb_qos_setup_dscp() to rtl8365mb_dcb_init(), which seeds all 64 entries from ietf_dscp_to_ieee8021q_tt(). It also adds the dscp_prio ops and sets ds->dscp_prio_mapping_is_global. So the problem only shows up when a bisect stops exactly at this commit. Could the patches be reordered, or could DSCP be left out of rtl8365mb_apptrust_map[] until the DSCP map is programmed? [ ... ]
quoted hunk ↗ jump to hunk
@@ -285,3 +323,158 @@ int rtl8365mb_port_set_default_prio(struct dsa_switch *ds, int port, u8 prio)
[ ... ]
quoted hunk ↗ jump to hunk
+int rtl8365mb_port_set_apptrust(struct dsa_switch *ds, int port, const u8 *sel, + int nsel) +{
[ ... ]
quoted hunk ↗ jump to hunk
+ /* The trusted table is a single switch-wide resource. If another port + * already uses it, this request must trust the same selectors. + */ + if (idx & ~BIT(port)) {
[ ... ]
quoted hunk ↗ jump to hunk
+ } + + for (i = 0; i < ARRAY_SIZE(rtl8365mb_apptrust_map); i++) { + u8 weight = trust[i] ? rtl8365mb_apptrust_weight(i) : + RTL8365MB_QOS_WEIGHT_UNTRUSTED; + + ret = rtl8365mb_qos_set_pridec(priv, + RTL8365MB_QOS_PRIDEC_TABLE_TRUSTED, + rtl8365mb_apptrust_map[i].src, + weight); + if (ret) + return ret; + }
[Severity: Low]
What happens to a port that is already on the trusted table if the
second register write in this loop fails?
Say port P is the only user of the trusted table. The idx & ~BIT(port)
check is then skipped, and the table is rewritten in place while P's
bit in RTL8365MB_QOS_PRIDEC_IDX_REG is still set.
The loop does two separate read-modify-writes on the same register:
1Q weight (source 3) -> bits 15:8 of 0x0886
DSCP weight (source 2) -> bits 7:0 of 0x0886
Suppose the 1Q write succeeds and the DSCP write fails, for example
with -ETIMEDOUT from realtek_smi_wait_for_ack(). The function then
returns without restoring the old 1Q weight. Neither
dsa_user_dcbnl_set_apptrust() nor dcbnl_ieee_set() rolls this back.
The port would then end up in a state that is neither the old profile
nor the new one:
DSCP-only -> PCP-only: the port trusts both PCP and DSCP
PCP-only -> DSCP-only: the port is on the trusted table but trusts
neither
Both weights live in the same register. Could they be written with a
single combined-mask update, so a failure can't leave the table half
written?
--
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261002115850.962676-1-o.rempel%40pengutronix.de