Subject: Re: [RFC net] net: make page pool stall netdev unregistration to avoid IOMMU crashes

From: Jakub Kicinski <[email-masked]>
Date: Thu, 8 Aug 2024 07:51:59 -0700

On Thu, 8 Aug 2024 17:30:31 +0300 Ilias Apalodimas wrote:
> > we get_device() hoping that it will keep the IOMMU machinery active
> > (even if the device won't use the page we need to unmap it when it's
> > freed), but it sounds like IOMMU dies when driver is unbound. Even if
> > there are outstanding references to the device.
> >
> > I occasionally hit this problem reloading drivers during development,
> > TBH, too. And we have been told we "use the API wrong" so let's fix
> > it on our end?..  
> 
> It's been a while since I looked at the use cases, but I don't love
> the idea of stalling the netdev removal until sockets process all
> packets. There's a chance that the device will stay there forever.

True, my thinking is that there are 3 cases:
 - good case, nothing gets stalled
 - pages held, no IOMMU, we may make it worse, user doesn't care
 - pages held, IOMMU enabled, it would have crashed

given that we get so few reports about the third one, I tend towards
thinking that the risk of stall is somewhat limited.

> I'll have to take a closer look but the first thing that comes to mind
> is to unmap the pages early, before page_pool_destroy() is called and
> perhaps add a flag that says "the pool is there only to process
> existing packets, but you can't DMA into it anymore".

Yeah, but we need to find them... Maybe Alex knows how many rotten veg
will be thrown our way if we try to scan all struct pages, IDK if that's
considered acceptable.