Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bgpd: vpn - vrf route leaking #1739

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
139 changes: 139 additions & 0 deletions bgpd/bgp_debug.c
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ unsigned long conf_bgp_debug_zebra;
unsigned long conf_bgp_debug_allow_martians;
unsigned long conf_bgp_debug_nht;
unsigned long conf_bgp_debug_update_groups;
unsigned long conf_bgp_debug_vpn;

unsigned long term_bgp_debug_as4;
unsigned long term_bgp_debug_neighbor_events;
Expand All @@ -68,6 +69,7 @@ unsigned long term_bgp_debug_zebra;
unsigned long term_bgp_debug_allow_martians;
unsigned long term_bgp_debug_nht;
unsigned long term_bgp_debug_update_groups;
unsigned long term_bgp_debug_vpn;

struct list *bgp_debug_neighbor_events_peers = NULL;
struct list *bgp_debug_keepalive_peers = NULL;
Expand Down Expand Up @@ -1560,6 +1562,96 @@ DEFUN (no_debug_bgp_update_groups,
return CMD_SUCCESS;
}

DEFUN (debug_bgp_vpn,
debug_bgp_vpn_cmd,
"debug bgp vpn <leak-from-vrf|leak-to-vrf|rmap-event|label>",
DEBUG_STR
BGP_STR
"VPN routes\n"
"leaked from vrf to vpn\n"
"leaked to vrf from vpn\n"
"route-map updates\n"
"labels\n")
{
int idx = 3;

if (argv_find(argv, argc, "leak-from-vrf", &idx)) {
if (vty->node == CONFIG_NODE)
DEBUG_ON(vpn, VPN_LEAK_FROM_VRF);
else
TERM_DEBUG_ON(vpn, VPN_LEAK_FROM_VRF);
} else if (argv_find(argv, argc, "leak-to-vrf", &idx)) {
if (vty->node == CONFIG_NODE)
DEBUG_ON(vpn, VPN_LEAK_TO_VRF);
else
TERM_DEBUG_ON(vpn, VPN_LEAK_TO_VRF);
} else if (argv_find(argv, argc, "rmap-event", &idx)) {
if (vty->node == CONFIG_NODE)
DEBUG_ON(vpn, VPN_LEAK_RMAP_EVENT);
else
TERM_DEBUG_ON(vpn, VPN_LEAK_RMAP_EVENT);
} else if (argv_find(argv, argc, "label", &idx)) {
if (vty->node == CONFIG_NODE)
DEBUG_ON(vpn, VPN_LEAK_LABEL);
else
TERM_DEBUG_ON(vpn, VPN_LEAK_LABEL);
} else {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Once the issues above are resolved you can delete this block.

vty_out(vty, "%% unknown debug bgp vpn keyword\n");
return CMD_WARNING_CONFIG_FAILED;
}

if (vty->node != CONFIG_NODE)
vty_out(vty, "enabled debug bgp vpn %s\n", argv[idx]->text);

return CMD_SUCCESS;
}

DEFUN (no_debug_bgp_vpn,
no_debug_bgp_vpn_cmd,
"no debug bgp vpn <leak-from-vrf|leak-to-vrf|rmap-event|label>",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As above. Additionally, you can remove this entire DEFUN by placing [no] at the start of the previous command and merely flipping which macro you use (DEBUG_TERM_ON / DEBUG_TERM_OFF / etc.) based on whether no is present.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed on [no], but since the debug variables aren't parameterised at run-time, it doesn't seem like a LOC win. It would sure be nice if they were an array indexed by enum so we could do something like

#define TYPE_CONF 0x2
#define TYPE_TERM 0x4
enum subsystem {
    as4,
    neighbor,
    packet,
    ...
};
bgp_debug_set(types, subsystem, enable)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can delete the entire command definition and body and replace it with a 3-line if/else statement, remove the installation call to the CLI graph, and improve future maintainability by not having to update two different functions that are copy-pastes of each other with only minor differences by doing this. But it is just a suggestion and certainly not a blocking issue on the PR.

As for your suggestion I implemented more or less the same in a PR submitted a couple weeks ago (#1808)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this what you mean? I must be missing something:

{
        int idx = 0;
        int enable = 0;
        int notfound = 0;

        if (argv_find(argv, argc, "no", &idx)) {
                if (argv_find(argv, argc, "leak-from-vrf", &idx)) {
                        if (vty->node == CONFIG_NODE)
                                DEBUG_OFF(vpn, VPN_LEAK_FROM_VRF);
                        else
                                TERM_DEBUG_OFF(vpn, VPN_LEAK_FROM_VRF);

                } else if (argv_find(argv, argc, "leak-to-vrf", &idx)) {
                        if (vty->node == CONFIG_NODE)
                                DEBUG_OFF(vpn, VPN_LEAK_TO_VRF);
                        else
                                TERM_DEBUG_OFF(vpn, VPN_LEAK_TO_VRF);
                } else if (argv_find(argv, argc, "rmap-event", &idx)) {
                        if (vty->node == CONFIG_NODE)
                                DEBUG_OFF(vpn, VPN_LEAK_RMAP_EVENT);
                        else
                                TERM_DEBUG_OFF(vpn, VPN_LEAK_RMAP_EVENT);
                } else if (argv_find(argv, argc, "label", &idx)) {
                        if (vty->node == CONFIG_NODE)
                                DEBUG_OFF(vpn, VPN_LEAK_LABEL);
                        else
                                TERM_DEBUG_OFF(vpn, VPN_LEAK_LABEL);
                } else {
                        notfound = 1;
                }
        } else {
                enable = 1;
                if (argv_find(argv, argc, "leak-from-vrf", &idx)) {
                        if (vty->node == CONFIG_NODE)
                                DEBUG_ON(vpn, VPN_LEAK_FROM_VRF);
                        else
                                TERM_DEBUG_ON(vpn, VPN_LEAK_FROM_VRF);
                } else if (argv_find(argv, argc, "leak-to-vrf", &idx)) {
                        if (vty->node == CONFIG_NODE)
                                DEBUG_ON(vpn, VPN_LEAK_TO_VRF);
                        else
                                TERM_DEBUG_ON(vpn, VPN_LEAK_TO_VRF);
                } else if (argv_find(argv, argc, "rmap-event", &idx)) {
                        if (vty->node == CONFIG_NODE)
                                DEBUG_ON(vpn, VPN_LEAK_RMAP_EVENT);
                        else
                                TERM_DEBUG_ON(vpn, VPN_LEAK_RMAP_EVENT);
                } else if (argv_find(argv, argc, "label", &idx)) {
                        if (vty->node == CONFIG_NODE)
                                DEBUG_ON(vpn, VPN_LEAK_LABEL);
                        else
                                TERM_DEBUG_ON(vpn, VPN_LEAK_LABEL);
                } else {
                        notfound = 1;
                }
        }
        if (notfound) {
                vty_out(vty, "%% unknown debug bgp vpn keyword\n");
                return CMD_WARNING_CONFIG_FAILED;
        }

        if (vty->node != CONFIG_NODE)
                vty_out(vty, "%abled debug bgp vpn %s\n",
                        (enable? "en": "dis"), argv[idx]->text);

                 return CMD_SUCCESS;
}

NO_STR
DEBUG_STR
BGP_STR
"VPN routes\n"
"leaked from vrf to vpn\n"
"leaked to vrf from vpn\n"
"route-map updates\n"
"labels\n")
{
int idx = 4;

if (argv_find(argv, argc, "leak-from-vrf", &idx)) {
if (vty->node == CONFIG_NODE)
DEBUG_OFF(vpn, VPN_LEAK_FROM_VRF);
else
TERM_DEBUG_OFF(vpn, VPN_LEAK_FROM_VRF);

} else if (argv_find(argv, argc, "leak-to-vrf", &idx)) {
if (vty->node == CONFIG_NODE)
DEBUG_OFF(vpn, VPN_LEAK_TO_VRF);
else
TERM_DEBUG_OFF(vpn, VPN_LEAK_TO_VRF);
} else if (argv_find(argv, argc, "rmap-event", &idx)) {
if (vty->node == CONFIG_NODE)
DEBUG_OFF(vpn, VPN_LEAK_RMAP_EVENT);
else
TERM_DEBUG_OFF(vpn, VPN_LEAK_RMAP_EVENT);
} else if (argv_find(argv, argc, "label", &idx)) {
if (vty->node == CONFIG_NODE)
DEBUG_OFF(vpn, VPN_LEAK_LABEL);
else
TERM_DEBUG_OFF(vpn, VPN_LEAK_LABEL);
} else {
vty_out(vty, "%% unknown debug bgp vpn keyword\n");
return CMD_WARNING_CONFIG_FAILED;
}

if (vty->node != CONFIG_NODE)
vty_out(vty, "disabled debug bgp vpn %s\n", argv[idx]->text);

return CMD_SUCCESS;
}

DEFUN (no_debug_bgp,
no_debug_bgp_cmd,
"no debug bgp",
Expand Down Expand Up @@ -1592,6 +1684,10 @@ DEFUN (no_debug_bgp,
TERM_DEBUG_OFF(zebra, ZEBRA);
TERM_DEBUG_OFF(allow_martians, ALLOW_MARTIANS);
TERM_DEBUG_OFF(nht, NHT);
TERM_DEBUG_OFF(vpn, VPN_LEAK_FROM_VRF);
TERM_DEBUG_OFF(vpn, VPN_LEAK_TO_VRF);
TERM_DEBUG_OFF(vpn, VPN_LEAK_RMAP_EVENT);
TERM_DEBUG_OFF(vpn, VPN_LEAK_LABEL);
vty_out(vty, "All possible debugging has been turned off\n");

return CMD_SUCCESS;
Expand Down Expand Up @@ -1651,6 +1747,18 @@ DEFUN_NOSH (show_debugging_bgp,

if (BGP_DEBUG(allow_martians, ALLOW_MARTIANS))
vty_out(vty, " BGP allow martian next hop debugging is on\n");

if (BGP_DEBUG(vpn, VPN_LEAK_FROM_VRF))
vty_out(vty,
" BGP route leak from vrf to vpn debugging is on\n");
if (BGP_DEBUG(vpn, VPN_LEAK_TO_VRF))
vty_out(vty,
" BGP route leak to vrf from vpn debugging is on\n");
if (BGP_DEBUG(vpn, VPN_LEAK_RMAP_EVENT))
vty_out(vty, " BGP vpn route-map event debugging is on\n");
if (BGP_DEBUG(vpn, VPN_LEAK_LABEL))
vty_out(vty, " BGP vpn label event debugging is on\n");

vty_out(vty, "\n");
return CMD_SUCCESS;
}
Expand Down Expand Up @@ -1695,6 +1803,15 @@ int bgp_debug_count(void)
if (BGP_DEBUG(allow_martians, ALLOW_MARTIANS))
ret++;

if (BGP_DEBUG(vpn, VPN_LEAK_FROM_VRF))
ret++;
if (BGP_DEBUG(vpn, VPN_LEAK_TO_VRF))
ret++;
if (BGP_DEBUG(vpn, VPN_LEAK_RMAP_EVENT))
ret++;
if (BGP_DEBUG(vpn, VPN_LEAK_LABEL))
ret++;

return ret;
}

Expand Down Expand Up @@ -1771,6 +1888,23 @@ static int bgp_config_write_debug(struct vty *vty)
write++;
}

if (CONF_BGP_DEBUG(vpn, VPN_LEAK_FROM_VRF)) {
vty_out(vty, "debug bgp vpn leak-from-vrf\n");
write++;
}
if (CONF_BGP_DEBUG(vpn, VPN_LEAK_TO_VRF)) {
vty_out(vty, "debug bgp vpn leak-to-vrf\n");
write++;
}
if (CONF_BGP_DEBUG(vpn, VPN_LEAK_RMAP_EVENT)) {
vty_out(vty, "debug bgp vpn rmap-event\n");
write++;
}
if (CONF_BGP_DEBUG(vpn, VPN_LEAK_LABEL)) {
vty_out(vty, "debug bgp vpn label\n");
write++;
}

return write;
}

Expand Down Expand Up @@ -1864,6 +1998,11 @@ void bgp_debug_init(void)
install_element(CONFIG_NODE, &no_debug_bgp_bestpath_cmd);
install_element(ENABLE_NODE, &no_debug_bgp_bestpath_prefix_cmd);
install_element(CONFIG_NODE, &no_debug_bgp_bestpath_prefix_cmd);

install_element(ENABLE_NODE, &debug_bgp_vpn_cmd);
install_element(CONFIG_NODE, &debug_bgp_vpn_cmd);
install_element(ENABLE_NODE, &no_debug_bgp_vpn_cmd);
install_element(CONFIG_NODE, &no_debug_bgp_vpn_cmd);
}

/* Return true if this prefix is on the per_prefix_list of prefixes to debug
Expand Down
6 changes: 6 additions & 0 deletions bgpd/bgp_debug.h
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ extern unsigned long conf_bgp_debug_zebra;
extern unsigned long conf_bgp_debug_allow_martians;
extern unsigned long conf_bgp_debug_nht;
extern unsigned long conf_bgp_debug_update_groups;
extern unsigned long conf_bgp_debug_vpn;

extern unsigned long term_bgp_debug_as4;
extern unsigned long term_bgp_debug_neighbor_events;
Expand All @@ -83,6 +84,7 @@ extern unsigned long term_bgp_debug_zebra;
extern unsigned long term_bgp_debug_allow_martians;
extern unsigned long term_bgp_debug_nht;
extern unsigned long term_bgp_debug_update_groups;
extern unsigned long term_bgp_debug_vpn;

extern struct list *bgp_debug_neighbor_events_peers;
extern struct list *bgp_debug_keepalive_peers;
Expand Down Expand Up @@ -111,6 +113,10 @@ struct bgp_debug_filter {
#define BGP_DEBUG_ALLOW_MARTIANS 0x01
#define BGP_DEBUG_NHT 0x01
#define BGP_DEBUG_UPDATE_GROUPS 0x01
#define BGP_DEBUG_VPN_LEAK_FROM_VRF 0x01
#define BGP_DEBUG_VPN_LEAK_TO_VRF 0x02
#define BGP_DEBUG_VPN_LEAK_RMAP_EVENT 0x04
#define BGP_DEBUG_VPN_LEAK_LABEL 0x08

#define BGP_DEBUG_PACKET_SEND 0x01
#define BGP_DEBUG_PACKET_SEND_DETAIL 0x02
Expand Down
6 changes: 6 additions & 0 deletions bgpd/bgp_main.c
Original file line number Diff line number Diff line change
Expand Up @@ -265,6 +265,8 @@ static int bgp_vrf_enable(struct vrf *vrf)
if (old_vrf_id != bgp->vrf_id)
bgp_update_redist_vrf_bitmaps(bgp, old_vrf_id);
bgp_instance_up(bgp);
vpn_leak_zebra_vrf_label_update(bgp, AFI_IP);
vpn_leak_zebra_vrf_label_update(bgp, AFI_IP6);
}

return 0;
Expand All @@ -283,6 +285,10 @@ static int bgp_vrf_disable(struct vrf *vrf)

bgp = bgp_lookup_by_name(vrf->name);
if (bgp) {

vpn_leak_zebra_vrf_label_withdraw(bgp, AFI_IP);
vpn_leak_zebra_vrf_label_withdraw(bgp, AFI_IP6);

old_vrf_id = bgp->vrf_id;
bgp_handle_socket(bgp, vrf, VRF_UNKNOWN, false);
/* We have instance configured, unlink from VRF and make it
Expand Down
Loading