Re: handle updates via Adj-RIB-Out
On Fri, Mar 02, 2018 at 04:55:23PM +0100, Claudio Jeker wrote:
> On Wed, Feb 07, 2018 at 05:52:09AM +0100, Claudio Jeker wrote:
> > This diff changes the way bgpd does updates. Instead of having its own
> > special update queue/tree it uses a regular RIB (Adj-RIB-Out) to store all
> > updates to be sent. Stuff that has been sent is linked to the prefixes
> > queue. On the peer there are also queues for updates and withdraws.
> > The whole update code becomes a lot simpler but also results in the bulk
> > of the diff. Other changes include the bgpctl show rib handling (we can
> > just walk the Adj-RIB-Out now). Last but not least the EOR records are
> > also now a magic rde_aspath (flag F_ATTR_EOR) which is added to the update
> > queue.
> >
> > This diff is still very large and the changes are intrusive so reviews and
> > testing is very welcome.
>
> No news on this? Anyone?
"Rebased" diff to -current (thanks job@ for the cluestick)
--
:wq Claudio
Index: rde.c
===
RCS file: /cvs/src/usr.sbin/bgpd/rde.c,v
retrieving revision 1.379
diff -u -p -r1.379 rde.c
--- rde.c 10 Feb 2018 05:54:31 - 1.379
+++ rde.c 6 Mar 2018 09:41:34 -
@@ -81,8 +81,6 @@ void rde_dump_rib_as(struct prefix *,
int);
voidrde_dump_filter(struct prefix *,
struct ctl_show_rib_request *);
-voidrde_dump_filterout(struct rde_peer *, struct prefix *,
-struct ctl_show_rib_request *);
voidrde_dump_upcall(struct rib_entry *, void *);
voidrde_dump_prefix_upcall(struct rib_entry *, void *);
voidrde_dump_ctx_new(struct ctl_show_rib_request *, pid_t,
@@ -2317,71 +2315,33 @@ rde_dump_rib_as(struct prefix *p, struct
}
void
-rde_dump_filterout(struct rde_peer *peer, struct prefix *p,
-struct ctl_show_rib_request *req)
+rde_dump_filter(struct prefix *p, struct ctl_show_rib_request *req)
{
- struct bgpd_addr addr;
- struct rde_aspath *asp, *fasp;
- enum filter_actions a;
+ struct rde_aspath *asp;
- if (up_test_update(peer, p) != 1)
+ if (req->peerid && req->peerid != prefix_peer(p)->conf.id)
return;
+ if (p->flags & F_PREFIX_USE_PEER)
+ return; /* pending withdraw, skip */
- pt_getaddr(p->re->prefix, &addr);
asp = prefix_aspath(p);
- a = rde_filter(out_rules, &fasp, peer, asp, &addr,
- p->re->prefix->prefixlen, asp->peer);
- if (fasp)
- fasp->peer = asp->peer;
- else
- fasp = asp;
-
- if (a == ACTION_ALLOW)
- rde_dump_rib_as(p, fasp, req->pid, req->flags);
-
- if (fasp != asp)
- path_put(fasp);
-}
-
-void
-rde_dump_filter(struct prefix *p, struct ctl_show_rib_request *req)
-{
- struct rde_peer *peer;
- struct rde_aspath *asp;
-
- if (req->flags & F_CTL_ADJ_IN ||
- !(req->flags & (F_CTL_ADJ_IN|F_CTL_ADJ_OUT))) {
- asp = prefix_aspath(p);
- if (req->peerid && req->peerid != asp->peer->conf.id)
- return;
- if (req->type == IMSG_CTL_SHOW_RIB_AS &&
- !aspath_match(asp->aspath->data, asp->aspath->len,
- &req->as, req->as.as))
- return;
- if (req->type == IMSG_CTL_SHOW_RIB_COMMUNITY &&
- !community_match(asp, req->community.as,
- req->community.type))
- return;
- if (req->type == IMSG_CTL_SHOW_RIB_EXTCOMMUNITY &&
- !community_ext_match(asp, &req->extcommunity, 0))
- return;
- if (req->type == IMSG_CTL_SHOW_RIB_LARGECOMMUNITY &&
- !community_large_match(asp, req->large_community.as,
- req->large_community.ld1, req->large_community.ld2))
- return;
- if ((req->flags & F_CTL_ACTIVE) && p->re->active != p)
- return;
- rde_dump_rib_as(p, asp, req->pid, req->flags);
- } else if (req->flags & F_CTL_ADJ_OUT) {
- if (p->re->active != p)
- /* only consider active prefix */
- return;
- if (req->peerid) {
- if ((peer = peer_get(req->peerid)) != NULL)
- rde_dump_filterout(peer, p, req);
- return;
- }
- }
+ if (req->type == IMSG_CTL_SHOW_RIB_AS &&
+ !aspath_match(asp->aspath->data, asp->aspath->len,
+ &req->as, req->as.as))
+ return;
+ if (req->type == IMSG_CTL_SHOW_RIB_COMMUNITY &&
+ !community_match(asp, req->community.as, req->community.type))
+ return;
+ if (req->type == IMSG_
Re: handle updates via Adj-RIB-Out
Claudio,
How best to test this change proposal? Should this maybe be tested on
one of the yycix route servers?
I'll let it run on my home router, if that doesn't cause issues in a
week or so; we can consider rs2.yycix.ca
Kind regards,
Job
On Fri, Mar 02, 2018 at 04:55:23PM +0100, Claudio Jeker wrote:
> On Wed, Feb 07, 2018 at 05:52:09AM +0100, Claudio Jeker wrote:
> > This diff changes the way bgpd does updates. Instead of having its own
> > special update queue/tree it uses a regular RIB (Adj-RIB-Out) to store all
> > updates to be sent. Stuff that has been sent is linked to the prefixes
> > queue. On the peer there are also queues for updates and withdraws.
> > The whole update code becomes a lot simpler but also results in the bulk
> > of the diff. Other changes include the bgpctl show rib handling (we can
> > just walk the Adj-RIB-Out now). Last but not least the EOR records are
> > also now a magic rde_aspath (flag F_ATTR_EOR) which is added to the update
> > queue.
> >
> > This diff is still very large and the changes are intrusive so reviews and
> > testing is very welcome.
>
> No news on this? Anyone?
>
> --
> :wq Claudio
>
> Index: rde.c
> ===
> RCS file: /cvs/src/usr.sbin/bgpd/rde.c,v
> retrieving revision 1.377
> diff -u -p -r1.377 rde.c
> --- rde.c 7 Feb 2018 00:02:02 - 1.377
> +++ rde.c 7 Feb 2018 00:02:18 -
> @@ -80,8 +80,6 @@ void rde_dump_rib_as(struct prefix *,
>int);
> void rde_dump_filter(struct prefix *,
>struct ctl_show_rib_request *);
> -void rde_dump_filterout(struct rde_peer *, struct prefix *,
> - struct ctl_show_rib_request *);
> void rde_dump_upcall(struct rib_entry *, void *);
> void rde_dump_prefix_upcall(struct rib_entry *, void *);
> void rde_dump_ctx_new(struct ctl_show_rib_request *, pid_t,
> @@ -2262,71 +2260,33 @@ rde_dump_rib_as(struct prefix *p, struct
> }
>
> void
> -rde_dump_filterout(struct rde_peer *peer, struct prefix *p,
> -struct ctl_show_rib_request *req)
> +rde_dump_filter(struct prefix *p, struct ctl_show_rib_request *req)
> {
> - struct bgpd_addr addr;
> - struct rde_aspath *asp, *fasp;
> - enum filter_actions a;
> + struct rde_aspath *asp;
>
> - if (up_test_update(peer, p) != 1)
> + if (req->peerid && req->peerid != prefix_peer(p)->conf.id)
> return;
> + if (p->flags & F_PREFIX_USE_PEER)
> + return; /* pending withdraw, skip */
>
> - pt_getaddr(p->re->prefix, &addr);
> asp = prefix_aspath(p);
> - a = rde_filter(out_rules, &fasp, peer, asp, &addr,
> - p->re->prefix->prefixlen, asp->peer);
> - if (fasp)
> - fasp->peer = asp->peer;
> - else
> - fasp = asp;
> -
> - if (a == ACTION_ALLOW)
> - rde_dump_rib_as(p, fasp, req->pid, req->flags);
> -
> - if (fasp != asp)
> - path_put(fasp);
> -}
> -
> -void
> -rde_dump_filter(struct prefix *p, struct ctl_show_rib_request *req)
> -{
> - struct rde_peer *peer;
> - struct rde_aspath *asp;
> -
> - if (req->flags & F_CTL_ADJ_IN ||
> - !(req->flags & (F_CTL_ADJ_IN|F_CTL_ADJ_OUT))) {
> - asp = prefix_aspath(p);
> - if (req->peerid && req->peerid != asp->peer->conf.id)
> - return;
> - if (req->type == IMSG_CTL_SHOW_RIB_AS &&
> - !aspath_match(asp->aspath->data, asp->aspath->len,
> - &req->as, req->as.as))
> - return;
> - if (req->type == IMSG_CTL_SHOW_RIB_COMMUNITY &&
> - !community_match(asp, req->community.as,
> - req->community.type))
> - return;
> - if (req->type == IMSG_CTL_SHOW_RIB_EXTCOMMUNITY &&
> - !community_ext_match(asp, &req->extcommunity, 0))
> - return;
> - if (req->type == IMSG_CTL_SHOW_RIB_LARGECOMMUNITY &&
> - !community_large_match(asp, req->large_community.as,
> - req->large_community.ld1, req->large_community.ld2))
> - return;
> - if ((req->flags & F_CTL_ACTIVE) && p->re->active != p)
> - return;
> - rde_dump_rib_as(p, asp, req->pid, req->flags);
> - } else if (req->flags & F_CTL_ADJ_OUT) {
> - if (p->re->active != p)
> - /* only consider active prefix */
> - return;
> - if (req->peerid) {
> - if ((peer = peer_get(req->peerid)) != NULL)
> - rde_dump_filterout(peer, p, req);
> - return;
> - }
> - }
> + if (req->type == IMSG_CTL_SHOW_RIB_AS &&
> + !aspath_match(asp->aspath->data, asp->aspath->len
Re: handle updates via Adj-RIB-Out
On Wed, Feb 07, 2018 at 05:52:09AM +0100, Claudio Jeker wrote:
> This diff changes the way bgpd does updates. Instead of having its own
> special update queue/tree it uses a regular RIB (Adj-RIB-Out) to store all
> updates to be sent. Stuff that has been sent is linked to the prefixes
> queue. On the peer there are also queues for updates and withdraws.
> The whole update code becomes a lot simpler but also results in the bulk
> of the diff. Other changes include the bgpctl show rib handling (we can
> just walk the Adj-RIB-Out now). Last but not least the EOR records are
> also now a magic rde_aspath (flag F_ATTR_EOR) which is added to the update
> queue.
>
> This diff is still very large and the changes are intrusive so reviews and
> testing is very welcome.
No news on this? Anyone?
--
:wq Claudio
Index: rde.c
===
RCS file: /cvs/src/usr.sbin/bgpd/rde.c,v
retrieving revision 1.377
diff -u -p -r1.377 rde.c
--- rde.c 7 Feb 2018 00:02:02 - 1.377
+++ rde.c 7 Feb 2018 00:02:18 -
@@ -80,8 +80,6 @@ void rde_dump_rib_as(struct prefix *,
int);
voidrde_dump_filter(struct prefix *,
struct ctl_show_rib_request *);
-voidrde_dump_filterout(struct rde_peer *, struct prefix *,
-struct ctl_show_rib_request *);
voidrde_dump_upcall(struct rib_entry *, void *);
voidrde_dump_prefix_upcall(struct rib_entry *, void *);
voidrde_dump_ctx_new(struct ctl_show_rib_request *, pid_t,
@@ -2262,71 +2260,33 @@ rde_dump_rib_as(struct prefix *p, struct
}
void
-rde_dump_filterout(struct rde_peer *peer, struct prefix *p,
-struct ctl_show_rib_request *req)
+rde_dump_filter(struct prefix *p, struct ctl_show_rib_request *req)
{
- struct bgpd_addr addr;
- struct rde_aspath *asp, *fasp;
- enum filter_actions a;
+ struct rde_aspath *asp;
- if (up_test_update(peer, p) != 1)
+ if (req->peerid && req->peerid != prefix_peer(p)->conf.id)
return;
+ if (p->flags & F_PREFIX_USE_PEER)
+ return; /* pending withdraw, skip */
- pt_getaddr(p->re->prefix, &addr);
asp = prefix_aspath(p);
- a = rde_filter(out_rules, &fasp, peer, asp, &addr,
- p->re->prefix->prefixlen, asp->peer);
- if (fasp)
- fasp->peer = asp->peer;
- else
- fasp = asp;
-
- if (a == ACTION_ALLOW)
- rde_dump_rib_as(p, fasp, req->pid, req->flags);
-
- if (fasp != asp)
- path_put(fasp);
-}
-
-void
-rde_dump_filter(struct prefix *p, struct ctl_show_rib_request *req)
-{
- struct rde_peer *peer;
- struct rde_aspath *asp;
-
- if (req->flags & F_CTL_ADJ_IN ||
- !(req->flags & (F_CTL_ADJ_IN|F_CTL_ADJ_OUT))) {
- asp = prefix_aspath(p);
- if (req->peerid && req->peerid != asp->peer->conf.id)
- return;
- if (req->type == IMSG_CTL_SHOW_RIB_AS &&
- !aspath_match(asp->aspath->data, asp->aspath->len,
- &req->as, req->as.as))
- return;
- if (req->type == IMSG_CTL_SHOW_RIB_COMMUNITY &&
- !community_match(asp, req->community.as,
- req->community.type))
- return;
- if (req->type == IMSG_CTL_SHOW_RIB_EXTCOMMUNITY &&
- !community_ext_match(asp, &req->extcommunity, 0))
- return;
- if (req->type == IMSG_CTL_SHOW_RIB_LARGECOMMUNITY &&
- !community_large_match(asp, req->large_community.as,
- req->large_community.ld1, req->large_community.ld2))
- return;
- if ((req->flags & F_CTL_ACTIVE) && p->re->active != p)
- return;
- rde_dump_rib_as(p, asp, req->pid, req->flags);
- } else if (req->flags & F_CTL_ADJ_OUT) {
- if (p->re->active != p)
- /* only consider active prefix */
- return;
- if (req->peerid) {
- if ((peer = peer_get(req->peerid)) != NULL)
- rde_dump_filterout(peer, p, req);
- return;
- }
- }
+ if (req->type == IMSG_CTL_SHOW_RIB_AS &&
+ !aspath_match(asp->aspath->data, asp->aspath->len,
+ &req->as, req->as.as))
+ return;
+ if (req->type == IMSG_CTL_SHOW_RIB_COMMUNITY &&
+ !community_match(asp, req->community.as, req->community.type))
+ return;
+ if (req->type == IMSG_CTL_SHOW_RIB_EXTCOMMUNITY &&
+ !community_ext_match(asp, &req->extcommunity, 0))
+ return;
+ if (req->type == IMSG_CTL_SHOW
handle updates via Adj-RIB-Out
This diff changes the way bgpd does updates. Instead of having its own
special update queue/tree it uses a regular RIB (Adj-RIB-Out) to store all
updates to be sent. Stuff that has been sent is linked to the prefixes
queue. On the peer there are also queues for updates and withdraws.
The whole update code becomes a lot simpler but also results in the bulk
of the diff. Other changes include the bgpctl show rib handling (we can
just walk the Adj-RIB-Out now). Last but not least the EOR records are
also now a magic rde_aspath (flag F_ATTR_EOR) which is added to the update
queue.
This diff is still very large and the changes are intrusive so reviews and
testing is very welcome.
--
:wq Claudio
Index: rde.c
===
RCS file: /cvs/src/usr.sbin/bgpd/rde.c,v
retrieving revision 1.377
diff -u -p -r1.377 rde.c
--- rde.c 7 Feb 2018 00:02:02 - 1.377
+++ rde.c 7 Feb 2018 00:02:18 -
@@ -80,8 +80,6 @@ void rde_dump_rib_as(struct prefix *,
int);
voidrde_dump_filter(struct prefix *,
struct ctl_show_rib_request *);
-voidrde_dump_filterout(struct rde_peer *, struct prefix *,
-struct ctl_show_rib_request *);
voidrde_dump_upcall(struct rib_entry *, void *);
voidrde_dump_prefix_upcall(struct rib_entry *, void *);
voidrde_dump_ctx_new(struct ctl_show_rib_request *, pid_t,
@@ -2262,71 +2260,33 @@ rde_dump_rib_as(struct prefix *p, struct
}
void
-rde_dump_filterout(struct rde_peer *peer, struct prefix *p,
-struct ctl_show_rib_request *req)
+rde_dump_filter(struct prefix *p, struct ctl_show_rib_request *req)
{
- struct bgpd_addr addr;
- struct rde_aspath *asp, *fasp;
- enum filter_actions a;
+ struct rde_aspath *asp;
- if (up_test_update(peer, p) != 1)
+ if (req->peerid && req->peerid != prefix_peer(p)->conf.id)
return;
+ if (p->flags & F_PREFIX_USE_PEER)
+ return; /* pending withdraw, skip */
- pt_getaddr(p->re->prefix, &addr);
asp = prefix_aspath(p);
- a = rde_filter(out_rules, &fasp, peer, asp, &addr,
- p->re->prefix->prefixlen, asp->peer);
- if (fasp)
- fasp->peer = asp->peer;
- else
- fasp = asp;
-
- if (a == ACTION_ALLOW)
- rde_dump_rib_as(p, fasp, req->pid, req->flags);
-
- if (fasp != asp)
- path_put(fasp);
-}
-
-void
-rde_dump_filter(struct prefix *p, struct ctl_show_rib_request *req)
-{
- struct rde_peer *peer;
- struct rde_aspath *asp;
-
- if (req->flags & F_CTL_ADJ_IN ||
- !(req->flags & (F_CTL_ADJ_IN|F_CTL_ADJ_OUT))) {
- asp = prefix_aspath(p);
- if (req->peerid && req->peerid != asp->peer->conf.id)
- return;
- if (req->type == IMSG_CTL_SHOW_RIB_AS &&
- !aspath_match(asp->aspath->data, asp->aspath->len,
- &req->as, req->as.as))
- return;
- if (req->type == IMSG_CTL_SHOW_RIB_COMMUNITY &&
- !community_match(asp, req->community.as,
- req->community.type))
- return;
- if (req->type == IMSG_CTL_SHOW_RIB_EXTCOMMUNITY &&
- !community_ext_match(asp, &req->extcommunity, 0))
- return;
- if (req->type == IMSG_CTL_SHOW_RIB_LARGECOMMUNITY &&
- !community_large_match(asp, req->large_community.as,
- req->large_community.ld1, req->large_community.ld2))
- return;
- if ((req->flags & F_CTL_ACTIVE) && p->re->active != p)
- return;
- rde_dump_rib_as(p, asp, req->pid, req->flags);
- } else if (req->flags & F_CTL_ADJ_OUT) {
- if (p->re->active != p)
- /* only consider active prefix */
- return;
- if (req->peerid) {
- if ((peer = peer_get(req->peerid)) != NULL)
- rde_dump_filterout(peer, p, req);
- return;
- }
- }
+ if (req->type == IMSG_CTL_SHOW_RIB_AS &&
+ !aspath_match(asp->aspath->data, asp->aspath->len,
+ &req->as, req->as.as))
+ return;
+ if (req->type == IMSG_CTL_SHOW_RIB_COMMUNITY &&
+ !community_match(asp, req->community.as, req->community.type))
+ return;
+ if (req->type == IMSG_CTL_SHOW_RIB_EXTCOMMUNITY &&
+ !community_ext_match(asp, &req->extcommunity, 0))
+ return;
+ if (req->type == IMSG_CTL_SHOW_RIB_LARGECOMMUNITY &&
+ !community_large_match(asp, req->large_community.as,
+ req->large_co
