CC @envoyproxy/api-shepherds: Your approval is needed for changes made to api/.

http inspector listener has the exact same issue. Will apply to http_inspector once approved.

Request for comment: @htuch @rshriram

CC @envoyproxy/api-shepherds: Your approval is needed for changes made to api/.

@lizan
I think It's hard to say if envoy could bypass the opinion of each listener filter. It's quite different from the decision of rejecting the connection.
If any listener filter cannot tolerate the bypass, we should not pass. Where do you provide the chance for those listener filters to say no?

Do you really need that level of granularity though? Even in Istio, we are most likely to only ever set the same timeout value for every filter chain. There is actually no scenario where we would set the timeout for one chain but not for the other because we do not know the protocol apriori.

IMO it is easy to go from a global option to a more fine grained option (local overrides global), than it is to go from a local only option to a global option, IMO. I dont mind either implementation - which ever is quicker works!

@lizan Sorry I may misinterpret your comment. I total agree if there is a single timeout for the listener.

What I am proposed is that if any listener filter cannot follow the fallback timeout, that listener filter can block the onAccept, until envoy decide to close the connection

Let me pull up my WIP branch to a Draft PR soon.

I think It's hard to say if envoy could bypass the opinion of each listener filter. It's quite different from the decision of rejecting the connection.
If any listener filter cannot tolerate the bypass, we should not pass. Where do you provide the chance for those listener filters to say no?

We don't do this today, a StopIteration from listener filters today just means it wants more data, it is not an intent to decline a connection. We can always provide closing function via ListenerFilterCallbacks, which actively close the connection, if there is any desire for that.

We can always provide closing function via ListenerFilterCallbacks, which actively close the connection, if there is any desire for that.

Actually this is what continueFilterChain(false) do.

I read through #7859 It pushes the flag continueOnListenerFiltersTimeout to listener but we do have something.
I have small concerns:

• Ideally the timeout to close connection and timeout to continue could be 2 things... But istio survives without a timeout to close. I am fine with single timeout value :)
• Previously it's either close the connection or envoy go through each listener filter so that each listener could have the opportunity to clean up in onAccept, or on file event. E.g. tls_inspector may created a file event and waiting to be triggered, with your current PR that file event is leaked or somehow triggered later. IMHO it continueOnListenerFiltersTimeout should do something before call newConnection().

I believe #7859 will eventually address the my concern. Close this PR

your current PR that file event is leaked or somehow triggered later. IMHO it continueOnListenerFiltersTimeout should do something before call newConnection().

I'm spotting this working on tests, should be fixed.

@lizan Thanks!