On 2018/08/02 14:32, Claudio Jeker wrote:
> Currently if anyone uses the example bgpd filter rules all neighbors will
> do a full softreconfiguration even when no rule have been changed. This is
> because the skip logic was wrongly implemented and so rules like 'pass to
> ebgp' will result in non equality of the ruleset even though it is fine.
>
> By introducing a rde_filter_skip_rule() function the skip logic goes into
> a single place and so less errors should happen.
>
> OK?
It reads well, and is much clearer split off to a function this way.
Testing while watching bgpd -dv output it detects filter changes when
I expect them and does the right thing. OK sthen@
> --
> :wq Claudio
>
> PS: this may speed up config reloads in some cases a fair amount
>
> Index: rde_filter.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/bgpd/rde_filter.c,v
> retrieving revision 1.97
> diff -u -p -r1.97 rde_filter.c
> --- rde_filter.c 22 Jul 2018 16:59:08 -0000 1.97
> +++ rde_filter.c 2 Aug 2018 12:22:10 -0000
> @@ -549,6 +549,37 @@ rde_prefix_match(struct filter_prefix *f
> return (0); /* should not be reached */
> }
>
> +/* return true when the rule f can never match for this peer */
> +static int
> +rde_filter_skip_rule(struct rde_peer *peer, struct filter_rule *f)
> +{
> + /* if any of the two is unset then rule can't be skipped */
> + if (peer == NULL || f == NULL)
> + return (0);
> +
> + if (f->peer.groupid != 0 &&
> + f->peer.groupid != peer->conf.groupid)
> + return (1);
> +
> + if (f->peer.peerid != 0 &&
> + f->peer.peerid != peer->conf.id)
> + return (1);
> +
> + if (f->peer.remote_as != 0 &&
> + f->peer.remote_as != peer->conf.remote_as)
> + return (1);
> +
> + if (f->peer.ebgp != 0 &&
> + f->peer.ebgp != peer->conf.ebgp)
> + return (1);
> +
> + if (f->peer.ibgp != 0 &&
> + f->peer.ibgp != !peer->conf.ebgp)
> + return (1);
> +
> + return (0);
> +}
> +
> int
> rde_filter_equal(struct filter_head *a, struct filter_head *b,
> struct rde_peer *peer, struct prefixset_head *psh)
> @@ -561,43 +592,12 @@ rde_filter_equal(struct filter_head *a,
>
> while (fa != NULL || fb != NULL) {
> /* skip all rules with wrong peer */
> - if (peer != NULL && fa != NULL && fa->peer.groupid != 0 &&
> - fa->peer.groupid != peer->conf.groupid) {
> - fa = TAILQ_NEXT(fa, entry);
> - continue;
> - }
> - if (peer != NULL && fa != NULL && fa->peer.peerid != 0 &&
> - fa->peer.peerid != peer->conf.id) {
> + if (rde_filter_skip_rule(peer, fa)) {
> fa = TAILQ_NEXT(fa, entry);
> continue;
> }
> -
> - if (peer != NULL && fb != NULL && fb->peer.groupid != 0 &&
> - fb->peer.groupid != peer->conf.groupid) {
> + if (rde_filter_skip_rule(peer, fb)) {
> fb = TAILQ_NEXT(fb, entry);
> - continue;
> - }
> - if (peer != NULL && fb != NULL && fb->peer.peerid != 0 &&
> - fb->peer.peerid != peer->conf.id) {
> - fb = TAILQ_NEXT(fb, entry);
> - continue;
> - }
> -
> - if (peer != NULL && fa != NULL && fa->peer.remote_as != 0 &&
> - fa->peer.remote_as != peer->conf.remote_as) {
> - fa = TAILQ_NEXT(fa, entry);
> - continue;
> - }
> -
> - if (peer != NULL && fa != NULL && fa->peer.ebgp != 0 &&
> - fa->peer.ebgp != peer->conf.ebgp) {
> - fa = TAILQ_NEXT(fa, entry);
> - continue;
> - }
> -
> - if (peer != NULL && fa != NULL && fa->peer.ibgp != 0 &&
> - fa->peer.ibgp != !peer->conf.ebgp) {
> - fa = TAILQ_NEXT(fa, entry);
> continue;
> }
>
>
