Re: [PATCH v18 net-next 0/2] octeontx2: mqprio bandwidth offload for NIX TX schedulers

[Ratheesh Kannoth]

On 2026-10-02 at 15:07:52, David Laight (david.laight.linux@xxxxxxxxx) wrote:
> > 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.
>
> Can't you just move those two flags to a separate structure member?
> In at least one place the code separately clears one and sets the other.
> That makes me think it should a a three-valued state not two bits.
>
> That would save all the expensive locked operations.

Thanks for your review.

I agree that atomizing the entire flags bitmap is broader than strictly required for the
mqprio/mbox concurrency: the cross-CPU hazard is otx2_sync_flags_from_rep() doing a non-atomic
read-modify-write on nic->flags while other CPUs update bits in the same word. Relocating
OTX2_FLAG_INTF_DOWN and OTX2_FLAG_PORT_UP (or restricting rep sync so it cannot clobber those
bits) would be a narrower approach than converting every OTX2_FLAG_* accessor to
set_bit()/clear_bit().

On the three-valued model: INTF_DOWN and PORT_UP are not a single FSM in the driver today.
INTF_DOWN reflects netdev teardown and is consulted from NAPI completion etc.
PORT_UP is set and cleared from rep RVU_EVENT_PORT_STATE in the mbox up-handler and
suppresses a duplicate carrier/queue bring-up in otx2_handle_link_event() when rep has already
applied port state. Combinations such as INTF_DOWN set after otx2_stop() while PORT_UP remains
set are deliberate, so folding the two bits into one enum would need a seperate work
and review beyond this series.

please note that this restructuring feels somewhat orthogonal to the goals of the
current series. Patch 1 focuses on replacing the non-atomic |=/&=~ operations on the
stop/open and mbox paths with atomic bitops for INTF_DOWN/PORT_UP, which patch 2's netdev
bounce depends on. The rep-flag sync behavior in otx2_sync_flags_from_rep() is a related but
separate concern, and I'd prefer to address the lifecycle-bit layout and sync logic in a
dedicated follow-up rather than expand the scope of this patch series.

>