Skip to content

Commit

Permalink
Merge pull request #982
Browse files Browse the repository at this point in the history
ifconfig: fix arp notify loop (boo#1212806) and burst sending
  • Loading branch information
mtomaschewski committed Jul 4, 2023
2 parents 4749fef + 5980177 commit 286008b
Show file tree
Hide file tree
Showing 4 changed files with 56 additions and 20 deletions.
17 changes: 16 additions & 1 deletion src/appconfig.c
Expand Up @@ -75,6 +75,7 @@ static void ni_config_addrconf_arp_default(ni_config_arp_t *);
#define NI_ADDRCONF_ARP_NOTIFY_INTERVAL 300 /* ANNOUNCE_INTERVAL */
#define NI_ADDRCONF_ARP_INTERVAL_MIN 100
#define NI_ADDRCONF_ARP_MAX_DURATION 15000
#define NI_ADDRCONF_ARP_COUNT_MIN 1

/*
* Create an empty config object
Expand Down Expand Up @@ -1316,15 +1317,23 @@ static ni_bool_t
ni_config_parse_addrconf_arp_verify(ni_config_arp_verify_t *verify, const xml_node_t *node)
{
xml_node_t *child;
unsigned int tmp;
ni_config_arp_verify_t vfy_tmp = *verify;

for (child = node->children; child; child = child->next) {
if (ni_string_eq(child->name, "count")) {
if (ni_parse_uint(child->cdata, &vfy_tmp.count, 0) < 0) {
if (ni_parse_uint(child->cdata, &tmp, 0) < 0) {
ni_warn("[%s] unable to parse <count>%s</count>",
xml_node_location(child), child->cdata);
continue;
}
if (tmp < NI_ADDRCONF_ARP_COUNT_MIN) {
ni_warn("[%s] ignore invalid count value %u, min: %d",
xml_node_location(child), tmp,
NI_ADDRCONF_ARP_COUNT_MIN);
continue;
}
vfy_tmp.count = tmp;
}

if (ni_string_eq(child->name, "interval")) {
Expand Down Expand Up @@ -1393,6 +1402,12 @@ ni_config_parse_addrconf_arp_notify(ni_config_arp_notify_t *notify, const xml_no
xml_node_location(child), child->cdata);
continue;
}
if (tmp < NI_ADDRCONF_ARP_COUNT_MIN) {
ni_warn("[%s] ignore invalid count value %u, min: %d",
xml_node_location(child), tmp,
NI_ADDRCONF_ARP_COUNT_MIN);
continue;
}
nfy_tmp.count = tmp;
}

Expand Down
26 changes: 23 additions & 3 deletions src/arp.c
Expand Up @@ -232,6 +232,7 @@ static ni_define_ptr_array_init(ni_arp_address);
static ni_define_ptr_array_destroy(ni_arp_address);
static ni_define_ptr_array_realloc(ni_arp_address, NI_ARP_ADDRESS_ARRAY_CHUNK);
static ni_define_ptr_array_append(ni_arp_address);
static ni_define_ptr_array_delete_at(ni_arp_address);

static ni_bool_t
ni_arp_address_array_append_addr(ni_arp_address_array_t *arr, ni_address_t *addr)
Expand Down Expand Up @@ -320,6 +321,24 @@ ni_arp_verify_add_address(ni_arp_verify_t *vfy, ni_address_t *ap)
return vfy->ipaddrs.count;
}

ni_bool_t
ni_arp_verify_remove_address(ni_arp_verify_t *vfy, ni_address_t *ap)
{
unsigned int index = 0;

if (!vfy || !ap)
return FALSE;

if (ap->family != AF_INET || !ni_sockaddr_is_ipv4_specified(&ap->local_addr))
return FALSE;

if (!ni_arp_address_array_find_match_addr(&vfy->ipaddrs, ap, &index,
ni_address_equal_local_addr))
return FALSE;

return ni_arp_address_array_delete_at(&vfy->ipaddrs, index);
}

unsigned int
ni_arp_verify_add_in_addr(ni_arp_verify_t * vfy, struct in_addr addr)
{
Expand Down Expand Up @@ -440,7 +459,6 @@ ni_arp_verify_send(ni_arp_socket_t *sock, ni_arp_verify_t *vfy, ni_timeout_t *ti
}

need_wait = FALSE;
vfy->started = now;
for (i = 0; i < vfy->ipaddrs.count; ++i) {
vap = vfy->ipaddrs.data[i];
ap = vap->address;
Expand Down Expand Up @@ -485,6 +503,7 @@ ni_arp_verify_send(ni_arp_socket_t *sock, ni_arp_verify_t *vfy, ni_timeout_t *ti
}
}
if (need_wait) {
ni_timer_get_time(&vfy->started);
vfy->last_timeout = ni_timeout_random_range(vfy->probe_min_ms, vfy->probe_max_ms);
*timeout = vfy->last_timeout;
return NI_ARP_SEND_PROGRESS;
Expand Down Expand Up @@ -561,7 +580,6 @@ ni_arp_notify_send(ni_arp_socket_t *sock, ni_arp_notify_t *nfy, ni_timeout_t *ti
return TRUE;

if (nfy->nclaims && nfy->ipaddrs.count) {
nfy->started = now;
need_wait = FALSE;

for (i = 0; i < nfy->ipaddrs.count; ++i) {
Expand All @@ -585,7 +603,8 @@ ni_arp_notify_send(ni_arp_socket_t *sock, ni_arp_notify_t *nfy, ni_timeout_t *ti
ip = &ap->local_addr.sin.sin_addr;
if (ni_arp_send_grat_request(sock, *ip) > 0) {
aa->nattempts++;
need_wait = TRUE;
if (nfy->nclaims > aa->nattempts)
need_wait = TRUE;
} else {
if (errno == ENOBUFS) {
aa->nerrors++;
Expand All @@ -606,6 +625,7 @@ ni_arp_notify_send(ni_arp_socket_t *sock, ni_arp_notify_t *nfy, ni_timeout_t *ti
}
}
if (need_wait) {
ni_timer_get_time(&nfy->started);
*timeout = nfy->wait_ms;
return TRUE;
}
Expand Down
32 changes: 16 additions & 16 deletions src/ifconfig.c
Expand Up @@ -4861,9 +4861,9 @@ ni_netdev_addr_needs_update(ni_netdev_t *dev, ni_address_t *o, ni_address_t *n)
* Update the addresses and routes assigned to an interface
* for a given addrconf method
*/
#define NI_ADDRCONF_UPDATER_MAX_ADDR_CHANGES 256
#define NI_ADDRCONF_UPDATER_MAX_ARP_MESSAGES 256
#define NI_ADDRCONF_UPDATER_MAX_ADDR_CHANGES (NI_ADDRCONF_UPDATER_MAX_ARP_MESSAGES / 2)
#define NI_ADDRCONF_UPDATER_MAX_ADDR_TIMEOUT 100
#define NI_ADDRCONF_UPDATER_MAX_ARP_MESSAGES NI_ADDRCONF_UPDATER_MAX_ADDR_CHANGES

typedef struct ni_address_updater {
ni_arp_verify_t verify;
Expand Down Expand Up @@ -5066,25 +5066,18 @@ ni_address_updater_arp_send(ni_addrconf_updater_t *updater, ni_netdev_t *dev, ni
{
ni_timeout_t wait_verify = 0, wait_notify = 0;
ni_address_updater_t *au;
const ni_config_arp_t *arpcfg = ni_config_addrconf_arp(owner, dev->name);

if (!dev || !(au = ni_addrconf_address_updater_get(updater)))
return FALSE;

if (au->notify.nclaims) {
if (ni_arp_notify_send(au->sock, &au->notify, &wait_notify)) {
updater->timeout = wait_notify;
return TRUE;
}
ni_arp_notify_reset(&au->notify, &arpcfg->notify);
if (ni_arp_notify_send(au->sock, &au->notify, &wait_notify)) {
updater->timeout = wait_notify;
return TRUE;
}

if (au->verify.nprobes) {
if (ni_arp_verify_send(au->sock, &au->verify, &wait_verify) == NI_ARP_SEND_PROGRESS) {
updater->timeout = wait_verify;
return TRUE;
}
ni_arp_verify_reset(&au->verify, &arpcfg->verify);
if (ni_arp_verify_send(au->sock, &au->verify, &wait_verify) == NI_ARP_SEND_PROGRESS) {
updater->timeout = wait_verify;
return TRUE;
}

return FALSE;
Expand Down Expand Up @@ -5243,7 +5236,14 @@ __ni_netdev_update_addrs(ni_netdev_t *dev,
if (ni_address_is_duplicate(ap))
continue;

if (ni_address_is_tentative(ap)) {
if (!ni_address_is_tentative(ap)) {
/*
* Remove address from verify array, as it isn't
* tentative anymore and we need space for the
* next burst (NI_ADDRCONF_UPDATE_MAX_ADDR_CHANGES)
*/
ni_arp_verify_remove_address(&au->verify, ap);
} else {
count = ni_arp_verify_add_address(&au->verify, ap);
if (count >= NI_ADDRCONF_UPDATER_MAX_ADDR_CHANGES)
break;
Expand Down
1 change: 1 addition & 0 deletions src/netinfo_priv.h
Expand Up @@ -234,6 +234,7 @@ extern void ni_arp_verify_init(ni_arp_verify_t *, const ni_config_arp_verify_
extern void ni_arp_verify_reset(ni_arp_verify_t *, const ni_config_arp_verify_t *);
extern void ni_arp_verify_destroy(ni_arp_verify_t *);
extern unsigned int ni_arp_verify_add_address(ni_arp_verify_t *, ni_address_t *);
extern ni_bool_t ni_arp_verify_remove_address(ni_arp_verify_t *, ni_address_t *);
extern unsigned int ni_arp_verify_add_in_addr(ni_arp_verify_t *, struct in_addr);
extern ni_arp_send_status_t ni_arp_verify_send(ni_arp_socket_t *, ni_arp_verify_t *, ni_timeout_t *);
extern ni_arp_address_t * ni_arp_reply_match_address(ni_arp_socket_t *, ni_arp_address_array_t *,
Expand Down

0 comments on commit 286008b

Please sign in to comment.