mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH v16 net-next 0/2] octeontx2: mqprio bandwidth offload for NIX TX schedulers
@ 2026-09-18  1:59 Ratheesh Kannoth
  2026-09-18  1:59 ` [PATCH v16 net-next 1/2] octeontx2: use atomic bitops for PF/VF and rep flags Ratheesh Kannoth
  2026-09-18  1:59 ` [PATCH v16 net-next 2/2] octeontx2-pf: add mqprio bandwidth offload for NIX TX schedulers Ratheesh Kannoth
  0 siblings, 2 replies; 5+ messages in thread
From: Ratheesh Kannoth @ 2026-09-18  1:59 UTC (permalink / raw)
  To: bpf, linux-kernel, netdev
  Cc: andrew+netdev, ast, daniel, davem, edumazet, hawk,
	john.fastabend, kuba, pabeni, sdf, sgoutham, Ratheesh Kannoth

This series adds hardware offload for channel-mode mqprio with
TC_MQPRIO_SHAPER_BW_RATE on Marvell octeontx2/cn10k PF and VF RVU
netdevices. Each non-QoS transmit queue is shaped by programming MDQ CIR/PIR
on the NIX TX scheduler. When bandwidth offload is active, the driver
allocates one SMQ per queue, parents every MDQ under TL4[0], and maps each
traffic class min/max rate to the queue(s) in that class.

The NIX TX scheduler hierarchy cannot be reprogrammed live today, so
mqprio add, replace, delete, and failed-replace rollback rebuild it by
bouncing the netdev through ndo_stop()/ndo_open(). That intentionally
drops in-flight traffic on each change. otx2_mqprio_restart_netdev() clears
__LINK_STATE_START before ndo_stop() and does not call
dev_deactivate()/dev_activate(); carrier and TX queues are restored after
ndo_open() via the normal link-event path when the link is up. Cache the
active rates and restore MDQ shapers from otx2_mqprio_up() during ndo_open();
fail closed if restoration fails, leaving ndo_open() unsuccessful and the
interface administratively down.

Track mqprio configuration in mq_offload_snap snapshots (TC layout and
rates). On tc qdisc replace, stage the new configuration while keeping
the previous snapshot for rollback: failed setup restores the old
snapshot via netdev restart when the interface is running, successful
graft is recorded through TC_ROOT_GRAFT, and teardown of the replaced
qdisc instance commits the staged snapshot without tearing down the live
offload.

Patch 1 converts PF/VF and representor flag access to atomic bitops.
Patch 2 depends on it for safe OTX2_FLAG_INTF_DOWN and OTX2_FLAG_PORT_UP
updates on asynchronous mbox paths and during the mqprio netdev bounce.

The driver rejects offload unless the interface is running and the device
advertises CIR+PIR support. PF and VF RVU netdevices share the same TC
offload path via ndo_setup_tc / otx2_open(); SDP representors are not
supported. Per-TC rates are rejected when a traffic class spans more than
one queue. Concurrent PFC, XDP, SDP rep, or HTB use is blocked, and ethtool
channel count changes are blocked while mqprio bandwidth offload is active.

Ratheesh Kannoth (2):
  octeontx2: use atomic bitops for PF/VF and rep flags
  octeontx2-pf: add mqprio bandwidth offload for NIX TX schedulers

 .../ethernet/marvell/octeontx2/nic/cn10k_ipsec.c   |   8 +-
 .../ethernet/marvell/octeontx2/nic/otx2_common.c   | 154 +++-
 .../ethernet/marvell/octeontx2/nic/otx2_common.h   | 107 ++-
 .../ethernet/marvell/octeontx2/nic/otx2_dcbnl.c    |   6 +
 .../ethernet/marvell/octeontx2/nic/otx2_devlink.c  |   2 +-
 .../ethernet/marvell/octeontx2/nic/otx2_ethtool.c  |  29 +-
 .../ethernet/marvell/octeontx2/nic/otx2_flows.c    |  34 +-
 .../net/ethernet/marvell/octeontx2/nic/otx2_pf.c   |  95 ++-
 .../net/ethernet/marvell/octeontx2/nic/otx2_tc.c   | 838 ++++++++++++++++++++-
 .../net/ethernet/marvell/octeontx2/nic/otx2_txrx.c |  16 +-
 .../net/ethernet/marvell/octeontx2/nic/otx2_vf.c   |  10 +-
 .../net/ethernet/marvell/octeontx2/nic/otx2_xsk.c  |   4 +-
 drivers/net/ethernet/marvell/octeontx2/nic/qos.c   |  11 +
 .../net/ethernet/marvell/octeontx2/nic/qos_sq.c    |   4 +-
 drivers/net/ethernet/marvell/octeontx2/nic/rep.c   |  32 +-
 drivers/net/ethernet/marvell/octeontx2/nic/rep.h   |   3 +-
 16 files changed, 1203 insertions(+), 150 deletions(-)

---

v15 -> v16: Addressed sashiko comments on v15 and aligned documentation with code.
- Sync representor flags through OTX2_FLAG_MAX in otx2_sync_flags_from_rep()
  instead of hard-coding OTX2_REP_VF_INITIALIZED as the loop bound.
- Drop the rvu_nix.c is_valid_txschq() ratelimited error print from the mqprio
  patch; remove the misplaced atomic-bitops and AF-debug paragraphs from the
  mqprio commit message (they belong to patch 1 or are out of scope).
- Extend mq_offload_snap to record prio_tc_map[] and mqprio rate flags; restore
  the full netdev TC layout (num_tc, queue ranges, and priority map) via
  otx2_mqprio_apply_snap_netdev() on rollback paths.
- Defer netdev TC restore on failed replace (otx2_mqprio_netdev_tc_work) so
  rollback survives mqprio_destroy() clearing dev->num_tc after setup errors
  once the core unwinds the failed qdisc instance.
- Stop calling dev_deactivate()/dev_activate() from otx2_mqprio_restart_netdev();
  bounce the interface with ndo_stop()/ndo_open() only and restore carrier
  through the normal link-event path after ndo_open(), avoiding qdisc
  reentrancy during tc replace graft.
- Preserve netdev TC mappings when tearing down an offloaded instance that is
  replaced by a software mqprio graft (otx2_mqprio_keep_netdev_tc()) instead
  of always calling netdev_set_num_tc(0) and breaking the live replacement.
- Return an error from otx2_mqprio_down() when clearing hardware shapers fails
  and keep offload software state, instead of v15's behaviour of clearing
  rate_limit while stale MDQ limits may remain programmed.
- Rebuild the TX scheduler via netdev restart in otx2_mqprio_restore_old() on
  a running interface after failed-replace rollback so partially applied MDQ
  shapers are not left running with mismatched software state.
- Clear txschq_cnt[] in otx2_txschq_stop() after freeing scheduler nodes so
  post-stop shaper mailbox operations do not consult stale counts.
- Document fail-closed ndo_open() when otx2_mqprio_up() cannot restore shapers,
  and that PF/VF RVU netdevices share the ndo_setup_tc / otx2_open() offload
  path (SDP representors remain unsupported); downgrade the mqprio restart
  notice to netdev_dbg().
	https://lore.kernel.org/netdev/20260911105521.689565-1-rkannoth@marvell.com/

v14 -> v15: Addressed sashiko comments.
- Split atomic PF/VF and representor flag access into a preparatory patch
  so mqprio netdev-restart and mbox paths can update OTX2_FLAG_INTF_DOWN
  and OTX2_FLAG_PORT_UP without data races on the shared flags word.
- Clear mqprio software state when hardware shaper teardown fails, warn,
  and still bounce the netdev on delete so offload does not remain stuck
  active after a mailbox error.
	https://lore.kernel.org/netdev/20260904031553.3196916-1-rkannoth@marvell.com/

v13 -> v14: Addressed sashiko comments.
- Use atomic set_bit()/clear_bit() for OTX2_FLAG_INTF_DOWN and
  OTX2_FLAG_PORT_UP updates on netdev-restart and mbox paths.
- Block concurrent mqprio bandwidth offload and HTB shaping.
- Fail ndo_open() if otx2_mqprio_up() cannot restore MDQ shapers.
- Rebuild the TX scheduler via netdev restart in otx2_mqprio_restore_old()
  when rolling back a failed replace on a running interface.
- Return an error from otx2_mqprio_down() if clearing hardware shapers
  fails instead of clearing software state anyway.
	https://lore.kernel.org/netdev/20260904031553.3196916-1-rkannoth@marvell.com/

v12 -> v13: Addressed sashiko comments.
	https://sashiko.dev/#/patchset/20260903023324.3078284-1-rkannoth%40marvell.com

v11 -> v12: Addressed sashiko comments.
	https://sashiko.dev/#/patchset/20260902015500.2985371-1-rkannoth%40marvell.com
v10 -> v11: Addressed sashiko comments.
	https://sashiko.dev/#/patchset/20260831131014.2639581-1-rkannoth%40marvell.com

v9 -> v10: Addressed sashiko/jacub comments.
	https://sashiko.dev/#/message/20260817032747.1765883-1-rkannoth%40marvell.com

v8 -> v9: Addressed Sashiko comments
	https://lore.kernel.org/netdev/aoJ6FhtWue0FHDQV@rkannoth-OptiPlex-7090/
v7 -> v8: Addressed Sashiko comments
	https://sashiko.dev/#/patchset/20260811085050.3212280-1-rkannoth%40marvell.com
v6 -> v7: Addressed Sashiko comments
	https://sashiko.dev/#/message/20260810034738.1786029-1-rkannoth%40marvell.com
v5 -> v6: Addressed Sashiko comments
	https://lore.kernel.org/netdev/20260806095434.1144397-1-rkannoth@marvell.com/
v4 -> v5: Addressed sashiko comments
	https://sashiko.dev/#/patchset/20260803042724.3380209-1-rkannoth%40marvell.com
v3 -> v4: Addressed sashiko comments
	https://lore.kernel.org/netdev/20260729105139.2302908-1-rkannoth@marvell.com/
v2 -> v3: Addressed sashiko comments
	https://lore.kernel.org/netdev/amnYX866mYx02cBe@rkannoth-OptiPlex-7090/T/#m67310cbec48b21c7720858ab3a1ea083a0f8dc10
v1 -> v2: Addressed sashiko comments
	https://lore.kernel.org/netdev/20260724075010.2665758-1-rkannoth@marvell.com/

--
2.43.0

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH v16 net-next 1/2] octeontx2: use atomic bitops for PF/VF and rep flags
  2026-09-18  1:59 [PATCH v16 net-next 0/2] octeontx2: mqprio bandwidth offload for NIX TX schedulers Ratheesh Kannoth
@ 2026-09-18  1:59 ` Ratheesh Kannoth
  2026-09-22  2:19   ` netdev-bot+sashiko
  2026-09-18  1:59 ` [PATCH v16 net-next 2/2] octeontx2-pf: add mqprio bandwidth offload for NIX TX schedulers Ratheesh Kannoth
  1 sibling, 1 reply; 5+ messages in thread
From: Ratheesh Kannoth @ 2026-09-18  1:59 UTC (permalink / raw)
  To: bpf, linux-kernel, netdev
  Cc: andrew+netdev, ast, daniel, davem, edumazet, hawk,
	john.fastabend, kuba, pabeni, sdf, sgoutham, Ratheesh Kannoth

Replace non-atomic u64 flag read-modify-write with unsigned long
bitmaps and set_bit/clear_bit/test_bit access across the NIC driver.

Add otx2_set_flag(), otx2_clear_flag() and otx2_test_flag() helpers
for struct otx2_nic, use bitops directly on rep_dev->flags, and sync
representor state to the PF mailbox context via otx2_sync_flags_from_rep().
Define representor VF initialization as OTX2_REP_VF_INITIALIZED (bit 21)
in the shared flag namespace.

Signed-off-by: Ratheesh Kannoth <rkannoth@marvell.com>
---
 .../marvell/octeontx2/nic/cn10k_ipsec.c       |  8 +-
 .../marvell/octeontx2/nic/otx2_common.c       |  8 +-
 .../marvell/octeontx2/nic/otx2_common.h       | 77 ++++++++++++------
 .../marvell/octeontx2/nic/otx2_devlink.c      |  2 +-
 .../marvell/octeontx2/nic/otx2_ethtool.c      | 21 +++--
 .../marvell/octeontx2/nic/otx2_flows.c        | 34 ++++----
 .../ethernet/marvell/octeontx2/nic/otx2_pf.c  | 78 +++++++++----------
 .../ethernet/marvell/octeontx2/nic/otx2_tc.c  | 30 +++----
 .../marvell/octeontx2/nic/otx2_txrx.c         | 16 ++--
 .../ethernet/marvell/octeontx2/nic/otx2_vf.c  | 10 +--
 .../ethernet/marvell/octeontx2/nic/otx2_xsk.c |  4 +-
 .../ethernet/marvell/octeontx2/nic/qos_sq.c   |  4 +-
 .../net/ethernet/marvell/octeontx2/nic/rep.c  | 32 ++++----
 .../net/ethernet/marvell/octeontx2/nic/rep.h  |  3 +-
 14 files changed, 178 insertions(+), 149 deletions(-)

diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/cn10k_ipsec.c b/drivers/net/ethernet/marvell/octeontx2/nic/cn10k_ipsec.c
index 77543d472345..50ec4542c418 100644
--- a/drivers/net/ethernet/marvell/octeontx2/nic/cn10k_ipsec.c
+++ b/drivers/net/ethernet/marvell/octeontx2/nic/cn10k_ipsec.c
@@ -334,7 +334,7 @@ static int cn10k_outb_cpt_init(struct net_device *netdev)
 						CN10K_CPT_LF_NQX(0));
 
 	/* Set ipsec offload enabled for this device */
-	pf->flags |= OTX2_FLAG_IPSEC_OFFLOAD_ENABLED;
+	otx2_set_flag(pf, OTX2_FLAG_IPSEC_OFFLOAD_ENABLED);
 
 	cn10k_cpt_device_set_available(pf);
 	return 0;
@@ -356,7 +356,7 @@ static int cn10k_outb_cpt_clean(struct otx2_nic *pf)
 	}
 
 	/* Set ipsec offload disabled for this device */
-	pf->flags &= ~OTX2_FLAG_IPSEC_OFFLOAD_ENABLED;
+	otx2_clear_flag(pf, OTX2_FLAG_IPSEC_OFFLOAD_ENABLED);
 
 	/* Disable CPTLF Instruction Queue (IQ) */
 	cn10k_outb_cptlf_iq_disable(pf);
@@ -820,7 +820,7 @@ void cn10k_ipsec_clean(struct otx2_nic *pf)
 	if (!is_dev_support_ipsec_offload(pf->pdev))
 		return;
 
-	if (!(pf->flags & OTX2_FLAG_IPSEC_OFFLOAD_ENABLED))
+	if (!otx2_test_flag(pf, OTX2_FLAG_IPSEC_OFFLOAD_ENABLED))
 		return;
 
 	if (pf->ipsec.sa_workq) {
@@ -945,7 +945,7 @@ bool cn10k_ipsec_transmit(struct otx2_nic *pf, struct netdev_queue *txq,
 	u16 dlen;
 
 	/* Check for IPSEC offload enabled */
-	if (!(pf->flags & OTX2_FLAG_IPSEC_OFFLOAD_ENABLED))
+	if (!otx2_test_flag(pf, OTX2_FLAG_IPSEC_OFFLOAD_ENABLED))
 		goto drop;
 
 	sp = skb_sec_path(skb);
diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.c b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.c
index 175992188c18..b421cb75e44b 100644
--- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.c
+++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.c
@@ -220,10 +220,10 @@ int otx2_set_mac_address(struct net_device *netdev, void *p)
 		eth_hw_addr_set(netdev, addr->sa_data);
 		/* update dmac field in vlan offload rule */
 		if (netif_running(netdev) &&
-		    pfvf->flags & OTX2_FLAG_RX_VLAN_SUPPORT)
+		    otx2_test_flag(pfvf, OTX2_FLAG_RX_VLAN_SUPPORT))
 			otx2_install_rxvlan_offload_flow(pfvf);
 		/* update dmac address in ntuple and DMAC filter list */
-		if (pfvf->flags & OTX2_FLAG_DMACFLTR_SUPPORT)
+		if (otx2_test_flag(pfvf, OTX2_FLAG_DMACFLTR_SUPPORT))
 			otx2_dmacflt_update_pfmac_flow(pfvf);
 	} else {
 		return -EPERM;
@@ -275,8 +275,8 @@ int otx2_config_pause_frm(struct otx2_nic *pfvf)
 		goto unlock;
 	}
 
-	req->rx_pause = !!(pfvf->flags & OTX2_FLAG_RX_PAUSE_ENABLED);
-	req->tx_pause = !!(pfvf->flags & OTX2_FLAG_TX_PAUSE_ENABLED);
+	req->rx_pause = otx2_test_flag(pfvf, OTX2_FLAG_RX_PAUSE_ENABLED);
+	req->tx_pause = otx2_test_flag(pfvf, OTX2_FLAG_TX_PAUSE_ENABLED);
 	req->set = 1;
 
 	err = otx2_sync_mbox_msg(&pfvf->mbox);
diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.h b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.h
index 5850bc1870a1..90cf302bbe6d 100644
--- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.h
+++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.h
@@ -482,6 +482,32 @@ struct pf_irq_data {
 	int mdevs;
 };
 
+enum otx2_flag_bits {
+	OTX2_FLAG_RX_TSTAMP_ENABLED,
+	OTX2_FLAG_TX_TSTAMP_ENABLED,
+	OTX2_FLAG_INTF_DOWN,
+	OTX2_FLAG_MCAM_ENTRIES_ALLOC,
+	OTX2_FLAG_NTUPLE_SUPPORT,
+	OTX2_FLAG_UCAST_FLTR_SUPPORT,
+	OTX2_FLAG_RX_VLAN_SUPPORT,
+	OTX2_FLAG_VF_VLAN_SUPPORT,
+	OTX2_FLAG_PF_SHUTDOWN,
+	OTX2_FLAG_RX_PAUSE_ENABLED,
+	OTX2_FLAG_TX_PAUSE_ENABLED,
+	OTX2_FLAG_TC_FLOWER_SUPPORT,
+	OTX2_FLAG_TC_MATCHALL_EGRESS_ENABLED,
+	OTX2_FLAG_TC_MATCHALL_INGRESS_ENABLED,
+	OTX2_FLAG_DMACFLTR_SUPPORT,
+	OTX2_FLAG_PTP_ONESTEP_SYNC,
+	OTX2_FLAG_ADPTV_INT_COAL_ENABLED,
+	OTX2_FLAG_TC_MARK_ENABLED,
+	OTX2_FLAG_REP_MODE_ENABLED,
+	OTX2_FLAG_PORT_UP,
+	OTX2_FLAG_IPSEC_OFFLOAD_ENABLED,
+	OTX2_FLAG_REP_VF_INITIALIZED,
+	OTX2_FLAG_MAX,
+};
+
 struct otx2_nic {
 	void __iomem		*reg_base;
 	struct net_device	*netdev;
@@ -490,28 +516,7 @@ struct otx2_nic {
 	u16			tx_max_pktlen;
 	u16			rbsize; /* Receive buffer size */
 
-#define OTX2_FLAG_RX_TSTAMP_ENABLED		BIT_ULL(0)
-#define OTX2_FLAG_TX_TSTAMP_ENABLED		BIT_ULL(1)
-#define OTX2_FLAG_INTF_DOWN			BIT_ULL(2)
-#define OTX2_FLAG_MCAM_ENTRIES_ALLOC		BIT_ULL(3)
-#define OTX2_FLAG_NTUPLE_SUPPORT		BIT_ULL(4)
-#define OTX2_FLAG_UCAST_FLTR_SUPPORT		BIT_ULL(5)
-#define OTX2_FLAG_RX_VLAN_SUPPORT		BIT_ULL(6)
-#define OTX2_FLAG_VF_VLAN_SUPPORT		BIT_ULL(7)
-#define OTX2_FLAG_PF_SHUTDOWN			BIT_ULL(8)
-#define OTX2_FLAG_RX_PAUSE_ENABLED		BIT_ULL(9)
-#define OTX2_FLAG_TX_PAUSE_ENABLED		BIT_ULL(10)
-#define OTX2_FLAG_TC_FLOWER_SUPPORT		BIT_ULL(11)
-#define OTX2_FLAG_TC_MATCHALL_EGRESS_ENABLED	BIT_ULL(12)
-#define OTX2_FLAG_TC_MATCHALL_INGRESS_ENABLED	BIT_ULL(13)
-#define OTX2_FLAG_DMACFLTR_SUPPORT		BIT_ULL(14)
-#define OTX2_FLAG_PTP_ONESTEP_SYNC		BIT_ULL(15)
-#define OTX2_FLAG_ADPTV_INT_COAL_ENABLED BIT_ULL(16)
-#define OTX2_FLAG_TC_MARK_ENABLED		BIT_ULL(17)
-#define OTX2_FLAG_REP_MODE_ENABLED		 BIT_ULL(18)
-#define OTX2_FLAG_PORT_UP			BIT_ULL(19)
-#define OTX2_FLAG_IPSEC_OFFLOAD_ENABLED		BIT_ULL(20)
-	u64			flags;
+	unsigned long		flags;
 	u64			*cq_op_addr;
 
 	struct bpf_prog		*xdp_prog;
@@ -593,6 +598,34 @@ struct otx2_nic {
 	unsigned long		*af_xdp_zc_qidx;
 };
 
+static inline void otx2_set_flag(struct otx2_nic *nic, unsigned int flag)
+{
+	set_bit(flag, &nic->flags);
+}
+
+static inline void otx2_clear_flag(struct otx2_nic *nic, unsigned int flag)
+{
+	clear_bit(flag, &nic->flags);
+}
+
+static inline bool otx2_test_flag(struct otx2_nic *nic, unsigned int flag)
+{
+	return test_bit(flag, &nic->flags);
+}
+
+static inline void otx2_sync_flags_from_rep(struct otx2_nic *dst,
+					    unsigned long *src_flags)
+{
+	unsigned int flag;
+
+	for (flag = 0; flag < OTX2_FLAG_MAX; flag++) {
+		if (test_bit(flag, src_flags))
+			set_bit(flag, &dst->flags);
+		else
+			clear_bit(flag, &dst->flags);
+	}
+}
+
 static inline bool is_otx2_lbkvf(struct pci_dev *pdev)
 {
 	return (pdev->device == PCI_DEVID_OCTEONTX2_RVU_AFVF) ||
diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_devlink.c b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_devlink.c
index 4a5ce0e67dda..863a5ced9a26 100644
--- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_devlink.c
+++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_devlink.c
@@ -104,7 +104,7 @@ static int otx2_dl_ucast_flt_cnt_validate(struct devlink *devlink, u32 id,
 	struct otx2_nic *pfvf = otx2_dl->pfvf;
 
 	/* Check for UNICAST filter support*/
-	if (!(pfvf->flags & OTX2_FLAG_UCAST_FLTR_SUPPORT)) {
+	if (!otx2_test_flag(pfvf, OTX2_FLAG_UCAST_FLTR_SUPPORT)) {
 		NL_SET_ERR_MSG_MOD(extack,
 				   "Unicast filter not enabled");
 		return -EINVAL;
diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_ethtool.c b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_ethtool.c
index a05dee0085a3..4fe473d9ea0d 100644
--- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_ethtool.c
+++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_ethtool.c
@@ -354,14 +354,14 @@ static int otx2_set_pauseparam(struct net_device *netdev,
 		return -EOPNOTSUPP;
 
 	if (pause->rx_pause)
-		pfvf->flags |= OTX2_FLAG_RX_PAUSE_ENABLED;
+		otx2_set_flag(pfvf, OTX2_FLAG_RX_PAUSE_ENABLED);
 	else
-		pfvf->flags &= ~OTX2_FLAG_RX_PAUSE_ENABLED;
+		otx2_clear_flag(pfvf, OTX2_FLAG_RX_PAUSE_ENABLED);
 
 	if (pause->tx_pause)
-		pfvf->flags |= OTX2_FLAG_TX_PAUSE_ENABLED;
+		otx2_set_flag(pfvf, OTX2_FLAG_TX_PAUSE_ENABLED);
 	else
-		pfvf->flags &= ~OTX2_FLAG_TX_PAUSE_ENABLED;
+		otx2_clear_flag(pfvf, OTX2_FLAG_TX_PAUSE_ENABLED);
 
 	return otx2_config_pause_frm(pfvf);
 }
@@ -470,8 +470,7 @@ static int otx2_get_coalesce(struct net_device *netdev,
 	cmd->rx_max_coalesced_frames = hw->cq_ecount_wait;
 	cmd->tx_coalesce_usecs = hw->cq_time_wait;
 	cmd->tx_max_coalesced_frames = hw->cq_ecount_wait;
-	if ((pfvf->flags & OTX2_FLAG_ADPTV_INT_COAL_ENABLED) ==
-			OTX2_FLAG_ADPTV_INT_COAL_ENABLED) {
+	if (otx2_test_flag(pfvf, OTX2_FLAG_ADPTV_INT_COAL_ENABLED)) {
 		cmd->use_adaptive_rx_coalesce = 1;
 		cmd->use_adaptive_tx_coalesce = 1;
 	} else {
@@ -502,15 +501,14 @@ static int otx2_set_coalesce(struct net_device *netdev,
 	}
 
 	/* Check and update coalesce status */
-	if ((pfvf->flags & OTX2_FLAG_ADPTV_INT_COAL_ENABLED) ==
-			OTX2_FLAG_ADPTV_INT_COAL_ENABLED) {
+	if (otx2_test_flag(pfvf, OTX2_FLAG_ADPTV_INT_COAL_ENABLED)) {
 		priv_coalesce_status = 1;
 		if (!ec->use_adaptive_rx_coalesce)
-			pfvf->flags &= ~OTX2_FLAG_ADPTV_INT_COAL_ENABLED;
+			otx2_clear_flag(pfvf, OTX2_FLAG_ADPTV_INT_COAL_ENABLED);
 	} else {
 		priv_coalesce_status = 0;
 		if (ec->use_adaptive_rx_coalesce)
-			pfvf->flags |= OTX2_FLAG_ADPTV_INT_COAL_ENABLED;
+			otx2_set_flag(pfvf, OTX2_FLAG_ADPTV_INT_COAL_ENABLED);
 	}
 
 	/* 'cq_time_wait' is 8bit and is in multiple of 100ns,
@@ -556,8 +554,7 @@ static int otx2_set_coalesce(struct net_device *netdev,
 	 * 'on' to 'off'.
 	 */
 	if (priv_coalesce_status &&
-	    ((pfvf->flags & OTX2_FLAG_ADPTV_INT_COAL_ENABLED) !=
-	     OTX2_FLAG_ADPTV_INT_COAL_ENABLED)) {
+	    (!otx2_test_flag(pfvf, OTX2_FLAG_ADPTV_INT_COAL_ENABLED))) {
 		hw->cq_time_wait = CQ_TIMER_THRESH_DEFAULT;
 		hw->cq_ecount_wait = CQ_CQE_THRESH_DEFAULT;
 	}
diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_flows.c b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_flows.c
index 99d78fc5a2c4..b8ff49f0f6e3 100644
--- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_flows.c
+++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_flows.c
@@ -270,9 +270,9 @@ int otx2_alloc_mcam_entries(struct otx2_nic *pfvf, u16 count)
 	flow_cfg->max_flows = allocated;
 
 	if (allocated) {
-		pfvf->flags |= OTX2_FLAG_MCAM_ENTRIES_ALLOC;
-		pfvf->flags |= OTX2_FLAG_NTUPLE_SUPPORT;
-		pfvf->flags |= OTX2_FLAG_TC_FLOWER_SUPPORT;
+		otx2_set_flag(pfvf, OTX2_FLAG_MCAM_ENTRIES_ALLOC);
+		otx2_set_flag(pfvf, OTX2_FLAG_NTUPLE_SUPPORT);
+		otx2_set_flag(pfvf, OTX2_FLAG_TC_FLOWER_SUPPORT);
 	}
 
 	if (allocated != count)
@@ -376,7 +376,7 @@ int otx2_mcam_entry_init(struct otx2_nic *pfvf)
 	flow_cfg->unicast_offset = vf_vlan_max_flows;
 	flow_cfg->rx_vlan_offset = flow_cfg->unicast_offset +
 					flow_cfg->ucast_flt_cnt;
-	pfvf->flags |= OTX2_FLAG_UCAST_FLTR_SUPPORT;
+	otx2_set_flag(pfvf, OTX2_FLAG_UCAST_FLTR_SUPPORT);
 
 	/* Check if NPC_DMAC field is supported
 	 * by the mkex profile before setting VLAN support flag.
@@ -401,11 +401,11 @@ int otx2_mcam_entry_init(struct otx2_nic *pfvf)
 	}
 
 	if (frsp->enable) {
-		pfvf->flags |= OTX2_FLAG_RX_VLAN_SUPPORT;
-		pfvf->flags |= OTX2_FLAG_VF_VLAN_SUPPORT;
+		otx2_set_flag(pfvf, OTX2_FLAG_RX_VLAN_SUPPORT);
+		otx2_set_flag(pfvf, OTX2_FLAG_VF_VLAN_SUPPORT);
 	}
 
-	pfvf->flags |= OTX2_FLAG_MCAM_ENTRIES_ALLOC;
+	otx2_set_flag(pfvf, OTX2_FLAG_MCAM_ENTRIES_ALLOC);
 	mutex_unlock(&pfvf->mbox.lock);
 
 	/* Allocate entries for Ntuple filters */
@@ -415,7 +415,7 @@ int otx2_mcam_entry_init(struct otx2_nic *pfvf)
 		return 0;
 	}
 
-	pfvf->flags |= OTX2_FLAG_TC_FLOWER_SUPPORT;
+	otx2_set_flag(pfvf, OTX2_FLAG_TC_FLOWER_SUPPORT);
 
 	refcount_set(&flow_cfg->mark_flows, 1);
 	return 0;
@@ -479,7 +479,7 @@ int otx2_mcam_flow_init(struct otx2_nic *pf)
 		return err;
 
 	/* Check if MCAM entries are allocate or not */
-	if (!(pf->flags & OTX2_FLAG_UCAST_FLTR_SUPPORT))
+	if (!otx2_test_flag(pf, OTX2_FLAG_UCAST_FLTR_SUPPORT))
 		return 0;
 
 	pf->mac_table = devm_kzalloc(pf->dev, sizeof(struct otx2_mac_table)
@@ -501,7 +501,7 @@ int otx2_mcam_flow_init(struct otx2_nic *pf)
 	if (!pf->flow_cfg->bmap_to_dmacindex)
 		return -ENOMEM;
 
-	pf->flags |= OTX2_FLAG_DMACFLTR_SUPPORT;
+	otx2_set_flag(pf, OTX2_FLAG_DMACFLTR_SUPPORT);
 
 	return 0;
 }
@@ -521,7 +521,7 @@ static int otx2_do_add_macfilter(struct otx2_nic *pf, const u8 *mac)
 	struct npc_install_flow_req *req;
 	int err, i;
 
-	if (!(pf->flags & OTX2_FLAG_UCAST_FLTR_SUPPORT))
+	if (!otx2_test_flag(pf, OTX2_FLAG_UCAST_FLTR_SUPPORT))
 		return -ENOMEM;
 
 	/* dont have free mcam entries or uc list is greater than alloted */
@@ -1167,7 +1167,7 @@ static int otx2_is_flow_rule_dmacfilter(struct otx2_nic *pfvf,
 	u64 ring_cookie = fsp->ring_cookie;
 	u32 flow_type;
 
-	if (!(pfvf->flags & OTX2_FLAG_DMACFLTR_SUPPORT))
+	if (!otx2_test_flag(pfvf, OTX2_FLAG_DMACFLTR_SUPPORT))
 		return false;
 
 	flow_type = fsp->flow_type & ~(FLOW_EXT | FLOW_MAC_EXT | FLOW_RSS);
@@ -1364,7 +1364,7 @@ int otx2_add_flow(struct otx2_nic *pfvf, struct ethtool_rxnfc *nfc)
 	}
 
 	ring = ethtool_get_flow_spec_ring(fsp->ring_cookie);
-	if (!(pfvf->flags & OTX2_FLAG_NTUPLE_SUPPORT))
+	if (!otx2_test_flag(pfvf, OTX2_FLAG_NTUPLE_SUPPORT))
 		return -ENOMEM;
 
 	/* Number of queues on a VF can be greater or less than
@@ -1596,7 +1596,7 @@ int otx2_destroy_ntuple_flows(struct otx2_nic *pfvf)
 	struct otx2_flow *iter, *tmp;
 	int err;
 
-	if (!(pfvf->flags & OTX2_FLAG_NTUPLE_SUPPORT))
+	if (!otx2_test_flag(pfvf, OTX2_FLAG_NTUPLE_SUPPORT))
 		return 0;
 
 	if (!flow_cfg->max_flows)
@@ -1629,7 +1629,7 @@ int otx2_destroy_mcam_flows(struct otx2_nic *pfvf)
 	struct otx2_flow *iter, *tmp;
 	int err;
 
-	if (!(pfvf->flags & OTX2_FLAG_MCAM_ENTRIES_ALLOC))
+	if (!otx2_test_flag(pfvf, OTX2_FLAG_MCAM_ENTRIES_ALLOC))
 		return 0;
 
 	/* remove all flows */
@@ -1658,7 +1658,7 @@ int otx2_destroy_mcam_flows(struct otx2_nic *pfvf)
 		return err;
 	}
 
-	pfvf->flags &= ~OTX2_FLAG_MCAM_ENTRIES_ALLOC;
+	otx2_clear_flag(pfvf, OTX2_FLAG_MCAM_ENTRIES_ALLOC);
 	flow_cfg->max_flows = 0;
 	mutex_unlock(&pfvf->mbox.lock);
 
@@ -1721,7 +1721,7 @@ int otx2_enable_rxvlan(struct otx2_nic *pf, bool enable)
 	int err;
 
 	/* Dont have enough mcam entries */
-	if (!(pf->flags & OTX2_FLAG_RX_VLAN_SUPPORT))
+	if (!otx2_test_flag(pf, OTX2_FLAG_RX_VLAN_SUPPORT))
 		return -ENOMEM;
 
 	if (enable) {
diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c
index c0e2100de1d9..32582b6347ea 100644
--- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c
+++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c
@@ -879,7 +879,7 @@ static void otx2_handle_link_event(struct otx2_nic *pf)
 	struct cgx_link_user_info *linfo = &pf->linfo;
 	struct net_device *netdev = pf->netdev;
 
-	if (pf->flags & OTX2_FLAG_PORT_UP)
+	if (otx2_test_flag(pf, OTX2_FLAG_PORT_UP))
 		return;
 
 	pr_info("%s NIC Link is %s %d Mbps %s duplex\n", netdev->name,
@@ -907,11 +907,11 @@ static int otx2_mbox_up_handler_rep_event_up_notify(struct otx2_nic *pf,
 
 	if (info->event == RVU_EVENT_PORT_STATE) {
 		if (info->evt_data.port_state) {
-			pf->flags |= OTX2_FLAG_PORT_UP;
+			otx2_set_flag(pf, OTX2_FLAG_PORT_UP);
 			netif_carrier_on(netdev);
 			netif_tx_start_all_queues(netdev);
 		} else {
-			pf->flags &= ~OTX2_FLAG_PORT_UP;
+			otx2_clear_flag(pf, OTX2_FLAG_PORT_UP);
 			netif_tx_stop_all_queues(netdev);
 			netif_carrier_off(netdev);
 		}
@@ -953,7 +953,7 @@ int otx2_mbox_up_handler_cgx_link_event(struct otx2_nic *pf,
 	}
 
 	/* interface has not been fully configured yet */
-	if (pf->flags & OTX2_FLAG_INTF_DOWN)
+	if (otx2_test_flag(pf, OTX2_FLAG_INTF_DOWN))
 		return 0;
 
 	otx2_handle_link_event(pf);
@@ -1828,7 +1828,7 @@ void otx2_free_hw_resources(struct otx2_nic *pf)
 	free_req = otx2_mbox_alloc_msg_nix_lf_free(mbox);
 	if (free_req) {
 		free_req->flags = NIX_LF_DISABLE_FLOWS | NIX_LF_DONT_FREE_DFT_IDXS;
-		if (!(pf->flags & OTX2_FLAG_PF_SHUTDOWN))
+		if (!otx2_test_flag(pf, OTX2_FLAG_PF_SHUTDOWN))
 			free_req->flags |= NIX_LF_DONT_FREE_TX_VTAG;
 		if (otx2_sync_mbox_msg(mbox))
 			dev_err(pf->dev, "%s failed to free nixlf\n", __func__);
@@ -2135,21 +2135,21 @@ int otx2_open(struct net_device *netdev)
 	}
 	otx2_write64(pf, NIX_LF_RAS_ENA_W1S, NIX_LF_RAS_MASK);
 
-	if (pf->flags & OTX2_FLAG_RX_VLAN_SUPPORT)
+	if (otx2_test_flag(pf, OTX2_FLAG_RX_VLAN_SUPPORT))
 		otx2_enable_rxvlan(pf, true);
 
 	/* When reinitializing enable time stamping if it is enabled before */
-	if (pf->flags & OTX2_FLAG_TX_TSTAMP_ENABLED) {
-		pf->flags &= ~OTX2_FLAG_TX_TSTAMP_ENABLED;
+	if (otx2_test_flag(pf, OTX2_FLAG_TX_TSTAMP_ENABLED)) {
+		otx2_clear_flag(pf, OTX2_FLAG_TX_TSTAMP_ENABLED);
 		otx2_config_hw_tx_tstamp(pf, true);
 	}
-	if (pf->flags & OTX2_FLAG_RX_TSTAMP_ENABLED) {
-		pf->flags &= ~OTX2_FLAG_RX_TSTAMP_ENABLED;
+	if (otx2_test_flag(pf, OTX2_FLAG_RX_TSTAMP_ENABLED)) {
+		otx2_clear_flag(pf, OTX2_FLAG_RX_TSTAMP_ENABLED);
 		otx2_config_hw_rx_tstamp(pf, true);
 	}
 
-	pf->flags &= ~OTX2_FLAG_INTF_DOWN;
-	pf->flags &= ~OTX2_FLAG_PORT_UP;
+	otx2_clear_flag(pf, OTX2_FLAG_INTF_DOWN);
+	otx2_clear_flag(pf, OTX2_FLAG_PORT_UP);
 	/* 'intf_down' may be checked on any cpu */
 	smp_wmb();
 
@@ -2161,7 +2161,7 @@ int otx2_open(struct net_device *netdev)
 		otx2_handle_link_event(pf);
 
 	/* Install DMAC Filters */
-	if (pf->flags & OTX2_FLAG_DMACFLTR_SUPPORT)
+	if (otx2_test_flag(pf, OTX2_FLAG_DMACFLTR_SUPPORT))
 		otx2_dmacflt_reinstall_flows(pf);
 
 	otx2_tc_apply_ingress_police_rules(pf);
@@ -2186,7 +2186,7 @@ int otx2_open(struct net_device *netdev)
 err_tx_stop_queues:
 	netif_tx_stop_all_queues(netdev);
 	netif_carrier_off(netdev);
-	pf->flags |= OTX2_FLAG_INTF_DOWN;
+	otx2_set_flag(pf, OTX2_FLAG_INTF_DOWN);
 	/* free NIXLF POISON irq */
 	vec = pci_irq_vector(pf->pdev,
 			     pf->hw.nix_msixoff + NIX_LF_POISON_VEC);
@@ -2220,13 +2220,13 @@ int otx2_stop(struct net_device *netdev)
 	int qidx, vec, wrk;
 
 	/* If the DOWN flag is set resources are already freed */
-	if (pf->flags & OTX2_FLAG_INTF_DOWN)
+	if (otx2_test_flag(pf, OTX2_FLAG_INTF_DOWN))
 		return 0;
 
 	netif_carrier_off(netdev);
 	netif_tx_stop_all_queues(netdev);
 
-	pf->flags |= OTX2_FLAG_INTF_DOWN;
+	otx2_set_flag(pf, OTX2_FLAG_INTF_DOWN);
 	/* 'intf_down' may be checked on any cpu */
 	smp_wmb();
 
@@ -2457,7 +2457,7 @@ static int otx2_config_hw_rx_tstamp(struct otx2_nic *pfvf, bool enable)
 	struct msg_req *req;
 	int err;
 
-	if (pfvf->flags & OTX2_FLAG_RX_TSTAMP_ENABLED && enable)
+	if (otx2_test_flag(pfvf, OTX2_FLAG_RX_TSTAMP_ENABLED) && enable)
 		return 0;
 
 	mutex_lock(&pfvf->mbox.lock);
@@ -2478,9 +2478,9 @@ static int otx2_config_hw_rx_tstamp(struct otx2_nic *pfvf, bool enable)
 
 	mutex_unlock(&pfvf->mbox.lock);
 	if (enable)
-		pfvf->flags |= OTX2_FLAG_RX_TSTAMP_ENABLED;
+		otx2_set_flag(pfvf, OTX2_FLAG_RX_TSTAMP_ENABLED);
 	else
-		pfvf->flags &= ~OTX2_FLAG_RX_TSTAMP_ENABLED;
+		otx2_clear_flag(pfvf, OTX2_FLAG_RX_TSTAMP_ENABLED);
 	return 0;
 }
 
@@ -2489,7 +2489,7 @@ static int otx2_config_hw_tx_tstamp(struct otx2_nic *pfvf, bool enable)
 	struct msg_req *req;
 	int err;
 
-	if (pfvf->flags & OTX2_FLAG_TX_TSTAMP_ENABLED && enable)
+	if (otx2_test_flag(pfvf, OTX2_FLAG_TX_TSTAMP_ENABLED) && enable)
 		return 0;
 
 	mutex_lock(&pfvf->mbox.lock);
@@ -2510,9 +2510,9 @@ static int otx2_config_hw_tx_tstamp(struct otx2_nic *pfvf, bool enable)
 
 	mutex_unlock(&pfvf->mbox.lock);
 	if (enable)
-		pfvf->flags |= OTX2_FLAG_TX_TSTAMP_ENABLED;
+		otx2_set_flag(pfvf, OTX2_FLAG_TX_TSTAMP_ENABLED);
 	else
-		pfvf->flags &= ~OTX2_FLAG_TX_TSTAMP_ENABLED;
+		otx2_clear_flag(pfvf, OTX2_FLAG_TX_TSTAMP_ENABLED);
 	return 0;
 }
 
@@ -2537,8 +2537,8 @@ int otx2_config_hwtstamp_set(struct net_device *netdev,
 
 	switch (config->tx_type) {
 	case HWTSTAMP_TX_OFF:
-		if (pfvf->flags & OTX2_FLAG_PTP_ONESTEP_SYNC)
-			pfvf->flags &= ~OTX2_FLAG_PTP_ONESTEP_SYNC;
+		if (otx2_test_flag(pfvf, OTX2_FLAG_PTP_ONESTEP_SYNC))
+			otx2_clear_flag(pfvf, OTX2_FLAG_PTP_ONESTEP_SYNC);
 
 		cancel_delayed_work(&pfvf->ptp->synctstamp_work);
 		otx2_config_hw_tx_tstamp(pfvf, false);
@@ -2549,7 +2549,7 @@ int otx2_config_hwtstamp_set(struct net_device *netdev,
 					   "One-step time stamping is not supported");
 			return -ERANGE;
 		}
-		pfvf->flags |= OTX2_FLAG_PTP_ONESTEP_SYNC;
+		otx2_set_flag(pfvf, OTX2_FLAG_PTP_ONESTEP_SYNC);
 		schedule_delayed_work(&pfvf->ptp->synctstamp_work,
 				      msecs_to_jiffies(500));
 		fallthrough;
@@ -2835,7 +2835,7 @@ static int otx2_set_vf_vlan(struct net_device *netdev, int vf, u16 vlan, u8 qos,
 	if (proto != htons(ETH_P_8021Q))
 		return -EPROTONOSUPPORT;
 
-	if (!(pf->flags & OTX2_FLAG_VF_VLAN_SUPPORT))
+	if (!otx2_test_flag(pf, OTX2_FLAG_VF_VLAN_SUPPORT))
 		return -EOPNOTSUPP;
 
 	return otx2_do_set_vf_vlan(pf, vf, vlan, qos, proto);
@@ -3086,7 +3086,7 @@ int otx2_realloc_msix_vectors(struct otx2_nic *pf)
 	 * interrupt range (QINT, CINT, GINT, ERR and POISON vectors).
 	 */
 	num_vec = hw->nix_msixoff;
-	if (pf->flags & OTX2_FLAG_REP_MODE_ENABLED)
+	if (otx2_test_flag(pf, OTX2_FLAG_REP_MODE_ENABLED))
 		num_vec += NIX_LF_CINT_VEC_START + hw->max_queues;
 	else
 		num_vec += NIX_LF_POISON_VEC + 1;
@@ -3273,7 +3273,7 @@ static int otx2_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 	pf->pdev = pdev;
 	pf->dev = dev;
 	pf->total_vfs = pci_sriov_get_totalvfs(pdev);
-	pf->flags |= OTX2_FLAG_INTF_DOWN;
+	otx2_set_flag(pf, OTX2_FLAG_INTF_DOWN);
 
 	hw = &pf->hw;
 	hw->pdev = pdev;
@@ -3328,23 +3328,23 @@ static int otx2_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 	if (err)
 		goto err_del_mcam_entries;
 
-	if (pf->flags & OTX2_FLAG_NTUPLE_SUPPORT)
+	if (otx2_test_flag(pf, OTX2_FLAG_NTUPLE_SUPPORT))
 		netdev->hw_features |= NETIF_F_NTUPLE;
 
-	if (pf->flags & OTX2_FLAG_UCAST_FLTR_SUPPORT)
+	if (otx2_test_flag(pf, OTX2_FLAG_UCAST_FLTR_SUPPORT))
 		netdev->priv_flags |= IFF_UNICAST_FLT;
 
 	/* Support TSO on tag interface */
 	netdev->vlan_features |= netdev->features;
 	netdev->hw_features  |= NETIF_F_HW_VLAN_CTAG_TX |
 				NETIF_F_HW_VLAN_STAG_TX;
-	if (pf->flags & OTX2_FLAG_RX_VLAN_SUPPORT)
+	if (otx2_test_flag(pf, OTX2_FLAG_RX_VLAN_SUPPORT))
 		netdev->hw_features |= NETIF_F_HW_VLAN_CTAG_RX |
 				       NETIF_F_HW_VLAN_STAG_RX;
 	netdev->features |= netdev->hw_features;
 
 	/* HW supports tc offload but mutually exclusive with n-tuple filters */
-	if (pf->flags & OTX2_FLAG_TC_FLOWER_SUPPORT)
+	if (otx2_test_flag(pf, OTX2_FLAG_TC_FLOWER_SUPPORT))
 		netdev->hw_features |= NETIF_F_HW_TC;
 
 	netdev->hw_features |= NETIF_F_LOOPBACK | NETIF_F_RXALL;
@@ -3595,18 +3595,18 @@ static void otx2_remove(struct pci_dev *pdev)
 
 	pf = netdev_priv(netdev);
 
-	pf->flags |= OTX2_FLAG_PF_SHUTDOWN;
+	otx2_set_flag(pf, OTX2_FLAG_PF_SHUTDOWN);
 
-	if (pf->flags & OTX2_FLAG_TX_TSTAMP_ENABLED)
+	if (otx2_test_flag(pf, OTX2_FLAG_TX_TSTAMP_ENABLED))
 		otx2_config_hw_tx_tstamp(pf, false);
-	if (pf->flags & OTX2_FLAG_RX_TSTAMP_ENABLED)
+	if (otx2_test_flag(pf, OTX2_FLAG_RX_TSTAMP_ENABLED))
 		otx2_config_hw_rx_tstamp(pf, false);
 
 	/* Disable 802.3x pause frames */
-	if (pf->flags & OTX2_FLAG_RX_PAUSE_ENABLED ||
-	    (pf->flags & OTX2_FLAG_TX_PAUSE_ENABLED)) {
-		pf->flags &= ~OTX2_FLAG_RX_PAUSE_ENABLED;
-		pf->flags &= ~OTX2_FLAG_TX_PAUSE_ENABLED;
+	if (otx2_test_flag(pf, OTX2_FLAG_RX_PAUSE_ENABLED) ||
+	    otx2_test_flag(pf, OTX2_FLAG_TX_PAUSE_ENABLED)) {
+		otx2_clear_flag(pf, OTX2_FLAG_RX_PAUSE_ENABLED);
+		otx2_clear_flag(pf, OTX2_FLAG_TX_PAUSE_ENABLED);
 		otx2_config_pause_frm(pf);
 	}
 
diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_tc.c b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_tc.c
index 039fd47ebf52..ddb46b580c3b 100644
--- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_tc.c
+++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_tc.c
@@ -159,7 +159,7 @@ static int otx2_tc_validate_flow(struct otx2_nic *nic,
 				 struct flow_action *actions,
 				 struct netlink_ext_ack *extack)
 {
-	if (nic->flags & OTX2_FLAG_INTF_DOWN) {
+	if (otx2_test_flag(nic, OTX2_FLAG_INTF_DOWN)) {
 		NL_SET_ERR_MSG_MOD(extack, "Interface not initialized");
 		return -EINVAL;
 	}
@@ -223,7 +223,7 @@ static int otx2_tc_egress_matchall_install(struct otx2_nic *nic,
 	if (err)
 		return err;
 
-	if (nic->flags & OTX2_FLAG_TC_MATCHALL_EGRESS_ENABLED) {
+	if (otx2_test_flag(nic, OTX2_FLAG_TC_MATCHALL_EGRESS_ENABLED)) {
 		NL_SET_ERR_MSG_MOD(extack,
 				   "Only one Egress MATCHALL ratelimiter can be offloaded");
 		return -ENOMEM;
@@ -244,7 +244,7 @@ static int otx2_tc_egress_matchall_install(struct otx2_nic *nic,
 						    otx2_convert_rate(entry->police.rate_bytes_ps));
 		if (err)
 			return err;
-		nic->flags |= OTX2_FLAG_TC_MATCHALL_EGRESS_ENABLED;
+		otx2_set_flag(nic, OTX2_FLAG_TC_MATCHALL_EGRESS_ENABLED);
 		break;
 	default:
 		NL_SET_ERR_MSG_MOD(extack,
@@ -261,13 +261,13 @@ static int otx2_tc_egress_matchall_delete(struct otx2_nic *nic,
 	struct netlink_ext_ack *extack = cls->common.extack;
 	int err;
 
-	if (nic->flags & OTX2_FLAG_INTF_DOWN) {
+	if (otx2_test_flag(nic, OTX2_FLAG_INTF_DOWN)) {
 		NL_SET_ERR_MSG_MOD(extack, "Interface not initialized");
 		return -EINVAL;
 	}
 
 	err = otx2_set_matchall_egress_rate(nic, 0, 0);
-	nic->flags &= ~OTX2_FLAG_TC_MATCHALL_EGRESS_ENABLED;
+	otx2_clear_flag(nic, OTX2_FLAG_TC_MATCHALL_EGRESS_ENABLED);
 	return err;
 }
 
@@ -505,7 +505,7 @@ static int otx2_tc_parse_actions(struct otx2_nic *nic,
 			mark = act->mark;
 			req->match_id = mark & OTX2_RX_MATCH_ID_MASK;
 			req->op = NIX_RX_ACTION_DEFAULT;
-			nic->flags |= OTX2_FLAG_TC_MARK_ENABLED;
+			otx2_set_flag(nic, OTX2_FLAG_TC_MARK_ENABLED);
 			refcount_inc(&nic->flow_cfg->mark_flows);
 			break;
 
@@ -942,7 +942,7 @@ static void otx2_destroy_tc_flow_list(struct otx2_nic *pfvf)
 	struct otx2_flow_config *flow_cfg = pfvf->flow_cfg;
 	struct otx2_tc_flow *iter, *tmp;
 
-	if (!(pfvf->flags & OTX2_FLAG_MCAM_ENTRIES_ALLOC))
+	if (!otx2_test_flag(pfvf, OTX2_FLAG_MCAM_ENTRIES_ALLOC))
 		return;
 
 	list_for_each_entry_safe(iter, tmp, &flow_cfg->flow_list_tc, list) {
@@ -1195,12 +1195,12 @@ static int otx2_tc_del_flow(struct otx2_nic *nic,
 	/* Disable TC MARK flag if they are no rules with skbedit mark action */
 	if (flow_node->req.match_id)
 		if (!refcount_dec_and_test(&flow_cfg->mark_flows))
-			nic->flags &= ~OTX2_FLAG_TC_MARK_ENABLED;
+			otx2_clear_flag(nic, OTX2_FLAG_TC_MARK_ENABLED);
 
 	if (flow_node->is_act_police) {
 		__clear_bit(flow_node->rq, &nic->rq_bmap);
 
-		if (nic->flags & OTX2_FLAG_INTF_DOWN)
+		if (otx2_test_flag(nic, OTX2_FLAG_INTF_DOWN))
 			goto free_mcam_flow;
 
 		mutex_lock(&nic->mbox.lock);
@@ -1246,10 +1246,10 @@ static int otx2_tc_add_flow(struct otx2_nic *nic,
 	struct npc_install_flow_req *req, dummy;
 	int rc, err, entry;
 
-	if (!(nic->flags & OTX2_FLAG_TC_FLOWER_SUPPORT))
+	if (!otx2_test_flag(nic, OTX2_FLAG_TC_FLOWER_SUPPORT))
 		return -ENOMEM;
 
-	if (nic->flags & OTX2_FLAG_INTF_DOWN) {
+	if (otx2_test_flag(nic, OTX2_FLAG_INTF_DOWN)) {
 		NL_SET_ERR_MSG_MOD(extack, "Interface not initialized");
 		return -EINVAL;
 	}
@@ -1444,7 +1444,7 @@ static int otx2_tc_ingress_matchall_install(struct otx2_nic *nic,
 	if (err)
 		return err;
 
-	if (nic->flags & OTX2_FLAG_TC_MATCHALL_INGRESS_ENABLED) {
+	if (otx2_test_flag(nic, OTX2_FLAG_TC_MATCHALL_INGRESS_ENABLED)) {
 		NL_SET_ERR_MSG_MOD(extack,
 				   "Only one ingress MATCHALL ratelimitter can be offloaded");
 		return -ENOMEM;
@@ -1469,7 +1469,7 @@ static int otx2_tc_ingress_matchall_install(struct otx2_nic *nic,
 		err = cn10k_set_matchall_ipolicer_rate(nic, entry->police.burst, rate);
 		if (err)
 			return err;
-		nic->flags |= OTX2_FLAG_TC_MATCHALL_INGRESS_ENABLED;
+		otx2_set_flag(nic, OTX2_FLAG_TC_MATCHALL_INGRESS_ENABLED);
 		break;
 	default:
 		NL_SET_ERR_MSG_MOD(extack,
@@ -1486,13 +1486,13 @@ static int otx2_tc_ingress_matchall_delete(struct otx2_nic *nic,
 	struct netlink_ext_ack *extack = cls->common.extack;
 	int err;
 
-	if (nic->flags & OTX2_FLAG_INTF_DOWN) {
+	if (otx2_test_flag(nic, OTX2_FLAG_INTF_DOWN)) {
 		NL_SET_ERR_MSG_MOD(extack, "Interface not initialized");
 		return -EINVAL;
 	}
 
 	err = cn10k_free_matchall_ipolicer(nic);
-	nic->flags &= ~OTX2_FLAG_TC_MATCHALL_INGRESS_ENABLED;
+	otx2_clear_flag(nic, OTX2_FLAG_TC_MATCHALL_INGRESS_ENABLED);
 	return err;
 }
 
diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_txrx.c b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_txrx.c
index 8d2d607bc92f..f65ba44db60b 100644
--- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_txrx.c
+++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_txrx.c
@@ -171,7 +171,7 @@ static void otx2_set_rxtstamp(struct otx2_nic *pfvf,
 	u64 timestamp, tsns;
 	int err;
 
-	if (!(pfvf->flags & OTX2_FLAG_RX_TSTAMP_ENABLED))
+	if (!otx2_test_flag(pfvf, OTX2_FLAG_RX_TSTAMP_ENABLED))
 		return;
 
 	timestamp = pfvf->ptp->convert_rx_ptp_tstmp(*(u64 *)data);
@@ -374,13 +374,13 @@ static void otx2_rcv_pkt_handler(struct otx2_nic *pfvf,
 	}
 	otx2_set_rxhash(pfvf, cqe, skb);
 
-	if (!(pfvf->flags & OTX2_FLAG_REP_MODE_ENABLED)) {
+	if (!otx2_test_flag(pfvf, OTX2_FLAG_REP_MODE_ENABLED)) {
 		skb_record_rx_queue(skb, cq->cq_idx);
 		if (pfvf->netdev->features & NETIF_F_RXCSUM)
 			skb->ip_summed = CHECKSUM_UNNECESSARY;
 	}
 
-	if (pfvf->flags & OTX2_FLAG_TC_MARK_ENABLED)
+	if (otx2_test_flag(pfvf, OTX2_FLAG_TC_MARK_ENABLED))
 		skb->mark = parse->match_id;
 
 	skb_mark_for_recycle(skb);
@@ -513,7 +513,7 @@ static int otx2_tx_napi_handler(struct otx2_nic *pfvf,
 		     ((u64)cq->cq_idx << 32) | processed_cqe);
 
 #if IS_ENABLED(CONFIG_RVU_ESWITCH)
-	if (pfvf->flags & OTX2_FLAG_REP_MODE_ENABLED)
+	if (otx2_test_flag(pfvf, OTX2_FLAG_REP_MODE_ENABLED))
 		ndev = pfvf->reps[qidx]->netdev;
 	else
 #endif
@@ -526,7 +526,7 @@ static int otx2_tx_napi_handler(struct otx2_nic *pfvf,
 
 		if (qidx >= pfvf->hw.tx_queues)
 			qidx -= pfvf->hw.xdp_queues;
-		if (pfvf->flags & OTX2_FLAG_REP_MODE_ENABLED)
+		if (otx2_test_flag(pfvf, OTX2_FLAG_REP_MODE_ENABLED))
 			qidx = 0;
 		txq = netdev_get_tx_queue(ndev, qidx);
 		netdev_tx_completed_queue(txq, tx_pkts, tx_bytes);
@@ -599,11 +599,11 @@ int otx2_napi_handler(struct napi_struct *napi, int budget)
 
 	if (workdone < budget && napi_complete_done(napi, workdone)) {
 		/* If interface is going down, don't re-enable IRQ */
-		if (pfvf->flags & OTX2_FLAG_INTF_DOWN)
+		if (otx2_test_flag(pfvf, OTX2_FLAG_INTF_DOWN))
 			return workdone;
 
 		/* Adjust irq coalese using net_dim */
-		if (pfvf->flags & OTX2_FLAG_ADPTV_INT_COAL_ENABLED)
+		if (otx2_test_flag(pfvf, OTX2_FLAG_ADPTV_INT_COAL_ENABLED))
 			otx2_adjust_adaptive_coalese(pfvf, cq_poll);
 
 		if (likely(cq))
@@ -1137,7 +1137,7 @@ static void otx2_set_txtstamp(struct otx2_nic *pfvf, struct sk_buff *skb,
 
 	if (unlikely(!skb_shinfo(skb)->gso_size &&
 		     (skb_shinfo(skb)->tx_flags & SKBTX_HW_TSTAMP))) {
-		if (unlikely(pfvf->flags & OTX2_FLAG_PTP_ONESTEP_SYNC &&
+		if (unlikely(otx2_test_flag(pfvf, OTX2_FLAG_PTP_ONESTEP_SYNC) &&
 			     otx2_ptp_is_sync(skb, &ptp_offset, &udp_csum_crt))) {
 			origin_tstamp = (struct ptpv2_tstamp *)
 					((u8 *)skb->data + ptp_offset +
diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_vf.c b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_vf.c
index f7765e19d78a..5f7915231ca3 100644
--- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_vf.c
+++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_vf.c
@@ -610,7 +610,7 @@ static int otx2vf_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 	vf->dev = dev;
 	vf->iommu_domain = iommu_get_domain_for_dev(dev);
 
-	vf->flags |= OTX2_FLAG_INTF_DOWN;
+	otx2_set_flag(vf, OTX2_FLAG_INTF_DOWN);
 	hw = &vf->hw;
 	hw->pdev = vf->pdev;
 	hw->rx_queues = qcount;
@@ -824,10 +824,10 @@ static void otx2vf_remove(struct pci_dev *pdev)
 	vf = netdev_priv(netdev);
 
 	/* Disable 802.3x pause frames */
-	if (vf->flags & OTX2_FLAG_RX_PAUSE_ENABLED ||
-	    (vf->flags & OTX2_FLAG_TX_PAUSE_ENABLED)) {
-		vf->flags &= ~OTX2_FLAG_RX_PAUSE_ENABLED;
-		vf->flags &= ~OTX2_FLAG_TX_PAUSE_ENABLED;
+	if (otx2_test_flag(vf, OTX2_FLAG_RX_PAUSE_ENABLED) ||
+	    otx2_test_flag(vf, OTX2_FLAG_TX_PAUSE_ENABLED)) {
+		otx2_clear_flag(vf, OTX2_FLAG_RX_PAUSE_ENABLED);
+		otx2_clear_flag(vf, OTX2_FLAG_TX_PAUSE_ENABLED);
 		otx2_config_pause_frm(vf);
 	}
 
diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_xsk.c b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_xsk.c
index 0e8a6a6486c4..7808588a0234 100644
--- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_xsk.c
+++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_xsk.c
@@ -96,7 +96,7 @@ static void otx2_clean_up_rq(struct otx2_nic *pfvf, int qidx)
 	u64 iova;
 
 	/* If the DOWN flag is set SQs are already freed */
-	if (pfvf->flags & OTX2_FLAG_INTF_DOWN)
+	if (otx2_test_flag(pfvf, OTX2_FLAG_INTF_DOWN))
 		return;
 
 	cq = &qset->cq[qidx];
@@ -172,7 +172,7 @@ int otx2_xsk_wakeup(struct net_device *dev, u32 queue_id, u32 flags)
 	struct otx2_cq_poll *cq_poll = NULL;
 	struct otx2_qset *qset = &pf->qset;
 
-	if (pf->flags & OTX2_FLAG_INTF_DOWN)
+	if (otx2_test_flag(pf, OTX2_FLAG_INTF_DOWN))
 		return -ENETDOWN;
 
 	if (queue_id >= pf->hw.rx_queues || queue_id >= pf->hw.tx_queues)
diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/qos_sq.c b/drivers/net/ethernet/marvell/octeontx2/nic/qos_sq.c
index 2872adabc830..5f09e2960144 100644
--- a/drivers/net/ethernet/marvell/octeontx2/nic/qos_sq.c
+++ b/drivers/net/ethernet/marvell/octeontx2/nic/qos_sq.c
@@ -238,7 +238,7 @@ int otx2_qos_enable_sq(struct otx2_nic *pfvf, int qidx)
 	struct otx2_hw *hw = &pfvf->hw;
 	int pool_id, sq_idx, err;
 
-	if (pfvf->flags & OTX2_FLAG_INTF_DOWN)
+	if (otx2_test_flag(pfvf, OTX2_FLAG_INTF_DOWN))
 		return -EPERM;
 
 	sq_idx = hw->non_qos_queues + qidx;
@@ -288,7 +288,7 @@ void otx2_qos_disable_sq(struct otx2_nic *pfvf, int qidx)
 	sq_idx = hw->non_qos_queues + qidx;
 
 	/* If the DOWN flag is set SQs are already freed */
-	if (pfvf->flags & OTX2_FLAG_INTF_DOWN)
+	if (otx2_test_flag(pfvf, OTX2_FLAG_INTF_DOWN))
 		return;
 
 	sq = &pfvf->qset.sq[sq_idx];
diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/rep.c b/drivers/net/ethernet/marvell/octeontx2/nic/rep.c
index 0f5d5642d3f7..7df82c22cc12 100644
--- a/drivers/net/ethernet/marvell/octeontx2/nic/rep.c
+++ b/drivers/net/ethernet/marvell/octeontx2/nic/rep.c
@@ -93,9 +93,9 @@ static int rvu_rep_mcam_flow_init(struct rep_dev *rep)
 	rep->flow_cfg->max_flows = allocated;
 
 	if (allocated) {
-		rep->flags |= OTX2_FLAG_MCAM_ENTRIES_ALLOC;
-		rep->flags |= OTX2_FLAG_NTUPLE_SUPPORT;
-		rep->flags |= OTX2_FLAG_TC_FLOWER_SUPPORT;
+		set_bit(OTX2_FLAG_MCAM_ENTRIES_ALLOC, &rep->flags);
+		set_bit(OTX2_FLAG_NTUPLE_SUPPORT, &rep->flags);
+		set_bit(OTX2_FLAG_TC_FLOWER_SUPPORT, &rep->flags);
 	}
 
 	INIT_LIST_HEAD(&rep->flow_cfg->flow_list);
@@ -109,14 +109,14 @@ static int rvu_rep_setup_tc_cb(enum tc_setup_type type,
 	struct rep_dev *rep = cb_priv;
 	struct otx2_nic *priv = rep->mdev;
 
-	if (!(rep->flags & RVU_REP_VF_INITIALIZED))
+	if (!test_bit(OTX2_FLAG_REP_VF_INITIALIZED, &rep->flags))
 		return -EINVAL;
 
-	if (!(rep->flags & OTX2_FLAG_TC_FLOWER_SUPPORT))
+	if (!test_bit(OTX2_FLAG_TC_FLOWER_SUPPORT, &rep->flags))
 		rvu_rep_mcam_flow_init(rep);
 
 	priv->netdev = rep->netdev;
-	priv->flags = rep->flags;
+	otx2_sync_flags_from_rep(priv, &rep->flags);
 	priv->pcifunc = rep->pcifunc;
 	priv->flow_cfg = rep->flow_cfg;
 
@@ -303,9 +303,9 @@ static void rvu_rep_state_evt_handler(struct otx2_nic *priv,
 	rep_id = rvu_rep_get_repid(priv, info->pcifunc);
 	rep = priv->reps[rep_id];
 	if (info->evt_data.vf_state)
-		rep->flags |= RVU_REP_VF_INITIALIZED;
+		set_bit(OTX2_FLAG_REP_VF_INITIALIZED, &rep->flags);
 	else
-		rep->flags &= ~RVU_REP_VF_INITIALIZED;
+		clear_bit(OTX2_FLAG_REP_VF_INITIALIZED, &rep->flags);
 }
 
 int rvu_event_up_notify(struct otx2_nic *pf, struct rep_event *info)
@@ -382,7 +382,7 @@ static void rvu_rep_get_stats64(struct net_device *dev,
 {
 	struct rep_dev *rep = netdev_priv(dev);
 
-	if (!(rep->flags & RVU_REP_VF_INITIALIZED))
+	if (!test_bit(OTX2_FLAG_REP_VF_INITIALIZED, &rep->flags))
 		return;
 
 	stats->rx_packets = rep->stats.rx_frames;
@@ -453,7 +453,7 @@ static int rvu_rep_open(struct net_device *dev)
 	struct otx2_nic *priv = rep->mdev;
 	struct rep_event evt = {0};
 
-	if (!(rep->flags & RVU_REP_VF_INITIALIZED))
+	if (!test_bit(OTX2_FLAG_REP_VF_INITIALIZED, &rep->flags))
 		return 0;
 
 	netif_carrier_on(dev);
@@ -472,7 +472,7 @@ static int rvu_rep_stop(struct net_device *dev)
 	struct otx2_nic *priv = rep->mdev;
 	struct rep_event evt = {0};
 
-	if (!(rep->flags & RVU_REP_VF_INITIALIZED))
+	if (!test_bit(OTX2_FLAG_REP_VF_INITIALIZED, &rep->flags))
 		return 0;
 
 	netif_carrier_off(dev);
@@ -547,7 +547,7 @@ static int rvu_rep_napi_init(struct otx2_nic *priv,
 		otx2_write64(priv, NIX_LF_CINTX_INT(qidx), BIT_ULL(0));
 		otx2_write64(priv, NIX_LF_CINTX_ENA_W1S(qidx), BIT_ULL(0));
 	}
-	priv->flags &= ~OTX2_FLAG_INTF_DOWN;
+	otx2_clear_flag(priv, OTX2_FLAG_INTF_DOWN);
 	return 0;
 
 err_free_cints:
@@ -632,7 +632,7 @@ void rvu_rep_destroy(struct otx2_nic *priv)
 	int rep_id;
 
 	rvu_eswitch_config(priv, false);
-	priv->flags |= OTX2_FLAG_INTF_DOWN;
+	otx2_set_flag(priv, OTX2_FLAG_INTF_DOWN);
 	rvu_rep_free_cq_rsrc(priv);
 	for (rep_id = 0; rep_id < priv->rep_cnt; rep_id++) {
 		rep = priv->reps[rep_id];
@@ -801,8 +801,8 @@ static int rvu_rep_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 	pci_set_drvdata(pdev, priv);
 	priv->pdev = pdev;
 	priv->dev = dev;
-	priv->flags |= OTX2_FLAG_INTF_DOWN;
-	priv->flags |= OTX2_FLAG_REP_MODE_ENABLED;
+	otx2_set_flag(priv, OTX2_FLAG_INTF_DOWN);
+	otx2_set_flag(priv, OTX2_FLAG_REP_MODE_ENABLED);
 
 	hw = &priv->hw;
 	hw->pdev = pdev;
@@ -845,7 +845,7 @@ static void rvu_rep_remove(struct pci_dev *pdev)
 	struct otx2_nic *priv = pci_get_drvdata(pdev);
 
 	otx2_unregister_dl(priv);
-	if (!(priv->flags & OTX2_FLAG_INTF_DOWN))
+	if (!otx2_test_flag(priv, OTX2_FLAG_INTF_DOWN))
 		rvu_rep_destroy(priv);
 	otx2_detach_resources(&priv->mbox);
 	if (priv->hw.lmt_info)
diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/rep.h b/drivers/net/ethernet/marvell/octeontx2/nic/rep.h
index 5bc9e2c7d800..45707c434d89 100644
--- a/drivers/net/ethernet/marvell/octeontx2/nic/rep.h
+++ b/drivers/net/ethernet/marvell/octeontx2/nic/rep.h
@@ -37,8 +37,7 @@ struct rep_dev {
 	struct delayed_work stats_wrk;
 	struct devlink_port dl_port;
 	struct otx2_flow_config	*flow_cfg;
-#define RVU_REP_VF_INITIALIZED		BIT_ULL(0)
-	u64 flags;
+	unsigned long		flags;
 	u16 rep_id;
 	u16 pcifunc;
 	u8 mac[ETH_ALEN];
-- 
2.43.0


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH v16 net-next 2/2] octeontx2-pf: add mqprio bandwidth offload for NIX TX schedulers
  2026-09-18  1:59 [PATCH v16 net-next 0/2] octeontx2: mqprio bandwidth offload for NIX TX schedulers Ratheesh Kannoth
  2026-09-18  1:59 ` [PATCH v16 net-next 1/2] octeontx2: use atomic bitops for PF/VF and rep flags Ratheesh Kannoth
@ 2026-09-18  1:59 ` Ratheesh Kannoth
  2026-09-22  2:19   ` netdev-bot+sashiko
  1 sibling, 1 reply; 5+ messages in thread
From: Ratheesh Kannoth @ 2026-09-18  1:59 UTC (permalink / raw)
  To: bpf, linux-kernel, netdev
  Cc: andrew+netdev, ast, daniel, davem, edumazet, hawk,
	john.fastabend, kuba, pabeni, sdf, sgoutham, Ratheesh Kannoth

Add TC_SETUP_QDISC_MQPRIO offload for channel-mode mqprio with
TC_MQPRIO_SHAPER_BW_RATE on PF and VF RVU netdevices. Program per-queue
MDQ CIR/PIR for non-QoS transmit queues via the NIX TX scheduler mailbox.
When active, allocate one SMQ per queue and parent MDQs under TL4[0].

The NIX TX scheduler cannot be reprogrammed live, so add, replace,
delete, and rollback rebuild the hierarchy by bouncing the netdev through
ndo_stop()/ndo_open(), dropping in-flight traffic. Cache rates in software
and restore shapers from otx2_mqprio_up() on ndo_open(); fail closed if
restore fails, leaving ndo_open() unsuccessful and the interface down.

Stage configuration in mq_offload_snap snapshots for tc replace:
failed setup rolls back via netdev restart, TC_ROOT_GRAFT commits a
successful graft, and teardown of the replaced qdisc instance commits
the staged snapshot without disabling live offload.

Require a running interface and CIR+PIR support. PF and VF share the
same TC offload path; SDP representors are not supported. Reject per-TC
rates when a traffic class maps to more than one queue. Block concurrent
use with PFC, XDP, SDP rep, or HTB, and block ethtool channel changes
while offload is active.

Signed-off-by: Ratheesh Kannoth <rkannoth@marvell.com>
---
 .../marvell/octeontx2/nic/otx2_common.c       | 146 +++-
 .../marvell/octeontx2/nic/otx2_common.h       |  30 +
 .../marvell/octeontx2/nic/otx2_dcbnl.c        |   6 +
 .../marvell/octeontx2/nic/otx2_ethtool.c      |   8 +
 .../ethernet/marvell/octeontx2/nic/otx2_pf.c  |  17 +
 .../ethernet/marvell/octeontx2/nic/otx2_tc.c  | 812 ++++++++++++++++++
 .../net/ethernet/marvell/octeontx2/nic/qos.c  |  11 +
 7 files changed, 1029 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.c b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.c
index b421cb75e44b..5bad2466da0c 100644
--- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.c
+++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.c
@@ -615,6 +615,142 @@ void otx2_get_mac_from_af(struct net_device *netdev)
 }
 EXPORT_SYMBOL(otx2_get_mac_from_af);
 
+static int
+otx2_nix_tmq_reg_write(struct otx2_nic *pfvf, int cnt,
+		       u64 reg_addr[MAX_REGS_PER_MBOX_MSG],
+		       u64 reg_val[MAX_REGS_PER_MBOX_MSG])
+{
+	struct mbox *mbox = &pfvf->mbox;
+	struct nix_txschq_config *req;
+	int i, err;
+
+	mutex_lock(&mbox->lock);
+	req = otx2_mbox_alloc_msg_nix_txschq_cfg(mbox);
+	if (!req) {
+		mutex_unlock(&mbox->lock);
+		return -ENOMEM;
+	}
+
+	req->lvl = NIX_TXSCH_LVL_MDQ;
+	req->num_regs = cnt;
+
+	for (i = 0; i < cnt; i++) {
+		req->reg[i] = reg_addr[i];
+		req->regval[i] = reg_val[i];
+	}
+
+	err = otx2_sync_mbox_msg(mbox);
+	mutex_unlock(&mbox->lock);
+
+	return err;
+}
+
+int otx2_nix_tm_clear_queue_shaper(struct otx2_nic *pfvf)
+{
+	u64 reg_addr[MAX_REGS_PER_MBOX_MSG];
+	u64 reg_val[MAX_REGS_PER_MBOX_MSG];
+	int err, smq, i, cnt = 0;
+
+	for (i = 0; i < pfvf->hw.txschq_cnt[NIX_TXSCH_LVL_SMQ]; i++) {
+		smq = pfvf->hw.txschq_list[NIX_TXSCH_LVL_SMQ][i];
+
+		reg_addr[cnt] = NIX_AF_MDQX_PIR(smq);
+		reg_val[cnt] = 0;
+		cnt++;
+
+		reg_addr[cnt] = NIX_AF_MDQX_CIR(smq);
+		reg_val[cnt] = 0;
+		cnt++;
+
+		if (cnt < MAX_REGS_PER_MBOX_MSG - 1)
+			continue;
+
+		err = otx2_nix_tmq_reg_write(pfvf, cnt,
+					     reg_addr, reg_val);
+		if (err)
+			goto fail;
+		cnt = 0;
+	}
+
+	if (cnt) {
+		err = otx2_nix_tmq_reg_write(pfvf, cnt,
+					     reg_addr, reg_val);
+		if (err)
+			goto fail;
+	}
+
+	return 0;
+fail:
+	return err;
+}
+
+int otx2_nix_tm_set_queue_shaper(struct otx2_nic *pfvf,
+				 int txq, u64 minrate, u64 maxrate)
+{
+	struct mbox *mbox = &pfvf->mbox;
+	struct nix_txschq_config *req;
+	int err, smq, n = 0;
+	u64 reg_addr[2];
+	u64 reg_val[2];
+	u64 rate;
+
+	if (!maxrate && !minrate) {
+		smq = otx2_get_smq_idx(pfvf, txq);
+		reg_addr[0] = NIX_AF_MDQX_PIR(smq);
+		reg_val[0] = 0;
+		reg_addr[1] = NIX_AF_MDQX_CIR(smq);
+		reg_val[1] = 0;
+		return otx2_nix_tmq_reg_write(pfvf, 2, reg_addr, reg_val);
+	}
+
+	smq = otx2_get_smq_idx(pfvf, txq);
+
+	mutex_lock(&mbox->lock);
+	req = otx2_mbox_alloc_msg_nix_txschq_cfg(mbox);
+	if (!req) {
+		mutex_unlock(&mbox->lock);
+		return -ENOMEM;
+	}
+
+	req->lvl = NIX_TXSCH_LVL_MDQ;
+
+	/* MQPRIO exposes only min/max rate, not burst.  Pass burst 0 so
+	 * otx2_get_egress_burst_cfg() programmes the largest burst the NIX
+	 * encoding supports (CN10K_MAX_BURST_SIZE on CN10K).  This differs
+	 * from the 65536 byte default used in the HTB path, which is a
+	 * kernel-side default when no explicit burst is configured, not a
+	 * hardware cap.
+	 *
+	 * mqprio setup restarts the netdev (otx2_mqprio_restart_netdev),
+	 * which resets MDQ shapers to zero.  Program both PIR and CIR on
+	 * every update so omitted rates are applied explicitly rather than
+	 * relying on stale hardware state.
+	 */
+	req->reg[n] = NIX_AF_MDQX_PIR(smq);
+	if (maxrate) {
+		rate = otx2_convert_rate(maxrate);
+		req->regval[n] = otx2_get_txschq_rate_regval(pfvf, rate, 0);
+	} else {
+		req->regval[n] = 0;
+	}
+	n++;
+
+	/* CIR+PIR support is required and checked at mqprio setup. */
+	req->reg[n] = NIX_AF_MDQX_CIR(smq);
+	if (minrate) {
+		rate = otx2_convert_rate(minrate);
+		req->regval[n] = otx2_get_txschq_rate_regval(pfvf, rate, 0);
+	} else {
+		req->regval[n] = 0;
+	}
+	n++;
+	req->num_regs = n;
+
+	err = otx2_sync_mbox_msg(mbox);
+	mutex_unlock(&mbox->lock);
+	return err;
+}
+
 int otx2_txschq_config(struct otx2_nic *pfvf, int lvl, int prio, bool txschq_for_pfc)
 {
 	u16 (*schq_list)[MAX_TXSCHQ_PER_FUNC];
@@ -651,7 +787,11 @@ int otx2_txschq_config(struct otx2_nic *pfvf, int lvl, int prio, bool txschq_for
 						(u64)hw->smq_link_type);
 		req->num_regs++;
 		/* MDQ config */
-		parent = schq_list[NIX_TXSCH_LVL_TL4][prio];
+		if (pfvf->mqprio.rate_limit)
+			parent = schq_list[NIX_TXSCH_LVL_TL4][0];
+		else
+			parent = schq_list[NIX_TXSCH_LVL_TL4][prio];
+
 		req->reg[1] = NIX_AF_MDQX_PARENT(schq);
 		req->regval[1] = parent << 16;
 		req->num_regs++;
@@ -779,6 +919,9 @@ int otx2_txsch_alloc(struct otx2_nic *pfvf)
 		req->schq[NIX_TXSCH_LVL_TL4] = chan_cnt;
 	}
 
+	if (pfvf->mqprio.rate_limit)
+		req->schq[NIX_TXSCH_LVL_SMQ] = pfvf->hw.non_qos_queues;
+
 	rc = otx2_sync_mbox_msg(&pfvf->mbox);
 	if (rc)
 		return rc;
@@ -844,6 +987,7 @@ void otx2_txschq_stop(struct otx2_nic *pfvf)
 
 	/* Clear the txschq list */
 	for (lvl = 0; lvl < NIX_TXSCH_LVL_CNT; lvl++) {
+		pfvf->hw.txschq_cnt[lvl] = 0;
 		for (schq = 0; schq < MAX_TXSCHQ_PER_FUNC; schq++)
 			pfvf->hw.txschq_list[lvl][schq] = 0;
 	}
diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.h b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.h
index 90cf302bbe6d..820bfe75d2b2 100644
--- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.h
+++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.h
@@ -17,6 +17,7 @@
 #include <linux/soc/marvell/silicons.h>
 #include <linux/soc/marvell/octeontx2/asm.h>
 #include <net/macsec.h>
+#include <uapi/linux/pkt_sched.h>
 #include <net/pkt_cls.h>
 #include <net/devlink.h>
 #include <linux/time64.h>
@@ -508,6 +509,26 @@ enum otx2_flag_bits {
 	OTX2_FLAG_MAX,
 };
 
+struct mq_offload_snap {
+	u64 min_rate[TC_QOPT_MAX_QUEUE];
+	u64 max_rate[TC_QOPT_MAX_QUEUE];
+	u32 flags;
+	__u8 num_tc;
+	__u16 count[TC_QOPT_MAX_QUEUE];
+	__u16 offset[TC_QOPT_MAX_QUEUE];
+	__u8 prio_tc_map[TC_QOPT_BITMASK + 1];
+};
+
+struct otx2_mqprio {
+	u32	flags;
+	u64	*min_rate;
+	u64	*max_rate;
+	bool	rate_limit;
+	bool	replace_setup_done;
+	bool	replace_graft_done;
+	struct work_struct netdev_tc_work;
+};
+
 struct otx2_nic {
 	void __iomem		*reg_base;
 	struct net_device	*netdev;
@@ -519,6 +540,10 @@ struct otx2_nic {
 	unsigned long		flags;
 	u64			*cq_op_addr;
 
+	struct otx2_mqprio	mqprio;
+	struct mq_offload_snap	*cur_mq_snap;
+	struct mq_offload_snap	*old_mq_snap;
+
 	struct bpf_prog		*xdp_prog;
 	struct otx2_qset	qset;
 	struct otx2_hw		hw;
@@ -1278,6 +1303,11 @@ dma_addr_t otx2_dma_map_skb_frag(struct otx2_nic *pfvf,
 				 struct sk_buff *skb, int seg, int *len);
 void otx2_dma_unmap_skb_frags(struct otx2_nic *pfvf, struct sg_list *sg);
 int otx2_read_free_sqe(struct otx2_nic *pfvf, u16 qidx);
+int otx2_nix_tm_set_queue_shaper(struct otx2_nic *pfvf, int txq,
+				 u64 minrate, u64 maxrate);
+int otx2_nix_tm_clear_queue_shaper(struct otx2_nic *pfvf);
+int otx2_mqprio_down(struct otx2_nic *pfvf);
+int otx2_mqprio_up(struct otx2_nic *pfvf);
 void otx2_queue_vf_work(struct mbox *mw, struct workqueue_struct *mbox_wq,
 			int first, int mdevs, u64 intr);
 int otx2_del_mcam_flow_entry(struct otx2_nic *nic, u16 entry,
diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_dcbnl.c b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_dcbnl.c
index 91d346d114af..b7bd08129fb6 100644
--- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_dcbnl.c
+++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_dcbnl.c
@@ -413,6 +413,12 @@ static int otx2_dcbnl_ieee_setpfc(struct net_device *dev, struct ieee_pfc *pfc)
 	u8 old_pfc_en;
 	int err;
 
+	if (pfvf->mqprio.rate_limit && pfc->pfc_en) {
+		netdev_err(dev,
+			   "PFC: cannot enable while mqprio bandwidth offload is active\n");
+		return -EOPNOTSUPP;
+	}
+
 	old_pfc_en = pfvf->pfc_en;
 	pfvf->pfc_en = pfc->pfc_en;
 
diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_ethtool.c b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_ethtool.c
index 4fe473d9ea0d..5428b3d1b332 100644
--- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_ethtool.c
+++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_ethtool.c
@@ -287,6 +287,14 @@ static int otx2_set_channels(struct net_device *dev,
 		return -EINVAL;
 	}
 
+	if (pfvf->mqprio.rate_limit &&
+	    (channel->tx_count != pfvf->hw.tx_queues ||
+	     channel->rx_count != pfvf->hw.rx_queues)) {
+		netdev_info(dev,
+			    "Not permitted to change channel count while MQ prio is active\n");
+		return -EINVAL;
+	}
+
 	if (if_up)
 		dev->netdev_ops->ndo_stop(dev);
 
diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c
index 32582b6347ea..5ff99ad986d0 100644
--- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c
+++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c
@@ -2007,6 +2007,15 @@ int otx2_open(struct net_device *netdev)
 	if (err)
 		goto err_free_mem;
 
+	/* Fail closed: abort open if cached mqprio shapers cannot be restored. */
+	err = otx2_mqprio_up(pf);
+	if (err) {
+		netdev_err(pf->netdev,
+			   "mqprio: failed to restore shapers during open: %d\n",
+			   err);
+		goto err_free_hw;
+	}
+
 	/* Register NAPI handler */
 	for (qidx = 0; qidx < pf->hw.cint_cnt; qidx++) {
 		cq_poll = &qset->napi[qidx];
@@ -2205,6 +2214,7 @@ int otx2_open(struct net_device *netdev)
 	free_irq(vec, pf);
 err_disable_napi:
 	otx2_disable_napi(pf);
+err_free_hw:
 	otx2_free_hw_resources(pf);
 err_free_mem:
 	otx2_free_queue_mem(qset);
@@ -2280,6 +2290,7 @@ int otx2_stop(struct net_device *netdev)
 	for (qidx = 0; qidx < netdev->num_tx_queues; qidx++)
 		netdev_tx_reset_queue(netdev_get_tx_queue(netdev, qidx));
 
+	synchronize_net();
 	otx2_free_queue_mem(qset);
 	/* Do not clear RQ/SQ ringsize settings */
 	memset_startat(qset, 0, sqe_cnt);
@@ -2923,6 +2934,12 @@ static int otx2_xdp_setup(struct otx2_nic *pf, struct bpf_prog *prog)
 	bool if_up = netif_running(pf->netdev);
 	struct bpf_prog *old_prog;
 
+	if (prog && pf->mqprio.rate_limit) {
+		netdev_err(dev,
+			   "XDP: cannot attach while mqprio bandwidth offload is active\n");
+		return -EOPNOTSUPP;
+	}
+
 	if (prog && dev->mtu > MAX_XDP_MTU) {
 		netdev_warn(dev, "Jumbo frames not yet supported with XDP\n");
 		return -EOPNOTSUPP;
diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_tc.c b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_tc.c
index ddb46b580c3b..edd7c02efb47 100644
--- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_tc.c
+++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_tc.c
@@ -6,6 +6,8 @@
  */
 
 #include <linux/netdevice.h>
+#include <linux/rtnetlink.h>
+#include <linux/string.h>
 #include <linux/etherdevice.h>
 #include <linux/inetdevice.h>
 #include <linux/rhashtable.h>
@@ -16,6 +18,7 @@
 #include <net/tc_act/tc_mirred.h>
 #include <net/tc_act/tc_vlan.h>
 #include <net/ipv6.h>
+#include <net/pkt_sched.h>
 
 #include "cn10k.h"
 #include "otx2_common.h"
@@ -31,6 +34,20 @@
 
 #define MCAST_INVALID_GRP		(-1U)
 #define RATE_MANTISSA_BITS		8
+/* Min per-queue egress shaping rate the NIX TLX encoder supports (2 Mbps). */
+#define OTX2_MQPRIO_MIN_RATE_BYTES_PS	250000ULL
+
+static u64 otx2_mqprio_max_rate_bytes_ps(struct otx2_nic *pfvf)
+{
+	u64 max_burst;
+
+	if (is_dev_otx2(pfvf->pdev))
+		max_burst = MAX_BURST_SIZE;
+	else
+		max_burst = CN10K_MAX_BURST_SIZE;
+
+	return (max_burst * 1000000ULL) / 8ULL;
+}
 
 static void otx2_get_egress_burst_cfg(struct otx2_nic *nic, u32 burst,
 				      u32 *burst_exp, u32 *burst_mantissa)
@@ -61,6 +78,9 @@ static void otx2_get_egress_burst_cfg(struct otx2_nic *nic, u32 burst,
 			*burst_mantissa = tmp / (1ULL << (*burst_exp - 7));
 		}
 	} else {
+		/* burst 0: largest encodable burst (CN10K_MAX_BURST_SIZE on
+		 * CN10K), not a minimal burst.
+		 */
 		*burst_exp = MAX_BURST_EXPONENT;
 		*burst_mantissa = max_mantissa;
 	}
@@ -1600,14 +1620,802 @@ static int otx2_setup_tc_block(struct net_device *netdev,
 					  nic, nic, ingress);
 }
 
+/* Free the per-queue min/max rate caches. */
+static void otx2_mqprio_free_cache(struct otx2_nic *pfvf)
+{
+	devm_kfree(pfvf->dev, pfvf->mqprio.min_rate);
+	devm_kfree(pfvf->dev, pfvf->mqprio.max_rate);
+	pfvf->mqprio.min_rate = NULL;
+	pfvf->mqprio.max_rate = NULL;
+	pfvf->mqprio.flags = 0;
+}
+
+static int otx2_mqprio_alloc_cache(struct otx2_nic *pfvf, bool replacing)
+{
+	u16 num_txq = pfvf->hw.non_qos_queues;
+	u64 *min_rate, *max_rate;
+
+	if (replacing && pfvf->mqprio.min_rate && pfvf->mqprio.max_rate) {
+		memset(pfvf->mqprio.min_rate, 0,
+		       num_txq * sizeof(*pfvf->mqprio.min_rate));
+		memset(pfvf->mqprio.max_rate, 0,
+		       num_txq * sizeof(*pfvf->mqprio.max_rate));
+		pfvf->mqprio.flags = 0;
+		return 0;
+	}
+
+	min_rate = devm_kcalloc(pfvf->dev, num_txq, sizeof(*min_rate), GFP_KERNEL);
+	max_rate = devm_kcalloc(pfvf->dev, num_txq, sizeof(*max_rate), GFP_KERNEL);
+	if (!min_rate || !max_rate) {
+		devm_kfree(pfvf->dev, min_rate);
+		devm_kfree(pfvf->dev, max_rate);
+		return -ENOMEM;
+	}
+
+	otx2_mqprio_free_cache(pfvf);
+	pfvf->mqprio.min_rate = min_rate;
+	pfvf->mqprio.max_rate = max_rate;
+
+	return 0;
+}
+
+static void otx2_mqprio_snap_free(struct otx2_nic *pfvf,
+				  struct mq_offload_snap **snap)
+{
+	if (!*snap)
+		return;
+
+	devm_kfree(pfvf->dev, *snap);
+	*snap = NULL;
+}
+
+static int otx2_mqprio_snap_copy(struct otx2_nic *pfvf,
+				 struct mq_offload_snap **dst,
+				 const struct tc_mqprio_qopt_offload *mqprio)
+{
+	const struct tc_mqprio_qopt *qopt = &mqprio->qopt;
+	struct mq_offload_snap *snap;
+	int tc;
+
+	if (!*dst) {
+		snap = devm_kzalloc(pfvf->dev, sizeof(*snap), GFP_KERNEL);
+		if (!snap)
+			return -ENOMEM;
+		*dst = snap;
+	} else {
+		snap = *dst;
+	}
+
+	snap->num_tc = qopt->num_tc;
+	snap->flags = mqprio->flags;
+	for (tc = 0; tc < TC_QOPT_MAX_QUEUE; tc++) {
+		snap->count[tc] = qopt->count[tc];
+		snap->offset[tc] = qopt->offset[tc];
+		snap->min_rate[tc] = 0;
+		snap->max_rate[tc] = 0;
+	}
+
+	for (tc = 0; tc < qopt->num_tc; tc++) {
+		if (mqprio->flags & TC_MQPRIO_F_MIN_RATE)
+			snap->min_rate[tc] = mqprio->min_rate[tc];
+		if (mqprio->flags & TC_MQPRIO_F_MAX_RATE)
+			snap->max_rate[tc] = mqprio->max_rate[tc];
+	}
+	memcpy(snap->prio_tc_map, qopt->prio_tc_map, sizeof(snap->prio_tc_map));
+
+	return 0;
+}
+
+static int otx2_mqprio_stage_cur(struct otx2_nic *pfvf,
+				 const struct tc_mqprio_qopt_offload *mqprio)
+{
+	return otx2_mqprio_snap_copy(pfvf, &pfvf->cur_mq_snap, mqprio);
+}
+
+static void otx2_mqprio_snap_commit(struct otx2_nic *pfvf)
+{
+	otx2_mqprio_snap_free(pfvf, &pfvf->old_mq_snap);
+	pfvf->old_mq_snap = pfvf->cur_mq_snap;
+	pfvf->cur_mq_snap = NULL;
+}
+
+static void otx2_mqprio_clear_replace_state(struct otx2_nic *pfvf)
+{
+	pfvf->mqprio.replace_setup_done = false;
+	pfvf->mqprio.replace_graft_done = false;
+}
+
+static bool otx2_mqprio_mdq_allocated(struct otx2_nic *pfvf)
+{
+	return pfvf->hw.txschq_cnt[NIX_TXSCH_LVL_MDQ] != 0;
+}
+
+static int otx2_mqprio_restart_netdev(struct net_device *netdev, bool rate_limit);
+
+static void otx2_mqprio_apply_snap_netdev(struct net_device *netdev,
+					  const struct mq_offload_snap *snap)
+{
+	int tc;
+
+	if (!snap)
+		return;
+
+	netdev_set_num_tc(netdev, snap->num_tc);
+	for (tc = 0; tc < snap->num_tc; tc++)
+		netdev_set_tc_queue(netdev, tc, snap->count[tc],
+				    snap->offset[tc]);
+	for (tc = 0; tc < TC_QOPT_BITMASK + 1; tc++)
+		netdev_set_prio_tc_map(netdev, tc, snap->prio_tc_map[tc]);
+}
+
+static void otx2_mqprio_netdev_tc_work(struct work_struct *work)
+{
+	struct otx2_mqprio *mqprio = container_of(work, struct otx2_mqprio,
+						  netdev_tc_work);
+	struct otx2_nic *pfvf = container_of(mqprio, struct otx2_nic, mqprio);
+
+	if (!pfvf->mqprio.rate_limit || !pfvf->old_mq_snap)
+		return;
+
+	rtnl_lock();
+	otx2_mqprio_apply_snap_netdev(pfvf->netdev, pfvf->old_mq_snap);
+	rtnl_unlock();
+}
+
+static void otx2_mqprio_defer_netdev_tc_restore(struct otx2_nic *pfvf)
+{
+	schedule_work(&pfvf->mqprio.netdev_tc_work);
+}
+
+static int otx2_mqprio_restore_old(struct otx2_nic *pfvf)
+{
+	struct mq_offload_snap *snap = pfvf->old_mq_snap;
+	struct net_device *netdev = pfvf->netdev;
+	u16 num_txq = pfvf->hw.non_qos_queues;
+	int tc, txq, err;
+
+	if (!snap)
+		return 0;
+
+	err = otx2_mqprio_alloc_cache(pfvf, false);
+	if (err)
+		return err;
+
+	memset(pfvf->mqprio.min_rate, 0, num_txq * sizeof(*pfvf->mqprio.min_rate));
+	memset(pfvf->mqprio.max_rate, 0, num_txq * sizeof(*pfvf->mqprio.max_rate));
+	pfvf->mqprio.flags = snap->flags;
+
+	for (tc = 0; tc < snap->num_tc; tc++) {
+		u64 min_rate = snap->min_rate[tc];
+		u64 max_rate = snap->max_rate[tc];
+
+		for (txq = snap->offset[tc];
+		     txq < snap->offset[tc] + snap->count[tc]; txq++) {
+			pfvf->mqprio.min_rate[txq] = min_rate;
+			pfvf->mqprio.max_rate[txq] = max_rate;
+		}
+	}
+
+	otx2_mqprio_apply_snap_netdev(netdev, snap);
+
+	if (otx2_mqprio_mdq_allocated(pfvf)) {
+		err = otx2_nix_tm_clear_queue_shaper(pfvf);
+		if (err)
+			return err;
+	}
+
+	/* Rebuild the TX scheduler via netdev restart when running; otx2_mqprio_up()
+	 * alone is insufficient after a failed replace that already bounced the
+	 * interface. If open failed, TX schedulers were freed; defer shaper restore
+	 * to the next successful ndo_open() via otx2_mqprio_up().
+	 */
+	pfvf->mqprio.rate_limit = true;
+
+	if (netif_running(netdev)) {
+		err = otx2_mqprio_restart_netdev(netdev, true);
+		if (err)
+			return err;
+	} else if (pfvf->hw.txschq_cnt[NIX_TXSCH_LVL_SMQ]) {
+		err = otx2_mqprio_up(pfvf);
+		if (err)
+			return err;
+	}
+
+	otx2_mqprio_snap_free(pfvf, &pfvf->cur_mq_snap);
+
+	return 0;
+}
+
+static void otx2_mqprio_snap_destroy(struct otx2_nic *pfvf)
+{
+	otx2_mqprio_snap_free(pfvf, &pfvf->cur_mq_snap);
+	otx2_mqprio_snap_free(pfvf, &pfvf->old_mq_snap);
+}
+
+/* Offloaded mqprio replaced by software mqprio installs netdev TC layout in
+ * mqprio_init() before the old offload instance is destroyed during graft.
+ */
+static bool otx2_mqprio_keep_netdev_tc(struct otx2_nic *pfvf)
+{
+	struct Qdisc *qdisc = rtnl_dereference(pfvf->netdev->qdisc);
+
+	return qdisc && qdisc->ops && !strcmp(qdisc->ops->id, "mqprio");
+}
+
+static void otx2_mqprio_clear_sw(struct otx2_nic *pfvf)
+{
+	struct net_device *netdev = pfvf->netdev;
+
+	pfvf->mqprio.rate_limit = false;
+	otx2_mqprio_clear_replace_state(pfvf);
+	if (!otx2_mqprio_keep_netdev_tc(pfvf))
+		netdev_set_num_tc(netdev, 0);
+	otx2_mqprio_free_cache(pfvf);
+}
+
+/* Tear down mqprio bandwidth offload: clear per-queue shapers,
+ * mqprio_rate_limit, netdev TC mappings, and the cached rates.  Called on
+ * explicit mqprio teardown (tc qdisc del) and error cleanup, not on
+ * routine netdev stop/open cycles where the offload stays active.
+ */
+int otx2_mqprio_down(struct otx2_nic *pfvf)
+{
+	int err = 0;
+
+	if (!pfvf->mqprio.rate_limit)
+		return 0;
+
+	if (netif_running(pfvf->netdev) &&
+	    otx2_mqprio_mdq_allocated(pfvf))
+		err = otx2_nix_tm_clear_queue_shaper(pfvf);
+
+	if (err) {
+		netdev_warn(pfvf->netdev,
+			    "mqprio: failed to clear hardware shapers: %d; keeping offload state\n",
+			    err);
+		return err;
+	}
+
+	otx2_mqprio_clear_sw(pfvf);
+
+	return 0;
+}
+
+/* Restore cached mqprio MDQ shapers after ndo_open() reprograms the TX
+ * scheduler. Called from otx2_open() when bandwidth offload stays active
+ * across admin down/up or an mqprio netdev bounce.
+ *
+ * Returns an error if any shaper mailbox operation fails. otx2_open()
+ * fail-closes on that error: it aborts open and leaves the interface down
+ * rather than running with partial or missing bandwidth limits.
+ */
+int otx2_mqprio_up(struct otx2_nic *pfvf)
+{
+	struct net_device *netdev = pfvf->netdev;
+	int txq, err;
+
+	if (!pfvf->mqprio.rate_limit)
+		return 0;
+
+	if (!pfvf->mqprio.min_rate || !pfvf->mqprio.max_rate)
+		return 0;
+
+	for (txq = 0; txq < pfvf->hw.non_qos_queues; txq++) {
+		u64 min_rate = 0, max_rate = 0;
+
+		if (pfvf->mqprio.flags & TC_MQPRIO_F_MIN_RATE)
+			min_rate = pfvf->mqprio.min_rate[txq];
+		if (pfvf->mqprio.flags & TC_MQPRIO_F_MAX_RATE)
+			max_rate = pfvf->mqprio.max_rate[txq];
+
+		if (!min_rate && !max_rate)
+			continue;
+
+		err = otx2_nix_tm_set_queue_shaper(pfvf, txq, min_rate,
+						   max_rate);
+		if (err) {
+			netdev_err(netdev,
+				   "mqprio: failed to restore shaper for txq %d: %d\n",
+				   txq, err);
+			if (otx2_mqprio_mdq_allocated(pfvf) &&
+			    otx2_nix_tm_clear_queue_shaper(pfvf))
+				netdev_warn(netdev,
+					    "mqprio: failed to clear shapers after partial restore\n");
+			return err;
+		}
+	}
+
+	return 0;
+}
+
+/* Restart the netdev to reprogram the TX scheduler hierarchy for mqprio
+ * bandwidth offload.  Both mqprio add and delete (when offload was active)
+ * take this path via ndo_stop()/ndo_open() so VF-specific open logic (e.g.
+ * LBK carrier on) runs correctly.
+ *
+ * Intentional behaviour: this full stop/open cycle drops in-flight traffic
+ * (carrier off, IRQ/NAPI teardown, queue drain).  The NIX TX scheduler must
+ * be reallocated (e.g. one SMQ per non-QoS queue) and cannot be reprogrammed
+ * live today, so a netdev bounce is required on every mqprio add, replace,
+ * delete, and rollback.  Users see a brief connectivity blip; this is not a
+ * bug to "fix" without implementing the live-reprogramming path noted below.
+ * If open fails, the interface is left administratively down without calling
+ * ndo_stop() again on resources already torn down by the open error path.
+ *
+ * Do not call dev_deactivate()/dev_activate() here.  On replace,
+ * qdisc_graft() already deactivates qdiscs around offload teardown;
+ * dev_activate() from ndo_setup_tc() would republish qdiscs before graft
+ * completes and race __qdisc_run() on the old root qdisc.  After
+ * ndo_open(), carrier and TX queues are restored via otx2_handle_link_event()
+ * when link is up, same as otx2_change_mtu(), not via dev_activate().
+ *
+ * Clear __LINK_STATE_START before ndo_stop() so netif_running() is false
+ * for the duration of the bounce.
+ */
+static int otx2_mqprio_restart_netdev(struct net_device *netdev, bool rate_limit)
+{
+	struct otx2_nic *pfvf = netdev_priv(netdev);
+	const struct net_device_ops *ops = netdev->netdev_ops;
+	bool running = netif_running(netdev);
+	int err;
+
+	/* TODO: Explore live TX scheduler reprogramming to avoid a full
+	 * ndo_stop()/ndo_open() bounce on every mqprio change.
+	 */
+	netdev_dbg(netdev,
+		   "mqprio: restarting interface to reprogram TX scheduler; in-flight traffic will be dropped\n");
+
+	if (running) {
+		clear_bit(__LINK_STATE_START, &netdev->state);
+		smp_mb__after_atomic(); /* Commit netif_running(). */
+	}
+
+	err = ops->ndo_stop(netdev);
+	if (err) {
+		if (running)
+			set_bit(__LINK_STATE_START, &netdev->state);
+		return err;
+	}
+
+	/* Set before ndo_open() so otx2_txsch_alloc() widens SMQ allocation.
+	 * On teardown, drop mqprio software state so ndo_open() does not
+	 * re-apply bandwidth limits via otx2_mqprio_up() after the kernel
+	 * removed the qdisc.
+	 */
+	if (rate_limit)
+		pfvf->mqprio.rate_limit = true;
+	else
+		otx2_mqprio_clear_sw(pfvf);
+
+	err = ops->ndo_open(netdev);
+	if (!err && running) {
+		set_bit(__LINK_STATE_START, &netdev->state);
+	} else if (err) {
+		netdev_err(netdev,
+			   "Failed to restart device after mqprio change: %d\n",
+			   err);
+		/* ndo_open() already freed the TX schedulers on failure while
+		 * netif_running() may still be true; drop mqprio software state
+		 * only instead of sending shaper clears to freed queues.
+		 */
+		otx2_mqprio_clear_sw(pfvf);
+		/* ndo_open() rolls back on failure; mark the interface down so
+		 * netif_close() does not invoke ndo_stop() on freed NAPI/queue
+		 * state. Caller holds RTNL; dev_close() would deadlock.
+		 */
+		otx2_set_flag(pfvf, OTX2_FLAG_INTF_DOWN);
+		/* visible to otx2_stop() on other cpus */
+		smp_wmb();
+		netif_close(netdev);
+	}
+
+	return err;
+}
+
+static int otx2_mqprio_validate_tc_rate(struct net_device *netdev,
+					struct netlink_ext_ack *extack,
+					u64 rate, u32 qcount, int tc,
+					const char *name)
+{
+	if (!rate)
+		return 0;
+
+	if (qcount <= 1)
+		return 0;
+
+	/* TODO: per-TC TL4 shapers or equal per-queue MDQ split for multi-queue TC rates. */
+	netdev_err(netdev,
+		   "mqprio: %s rate for tc %d not supported with %u queues\n",
+		   name, tc, qcount);
+	NL_SET_ERR_MSG_FMT_MOD(extack,
+			       "mqprio: %s rate for tc %d not supported with %u queues",
+			       name, tc, qcount);
+	return -EOPNOTSUPP;
+}
+
+static int otx2_mqprio_validate_txqs(struct net_device *netdev,
+				     struct netlink_ext_ack *extack,
+				     struct tc_mqprio_qopt *qopt)
+{
+	struct otx2_nic *pfvf = netdev_priv(netdev);
+	u16 num_txq = pfvf->hw.non_qos_queues;
+	int tc, txq;
+
+	if (qopt->num_tc > num_txq) {
+		netdev_err(netdev, "Number of TCs (%u) exceeds hw queues %u\n",
+			   qopt->num_tc, num_txq);
+		NL_SET_ERR_MSG_FMT_MOD(extack,
+				       "Number of TCs (%u) exceeds hw queues %u",
+				       qopt->num_tc, num_txq);
+		return -EINVAL;
+	}
+
+	if (num_txq > MAX_TXSCHQ_PER_FUNC) {
+		netdev_err(netdev,
+			   "Number of queues (%u) exceeds max scheduler queues %u\n",
+			   num_txq, MAX_TXSCHQ_PER_FUNC);
+		NL_SET_ERR_MSG_FMT_MOD(extack,
+				       "Number of queues (%u) exceeds max scheduler queues %u",
+				       num_txq, MAX_TXSCHQ_PER_FUNC);
+		return -EINVAL;
+	}
+
+	for (tc = 0; tc < qopt->num_tc; tc++) {
+		u32 qcount = qopt->count[tc];
+
+		for (txq = qopt->offset[tc];
+		     txq < qopt->offset[tc] + qcount; txq++) {
+			if (txq >= num_txq) {
+				netdev_err(netdev,
+					   "mqprio: txq %d exceeds offload queue count %u\n",
+					   txq, num_txq);
+				NL_SET_ERR_MSG_FMT_MOD(extack,
+						       "mqprio: txq %d exceeds offload queue count %u",
+						       txq, num_txq);
+				return -EINVAL;
+			}
+		}
+	}
+
+	return 0;
+}
+
+static bool otx2_mqprio_rate_valid(struct otx2_nic *pfvf, u64 rate_bytes_ps)
+{
+	u64 mbps;
+
+	if (!rate_bytes_ps)
+		return true;
+
+	if (rate_bytes_ps < OTX2_MQPRIO_MIN_RATE_BYTES_PS)
+		return false;
+
+	if (rate_bytes_ps > otx2_mqprio_max_rate_bytes_ps(pfvf))
+		return false;
+
+	if (rate_bytes_ps > div_u64(U64_MAX, 8))
+		return false;
+
+	mbps = otx2_convert_rate(rate_bytes_ps);
+	return ilog2(mbps / 2) <= MAX_RATE_EXPONENT;
+}
+
+static int otx2_teardown_tc_mqprio(struct otx2_nic *pfvf,
+				   struct tc_mqprio_qopt_offload *mqprio)
+{
+	struct tc_mqprio_qopt *qopt = &mqprio->qopt;
+	bool had_mqprio = pfvf->mqprio.rate_limit;
+	struct net_device *netdev = pfvf->netdev;
+	bool if_up = netif_running(netdev);
+	int err;
+
+	qopt->hw = 0;
+
+	/* tc qdisc replace runs setup on the new mqprio before destroying the
+	 * old one. replace_setup_done and TC_ROOT_GRAFT distinguish stale
+	 * old-instance teardown from graft failure after setup.
+	 */
+	if (pfvf->mqprio.replace_setup_done && pfvf->cur_mq_snap) {
+		err = 0;
+		if (pfvf->mqprio.replace_graft_done)
+			otx2_mqprio_snap_commit(pfvf);
+		else
+			err = otx2_mqprio_restore_old(pfvf);
+		otx2_mqprio_clear_replace_state(pfvf);
+		return err;
+	}
+
+	/* Skip the netdev restart when mqprio offload was not active. */
+	if (!had_mqprio)
+		return 0;
+
+	if (if_up) {
+		err = otx2_mqprio_down(pfvf);
+		if (err)
+			return err;
+
+		return otx2_mqprio_restart_netdev(netdev, false);
+	}
+
+	/* ndo_stop() already freed the TX scheduler TL nodes; drop software
+	 * state only.
+	 */
+	otx2_mqprio_clear_sw(pfvf);
+	return 0;
+}
+
+static int otx2_setup_tc_mqprio(struct net_device *netdev,
+				struct tc_mqprio_qopt_offload *mqprio)
+{
+	struct netlink_ext_ack *extack = mqprio->extack;
+	struct otx2_nic *pfvf = netdev_priv(netdev);
+	struct tc_mqprio_qopt *qopt = &mqprio->qopt;
+	bool replacing = pfvf->mqprio.rate_limit;
+	bool if_up = netif_running(netdev);
+	int tc, txq, err, i;
+
+	if (!qopt->hw)
+		return otx2_teardown_tc_mqprio(pfvf, mqprio);
+
+	if (!if_up) {
+		netdev_err(netdev, "mqprio: setup requires interface UP\n");
+		NL_SET_ERR_MSG_MOD(extack, "mqprio: setup requires interface UP");
+		return -EOPNOTSUPP;
+	}
+
+	if (mqprio->shaper != TC_MQPRIO_SHAPER_BW_RATE) {
+		netdev_err(netdev, "Unsupported mqprio shaper %#x\n", mqprio->shaper);
+		NL_SET_ERR_MSG_FMT_MOD(extack, "Unsupported mqprio shaper %#x",
+				       mqprio->shaper);
+		return -EOPNOTSUPP;
+	}
+
+	if (!test_bit(QOS_CIR_PIR_SUPPORT, &pfvf->hw.cap_flag)) {
+		netdev_err(netdev,
+			   "mqprio: bandwidth offload requires CIR+PIR support\n");
+		NL_SET_ERR_MSG_MOD(extack,
+				   "mqprio: bandwidth offload requires CIR+PIR support");
+		return -EOPNOTSUPP;
+	}
+
+	if (is_otx2_sdp_rep(pfvf->pdev)) {
+		netdev_err(netdev, "mqprio: bandwidth offload not supported on SDP rep\n");
+		NL_SET_ERR_MSG_MOD(extack,
+				   "mqprio: bandwidth offload not supported on SDP rep");
+		return -EOPNOTSUPP;
+	}
+
+	if (pfvf->pfc_en) {
+		netdev_err(netdev,
+			   "mqprio: cannot enable offload while PFC is enabled\n");
+		NL_SET_ERR_MSG_MOD(extack,
+				   "mqprio: cannot enable offload while PFC is enabled");
+		return -EOPNOTSUPP;
+	}
+
+	if (pfvf->xdp_prog) {
+		netdev_err(netdev,
+			   "mqprio: cannot enable offload while XDP is active\n");
+		NL_SET_ERR_MSG_MOD(extack,
+				   "mqprio: cannot enable offload while XDP is active");
+		return -EOPNOTSUPP;
+	}
+
+	if (!list_empty(&pfvf->qos.qos_tree)) {
+		netdev_err(netdev,
+			   "mqprio: cannot enable offload while HTB is active\n");
+		NL_SET_ERR_MSG_MOD(extack,
+				   "mqprio: cannot enable offload while HTB is active");
+		return -EOPNOTSUPP;
+	}
+
+	for (tc = 0; tc < qopt->num_tc; tc++) {
+		u64 min_rate = 0, max_rate = 0;
+		u32 qcount = qopt->count[tc];
+
+		if (mqprio->flags & TC_MQPRIO_F_MIN_RATE)
+			min_rate = mqprio->min_rate[tc];
+		if (mqprio->flags & TC_MQPRIO_F_MAX_RATE)
+			max_rate = mqprio->max_rate[tc];
+
+		if (min_rate && max_rate && min_rate > max_rate) {
+			netdev_err(netdev,
+				   "min_rate %llu exceeds max_rate %llu for tc %d\n",
+				   min_rate, max_rate, tc);
+			NL_SET_ERR_MSG_FMT_MOD(extack,
+					       "min_rate %llu exceeds max_rate %llu for tc %d",
+					       min_rate, max_rate, tc);
+			return -EINVAL;
+		}
+
+		if (mqprio->flags & TC_MQPRIO_F_MIN_RATE) {
+			err = otx2_mqprio_validate_tc_rate(netdev, extack, min_rate,
+							   qcount, tc, "min");
+			if (err)
+				return err;
+		}
+
+		if (mqprio->flags & TC_MQPRIO_F_MAX_RATE) {
+			err = otx2_mqprio_validate_tc_rate(netdev, extack, max_rate,
+							   qcount, tc, "max");
+			if (err)
+				return err;
+		}
+
+		if (mqprio->flags & TC_MQPRIO_F_MIN_RATE &&
+		    !otx2_mqprio_rate_valid(pfvf, min_rate)) {
+			netdev_err(netdev,
+				   "mqprio: min_rate %llu for tc %d is outside hardware limits\n",
+				   min_rate, tc);
+			NL_SET_ERR_MSG_FMT_MOD(extack,
+					       "mqprio: min_rate %llu for tc %d is outside hardware limits",
+					       min_rate, tc);
+			return -EINVAL;
+		}
+
+		if (mqprio->flags & TC_MQPRIO_F_MAX_RATE &&
+		    !otx2_mqprio_rate_valid(pfvf, max_rate)) {
+			netdev_err(netdev,
+				   "mqprio: max_rate %llu for tc %d is outside hardware limits\n",
+				   max_rate, tc);
+			NL_SET_ERR_MSG_FMT_MOD(extack,
+					       "mqprio: max_rate %llu for tc %d is outside hardware limits",
+					       max_rate, tc);
+			return -EINVAL;
+		}
+	}
+
+	err = otx2_mqprio_validate_txqs(netdev, extack, qopt);
+	if (err)
+		return err;
+
+	err = otx2_mqprio_stage_cur(pfvf, mqprio);
+	if (err)
+		return err;
+
+	err = otx2_mqprio_restart_netdev(pfvf->netdev, true);
+	if (err)
+		goto cleanup;
+
+	err = otx2_mqprio_alloc_cache(pfvf, replacing);
+	if (err)
+		goto cleanup;
+
+	/* otx2_mqprio_up() may have restored the previous configuration during
+	 * the restart above. Clear every MDQ shaper before applying the new
+	 * mapping so queues dropped from the TC layout do not keep stale
+	 * limits in hardware.
+	 */
+	if (otx2_mqprio_mdq_allocated(pfvf)) {
+		err = otx2_nix_tm_clear_queue_shaper(pfvf);
+		if (err)
+			goto cleanup;
+	}
+
+	pfvf->mqprio.flags = mqprio->flags;
+
+	for (tc = 0; tc < qopt->num_tc; tc++) {
+		u64 min_rate = 0, max_rate = 0;
+		u32 qcount = qopt->count[tc];
+
+		/* Rates omitted from tc mqprio are passed as zero and both MDQ
+		 * shaper registers are programmed; see
+		 * otx2_nix_tm_set_queue_shaper().
+		 * TODO: multi-queue TC rates need per-queue split; see
+		 * otx2_mqprio_validate_tc_rate().
+		 */
+		if (mqprio->flags & TC_MQPRIO_F_MIN_RATE)
+			min_rate = mqprio->min_rate[tc];
+		if (mqprio->flags & TC_MQPRIO_F_MAX_RATE)
+			max_rate = mqprio->max_rate[tc];
+
+		for (txq = qopt->offset[tc];
+		     txq < qopt->offset[tc] + qcount; txq++) {
+			netdev_dbg(netdev,
+				   "mqprio: tc %d txq %d min_rate %llu max_rate %llu\n",
+				   tc, txq, min_rate, max_rate);
+
+			pfvf->mqprio.min_rate[txq] = min_rate;
+			pfvf->mqprio.max_rate[txq] = max_rate;
+
+			err = otx2_nix_tm_set_queue_shaper(pfvf, txq,
+							   min_rate, max_rate);
+			if (err)
+				goto cleanup;
+		}
+	}
+
+	netdev_set_num_tc(netdev, pfvf->cur_mq_snap->num_tc);
+	for (i = 0; i < pfvf->cur_mq_snap->num_tc; i++)
+		netdev_set_tc_queue(netdev, i, pfvf->cur_mq_snap->count[i],
+				    qopt->offset[i]);
+
+	qopt->hw = TC_MQPRIO_HW_OFFLOAD_TCS;
+
+	if (replacing) {
+		pfvf->mqprio.replace_setup_done = true;
+		pfvf->mqprio.replace_graft_done = false;
+	} else {
+		otx2_mqprio_snap_commit(pfvf);
+	}
+
+	return 0;
+
+cleanup:
+	qopt->hw = 0;
+	if (replacing) {
+		int restore_err = otx2_mqprio_restore_old(pfvf);
+
+		otx2_mqprio_clear_replace_state(pfvf);
+		if (restore_err) {
+			netdev_err(netdev,
+				   "mqprio: replace failed and prior configuration rollback failed: %d\n",
+				   restore_err);
+			if (extack)
+				NL_SET_ERR_MSG_FMT_MOD(extack,
+						       "mqprio: replace failed and prior configuration rollback failed: %d",
+						       restore_err);
+		} else {
+			netdev_err(netdev,
+				   "mqprio: replace failed; prior configuration restored\n");
+			if (extack)
+				NL_SET_ERR_MSG_MOD(extack,
+						   "mqprio: replace failed; prior configuration restored");
+			/* Failed replace destroys the new qdisc with hw_offload
+			 * unset, so mqprio_destroy() clears netdev TC after we
+			 * return. Re-apply the restored layout once that unwind
+			 * finishes.
+			 */
+			otx2_mqprio_defer_netdev_tc_restore(pfvf);
+		}
+		return err ? err : -EIO;
+	}
+	otx2_mqprio_snap_free(pfvf, &pfvf->cur_mq_snap);
+	otx2_teardown_tc_mqprio(pfvf, mqprio);
+	return err;
+}
+
+static int otx2_setup_tc_root(struct otx2_nic *pfvf,
+			      struct tc_root_qopt_offload *root)
+{
+	switch (root->command) {
+	case TC_ROOT_GRAFT:
+		if (pfvf->mqprio.replace_setup_done)
+			pfvf->mqprio.replace_graft_done = true;
+		return 0;
+	default:
+		return -EOPNOTSUPP;
+	}
+}
+
+static int otx2_setup_tc_query_caps(void *type_data)
+{
+	struct tc_query_caps_base *base = type_data;
+	struct tc_mqprio_caps *caps;
+
+	if (base->type != TC_SETUP_QDISC_MQPRIO)
+		return -EOPNOTSUPP;
+
+	caps = base->caps;
+	caps->validate_queue_counts = true;
+
+	return 0;
+}
+
 int otx2_setup_tc(struct net_device *netdev, enum tc_setup_type type,
 		  void *type_data)
 {
 	switch (type) {
+	case TC_QUERY_CAPS:
+		return otx2_setup_tc_query_caps(type_data);
 	case TC_SETUP_BLOCK:
 		return otx2_setup_tc_block(netdev, type_data);
 	case TC_SETUP_QDISC_HTB:
 		return otx2_setup_tc_htb(netdev, type_data);
+	case TC_SETUP_QDISC_MQPRIO:
+		return otx2_setup_tc_mqprio(netdev, type_data);
+	case TC_SETUP_ROOT_QDISC:
+		return otx2_setup_tc_root(netdev_priv(netdev), type_data);
 	default:
 		return -EOPNOTSUPP;
 	}
@@ -1625,13 +2433,17 @@ int otx2_init_tc(struct otx2_nic *nic)
 		return -EINVAL;
 	}
 
+	INIT_WORK(&nic->mqprio.netdev_tc_work, otx2_mqprio_netdev_tc_work);
+
 	return 0;
 }
 EXPORT_SYMBOL(otx2_init_tc);
 
 void otx2_shutdown_tc(struct otx2_nic *nic)
 {
+	cancel_work_sync(&nic->mqprio.netdev_tc_work);
 	otx2_destroy_tc_flow_list(nic);
+	otx2_mqprio_snap_destroy(nic);
 }
 EXPORT_SYMBOL(otx2_shutdown_tc);
 
diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/qos.c b/drivers/net/ethernet/marvell/octeontx2/nic/qos.c
index f160b1618efa..9ef55a6db50b 100644
--- a/drivers/net/ethernet/marvell/octeontx2/nic/qos.c
+++ b/drivers/net/ethernet/marvell/octeontx2/nic/qos.c
@@ -118,6 +118,9 @@ static void otx2_config_sched_shaping(struct otx2_nic *pfvf,
 	/* configure PIR */
 	maxrate = (node->rate > node->ceil) ? node->rate : node->ceil;
 
+	/* 65536 is the kernel-side default burst when HTB does not supply an
+	 * explicit value, not the NIX hardware maximum (CN10K_MAX_BURST_SIZE).
+	 */
 	cfg->regval[*num_regs] =
 		otx2_get_txschq_rate_regval(pfvf, maxrate, 65536);
 	(*num_regs)++;
@@ -1088,6 +1091,14 @@ static int otx2_qos_root_add(struct otx2_nic *pfvf, u16 htb_maj_id, u16 htb_defc
 		   "TC_HTB_CREATE: handle=0x%x defcls=0x%x\n",
 		   htb_maj_id, htb_defcls);
 
+	if (pfvf->mqprio.rate_limit) {
+		netdev_err(pfvf->netdev,
+			   "HTB: cannot enable while mqprio bandwidth offload is active\n");
+		NL_SET_ERR_MSG_MOD(extack,
+				   "HTB: cannot enable while mqprio bandwidth offload is active");
+		return -EOPNOTSUPP;
+	}
+
 	root = otx2_qos_alloc_root(pfvf);
 	if (IS_ERR(root)) {
 		err = PTR_ERR(root);
-- 
2.43.0


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v16 net-next 1/2] octeontx2: use atomic bitops for PF/VF and rep flags
  2026-09-18  1:59 ` [PATCH v16 net-next 1/2] octeontx2: use atomic bitops for PF/VF and rep flags Ratheesh Kannoth
@ 2026-09-22  2:19   ` netdev-bot+sashiko
  0 siblings, 0 replies; 5+ messages in thread
From: netdev-bot+sashiko @ 2026-09-22  2:19 UTC (permalink / raw)
  To: rkannoth
  Cc: bpf, linux-kernel, netdev, andrew+netdev, ast, daniel, davem,
	edumazet, hawk, john.fastabend, kuba, pabeni, sdf, sgoutham

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

Critical: 0 (1) · High: 0 (4) · Medium: 2 (3) · 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] The changelog misdescribes the patch.
- [Medium] The new helper otx2_sync_flags_from_rep() replaces the single-
  word store `priv->flags = rep->flags;` with a loop of 22 independent…

Pre-existing issues:
- [Critical] otx2_sync_flags_from_rep() copies the *entire* flag
  namespace (bits 0..OTX2_FLAG_MAX-1) from a representor's…
- [High] rvu_rep_mcam_flow_init() unconditionally allocates rep->flow_cfg
  (kzalloc_objs) and rep->flow_cfg->flow_ent (kcalloc) on every…
- [High] rvu_rep_destroy() dereferences the representor structure after
  the memory holding it has been freed: `rep` is obtained from…
- [High] rvu_rep_get_stats64() re-arms the per-representor delayed work
  `rep->stats_wrk` (100 ms) on every statistics read, but nothing…
- [High] rvu_rep_mcam_flow_init() calls `mutex_unlock(&priv->mbox.lock)`
  at its exit label but never acquires that mutex anywhere in the…
- [Medium] In otx2_tc_del_flow() the TC-mark bookkeeping is inverted: `if
  (!refcount_dec_and_test(&flow_cfg->mark_flows)) otx2_clear_flag(nic,…
- [Medium] In otx2_config_hwtstamp_set(), OTX2_FLAG_PTP_ONESTEP_SYNC is
  only cleared in the HWTSTAMP_TX_OFF case.
- [Medium] otx2_config_hwtstamp_set() ignores the return values of
  otx2_config_hw_rx_tstamp() and otx2_config_hw_tx_tstamp().

--

octeontx2: use atomic bitops for PF/VF and rep flags

This converts struct otx2_nic::flags and struct rep_dev::flags from u64
bitmask constants to an unsigned long bitmap driven by set_bit()/
clear_bit()/test_bit(), adds otx2_set_flag()/otx2_clear_flag()/
otx2_test_flag() wrappers, and introduces otx2_sync_flags_from_rep() to
copy representor state into the shared PF mailbox context.  The
representor "VF initialized" bit moves out of rep.h into the shared
enum.

No Link: tags in the commit header.

A few questions about the changelog and about some paths this touches.

> Replace non-atomic u64 flag read-modify-write with unsigned long
> bitmaps and set_bit/clear_bit/test_bit access across the NIC driver.

[Severity: Medium]

Which concurrent writer pairs are being fixed here?  The changelog states
a race motivation but no symptom, no reproducer and no Fixes: tag, so it
is hard to tell whether this is a bug fix or a net-next cleanup.

> Define representor VF initialization as OTX2_REP_VF_INITIALIZED (bit 21)
> in the shared flag namespace.

The enum added to otx2_common.h spells this OTX2_FLAG_REP_VF_INITIALIZED,
not OTX2_REP_VF_INITIALIZED.

Moving this bit also looks like it silently fixes a real collision.  In
the baseline, rep.h had:

	#define RVU_REP_VF_INITIALIZED		BIT_ULL(0)

and otx2_common.h had:

	#define OTX2_FLAG_RX_TSTAMP_ENABLED	BIT_ULL(0)

while rvu_rep_setup_tc_cb() did:

	priv->flags = rep->flags;

So a representor whose VF was reported initialized turned on
OTX2_FLAG_RX_TSTAMP_ENABLED on the representor PF context, and
otx2_set_rxtstamp() then does:

	timestamp = pfvf->ptp->convert_rx_ptp_tstmp(*(u64 *)data);

rvu_rep_probe() never calls otx2_ptp_init(), so pfvf->ptp is NULL there.
Should the bit relocation be split into its own patch with a Fixes: tag
so it can be picked up for stable?

> diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.h b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.h
> index 5850bc1870a1..90cf302bbe6d 100644
> --- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.h
> +++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.h

[ ... ]

> @@ -593,6 +598,34 @@ struct otx2_nic {
>  	unsigned long		*af_xdp_zc_qidx;
>  };
>  
> +static inline void otx2_set_flag(struct otx2_nic *nic, unsigned int flag)
> +{
> +	set_bit(flag, &nic->flags);
> +}
> +
> +static inline void otx2_clear_flag(struct otx2_nic *nic, unsigned int flag)
> +{
> +	clear_bit(flag, &nic->flags);
> +}
> +
> +static inline bool otx2_test_flag(struct otx2_nic *nic, unsigned int flag)
> +{
> +	return test_bit(flag, &nic->flags);
> +}
> +
> +static inline void otx2_sync_flags_from_rep(struct otx2_nic *dst,
> +					    unsigned long *src_flags)
> +{
> +	unsigned int flag;
> +
> +	for (flag = 0; flag < OTX2_FLAG_MAX; flag++) {
> +		if (test_bit(flag, src_flags))
> +			set_bit(flag, &dst->flags);
> +		else
> +			clear_bit(flag, &dst->flags);
> +	}
> +}
> +

[Severity: Medium]

Can readers now observe intermediate states that never existed before?
The previous code published the whole word with a single store:

	priv->flags = rep->flags;

This loop turns that into 22 independent read-modify-write operations on
a word that is read locklessly from softirq context in otx2_napi_handler(),
otx2_rcv_pkt_handler() and otx2_tx_napi_handler().  Per-bit atomics do
not give an atomic snapshot of the word.

otx2_tx_napi_handler() samples the same bit twice:

	if (otx2_test_flag(pfvf, OTX2_FLAG_REP_MODE_ENABLED))
		ndev = pfvf->reps[qidx]->netdev;
	...
		if (otx2_test_flag(pfvf, OTX2_FLAG_REP_MODE_ENABLED))
			qidx = 0;
		txq = netdev_get_tx_queue(ndev, qidx);

If the bit changes between the two reads, ndev and the qidx clamp
disagree.

The loop also reads src_flags one bit at a time while
rvu_rep_state_evt_handler() concurrently does
set_bit()/clear_bit(OTX2_FLAG_REP_VF_INITIALIZED, &rep->flags) from the
mailbox up-notify handler, so the copied state can straddle a transition.

Would composing the value in a local unsigned long and publishing it with
a single store (or serialising the word against the NAPI readers) be
better here?

> diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c
> index c0e2100de1d9..32582b6347ea 100644
> --- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c
> +++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c

[ ... ]

> @@ -2478,9 +2478,9 @@ static int otx2_config_hw_rx_tstamp(struct otx2_nic *pfvf, bool enable)
>  
>  	mutex_unlock(&pfvf->mbox.lock);
>  	if (enable)
> -		pfvf->flags |= OTX2_FLAG_RX_TSTAMP_ENABLED;
> +		otx2_set_flag(pfvf, OTX2_FLAG_RX_TSTAMP_ENABLED);
>  	else
> -		pfvf->flags &= ~OTX2_FLAG_RX_TSTAMP_ENABLED;
> +		otx2_clear_flag(pfvf, OTX2_FLAG_RX_TSTAMP_ENABLED);
>  	return 0;
>  }

[Severity: Medium]

This is a pre-existing issue and not something this patch introduces, but
can the flag and the saved configuration get out of sync here?

otx2_config_hw_rx_tstamp() (and otx2_config_hw_tx_tstamp()) return early
on mailbox failure, before reaching these lines:

	req = otx2_mbox_alloc_msg_cgx_ptp_rx_enable(&pfvf->mbox);
	...
	if (!req) {
		mutex_unlock(&pfvf->mbox.lock);
		return -ENOMEM;
	}

	err = otx2_sync_mbox_msg(&pfvf->mbox);
	if (err) {
		mutex_unlock(&pfvf->mbox.lock);
		return err;
	}

otx2_config_hwtstamp_set() discards those return values, then does:

	pfvf->tstamp = *config;

	return 0;

so ndo_hwtstamp_get() reports timestamping enabled while
otx2_set_rxtstamp()/otx2_set_txtstamp() still see the flag clear.  Should
the errors be propagated?

[ ... ]

> @@ -2549,7 +2549,7 @@ int otx2_config_hwtstamp_set(struct net_device *netdev,
>  					   "One-step time stamping is not supported");
>  			return -ERANGE;
>  		}
> -		pfvf->flags |= OTX2_FLAG_PTP_ONESTEP_SYNC;
> +		otx2_set_flag(pfvf, OTX2_FLAG_PTP_ONESTEP_SYNC);
>  		schedule_delayed_work(&pfvf->ptp->synctstamp_work,
>  				      msecs_to_jiffies(500));
>  		fallthrough;

[Severity: Medium]

This is also pre-existing rather than introduced here, but is
OTX2_FLAG_PTP_ONESTEP_SYNC left stale on a direct transition from
HWTSTAMP_TX_ONESTEP_SYNC to HWTSTAMP_TX_ON?

The flag is only cleared in the HWTSTAMP_TX_OFF case, and a second
SIOCSHWTSTAMP with tx_type = HWTSTAMP_TX_ON enters the shared
case HWTSTAMP_TX_ON: directly.  pfvf->tstamp records the new request,
but otx2_set_txtstamp() still takes the one-step branch:

	if (unlikely(otx2_test_flag(pfvf, OTX2_FLAG_PTP_ONESTEP_SYNC) &&
		     otx2_ptp_is_sync(skb, &ptp_offset, &udp_csum_crt))) {

so Sync packets keep getting originTimestamp/correction rewritten and the
SKBTX_IN_PROGRESS path is skipped.

[ ... ]

> diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_tc.c b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_tc.c
> index 039fd47ebf52..ddb46b580c3b 100644
> --- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_tc.c
> +++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_tc.c

[ ... ]

> @@ -1195,12 +1195,12 @@ static int otx2_tc_del_flow(struct otx2_nic *nic,
>  	/* Disable TC MARK flag if they are no rules with skbedit mark action */
>  	if (flow_node->req.match_id)
>  		if (!refcount_dec_and_test(&flow_cfg->mark_flows))
> -			nic->flags &= ~OTX2_FLAG_TC_MARK_ENABLED;
> +			otx2_clear_flag(nic, OTX2_FLAG_TC_MARK_ENABLED);

[Severity: Medium]

This isn't a bug introduced by this patch, but is the test inverted?

mark_flows starts at the sentinel value 1 in otx2_mcam_entry_init():

	refcount_set(&flow_cfg->mark_flows, 1);

and otx2_tc_parse_actions() does refcount_inc() per marked rule.  With two
marked rules installed the counter is 3; deleting one leaves 2,
refcount_dec_and_test() returns false, and the negation therefore clears
OTX2_FLAG_TC_MARK_ENABLED while a marked rule is still installed.
otx2_rcv_pkt_handler() then stops doing:

	if (otx2_test_flag(pfvf, OTX2_FLAG_TC_MARK_ENABLED))
		skb->mark = parse->match_id;

for the surviving rule.

[ ... ]

> diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/rep.c b/drivers/net/ethernet/marvell/octeontx2/nic/rep.c
> index 0f5d5642d3f7..7df82c22cc12 100644
> --- a/drivers/net/ethernet/marvell/octeontx2/nic/rep.c
> +++ b/drivers/net/ethernet/marvell/octeontx2/nic/rep.c
> @@ -93,9 +93,9 @@ static int rvu_rep_mcam_flow_init(struct rep_dev *rep)
>  	rep->flow_cfg->max_flows = allocated;
>  
>  	if (allocated) {
> -		rep->flags |= OTX2_FLAG_MCAM_ENTRIES_ALLOC;
> -		rep->flags |= OTX2_FLAG_NTUPLE_SUPPORT;
> -		rep->flags |= OTX2_FLAG_TC_FLOWER_SUPPORT;
> +		set_bit(OTX2_FLAG_MCAM_ENTRIES_ALLOC, &rep->flags);
> +		set_bit(OTX2_FLAG_NTUPLE_SUPPORT, &rep->flags);
> +		set_bit(OTX2_FLAG_TC_FLOWER_SUPPORT, &rep->flags);
>  	}

[Severity: High]

This is pre-existing and not caused by this patch, but can repeated tc
commands leak rep->flow_cfg and rep->flow_cfg->flow_ent without bound?

The three set_bit() calls only run when allocated is non-zero, and
OTX2_FLAG_TC_FLOWER_SUPPORT is the only gate against re-entry:

rvu_rep_setup_tc_cb()
	if (!test_bit(OTX2_FLAG_TC_FLOWER_SUPPORT, &rep->flags))
		rvu_rep_mcam_flow_init(rep);

If the AF hands out zero entries, or the mbox alloc/sync fails and takes
one of the goto exit paths, the flag stays clear and every later tc
command re-enters:

rvu_rep_mcam_flow_init()
	rep->flow_cfg = kzalloc_objs(struct otx2_flow_config, 1);
	...
	rep->flow_cfg->flow_ent = kcalloc(count, sizeof(u16), GFP_KERNEL);

overwriting the previous pair with no unwind.  The function also returns 0
on that path and the caller ignores the return value.

Separately, rvu_rep_destroy() only does kfree(rep->flow_cfg), so
rep->flow_cfg->flow_ent looks like it is leaked for every representor at
teardown.

[Severity: High]

Still pre-existing rather than new here, but does this function unlock a
mutex it never took?  A few lines above these set_bit() calls:

	if (allocated)
		sort(&rep->flow_cfg->flow_ent[0], allocated,
		     sizeof(rep->flow_cfg->flow_ent[0]), mcam_entry_cmp, NULL);

	mutex_unlock(&priv->mbox.lock);

There is no mutex_lock(&priv->mbox.lock) anywhere in
rvu_rep_mcam_flow_init(), and rvu_rep_setup_tc_cb() does not hold it
either.  The mailbox traffic in the loop above
(otx2_mbox_alloc_msg_npc_mcam_alloc_entry(), otx2_sync_mbox_msg(),
otx2_mbox_get_rsp()) also runs without the lock, unlike
otx2_alloc_mcam_entries(), rvu_rep_notify_pfvf(), rvu_rep_get_stats() and
rvu_eswitch_config(), which all bracket alloc/sync with the mutex.  Can
this unlock break the critical section of whichever thread legitimately
owns the mailbox?

> @@ -109,14 +109,14 @@ static int rvu_rep_setup_tc_cb(enum tc_setup_type type,
>  	struct rep_dev *rep = cb_priv;
>  	struct otx2_nic *priv = rep->mdev;
>  
> -	if (!(rep->flags & RVU_REP_VF_INITIALIZED))
> +	if (!test_bit(OTX2_FLAG_REP_VF_INITIALIZED, &rep->flags))
>  		return -EINVAL;
>  
> -	if (!(rep->flags & OTX2_FLAG_TC_FLOWER_SUPPORT))
> +	if (!test_bit(OTX2_FLAG_TC_FLOWER_SUPPORT, &rep->flags))
>  		rvu_rep_mcam_flow_init(rep);
>  
>  	priv->netdev = rep->netdev;
> -	priv->flags = rep->flags;
> +	otx2_sync_flags_from_rep(priv, &rep->flags);
>  	priv->pcifunc = rep->pcifunc;
>  	priv->flow_cfg = rep->flow_cfg;

[Severity: Critical]

This isn't a regression introduced by this patch, since the old
priv->flags = rep->flags had the same effect, but can this write past the
representor netdev's _tx[] array?

rep->flags only ever carries MCAM_ENTRIES_ALLOC / NTUPLE_SUPPORT /
TC_FLOWER_SUPPORT (from rvu_rep_mcam_flow_init()) and
REP_VF_INITIALIZED (from rvu_rep_state_evt_handler()).  The helper's else
branch therefore clears every PF-owned bit on the live datapath context,
including OTX2_FLAG_REP_MODE_ENABLED, which is set once in
rvu_rep_probe() and never restored:

rvu_rep_probe()
	otx2_set_flag(priv, OTX2_FLAG_INTF_DOWN);
	otx2_set_flag(priv, OTX2_FLAG_REP_MODE_ENABLED);

With that bit cleared, the next TX completion takes the non-rep branch in
otx2_tx_napi_handler() while priv->netdev is the representor netdev that
this function just installed:

	ndev = pfvf->netdev;
	...
		if (otx2_test_flag(pfvf, OTX2_FLAG_REP_MODE_ENABLED))
			qidx = 0;
		txq = netdev_get_tx_queue(ndev, qidx);
		netdev_tx_completed_queue(txq, tx_pkts, tx_bytes);

Representors are allocated with alloc_etherdev(sizeof(*rep)) in
rvu_rep_create(), so num_tx_queues is 1, while qidx here runs up to
rep_cnt - 1.  netdev_get_tx_queue() only warns under DEBUG_NET and still
returns the pointer:

	DEBUG_NET_WARN_ON_ONCE(index >= dev->num_tx_queues);
	return &dev->_tx[index];

so netdev_tx_completed_queue() and netif_tx_wake_queue() then read and
write BQL state past the end of _tx[].

Clearing OTX2_FLAG_INTF_DOWN in the same loop also defeats the teardown
gating in rvu_rep_remove():

	if (!otx2_test_flag(priv, OTX2_FLAG_INTF_DOWN))
		rvu_rep_destroy(priv);

and clearing OTX2_FLAG_TC_MARK_ENABLED silently disables skb->mark for
already installed skbedit-mark rules.  Should the sync be restricted to
the bits a representor actually owns?

[ ... ]

> @@ -382,7 +382,7 @@ static void rvu_rep_get_stats64(struct net_device *dev,
>  {
>  	struct rep_dev *rep = netdev_priv(dev);
>  
> -	if (!(rep->flags & RVU_REP_VF_INITIALIZED))
> +	if (!test_bit(OTX2_FLAG_REP_VF_INITIALIZED, &rep->flags))
>  		return;
>  
>  	stats->rx_packets = rep->stats.rx_frames;

[Severity: High]

This is a pre-existing issue rather than one added here, but can
rep->stats_wrk still be queued after the representor is freed?

The tail of this function re-arms the work on every statistics read:

	schedule_delayed_work(&rep->stats_wrk, msecs_to_jiffies(100));

and the worker dereferences the containing rep_dev plus the shared
mailbox:

rvu_rep_get_stats()
	rep = container_of(del_work, struct rep_dev, stats_wrk);
	priv = rep->mdev;

	mutex_lock(&priv->mbox.lock);

stats_wrk lives in the netdev private area, but nothing cancels it:
rvu_rep_destroy() free_netdev()s each representor, rvu_rep_free_cq_rsrc()
only synchronises NAPI and IRQs, and rvu_rep_rsrc_free() cancels only
pool_refill_work.  Is a cancel_delayed_work_sync(&rep->stats_wrk) missing
before free_netdev()?

> @@ -632,7 +632,7 @@ void rvu_rep_destroy(struct otx2_nic *priv)
>  	int rep_id;
>  
>  	rvu_eswitch_config(priv, false);
> -	priv->flags |= OTX2_FLAG_INTF_DOWN;
> +	otx2_set_flag(priv, OTX2_FLAG_INTF_DOWN);
>  	rvu_rep_free_cq_rsrc(priv);
>  	for (rep_id = 0; rep_id < priv->rep_cnt; rep_id++) {
>  		rep = priv->reps[rep_id];

[Severity: High]

This ordering is pre-existing and not changed by this patch, but does the
loop below read rep after the memory holding it is freed?

	free_netdev(rep->netdev);
	kfree(rep->flow_cfg);

rep_dev is the netdev private area:

rvu_rep_create()
	ndev = alloc_etherdev(sizeof(*rep));
	...
	rep = netdev_priv(ndev);

so free_netdev() releases the allocation that contains rep, and the next
statement reads rep->flow_cfg out of it and hands whatever it finds to
kfree().  Should flow_cfg be freed before free_netdev()?

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260918015906.1255204-1-rkannoth%40marvell.com

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v16 net-next 2/2] octeontx2-pf: add mqprio bandwidth offload for NIX TX schedulers
  2026-09-18  1:59 ` [PATCH v16 net-next 2/2] octeontx2-pf: add mqprio bandwidth offload for NIX TX schedulers Ratheesh Kannoth
@ 2026-09-22  2:19   ` netdev-bot+sashiko
  0 siblings, 0 replies; 5+ messages in thread
From: netdev-bot+sashiko @ 2026-09-22  2:19 UTC (permalink / raw)
  To: rkannoth
  Cc: bpf, linux-kernel, netdev, andrew+netdev, ast, daniel, davem,
	edumazet, hawk, john.fastabend, kuba, pabeni, sdf, sgoutham

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

Critical: 0 · High: 7 · Medium: 9 · Low: 1

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).

- [High] otx2_mqprio_restart_netdev() manipulates the core-owned
  __LINK_STATE_START bit directly (clear_bit()/set_bit()) to fake…
- [High] When otx2_nix_tm_clear_queue_shaper() fails, otx2_mqprio_down()
  deliberately returns the error and keeps pfvf->mqprio.rate_limit =…
- [High] otx2_mqprio_netdev_tc_work() reads pfvf->mqprio.rate_limit and
  pfvf->old_mq_snap before taking rtnl_lock() and never re-validates…
- [High] Every early validation return in otx2_setup_tc_mqprio()
  (interface not UP, wrong shaper, no CIR+PIR, SDP rep, PFC enabled, XDP…
- [High] A successful mqprio replacement is rolled back when
  NETIF_F_HW_TC is not enabled, which is the default for these devices.
- [High] Replacing a software mqprio root with an offloaded mqprio
  succeeds but loses the newly installed traffic-class layout.
- [High] The new mqprio.netdev_tc_work dereferences pfvf->netdev and
  pfvf->old_mq_snap and mutates core netdev TC state, but…
- [Medium] Two changes to code shared by all PF/VF configurations are
  bundled into this feature patch with no mention in the commit message:…
- [Medium] The commit message claims failed setup 'rolls back via netdev
  restart' and the code reports 'mqprio: replace failed; prior…
- [Medium] otx2_mqprio_max_rate_bytes_ps() derives a maximum *rate*
  (bytes/s) from the NIX *burst-size* constants MAX_BURST_SIZE (130816…
- [Medium] The driver reconstructs the core qdisc replace state machine:
  otx2_mqprio_keep_netdev_tc() strcmp()s…
- [Medium] otx2_txschq_stop() now zeroes pfvf->hw.txschq_cnt[] on every
  interface stop, but otx2_get_smq_idx() computes 'idx = qidx %…
- [Medium] A failed mqprio replacement can leave the surviving old qdisc
  inconsistent with both the driver's cached state and the programmed…
- [Medium] pf->refill_wrk (devm_kcalloc'd array of INIT_DELAYED_WORK'd
  struct refill_work, allocated in otx2_config_nix_queues() from…
- [Medium] INIT_WORK(&nic->mqprio.netdev_tc_work,
  otx2_mqprio_netdev_tc_work) is performed in otx2_init_tc(), which runs…
- [Medium] otx2_nix_tm_set_queue_shaper() programs both MDQ PIR and CIR
  via otx2_get_txschq_rate_regval(pfvf, rate, 0), and burst == 0 in…
- [Low] Two new comments describe the hard-coded burst value 65536 in the
  HTB shaper path as 'the kernel-side default burst when HTB does not…

--

octeontx2-pf: add mqprio bandwidth offload for NIX TX schedulers

This adds TC_SETUP_QDISC_MQPRIO offload with TC_MQPRIO_SHAPER_BW_RATE for
PF and VF netdevs, programming per-queue MDQ CIR/PIR through the NIX TX
scheduler mailbox.  Because the scheduler cannot be reprogrammed live, add,
replace, delete and rollback all rebuild the hierarchy by bouncing the
netdev through ndo_stop()/ndo_open(), with rates cached in software and
restored from otx2_mqprio_up() on open.

A few questions below about the state machine, the netdev bounce, the rate
limits and the new work item.

> diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.c b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.c
> index b421cb75e44b..5bad2466da0c 100644
> --- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.c
> +++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.c
> @@ -615,6 +615,142 @@ void otx2_get_mac_from_af(struct net_device *netdev)

[ ... ]

> +	/* MQPRIO exposes only min/max rate, not burst.  Pass burst 0 so
> +	 * otx2_get_egress_burst_cfg() programmes the largest burst the NIX
> +	 * encoding supports (CN10K_MAX_BURST_SIZE on CN10K).  This differs
> +	 * from the 65536 byte default used in the HTB path, which is a
> +	 * kernel-side default when no explicit burst is configured, not a
> +	 * hardware cap.
> +	 *
> +	 * mqprio setup restarts the netdev (otx2_mqprio_restart_netdev),
> +	 * which resets MDQ shapers to zero.  Program both PIR and CIR on
> +	 * every update so omitted rates are applied explicitly rather than
> +	 * relying on stale hardware state.
> +	 */
> +	req->reg[n] = NIX_AF_MDQX_PIR(smq);
> +	if (maxrate) {
> +		rate = otx2_convert_rate(maxrate);
> +		req->regval[n] = otx2_get_txschq_rate_regval(pfvf, rate, 0);
> +	} else {
> +		req->regval[n] = 0;
> +	}

[Severity: Medium]
Does passing burst 0 here make the configured rate ineffective for bursty
traffic?  In otx2_get_egress_burst_cfg() the burst == 0 case selects the
largest encodable bucket:

	} else {
		/* burst 0: largest encodable burst (CN10K_MAX_BURST_SIZE on
		 * CN10K), not a minimal burst.
		 */
		*burst_exp = MAX_BURST_EXPONENT;
		*burst_mantissa = max_mantissa;
	}

That encodes 130,816 bytes on OTx2 and 8,453,888 bytes on CN10K.  With
max_rate 2 Mbps and an 8.45 MB bucket, an idle queue accumulates roughly
33 seconds worth of credit that can be released at line rate before CIR/PIR
starts limiting.

The HTB path uses 65536 bytes for the same registers.  Was a smaller,
rate-derived burst considered for the mqprio path?

> +
> +	/* CIR+PIR support is required and checked at mqprio setup. */
> +	req->reg[n] = NIX_AF_MDQX_CIR(smq);

[ ... ]

> @@ -779,6 +919,9 @@ int otx2_txsch_alloc(struct otx2_nic *pfvf)
>  		req->schq[NIX_TXSCH_LVL_TL4] = chan_cnt;
>  	}
>  
> +	if (pfvf->mqprio.rate_limit)
> +		req->schq[NIX_TXSCH_LVL_SMQ] = pfvf->hw.non_qos_queues;
> +
>  	rc = otx2_sync_mbox_msg(&pfvf->mbox);
>  	if (rc)
>  		return rc;
> @@ -844,6 +987,7 @@ void otx2_txschq_stop(struct otx2_nic *pfvf)
>  
>  	/* Clear the txschq list */
>  	for (lvl = 0; lvl < NIX_TXSCH_LVL_CNT; lvl++) {
> +		pfvf->hw.txschq_cnt[lvl] = 0;
>  		for (schq = 0; schq < MAX_TXSCHQ_PER_FUNC; schq++)
>  			pfvf->hw.txschq_list[lvl][schq] = 0;
>  	}

[Severity: Medium]
Can this new zeroing produce a division by zero in otx2_get_smq_idx()?
That helper has no zero check:

drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.h:otx2_get_smq_idx() {
	...
	idx = qidx % pfvf->hw.txschq_cnt[NIX_TXSCH_LVL_SMQ];
	smq = pfvf->hw.txschq_list[NIX_TXSCH_LVL_SMQ][idx];
	...
}

After a failed otx2_open() that unwinds through err_free_txsch with pf->pfc_en
set, otx2_txschq_stop() zeroes txschq_cnt[] but that unwind does not call
otx2_pfc_txschq_stop(), so pfvf->pfc_alloc_status[prio] stays true.  A later
'dcb pfc set' clearing such a priority takes the disable branch of
otx2_pfc_txschq_update(), which clears pfc_alloc_status[prio] and then calls
otx2_pfc_update_sq_smq_mapping() -> otx2_get_smq_idx(pfvf, prio), i.e.
prio % 0.

On arm64 the divide returns 0 rather than trapping, so the SQ context ends up
programmed with SMQ index 0 (which this same loop just zeroed); on a
64BIT && COMPILE_TEST build it is a divide-error Oops, and UBSAN flags it.
Before this patch the counts retained their last non-zero values after a
stop, so the divisor was never 0.

Would a zero check in otx2_get_smq_idx(), or leaving the counts alone here,
be preferable?

> diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c
> index 32582b6347ea..5ff99ad986d0 100644
> --- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c
> +++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c
> @@ -2007,6 +2007,15 @@ int otx2_open(struct net_device *netdev)
>  	if (err)
>  		goto err_free_mem;
>  
> +	/* Fail closed: abort open if cached mqprio shapers cannot be restored. */
> +	err = otx2_mqprio_up(pf);
> +	if (err) {
> +		netdev_err(pf->netdev,
> +			   "mqprio: failed to restore shapers during open: %d\n",
> +			   err);
> +		goto err_free_hw;
> +	}
> +

[Severity: Medium]
Does this new failure branch leak pf->refill_wrk?

The array is allocated in otx2_config_nix_queues(), reached from
otx2_init_hw_resources() just above:

	pfvf->refill_wrk = devm_kcalloc(pfvf->dev, pfvf->qset.cq_cnt,
					sizeof(struct refill_work), GFP_KERNEL);

The only place it is cancelled and freed is otx2_stop():

	for (wrk = 0; wrk < pf->qset.cq_cnt; wrk++)
		cancel_delayed_work_sync(&pf->refill_wrk[wrk].pool_refill_work);
	devm_kfree(pf->dev, pf->refill_wrk);

and that function returns early when OTX2_FLAG_INTF_DOWN is set.  Neither
err_free_hw nor err_free_mem touches it, and the next open overwrites the
pointer.

Because an otx2_mqprio_up() failure is sticky (the cached rates are
unchanged), each 'ip link set up' retry leaks another cq_cnt-sized array
until device detach.  The gap in the unwind chain itself predates this
patch, but this adds a new and easily repeated branch into it.

>  	/* Register NAPI handler */
>  	for (qidx = 0; qidx < pf->hw.cint_cnt; qidx++) {
>  		cq_poll = &qset->napi[qidx];
> @@ -2205,6 +2214,7 @@ int otx2_open(struct net_device *netdev)
>  	free_irq(vec, pf);
>  err_disable_napi:
>  	otx2_disable_napi(pf);
> +err_free_hw:
>  	otx2_free_hw_resources(pf);
>  err_free_mem:
>  	otx2_free_queue_mem(qset);
> @@ -2280,6 +2290,7 @@ int otx2_stop(struct net_device *netdev)
>  	for (qidx = 0; qidx < netdev->num_tx_queues; qidx++)
>  		netdev_tx_reset_queue(netdev_get_tx_queue(netdev, qidx));
>  
> +	synchronize_net();
>  	otx2_free_queue_mem(qset);

[Severity: Medium]
Could the commit message mention this hunk and the txschq_cnt zeroing in
otx2_txschq_stop()?  Both change code shared by every PF/VF configuration,
not just the mqprio path:

 - synchronize_net() here runs on every interface down (and via
   rtnl_is_locked() that is synchronize_rcu_expedited()), for all users.
 - zeroing pfvf->hw.txschq_cnt[lvl] changes a global invariant, since that
   value is the modulo divisor in otx2_get_smq_idx().

What problem do they fix, and are they required by this feature or
independent fixes that could be separate patches?

> diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_tc.c b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_tc.c
> index ddb46b580c3b..edd7c02efb47 100644
> --- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_tc.c
> +++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_tc.c
> @@ -31,6 +34,20 @@
>  
>  #define MCAST_INVALID_GRP		(-1U)
>  #define RATE_MANTISSA_BITS		8
> +/* Min per-queue egress shaping rate the NIX TLX encoder supports (2 Mbps). */
> +#define OTX2_MQPRIO_MIN_RATE_BYTES_PS	250000ULL
> +
> +static u64 otx2_mqprio_max_rate_bytes_ps(struct otx2_nic *pfvf)
> +{
> +	u64 max_burst;
> +
> +	if (is_dev_otx2(pfvf->pdev))
> +		max_burst = MAX_BURST_SIZE;
> +	else
> +		max_burst = CN10K_MAX_BURST_SIZE;
> +
> +	return (max_burst * 1000000ULL) / 8ULL;
> +}

[Severity: Medium]
Should the maximum rate be derived from the burst-size constants?

MAX_BURST_SIZE (130816) and CN10K_MAX_BURST_SIZE (8453888) are byte-count
caps of the TLX BURST_EXPONENT/BURST_MANTISSA encoder and carry no rate
meaning.  The rate ceiling comes from MAX_RATE_EXPONENT (0xF) and
MAX_RATE_MANTISSA (0xFF) in otx2_get_egress_rate_cfg(), i.e.
2 * ((256 + 255) << 15) / 256 = 130,816 Mbps on both OTx2 and CN10K.

The OTx2 arithmetic happens to coincide (130816 * 1e6 / 8 = 16.352 GB/s =
130,816 Mbps), but on CN10K this advertises ~1.0567e12 B/s, about 8.45 Tbps,
roughly 65x the real ceiling.

In otx2_mqprio_rate_valid() the remaining checks do not close the gap:

	if (rate_bytes_ps > otx2_mqprio_max_rate_bytes_ps(pfvf))
		return false;

	if (rate_bytes_ps > div_u64(U64_MAX, 8))
		return false;

	mbps = otx2_convert_rate(rate_bytes_ps);
	return ilog2(mbps / 2) <= MAX_RATE_EXPONENT;

ilog2(65535) is 15, so 130,817..131,071 Mbps still pass and are then
silently floored to 130,816 Mbps by the mantissa clamp in
otx2_get_egress_rate_cfg().  For min_rate/CIR that means the hardware
guarantees less than what was accepted.

The 'rate_bytes_ps > div_u64(U64_MAX, 8)' test also looks unreachable, since
the earlier ceiling check already rejects everything above ~1e12.

[ ... ]

> +static void otx2_mqprio_netdev_tc_work(struct work_struct *work)
> +{
> +	struct otx2_mqprio *mqprio = container_of(work, struct otx2_mqprio,
> +						  netdev_tc_work);
> +	struct otx2_nic *pfvf = container_of(mqprio, struct otx2_nic, mqprio);
> +
> +	if (!pfvf->mqprio.rate_limit || !pfvf->old_mq_snap)
> +		return;
> +
> +	rtnl_lock();
> +	otx2_mqprio_apply_snap_netdev(pfvf->netdev, pfvf->old_mq_snap);
> +	rtnl_unlock();
> +}

[Severity: High]
Are these two fields safe to test before taking rtnl_lock()?

Both mqprio.rate_limit and old_mq_snap are written only under RTNL, and this
work is always queued while the caller already holds RTNL, so the worker
necessarily blocks and runs in some later RTNL section.  A 'tc qdisc del' in
that window runs otx2_teardown_tc_mqprio() -> otx2_mqprio_clear_sw():

	pfvf->mqprio.rate_limit = false;
	otx2_mqprio_clear_replace_state(pfvf);
	if (!otx2_mqprio_keep_netdev_tc(pfvf))
		netdev_set_num_tc(netdev, 0);
	otx2_mqprio_free_cache(pfvf);

That clears rate_limit and zeroes num_tc, but does not free old_mq_snap and
does not cancel this work.  The worker then wins the lock and re-installs
num_tc, tc_to_txq and prio_tc_map for an offload that no longer exists.

Since otx2_set_channels() only blocks queue-count changes while
mqprio.rate_limit is true, a subsequent channel reduction can then leave the
re-published offset+count beyond real_num_tx_queues, so
skb_tx_hash()/netdev_pick_tx() pick queue indices outside the valid range.

Should the guard be re-evaluated inside the rtnl_lock() section?

[ ... ]

> +static int otx2_mqprio_restore_old(struct otx2_nic *pfvf)
> +{
[ ... ]
> +	if (otx2_mqprio_mdq_allocated(pfvf)) {
> +		err = otx2_nix_tm_clear_queue_shaper(pfvf);
> +		if (err)
> +			return err;
> +	}
[ ... ]
> +	if (netif_running(netdev)) {
> +		err = otx2_mqprio_restart_netdev(netdev, true);
> +		if (err)
> +			return err;
> +	} else if (pfvf->hw.txschq_cnt[NIX_TXSCH_LVL_SMQ]) {
> +		err = otx2_mqprio_up(pfvf);
> +		if (err)
> +			return err;
> +	}

[Severity: Medium]
What state is the interface left in when the rollback itself fails?

This function is fallible at four points (otx2_mqprio_alloc_cache(),
otx2_nix_tm_clear_queue_shaper(), otx2_mqprio_restart_netdev() and
otx2_mqprio_up()), and the software rates are installed before hardware is
touched.  Each early 'return err' precedes the restart/up that would rebuild
the configuration.

otx2_nix_tm_clear_queue_shaper() is also split into several mailbox batches:

	err = otx2_nix_tmq_reg_write(pfvf, cnt, reg_addr, reg_val);
	if (err)
		goto fail;

so an error can be returned after earlier batches already zeroed some MDQ
CIR/PIR registers.

The caller only logs restore_err and returns, so the interface keeps running
with partially programmed or fully cleared limits that no longer match the
software cache, while the surviving old qdisc still reports hw_offload.  If
ndo_open() failed during the rollback, otx2_mqprio_restart_netdev() also
calls otx2_mqprio_clear_sw(), leaving the old offloaded qdisc installed with
rate_limit disabled.

> +/* Offloaded mqprio replaced by software mqprio installs netdev TC layout in
> + * mqprio_init() before the old offload instance is destroyed during graft.
> + */
> +static bool otx2_mqprio_keep_netdev_tc(struct otx2_nic *pfvf)
> +{
> +	struct Qdisc *qdisc = rtnl_dereference(pfvf->netdev->qdisc);
> +
> +	return qdisc && qdisc->ops && !strcmp(qdisc->ops->id, "mqprio");
> +}

[Severity: Medium]
Is a driver expected to inspect netdev->qdisc and strcmp() the qdisc
ops->id string to infer which stage of a graft it is in?  There is no
exported contract for that, so this ties the driver to the internal
ordering of qdisc_graft()/notify_and_destroy().

Related: otx2_setup_tc_root() treats the generic TC_ROOT_GRAFT notification
as a private "my replace committed" flag while dropping the discriminators
the core supplies.  qdisc_offload_graft_root() in net/sched/sch_api.c fills
in both:

	struct tc_root_qopt_offload graft_offload = {
		.command	= TC_ROOT_GRAFT,
		.handle		= new ? new->handle : 0,
		.ingress	= (new && new->flags & TCQ_F_INGRESS) || ...

and the notification is emitted for every root graft, including
ingress/clsact.  Could the commit point be derived from state the driver
owns instead?

[ ... ]

> +int otx2_mqprio_down(struct otx2_nic *pfvf)
> +{
> +	int err = 0;
> +
> +	if (!pfvf->mqprio.rate_limit)
> +		return 0;
> +
> +	if (netif_running(pfvf->netdev) &&
> +	    otx2_mqprio_mdq_allocated(pfvf))
> +		err = otx2_nix_tm_clear_queue_shaper(pfvf);
> +
> +	if (err) {
> +		netdev_warn(pfvf->netdev,
> +			    "mqprio: failed to clear hardware shapers: %d; keeping offload state\n",
> +			    err);
> +		return err;
> +	}
> +
> +	otx2_mqprio_clear_sw(pfvf);
> +
> +	return 0;
> +}

[Severity: High]
Who consumes this error?  In net/sched/sch_mqprio.c the teardown caller has
a void interface and discards the return value:

static void mqprio_disable_offload(struct Qdisc *sch)
{
	...
		dev->netdev_ops->ndo_setup_tc(dev, TC_SETUP_QDISC_MQPRIO,
					      &mqprio);

and mqprio_destroy() destroys the qdisc unconditionally.  So when
otx2_nix_tm_clear_queue_shaper() fails (otx2_nix_tmq_reg_write() returns
-ENOMEM on message-alloc failure, and propagates mailbox timeout / AF
errors), the qdisc goes away while the driver keeps mqprio.rate_limit true,
the rate cache allocated and netdev->num_tc set, with no qdisc left to retry
teardown.

>From then on otx2_set_channels(), otx2_xdp_setup(), otx2_dcbnl_ieee_setpfc()
and otx2_qos_root_add() reject forever, otx2_txsch_alloc() keeps requesting
non_qos_queues SMQs, otx2_txschq_config() keeps re-parenting MDQs to TL4[0],
and the stale rates are re-applied on every open.

Worse, since the replace state machine keys off mqprio.rate_limit, a later
'tc qdisc add ... mqprio hw 1' is then classified as a replace, and its
deletion takes the replace-commit branch in otx2_teardown_tc_mqprio() that
returns 0 without any shaper clear, clear_sw or netdev restart.

Since ndo_stop() frees the hierarchy anyway, would it be better to
reconcile the software state unconditionally here and only warn about the
mailbox failure?

[ ... ]

> +static int otx2_mqprio_restart_netdev(struct net_device *netdev, bool rate_limit)
> +{
> +	struct otx2_nic *pfvf = netdev_priv(netdev);
> +	const struct net_device_ops *ops = netdev->netdev_ops;
> +	bool running = netif_running(netdev);
> +	int err;
[ ... ]
> +	if (running) {
> +		clear_bit(__LINK_STATE_START, &netdev->state);
> +		smp_mb__after_atomic(); /* Commit netif_running(). */
> +	}
> +
> +	err = ops->ndo_stop(netdev);
> +	if (err) {
> +		if (running)
> +			set_bit(__LINK_STATE_START, &netdev->state);
> +		return err;
> +	}
[ ... ]
> +	err = ops->ndo_open(netdev);
> +	if (!err && running) {
> +		set_bit(__LINK_STATE_START, &netdev->state);
> +	} else if (err) {
[ ... ]
> +		otx2_set_flag(pfvf, OTX2_FLAG_INTF_DOWN);
> +		/* visible to otx2_stop() on other cpus */
> +		smp_wmb();
> +		netif_close(netdev);
> +	}

[Severity: High]
Can a driver write the core-owned __LINK_STATE_START bit like this?
netif_running() is defined as test_bit(__LINK_STATE_START, &dev->state) and
that bit is set and cleared only by __dev_open()/__dev_close_many().

While it is cleared here the netdev is in a state the core never produces:
IFF_UP set but netif_running() false.  Code that keys off netif_running()
then reads it as "interface down", including otx2_set_channels(),
otx2_xdp_setup(), otx2_mqprio_down() and otx2_mqprio_restore_old().

Separately, is calling netif_close() on its own netdev from inside
ndo_setup_tc() acceptable?  A plain 'tc qdisc add/replace ... mqprio ...
hw 1' would then clear IFF_UP and emit NETDEV_GOING_DOWN/NETDEV_DOWN
(address and route teardown, bond/bridge reactions) as a side effect of a
qdisc operation.

netif_close() also performs dev_deactivate_many(), which is exactly what the
comment a few lines above in this same function forbids:

 * Do not call dev_deactivate()/dev_activate() here.  On replace,
 * qdisc_graft() already deactivates qdiscs around offload teardown;

This path is reachable in practice because the patch widens the SMQ request
to pfvf->hw.non_qos_queues in otx2_txsch_alloc(), so ndo_open() can fail on
AF refusal or a mailbox timeout.

[ ... ]

> +static int otx2_setup_tc_mqprio(struct net_device *netdev,
> +				struct tc_mqprio_qopt_offload *mqprio)
> +{
[ ... ]
> +	if (!if_up) {
> +		netdev_err(netdev, "mqprio: setup requires interface UP\n");
> +		NL_SET_ERR_MSG_MOD(extack, "mqprio: setup requires interface UP");
> +		return -EOPNOTSUPP;
> +	}
[ ... ]
> +		if (mqprio->flags & TC_MQPRIO_F_MAX_RATE &&
> +		    !otx2_mqprio_rate_valid(pfvf, max_rate)) {
> +			netdev_err(netdev,
> +				   "mqprio: max_rate %llu for tc %d is outside hardware limits\n",
> +				   max_rate, tc);
> +			NL_SET_ERR_MSG_FMT_MOD(extack,
> +					       "mqprio: max_rate %llu for tc %d is outside hardware limits",
> +					       max_rate, tc);
> +			return -EINVAL;
> +		}
> +	}
> +
> +	err = otx2_mqprio_validate_txqs(netdev, extack, qopt);
> +	if (err)
> +		return err;

[Severity: High]
Do these early returns need the same compensation as the cleanup label?

All eleven validation returns above (interface not UP, wrong shaper, no
CIR+PIR, SDP rep, PFC, XDP, HTB, min > max, per-TC multi-queue rate, rate
outside limits, otx2_mqprio_validate_txqs()) bypass cleanup: and therefore
otx2_mqprio_defer_netdev_tc_restore().

When an offload is already active, mqprio_init() of the new instance has
already installed its TC layout, and on error mqprio_destroy() runs with
priv->hw_offload == 0:

net/sched/sch_mqprio.c:mqprio_destroy() {
	if (priv->hw_offload && dev->netdev_ops->ndo_setup_tc)
		mqprio_disable_offload(sch);
	else
		netdev_set_num_tc(dev, 0);
}

So dev->num_tc becomes 0 while pfvf->mqprio.rate_limit stays true and the
MDQ CIR/PIR shapers stay programmed and get re-applied by otx2_mqprio_up()
on the next open.  The prio->tc->txq mapping of the still-active offload is
silently gone.

A single 'tc qdisc replace dev X root handle 8002: mqprio ... hw 1' with
max_rate below OTX2_MQPRIO_MIN_RATE_BYTES_PS reaches this.

[ ... ]

> +	netdev_set_num_tc(netdev, pfvf->cur_mq_snap->num_tc);
> +	for (i = 0; i < pfvf->cur_mq_snap->num_tc; i++)
> +		netdev_set_tc_queue(netdev, i, pfvf->cur_mq_snap->count[i],
> +				    qopt->offset[i]);
> +
> +	qopt->hw = TC_MQPRIO_HW_OFFLOAD_TCS;
> +
> +	if (replacing) {
> +		pfvf->mqprio.replace_setup_done = true;
> +		pfvf->mqprio.replace_graft_done = false;
> +	} else {
> +		otx2_mqprio_snap_commit(pfvf);
> +	}

[Severity: High]
What happens when a software mqprio root is replaced by an offloaded one?

mqprio.rate_limit is false in that case, so 'replacing' is false, the
snapshot is committed and no restore is scheduled.  qdisc_graft() then
destroys the old software instance, whose mqprio_destroy() takes the
'else netdev_set_num_tc(dev, 0);' branch because hw_offload is unset, wiping
the layout just installed here.  mqprio_attach() for the new instance only
attaches child qdiscs; it does not reinstall the layout.

The command reports success and the shapers are programmed, but dev->num_tc
is 0 so skb_tx_hash()/netdev_pick_tx() ignore the requested prio->tc->txq
mapping.  otx2_mqprio_keep_netdev_tc() only covers the opposite direction
(offloaded replaced by software).

> +
> +	return 0;
> +
> +cleanup:
> +	qopt->hw = 0;
> +	if (replacing) {
> +		int restore_err = otx2_mqprio_restore_old(pfvf);
[ ... ]
> +		} else {
> +			netdev_err(netdev,
> +				   "mqprio: replace failed; prior configuration restored\n");
> +			if (extack)
> +				NL_SET_ERR_MSG_MOD(extack,
> +						   "mqprio: replace failed; prior configuration restored");
[ ... ]
> +			otx2_mqprio_defer_netdev_tc_restore(pfvf);
> +		}

[Severity: Medium]
Is "prior configuration restored" accurate at this point?  The netdev TC
layout part (num_tc, tc_to_txq, prio_tc_map) is only scheduled here, and the
worker can decline to do anything at all:

	if (!pfvf->mqprio.rate_limit || !pfvf->old_mq_snap)
		return;

Between mqprio_destroy() of the failed instance running
netdev_set_num_tc(dev, 0) and the work item acquiring RTNL, the surviving
old mqprio qdisc has a TC layout the netdev no longer reflects, and if the
guard fails the restore never happens and nothing is logged.

The commit message says failed setup "rolls back via netdev restart", which
reads as a synchronous, complete rollback.  Could both the message and this
string reflect that the layout restore is asynchronous and best effort?

> +static int otx2_setup_tc_root(struct otx2_nic *pfvf,
> +			      struct tc_root_qopt_offload *root)
> +{
> +	switch (root->command) {
> +	case TC_ROOT_GRAFT:
> +		if (pfvf->mqprio.replace_setup_done)
> +			pfvf->mqprio.replace_graft_done = true;
> +		return 0;
> +	default:
> +		return -EOPNOTSUPP;
> +	}
> +}

[Severity: High]
Is TC_ROOT_GRAFT guaranteed to arrive?  In net/sched/sch_api.c the notifier
is skipped when the feature bit is off:

qdisc_offload_graft_helper() {
	if (!tc_can_offload(dev) || !dev->netdev_ops->ndo_setup_tc)
		return;

while mqprio_enable_offload() calls ndo_setup_tc() directly with no such
check.  In otx2_probe() the bit is offered but not enabled by default:

	netdev->features |= netdev->hw_features;

	/* HW supports tc offload but mutually exclusive with n-tuple filters */
	if (otx2_test_flag(pf, OTX2_FLAG_TC_FLOWER_SUPPORT))
		netdev->hw_features |= NETIF_F_HW_TC;

NETIF_F_HW_TC is added to hw_features after features was assigned, so
tc_can_offload(dev) is false unless the admin runs
'ethtool -K ethX hw-tc-offload on' (otx2_vf.c has the same pattern).

So on a successful 'tc qdisc replace ... mqprio ... hw 1' in the default
configuration, replace_graft_done stays false, and when the old instance is
destroyed otx2_teardown_tc_mqprio() takes:

		else
			err = otx2_mqprio_restore_old(pfvf);

reverting to the previous rates and queue layout even though the new qdisc
was grafted.  The command reports success while the hardware keeps the old
configuration.

[ ... ]

> @@ -1625,13 +2433,17 @@ int otx2_init_tc(struct otx2_nic *nic)
>  		return -EINVAL;
>  	}
>  
> +	INIT_WORK(&nic->mqprio.netdev_tc_work, otx2_mqprio_netdev_tc_work);
> +
>  	return 0;
>  }

[Severity: Medium]
Is this INIT_WORK too late?  otx2_init_tc() runs after register_netdev() in
both otx2_probe() and otx2vf_probe(), so ndo_setup_tc - including the new
TC_SETUP_QDISC_MQPRIO handler that can reach schedule_work() through
otx2_mqprio_defer_netdev_tc_restore() - is reachable from userspace while
the work_struct is still all zeroes and work->func is NULL.

The window also needs 'replacing == true', i.e. a prior successful offload
plus a failing replace in that interval, so it is narrow.  Would moving
otx2_init_tc() before register_netdev(), or the INIT_WORK earlier, close it?

>  EXPORT_SYMBOL(otx2_init_tc);
>  
>  void otx2_shutdown_tc(struct otx2_nic *nic)
>  {
> +	cancel_work_sync(&nic->mqprio.netdev_tc_work);
>  	otx2_destroy_tc_flow_list(nic);
> +	otx2_mqprio_snap_destroy(nic);
>  }
>  EXPORT_SYMBOL(otx2_shutdown_tc);

[Severity: High]
Is this the right place for the cancel?  This is the only cancel_work_sync()
for netdev_tc_work, and both otx2_remove() and otx2vf_remove() call
otx2_shutdown_tc() after unregister_netdev().

A work item queued shortly before removal blocks on rtnl_lock() while
unregister_netdev() closes the device and destroys its qdiscs, then wins the
lock and calls netdev_set_num_tc()/netdev_set_tc_queue()/
netdev_set_prio_tc_map() on a NETREG_UNREGISTERED device (those also walk
dev->_tx and reset XPS).

On the probe unwind the ordering is inverted:

	err_pf_sriov_init:
		otx2_unregister_dl(pf);
		otx2_shutdown_tc(pf);
	err_mcam_flow_del:
		otx2_mcam_flow_del(pf);
	err_unreg_netdev:
		unregister_netdev(netdev);

Paths entering at err_mcam_flow_del skip the cancel entirely, and paths
entering above it cancel while the netdev is still registered and
ndo_setup_tc still reachable, so the work can be re-armed after the cancel
and then run against memory released by free_netdev(), which is a
use-after-free of the struct otx2_nic in the netdev private area.

The mqprio teardown path does not cancel a pending work either; it relies
only on the flag re-check inside the worker.

> diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/qos.c b/drivers/net/ethernet/marvell/octeontx2/nic/qos.c
> index f160b1618efa..9ef55a6db50b 100644
> --- a/drivers/net/ethernet/marvell/octeontx2/nic/qos.c
> +++ b/drivers/net/ethernet/marvell/octeontx2/nic/qos.c
> @@ -118,6 +118,9 @@ static void otx2_config_sched_shaping(struct otx2_nic *pfvf,
>  	/* configure PIR */
>  	maxrate = (node->rate > node->ceil) ? node->rate : node->ceil;
>  
> +	/* 65536 is the kernel-side default burst when HTB does not supply an
> +	 * explicit value, not the NIX hardware maximum (CN10K_MAX_BURST_SIZE).
> +	 */
>  	cfg->regval[*num_regs] =
>  		otx2_get_txschq_rate_regval(pfvf, maxrate, 65536);
>  	(*num_regs)++;

[Severity: Low]
This isn't a bug, but does the "when HTB does not supply an explicit value"
wording describe a condition that exists?  struct tc_htb_qopt_offload has no
burst member (otx2_setup_tc_htb() uses only rate, ceil, prio and quantum),
and both calls here pass the literal 65536 unconditionally, for PIR and for
CIR.  The same wording appears in the new comment in
otx2_nix_tm_set_queue_shaper().

Thanks for considering these.

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260918015906.1255204-1-rkannoth%40marvell.com

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2026-09-22  2:19 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-09-18  1:59 [PATCH v16 net-next 0/2] octeontx2: mqprio bandwidth offload for NIX TX schedulers Ratheesh Kannoth
2026-09-18  1:59 ` [PATCH v16 net-next 1/2] octeontx2: use atomic bitops for PF/VF and rep flags Ratheesh Kannoth
2026-09-22  2:19   ` netdev-bot+sashiko
2026-09-18  1:59 ` [PATCH v16 net-next 2/2] octeontx2-pf: add mqprio bandwidth offload for NIX TX schedulers Ratheesh Kannoth
2026-09-22  2:19   ` netdev-bot+sashiko

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®