rdisc: fix double-addition of gateways & routes if priority increases

If a route or gateway's priority increased, the item would be added
to the array again without removing the older entry.  At the same time
don't bother adding an item with a zero lifetime, since it will just
be removed again by the clean_* functions.
This commit is contained in:
Dan Williams 2015-04-09 17:46:31 -05:00
parent 39fd8f7683
commit 415b7b3e25
2 changed files with 131 additions and 29 deletions

View file

@ -56,27 +56,34 @@ static guint signals[LAST_SIGNAL] = { 0 };
gboolean
nm_rdisc_add_gateway (NMRDisc *rdisc, const NMRDiscGateway *new)
{
int i;
int i, insert_idx = -1;
for (i = 0; i < rdisc->gateways->len; i++) {
NMRDiscGateway *item = &g_array_index (rdisc->gateways, NMRDiscGateway, i);
if (IN6_ARE_ADDR_EQUAL (&item->address, &new->address)) {
if (new->lifetime == 0) {
g_array_remove_index (rdisc->gateways, i--);
return TRUE;
}
if (item->preference != new->preference) {
g_array_remove_index (rdisc->gateways, i--);
continue;
}
memcpy (item, new, sizeof (*new));
return FALSE;
}
/* Put before less preferable gateways. */
if (item->preference < new->preference)
break;
if (item->preference < new->preference && insert_idx < 0)
insert_idx = i;
}
g_array_insert_val (rdisc->gateways, i, *new);
return TRUE;
if (new->lifetime)
g_array_insert_val (rdisc->gateways, CLAMP (insert_idx, 0, G_MAXINT), *new);
return !!new->lifetime;
}
gboolean
@ -88,9 +95,15 @@ nm_rdisc_add_address (NMRDisc *rdisc, const NMRDiscAddress *new)
NMRDiscAddress *item = &g_array_index (rdisc->addresses, NMRDiscAddress, i);
if (IN6_ARE_ADDR_EQUAL (&item->address, &new->address)) {
gboolean changed = item->timestamp + item->lifetime != new->timestamp + new->lifetime ||
item->timestamp + item->preferred != new->timestamp + new->preferred;
gboolean changed;
if (new->lifetime == 0) {
g_array_remove_index (rdisc->addresses, i--);
return TRUE;
}
changed = item->timestamp + item->lifetime != new->timestamp + new->lifetime ||
item->timestamp + item->preferred != new->timestamp + new->preferred;
*item = *new;
return changed;
}
@ -103,34 +116,42 @@ nm_rdisc_add_address (NMRDisc *rdisc, const NMRDiscAddress *new)
if (rdisc->max_addresses && rdisc->addresses->len >= rdisc->max_addresses)
return FALSE;
g_array_insert_val (rdisc->addresses, i, *new);
return TRUE;
if (new->lifetime)
g_array_insert_val (rdisc->addresses, i, *new);
return !!new->lifetime;
}
gboolean
nm_rdisc_add_route (NMRDisc *rdisc, const NMRDiscRoute *new)
{
int i;
int i, insert_idx = -1;
for (i = 0; i < rdisc->routes->len; i++) {
NMRDiscRoute *item = &g_array_index (rdisc->routes, NMRDiscRoute, i);
if (IN6_ARE_ADDR_EQUAL (&item->network, &new->network) && item->plen == new->plen) {
if (new->lifetime == 0) {
g_array_remove_index (rdisc->routes, i--);
return TRUE;
}
if (item->preference != new->preference) {
g_array_remove_index (rdisc->routes, i--);
continue;
}
memcpy (item, new, sizeof (*new));
return FALSE;
}
/* Put before less preferable routes. */
if (item->preference < new->preference)
break;
if (item->preference < new->preference && insert_idx < 0)
insert_idx = i;
}
g_array_insert_val (rdisc->routes, i, *new);
return TRUE;
if (new->lifetime)
g_array_insert_val (rdisc->routes, CLAMP (insert_idx, 0, G_MAXINT), *new);
return !!new->lifetime;
}
gboolean
@ -142,25 +163,21 @@ nm_rdisc_add_dns_server (NMRDisc *rdisc, const NMRDiscDNSServer *new)
NMRDiscDNSServer *item = &g_array_index (rdisc->dns_servers, NMRDiscDNSServer, i);
if (IN6_ARE_ADDR_EQUAL (&item->address, &new->address)) {
gboolean changed;
if (new->lifetime == 0) {
g_array_remove_index (rdisc->dns_servers, i);
return TRUE;
}
changed = (item->timestamp != new->timestamp ||
item->lifetime != new->lifetime);
if (changed) {
item->timestamp = new->timestamp;
item->lifetime = new->lifetime;
if (item->timestamp != new->timestamp || item->lifetime != new->lifetime) {
*item = *new;
return TRUE;
}
return changed;
return FALSE;
}
}
g_array_insert_val (rdisc->dns_servers, i, *new);
return TRUE;
if (new->lifetime)
g_array_insert_val (rdisc->dns_servers, i, *new);
return !!new->lifetime;
}
/* Copies new->domain if 'new' is added to the dns_domains list */
@ -191,10 +208,12 @@ nm_rdisc_add_dns_domain (NMRDisc *rdisc, const NMRDiscDNSDomain *new)
}
}
g_array_insert_val (rdisc->dns_domains, i, *new);
item = &g_array_index (rdisc->dns_domains, NMRDiscDNSDomain, i);
item->domain = g_strdup (new->domain);
return TRUE;
if (new->lifetime) {
g_array_insert_val (rdisc->dns_domains, i, *new);
item = &g_array_index (rdisc->dns_domains, NMRDiscDNSDomain, i);
item->domain = g_strdup (new->domain);
}
return !!new->lifetime;
}
/******************************************************************/

View file

@ -262,6 +262,88 @@ test_everything (void)
g_main_loop_unref (data.loop);
}
static void
test_preference_changed (NMRDisc *rdisc, NMRDiscConfigMap changed, TestData *data)
{
if (data->counter == 1) {
g_assert_cmpint (changed, ==, NM_RDISC_CONFIG_GATEWAYS |
NM_RDISC_CONFIG_ADDRESSES |
NM_RDISC_CONFIG_ROUTES);
g_assert_cmpint (rdisc->gateways->len, ==, 2);
match_gateway (rdisc->gateways, 0, "fe80::2", data->timestamp1 + 1, 10, NM_RDISC_PREFERENCE_MEDIUM);
match_gateway (rdisc->gateways, 1, "fe80::1", data->timestamp1, 10, NM_RDISC_PREFERENCE_LOW);
g_assert_cmpint (rdisc->addresses->len, ==, 2);
match_address (rdisc->addresses, 0, "2001:db8:a:a::1", data->timestamp1, 10, 10);
match_address (rdisc->addresses, 1, "2001:db8:a:a::2", data->timestamp1 + 1, 10, 10);
g_assert_cmpint (rdisc->routes->len, ==, 2);
match_route (rdisc->routes, 0, "2001:db8:a:b::", 64, "fe80::2", data->timestamp1 + 1, 10, 10);
match_route (rdisc->routes, 1, "2001:db8:a:a::", 64, "fe80::1", data->timestamp1, 10, 5);
} else if (data->counter == 2) {
g_assert_cmpint (changed, ==, NM_RDISC_CONFIG_GATEWAYS |
NM_RDISC_CONFIG_ADDRESSES |
NM_RDISC_CONFIG_ROUTES);
g_assert_cmpint (rdisc->gateways->len, ==, 2);
match_gateway (rdisc->gateways, 0, "fe80::1", data->timestamp1 + 2, 10, NM_RDISC_PREFERENCE_HIGH);
match_gateway (rdisc->gateways, 1, "fe80::2", data->timestamp1 + 1, 10, NM_RDISC_PREFERENCE_MEDIUM);
g_assert_cmpint (rdisc->addresses->len, ==, 2);
match_address (rdisc->addresses, 0, "2001:db8:a:a::1", data->timestamp1 + 2, 10, 10);
match_address (rdisc->addresses, 1, "2001:db8:a:a::2", data->timestamp1 + 1, 10, 10);
g_assert_cmpint (rdisc->routes->len, ==, 2);
match_route (rdisc->routes, 0, "2001:db8:a:a::", 64, "fe80::1", data->timestamp1 + 2, 10, 15);
match_route (rdisc->routes, 1, "2001:db8:a:b::", 64, "fe80::2", data->timestamp1 + 1, 10, 10);
g_assert (nm_fake_rdisc_done (NM_FAKE_RDISC (rdisc)));
g_main_loop_quit (data->loop);
}
data->counter++;
}
static void
test_preference (void)
{
NMFakeRDisc *rdisc = rdisc_new ();
guint32 now = nm_utils_get_monotonic_timestamp_s ();
TestData data = { g_main_loop_new (NULL, FALSE), 0, 0, now };
guint id;
/* Test that when a low-preference and medium gateway send advertisements,
* that if the low-preference gateway switches to high-preference, we do
* not get duplicates in the gateway list.
*/
id = nm_fake_rdisc_add_ra (rdisc, 1, NM_RDISC_DHCP_LEVEL_NONE, 4, 1500);
g_assert (id);
nm_fake_rdisc_add_gateway (rdisc, id, "fe80::1", now, 10, NM_RDISC_PREFERENCE_LOW);
nm_fake_rdisc_add_address (rdisc, id, "2001:db8:a:a::1", now, 10, 10);
nm_fake_rdisc_add_route (rdisc, id, "2001:db8:a:a::", 64, "fe80::1", now, 10, 5);
id = nm_fake_rdisc_add_ra (rdisc, 1, NM_RDISC_DHCP_LEVEL_NONE, 4, 1500);
g_assert (id);
nm_fake_rdisc_add_gateway (rdisc, id, "fe80::2", ++now, 10, NM_RDISC_PREFERENCE_MEDIUM);
nm_fake_rdisc_add_address (rdisc, id, "2001:db8:a:a::2", now, 10, 10);
nm_fake_rdisc_add_route (rdisc, id, "2001:db8:a:b::", 64, "fe80::2", now, 10, 10);
id = nm_fake_rdisc_add_ra (rdisc, 1, NM_RDISC_DHCP_LEVEL_NONE, 4, 1500);
g_assert (id);
nm_fake_rdisc_add_gateway (rdisc, id, "fe80::1", ++now, 10, NM_RDISC_PREFERENCE_HIGH);
nm_fake_rdisc_add_address (rdisc, id, "2001:db8:a:a::1", now, 10, 10);
nm_fake_rdisc_add_route (rdisc, id, "2001:db8:a:a::", 64, "fe80::1", now, 10, 15);
g_signal_connect (rdisc,
NM_RDISC_CONFIG_CHANGED,
G_CALLBACK (test_preference_changed),
&data);
nm_rdisc_start (NM_RDISC (rdisc));
g_main_loop_run (data.loop);
g_assert_cmpint (data.counter, ==, 3);
g_object_unref (rdisc);
g_main_loop_unref (data.loop);
}
NMTST_DEFINE ();
int
@ -278,6 +360,7 @@ main (int argc, char **argv)
g_test_add_func ("/rdisc/simple", test_simple);
g_test_add_func ("/rdisc/everything-changed", test_everything);
g_test_add_func ("/rdisc/preference-changed", test_preference);
return g_test_run ();
}