From 83075866474ada8ec18f03b7ead7e24d2aea34eb Mon Sep 17 00:00:00 2001 From: Quentin Armitage Date: Mon, 21 Oct 2024 21:32:22 +0100 Subject: [PATCH 01/16] doc: reorder some entries in keepalived.conf(5) man page This makes the order of some entries more logical. Signed-off-by: Quentin Armitage --- doc/man/man5/keepalived.conf.5.in | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/doc/man/man5/keepalived.conf.5.in b/doc/man/man5/keepalived.conf.5.in index 2bb729bdf..f5c5825be 100644 --- a/doc/man/man5/keepalived.conf.5.in +++ b/doc/man/man5/keepalived.conf.5.in @@ -367,29 +367,26 @@ possibly following any cleanup actions needed. # remove them). \fBlvs_flush_on_stop [VS]\fR + # number of gratuitous ARP messages to send at a time after + # transition to MASTER. + # (default: 5) + \fBvrrp_garp_master_repeat \fR1 + # delay for second set of gratuitous ARPs after transition to MASTER. # in seconds, 0 for no second set. # (default: 5) \fBvrrp_garp_master_delay \fR10 # number of gratuitous ARP messages to send at a time after - # transition to MASTER. - # (default: 5) - \fBvrrp_garp_master_repeat \fR1 + # lower priority advert received when MASTER. + # (default: vrrp_garp_master_repeat) + \fBvrrp_garp_lower_prio_repeat \fR1 # delay for second set of gratuitous ARPs after lower priority # advert received when MASTER. # (default: vrrp_garp_master_delay) \fBvrrp_garp_lower_prio_delay \fR10 - # Default value for vrrp down_timer_adverts. - \fBvrrp_down_timer_adverts \fR[1:100] - - # number of gratuitous ARP messages to send at a time after - # lower priority advert received when MASTER. - # (default: vrrp_garp_master_repeat) - \fBvrrp_garp_lower_prio_repeat \fR1 - # minimum time interval for refreshing gratuitous ARPs while MASTER. # in seconds (resolution seconds). # (default: 0 (no refreshing)) @@ -428,9 +425,12 @@ possibly following any cleanup actions needed. # will cause it to send GARP/NA on each interface used by the VRRP instance. \fBvrrp_garp_extra_if [all] \fR100 + # Default value for vrrp down_timer_adverts. + \fBvrrp_down_timer_adverts \fR[1:100] + # If a lower priority advert is received, don't send another advert. - # This causes adherence to the RFCs. Defaults to false, unless - # strict_mode is set. + # This causes adherence to the RFCs prior to RFC9568. + # Defaults to false, unless strict_mode is set. \fBvrrp_lower_prio_no_advert \fR[] # If we are master and receive a higher priority advert, send an advert @@ -1572,7 +1572,7 @@ and the limits apply to the switch as a whole. If the global vrrp_garp_interval and/or vrrp_gna_interval are set, any interfaces that aren't specified in a garp_group will inherit the global -settings. +settings on a per interface basis. .PP .nf The syntax for garp_group is : From 4585a0d293f955287eefeaea05bbf02834584bea Mon Sep 17 00:00:00 2001 From: Quentin Armitage Date: Mon, 21 Oct 2024 21:33:46 +0100 Subject: [PATCH 02/16] vrrp: Use TIMER_HZ instead of 1000000 for garp/gna interval Signed-off-by: Quentin Armitage --- keepalived/vrrp/vrrp_if.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/keepalived/vrrp/vrrp_if.c b/keepalived/vrrp/vrrp_if.c index 7834355dd..3a404f9c9 100644 --- a/keepalived/vrrp/vrrp_if.c +++ b/keepalived/vrrp/vrrp_if.c @@ -482,13 +482,13 @@ set_default_garp_delay(void) ip_address_t *vip; if (global_data->vrrp_garp_interval) { - default_delay.garp_interval.tv_sec = global_data->vrrp_garp_interval / 1000000; - default_delay.garp_interval.tv_usec = global_data->vrrp_garp_interval % 1000000; + default_delay.garp_interval.tv_sec = global_data->vrrp_garp_interval / TIMER_HZ; + default_delay.garp_interval.tv_usec = global_data->vrrp_garp_interval % TIMER_HZ; default_delay.have_garp_interval = true; } if (global_data->vrrp_gna_interval) { - default_delay.gna_interval.tv_sec = global_data->vrrp_gna_interval / 1000000; - default_delay.gna_interval.tv_usec = global_data->vrrp_gna_interval % 1000000; + default_delay.gna_interval.tv_sec = global_data->vrrp_gna_interval / TIMER_HZ; + default_delay.gna_interval.tv_usec = global_data->vrrp_gna_interval % TIMER_HZ; default_delay.have_gna_interval = true; } From 9646fcca5e42be55011ffdb4fbf377989c14b2a1 Mon Sep 17 00:00:00 2001 From: Quentin Armitage Date: Mon, 21 Oct 2024 21:35:00 +0100 Subject: [PATCH 03/16] vrrp: On reload with addresses added to VRRP instance send 2nd GARPs If garp_master_delay is non zero, then after a reload when VIPs are added to a VRRP instance in master state, as well as the initial block of GARP messages that are sent, the messages need to be repeated after garp_master_delay seconds. This commit adds sending the second block. Signed-off-by: Quentin Armitage --- keepalived/vrrp/vrrp.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/keepalived/vrrp/vrrp.c b/keepalived/vrrp/vrrp.c index 152659882..e478716f6 100644 --- a/keepalived/vrrp/vrrp.c +++ b/keepalived/vrrp/vrrp.c @@ -5092,6 +5092,11 @@ clear_diff_vrrp(void) * all the addresses, but at least we will do so for the new addresses. */ vrrp_send_link_update(new_vrrp, new_vrrp->garp_rep); + /* Add thread for second block of GARPs */ + if (vrrp->garp_delay) + thread_add_timer(master, vrrp_gratuitous_arp_thread, + vrrp, vrrp->garp_delay); + /* set refresh timer */ if (timerisset(&new_vrrp->garp_refresh)) new_vrrp->garp_refresh_timer = timer_add_now(new_vrrp->garp_refresh); From d3f036f3108d944155938ef49b39ab3c26609804 Mon Sep 17 00:00:00 2001 From: Quentin Armitage Date: Mon, 21 Oct 2024 21:38:10 +0100 Subject: [PATCH 04/16] vrrp: improve some code indentation so then and else blocks match Signed-off-by: Quentin Armitage --- keepalived/vrrp/vrrp_scheduler.c | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/keepalived/vrrp/vrrp_scheduler.c b/keepalived/vrrp/vrrp_scheduler.c index e10f0c81e..259a71fba 100644 --- a/keepalived/vrrp/vrrp_scheduler.c +++ b/keepalived/vrrp/vrrp_scheduler.c @@ -1415,17 +1415,16 @@ vrrp_arpna_send(vrrp_t *vrrp, list_head_t *l, timeval_t *n) if (timercmp(&ifp->garp_delay->garp_next_time, n, <)) *n = ifp->garp_delay->garp_next_time; } - continue; - } - - /* IPv6 handling */ - if (timercmp(&time_now, &ifp->garp_delay->gna_next_time, >=)) { - ndisc_send_unsolicited_na_immediate(ifp, ip_addr); - ip_addr->garp_gna_pending = false; } else { - vrrp->gna_pending = true; - if (timercmp(&ifp->garp_delay->gna_next_time, n, <)) - *n = ifp->garp_delay->gna_next_time; + /* IPv6 handling */ + if (timercmp(&time_now, &ifp->garp_delay->gna_next_time, >=)) { + ndisc_send_unsolicited_na_immediate(ifp, ip_addr); + ip_addr->garp_gna_pending = false; + } else { + vrrp->gna_pending = true; + if (timercmp(&ifp->garp_delay->gna_next_time, n, <)) + *n = ifp->garp_delay->gna_next_time; + } } } From d7aeabbedd722b83ddda2a19cb6b44eca6b1b091 Mon Sep 17 00:00:00 2001 From: Quentin Armitage Date: Mon, 21 Oct 2024 21:38:54 +0100 Subject: [PATCH 05/16] vrrp: merge vrrp instance garp_pending and gna_pending flags Combine garp_pending and gna_pending flags into a single flags; that is all that is necessary and simplifies the code. Signed-off-by: Quentin Armitage --- keepalived/include/vrrp.h | 3 +-- keepalived/vrrp/vrrp.c | 3 +-- keepalived/vrrp/vrrp_arp.c | 15 ++++++--------- keepalived/vrrp/vrrp_ndisc.c | 17 ++++++++--------- keepalived/vrrp/vrrp_scheduler.c | 9 ++++----- 5 files changed, 20 insertions(+), 27 deletions(-) diff --git a/keepalived/include/vrrp.h b/keepalived/include/vrrp.h index 2adaf6560..aa9d2bce8 100644 --- a/keepalived/include/vrrp.h +++ b/keepalived/include/vrrp.h @@ -303,9 +303,8 @@ typedef struct _vrrp_t { unsigned garp_rep; /* gratuitous ARP repeat value */ unsigned garp_refresh_rep; /* refresh gratuitous ARP repeat value */ unsigned garp_lower_prio_delay; /* Delay to second set or ARP messages */ - bool garp_pending; /* Are there gratuitous ARP messages still to be sent */ - bool gna_pending; /* Are there gratuitous NA messages still to be sent */ unsigned garp_lower_prio_rep; /* Number of ARP messages to send at a time */ + bool garp_gna_pending; /* Are there gratuitous ARP or NA messages still to be sent */ unsigned down_timer_adverts; /* Number of adverts missed before backup takes over as master */ unsigned lower_prio_no_advert; /* Don't send advert after lower prio advert received */ unsigned higher_prio_send_advert; /* Send advert after higher prio advert received */ diff --git a/keepalived/vrrp/vrrp.c b/keepalived/vrrp/vrrp.c index e478716f6..973b0f34f 100644 --- a/keepalived/vrrp/vrrp.c +++ b/keepalived/vrrp/vrrp.c @@ -1689,8 +1689,7 @@ vrrp_remove_delayed_arp(vrrp_t *vrrp) list_for_each_entry(ip_addr, &vrrp->evip, e_list) { ip_addr->garp_gna_pending = false; } - vrrp->garp_pending = false; - vrrp->gna_pending = false; + vrrp->garp_gna_pending = false; } /* becoming master */ diff --git a/keepalived/vrrp/vrrp_arp.c b/keepalived/vrrp/vrrp_arp.c index 9e1d7d628..7a748db3a 100644 --- a/keepalived/vrrp/vrrp_arp.c +++ b/keepalived/vrrp/vrrp_arp.c @@ -162,7 +162,7 @@ static void queue_garp(vrrp_t *vrrp, interface_t *ifp, ip_address_t *ipaddress) { timeval_t next_time = timer_add_now(ifp->garp_delay->garp_interval); - vrrp->garp_pending = true; + vrrp->garp_gna_pending = true; ipaddress->garp_gna_pending = true; /* Do we need to reschedule the garp thread? */ @@ -189,14 +189,11 @@ void send_gratuitous_arp(vrrp_t *vrrp, ip_address_t *ipaddress) /* Do we need to delay sending the garp? */ if (ifp->garp_delay && ifp->garp_delay->have_garp_interval && - ifp->garp_delay->garp_next_time.tv_sec) { - if (timercmp(&time_now, &ifp->garp_delay->garp_next_time, <)) { - queue_garp(vrrp, ifp, ipaddress); - return; - } - } - - send_gratuitous_arp_immediate(ifp, ipaddress); + ifp->garp_delay->garp_next_time.tv_sec && + timercmp(&time_now, &ifp->garp_delay->garp_next_time, <)) + queue_garp(vrrp, ifp, ipaddress); + else + send_gratuitous_arp_immediate(ifp, ipaddress); } /* diff --git a/keepalived/vrrp/vrrp_ndisc.c b/keepalived/vrrp/vrrp_ndisc.c index e418fd4d4..cc997151c 100644 --- a/keepalived/vrrp/vrrp_ndisc.c +++ b/keepalived/vrrp/vrrp_ndisc.c @@ -267,7 +267,7 @@ queue_ndisc(vrrp_t *vrrp, interface_t *ifp, ip_address_t *ipaddress) { timeval_t next_time = timer_add_now(ifp->garp_delay->gna_interval); - vrrp->gna_pending = true; + vrrp->garp_gna_pending = true; ipaddress->garp_gna_pending = true; /* Do we need to schedule/reschedule the garp thread? */ @@ -293,14 +293,13 @@ ndisc_send_unsolicited_na(vrrp_t *vrrp, ip_address_t *ipaddress) set_time_now(); /* Do we need to delay sending the ndisc? */ - if (ifp->garp_delay && ifp->garp_delay->have_gna_interval && ifp->garp_delay->gna_next_time.tv_sec) { - if (timercmp(&time_now, &ifp->garp_delay->gna_next_time, <)) { - queue_ndisc(vrrp, ifp, ipaddress); - return; - } - } - - ndisc_send_unsolicited_na_immediate(ifp, ipaddress); + if (ifp->garp_delay && + ifp->garp_delay->have_gna_interval && + ifp->garp_delay->gna_next_time.tv_sec && + timercmp(&time_now, &ifp->garp_delay->gna_next_time, <)) + queue_ndisc(vrrp, ifp, ipaddress); + else + ndisc_send_unsolicited_na_immediate(ifp, ipaddress); } /* diff --git a/keepalived/vrrp/vrrp_scheduler.c b/keepalived/vrrp/vrrp_scheduler.c index 259a71fba..de2cfdceb 100644 --- a/keepalived/vrrp/vrrp_scheduler.c +++ b/keepalived/vrrp/vrrp_scheduler.c @@ -1411,7 +1411,7 @@ vrrp_arpna_send(vrrp_t *vrrp, list_head_t *l, timeval_t *n) send_gratuitous_arp_immediate(ifp, ip_addr); ip_addr->garp_gna_pending = false; } else { - vrrp->garp_pending = true; + vrrp->garp_gna_pending = true; if (timercmp(&ifp->garp_delay->garp_next_time, n, <)) *n = ifp->garp_delay->garp_next_time; } @@ -1421,7 +1421,7 @@ vrrp_arpna_send(vrrp_t *vrrp, list_head_t *l, timeval_t *n) ndisc_send_unsolicited_na_immediate(ifp, ip_addr); ip_addr->garp_gna_pending = false; } else { - vrrp->gna_pending = true; + vrrp->garp_gna_pending = true; if (timercmp(&ifp->garp_delay->gna_next_time, n, <)) *n = ifp->garp_delay->gna_next_time; } @@ -1442,11 +1442,10 @@ vrrp_arp_thread(thread_ref_t thread) set_time_now(); list_for_each_entry(vrrp, &vrrp_data->vrrp, e_list) { - if (!vrrp->garp_pending && !vrrp->gna_pending) + if (!vrrp->garp_gna_pending) continue; - vrrp->garp_pending = false; - vrrp->gna_pending = false; + vrrp->garp_gna_pending = false; if (vrrp->state != VRRP_STATE_MAST || !vrrp->vipset) continue; From 1d695bd2d0fd207194fe0c10b28b91fa3abe72f8 Mon Sep 17 00:00:00 2001 From: Quentin Armitage Date: Mon, 21 Oct 2024 21:40:28 +0100 Subject: [PATCH 06/16] vrrp: Correct formatting of GARP interval in config/status dump Signed-off-by: Quentin Armitage --- keepalived/vrrp/vrrp_if.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/keepalived/vrrp/vrrp_if.c b/keepalived/vrrp/vrrp_if.c index 3a404f9c9..5f00e8d11 100644 --- a/keepalived/vrrp/vrrp_if.c +++ b/keepalived/vrrp/vrrp_if.c @@ -419,7 +419,7 @@ dump_garp_delay(FILE *fp, const garp_delay_t *gd) conf_write(fp, "------< GARP delay group %d >------", gd->aggregation_group); if (gd->have_garp_interval) { - conf_write(fp, " GARP interval = %" PRI_tv_sec "%6.6" PRI_tv_usec, gd->garp_interval.tv_sec, gd->garp_interval.tv_usec); + conf_write(fp, " GARP interval = %" PRI_tv_sec ".%6.6" PRI_tv_usec, gd->garp_interval.tv_sec, gd->garp_interval.tv_usec); if (!ctime_r(&gd->garp_next_time.tv_sec, time_str)) strcpy(time_str, "invalid time "); conf_write(fp, " GARP next time %" PRI_tv_sec ".%6.6" PRI_tv_usec " (%.19s.%6.6" PRI_tv_usec ")", gd->garp_next_time.tv_sec, gd->garp_next_time.tv_usec, time_str, gd->garp_next_time.tv_usec); From b1d9e3758db34a1152b339b27c583294904ffe19 Mon Sep 17 00:00:00 2001 From: Quentin Armitage Date: Mon, 21 Oct 2024 21:43:37 +0100 Subject: [PATCH 07/16] vrrp: Use timer threads for delayed sending of GARPs/GNAs Previously whenever a VRRP instance send an advert, it checked to see if any more GARPs/GNAs were due to be sent, either for garp_master_delay or garp_master_refresh. Using timer threads removes the checking every time an advert is sent, and the relevant code is only triggered when a timer expires. Further, if any GARPs/GNAs wre delayed to to rate limiting of sending these messages on an interface or group, there was a single timer thread for the next time a delayed GARP/GNA could be sent, and when the timer expired, all VRRP instances had to be checked, and any instances that had been delayed sending had to check all VIPs/eVIPs to see if a GARP/GNA was needed for it. This also meant that once a GARP/GNA had been delayed for a VIP, only one more GARP/GNA would be sent, rather than the number configured by garp_master_repeat/garp_lower_prio_repeat/garp_master_refresh_repeat. This commit now uses a thread per interface/garp_group and address family for handling delayed sending, and will always send the configured number of repeat messages configure by the garp_..._repeat parameters. This approach of using threads per activity will enable keepalived to scale much better with very large configurations that send multiple GARP/GNA messages. Signed-off-by: Quentin Armitage --- keepalived/include/vrrp.h | 4 +- keepalived/include/vrrp_arp.h | 2 +- keepalived/include/vrrp_if.h | 6 +- keepalived/include/vrrp_ipaddress.h | 3 +- keepalived/include/vrrp_ndisc.h | 2 +- keepalived/include/vrrp_scheduler.h | 7 +- keepalived/vrrp/vrrp.c | 156 +++++++++++++++------------- keepalived/vrrp/vrrp_arp.c | 29 +++--- keepalived/vrrp/vrrp_data.c | 2 - keepalived/vrrp/vrrp_if.c | 2 + keepalived/vrrp/vrrp_ipaddress.c | 2 + keepalived/vrrp/vrrp_ndisc.c | 28 +++-- keepalived/vrrp/vrrp_scheduler.c | 122 +++++++++++----------- 13 files changed, 189 insertions(+), 176 deletions(-) diff --git a/keepalived/include/vrrp.h b/keepalived/include/vrrp.h index aa9d2bce8..c391873b6 100644 --- a/keepalived/include/vrrp.h +++ b/keepalived/include/vrrp.h @@ -299,18 +299,15 @@ typedef struct _vrrp_t { timeval_t last_transition; /* Store transition time */ unsigned garp_delay; /* Delay to launch gratuitous ARP */ timeval_t garp_refresh; /* Next scheduled gratuitous ARP refresh */ - timeval_t garp_refresh_timer; /* Next scheduled gratuitous ARP timer */ unsigned garp_rep; /* gratuitous ARP repeat value */ unsigned garp_refresh_rep; /* refresh gratuitous ARP repeat value */ unsigned garp_lower_prio_delay; /* Delay to second set or ARP messages */ unsigned garp_lower_prio_rep; /* Number of ARP messages to send at a time */ - bool garp_gna_pending; /* Are there gratuitous ARP or NA messages still to be sent */ unsigned down_timer_adverts; /* Number of adverts missed before backup takes over as master */ unsigned lower_prio_no_advert; /* Don't send advert after lower prio advert received */ unsigned higher_prio_send_advert; /* Send advert after higher prio advert received */ #ifdef _HAVE_VRRP_VMAC_ timeval_t vmac_garp_intvl; /* Interval between GARPs on each VMAC */ - timeval_t vmac_garp_timer; /* Next scheduled GARP for each VMAC */ #endif uint8_t vrid; /* virtual id. from 1(!) to 255 */ uint8_t base_priority; /* configured priority value */ @@ -476,6 +473,7 @@ extern void open_sockpool_socket(sock_t *); extern int new_vrrp_socket(vrrp_t *); extern void vrrp_send_adv(vrrp_t *, uint8_t); extern void vrrp_send_link_update(vrrp_t *, unsigned); +extern void vrrp_send_vmac_update(vrrp_t *); extern void add_vrrp_to_interface(vrrp_t *, interface_t *, int, bool, bool, track_t); extern void del_vrrp_from_interface(vrrp_t *, interface_t *); extern bool vrrp_state_master_rx(vrrp_t *, const vrrphdr_t *, const char *, ssize_t); diff --git a/keepalived/include/vrrp_arp.h b/keepalived/include/vrrp_arp.h index d7fdcbf80..0a9ab2b46 100644 --- a/keepalived/include/vrrp_arp.h +++ b/keepalived/include/vrrp_arp.h @@ -69,6 +69,6 @@ typedef struct ipoib_hdr { /* prototypes */ extern bool gratuitous_arp_init(void); extern void gratuitous_arp_close(void); -extern void send_gratuitous_arp(vrrp_t *, ip_address_t *); +extern void send_gratuitous_arp(ip_address_t *, unsigned); extern ssize_t send_gratuitous_arp_immediate(interface_t *, ip_address_t *); #endif diff --git a/keepalived/include/vrrp_if.h b/keepalived/include/vrrp_if.h index 0ee5bb757..fb0bd3862 100644 --- a/keepalived/include/vrrp_if.h +++ b/keepalived/include/vrrp_if.h @@ -80,7 +80,11 @@ typedef struct _garp_delay { timeval_t gna_next_time; /* Time when next gratuitous NA message can be sent */ int aggregation_group; /* Index of multi-interface group */ - /* linked list member */ + /* linked list of ip_address_t that have GARP/NAs pending */ + list_head_t garp_list; + list_head_t gna_list; + + /* linked list member of garp_delay_t */ list_head_t e_list; } garp_delay_t; diff --git a/keepalived/include/vrrp_ipaddress.h b/keepalived/include/vrrp_ipaddress.h index 361df7280..a0b65ea14 100644 --- a/keepalived/include/vrrp_ipaddress.h +++ b/keepalived/include/vrrp_ipaddress.h @@ -77,7 +77,8 @@ typedef struct _ip_address { #ifdef _WITH_NFTABLES_ bool nftable_rule_set; /* TRUE if in nftables set */ #endif - bool garp_gna_pending; /* Is a gratuitous ARP/NA message still to be sent */ + unsigned garp_gna_pending; /* Number of GARPs/GNAs still to be sent */ + list_head_t garp_gna_list; uint32_t preferred_lft; /* IPv6 preferred_lft (0 means address deprecated) */ /* linked list member */ diff --git a/keepalived/include/vrrp_ndisc.h b/keepalived/include/vrrp_ndisc.h index 41ebc6d83..c23817143 100644 --- a/keepalived/include/vrrp_ndisc.h +++ b/keepalived/include/vrrp_ndisc.h @@ -63,7 +63,7 @@ struct ip6hdr { /* prototypes */ extern bool ndisc_init(void); extern void ndisc_close(void); -extern void ndisc_send_unsolicited_na(vrrp_t *, ip_address_t *); +extern void ndisc_send_unsolicited_na(ip_address_t *, unsigned); extern void ndisc_send_unsolicited_na_immediate(interface_t *, ip_address_t *); #endif diff --git a/keepalived/include/vrrp_scheduler.h b/keepalived/include/vrrp_scheduler.h index babb2ca4f..7a2b6576e 100644 --- a/keepalived/include/vrrp_scheduler.h +++ b/keepalived/include/vrrp_scheduler.h @@ -36,8 +36,6 @@ #include "vrrp.h" /* global vars */ -extern timeval_t garp_next_time; -extern thread_ref_t garp_thread; extern bool vrrp_initialised; extern timeval_t vrrp_delayed_start_time; @@ -68,7 +66,12 @@ extern void cancel_vrrp_threads(void); extern void vrrp_dispatcher_release(vrrp_data_t *); extern void vrrp_gratuitous_arp_thread(thread_ref_t); extern void vrrp_lower_prio_gratuitous_arp_thread(thread_ref_t); +extern void vrrp_gratuitous_arp_refresh_thread(thread_ref_t); +#ifdef _HAVE_VRRP_VMAC_ +extern void vrrp_gratuitous_arp_vmac_update_thread(thread_ref_t); +#endif extern void vrrp_arp_thread(thread_ref_t); +extern void vrrp_gna_thread(thread_ref_t); extern void try_up_instance(vrrp_t *, bool); #ifdef _WITH_DUMP_THREADS_ extern void dump_threads(void); diff --git a/keepalived/vrrp/vrrp.c b/keepalived/vrrp/vrrp.c index 973b0f34f..96b2c4c8f 100644 --- a/keepalived/vrrp/vrrp.c +++ b/keepalived/vrrp/vrrp.c @@ -1577,7 +1577,7 @@ vrrp_send_adv(vrrp_t * vrrp, uint8_t prio) /* Gratuitous ARP on each VIP */ static void -vrrp_send_update(vrrp_t * vrrp, ip_address_t * ipaddress, bool log_msg) +vrrp_send_update(vrrp_t * vrrp, ip_address_t * ipaddress, bool log_msg, unsigned rep) { const char *msg; char addr_str[INET6_ADDRSTRLEN]; @@ -1596,9 +1596,9 @@ vrrp_send_update(vrrp_t * vrrp, ip_address_t * ipaddress, bool log_msg) } if (!IP_IS6(ipaddress)) - send_gratuitous_arp(vrrp, ipaddress); + send_gratuitous_arp(ipaddress, rep); else - ndisc_send_unsolicited_na(vrrp, ipaddress); + ndisc_send_unsolicited_na(ipaddress, rep); } void @@ -1611,18 +1611,21 @@ vrrp_send_link_update(vrrp_t * vrrp, unsigned rep) if (!VRRP_VIP_ISSET(vrrp)) return; - /* send gratuitous arp for each virtual ip */ + /* send gratuitous arp for each virtual ip. + * Looping rep times through all VIPs of the vrrp instance doesn't + * seem very efficient, but I haven't thought of a better way when + * the GARP/NA may either be sent or queued. */ for (j = 0; j < rep; j++) { list_for_each_entry(ip_addr, &vrrp->vip, e_list) - vrrp_send_update(vrrp, ip_addr, !j); + vrrp_send_update(vrrp, ip_addr, !j, rep); list_for_each_entry(ip_addr, &vrrp->evip, e_list) - vrrp_send_update(vrrp, ip_addr, !j); + vrrp_send_update(vrrp, ip_addr, !j, rep); } } #ifdef _HAVE_VRRP_VMAC_ -static void +void vrrp_send_vmac_update(vrrp_t *vrrp) { struct ifs { @@ -1661,9 +1664,9 @@ vrrp_send_vmac_update(vrrp_t *vrrp) if (already_done) continue; - vrrp_send_update(vrrp, ip_addr, true); + vrrp_send_update(vrrp, ip_addr, true, 1); - /* Save if ifindex to avoid sending on that interface again */ + /* Save interface ifindex to avoid sending on that interface again */ PMALLOC(if_entry); INIT_LIST_HEAD(&if_entry->e_list); if_entry->ifindex = ip_addr->ifp->ifindex; @@ -1671,7 +1674,7 @@ vrrp_send_vmac_update(vrrp_t *vrrp) } } - /* Free the list of ifindices we have sent on */ + /* Free the list of interface indices we have sent on */ list_for_each_entry_safe(if_entry, next_if_entry, &if_list, e_list) FREE(if_entry); } @@ -1683,13 +1686,14 @@ vrrp_remove_delayed_arp(vrrp_t *vrrp) ip_address_t *ip_addr; list_for_each_entry(ip_addr, &vrrp->vip, e_list) { - ip_addr->garp_gna_pending = false; + ip_addr->garp_gna_pending = 0; + list_del_init(&ip_addr->garp_gna_list); } list_for_each_entry(ip_addr, &vrrp->evip, e_list) { - ip_addr->garp_gna_pending = false; + ip_addr->garp_gna_pending = 0; + list_del_init(&ip_addr->garp_gna_list); } - vrrp->garp_gna_pending = false; } /* becoming master */ @@ -1728,13 +1732,18 @@ vrrp_state_become_master(vrrp_t * vrrp) vrrp_send_link_update(vrrp, vrrp->garp_rep); - /* set GARP/NA refresh timer */ + if (vrrp->garp_delay) + thread_add_timer(master, vrrp_gratuitous_arp_thread, + vrrp, vrrp->garp_delay); + if (timerisset(&vrrp->garp_refresh)) - vrrp->garp_refresh_timer = timer_add_now(vrrp->garp_refresh); + thread_add_timer(master, vrrp_gratuitous_arp_refresh_thread, + vrrp, vrrp->garp_delay + timer_long(vrrp->garp_refresh)); #ifdef _HAVE_VRRP_VMAC_ if (timerisset(&vrrp->vmac_garp_intvl)) - vrrp->vmac_garp_timer = timer_add_now(vrrp->vmac_garp_intvl); + thread_add_timer(master, vrrp_gratuitous_arp_vmac_update_thread, + vrrp, vrrp->garp_delay + timer_long(vrrp->vmac_garp_intvl)); #endif /* Check if notify is needed */ @@ -2056,27 +2065,6 @@ vrrp_state_master_tx(vrrp_t * vrrp) log_message(LOG_INFO, "(%s) Entering MASTER STATE" , vrrp->iname); vrrp_state_become_master(vrrp); - /* - * If we catch the master transition - * register a gratuitous arp thread delayed to garp_delay secs. - */ - if (vrrp->garp_delay) - thread_add_timer(master, vrrp_gratuitous_arp_thread, - vrrp, vrrp->garp_delay); - } else { - if (timerisset(&vrrp->garp_refresh) && - timercmp(&time_now, &vrrp->garp_refresh_timer, >)) { - vrrp_send_link_update(vrrp, vrrp->garp_refresh_rep); - vrrp->garp_refresh_timer = timer_add_now(vrrp->garp_refresh); - } - -#ifdef _HAVE_VRRP_VMAC_ - if (timerisset(&vrrp->vmac_garp_intvl) && - timercmp(&time_now, &vrrp->vmac_garp_timer, >)) { - vrrp_send_vmac_update(vrrp); - vrrp->vmac_garp_timer = timer_add_now(vrrp->vmac_garp_intvl); - } -#endif } } @@ -2201,9 +2189,15 @@ vrrp_state_master_rx(vrrp_t * vrrp, const vrrphdr_t *hd, const char *buf, ssize_ vrrp, vrrp->garp_lower_prio_delay); } - /* If we are a member of a sync group, send GARP messages - * for any other member of the group that has - * garp_lower_prio_rep set */ + /* If we are a member of a sync group, send GARP messages for any other member + * of the group that has garp_lower_prio_rep set. + * The reason for this is we must have been in some sort of split brain situation, + * or this keepalived process was not scheduled to run for a while, and a lower + * priority instance has become master, causing it to send adverts and GARP + * messages. When we send this advert, all the sync group members on the lower + * priority instance will transition to backup state, and we will not see + * adverts from those members of the sync group. However, the other VRRP + * instances need to refresh ARP caches. */ if (vrrp->sync) { list_for_each_entry(isync, &vrrp->sync->vrrp_instances, s_list) { if (isync == vrrp) @@ -5013,6 +5007,7 @@ clear_diff_vrrp(void) { vrrp_t *vrrp; vrrp_t *new_vrrp; + bool have_new_addr; list_for_each_entry(vrrp, &old_vrrp_data->vrrp, e_list) { /* @@ -5052,12 +5047,15 @@ clear_diff_vrrp(void) * reloaded. */ new_vrrp = vrrp_exist(vrrp, &vrrp_data->vrrp); - if (new_vrrp) { - /* - * If this vrrp instance exist in new - * data, then perform a VIP|EVIP diff. - */ + if (!new_vrrp) + continue; + + /* + * If this vrrp instance exist in new + * data, then perform a VIP|EVIP diff. + */ // !!!! Isn't this only necessary if MASTER ???? TODO + if (vrrp->state == VRRP_STATE_MAST) { /* virtual rules diff */ clear_diff_vrrp_vrules(vrrp, new_vrrp); @@ -5065,42 +5063,56 @@ clear_diff_vrrp(void) clear_diff_vrrp_vroutes(vrrp, new_vrrp); clear_diff_vrrp_vip(vrrp, new_vrrp); + } #ifdef _HAVE_VRRP_VMAC_ - /* - * Remove VMAC/IPVLAN if it existed in old vrrp instance, - * but not the new one. - */ - if (vrrp->ifp && - vrrp->ifp->is_ours && - ((__test_bit(VRRP_VMAC_BIT, &vrrp->flags) && - !__test_bit(VRRP_VMAC_BIT, &new_vrrp->flags)) + /* + * Remove VMAC/IPVLAN if it existed in old vrrp instance, + * but not the new one. + */ + if (vrrp->ifp && + vrrp->ifp->is_ours && + ((__test_bit(VRRP_VMAC_BIT, &vrrp->flags) && + !__test_bit(VRRP_VMAC_BIT, &new_vrrp->flags)) #ifdef _HAVE_VRRP_IPVLAN_ - || (__test_bit(VRRP_IPVLAN_BIT, &vrrp->flags) && - !__test_bit(VRRP_IPVLAN_BIT, &new_vrrp->flags)) + || (__test_bit(VRRP_IPVLAN_BIT, &vrrp->flags) && + !__test_bit(VRRP_IPVLAN_BIT, &new_vrrp->flags)) #endif - )) { - netlink_link_del_vmac(vrrp); - } + )) { + netlink_link_del_vmac(vrrp); + } +// What about VMACs for addresses? #endif - /* reset the state */ - if (restore_vrrp_state(vrrp, new_vrrp)) { - /* There were addresses added, so set GARP/GNA for them. - * This is a bit over the top since it will send GARPs/GNAs for - * all the addresses, but at least we will do so for the new addresses. */ - vrrp_send_link_update(new_vrrp, new_vrrp->garp_rep); + /* reset the state */ + have_new_addr = restore_vrrp_state(vrrp, new_vrrp); + + if (new_vrrp->state != VRRP_STATE_MAST) + continue; - /* Add thread for second block of GARPs */ - if (vrrp->garp_delay) - thread_add_timer(master, vrrp_gratuitous_arp_thread, - vrrp, vrrp->garp_delay); + if (have_new_addr || timerisset(&new_vrrp->garp_refresh)) { + /* There were addresses added, or we do periodic GARP/GNA + * refreshes, so send GARP/GNAs for them. + * This is a bit over the top if only for added addresses, + * since it will send GARPs/GNAs for * all the addresses, + * but at least we will do so for the new addresses. */ + vrrp_send_link_update(new_vrrp, new_vrrp->garp_rep); - /* set refresh timer */ - if (timerisset(&new_vrrp->garp_refresh)) - new_vrrp->garp_refresh_timer = timer_add_now(new_vrrp->garp_refresh); - } + /* Add thread for second block of GARPs */ + if (have_new_addr && new_vrrp->garp_delay) + thread_add_timer(master, vrrp_gratuitous_arp_thread, + new_vrrp, new_vrrp->garp_delay); } + + if (timerisset(&new_vrrp->garp_refresh)) + thread_add_timer(master, vrrp_gratuitous_arp_refresh_thread, + new_vrrp, (have_new_addr ? new_vrrp->garp_delay : 0) + timer_long(new_vrrp->garp_refresh)); + +#ifdef _HAVE_VRRP_VMAC_ + if (timerisset(&new_vrrp->vmac_garp_intvl)) + thread_add_timer(master, vrrp_gratuitous_arp_vmac_update_thread, + new_vrrp, (have_new_addr ? new_vrrp->garp_delay : 0) + timer_long(new_vrrp->vmac_garp_intvl)); +#endif } #ifdef _HAVE_VRRP_VMAC_ diff --git a/keepalived/vrrp/vrrp_arp.c b/keepalived/vrrp/vrrp_arp.c index 7a748db3a..2dc31dc9e 100644 --- a/keepalived/vrrp/vrrp_arp.c +++ b/keepalived/vrrp/vrrp_arp.c @@ -158,25 +158,20 @@ ssize_t send_gratuitous_arp_immediate(interface_t *ifp, ip_address_t *ipaddress) return len; } -static void queue_garp(vrrp_t *vrrp, interface_t *ifp, ip_address_t *ipaddress) +static void +queue_garp(interface_t *ifp, ip_address_t *ipaddress) { - timeval_t next_time = timer_add_now(ifp->garp_delay->garp_interval); - vrrp->garp_gna_pending = true; - ipaddress->garp_gna_pending = true; + ipaddress->garp_gna_pending = 1; - /* Do we need to reschedule the garp thread? */ - if (!garp_thread || timercmp(&next_time, &garp_next_time, <)) { - if (garp_thread) - thread_cancel(garp_thread); + if (list_empty(&ifp->garp_delay->garp_list)) + thread_add_timer(master, vrrp_arp_thread, ifp, timer_long(timer_sub_now(ifp->garp_delay->garp_next_time))); - garp_next_time = next_time; - - garp_thread = thread_add_timer(master, vrrp_arp_thread, NULL, timer_long(timer_sub_now(garp_next_time))); - } + list_add_tail(&ipaddress->garp_gna_list, &ifp->garp_delay->garp_list); } -void send_gratuitous_arp(vrrp_t *vrrp, ip_address_t *ipaddress) +void +send_gratuitous_arp(ip_address_t *ipaddress, unsigned rep) { interface_t *ifp = IF_BASE_IFP(ipaddress->ifp); @@ -184,6 +179,12 @@ void send_gratuitous_arp(vrrp_t *vrrp, ip_address_t *ipaddress) if (ifp->ifi_flags & IFF_NOARP) return; + if (ipaddress->garp_gna_pending) { + if (ipaddress->garp_gna_pending < rep) + ipaddress->garp_gna_pending++; + return; + } + set_time_now(); /* Do we need to delay sending the garp? */ @@ -191,7 +192,7 @@ void send_gratuitous_arp(vrrp_t *vrrp, ip_address_t *ipaddress) ifp->garp_delay->have_garp_interval && ifp->garp_delay->garp_next_time.tv_sec && timercmp(&time_now, &ifp->garp_delay->garp_next_time, <)) - queue_garp(vrrp, ifp, ipaddress); + queue_garp(ifp, ipaddress); else send_gratuitous_arp_immediate(ifp, ipaddress); } diff --git a/keepalived/vrrp/vrrp_data.c b/keepalived/vrrp/vrrp_data.c index 9c7a0b9c8..16594c9a1 100644 --- a/keepalived/vrrp/vrrp_data.c +++ b/keepalived/vrrp/vrrp_data.c @@ -734,8 +734,6 @@ dump_vrrp(FILE *fp, const vrrp_t *vrrp) #ifdef _HAVE_VRRP_VMAC_ if (vrrp->vmac_garp_intvl.tv_sec) { conf_write(fp, " Gratuitous ARP for each secondary %s = %" PRI_time_t, __test_bit(VRRP_FLAG_VMAC_GARP_ALL_IF, &vrrp->flags) ? "i/f" : "VMAC", vrrp->vmac_garp_intvl.tv_sec); - ctime_r(&vrrp->vmac_garp_timer.tv_sec, time_str); - conf_write(fp, " Next gratuitous ARP for such secondary = %" PRI_tv_sec ".%6.6" PRI_tv_usec " (%.24s.%6.6" PRI_tv_usec ")", vrrp->vmac_garp_timer.tv_sec, vrrp->vmac_garp_timer.tv_usec, time_str, vrrp->vmac_garp_timer.tv_usec); } #endif conf_write(fp, " Send advert after receive lower priority advert = %s", vrrp->lower_prio_no_advert ? "false" : "true"); diff --git a/keepalived/vrrp/vrrp_if.c b/keepalived/vrrp/vrrp_if.c index 5f00e8d11..14ba84380 100644 --- a/keepalived/vrrp/vrrp_if.c +++ b/keepalived/vrrp/vrrp_if.c @@ -456,6 +456,8 @@ alloc_garp_delay(void) PMALLOC(gd); INIT_LIST_HEAD(&gd->e_list); + INIT_LIST_HEAD(&gd->garp_list); + INIT_LIST_HEAD(&gd->gna_list); list_add_tail(&gd->e_list, &garp_delay); return gd; diff --git a/keepalived/vrrp/vrrp_ipaddress.c b/keepalived/vrrp/vrrp_ipaddress.c index 7525e5229..116fca63d 100644 --- a/keepalived/vrrp/vrrp_ipaddress.c +++ b/keepalived/vrrp/vrrp_ipaddress.c @@ -268,6 +268,7 @@ free_ipaddress(ip_address_t *ip_addr) { FREE_PTR(ip_addr->label); list_del_init(&ip_addr->e_list); + list_del_init(&ip_addr->garp_gna_list); FREE(ip_addr); } @@ -484,6 +485,7 @@ alloc_ipaddress(const vector_t *strvec, bool static_addr) return NULL; } INIT_LIST_HEAD(&new->e_list); + INIT_LIST_HEAD(&new->garp_gna_list); /* We expect the address first */ if (!parse_ipaddress(new, strvec_slot(strvec, 0), true)) { diff --git a/keepalived/vrrp/vrrp_ndisc.c b/keepalived/vrrp/vrrp_ndisc.c index cc997151c..7a71cf7fa 100644 --- a/keepalived/vrrp/vrrp_ndisc.c +++ b/keepalived/vrrp/vrrp_ndisc.c @@ -263,26 +263,18 @@ ndisc_send_unsolicited_na_immediate(interface_t *ifp, ip_address_t *ipaddress) } static void -queue_ndisc(vrrp_t *vrrp, interface_t *ifp, ip_address_t *ipaddress) +queue_ndisc(interface_t *ifp, ip_address_t *ipaddress) { - timeval_t next_time = timer_add_now(ifp->garp_delay->gna_interval); + ipaddress->garp_gna_pending = 1; - vrrp->garp_gna_pending = true; - ipaddress->garp_gna_pending = true; + if (list_empty(&ifp->garp_delay->gna_list)) + thread_add_timer(master, vrrp_gna_thread, ifp, timer_long(timer_sub_now(ifp->garp_delay->gna_next_time))); - /* Do we need to schedule/reschedule the garp thread? */ - if (!garp_thread || timercmp(&next_time, &garp_next_time, <)) { - if (garp_thread) - thread_cancel(garp_thread); - - garp_next_time = next_time; - - garp_thread = thread_add_timer(master, vrrp_arp_thread, NULL, timer_long(ifp->garp_delay->gna_interval)); - } + list_add_tail(&ipaddress->garp_gna_list, &ifp->garp_delay->gna_list); } void -ndisc_send_unsolicited_na(vrrp_t *vrrp, ip_address_t *ipaddress) +ndisc_send_unsolicited_na(ip_address_t *ipaddress, unsigned rep) { interface_t *ifp = IF_BASE_IFP(ipaddress->ifp); @@ -290,6 +282,12 @@ ndisc_send_unsolicited_na(vrrp_t *vrrp, ip_address_t *ipaddress) if (ifp->ifi_flags & IFF_NOARP) return; + if (ipaddress->garp_gna_pending) { + if (ipaddress->garp_gna_pending < rep) + ipaddress->garp_gna_pending++; + return; + } + set_time_now(); /* Do we need to delay sending the ndisc? */ @@ -297,7 +295,7 @@ ndisc_send_unsolicited_na(vrrp_t *vrrp, ip_address_t *ipaddress) ifp->garp_delay->have_gna_interval && ifp->garp_delay->gna_next_time.tv_sec && timercmp(&time_now, &ifp->garp_delay->gna_next_time, <)) - queue_ndisc(vrrp, ifp, ipaddress); + queue_ndisc(ifp, ipaddress); else ndisc_send_unsolicited_na_immediate(ifp, ipaddress); } diff --git a/keepalived/vrrp/vrrp_scheduler.c b/keepalived/vrrp/vrrp_scheduler.c index de2cfdceb..df4d80b7b 100644 --- a/keepalived/vrrp/vrrp_scheduler.c +++ b/keepalived/vrrp/vrrp_scheduler.c @@ -77,8 +77,6 @@ #endif /* global vars */ -timeval_t garp_next_time; -thread_ref_t garp_thread; bool vrrp_initialised; timeval_t vrrp_delayed_start_time; @@ -664,6 +662,31 @@ vrrp_lower_prio_gratuitous_arp_thread(thread_ref_t thread) vrrp_send_link_update(vrrp, vrrp->garp_lower_prio_rep); } +/* Gratuitous ARP refresh thread (i.e. periodic send of GARP messages) */ +void +vrrp_gratuitous_arp_refresh_thread(thread_ref_t thread) +{ + vrrp_t *vrrp = THREAD_ARG(thread); + + vrrp_send_link_update(vrrp, vrrp->garp_refresh_rep); + thread_add_timer(master, vrrp_gratuitous_arp_refresh_thread, + vrrp, timer_long(vrrp->garp_refresh)); +} + +#ifdef _HAVE_VRRP_VMAC_ +/* Gratuitous ARP VMAC update thread (i.e. one GARP per VMAC interface + * on which VIPs are configured. */ +void +vrrp_gratuitous_arp_vmac_update_thread(thread_ref_t thread) +{ + vrrp_t *vrrp = THREAD_ARG(thread); + + vrrp_send_vmac_update(vrrp); + thread_add_timer(master, vrrp_gratuitous_arp_vmac_update_thread, + vrrp, timer_long(vrrp->vmac_garp_intvl)); +} +#endif + void try_up_instance(vrrp_t *vrrp, bool leaving_init) { @@ -1381,86 +1404,54 @@ vrrp_script_child_thread(thread_ref_t thread) vscript->init_state = SCRIPT_INIT_STATE_DONE; } -/* Delayed ARP/NA thread */ -static int -vrrp_arpna_send(vrrp_t *vrrp, list_head_t *l, timeval_t *n) +/* Thread to send gratuitous ARPs when the sending is rate limited */ +void +vrrp_arp_thread(thread_ref_t thread) { ip_address_t *ip_addr; - interface_t *ifp; - - list_for_each_entry(ip_addr, l, e_list) { - if (!ip_addr->garp_gna_pending) - continue; + interface_t *ifp = THREAD_ARG(thread); - if (!ip_addr->set) { - ip_addr->garp_gna_pending = false; - continue; - } + while (!list_empty(&ifp->garp_delay->garp_list)) { + set_time_now(); + if (timercmp(&time_now, &ifp->garp_delay->garp_next_time, <)) + break; - ifp = IF_BASE_IFP(ip_addr->ifp); + ip_addr = list_first_entry(&ifp->garp_delay->garp_list, ip_address_t, garp_gna_list); - /* This should never happen */ - if (!ifp->garp_delay) { - ip_addr->garp_gna_pending = false; - continue; - } + send_gratuitous_arp_immediate(ifp, ip_addr); - /* IPv4 handling */ - if (!IP_IS6(ip_addr)) { - if (timercmp(&time_now, &ifp->garp_delay->garp_next_time, >=)) { - send_gratuitous_arp_immediate(ifp, ip_addr); - ip_addr->garp_gna_pending = false; - } else { - vrrp->garp_gna_pending = true; - if (timercmp(&ifp->garp_delay->garp_next_time, n, <)) - *n = ifp->garp_delay->garp_next_time; - } - } else { - /* IPv6 handling */ - if (timercmp(&time_now, &ifp->garp_delay->gna_next_time, >=)) { - ndisc_send_unsolicited_na_immediate(ifp, ip_addr); - ip_addr->garp_gna_pending = false; - } else { - vrrp->garp_gna_pending = true; - if (timercmp(&ifp->garp_delay->gna_next_time, n, <)) - *n = ifp->garp_delay->gna_next_time; - } - } + list_del_init(&ip_addr->garp_gna_list); + if (--ip_addr->garp_gna_pending) + list_add_tail(&ip_addr->garp_gna_list, &ifp->garp_delay->garp_list); } - return 0; + if (!list_empty(&ifp->garp_delay->garp_list)) + thread_add_timer(master, vrrp_arp_thread, ifp, timer_long(timer_sub_now(ifp->garp_delay->garp_next_time))); } +/* Thread to send gratuitous NDs when the sending is rate limited */ void -vrrp_arp_thread(thread_ref_t thread) +vrrp_gna_thread(thread_ref_t thread) { - vrrp_t *vrrp; - timeval_t next_time = { - .tv_sec = INT_MAX /* We're never going to delay this long - I hope! */ - }; - - set_time_now(); + ip_address_t *ip_addr; + interface_t *ifp = THREAD_ARG(thread); - list_for_each_entry(vrrp, &vrrp_data->vrrp, e_list) { - if (!vrrp->garp_gna_pending) - continue; + while (!list_empty(&ifp->garp_delay->gna_list)) { + set_time_now(); + if (timercmp(&time_now, &ifp->garp_delay->gna_next_time, <)) + break; - vrrp->garp_gna_pending = false; + ip_addr = list_first_entry(&ifp->garp_delay->gna_list, ip_address_t, garp_gna_list); - if (vrrp->state != VRRP_STATE_MAST || !vrrp->vipset) - continue; + ndisc_send_unsolicited_na_immediate(ifp, ip_addr); - vrrp_arpna_send(vrrp, &vrrp->vip, &next_time); - vrrp_arpna_send(vrrp, &vrrp->evip, &next_time); + list_del_init(&ip_addr->garp_gna_list); + if (--ip_addr->garp_gna_pending) + list_add_tail(&ip_addr->garp_gna_list, &ifp->garp_delay->gna_list); } - if (next_time.tv_sec != INT_MAX) { - /* Register next timer tracker */ - garp_next_time = next_time; - garp_thread = thread_add_timer(thread->master, vrrp_arp_thread, NULL, - timer_long(timer_sub_now(next_time))); - } else - garp_thread = NULL; + if (!list_empty(&ifp->garp_delay->gna_list)) + thread_add_timer(master, vrrp_gna_thread, ifp, timer_long(timer_sub_now(ifp->garp_delay->gna_next_time))); } #ifdef _WITH_DUMP_THREADS_ @@ -1507,9 +1498,12 @@ void register_vrrp_scheduler_addresses(void) { register_thread_address("vrrp_arp_thread", vrrp_arp_thread); + register_thread_address("vrrp_gna_thread", vrrp_gna_thread); register_thread_address("vrrp_dispatcher_init", vrrp_dispatcher_init); register_thread_address("vrrp_gratuitous_arp_thread", vrrp_gratuitous_arp_thread); register_thread_address("vrrp_lower_prio_gratuitous_arp_thread", vrrp_lower_prio_gratuitous_arp_thread); + register_thread_address("vrrp_gratuitous_arp_refresh_thread", vrrp_gratuitous_arp_refresh_thread); + register_thread_address("vrrp_gratuitous_arp_vmac_update_thread", vrrp_gratuitous_arp_vmac_update_thread); register_thread_address("vrrp_script_child_thread", vrrp_script_child_thread); register_thread_address("vrrp_script_thread", vrrp_script_thread); register_thread_address("vrrp_read_dispatcher_thread", vrrp_read_dispatcher_thread); From 5661fadce21cf20fd8d99efe6faf5d744180378b Mon Sep 17 00:00:00 2001 From: Quentin Armitage Date: Mon, 21 Oct 2024 22:00:22 +0100 Subject: [PATCH 08/16] vrrp: remove aggregation_group field from garp_delay_t structure The field simply was not needed, or really used. Signed-off-by: Quentin Armitage --- keepalived/include/vrrp_if.h | 1 - keepalived/vrrp/vrrp_if.c | 4 +--- keepalived/vrrp/vrrp_parser.c | 14 ++++++-------- 3 files changed, 7 insertions(+), 12 deletions(-) diff --git a/keepalived/include/vrrp_if.h b/keepalived/include/vrrp_if.h index fb0bd3862..7e2eb843b 100644 --- a/keepalived/include/vrrp_if.h +++ b/keepalived/include/vrrp_if.h @@ -78,7 +78,6 @@ typedef struct _garp_delay { bool have_gna_interval; /* True if delay */ timeval_t garp_next_time; /* Time when next gratuitous ARP message can be sent */ timeval_t gna_next_time; /* Time when next gratuitous NA message can be sent */ - int aggregation_group; /* Index of multi-interface group */ /* linked list of ip_address_t that have GARP/NAs pending */ list_head_t garp_list; diff --git a/keepalived/vrrp/vrrp_if.c b/keepalived/vrrp/vrrp_if.c index 14ba84380..fa8c8d8ea 100644 --- a/keepalived/vrrp/vrrp_if.c +++ b/keepalived/vrrp/vrrp_if.c @@ -416,7 +416,7 @@ dump_garp_delay(FILE *fp, const garp_delay_t *gd) char time_str[26]; interface_t *ifp; - conf_write(fp, "------< GARP delay group %d >------", gd->aggregation_group); + conf_write(fp, "------< GARP delay group >------"); if (gd->have_garp_interval) { conf_write(fp, " GARP interval = %" PRI_tv_sec ".%6.6" PRI_tv_usec, gd->garp_interval.tv_sec, gd->garp_interval.tv_usec); @@ -699,8 +699,6 @@ dump_if(FILE *fp, const interface_t *ifp) conf_write(fp, " Gratuitous NA interval %" PRI_time_t "ms", ifp->garp_delay->gna_interval.tv_sec * 1000 + ifp->garp_delay->gna_interval.tv_usec / (TIMER_HZ / 1000)); - if (ifp->garp_delay->aggregation_group) - conf_write(fp, " Gratuitous ARP aggregation group %d", ifp->garp_delay->aggregation_group); } #ifdef _HAVE_VRRP_VMAC_ diff --git a/keepalived/vrrp/vrrp_parser.c b/keepalived/vrrp/vrrp_parser.c index 751d7dbba..f8473dd97 100644 --- a/keepalived/vrrp/vrrp_parser.c +++ b/keepalived/vrrp/vrrp_parser.c @@ -83,6 +83,7 @@ static vrrp_script_t *current_vscr; #ifdef _WITH_TRACK_PROCESS_ static vrrp_tracked_process_t *current_tp; #endif +static unsigned cur_aggregation_group; /* track groups for static items */ @@ -1971,7 +1972,6 @@ garp_group_interfaces_handler(const vector_t *strvec) { interface_t *ifp; const vector_t *interface_vec = read_value_block(strvec); - garp_delay_t *gd; size_t i; /* Handle the interfaces block being empty */ @@ -1980,12 +1980,8 @@ garp_group_interfaces_handler(const vector_t *strvec) return; } - /* First set the next aggregation group number */ - current_ggd->aggregation_group = 1; - list_for_each_entry(gd, &garp_delay, e_list) { - if (gd->aggregation_group && gd != current_ggd) - current_ggd->aggregation_group++; - } + /* First set the configuration aggregation group number */ + cur_aggregation_group++; for (i = 0; i < vector_size(interface_vec); i++) { ifp = if_get_by_ifname(vector_slot(interface_vec, i), IF_CREATE_IF_DYNAMIC); @@ -2020,7 +2016,7 @@ garp_group_end_handler(void) list_head_t *ifq; if (!current_ggd->have_garp_interval && !current_ggd->have_gna_interval) { - report_config_error(CONFIG_GENERAL_ERROR, "garp group %d does not have any delay set - removing", current_ggd->aggregation_group); + report_config_error(CONFIG_GENERAL_ERROR, "garp group %u does not have any delay set - removing", cur_aggregation_group); /* Remove the garp_delay from any interfaces that are using it */ ifq = get_interface_queue(); @@ -2282,5 +2278,7 @@ vrrp_init_keywords(void) #endif add_track_file_keywords(true); + cur_aggregation_group = 0; + return keywords; } From 360ec2a8603c6c51e50b38270a6470b27211f6f7 Mon Sep 17 00:00:00 2001 From: Quentin Armitage Date: Mon, 21 Oct 2024 22:04:39 +0100 Subject: [PATCH 09/16] vrrp: allow garp_group garp_interval to take full range of unsigned values Also allies to gna_interval Signed-off-by: Quentin Armitage --- keepalived/vrrp/vrrp_parser.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/keepalived/vrrp/vrrp_parser.c b/keepalived/vrrp/vrrp_parser.c index f8473dd97..0980803db 100644 --- a/keepalived/vrrp/vrrp_parser.c +++ b/keepalived/vrrp/vrrp_parser.c @@ -1915,7 +1915,7 @@ garp_group_garp_interval_handler(const vector_t *strvec) { unsigned val; - if (!read_decimal_unsigned_strvec(strvec, 1, &val, 0, INT_MAX, TIMER_HZ_DIGITS, true)) { + if (!read_decimal_unsigned_strvec(strvec, 1, &val, 0, UINT_MAX, TIMER_HZ_DIGITS, true)) { report_config_error(CONFIG_GENERAL_ERROR, "garp_group garp_interval '%s' invalid", strvec_slot(strvec, 1)); return; } @@ -1932,7 +1932,7 @@ garp_group_gna_interval_handler(const vector_t *strvec) { unsigned val; - if (!read_decimal_unsigned_strvec(strvec, 1, &val, 0, INT_MAX, TIMER_HZ_DIGITS, true)) { + if (!read_decimal_unsigned_strvec(strvec, 1, &val, 0, UINT_MAX, TIMER_HZ_DIGITS, true)) { report_config_error(CONFIG_GENERAL_ERROR, "garp_group gna_interval '%s' invalid", strvec_slot(strvec, 1)); return; } From b0eed52abf58f2a99f6e474f7d6da0827aa55b80 Mon Sep 17 00:00:00 2001 From: Quentin Armitage Date: Mon, 21 Oct 2024 22:09:24 +0100 Subject: [PATCH 10/16] vrrp: only alloc garp delay structure if address family matches Signed-off-by: Quentin Armitage --- keepalived/vrrp/vrrp_if.c | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/keepalived/vrrp/vrrp_if.c b/keepalived/vrrp/vrrp_if.c index fa8c8d8ea..a9e1b128f 100644 --- a/keepalived/vrrp/vrrp_if.c +++ b/keepalived/vrrp/vrrp_if.c @@ -482,6 +482,7 @@ set_default_garp_delay(void) vrrp_t *vrrp; list_head_t *vip_list; ip_address_t *vip; + bool have_ipv4, have_ipv6; if (global_data->vrrp_garp_interval) { default_delay.garp_interval.tv_sec = global_data->vrrp_garp_interval / TIMER_HZ; @@ -499,6 +500,24 @@ set_default_garp_delay(void) list_for_each_entry(vrrp, &vrrp_data->vrrp, e_list) { if (!vrrp->ifp) continue; + + /* Check what family of addresses we have */ + have_ipv4 = vrrp->family == AF_INET; + have_ipv6 = vrrp->family == AF_INET6; + + list_for_each_entry(vip, &vrrp->evip, e_list) { + if (IP_IS6(vip)) + have_ipv6 = true; + else + have_ipv4 = true; + } + + /* We don't need a delay if there isn't a delay for the + * address family we are using */ + if (!((have_ipv4 && global_data->vrrp_garp_interval) || + (have_ipv6 && global_data->vrrp_gna_interval))) + continue; + ifp = IF_BASE_IFP(vrrp->ifp); if (!ifp->garp_delay) set_garp_delay(ifp, &default_delay); From ecdb339308b375dec2e23ba07fc8018c6c427e2c Mon Sep 17 00:00:00 2001 From: Quentin Armitage Date: Mon, 21 Oct 2024 22:11:49 +0100 Subject: [PATCH 11/16] all: Fix parsing of \xNN in quoted strings Following \x keepalived processed all following hex digits, but only returned one byte. For example \x20file would result in a byte 0x0f followed by the string "ile". This commit limits the number of hex digits consumed to 2. Signed-off-by: Quentin Armitage --- doc/man/man5/keepalived.conf.5.in | 10 +++++----- lib/parser.c | 3 ++- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/doc/man/man5/keepalived.conf.5.in b/doc/man/man5/keepalived.conf.5.in index f5c5825be..2202297ec 100644 --- a/doc/man/man5/keepalived.conf.5.in +++ b/doc/man/man5/keepalived.conf.5.in @@ -128,11 +128,11 @@ will be the three strings: i.e. the " and ' characters are removed and any intervening whitespace is retained. .PP Quoted strings can also have escaped characters, like the shell. \\a, \\b, \\E, \\f, -\\n, \\r, \\t, \\v, \\nnn and \\xXX (where nnn is up to 3 octal digits, and XX is any -sequence of hex digits) and \\cC (which produces the control version of -character C) are all supported. \\C for any other character C is just -treated as an escaped version of character C, so \\\\ is a \\ character and -\\" will be a " character, but it won't start or terminate a quoted string. +\\n, \\r, \\t, \\v, \\nnn and \\xXX (where nnn is up to 3 octal digits, and XX is up +to two hex digits) and \\cC (which produces the control version of character C) are +all supported. \\C for any other character C is just treated as an escaped version +of character C, so \\\\ is a \\ character and \\" will be a " character, but it +won't start or terminate a quoted string. .PP For specifying scripts with parameters, unquoted spaces will separate the parameters. If it is required for a parameter to contain a space, it should be enclosed in single diff --git a/lib/parser.c b/lib/parser.c index f34059fd8..6b8088335 100644 --- a/lib/parser.c +++ b/lib/parser.c @@ -1033,6 +1033,7 @@ alloc_strvec_quoted_escaped(const char *src) char *op_buf; const char *ofs, *ofs1; char op_char; + unsigned i; if (!src) { if (!buf_extern) @@ -1086,7 +1087,7 @@ alloc_strvec_quoted_escaped(const char *src) if (*ofs == 'x' && isxdigit(ofs[1])) { op_char = 0; ofs++; - while (isxdigit(*ofs)) { + for (i = 0; i <= 1 && isxdigit(*ofs); i++) { op_char <<= 4; op_char |= isdigit(*ofs) ? *ofs - '0' : (10 + *ofs - (isupper(*ofs) ? 'A' : 'a')); ofs++; From af7aa8bf56eebf98a8a0290bbc066c42fccfec68 Mon Sep 17 00:00:00 2001 From: Quentin Armitage Date: Mon, 21 Oct 2024 22:16:35 +0100 Subject: [PATCH 12/16] all: fix parsing of escaped characters in quoted strings Signed-off-by: Quentin Armitage --- doc/man/man5/keepalived.conf.5.in | 18 ++++- keepalived/check/check_misc.c | 2 +- keepalived/vrrp/vrrp_parser.c | 2 +- lib/notify.c | 2 +- lib/parser.c | 121 +++++++++++++++++------------- lib/parser.h | 1 + 6 files changed, 90 insertions(+), 56 deletions(-) diff --git a/doc/man/man5/keepalived.conf.5.in b/doc/man/man5/keepalived.conf.5.in index 2202297ec..a8ce9a114 100644 --- a/doc/man/man5/keepalived.conf.5.in +++ b/doc/man/man5/keepalived.conf.5.in @@ -136,7 +136,23 @@ won't start or terminate a quoted string. .PP For specifying scripts with parameters, unquoted spaces will separate the parameters. If it is required for a parameter to contain a space, it should be enclosed in single -quotes ('). +quotes ('). For example + +.nf +.RS +$SG_NAME=SG1 +$INST=low +$USER=user +notify_master "/etc/keepalived/notify_event.sh ' spaces\\\\x20f\\x69le ' '\\"s p a c e \\"' ${SG_NAME}.$INST master" $USER group +.RE + +.fi +specifies a notify_master script /etc/keepalived/notify_event.sh that will be executed as user:group with parameters + +.nf +.RS +\' spaces\\x20file \', \'"s p a c e "\', \'SG1.low\' and \'master\' +.RE .PP .SH CONFIGURATION PARSER diff --git a/keepalived/check/check_misc.c b/keepalived/check/check_misc.c index a9df6883d..5367c1704 100644 --- a/keepalived/check/check_misc.c +++ b/keepalived/check/check_misc.c @@ -133,7 +133,7 @@ misc_path_handler(__attribute__((unused)) const vector_t *strvec) misc_checker_t *new_misck_checker = current_checker->data; /* We need to allow quoted and escaped strings for the script and parameters */ - strvec_qe = alloc_strvec_quoted_escaped(NULL); + strvec_qe = alloc_strvec_quoted(NULL); set_script_params_array(strvec_qe, &new_misck_checker->script, 0); diff --git a/keepalived/vrrp/vrrp_parser.c b/keepalived/vrrp/vrrp_parser.c index 0980803db..f9ca6bb6a 100644 --- a/keepalived/vrrp/vrrp_parser.c +++ b/keepalived/vrrp/vrrp_parser.c @@ -1539,7 +1539,7 @@ vrrp_vscript_script_handler(__attribute__((unused)) const vector_t *strvec) const vector_t *strvec_qe; /* We need to allow quoted and escaped strings for the script and parameters */ - strvec_qe = alloc_strvec_quoted_escaped(NULL); + strvec_qe = alloc_strvec_quoted(NULL); set_script_params_array(strvec_qe, ¤t_vscr->script, 0); free_strvec(strvec_qe); diff --git a/lib/notify.c b/lib/notify.c index 119c826c5..d49c26e35 100644 --- a/lib/notify.c +++ b/lib/notify.c @@ -1117,7 +1117,7 @@ notify_script_init(int extra_params, const char *type) const vector_t *strvec_qe; /* We need to reparse the command line, allowing for quoted and escaped strings */ - strvec_qe = alloc_strvec_quoted_escaped(NULL); + strvec_qe = alloc_strvec_quoted(NULL); if (!strvec_qe) { log_message(LOG_INFO, "Unable to parse notify script"); diff --git a/lib/parser.c b/lib/parser.c index 6b8088335..681647194 100644 --- a/lib/parser.c +++ b/lib/parser.c @@ -1024,8 +1024,8 @@ get_random(const def_t *def) return rand_str; } -const vector_t * -alloc_strvec_quoted_escaped(const char *src) +static const vector_t * +alloc_strvec_quoted_escaped_common(const char *src, bool escapes) { vector_t *strvec; char cur_quote = 0; @@ -1084,61 +1084,66 @@ alloc_strvec_quoted_escaped(const char *src) goto err_exit; } - if (*ofs == 'x' && isxdigit(ofs[1])) { - op_char = 0; - ofs++; - for (i = 0; i <= 1 && isxdigit(*ofs); i++) { - op_char <<= 4; - op_char |= isdigit(*ofs) ? *ofs - '0' : (10 + *ofs - (isupper(*ofs) ? 'A' : 'a')); + if (escapes) { + if (*ofs == 'x' && isxdigit(ofs[1])) { + op_char = 0; ofs++; + for (i = 0; i <= 1 && isxdigit(*ofs); i++) { + op_char <<= 4; + op_char |= isdigit(*ofs) ? *ofs - '0' : (10 + *ofs - (isupper(*ofs) ? 'A' : 'a')); + ofs++; + } } - } - else if (*ofs == 'c' && ofs[1]) { - op_char = *++ofs & 0x1f; /* Convert to control character */ - ofs++; - } - else if (*ofs >= '0' && *ofs <= '7') { - op_char = *ofs++ - '0'; - if (*ofs >= '0' && *ofs <= '7') { - op_char <<= 3; - op_char += *ofs++ - '0'; + else if (*ofs == 'c' && ofs[1]) { + op_char = *++ofs & 0x1f; /* Convert to control character */ + ofs++; } - if (*ofs >= '0' && *ofs <= '7') { - op_char <<= 3; - op_char += *ofs++ - '0'; + else if (*ofs >= '0' && *ofs <= '7') { + op_char = *ofs++ - '0'; + if (*ofs >= '0' && *ofs <= '7') { + op_char <<= 3; + op_char += *ofs++ - '0'; + } + if (*ofs >= '0' && *ofs <= '7') { + op_char <<= 3; + op_char += *ofs++ - '0'; + } } - } - else { - switch (*ofs) { - case 'a': - op_char = '\a'; - break; - case 'b': - op_char = '\b'; - break; - case 'E': - op_char = 0x1b; - break; - case 'f': - op_char = '\f'; - break; - case 'n': - op_char = '\n'; - break; - case 'r': - op_char = '\r'; - break; - case 't': - op_char = '\t'; - break; - case 'v': - op_char = '\v'; - break; - default: /* \"' */ - op_char = *ofs; - break; + else { + switch (*ofs) { + case 'a': + op_char = '\a'; + break; + case 'b': + op_char = '\b'; + break; + case 'E': + op_char = 0x1b; + break; + case 'f': + op_char = '\f'; + break; + case 'n': + op_char = '\n'; + break; + case 'r': + op_char = '\r'; + break; + case 't': + op_char = '\t'; + break; + case 'v': + op_char = '\v'; + break; + default: /* \"' */ + op_char = *ofs; + break; + } + ofs++; } - ofs++; + } else { + *ofs_op++ = '\\'; + op_char = *ofs++; } *ofs_op++ = op_char; @@ -1180,6 +1185,18 @@ alloc_strvec_quoted_escaped(const char *src) return NULL; } +const vector_t * +alloc_strvec_quoted_escaped(const char *src) +{ + return alloc_strvec_quoted_escaped_common(src, true); +} + +const vector_t * +alloc_strvec_quoted(const char *src) +{ + return alloc_strvec_quoted_escaped_common(src, false); +} + vector_t * alloc_strvec_r(const char *string) { diff --git a/lib/parser.h b/lib/parser.h index 10a6513ca..6222c1623 100644 --- a/lib/parser.h +++ b/lib/parser.h @@ -145,6 +145,7 @@ extern void install_sublevel_end(vpp_t); extern void install_level_end_handler(void (*handler) (void)); extern void install_keyword(const char *, void (*handler) (const vector_t *)); extern const vector_t *alloc_strvec_quoted_escaped(const char *); +extern const vector_t *alloc_strvec_quoted(const char *); extern vector_t *alloc_strvec_r(const char *); extern bool check_conf_file(const char*); extern const vector_t *read_value_block(const vector_t *); From c875649f678a40a918154cc766d63ac49faeee0c Mon Sep 17 00:00:00 2001 From: Quentin Armitage Date: Mon, 21 Oct 2024 22:17:25 +0100 Subject: [PATCH 13/16] vrrp: stop using alloc_strvec() for parsing rttables files It was a good idea at the time, but is not really appropriate. The parsing can be done just as simply without using alloc_strvec(). Signed-off-by: Quentin Armitage --- lib/rttables.c | 42 ++++++++++++++++++++---------------------- 1 file changed, 20 insertions(+), 22 deletions(-) diff --git a/lib/rttables.c b/lib/rttables.c index aaad772ea..e90cdd691 100644 --- a/lib/rttables.c +++ b/lib/rttables.c @@ -171,54 +171,57 @@ read_file(const char *file_name, list_head_t *l, uint32_t max) { FILE *fp; rt_entry_t *rte; - vector_t *strvec = NULL; char buf[MAX_RT_BUF]; unsigned long id; - const char *number; + const char *number, *name; char *endptr; + size_t len; fp = fopen(file_name, "r"); if (!fp) return; while (fgets(buf, MAX_RT_BUF, fp)) { - strvec = alloc_strvec(buf); + /* Remove comments */ + if ((endptr = strchr(buf, '#'))) + *endptr = '\0'; - if (!strvec) + /* Remove trailing '\n' and skip empty lines */ + if (!(len = strlen(buf))) continue; + if (buf[len - 1] == '\n') { + if (len == 1) + continue; + buf[len - 1] = '\0'; + } - if (vector_size(strvec) != 2) { - free_strvec(strvec); + /* check we have only two fields, and get them */ + if (!(number = strtok(buf, " \t"))) + continue; + if (!(name = strtok(NULL, " \t"))) + continue; + if (strtok(NULL, " \t")) continue; - } PMALLOC(rte); - if (!rte) { - free_strvec(strvec); + if (!rte) goto err; - } INIT_LIST_HEAD(&rte->e_list); - number = strvec_slot(strvec, 0); - number += strspn(number, " \t"); id = strtoul(number, &endptr, 0); if (*number == '-' || number == endptr || *endptr || id > max) { FREE(rte); - free_strvec(strvec); continue; } rte->id = (unsigned)id; - rte->name = STRDUP(strvec_slot(strvec, 1)); + rte->name = STRDUP(name); if (!rte->name) { FREE(rte); - free_strvec(strvec); goto err; } list_add_tail(&rte->e_list, l); - - free_strvec(strvec); } fclose(fp); @@ -227,12 +230,7 @@ read_file(const char *file_name, list_head_t *l, uint32_t max) err: fclose(fp); - if (strvec) - free_strvec(strvec); - free_rt_entry_list(l); - - return; } static void From 055a7b175311683ae8566f9e5c3778d639ff2e6d Mon Sep 17 00:00:00 2001 From: Quentin Armitage Date: Mon, 21 Oct 2024 22:19:13 +0100 Subject: [PATCH 14/16] all: stop "unmatched quotes" warning for quoted strings If a line with a quoted string has unbalanced quote characters when parsed as a standard (not quoted) string, an innapropriate warning was issued for unmatches quotes. This commit now stops the warning. This commit is not elegant, and it would be appreciated if a neater solution could be found. If anyone has a better solution, please submit a pull request or raise an issue explaining the solution. Signed-off-by: Quentin Armitage --- doc/man/man5/keepalived.conf.5.in | 10 +++--- keepalived/check/check_misc.c | 2 +- keepalived/check/check_parser.c | 8 ++--- keepalived/core/global_parser.c | 10 +++--- keepalived/vrrp/vrrp_parser.c | 26 ++++++++-------- lib/parser.c | 52 +++++++++++++++++++++++++------ lib/parser.h | 10 +++--- 7 files changed, 76 insertions(+), 42 deletions(-) diff --git a/doc/man/man5/keepalived.conf.5.in b/doc/man/man5/keepalived.conf.5.in index a8ce9a114..64eb70d01 100644 --- a/doc/man/man5/keepalived.conf.5.in +++ b/doc/man/man5/keepalived.conf.5.in @@ -747,7 +747,7 @@ possibly following any cleanup actions needed. # script to be run by keepalived to process notify events # The FIFO name will be passed to the script as the last parameter - \fBnotify_fifo_script \fRSTRING|QUOTED_STRING [username [groupname]] + \fBnotify_fifo_script \fRSTRING|QUOTED-STRING [username [groupname]] # FIFO to write vrrp notify events to. # The string written will be a line of the form: INSTANCE "VI_1" MASTER 100 @@ -758,7 +758,7 @@ possibly following any cleanup actions needed. # script to be run by keepalived to process vrrp notify events # The FIFO name will be passed to the script as the last parameter - \fBvrrp_notify_fifo_script \fRSTRING|QUOTED_STRING [username [groupname]] + \fBvrrp_notify_fifo_script \fRSTRING|QUOTED-STRING [username [groupname]] # FIFO to write notify healthchecker events to # The string written will be a line of the form: @@ -769,7 +769,7 @@ possibly following any cleanup actions needed. # script to be run by keepalived to process healthchecher notify events # The FIFO name will be passed to the script as the last parameter - \fBlvs_notify_fifo_script \fRSTRING|QUOTED_STRING [username [groupname]] + \fBlvs_notify_fifo_script \fRSTRING|QUOTED-STRING [username [groupname]] # By default, when keepalived reloads the vrrp instance and sync group states # are not written to the relevant FIFOs. Setting this option will cause the @@ -1143,7 +1143,7 @@ The syntax for track file is: \fBtrack_file \fR { # vrrp_track_file is a deprecated synonym # file to track (weight defaults to 1) - \fBfile \fR + \fBfile \fR # optional default weight \fBweight \fR<-2147483647..2147483647> [reverse] @@ -1171,7 +1171,7 @@ The configuration block looks like: # process "@KA_TMP_DIR@/a b" param1 "param 2" # would mean a process named '@KA_TMP_DIR@/a b' (quotes removed) with 2 parameters # 'param1' and 'param 2'. - \fBprocess\fR | [| ...] + \fBprocess\fR | [| ...] # If matching parameters, this specifies a partial match (i.e. the first # n parameters match exactly), or an initial match, i.e. the last diff --git a/keepalived/check/check_misc.c b/keepalived/check/check_misc.c index 5367c1704..a4d76719d 100644 --- a/keepalived/check/check_misc.c +++ b/keepalived/check/check_misc.c @@ -209,7 +209,7 @@ install_misc_check_keyword(void) install_keyword("MISC_CHECK", &misc_check_handler); check_ptr = install_sublevel(VPP ¤t_checker); install_checker_common_keywords(false); - install_keyword("misc_path", &misc_path_handler); + install_keyword_quoted("misc_path", &misc_path_handler); install_keyword("misc_timeout", &misc_timeout_handler); install_keyword("misc_dynamic", &misc_dynamic_handler); install_keyword("user", &misc_user_handler); diff --git a/keepalived/check/check_parser.c b/keepalived/check/check_parser.c index ce15fe49f..3ae30339e 100644 --- a/keepalived/check/check_parser.c +++ b/keepalived/check/check_parser.c @@ -987,8 +987,8 @@ init_check_keywords(bool active) /* Pool regression detection and handling. */ install_keyword("alpha", &vs_alpha_handler); install_keyword("omega", &omega_handler); - install_keyword("quorum_up", &quorum_up_handler); - install_keyword("quorum_down", &quorum_down_handler); + install_keyword_quoted("quorum_up", &quorum_up_handler); + install_keyword_quoted("quorum_down", &quorum_down_handler); install_keyword("quorum", &quorum_handler); install_keyword("hysteresis", &hysteresis_handler); install_keyword("weight", &vs_weight_handler); @@ -1004,8 +1004,8 @@ init_check_keywords(bool active) install_keyword("uthreshold", &uthreshold_handler); install_keyword("lthreshold", <hreshold_handler); install_keyword("inhibit_on_failure", &rs_inhibit_handler); - install_keyword("notify_up", ¬ify_up_handler); - install_keyword("notify_down", ¬ify_down_handler); + install_keyword_quoted("notify_up", ¬ify_up_handler); + install_keyword_quoted("notify_down", ¬ify_down_handler); install_keyword("alpha", &rs_alpha_handler); install_keyword("retry", &rs_retry_handler); install_keyword("delay_before_retry", &rs_delay_before_retry_handler); diff --git a/keepalived/core/global_parser.c b/keepalived/core/global_parser.c index 38f13807d..2b6e3ba64 100644 --- a/keepalived/core/global_parser.c +++ b/keepalived/core/global_parser.c @@ -2474,9 +2474,9 @@ init_global_keywords(bool global_active) install_keyword("smtp_connect_timeout", &smtpto_handler); install_keyword("notification_email", &email_handler); install_keyword("smtp_alert", &smtp_alert_handler); - install_keyword("startup_script", &startup_script_handler); + install_keyword_quoted("startup_script", &startup_script_handler); install_keyword("startup_script_timeout", &startup_script_timeout_handler); - install_keyword("shutdown_script", &shutdown_script_handler); + install_keyword_quoted("shutdown_script", &shutdown_script_handler); install_keyword("shutdown_script_timeout", &shutdown_script_timeout_handler); install_keyword("max_auto_priority", &max_auto_priority_handler); install_keyword("min_auto_priority_delay", &min_auto_priority_delay_handler); @@ -2556,16 +2556,16 @@ init_global_keywords(bool global_active) install_keyword("vrrp_rlimit_rtime", &vrrp_rt_rlimit_handler); /* Deprecated 02/02/2020 */ #endif install_keyword("notify_fifo", &global_notify_fifo); - install_keyword("notify_fifo_script", &global_notify_fifo_script); + install_keyword_quoted("notify_fifo_script", &global_notify_fifo_script); #ifdef _WITH_VRRP_ install_keyword("vrrp_notify_fifo", &vrrp_notify_fifo); - install_keyword("vrrp_notify_fifo_script", &vrrp_notify_fifo_script); + install_keyword_quoted("vrrp_notify_fifo_script", &vrrp_notify_fifo_script); install_keyword("vrrp_notify_priority_changes", &vrrp_notify_priority_changes); install_keyword("fifo_write_vrrp_states_on_reload", &fifo_write_vrrp_states_on_reload); #endif #ifdef _WITH_LVS_ install_keyword("lvs_notify_fifo", &lvs_notify_fifo); - install_keyword("lvs_notify_fifo_script", &lvs_notify_fifo_script); + install_keyword_quoted("lvs_notify_fifo_script", &lvs_notify_fifo_script); install_keyword("checker_priority", &checker_prio_handler); install_keyword("checker_no_swap", &checker_no_swap_handler); install_keyword("checker_rt_priority", &checker_rt_priority_handler); diff --git a/keepalived/vrrp/vrrp_parser.c b/keepalived/vrrp/vrrp_parser.c index f9ca6bb6a..cf62e3aec 100644 --- a/keepalived/vrrp/vrrp_parser.c +++ b/keepalived/vrrp/vrrp_parser.c @@ -2111,11 +2111,11 @@ init_vrrp_keywords(bool active) #ifdef _WITH_BFD_ install_keyword("track_bfd", &vrrp_group_track_bfd_handler); #endif - install_keyword("notify_backup", &vrrp_gnotify_backup_handler); - install_keyword("notify_master", &vrrp_gnotify_master_handler); - install_keyword("notify_fault", &vrrp_gnotify_fault_handler); - install_keyword("notify_stop", &vrrp_gnotify_stop_handler); - install_keyword("notify", &vrrp_gnotify_handler); + install_keyword_quoted("notify_backup", &vrrp_gnotify_backup_handler); + install_keyword_quoted("notify_master", &vrrp_gnotify_master_handler); + install_keyword_quoted("notify_fault", &vrrp_gnotify_fault_handler); + install_keyword_quoted("notify_stop", &vrrp_gnotify_stop_handler); + install_keyword_quoted("notify", &vrrp_gnotify_handler); install_keyword("smtp_alert", &vrrp_gsmtp_handler); install_keyword("global_tracking", &vrrp_gglobal_tracking_handler); install_keyword("sync_group_tracking_weight", &vrrp_sg_tracking_weight_handler); @@ -2197,13 +2197,13 @@ init_vrrp_keywords(bool active) install_keyword("nopreempt", &vrrp_nopreempt_handler); install_keyword("preempt_delay", &vrrp_preempt_delay_handler); install_keyword("debug", &vrrp_debug_handler); - install_keyword("notify_backup", &vrrp_notify_backup_handler); - install_keyword("notify_master", &vrrp_notify_master_handler); - install_keyword("notify_fault", &vrrp_notify_fault_handler); - install_keyword("notify_stop", &vrrp_notify_stop_handler); - install_keyword("notify_deleted", &vrrp_notify_deleted_handler); - install_keyword("notify", &vrrp_notify_handler); - install_keyword("notify_master_rx_lower_pri", vrrp_notify_master_rx_lower_pri); + install_keyword_quoted("notify_backup", &vrrp_notify_backup_handler); + install_keyword_quoted("notify_master", &vrrp_notify_master_handler); + install_keyword_quoted("notify_fault", &vrrp_notify_fault_handler); + install_keyword_quoted("notify_stop", &vrrp_notify_stop_handler); + install_keyword_quoted("notify_deleted", &vrrp_notify_deleted_handler); + install_keyword_quoted("notify", &vrrp_notify_handler); + install_keyword_quoted("notify_master_rx_lower_pri", vrrp_notify_master_rx_lower_pri); install_keyword("smtp_alert", &vrrp_smtp_handler); install_keyword("notify_priority_changes", &vrrp_notify_priority_changes_handler); #ifdef _WITH_LVS_ @@ -2234,7 +2234,7 @@ init_vrrp_keywords(bool active) #endif /* Script declarations */ install_keyword_root("vrrp_script", &vrrp_script_handler, active, VPP ¤t_vscr); - install_keyword("script", &vrrp_vscript_script_handler); + install_keyword_quoted("script", &vrrp_vscript_script_handler); install_keyword("interval", &vrrp_vscript_interval_handler); install_keyword("timeout", &vrrp_vscript_timeout_handler); install_keyword("weight", &vrrp_vscript_weight_handler); diff --git a/lib/parser.c b/lib/parser.c index 681647194..1bedb1a7b 100644 --- a/lib/parser.c +++ b/lib/parser.c @@ -79,7 +79,7 @@ * overridden by the global_defs tmp_config_directory option. * * The temporary file contains all the lines of the original configuration file(s) - * stripped of leading and training whitespace and comments, with the following + * stripped of leading and trailing whitespace and comments, with the following * exceptions: * 1. include statements are passed as blank lines. * 2. When an included file is opened, a line starting "# " followed by the file @@ -823,7 +823,7 @@ set_random_seed(unsigned int seed) } static void -keyword_alloc(vector_t *keywords_vec, const char *string, void (*handler) (const vector_t *), bool active) +keyword_alloc(vector_t *keywords_vec, const char *string, void (*handler) (const vector_t *), bool active, bool allow_mismatched_quotes) { keyword_t *keyword; @@ -834,12 +834,13 @@ keyword_alloc(vector_t *keywords_vec, const char *string, void (*handler) (const keyword->handler = handler; keyword->active = active; keyword->ptr = cur_check_ptr; + keyword->allow_mismatched_quotes = allow_mismatched_quotes; vector_set_slot(keywords_vec, keyword); } static void -keyword_alloc_sub(vector_t *keywords_vec, const char *string, void (*handler) (const vector_t *)) +keyword_alloc_sub(vector_t *keywords_vec, const char *string, void (*handler) (const vector_t *), bool allow_mismatched_quotes) { int i = 0; keyword_t *keyword; @@ -860,7 +861,7 @@ keyword_alloc_sub(vector_t *keywords_vec, const char *string, void (*handler) (c keyword->sub = vector_alloc(); /* add new sub keyword */ - keyword_alloc(keyword->sub, string, handler, true); + keyword_alloc(keyword->sub, string, handler, true, allow_mismatched_quotes); } /* Exported helpers */ @@ -889,14 +890,22 @@ install_keyword_root(const char *string, void (*handler) (const vector_t *), boo /* If the root keyword is inactive, the handler will still be called, * but with a NULL strvec */ cur_check_ptr = NULL; - keyword_alloc(keywords, string, handler, active); + keyword_alloc(keywords, string, handler, active, false); cur_check_ptr = ptr; } void install_keyword(const char *string, void (*handler) (const vector_t *)) { - keyword_alloc_sub(keywords, string, handler); + keyword_alloc_sub(keywords, string, handler, false); +} + +void +install_keyword_quoted(const char *string, void (*handler) (const vector_t *)) +{ + /* This is a special instance when the second parameter can be a + * quoted escaped string. */ + keyword_alloc_sub(keywords, string, handler, true); } void @@ -1198,11 +1207,15 @@ alloc_strvec_quoted(const char *src) } vector_t * -alloc_strvec_r(const char *string) +alloc_strvec_r(const char *string, const vector_t *keywords_vec) { const char *cp, *start; size_t str_len; vector_t *strvec; + unsigned i; + bool allow_mismatched_quotes; + keyword_t *keyword_vec; + const char *keyword; if (!string) return NULL; @@ -1222,7 +1235,26 @@ alloc_strvec_r(const char *string) if (*start == '"') { start++; if (!(cp = strchr(start, '"'))) { - report_config_error(CONFIG_UNMATCHED_QUOTE, "Unmatched quote: '%s'", string); + allow_mismatched_quotes = false; + if (vector_size(strvec) > 1 && keywords_vec) { + keyword = strvec_slot(strvec, 0); + + /* Check to see if the second string will be reprocessed */ + for (i = 0; i < vector_size(keywords_vec); i++) { + keyword_vec = vector_slot(keywords_vec, i); + + if (!strcmp(keyword_vec->string, keyword)) { + allow_mismatched_quotes = keyword_vec->allow_mismatched_quotes; + break; + } + } + } + if (!allow_mismatched_quotes +#ifndef _ONE_PROCESS_DEBUG_ + && prog_type != PROG_TYPE_PARENT +#endif + ) + report_config_error(CONFIG_UNMATCHED_QUOTE, "Unmatched quote: '%s'", string); break; } str_len = (size_t)(cp - start); @@ -2951,7 +2983,7 @@ alloc_value_block(void (*alloc_func) (const vector_t *), const vector_t *strvec) while (first_vec || read_line(buf, MAXBUF)) { if (first_vec) vec = first_vec; - else if (!(vec = alloc_strvec(buf))) + else if (!(vec = alloc_strvec(buf, NULL))) continue; if (!first_vec) { @@ -3015,7 +3047,7 @@ process_stream(vector_t *keywords_vec, int need_bob) buf = MALLOC(MAXBUF); while (read_line(buf, MAXBUF)) { - strvec = alloc_strvec(buf); + strvec = alloc_strvec(buf, keywords_vec); if (!strvec) continue; diff --git a/lib/parser.h b/lib/parser.h index 6222c1623..91993dd70 100644 --- a/lib/parser.h +++ b/lib/parser.h @@ -83,6 +83,7 @@ typedef struct _keyword { vector_t *sub; void (*sub_close_handler) (void); bool active; + bool allow_mismatched_quotes; vpp_t ptr; vpp_t sub_close_ptr; } keyword_t; @@ -111,13 +112,13 @@ set_value_r(const vector_t *strvec) } #ifdef _MEM_CHECK_ -#define alloc_strvec(str) (memcheck_log("alloc_strvec", str, (__FILE__), (__func__), (__LINE__)), \ - alloc_strvec_r(str)) +#define alloc_strvec(str, keyw) (memcheck_log("alloc_strvec", str, (__FILE__), (__func__), (__LINE__)), \ + alloc_strvec_r(str, keyw)) #define set_value(str) (memcheck_log("set_value", strvec_slot(str,1), (__FILE__), (__func__), (__LINE__)), \ set_value_r(str)) #else -#define alloc_strvec(str) (alloc_strvec_r(str)) +#define alloc_strvec(str, keyw) (alloc_strvec_r(str, keyw)) #define set_value(str) (set_value_r(str)) #endif @@ -144,9 +145,10 @@ extern vpp_t install_sublevel(vpp_t) WARN_UNUSED_RESULT; extern void install_sublevel_end(vpp_t); extern void install_level_end_handler(void (*handler) (void)); extern void install_keyword(const char *, void (*handler) (const vector_t *)); +extern void install_keyword_quoted(const char *, void (*handler) (const vector_t *)); extern const vector_t *alloc_strvec_quoted_escaped(const char *); extern const vector_t *alloc_strvec_quoted(const char *); -extern vector_t *alloc_strvec_r(const char *); +extern vector_t *alloc_strvec_r(const char *, const vector_t *); extern bool check_conf_file(const char*); extern const vector_t *read_value_block(const vector_t *); extern void alloc_value_block(void (*alloc_func) (const vector_t *), const vector_t *); From 3e842cb5d3a203b55d296864288a647527a2534c Mon Sep 17 00:00:00 2001 From: Quentin Armitage Date: Mon, 21 Oct 2024 22:25:07 +0100 Subject: [PATCH 15/16] configure: Remove -ffile-prefix-map= for repeatible builds Debian has a patch for building keepalived that removed -ffile-prefix-map=... to assist with repeatible build. The commit adds removing -ffile-prefix-map= from the gcc options if repeatible builds are enabled. Signed-off-by: Quentin Armitage --- configure.ac | 1 + 1 file changed, 1 insertion(+) diff --git a/configure.ac b/configure.ac index ff762482d..efbe2f8e4 100644 --- a/configure.ac +++ b/configure.ac @@ -438,6 +438,7 @@ AS_IF([test $? -eq 0], # save the configure arguments # args=`echo $ac_configure_args | $SED -e "s/'//g"` +AS_IF([test .$enable_reproducible_build = .yes], [`echo $ac_configure_args | $SED -e "s/-ffile-prefix-map=[[^ ]]*//g"`] AC_DEFINE_UNQUOTED(KEEPALIVED_CONFIGURE_OPTIONS,"$args", [configure options specified]) AS_IF([test .$enable_lto = .yes], From 07153cabf107984b53282b5fcce4580db6da5931 Mon Sep 17 00:00:00 2001 From: Quentin Armitage Date: Mon, 21 Oct 2024 22:43:32 +0100 Subject: [PATCH 16/16] configure: fix previous commit Commit 3e842cb5 - "configure: Remove -ffile-prefix-map= for repeatible builds" missed a closing ')' in the introduced AS_IF() test. Signed-off-by: Quentin Armitage --- configure.ac | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/configure.ac b/configure.ac index efbe2f8e4..db67fb0c1 100644 --- a/configure.ac +++ b/configure.ac @@ -438,7 +438,7 @@ AS_IF([test $? -eq 0], # save the configure arguments # args=`echo $ac_configure_args | $SED -e "s/'//g"` -AS_IF([test .$enable_reproducible_build = .yes], [`echo $ac_configure_args | $SED -e "s/-ffile-prefix-map=[[^ ]]*//g"`] +AS_IF([test .$enable_reproducible_build = .yes], [`echo $ac_configure_args | $SED -e "s/-ffile-prefix-map=[[^ ]]*//g"`]) AC_DEFINE_UNQUOTED(KEEPALIVED_CONFIGURE_OPTIONS,"$args", [configure options specified]) AS_IF([test .$enable_lto = .yes],