drouting: intentional behavior or dead code? random GW selection commented out in weight_based_sort() when weight_sum==0

[wlp2s0]

OpenSIPS version you are running

(any version including current master)

Describe the bug

While reading the weight_based_sort() function in the drouting module, I noticed that the random gateway selection in the weight_sum == 0 branch is commented out:

} else {
    /* randomly select index */
    //  i = (unsigned int)((size-first)*((double)rand()/((double)RAND_MAX)));
    i = first;
}

https://github.com/OpenSIPS/opensips/blob/master/modules/drouting/drouting.c#L2974

This means that when all gateways in the current subset have weight 0, the algorithm always picks the first element (i = first), making the sort fully deterministic and always preferring whichever gateway appears first in the list. If the weighted sort algorithm is selected, one would expect gateways with equal weight (including weight 0) to be picked at random with uniform distribution, but that is not happening.

To Reproduce

1. Configure a drouting rule with two or more gateways, all with weight 0 and the weight-based sort algorithm selected
2. Send traffic through the rule repeatedly
3. Notice that the first gateway in the list is always selected, regardless of how many gateways are configured

Expected behavior

When all gateways in a subset have weight 0, they should be treated as equal and one should be picked at random, providing uniform distribution across all of them. This is consistent with the general intent of weight-based sorting.

If the commented line were active, that would be the behavior. The corrected version of the line should also be consistent with the fix introduced in PR #3020, i.e. using double and RAND_MAX+1 to avoid rounding errors:

i = first + (unsigned int)((size - first) * ((double)rand() / ((double)RAND_MAX + 1.0)));

Relevant System Logs

No relevant logs, the behavior is observable purely from traffic distribution.

OS/environment information

• Operating System: any
• OpenSIPS installation: any

Additional context

I searched through the git log, related issues and PRs but could not find any explicit discussion explaining why this line was commented out. The most recent change to this function was PR #3020, which fixed a bug in the weight_sum > 0 branch but did not touch the zero-weight case. The older issue #108 addressed unbalanced random selection but also does not mention this case.

If i = first is actually the intended behavior (e.g. weight 0 meaning "no load balancing, always use the first configured gateway"), it would be helpful to have a short comment in the source explaining that, to avoid confusion. Thanks!

[bogdan-iancu]

Hi @wlp2s0 , I do not recall the whole history here (here is the commit 763ac6c changing that, but a bit useless), but it may be related to the consistence around what weight 0 means. If a GW is listed with weight 0 in a list with other GWs with non-zero weights, this 0-weight will never be selected... So 0-weight may translate into "do not select me"...

[wlp2s0]

Hi @bogdan-iancu, thanks for looking into this and for the commit reference!

I see your point about the mixed case and I agree. If you have something like gw1=10, gw2=0, gw3=10, then gw2 never gets selected because its weight adds nothing to the running sum. That makes total sense and it already works correctly in the weight_sum > 0 branch.

The thing is, the weight_sum == 0 branch tells a different story. If "weight 0" really means "do not select me", then when all gateways have weight 0, none of them should be selected at all, and the function should probably return an error or skip them. But instead the code just silently picks the first one every time (i = first). So it is not really "do not select me" semantics, it is more of an "always pick the first gateway in the list" which feels more like an accidental fallback than an intentional design choice.

I think the more important point in practice though is that the most common real-world scenario for weight_sum == 0 is probably just a user who configured their gateways without specifying weights, e.g. gwlist = "gw1, gw2, gw3" with sort algorithm W. Since pgwl is zeroed out with memset in parse_destination_list(), unspecified weights end up as 0. That user almost certainly expects equal load balancing across the three gateways, but what they actually get is 100% of the traffic going to gw1.

Looking at the commit you mentioned, the change is bundled with the gw_priprefix_avp work and there is no mention of the sort logic change in the message, so it might have just slipped in without much thought behind it.

Anyway, I would propose just uncommenting the random selection, applying the same double / RAND_MAX+1 correction from PR #3020, and adding the first offset that was actually missing in the original line too.

} else {
    /* all remaining gateways have weight 0, treat them as equal
       and randomly select one with uniform distribution */
    i = first + (unsigned int)((size - first) * ((double)rand() / ((double)RAND_MAX + 1.0)));
}

It is really just a one-line change and it does not touch the weight_sum > 0 path at all. I can put together a PR if you are ok with the approach, just let me know!

[bogdan-iancu]

@wlp2s0 , could you prepare a PR for the change you suggested ? Thanks