All things related to multicast

[Marshall Eubanks <>]

On Sep 3, 2008, at 10:47 AM, Bruce Rodger wrote:

> On Wed, 3 Sep 2008, Frank Fulchiero wrote:
>
> > Also, didn't someone fix this miniSAPserver bug, and make the  
> > patch, or newer version, available?
> > Sorry, I forgot...
>
> well, if they did, it never made it's way back to videolan.org...
>
> I will log it in the videolan bug tracker.
>
> Bruce.
Yes, they did, back in July - see below. Are you sure that the current  
spike is due to the miniSAPserver ?

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