1198730 - DataChannel PMTUD disable clears other flags by accident

Status: RESOLVED FIXED

(Core :: WebRTC: Networking, defect, P1)

Description

[Randell Jesup [:jesup] (needinfo me)]

+++ This bug was initially created as a clone of Bug #1194817 +++

From bug 1194817 comment 11:
> Coverity complains about this patch:
> CID 1320439 (#1 of 1): Logical vs. bitwise operator
> (CONSTANT_EXPRESSION_RESULT)logical_vs_bitwise: paddrparams.spp_flags &= 0U /* !8 */ always assigns 0 to paddrparams.spp_flags.
>    Did you intend to use '~' rather than '!'?

yes, it should be ~

Comment 1

[Randell Jesup [:jesup] (needinfo me)]

Attachment: fix simple bug in PMTUD disable that clears all other flags

Comment 2

[Randell Jesup [:jesup] (needinfo me)]

Wow, that was fast.  :-)

Comment 3

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/dda56b1c5210

Comment 4

[Michael Tüxen]

Sitting in a meeting...

Comment 5

[Sylvestre Ledru [:Sylvestre]]

Tracked for 41 to make sure we don't forget the uplift (but knowing Randell, I know this won't be necessary)

Comment 6

[Randell Jesup [:jesup] (needinfo me)]

Comment on attachment 8652836 [details] [diff] [review]
fix simple bug in PMTUD disable that clears all other flags

Approval Request Comment
[Feature/regressing bug #]: bug 1194817

[User impact if declined]: Kills the heartbeat, which catches (eventually) link failure.  Lack of heartbeat will not cause major loss of functionality, however.

[Describe test coverage new/current, TreeHerder]: DataChannels are hit in testing

[Risks and why]: virtually no risk
Note: there's an upstream bug with the spp flags reported by sctp_getopt() that could cause problem if we *later* did getopt/setopt on a different flag; however we don't do that (confirmed with GDB), so confirmed working and safe in operation.

[String/UUID change made/needed]: none

Comment 7

[Ryan VanderMeulen [:RyanVM]]

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

Comment 8

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Comment on attachment 8652836 [details] [diff] [review]
fix simple bug in PMTUD disable that clears all other flags

Safe to uplift to Beta and Aurora. Nice to this was caught by Coverity's static code analysis.

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/50d43aa31d3d

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/2be849821652