Issue52

Title the match for client prefix/mirror prefix should check for containment, not string match
Priority bug Status resolved
Superseder Nosy List poeml
Assigned To poeml Keywords

Created on 2010-04-22.23:38:12 by poeml, last changed by poeml.

Messages
msg203 (view) Author: poeml Date: 2010-09-01.15:53:42
I'm counting this as successfully resolved. The fix is in productive use since 
quite a while now; it'll be in the upcoming 2.13.0 release.

Thanks!
msg179 (view) Author: poeml Date: 2010-04-23.02:02:40
I'll keep this open and marked "in testing", upon positive confirmation that the 
fix does its job.
msg178 (view) Author: poeml Date: 2010-04-23.01:56:18
A fix has been committed to trunk (r8040).

http://svn.mirrorbrain.org/viewvc/mirrorbrain/trunk/mod_mirrorbrain/mod_mirrorbrai
n.c?r1=8040&r2=8039&pathrev=8040
msg177 (view) Author: poeml Date: 2010-04-22.23:38:11
We had a case where a client should be redirected to a local mirror, which lives in the same network 
prefix, but it didn't work. The clients prefix is:

 # mb iplookup 190.104.158.107
190.104.158.0/24 (AS27895)

The mirror which should be chosen is:

 # mb iplookup mirrors.paraguayeduca.org
190.104.152.0/23 (AS27895)

So the narrowst prefix that contains 190.104.158.107 is 190.104.158.0/24, but the IP is also contained 
in 190.104.152.0/23, which is the narrowst prefix containing the mirror IP.

The problem is that mod_mirrorbrain has this data as strings and does an (exact) string match:

/* same prefix? */
else if (strcmp(new->prefix, prefix) == 0) {
    *(void **)apr_array_push(mirrors_same_prefix) = new;
}

We should probably switch to checking for containment of the IP in the mirrors prefix. That could be 
done using the Apache Portable Runtime functions apr_ipsubnet_create() and apr_ipsubnet_test(). They 
have the benefit of dealing with IPv6 as well as IPv4 (and mapped addresses), and they are likely fast.

We'd have to call those functions for each mirror for each client, but I looked at them and the added 
overhead should be comparably small. If it turns out to be noticeable, it'd be possible to store the 
network block address of each mirror in the database as a number in its entry or so...
History
Date User Action Args
2010-09-01 15:53:43poemlsetstatus: testing -> resolved
messages: + msg203
2010-04-23 02:02:40poemlsetstatus: in-progress -> testing
messages: + msg179
2010-04-23 01:56:19poemlsetmessages: + msg178
2010-04-22 23:39:11poemlsettitle: mod_asn: the match for client prefix/mirror prefix should check for containment, not string match -> the match for client prefix/mirror prefix should check for containment, not string match
2010-04-22 23:38:56poemlsetkeyword: - mod_asn
2010-04-22 23:38:12poemlcreate