From ce239ce000963d8eb6c78471f696f1b127660c6c Mon Sep 17 00:00:00 2001 From: Philippe Guibert Date: Thu, 29 Nov 2018 15:03:03 +0100 Subject: [PATCH 1/9] bgpd: remove useless fields in bgp_pbr_entry_main main bgp structure that contains fs information is being cleaned. some fields are removed. Signed-off-by: Philippe Guibert --- bgpd/bgp_pbr.h | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/bgpd/bgp_pbr.h b/bgpd/bgp_pbr.h index 45c3c9ea13..f59aeea8b2 100644 --- a/bgpd/bgp_pbr.h +++ b/bgpd/bgp_pbr.h @@ -88,11 +88,6 @@ struct bgp_pbr_entry_action { /* BGP Policy Route structure */ struct bgp_pbr_entry_main { uint8_t type; - uint16_t instance; - - uint32_t flags; - - uint8_t message; /* * This is an enum but we are going to treat it as a uint8_t @@ -137,14 +132,6 @@ struct bgp_pbr_entry_main { uint16_t action_num; struct bgp_pbr_entry_action actions[ACTIONS_MAX_NUM]; - uint8_t distance; - - uint32_t metric; - - route_tag_t tag; - - uint32_t mtu; - vrf_id_t vrf_id; }; From 5fa779c9688fa01be8f5f8702de600afc1ba931a Mon Sep 17 00:00:00 2001 From: Philippe Guibert Date: Thu, 29 Nov 2018 15:04:52 +0100 Subject: [PATCH 2/9] bgpd: upon bgp fs study, determine if iprule can be used instead of using ipset based mechanism to forward packets, there are cases where it is possible to use ip rule based mechanisms (without ipset). Here, this applies to simple fs rules with only 'from any' or 'to any'. Signed-off-by: Philippe Guibert --- bgpd/bgp_flowspec_util.c | 19 +++++++++++++++++-- bgpd/bgp_pbr.c | 5 +++++ bgpd/bgp_pbr.h | 4 ++++ 3 files changed, 26 insertions(+), 2 deletions(-) diff --git a/bgpd/bgp_flowspec_util.c b/bgpd/bgp_flowspec_util.c index cd5bec6267..b9a0d81cc5 100644 --- a/bgpd/bgp_flowspec_util.c +++ b/bgpd/bgp_flowspec_util.c @@ -456,8 +456,7 @@ int bgp_flowspec_match_rules_fill(uint8_t *nlri_content, int len, */ if (prefix->family == AF_INET && prefix->u.prefix4.s_addr == 0) - memset(prefix, 0, - sizeof(struct prefix)); + bpem->match_bitmask_iprule |= bitmask; else bpem->match_bitmask |= bitmask; } @@ -580,6 +579,22 @@ int bgp_flowspec_match_rules_fill(uint8_t *nlri_content, int len, __func__, type); } } + if (bpem->match_packet_length_num || bpem->match_fragment_num || + bpem->match_tcpflags_num || bpem->match_dscp_num || + bpem->match_packet_length_num || bpem->match_icmp_code_num || + bpem->match_icmp_type_num || bpem->match_port_num || + bpem->match_src_port_num || bpem->match_dst_port_num || + bpem->match_protocol_num || bpem->match_bitmask) + bpem->type = BGP_PBR_IPSET; + else if ((bpem->match_bitmask_iprule & PREFIX_SRC_PRESENT) || + (bpem->match_bitmask_iprule & PREFIX_DST_PRESENT)) + /* the extracted policy rule may not need an + * iptables/ipset filtering. check this may not be + * a standard ip rule : permit any to any ( eg) + */ + bpem->type = BGP_PBR_IPRULE; + else + bpem->type = BGP_PBR_UNDEFINED; return error; } diff --git a/bgpd/bgp_pbr.c b/bgpd/bgp_pbr.c index f002154701..03c2d9d601 100644 --- a/bgpd/bgp_pbr.c +++ b/bgpd/bgp_pbr.c @@ -448,6 +448,11 @@ static int bgp_pbr_validate_policy_route(struct bgp_pbr_entry_main *api) { bool enumerate_icmp = false; + if (api->type == BGP_PBR_UNDEFINED) { + if (BGP_DEBUG(pbr, PBR)) + zlog_debug("BGP: pbr entry undefined. cancel."); + return 0; + } /* because bgp pbr entry may contain unsupported * combinations, a message will be displayed here if * not supported. diff --git a/bgpd/bgp_pbr.h b/bgpd/bgp_pbr.h index f59aeea8b2..eebfdf3715 100644 --- a/bgpd/bgp_pbr.h +++ b/bgpd/bgp_pbr.h @@ -87,6 +87,9 @@ struct bgp_pbr_entry_action { /* BGP Policy Route structure */ struct bgp_pbr_entry_main { +#define BGP_PBR_UNDEFINED 0 +#define BGP_PBR_IPSET 1 +#define BGP_PBR_IPRULE 2 uint8_t type; /* @@ -98,6 +101,7 @@ struct bgp_pbr_entry_main { #define PREFIX_SRC_PRESENT (1 << 0) #define PREFIX_DST_PRESENT (1 << 1) + uint8_t match_bitmask_iprule; uint8_t match_bitmask; uint8_t match_src_port_num; From 27e376d4e102ec0c4d00ca083bb992e6eeee0b37 Mon Sep 17 00:00:00 2001 From: Philippe Guibert Date: Thu, 29 Nov 2018 15:12:03 +0100 Subject: [PATCH 3/9] bgpd: an hash list of pbr iprule is created that iprule list stands for the list of fs entries that are created, based only on ip rule from/to rule. Signed-off-by: Philippe Guibert --- bgpd/bgp_pbr.c | 77 ++++++++++++++++++++++++++++++++++++++++++++++++++ bgpd/bgp_pbr.h | 14 +++++++++ bgpd/bgpd.h | 2 ++ 3 files changed, 93 insertions(+) diff --git a/bgpd/bgp_pbr.c b/bgpd/bgp_pbr.c index 03c2d9d601..96c1c6d408 100644 --- a/bgpd/bgp_pbr.c +++ b/bgpd/bgp_pbr.c @@ -38,6 +38,7 @@ DEFINE_MTYPE_STATIC(BGPD, PBR_MATCH_ENTRY, "PBR match entry") DEFINE_MTYPE_STATIC(BGPD, PBR_MATCH, "PBR match") DEFINE_MTYPE_STATIC(BGPD, PBR_ACTION, "PBR action") +DEFINE_MTYPE_STATIC(BGPD, PBR_RULE, "PBR rule") DEFINE_MTYPE_STATIC(BGPD, PBR, "BGP PBR Context") DEFINE_MTYPE_STATIC(BGPD, PBR_VALMASK, "BGP PBR Val Mask Value") @@ -837,6 +838,34 @@ static void *bgp_pbr_match_alloc_intern(void *arg) return new; } +static void bgp_pbr_rule_free(void *arg) +{ + struct bgp_pbr_rule *bpr; + + bpr = (struct bgp_pbr_rule *)arg; + + /* delete iprule */ + if (bpr->installed) { + bgp_send_pbr_rule_action(bpr->action, bpr, false); + bpr->installed = false; + bpr->action->refcnt--; + bpr->action = NULL; + } + XFREE(MTYPE_PBR_RULE, bpr); +} + +static void *bgp_pbr_rule_alloc_intern(void *arg) +{ + struct bgp_pbr_rule *bpr, *new; + + bpr = (struct bgp_pbr_rule *)arg; + + new = XCALLOC(MTYPE_PBR_RULE, sizeof(*new)); + memcpy(new, bpr, sizeof(*bpr)); + + return new; +} + static void bgp_pbr_action_free(void *arg) { struct bgp_pbr_action *bpa; @@ -937,6 +966,44 @@ bool bgp_pbr_match_hash_equal(const void *arg1, const void *arg2) return true; } +uint32_t bgp_pbr_rule_hash_key(void *arg) +{ + struct bgp_pbr_rule *pbr = (struct bgp_pbr_rule *)arg; + uint32_t key; + + key = prefix_hash_key(&pbr->src); + key = jhash_1word(pbr->vrf_id, key); + key = jhash_1word(pbr->flags, key); + return jhash_1word(prefix_hash_key(&pbr->dst), key); +} + +bool bgp_pbr_rule_hash_equal(const void *arg1, const void *arg2) +{ + const struct bgp_pbr_rule *r1, *r2; + + r1 = (const struct bgp_pbr_rule *)arg1; + r2 = (const struct bgp_pbr_rule *)arg2; + + if (r1->vrf_id != r2->vrf_id) + return false; + + if (r1->flags != r2->flags) + return false; + + if (r1->action != r2->action) + return false; + + if ((r1->flags & MATCH_IP_SRC_SET) && + !prefix_same(&r1->src, &r2->src)) + return false; + + if ((r1->flags & MATCH_IP_DST_SET) && + !prefix_same(&r1->dst, &r2->dst)) + return false; + + return true; +} + uint32_t bgp_pbr_match_entry_hash_key(void *arg) { struct bgp_pbr_match_entry *pbme; @@ -1095,6 +1162,11 @@ void bgp_pbr_cleanup(struct bgp *bgp) hash_free(bgp->pbr_match_hash); bgp->pbr_match_hash = NULL; } + if (bgp->pbr_rule_hash) { + hash_clean(bgp->pbr_rule_hash, bgp_pbr_rule_free); + hash_free(bgp->pbr_rule_hash); + bgp->pbr_rule_hash = NULL; + } if (bgp->pbr_action_hash) { hash_clean(bgp->pbr_action_hash, bgp_pbr_action_free); hash_free(bgp->pbr_action_hash); @@ -1118,6 +1190,11 @@ void bgp_pbr_init(struct bgp *bgp) bgp_pbr_action_hash_equal, "Match Hash Entry"); + bgp->pbr_rule_hash = + hash_create_size(8, bgp_pbr_rule_hash_key, + bgp_pbr_rule_hash_equal, + "Match Rule"); + bgp->bgp_pbr_cfg = XCALLOC(MTYPE_PBR, sizeof(struct bgp_pbr_config)); bgp->bgp_pbr_cfg->pbr_interface_any_ipv4 = true; } diff --git a/bgpd/bgp_pbr.h b/bgpd/bgp_pbr.h index eebfdf3715..7deb547ee7 100644 --- a/bgpd/bgp_pbr.h +++ b/bgpd/bgp_pbr.h @@ -158,6 +158,17 @@ struct bgp_pbr_config { extern struct bgp_pbr_config *bgp_pbr_cfg; +struct bgp_pbr_rule { + uint32_t flags; + struct prefix src; + struct prefix dst; + struct bgp_pbr_action *action; + vrf_id_t vrf_id; + uint32_t unique; + bool installed; + bool install_in_progress; +}; + struct bgp_pbr_match { char ipset_name[ZEBRA_IPSET_NAME_SIZE]; @@ -257,6 +268,9 @@ extern struct bgp_pbr_match *bgp_pbr_match_iptable_lookup(vrf_id_t vrf_id, extern void bgp_pbr_cleanup(struct bgp *bgp); extern void bgp_pbr_init(struct bgp *bgp); +extern uint32_t bgp_pbr_rule_hash_key(void *arg); +extern bool bgp_pbr_rule_hash_equal(const void *arg1, + const void *arg2); extern uint32_t bgp_pbr_action_hash_key(void *arg); extern bool bgp_pbr_action_hash_equal(const void *arg1, const void *arg2); diff --git a/bgpd/bgpd.h b/bgpd/bgpd.h index 484fc105e8..11af889cf5 100644 --- a/bgpd/bgpd.h +++ b/bgpd/bgpd.h @@ -414,6 +414,7 @@ struct bgp { * * pbr_action a <----- pbr_match i <--- pbr_match_entry 1..n * <----- pbr_match j <--- pbr_match_entry 1..m + * <----- pbr_rule k * * - here in BGP structure, the list of match and actions will * stand for the list of ipset sets, and table_ids in the kernel @@ -423,6 +424,7 @@ struct bgp { * contained in match, that lists the whole set of entries */ struct hash *pbr_match_hash; + struct hash *pbr_rule_hash; struct hash *pbr_action_hash; /* timer to re-evaluate neighbor default-originate route-maps */ From a35a794a23fd346f6078cdd5ba84fa113d5924a8 Mon Sep 17 00:00:00 2001 From: Philippe Guibert Date: Thu, 29 Nov 2018 15:08:36 +0100 Subject: [PATCH 4/9] bgpd: the fs entry is valid for any rule only, by using ipruleset cmd Before, it was not possible to create any rules. Now, it is possible to have flowspec rules relying only on ip rule command. The check is done here. Signed-off-by: Philippe Guibert --- bgpd/bgp_pbr.c | 40 ++++++++++++++++++++++++++++++++++++---- 1 file changed, 36 insertions(+), 4 deletions(-) diff --git a/bgpd/bgp_pbr.c b/bgpd/bgp_pbr.c index 96c1c6d408..88991b6760 100644 --- a/bgpd/bgp_pbr.c +++ b/bgpd/bgp_pbr.c @@ -618,13 +618,45 @@ static int bgp_pbr_validate_policy_route(struct bgp_pbr_entry_main *api) " too complex. ignoring."); return 0; } - if (!(api->match_bitmask & PREFIX_SRC_PRESENT) && - !(api->match_bitmask & PREFIX_DST_PRESENT)) { + /* iprule only supports redirect IP */ + if (api->type == BGP_PBR_IPRULE) { + int i; + + for (i = 0; i < api->action_num; i++) { + if (api->actions[i].action == ACTION_TRAFFICRATE && + api->actions[i].u.r.rate == 0) { + if (BGP_DEBUG(pbr, PBR)) { + bgp_pbr_print_policy_route(api); + zlog_debug("BGP: iprule match actions" + " drop not supported"); + } + return 0; + } + if (api->actions[i].action == ACTION_MARKING) { + if (BGP_DEBUG(pbr, PBR)) { + bgp_pbr_print_policy_route(api); + zlog_warn("PBR: iprule set DSCP %u" + " not supported", + api->actions[i].u.marking_dscp); + } + } + if (api->actions[i].action == ACTION_REDIRECT) { + if (BGP_DEBUG(pbr, PBR)) { + bgp_pbr_print_policy_route(api); + zlog_warn("PBR: iprule redirect VRF %u" + " not supported", + api->actions[i].u.redirect_vrf); + } + } + } + + } else if (!(api->match_bitmask & PREFIX_SRC_PRESENT) && + !(api->match_bitmask & PREFIX_DST_PRESENT)) { if (BGP_DEBUG(pbr, PBR)) { bgp_pbr_print_policy_route(api); zlog_debug("BGP: match actions without src" - " or dst address can not operate." - " ignoring."); + " or dst address can not operate." + " ignoring."); } return 0; } From 6cfe5d1533b2c8a5daeeb34432153f2f17e5c2e7 Mon Sep 17 00:00:00 2001 From: Philippe Guibert Date: Thu, 29 Nov 2018 15:14:41 +0100 Subject: [PATCH 5/9] bgpd: ip rule zebra layer adapted to handle both cases now, ip rule can be created from two differnt ways; however a single zebra API has been defined. so make it consistent by adding a parameter to the bgp zebra layer. the function will handle the rest. Signed-off-by: Philippe Guibert --- bgpd/bgp_pbr.c | 6 ++-- bgpd/bgp_zebra.c | 80 +++++++++++++++++++++++++++++++++++------------- bgpd/bgp_zebra.h | 4 ++- 3 files changed, 64 insertions(+), 26 deletions(-) diff --git a/bgpd/bgp_pbr.c b/bgpd/bgp_pbr.c index 88991b6760..66c20b1aa5 100644 --- a/bgpd/bgp_pbr.c +++ b/bgpd/bgp_pbr.c @@ -906,7 +906,7 @@ static void bgp_pbr_action_free(void *arg) if (bpa->refcnt == 0) { if (bpa->installed && bpa->table_id != 0) { - bgp_send_pbr_rule_action(bpa, false); + bgp_send_pbr_rule_action(bpa, NULL, false); bgp_zebra_announce_default(bpa->bgp, &(bpa->nh), AFI_IP, bpa->table_id, @@ -1414,7 +1414,7 @@ static void bgp_pbr_flush_entry(struct bgp *bgp, struct bgp_pbr_action *bpa, } if (bpa->refcnt == 0) { if (bpa->installed && bpa->table_id != 0) { - bgp_send_pbr_rule_action(bpa, false); + bgp_send_pbr_rule_action(bpa, NULL, false); bgp_zebra_announce_default(bpa->bgp, &(bpa->nh), AFI_IP, bpa->table_id, @@ -2020,7 +2020,7 @@ static void bgp_pbr_policyroute_add_to_zebra_unit(struct bgp *bgp, */ /* ip rule add */ if (!bpa->installed && !bpa->install_in_progress) { - bgp_send_pbr_rule_action(bpa, true); + bgp_send_pbr_rule_action(bpa, NULL, true); bgp_zebra_announce_default(bgp, nh, AFI_IP, bpa->table_id, true); } diff --git a/bgpd/bgp_zebra.c b/bgpd/bgp_zebra.c index 3c4b219466..620b10c25c 100644 --- a/bgpd/bgp_zebra.c +++ b/bgpd/bgp_zebra.c @@ -36,6 +36,7 @@ #include "filter.h" #include "mpls.h" #include "vxlan.h" +#include "pbr.h" #include "bgpd/bgpd.h" #include "bgpd/bgp_route.h" @@ -2242,31 +2243,52 @@ static int iptable_notify_owner(int command, struct zclient *zclient, return 0; } +/* this function is used to forge ip rule, + * - either for iptable/ipset using fwmark id + * - or for sample ip rule command + */ static void bgp_encode_pbr_rule_action(struct stream *s, - struct bgp_pbr_action *pbra) + struct bgp_pbr_action *pbra, + struct bgp_pbr_rule *pbr) { - struct prefix any; + struct prefix pfx; stream_putl(s, 0); /* seqno unused */ stream_putl(s, 0); /* ruleno unused */ - stream_putl(s, pbra->unique); - - memset(&any, 0, sizeof(any)); - any.family = AF_INET; - stream_putc(s, any.family); - stream_putc(s, any.prefixlen); - stream_put(s, &any.u.prefix, prefix_blen(&any)); + if (pbr) + stream_putl(s, pbr->unique); + else + stream_putl(s, pbra->unique); + if (pbr && pbr->flags & MATCH_IP_SRC_SET) + memcpy(&pfx, &(pbr->src), sizeof(struct prefix)); + else { + memset(&pfx, 0, sizeof(pfx)); + pfx.family = AF_INET; + } + stream_putc(s, pfx.family); + stream_putc(s, pfx.prefixlen); + stream_put(s, &pfx.u.prefix, prefix_blen(&pfx)); stream_putw(s, 0); /* src port */ - stream_putc(s, any.family); - stream_putc(s, any.prefixlen); - stream_put(s, &any.u.prefix, prefix_blen(&any)); + if (pbr && pbr->flags & MATCH_IP_DST_SET) + memcpy(&pfx, &(pbr->dst), sizeof(struct prefix)); + else { + memset(&pfx, 0, sizeof(pfx)); + pfx.family = AF_INET; + } + stream_putc(s, pfx.family); + stream_putc(s, pfx.prefixlen); + stream_put(s, &pfx.u.prefix, prefix_blen(&pfx)); stream_putw(s, 0); /* dst port */ - stream_putl(s, pbra->fwmark); /* fwmark */ + /* if pbr present, fwmark is not used */ + if (pbr) + stream_putl(s, 0); + else + stream_putl(s, pbra->fwmark); /* fwmark */ stream_putl(s, pbra->table_id); @@ -2672,16 +2694,26 @@ int bgp_zebra_num_connects(void) return zclient_num_connects; } -void bgp_send_pbr_rule_action(struct bgp_pbr_action *pbra, bool install) +void bgp_send_pbr_rule_action(struct bgp_pbr_action *pbra, + struct bgp_pbr_rule *pbr, + bool install) { struct stream *s; - if (pbra->install_in_progress) + if (pbra->install_in_progress && !pbr) return; - if (BGP_DEBUG(zebra, ZEBRA)) - zlog_debug("%s: table %d fwmark %d %d", - __PRETTY_FUNCTION__, - pbra->table_id, pbra->fwmark, install); + if (pbr && pbr->install_in_progress) + return; + if (BGP_DEBUG(zebra, ZEBRA)) { + if (pbr) + zlog_debug("%s: table %d (ip rule) %d", + __PRETTY_FUNCTION__, + pbra->table_id, install); + else + zlog_debug("%s: table %d fwmark %d %d", + __PRETTY_FUNCTION__, + pbra->table_id, pbra->fwmark, install); + } s = zclient->obuf; stream_reset(s); @@ -2690,11 +2722,15 @@ void bgp_send_pbr_rule_action(struct bgp_pbr_action *pbra, bool install) VRF_DEFAULT); stream_putl(s, 1); /* send one pbr action */ - bgp_encode_pbr_rule_action(s, pbra); + bgp_encode_pbr_rule_action(s, pbra, pbr); stream_putw_at(s, 0, stream_get_endp(s)); - if (!zclient_send_message(zclient) && install) - pbra->install_in_progress = true; + if (!zclient_send_message(zclient) && install) { + if (!pbr) + pbra->install_in_progress = true; + else + pbr->install_in_progress = true; + } } void bgp_send_pbr_ipset_match(struct bgp_pbr_match *pbrim, bool install) diff --git a/bgpd/bgp_zebra.h b/bgpd/bgp_zebra.h index e7b7d683af..c6520c43e4 100644 --- a/bgpd/bgp_zebra.h +++ b/bgpd/bgp_zebra.h @@ -84,9 +84,11 @@ extern bool bgp_zebra_nexthop_set(union sockunion *, union sockunion *, struct bgp_pbr_action; struct bgp_pbr_match; +struct bgp_pbr_rule; struct bgp_pbr_match_entry; extern void bgp_send_pbr_rule_action(struct bgp_pbr_action *pbra, - bool install); + struct bgp_pbr_rule *pbr, + bool install); extern void bgp_send_pbr_ipset_match(struct bgp_pbr_match *pbrim, bool install); extern void bgp_send_pbr_ipset_entry_match(struct bgp_pbr_match_entry *pbrime, From 9350f1dfd0013b22809228380bdc7523e6bf0d61 Mon Sep 17 00:00:00 2001 From: Philippe Guibert Date: Thu, 29 Nov 2018 15:17:36 +0100 Subject: [PATCH 6/9] bgpd: conversion from fs to pbr: support for ip rule from/to adding/suppressing flowspec to pbr is supported. the add and the remove code is being added. now,bgp supports the hash list of ip rule list. The removal of bgp ip rule is done via search. The search uses the action field. the reason is that when a pbr rule is added, to replace an old one, the old one is kept until the new one is installed, so as to avoid traffic to be cut. This is why at one moment, one can have two same iprules with different actions. And this is why the algorithm covers this case. Signed-off-by: Philippe Guibert --- bgpd/bgp_pbr.c | 163 ++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 161 insertions(+), 2 deletions(-) diff --git a/bgpd/bgp_pbr.c b/bgpd/bgp_pbr.c index 66c20b1aa5..941bee98b3 100644 --- a/bgpd/bgp_pbr.c +++ b/bgpd/bgp_pbr.c @@ -201,9 +201,11 @@ struct bgp_pbr_val_mask { * so that BGP can create pbr instructions to ZEBRA */ struct bgp_pbr_filter { + uint8_t type; vrf_id_t vrf_id; struct prefix *src; struct prefix *dst; + uint8_t bitmask_iprule; uint8_t protocol; struct bgp_pbr_range_port *pkt_len; struct bgp_pbr_range_port *src_port; @@ -1367,6 +1369,32 @@ void bgp_pbr_print_policy_route(struct bgp_pbr_entry_main *api) zlog_info("%s", return_string); } +static void bgp_pbr_flush_iprule(struct bgp *bgp, struct bgp_pbr_action *bpa, + struct bgp_pbr_rule *bpr) +{ + /* if bpr is null, do nothing + */ + if (bpr == NULL) + return; + if (bpr->installed) { + bgp_send_pbr_rule_action(bpa, bpr, false); + bpr->installed = false; + bpr->action->refcnt--; + bpr->action = NULL; + } + hash_release(bgp->pbr_rule_hash, bpr); + if (bpa->refcnt == 0) { + if (bpa->installed && bpa->table_id != 0) { + bgp_send_pbr_rule_action(bpa, NULL, false); + bgp_zebra_announce_default(bpa->bgp, &(bpa->nh), + AFI_IP, + bpa->table_id, + false); + bpa->installed = false; + } + } +} + static void bgp_pbr_flush_entry(struct bgp *bgp, struct bgp_pbr_action *bpa, struct bgp_pbr_match *bpm, struct bgp_pbr_match_entry *bpme) @@ -1429,6 +1457,50 @@ struct bgp_pbr_match_entry_remain { struct bgp_pbr_match_entry *bpme_found; }; +struct bgp_pbr_rule_remain { + struct bgp_pbr_rule *bpr_to_match; + struct bgp_pbr_rule *bpr_found; +}; + +static int bgp_pbr_get_same_rule(struct hash_backet *backet, void *arg) +{ + struct bgp_pbr_rule *r1 = (struct bgp_pbr_rule *)backet->data; + struct bgp_pbr_rule_remain *ctxt = + (struct bgp_pbr_rule_remain *)arg; + struct bgp_pbr_rule *r2; + + r2 = ctxt->bpr_to_match; + + if (r1->vrf_id != r2->vrf_id) + return HASHWALK_CONTINUE; + + if (r1->flags != r2->flags) + return HASHWALK_CONTINUE; + + if ((r1->flags & MATCH_IP_SRC_SET) && + !prefix_same(&r1->src, &r2->src)) + return HASHWALK_CONTINUE; + + if ((r1->flags & MATCH_IP_DST_SET) && + !prefix_same(&r1->dst, &r2->dst)) + return HASHWALK_CONTINUE; + + /* this function is used for two cases: + * - remove an entry upon withdraw request + * (case r2->action is null) + * - replace an old iprule with different action + * (case r2->action is != null) + * the old one is removed after the new one + * this is to avoid disruption in traffic + */ + if (r2->action == NULL || + r1->action != r2->action) { + ctxt->bpr_found = r1; + return HASHWALK_ABORT; + } + return HASHWALK_CONTINUE; +} + static int bgp_pbr_get_remaining_entry(struct hash_backet *backet, void *arg) { struct bgp_pbr_match *bpm = (struct bgp_pbr_match *)backet->data; @@ -1466,12 +1538,15 @@ static void bgp_pbr_policyroute_remove_from_zebra_unit( { struct bgp_pbr_match temp; struct bgp_pbr_match_entry temp2; + struct bgp_pbr_rule pbr_rule; + struct bgp_pbr_rule *bpr; struct bgp_pbr_match *bpm; struct bgp_pbr_match_entry *bpme; struct bgp_pbr_match_entry_remain bpmer; struct bgp_pbr_range_port *src_port; struct bgp_pbr_range_port *dst_port; struct bgp_pbr_range_port *pkt_len; + struct bgp_pbr_rule_remain bprr; if (!bpf) return; @@ -1488,6 +1563,37 @@ static void bgp_pbr_policyroute_remove_from_zebra_unit( */ memset(&temp2, 0, sizeof(temp2)); memset(&temp, 0, sizeof(temp)); + + if (bpf->type == BGP_PBR_IPRULE) { + memset(&pbr_rule, 0, sizeof(pbr_rule)); + pbr_rule.vrf_id = bpf->vrf_id; + if (bpf->src) { + prefix_copy(&pbr_rule.src, bpf->src); + pbr_rule.flags |= MATCH_IP_SRC_SET; + } + if (bpf->dst) { + prefix_copy(&pbr_rule.dst, bpf->dst); + pbr_rule.flags |= MATCH_IP_DST_SET; + } + bpr = &pbr_rule; + /* A previous entry may already exist + * flush previous entry if necessary + */ + bprr.bpr_to_match = bpr; + bprr.bpr_found = NULL; + hash_walk(bgp->pbr_rule_hash, bgp_pbr_get_same_rule, &bprr); + if (bprr.bpr_found) { + static struct bgp_pbr_rule *local_bpr; + static struct bgp_pbr_action *local_bpa; + + local_bpr = bprr.bpr_found; + local_bpa = local_bpr->action; + bgp_pbr_flush_iprule(bgp, local_bpa, + local_bpr); + } + return; + } + if (bpf->src) { temp.flags |= MATCH_IP_SRC_SET; prefix_copy(&temp2.src, bpf->src); @@ -1846,9 +1952,12 @@ static void bgp_pbr_policyroute_add_to_zebra_unit(struct bgp *bgp, struct bgp_pbr_action temp3; struct bgp_pbr_action *bpa = NULL; struct bgp_pbr_match_entry_remain bpmer; + struct bgp_pbr_rule_remain bprr; struct bgp_pbr_range_port *src_port; struct bgp_pbr_range_port *dst_port; struct bgp_pbr_range_port *pkt_len; + struct bgp_pbr_rule pbr_rule; + struct bgp_pbr_rule *bpr; bool bpme_found = false; if (!bpf) @@ -1885,7 +1994,51 @@ static void bgp_pbr_policyroute_add_to_zebra_unit(struct bgp *bgp, /* 0 value is forbidden */ bpa->install_in_progress = false; } + if (bpf->type == BGP_PBR_IPRULE) { + memset(&pbr_rule, 0, sizeof(pbr_rule)); + pbr_rule.vrf_id = bpf->vrf_id; + if (bpf->src) { + pbr_rule.flags |= MATCH_IP_SRC_SET; + prefix_copy(&pbr_rule.src, bpf->src); + } + if (bpf->dst) { + pbr_rule.flags |= MATCH_IP_DST_SET; + prefix_copy(&pbr_rule.dst, bpf->dst); + } + pbr_rule.action = bpa; + bpr = hash_get(bgp->pbr_rule_hash, &pbr_rule, + bgp_pbr_rule_alloc_intern); + if (bpr && bpr->unique == 0) { + bpr->unique = ++bgp_pbr_action_counter_unique; + bpr->installed = false; + bpr->install_in_progress = false; + } + if (!bpa->installed && !bpa->install_in_progress) { + bgp_send_pbr_rule_action(bpa, NULL, true); + bgp_zebra_announce_default(bgp, nh, + AFI_IP, bpa->table_id, true); + } + /* ip rule add */ + if (bpr && !bpr->installed) + bgp_send_pbr_rule_action(bpa, bpr, true); + /* A previous entry may already exist + * flush previous entry if necessary + */ + bprr.bpr_to_match = bpr; + bprr.bpr_found = NULL; + hash_walk(bgp->pbr_rule_hash, bgp_pbr_get_same_rule, &bprr); + if (bprr.bpr_found) { + static struct bgp_pbr_rule *local_bpr; + static struct bgp_pbr_action *local_bpa; + + local_bpr = bprr.bpr_found; + local_bpa = local_bpr->action; + bgp_pbr_flush_iprule(bgp, local_bpa, + local_bpr); + } + return; + } /* then look for bpm */ memset(&temp, 0, sizeof(temp)); temp.vrf_id = bpf->vrf_id; @@ -2162,10 +2315,16 @@ static void bgp_pbr_handle_entry(struct bgp *bgp, struct bgp_path_info *path, memset(&nh, 0, sizeof(struct nexthop)); memset(&bpf, 0, sizeof(struct bgp_pbr_filter)); memset(&bpof, 0, sizeof(struct bgp_pbr_or_filter)); - if (api->match_bitmask & PREFIX_SRC_PRESENT) + if (api->match_bitmask & PREFIX_SRC_PRESENT || + (api->type == BGP_PBR_IPRULE && + api->match_bitmask_iprule & PREFIX_SRC_PRESENT)) src = &api->src_prefix; - if (api->match_bitmask & PREFIX_DST_PRESENT) + if (api->match_bitmask & PREFIX_DST_PRESENT || + (api->type == BGP_PBR_IPRULE && + api->match_bitmask_iprule & PREFIX_DST_PRESENT)) dst = &api->dst_prefix; + if (api->type == BGP_PBR_IPRULE) + bpf.type = api->type; memset(&nh, 0, sizeof(struct nexthop)); nh.vrf_id = VRF_UNKNOWN; if (api->match_protocol_num) From ffee150ec42cce9ba6dd66d451965849ede0bd79 Mon Sep 17 00:00:00 2001 From: Philippe Guibert Date: Thu, 29 Nov 2018 14:35:41 +0100 Subject: [PATCH 7/9] bgpd: notify callback when ip rule from/to rule has been configured because ip rule creation is used to not only handle traffic marked by fwmark; but also for conveying traffic with from/to rules, a check of the creation must be done in the linked list of ip rules. Signed-off-by: Philippe Guibert --- bgpd/bgp_pbr.c | 33 +++++++++++++++++++++++++++++++++ bgpd/bgp_pbr.h | 3 +++ bgpd/bgp_zebra.c | 32 ++++++++++++++++++++++++-------- 3 files changed, 60 insertions(+), 8 deletions(-) diff --git a/bgpd/bgp_pbr.c b/bgpd/bgp_pbr.c index 941bee98b3..890fb64313 100644 --- a/bgpd/bgp_pbr.c +++ b/bgpd/bgp_pbr.c @@ -67,6 +67,25 @@ struct bgp_pbr_action_unique { struct bgp_pbr_action *bpa_found; }; +struct bgp_pbr_rule_unique { + uint32_t unique; + struct bgp_pbr_rule *bpr_found; +}; + +static int bgp_pbr_rule_walkcb(struct hash_backet *backet, void *arg) +{ + struct bgp_pbr_rule *bpr = (struct bgp_pbr_rule *)backet->data; + struct bgp_pbr_rule_unique *bpru = (struct bgp_pbr_rule_unique *) + arg; + uint32_t unique = bpru->unique; + + if (bpr->unique == unique) { + bpru->bpr_found = bpr; + return HASHWALK_ABORT; + } + return HASHWALK_CONTINUE; +} + static int bgp_pbr_action_walkcb(struct hash_backet *backet, void *arg) { struct bgp_pbr_action *bpa = (struct bgp_pbr_action *)backet->data; @@ -1123,6 +1142,20 @@ bool bgp_pbr_action_hash_equal(const void *arg1, const void *arg2) return true; } +struct bgp_pbr_rule *bgp_pbr_rule_lookup(vrf_id_t vrf_id, + uint32_t unique) +{ + struct bgp *bgp = bgp_lookup_by_vrf_id(vrf_id); + struct bgp_pbr_rule_unique bpru; + + if (!bgp || unique == 0) + return NULL; + bpru.unique = unique; + bpru.bpr_found = NULL; + hash_walk(bgp->pbr_rule_hash, bgp_pbr_rule_walkcb, &bpru); + return bpru.bpr_found; +} + struct bgp_pbr_action *bgp_pbr_action_rule_lookup(vrf_id_t vrf_id, uint32_t unique) { diff --git a/bgpd/bgp_pbr.h b/bgpd/bgp_pbr.h index 7deb547ee7..c3db10cbe7 100644 --- a/bgpd/bgp_pbr.h +++ b/bgpd/bgp_pbr.h @@ -253,6 +253,9 @@ struct bgp_pbr_action { struct bgp *bgp; }; +extern struct bgp_pbr_rule *bgp_pbr_rule_lookup(vrf_id_t vrf_id, + uint32_t unique); + extern struct bgp_pbr_action *bgp_pbr_action_rule_lookup(vrf_id_t vrf_id, uint32_t unique); diff --git a/bgpd/bgp_zebra.c b/bgpd/bgp_zebra.c index 620b10c25c..9212c36dd5 100644 --- a/bgpd/bgp_zebra.c +++ b/bgpd/bgp_zebra.c @@ -2052,6 +2052,7 @@ static int rule_notify_owner(int command, struct zclient *zclient, uint32_t seqno, priority, unique; enum zapi_rule_notify_owner note; struct bgp_pbr_action *bgp_pbra; + struct bgp_pbr_rule *bgp_pbr = NULL; ifindex_t ifi; if (!zapi_rule_notify_decode(zclient->ibuf, &seqno, &priority, &unique, @@ -2060,10 +2061,14 @@ static int rule_notify_owner(int command, struct zclient *zclient, bgp_pbra = bgp_pbr_action_rule_lookup(vrf_id, unique); if (!bgp_pbra) { - if (BGP_DEBUG(zebra, ZEBRA)) - zlog_debug("%s: Fail to look BGP rule (%u)", - __PRETTY_FUNCTION__, unique); - return 0; + /* look in bgp pbr rule */ + bgp_pbr = bgp_pbr_rule_lookup(vrf_id, unique); + if (!bgp_pbr && note != ZAPI_RULE_REMOVED) { + if (BGP_DEBUG(zebra, ZEBRA)) + zlog_debug("%s: Fail to look BGP rule (%u)", + __PRETTY_FUNCTION__, unique); + return 0; + } } switch (note) { @@ -2071,12 +2076,23 @@ static int rule_notify_owner(int command, struct zclient *zclient, if (BGP_DEBUG(zebra, ZEBRA)) zlog_debug("%s: Received RULE_FAIL_INSTALL", __PRETTY_FUNCTION__); - bgp_pbra->installed = false; - bgp_pbra->install_in_progress = false; + if (bgp_pbra) { + bgp_pbra->installed = false; + bgp_pbra->install_in_progress = false; + } else { + bgp_pbr->installed = false; + bgp_pbr->install_in_progress = false; + } break; case ZAPI_RULE_INSTALLED: - bgp_pbra->installed = true; - bgp_pbra->install_in_progress = false; + if (bgp_pbra) { + bgp_pbra->installed = true; + bgp_pbra->install_in_progress = false; + } else { + bgp_pbr->installed = true; + bgp_pbr->install_in_progress = false; + bgp_pbr->action->refcnt++; + } if (BGP_DEBUG(zebra, ZEBRA)) zlog_debug("%s: Received RULE_INSTALLED", __PRETTY_FUNCTION__); From 8112a7a0723561ea3d959a0c72ba8b3c4134f316 Mon Sep 17 00:00:00 2001 From: Philippe Guibert Date: Fri, 30 Nov 2018 14:13:37 +0100 Subject: [PATCH 8/9] bgpd: change priority of fs pbr rules two kind of rules are being set from bgp flowspec: ipset based rules, and ip rule rules. default route rules may have a lower priority than the other rules ( that do not support default rules). so, if an ipset rule without fwmark is being requested, then priority is arbitrarily set to 1. the other case, priority is set to 0. Signed-off-by: Philippe Guibert --- bgpd/bgp_pbr.c | 1 + bgpd/bgp_pbr.h | 1 + bgpd/bgp_zebra.c | 12 ++++++++++-- 3 files changed, 12 insertions(+), 2 deletions(-) diff --git a/bgpd/bgp_pbr.c b/bgpd/bgp_pbr.c index 890fb64313..9a6ada2058 100644 --- a/bgpd/bgp_pbr.c +++ b/bgpd/bgp_pbr.c @@ -2030,6 +2030,7 @@ static void bgp_pbr_policyroute_add_to_zebra_unit(struct bgp *bgp, if (bpf->type == BGP_PBR_IPRULE) { memset(&pbr_rule, 0, sizeof(pbr_rule)); pbr_rule.vrf_id = bpf->vrf_id; + pbr_rule.priority = 20; if (bpf->src) { pbr_rule.flags |= MATCH_IP_SRC_SET; prefix_copy(&pbr_rule.src, bpf->src); diff --git a/bgpd/bgp_pbr.h b/bgpd/bgp_pbr.h index c3db10cbe7..da21e0f9c4 100644 --- a/bgpd/bgp_pbr.h +++ b/bgpd/bgp_pbr.h @@ -165,6 +165,7 @@ struct bgp_pbr_rule { struct bgp_pbr_action *action; vrf_id_t vrf_id; uint32_t unique; + uint32_t priority; bool installed; bool install_in_progress; }; diff --git a/bgpd/bgp_zebra.c b/bgpd/bgp_zebra.c index 9212c36dd5..4513086ad0 100644 --- a/bgpd/bgp_zebra.c +++ b/bgpd/bgp_zebra.c @@ -2270,8 +2270,16 @@ static void bgp_encode_pbr_rule_action(struct stream *s, struct prefix pfx; stream_putl(s, 0); /* seqno unused */ - stream_putl(s, 0); /* ruleno unused */ - + if (pbr) + stream_putl(s, pbr->priority); + else + stream_putl(s, 0); + /* ruleno unused - priority change + * ruleno permits distinguishing various FS PBR entries + * - FS PBR entries based on ipset/iptables + * - FS PBR entries based on iprule + * the latter may contain default routing information injected by FS + */ if (pbr) stream_putl(s, pbr->unique); else From ce3c06147c78f690b95aa80eb91ccf1ba41879f8 Mon Sep 17 00:00:00 2001 From: Philippe Guibert Date: Fri, 30 Nov 2018 14:56:40 +0100 Subject: [PATCH 9/9] bgpd: display the list of iprules attached to a fs entry the list of iprules is displayed in the 'show bgp ipv4 flowspec detail' The list of iprules is displayed, only if it is installed. Signed-off-by: Philippe Guibert --- bgpd/bgp_flowspec_vty.c | 20 +++++++++++++++++--- bgpd/bgp_pbr.c | 36 +++++++++++++++++++++++++++++++----- bgpd/bgp_pbr.h | 1 + bgpd/bgp_route.c | 4 ++++ bgpd/bgp_route.h | 4 +++- bgpd/bgp_zebra.c | 9 +++++++-- 6 files changed, 63 insertions(+), 11 deletions(-) diff --git a/bgpd/bgp_flowspec_vty.c b/bgpd/bgp_flowspec_vty.c index 26f0fffb37..72ee8bb4ce 100644 --- a/bgpd/bgp_flowspec_vty.c +++ b/bgpd/bgp_flowspec_vty.c @@ -333,16 +333,17 @@ void route_vty_out_flowspec(struct vty *vty, struct prefix *p, struct bgp_path_info_extra *extra = bgp_path_info_extra_get(path); - if (extra->bgp_fs_pbr) { + if (listcount(extra->bgp_fs_pbr) || + listcount(extra->bgp_fs_iprule)) { struct listnode *node; struct bgp_pbr_match_entry *bpme; + struct bgp_pbr_rule *bpr; struct bgp_pbr_match *bpm; bool list_began = false; struct list *list_bpm; list_bpm = list_new(); - if (listcount(extra->bgp_fs_pbr)) - vty_out(vty, "\tinstalled in PBR"); + vty_out(vty, "\tinstalled in PBR"); for (ALL_LIST_ELEMENTS_RO(extra->bgp_fs_pbr, node, bpme)) { bpm = bpme->backpointer; @@ -356,6 +357,19 @@ void route_vty_out_flowspec(struct vty *vty, struct prefix *p, vty_out(vty, ", "); vty_out(vty, "%s", bpm->ipset_name); } + for (ALL_LIST_ELEMENTS_RO(extra->bgp_fs_iprule, + node, bpr)) { + if (!bpr->action) + continue; + if (!list_began) { + vty_out(vty, " ("); + list_began = true; + } else + vty_out(vty, ", "); + vty_out(vty, "-ipv4-rule %d action lookup %u-", + bpr->priority, + bpr->action->table_id); + } if (list_began) vty_out(vty, ")"); vty_out(vty, "\n"); diff --git a/bgpd/bgp_pbr.c b/bgpd/bgp_pbr.c index 9a6ada2058..c63eb83c1b 100644 --- a/bgpd/bgp_pbr.c +++ b/bgpd/bgp_pbr.c @@ -1414,6 +1414,16 @@ static void bgp_pbr_flush_iprule(struct bgp *bgp, struct bgp_pbr_action *bpa, bpr->installed = false; bpr->action->refcnt--; bpr->action = NULL; + if (bpr->path) { + struct bgp_path_info *path; + struct bgp_path_info_extra *extra; + + /* unlink path to bpme */ + path = (struct bgp_path_info *)bpr->path; + extra = bgp_path_info_extra_get(path); + listnode_delete(extra->bgp_fs_iprule, bpr); + bpr->path = NULL; + } } hash_release(bgp->pbr_rule_hash, bpr); if (bpa->refcnt == 0) { @@ -1445,11 +1455,10 @@ static void bgp_pbr_flush_entry(struct bgp *bgp, struct bgp_pbr_action *bpa, struct bgp_path_info *path; struct bgp_path_info_extra *extra; - /* unlink bgp_path_info to bpme */ + /* unlink path to bpme */ path = (struct bgp_path_info *)bpme->path; extra = bgp_path_info_extra_get(path); - if (extra->bgp_fs_pbr) - listnode_delete(extra->bgp_fs_pbr, bpme); + listnode_delete(extra->bgp_fs_pbr, bpme); bpme->path = NULL; } } @@ -1991,6 +2000,7 @@ static void bgp_pbr_policyroute_add_to_zebra_unit(struct bgp *bgp, struct bgp_pbr_range_port *pkt_len; struct bgp_pbr_rule pbr_rule; struct bgp_pbr_rule *bpr; + bool bpr_found = false; bool bpme_found = false; if (!bpf) @@ -2046,6 +2056,23 @@ static void bgp_pbr_policyroute_add_to_zebra_unit(struct bgp *bgp, bpr->unique = ++bgp_pbr_action_counter_unique; bpr->installed = false; bpr->install_in_progress = false; + /* link bgp info to bpr */ + bpr->path = (void *)path; + } else + bpr_found = true; + /* already installed */ + if (bpr_found && bpr) { + struct bgp_path_info_extra *extra = + bgp_path_info_extra_get(path); + + if (extra && listnode_lookup(extra->bgp_fs_iprule, + bpr)) { + if (BGP_DEBUG(pbr, PBR_ERROR)) + zlog_err("%s: entry %p/%p already " + "installed in bgp pbr iprule", + __func__, path, bpr); + return; + } } if (!bpa->installed && !bpa->install_in_progress) { bgp_send_pbr_rule_action(bpa, NULL, true); @@ -2186,8 +2213,7 @@ static void bgp_pbr_policyroute_add_to_zebra_unit(struct bgp *bgp, struct bgp_path_info_extra *extra = bgp_path_info_extra_get(path); - if (extra && extra->bgp_fs_pbr && - listnode_lookup(extra->bgp_fs_pbr, bpme)) { + if (extra && listnode_lookup(extra->bgp_fs_pbr, bpme)) { if (BGP_DEBUG(pbr, PBR_ERROR)) zlog_err( "%s: entry %p/%p already installed in bgp pbr", diff --git a/bgpd/bgp_pbr.h b/bgpd/bgp_pbr.h index da21e0f9c4..f7fddac7fb 100644 --- a/bgpd/bgp_pbr.h +++ b/bgpd/bgp_pbr.h @@ -168,6 +168,7 @@ struct bgp_pbr_rule { uint32_t priority; bool installed; bool install_in_progress; + void *path; }; struct bgp_pbr_match { diff --git a/bgpd/bgp_route.c b/bgpd/bgp_route.c index 07077dfe1f..0e583d6bd3 100644 --- a/bgpd/bgp_route.c +++ b/bgpd/bgp_route.c @@ -175,6 +175,8 @@ static struct bgp_path_info_extra *bgp_path_info_extra_new(void) sizeof(struct bgp_path_info_extra)); new->label[0] = MPLS_INVALID_LABEL; new->num_labels = 0; + new->bgp_fs_pbr = list_new(); + new->bgp_fs_iprule = list_new(); return new; } @@ -218,6 +220,8 @@ void bgp_path_info_extra_free(struct bgp_path_info_extra **extra) if (e->bgp_orig) bgp_unlock(e->bgp_orig); + if ((*extra)->bgp_fs_iprule) + list_delete(&((*extra)->bgp_fs_iprule)); if ((*extra)->bgp_fs_pbr) list_delete(&((*extra)->bgp_fs_pbr)); XFREE(MTYPE_BGP_ROUTE_EXTRA, *extra); diff --git a/bgpd/bgp_route.h b/bgpd/bgp_route.h index 0b3a203af7..97d4aaeeba 100644 --- a/bgpd/bgp_route.h +++ b/bgpd/bgp_route.h @@ -147,8 +147,10 @@ struct bgp_path_info_extra { * Set nexthop_orig.family to 0 if not valid. */ struct prefix nexthop_orig; - /* presence of FS pbr entry */ + /* presence of FS pbr firewall based entry */ struct list *bgp_fs_pbr; + /* presence of FS pbr iprule based entry */ + struct list *bgp_fs_iprule; }; struct bgp_path_info { diff --git a/bgpd/bgp_zebra.c b/bgpd/bgp_zebra.c index 4513086ad0..f3624228a7 100644 --- a/bgpd/bgp_zebra.c +++ b/bgpd/bgp_zebra.c @@ -2089,9 +2089,16 @@ static int rule_notify_owner(int command, struct zclient *zclient, bgp_pbra->installed = true; bgp_pbra->install_in_progress = false; } else { + struct bgp_path_info *path; + struct bgp_path_info_extra *extra; + bgp_pbr->installed = true; bgp_pbr->install_in_progress = false; bgp_pbr->action->refcnt++; + /* link bgp_info to bgp_pbr */ + path = (struct bgp_path_info *)bgp_pbr->path; + extra = bgp_path_info_extra_get(path); + listnode_add(extra->bgp_fs_iprule, bgp_pbr); } if (BGP_DEBUG(zebra, ZEBRA)) zlog_debug("%s: Received RULE_INSTALLED", @@ -2199,8 +2206,6 @@ static int ipset_entry_notify_owner(int command, struct zclient *zclient, /* link bgp_path_info to bpme */ path = (struct bgp_path_info *)bgp_pbime->path; extra = bgp_path_info_extra_get(path); - if (extra->bgp_fs_pbr == NULL) - extra->bgp_fs_pbr = list_new(); listnode_add(extra->bgp_fs_pbr, bgp_pbime); } break;