Closed
Bug 1198730
Opened 10 years ago
Closed 10 years ago
DataChannel PMTUD disable clears other flags by accident
Categories
(Core :: WebRTC: Networking, defect, P1)
Tracking
()
RESOLVED
FIXED
mozilla43
Tracking | Status | |
---|---|---|
firefox40 | --- | unaffected |
firefox41 | + | fixed |
firefox42 | --- | fixed |
firefox43 | --- | fixed |
firefox-esr31 | --- | unaffected |
firefox-esr38 | --- | unaffected |
backlog | webrtc/webaudio+ |
People
(Reporter: jesup, Assigned: jesup)
References
(Blocks 1 open bug)
Details
(Keywords: coverity, Whiteboard: [CID 1320439])
Attachments
(1 file)
1.50 KB,
patch
|
tuexen
:
review+
ritu
:
approval-mozilla-aurora+
ritu
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
+++ 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 ~
Assignee | ||
Comment 1•10 years ago
|
||
Attachment #8652836 -
Flags: review?(tuexen)
Assignee | ||
Updated•10 years ago
|
backlog: --- → webrtc/webaudio+
Rank: 10
Updated•10 years ago
|
Attachment #8652836 -
Flags: review?(tuexen) → review+
Assignee | ||
Updated•10 years ago
|
Summary: Certain high-traffic uses of DataChannels/sctp fail when the packets get too large due to PMTU → DataChannel PMTUD disable clears other flags by accident
Assignee | ||
Comment 2•10 years ago
|
||
Wow, that was fast. :-)
Comment 4•10 years ago
|
||
Sitting in a meeting...
Comment 5•10 years ago
|
||
Tracked for 41 to make sure we don't forget the uplift (but knowing Randell, I know this won't be necessary)
tracking-firefox41:
--- → +
Assignee | ||
Comment 6•10 years ago
|
||
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
Attachment #8652836 -
Flags: approval-mozilla-beta?
Attachment #8652836 -
Flags: approval-mozilla-aurora?
Comment 7•10 years ago
|
||
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla43
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.
Attachment #8652836 -
Flags: approval-mozilla-beta?
Attachment #8652836 -
Flags: approval-mozilla-beta+
Attachment #8652836 -
Flags: approval-mozilla-aurora?
Attachment #8652836 -
Flags: approval-mozilla-aurora+
Comment 9•10 years ago
|
||
Comment 10•10 years ago
|
||
Updated•7 years ago
|
Blocks: coverity-analysis
You need to log in
before you can comment on or make changes to this bug.
Description
•