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)

42 Branch
defect

Tracking

()

RESOLVED FIXED
mozilla43
Tracking Status
firefox40 --- unaffected
firefox41 + fixed
firefox42 --- fixed
firefox43 --- fixed
firefox-esr31 --- unaffected
firefox-esr38 --- unaffected

People

(Reporter: jesup, Assigned: jesup)

References

(Blocks 1 open bug)

Details

(Keywords: coverity, Whiteboard: [CID 1320439])

Attachments

(1 file)

+++ 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 ~
backlog: --- → webrtc/webaudio+
Rank: 10
Keywords: coverity
Whiteboard: [CID 1320439]
Attachment #8652836 - Flags: review?(tuexen) → review+
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
Wow, that was fast. :-)
Sitting in a meeting...
Tracked for 41 to make sure we don't forget the uplift (but knowing Randell, I know this won't be necessary)
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?
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+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: