bneradt opened a new issue, #13694:
URL: https://github.com/apache/trafficserver/issues/13694
### Problem
`proxy.config.http2.flow_control.policy_in` and `policy_out` are declared
`RECU_DYNAMIC` and documented as `:reloadable:`, but HTTP/2 keeps the policy
selected at process startup. `traffic_ctl config set` reports that a restart is
not required, and `config get` reports the new value even though new
connections still use the old policy.
### Reproduction and observed behavior
I reproduced the inbound case in a local master-based build at 11e11c77e9
(the branch of #13447, based on 1e884fae05). Current master at 804e5e2dd1 still
has the same policy initialization and record declarations. The relevant
configuration is:
```yaml
records:
http2:
initial_window_size_in: 65535
max_concurrent_streams_in: 100
flow_control:
policy_in: 0
```
1. Start ATS and open a fresh TLS/H2 connection. Send the HTTP/2 connection
preface and an empty SETTINGS frame; record connection-level WINDOW_UPDATE
increments.
2. Run `traffic_ctl config set proxy.config.http2.flow_control.policy_in 1`.
It says to wait for synchronization and that a restart is not required.
3. Wait more than 10 seconds, confirm `traffic_ctl config get
proxy.config.http2.flow_control.policy_in` returns `1`, and repeat the probe on
a **new** connection.
4. Persist `policy_in: 1` in records.yaml, restart ATS, and repeat the probe.
| State | Connection WINDOW_UPDATE increments | Effective connection receive
window |
|---|---:|---:|
| Started with policy 0 | none | 65,535 |
| Runtime value reports 1 | none | 65,535 |
| Restarted with policy 1 | 6,487,965 | 6,553,500 |
The restart control distinguishes this from existing connections retaining
their original settings. I measured the inbound policy on the wire; the
outbound policy uses the same defective initialization pattern described below.
### Cause
In
[`Http2::init()`](https://github.com/apache/trafficserver/blob/804e5e2dd1/src/proxy/http2/HTTP2.cc#L521-L537),
each policy is read through a function-local `uint32_t`, then copied once to
the static `Http2FlowControlPolicy` member:
```cpp
uint32_t flow_control_policy_in_int = 0;
RecEstablishStaticConfigUInt32(flow_control_policy_in_int,
"proxy.config.http2.flow_control.policy_in");
// validation ...
flow_control_policy_in =
static_cast<Http2FlowControlPolicy>(flow_control_policy_in_int);
```
`RecEstablishStaticConfigUInt32` also registers `RecLinkConfigUInt32` using
the supplied variable's address. The callback therefore retains a pointer to a
local variable whose lifetime ends when `Http2::init()` returns. It does not
update the static enum used by HTTP/2. The outbound policy repeats this pattern.
### Expected behavior
A configuration update should safely validate and update storage with
sufficient lifetime, and new connections should use the new policy. If runtime
policy changes are intentionally unsupported, the records and documentation
should instead require a restart, without registering callbacks to local
variables.
Persisting the value and restarting ATS is the current workaround.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
