Worth caching allowed IP list in middleware?

Yes, this is a problem and should not be done. IMHO the previous issue should be reverted, this is a significant performance regression that breaks major performance improvements in Drupal 11.3.

the fastest option would be to use a service provider and persist the information directly as a parameter to the service. However, that requires a container rebuild then, which is expensive. That *probably* doesn't happen often, but it's still a downside.

Config is already cached, the problem is that ConfigFactory is a pretty heavy dependency.

I also commented in #3579646: Injecting the config factory inside the middleware causes performance hit, which doesn't help this.

As mentioned above, my suggestion would be to revert this feature completely and consider performance from the start. Working with middlewares means that all changes must be carefully tested and profiled. I'd suggest adding a performance cache on a page cache hit response. That would not fully cover this, but it would at least point out the fact that this adds extra cache lookups.

I didn't carefully read the original issue and the requirements, I was also wondering if that's a thing that you'd want to change frequently and immediately or if a deployment/file change is acceptable, then I think either settings.php or a container parameter would be fine yes.

I did have a quick look at the original issue now, but I'm not sure I really know more as there isn't a lot of detail/discussion.

What exactly is the use case I wonder? Unless you use something like perimeter or some other similar logic, IP's aren't suddenly going to appear in the banned IP list. Against what is that protection/allowance? Does it need to be at runtime? Would it make more sense to apply the allow-list to blocking an IP and disallow that?

Similar to auto_unban, if it's applied at banIp() and not isBanned(), then the closure is useful (it would be on a different service though) However, that won't automatically work with a module like auto_unban as that overrides that method.

Depending on the hosting environment settings.php could be configured to read from a text file, env var or some other way (e.g. Pantheon secrets) to avoid deployments to add to the whitelist. We could make note of that in any documentation, although this applies pretty generally to anything else.

I'm fine with something settings or container parameter based.

Unsure when it should apply, on check or on ban, or or both. *If* the cost of checking the IP is near-zero (like settings.php), then there's even a performance improvement possible thanks to this, as the query doesn't need to run (as a follow-ip, it might be worth exploring to make either the ban ip manager in the middleware or the database connection in the ban ip manager a closure. Unfortunately, Drupal already opens the database connection when you inject the service and with this, there's a scenario where no query happens)

And yes, once such a setting exists, other modules could support it and don't block those IP's in the first place.

I'd still suggest opening a separate issue to add a performance test. It's fairly simple, see \Drupal\Tests\redirect\FunctionalJavascript\RedirectPerformanceTest for one in contrib, or the various core subclasses which do detailed query/cache statistics. The main challenge for a contrib module is that changes in core affect the results and it could be tedious to maintain and work on all supported core versions. That's why redirect only asserts it's own queries and ban could just check a page cache hit, where changes in core are quite unlikely. You'd not want to assert assets, just number of queries/cache statistics.