Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Medium] Concurrent writers to the same `u64_stats_sync` structure during RX queue swap. --
--- Patch [8]: [PATCH net-next v2 08/13] net: mana: keep per-queue statistics in the port context --- commit 48e144b542daf3b7f2ff07b74db60098e784159d Author: Long Li <[email protected]> net: mana: keep per-queue statistics in the port context This commit moves per-queue statistics out of the queue structures and into a port-context array to prevent statistics from resetting when the queue is reconfigured. The stats are indexed by the queue index, and the arrays outlive the queues so totals stay monotonic. > diff --git a/drivers/net/ethernet/microsoft/mana/mana_en.c > b/drivers/net/ethernet/microsoft/mana/mana_en.c > --- a/drivers/net/ethernet/microsoft/mana/mana_en.c > +++ b/drivers/net/ethernet/microsoft/mana/mana_en.c [ ... ] > @@ -1059,6 +1065,48 @@ static void mana_cleanup_port_context(struct > mana_port_context *apc) > apc->rxqs = NULL; > } > > +/* Counters belong to the port, not the queues, so a queue-set replacement > + * does not reset them. Sized to max_queues, allocated once. > + * > + * A swap adds no writer to a TX slot. RX slots do overlap briefly, since a > + * retiring rxq keeps its NAPI until mana_free_qset() destroys it. MANA is > + * 64-bit only, so u64_stats_sync has no seqcount and at worst a few > + * increments are lost; the alternatives are a lock in the receive path or > + * per-set slots that make ndo_get_stats64() dip during a swap. > + */ [Severity: Medium] Is it safe to allow concurrent writers to the same u64_stats_sync structure? The u64_stats_sync API requires that writers must be mutually exclusive for a given syncp. While MANA is 64-bit only and lacks a seqcount in the syncp, allowing both the retiring RX queue's NAPI and the new RX queue's NAPI to concurrently update the same apc->rxq_stats[rxq_idx] introduces a data race. For example, both could be running mana_rx_skb() at the same time: u64_stats_update_begin(&rx_stats->syncp); rx_stats->packets++; rx_stats->bytes += pkt_len; Could this lead to undefined behavior or KCSAN warnings when the queues overlap during a swap? > +static int mana_alloc_queue_stats(struct mana_port_context *apc) > +{ > + unsigned int i; -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=8
