Skip to Content.
Sympa Menu

wg-multicast - Fwd: [streaming] FIXED: Flawed announcement interval computation in miniSAPserver causing SAP storms

Subject: All things related to multicast

List archive

Fwd: [streaming] FIXED: Flawed announcement interval computation in miniSAPserver causing SAP storms


Chronological Thread 
  • From: Marshall Eubanks <>
  • To:
  • Subject: Fwd: [streaming] FIXED: Flawed announcement interval computation in miniSAPserver causing SAP storms
  • Date: Wed, 9 Jul 2008 18:04:47 -0400

I hope this is good news...

Marshall

Begin forwarded message:

From: Zenon Mousmoulas
<>
Date: July 9, 2008 4:41:19 PM EDT
To:

Cc:
,
wg-multicast
<>
Subject: [streaming] FIXED: Flawed announcement interval computation in miniSAPserver causing SAP storms


On 20 Ιουν 2008, at 8:38 ΜΜ,

wrote:


:r324 | courmisch | 2007-03-11 14:10:50 +0200 (Sun, 11 Mar 2007) | 3 lines
:Changed paths:
: M /miniSAPserver/trunk/sapserver.cpp
:
:Fix badly flawed announce delay computation.
:Bug reported by Pierre Beyssac.


the code itself:

unsigned n = config.Programs.size() ?: 1;
div_t d = div ((1000000000 / n) * config.GetDelay(), 1000000000);
struct timespec delay;
delay.tv_sec = d.quot;
delay.tv_nsec = d.rem;

i guess it could get b0rked by n...derived from Programs.size() being
erroneous...


Hi,

Alan was almost right about this one...

The problem is actually that the numerator argument in the div() function is supposed to be an integer, but the expression used there quite easily overflows, for example if n=1 and config.GetDelay returns 5, i.e. using a configuration that has sap_delay=5 and a single [program] section. The declaration of the operands is also a problem. Therefore d takes unpredictable values, which leads to an interval that is waaay off. My initial speculation that miniSAPserver couldn't be causing SAP storms on its' own was regrettably wrong. It can and will take your network down :)

This is certainly a critical issue that should be corrected, so I hacked two slightly different fixes (patches against 0.3.4 attached). The first one uses lldiv() and type casting. The second one just uses 1000 instead of 1000000000, at the expense of lower precision. I would never need to set the interval at a better than millisecond precision though, so for me this is the preferred solution. In any case, I tested both and they take care of the problem.

Earlier today, and unfortunately after I had gone through the above, I literally stumbled across the repository for miniSAPserver:
svn://svn.videolan.org/network/miniSAPserver

I was trying to find what was actually changed in r324 to supposedly fix the delay computation, obviously without success.

Then I came across the last revision:

------------------------------------------------------------------------
r342 | courmisch | 2008-06-30 20:05:19 +0300 (Δευ, 30 Ιον 2008) | 2 lines
Changed paths:
M /miniSAPserver/trunk/NEWS
M /miniSAPserver/trunk/sapserver.cpp

Fix timer computation for real

------------------------------------------------------------------------

Here is what was actually changed:

Index: trunk/sapserver.cpp
===================================================================
--- trunk/sapserver.cpp (revision 341)
+++ trunk/sapserver.cpp (revision 342)
@@ -240,7 +240,7 @@
}

unsigned n = config.Programs.size();
- div_t d = div ((1000000000 / n) * config.GetDelay(), 1000000000);
+ lldiv_t d = lldiv (1000000000LL * config.GetDelay() / n, 1000000000);
struct timespec delay;
delay.tv_sec = d.quot;
delay.tv_nsec = d.rem;

I tested it and I confirm this approach also works. I still prefer my hack #2 though :)

I can see that a new release 0.3.5 is in the works, which will include this fix. It hasn't been packaged and published yet though, so I had no clue this issue had already been addressed.

Anyway, this hopefully means fewer SAP storms for the future, or at least one less source for such unintentional storms...

Best regards,
Z.

Attachment: minisapserver-0.3.4-zmousm-fix1.diff
Description: Binary data

Attachment: minisapserver-0.3.4-zmousm-fix2.diff
Description: Binary data

_______________________________________________
streaming mailing list
To unsubscribe or modify your subscription options:
http://mailman.videolan.org/listinfo/streaming




Archive powered by MHonArc 2.6.16.

Top of Page