Hello Maria,

Thanks for the reply. It makes sense and I will wait for the team's
refactoring.

In the meantime, I did find a different approach that does not involve
API changes. It marks the channel as disabled if a next hop cannot be
identified. The change is here if it might be interesting:
https://github.com/myzhang1029/bird/commit/8348b5dab4fa0ab0c51248ba3277997d4704c090

Best,
Maiyun

On Thu, Jun 18, 2026 at 05:39:50PM +0200, Maria Matejka wrote:
> 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.  

Attachment: signature.asc
Description: PGP signature

Reply via email to