Howdy! Thank you for your patch. This is a real issue which we'd like to handle differently because your approach needs internal API changes.
We'll instead refactor the BGP channel startup code to not start the channel at all; we need this anyway to allow future development. Thank you again and have a nice day! Maria Internal reference: #447 On Thu, Jun 18, 2026 at 10:18:32AM -0400, Zhang Maiyun via Bird-users wrote: > Hello All, > > I was hoping to get some feedback on this change. For example, bgp's > channel start function gives up when it cannot find a proper next hop, > but the channel is still treated as successfully initialized. > This then leads to lots of errors in `bgp_decode_nlri` about bad NH. > > I think it might be a better idea to leave the channel in the DOWN state > when this happens, and this patch allows the channel start function to > reject the DOWN->START transition. > > Best, > Maiyun > > On Thu, Jun 18, 2026 at 10:12:32AM -0400, [email protected] wrote: > > From: Zhang Maiyun <[email protected]> > > > > Signed-off-by: Zhang Maiyun <[email protected]> > > --- > > nest/proto.c | 27 ++++++++++++++++++++------- > > proto/bgp/bgp.c | 2 +- > > 2 files changed, 21 insertions(+), 8 deletions(-) > > > > diff --git a/nest/proto.c b/nest/proto.c > > index 4472fe6f..2edbe142 100644 > > --- a/nest/proto.c > > +++ b/nest/proto.c > > @@ -1086,7 +1086,7 @@ channel_setup_in_table(struct channel *c) > > } > > > > > > -static void > > +static int > > channel_do_start(struct channel *c) > > { > > c->proto->active_channels++; > > @@ -1094,9 +1094,12 @@ channel_do_start(struct channel *c) > > if ((c->in_keep & RIK_PREFILTER) == RIK_PREFILTER) > > channel_setup_in_table(c); > > > > - CALL(c->class->start, c); > > + if (c->class->start) > > + if (c->class->start(c) != 0) > > + return -1; > > > > channel_start_import(c); > > + return 0; > > } > > > > static void > > @@ -1176,7 +1179,11 @@ channel_set_state(struct channel *c, uint state) > > ASSERT(cs == CS_DOWN || cs == CS_PAUSE); > > > > if (cs == CS_DOWN) > > - channel_do_start(c); > > + if (channel_do_start(c) != 0) { > > + // Reject the channel start and return to down state > > + c->channel_state = CS_DOWN; > > + c->last_state_change = current_time(); > > + } > > > > break; > > > > @@ -1184,12 +1191,18 @@ channel_set_state(struct channel *c, uint state) > > ASSERT(cs == CS_DOWN || cs == CS_START || cs == CS_PAUSE); > > > > if (cs == CS_DOWN) > > - channel_do_start(c); > > + if (channel_do_start(c) != 0) { > > + // Reject the channel start and return to down state > > + c->channel_state = CS_DOWN; > > + c->last_state_change = current_time(); > > + } > > > > - if (!c->gr_wait && c->proto->rt_notify) > > - channel_start_export(c); > > + if (c->channel_state != CS_DOWN) { > > + if (!c->gr_wait && c->proto->rt_notify) > > + channel_start_export(c); > > > > - channel_do_up(c); > > + channel_do_up(c); > > + } > > break; > > > > case CS_PAUSE: > > diff --git a/proto/bgp/bgp.c b/proto/bgp/bgp.c > > index 8d04a05d..1923a80d 100644 > > --- a/proto/bgp/bgp.c > > +++ b/proto/bgp/bgp.c > > @@ -3257,7 +3257,7 @@ bgp_channel_start(struct channel *C) > > if (ipa_zero(c->next_hop_addr)) > > { > > log(L_WARN "%s: Missing next hop address", p->p.name); > > - return 0; > > + return -1; > > } > > > > /* Set link-local address for IPv6 single-hop BGP */ > > -- > > 2.54.0 > > -- Maria Matejka (she/her) | BIRD Team Leader | CZ.NIC, z.s.p.o.
