1475420 - TRR timeout when using TRR to open TRR channel

Status: RESOLVED FIXED

(Core :: Networking: DNS, defect)

Description

[Patrick McManus [:mcmanus]]

the initial trr channel falls back to native dns beacuse the NS check has never passed.

however, its possible to get into a state where
 1] there is no open trr channel
 2] the origin is not in the dns cache
 3] the ns check succeeded in the past

and then we get into a deadlock of the trr channel trying to look itself up using trr. it times out and life goes on - this is a little more obvious on the esni branch but this is a general trr bug.

this patch marks the TRR channel as DISABLE_TRR to avoid the loop.

I confirmed the patch works ok in TRR_ONLY mode because the bootstrap code doesn't look at channel flags at all.

Comment 1

[Patrick McManus [:mcmanus]]

Attachment: Bug 1475420 - mark the TRR connection as DISABLE_TRR to avoid TXT deadlock

Review commit: https://reviewboard.mozilla.org/r/256706/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/256706/

Comment 2

[Patrick McManus [:mcmanus]]

Comment on attachment 8991780 [details]
Bug 1475420 - mark the TRR connection as DISABLE_TRR to avoid TXT deadlock

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/256706/diff/1-2/

Comment 3

[Valentin Gosu [:valentin] (he/him)]

Comment on attachment 8991780 [details]
Bug 1475420 - mark the TRR connection as DISABLE_TRR to avoid TXT deadlock

https://reviewboard.mozilla.org/r/256706/#review263660

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/cbcbae48a940
mark the TRR connection as DISABLE_TRR to avoid TXT deadlock r=valentin

Comment 5

[Noemi Erli[:noemi_erli]]

https://hg.mozilla.org/mozilla-central/rev/cbcbae48a940

Comment 6

[Daniel Stenberg [:bagder]]

Comment on attachment 8991780 [details]
Bug 1475420 - mark the TRR connection as DISABLE_TRR to avoid TXT deadlock

https://reviewboard.mozilla.org/r/256706/#review263876

Comment 7

[Patrick McManus [:mcmanus]]

Comment on attachment 8991780 [details]
Bug 1475420 - mark the TRR connection as DISABLE_TRR to avoid TXT deadlock

Approval Request Comment
[Feature/Bug causing the regression]: TRR (DoH) - original bug on 62, not a regression
[User impact if declined]: pauses in DNS resolution when DoH is enabled, especially after hard refreshes
[Is this code covered by automated tests?]: yes, though the timing is not verified
[Has the fix been verified in Nightly?]: yes
[Needs manual test from QE? If yes, steps to reproduce]:  no
[List of other uplifts needed for the feature/fix]: na
[Is the change risky?]: no
[Why is the change risky/not risky?]: DoH is not enabled by default on 62, but we'd like to do pref/shield studies that target it
[String changes made/needed]: none

Comment 8

[Liz Henry (:lizzard) (relman/hg->git project)]

Comment on attachment 8991780 [details]
Bug 1475420 - mark the TRR connection as DISABLE_TRR to avoid TXT deadlock

Looks like DoH studies are coming soon for 63, while I don't see anything in the works for 62, this seems OK to uplift in case we plan one during 62 release.

Comment 9

[Patrick McManus [:mcmanus]]

thanks! some background - the goal is to have the 62 doh code be as complete as the 63 code. Primarily to support manual testing. (obviously there are limits to that, but this is a low risk change).

There will likely be a beta-based DoH study on the horizon - though I suspect beta will be on 63 when that happens..  but its nice to be prepared just in case.

Comment 10

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/releases/mozilla-beta/rev/5bf0dbc446bb