On Mon, Nov 09, 2026 at 12:34:56PM +0000, [email protected] wrote: > - [High] virtnet_xsk_pools_release() unmaps the DMA of a zero-copy pool > sitting at a qid >= curr_queue_pairs (left there by removing XDP), but… > - [Medium] When virtnet_xsk_pool_attach() fails on restore, the pool > stays DMA mapped while rq->xsk_pool stays NULL. > - [Medium] If virtnet_xsk_pool_attach() fails during restore, > virtnet_xsk_pools_attach() only logs netdev_warn() and the resume… > - [Medium] virtnet_xsk_pools_release() is a new writer of AF_XDP driver > state (rq->xsk_pool, sq->xsk_pool, rq->xsk_buffs, sq->xsk_hdr) that… > > Pre-existing issues: > - [High] This problem wasn't introduced by this patch, but it defeats the > patch's cleanup model on the failed-restore path.
Thanks, these all hold. sashiko-bot raised the same points in its review of this patch. The High one and the first Medium have the same cause: once the queues are freed on freeze, the patch finds a pool again only through xsk_get_pool_from_qid(), which depends on the current queue counts. A pool on a queue above curr_queue_pairs can then be unmapped on freeze and bound again unmapped later, or not found when it has to be unmapped. The second Medium is right as well: when the attach fails, the socket stays bound but gets no traffic and no error. And the release on freeze runs without RTNL, unlike every other writer of that state. The failed-restore issue is older, as noted: after a failed virtnet_restore_up(), vi->rq and vi->sq are freed while the netdev stays registered, and virtnet_close() uses them too. I'd rather keep that separate. I'll drop this patch from the series, resend patches 1 and 2 as v3, and rework it to keep track of the pools in the driver across suspend instead of looking them up by queue id. pw-bot: cr
