0
0
mirror of https://gitlab.nic.cz/labs/bird.git synced 2024-12-22 09:41:54 +00:00

BFD: Fix session reconfiguration locking order

The sessions have to be updated asynchronously to avoid
cross-locking between protocols.

Testsuite: cf-ibgp-bfd-switch, cf-ibgp-multi-bfd-auth
Fixes: #139

Thanks to Daniel Suchy <danny@danysek.cz> for reporting:
https://trubka.network.cz/pipermail/bird-users/2024-December/017984.html
This commit is contained in:
Maria Matejka 2024-12-21 19:02:22 +01:00
parent 6779e5da69
commit 8349536278
5 changed files with 98 additions and 120 deletions

View File

@ -18,8 +18,11 @@ struct bfd_options {
u32 min_tx_int;
u32 idle_tx_int;
u8 multiplier;
u8 passive;
u8 passive_set;
PACKED enum bfd_opt_passive {
BFD_OPT_PASSIVE_UNKNOWN = 0,
BFD_OPT_PASSIVE,
BFD_OPT_NOT_PASSIVE,
} passive;
u8 mode;
u8 auth_type; /* Authentication type (BFD_AUTH_*) */
list *passwords; /* Passwords for authentication */

View File

@ -172,17 +172,17 @@ static void bfd_free_iface(struct bfd_iface *ifa);
* BFD sessions
*/
static inline struct bfd_session_config
bfd_merge_options(const struct bfd_iface_config *cf, const struct bfd_options *opts)
static inline struct bfd_options
bfd_merge_options(const struct bfd_options *bottom, const struct bfd_options *top)
{
return (struct bfd_session_config) {
.min_rx_int = opts->min_rx_int ?: cf->min_rx_int,
.min_tx_int = opts->min_tx_int ?: cf->min_tx_int,
.idle_tx_int = opts->idle_tx_int ?: cf->idle_tx_int,
.multiplier = opts->multiplier ?: cf->multiplier,
.passive = opts->passive_set ? opts->passive : cf->passive,
.auth_type = opts->auth_type ?: cf->auth_type,
.passwords = opts->passwords ?: cf->passwords,
return (struct bfd_options) {
.min_rx_int = top->min_rx_int ?: bottom->min_rx_int,
.min_tx_int = top->min_tx_int ?: bottom->min_tx_int,
.idle_tx_int = top->idle_tx_int ?: bottom->idle_tx_int,
.multiplier = top->multiplier ?: bottom->multiplier,
.passive = top->passive ?: bottom->passive,
.auth_type = top->auth_type ?: bottom->auth_type,
.passwords = top->passwords ?: bottom->passwords,
};
}
@ -478,7 +478,7 @@ bfd_add_session(struct bfd_proto *p, ip_addr addr, ip_addr local, struct iface *
HASH_INSERT(p->session_hash_id, HASH_ID, s);
HASH_INSERT(p->session_hash_ip, HASH_IP, s);
s->cf = bfd_merge_options(ifa->cf, opts);
s->cf = bfd_merge_options(&ifa->cf->opts, opts);
/* Initialization of state variables - see RFC 5880 6.8.1 */
s->loc_state = BFD_STATE_DOWN;
@ -561,17 +561,41 @@ bfd_remove_session(struct bfd_proto *p, struct bfd_session *s)
birdloop_leave(p->p.loop);
}
struct bfd_reconfigure_sessions_deferred_call {
struct deferred_call dc;
struct bfd_proto *p;
config_ref old_config;
};
static void
bfd_reconfigure_session(struct bfd_proto *p, struct bfd_session *s)
bfd_reconfigure_sessions(struct deferred_call *dc)
{
if (EMPTY_LIST(s->request_list))
return;
SKIP_BACK_DECLARE(struct bfd_reconfigure_sessions_deferred_call,
brsdc, dc, dc);
ASSERT_DIE(birdloop_inside(p->p.loop));
struct bfd_proto *p = brsdc->p;
birdloop_enter(p->p.loop);
HASH_WALK(p->session_hash_id, next_id, s)
{
if (!EMPTY_LIST(s->request_list))
{
SKIP_BACK_DECLARE(struct bfd_request, req, n, HEAD(s->request_list));
s->cf = bfd_merge_options(s->ifa->cf, &req->opts);
struct bfd_options opts = bfd_merge_options(&s->ifa->cf->opts, &req->opts);
#define CHK(x) (opts.x != s->cf.x) ||
bool reload = MACRO_FOREACH(CHK,
min_rx_int,
min_tx_int,
idle_tx_int,
multiplier,
passive) false; /* terminating the || chain */
#undef CHK
s->cf = opts;
if (reload)
{
u32 tx = (s->loc_state == BFD_STATE_UP) ? s->cf.min_tx_int : s->cf.idle_tx_int;
bfd_session_set_min_tx(s, tx);
bfd_session_set_min_rx(s, s->cf.min_rx_int);
@ -582,6 +606,14 @@ bfd_reconfigure_session(struct bfd_proto *p, struct bfd_session *s)
TRACE(D_EVENTS, "Session to %I reconfigured", s->addr);
}
}
}
HASH_WALK_END;
birdloop_leave(p->p.loop);
/* Now the config is clean */
OBSREF_CLEAR(brsdc->old_config);
}
/*
@ -589,10 +621,12 @@ bfd_reconfigure_session(struct bfd_proto *p, struct bfd_session *s)
*/
static struct bfd_iface_config bfd_default_iface = {
.opts = {
.min_rx_int = BFD_DEFAULT_MIN_RX_INT,
.min_tx_int = BFD_DEFAULT_MIN_TX_INT,
.idle_tx_int = BFD_DEFAULT_IDLE_TX_INT,
.multiplier = BFD_DEFAULT_MULTIPLIER,
},
};
static inline struct bfd_iface_config *
@ -650,24 +684,6 @@ bfd_free_iface(struct bfd_iface *ifa)
mb_free(ifa);
}
static void
bfd_reconfigure_iface(struct bfd_proto *p UNUSED, struct bfd_iface *ifa, struct bfd_config *nc)
{
struct bfd_iface_config *new = bfd_find_iface_config(nc, ifa->iface);
struct bfd_iface_config *old = ifa->cf;
/* Check options that are handled in bfd_reconfigure_session() */
ifa->changed =
(new->min_rx_int != old->min_rx_int) ||
(new->min_tx_int != old->min_tx_int) ||
(new->idle_tx_int != old->idle_tx_int) ||
(new->multiplier != old->multiplier) ||
(new->passive != old->passive);
/* This should be probably changed to not access ifa->cf from the BFD thread */
ifa->cf = new;
}
/*
* BFD requests
@ -900,20 +916,7 @@ bfd_request_session(pool *p, ip_addr addr, ip_addr local,
void
bfd_update_request(struct bfd_request *req, const struct bfd_options *opts)
{
struct bfd_session *s = req->session;
if (!memcmp(opts, &req->opts, sizeof(const struct bfd_options)))
return;
req->opts = *opts;
if (s)
{
struct bfd_proto *p = s->ifa->bfd;
birdloop_enter(p->p.loop);
bfd_reconfigure_session(p, s);
birdloop_leave(p->p.loop);
}
}
static void
@ -1193,21 +1196,22 @@ bfd_reconfigure(struct proto *P, struct proto_config *c)
(new->zero_udp6_checksum_rx != old->zero_udp6_checksum_rx))
return 0;
birdloop_mask_wakeups(p->p.loop);
WALK_LIST(ifa, p->iface_list)
bfd_reconfigure_iface(p, ifa, new);
HASH_WALK(p->session_hash_id, next_id, s)
{
if (s->ifa->changed)
bfd_reconfigure_session(p, s);
}
HASH_WALK_END;
ifa->cf = bfd_find_iface_config(new, ifa->iface);
bfd_reconfigure_neighbors(p, new);
birdloop_unmask_wakeups(p->p.loop);
/* Sessions get reconfigured after all the config is applied */
struct bfd_reconfigure_sessions_deferred_call brsdc = {
.dc.hook = bfd_reconfigure_sessions,
.p = p,
};
SKIP_BACK_DECLARE(struct bfd_reconfigure_sessions_deferred_call,
brsdcp, dc, defer_call(&brsdc.dc, sizeof brsdc));
/* We need to keep the old config alive until all the sessions get
* reconfigured */
OBSREF_SET(brsdcp->old_config, P->cf->global);
return 1;
}

View File

@ -54,24 +54,7 @@ struct bfd_config
struct bfd_iface_config
{
struct iface_patt i;
u32 min_rx_int;
u32 min_tx_int;
u32 idle_tx_int;
u8 multiplier;
u8 passive;
u8 auth_type; /* Authentication type (BFD_AUTH_*) */
list *passwords; /* Passwords for authentication */
};
struct bfd_session_config
{
u32 min_rx_int;
u32 min_tx_int;
u32 idle_tx_int;
u8 multiplier;
u8 passive;
u8 auth_type; /* Authentication type (BFD_AUTH_*) */
list *passwords; /* Passwords for authentication */
struct bfd_options opts;
};
struct bfd_neighbor
@ -146,7 +129,7 @@ struct bfd_session
u32 loc_id; /* Local session ID (local discriminator) */
u32 rem_id; /* Remote session ID (remote discriminator) */
struct bfd_session_config cf; /* Static configuration parameters */
struct bfd_options cf; /* Static configuration parameters */
u32 des_min_tx_int; /* Desired min rx interval, local option */
u32 des_min_tx_new; /* Used for des_min_tx_int change */

View File

@ -86,44 +86,37 @@ bfd_iface_start:
add_tail(&BFD_CFG->patt_list, NODE this_ipatt);
init_list(&this_ipatt->ipn_list);
BFD_IFACE->min_rx_int = BFD_DEFAULT_MIN_RX_INT;
BFD_IFACE->min_tx_int = BFD_DEFAULT_MIN_TX_INT;
BFD_IFACE->idle_tx_int = BFD_DEFAULT_IDLE_TX_INT;
BFD_IFACE->multiplier = BFD_DEFAULT_MULTIPLIER;
this_bfd_opts = &BFD_IFACE->opts;
this_bfd_opts->min_rx_int = BFD_DEFAULT_MIN_RX_INT;
this_bfd_opts->min_tx_int = BFD_DEFAULT_MIN_TX_INT;
this_bfd_opts->idle_tx_int = BFD_DEFAULT_IDLE_TX_INT;
this_bfd_opts->multiplier = BFD_DEFAULT_MULTIPLIER;
reset_passwords();
};
bfd_iface_finish:
{
BFD_IFACE->passwords = get_passwords();
this_bfd_opts->passwords = get_passwords();
if (!BFD_IFACE->auth_type != !BFD_IFACE->passwords)
if (!this_bfd_opts->auth_type != !this_bfd_opts->passwords)
cf_warn("Authentication and password options should be used together");
if (BFD_IFACE->passwords)
if (this_bfd_opts->passwords)
{
struct password_item *pass;
WALK_LIST(pass, *BFD_IFACE->passwords)
WALK_LIST(pass, *this_bfd_opts->passwords)
{
if (pass->alg)
cf_error("Password algorithm option not available in BFD protocol");
pass->alg = bfd_auth_type_to_hash_alg[BFD_IFACE->auth_type];
pass->alg = bfd_auth_type_to_hash_alg[this_bfd_opts->auth_type];
}
}
};
bfd_iface_item:
INTERVAL expr_us { BFD_IFACE->min_rx_int = BFD_IFACE->min_tx_int = $2; }
| MIN RX INTERVAL expr_us { BFD_IFACE->min_rx_int = $4; }
| MIN TX INTERVAL expr_us { BFD_IFACE->min_tx_int = $4; }
| IDLE TX INTERVAL expr_us { BFD_IFACE->idle_tx_int = $4; }
| MULTIPLIER expr { BFD_IFACE->multiplier = $2; }
| PASSIVE bool { BFD_IFACE->passive = $2; }
| AUTHENTICATION bfd_auth_type { BFD_IFACE->auth_type = $2; }
| password_list {}
;
this_bfd_opts = NULL;
};
bfd_auth_type:
NONE { $$ = BFD_AUTH_NONE; }
@ -134,14 +127,9 @@ bfd_auth_type:
| METICULOUS KEYED SHA1 { $$ = BFD_AUTH_METICULOUS_KEYED_SHA1; }
;
bfd_iface_opts:
/* empty */
| bfd_iface_opts bfd_iface_item ';'
;
bfd_iface_opt_list:
/* empty */
| '{' bfd_iface_opts '}'
| '{' bfd_items '}'
;
bfd_iface:
@ -194,7 +182,7 @@ bfd_item:
| MIN TX INTERVAL expr_us { this_bfd_opts->min_tx_int = $4; }
| IDLE TX INTERVAL expr_us { this_bfd_opts->idle_tx_int = $4; }
| MULTIPLIER expr { this_bfd_opts->multiplier = $2; }
| PASSIVE bool { this_bfd_opts->passive = $2; this_bfd_opts->passive_set = 1; }
| PASSIVE bool { this_bfd_opts->passive = $2 ? BFD_OPT_PASSIVE : BFD_OPT_NOT_PASSIVE; }
| GRACEFUL { this_bfd_opts->mode = BGP_BFD_GRACEFUL; }
| AUTHENTICATION bfd_auth_type { this_bfd_opts->auth_type = $2; }
| password_list {}

View File

@ -109,7 +109,7 @@ const u8 bfd_auth_type_to_hash_alg[] = {
static void
bfd_fill_authentication(struct bfd_proto *p, struct bfd_session *s, struct bfd_ctl_packet *pkt)
{
struct bfd_session_config *cf = &s->cf;
struct bfd_options *cf = &s->cf;
struct password_item *pass = password_find(cf->passwords, 0);
uint meticulous = 0;
@ -179,7 +179,7 @@ bfd_fill_authentication(struct bfd_proto *p, struct bfd_session *s, struct bfd_c
static int
bfd_check_authentication(struct bfd_proto *p, struct bfd_session *s, struct bfd_ctl_packet *pkt)
{
struct bfd_session_config *cf = &s->cf;
struct bfd_options *cf = &s->cf;
const char *err_dsc = NULL;
uint err_val = 0;
uint auth_type = 0;