Skip to content

Commit fae0bbc

Browse files
riw777riw777
authored
Merge pull request FRRouting#20288 from louis-6wind/fix-import-vrf
bgpd: fix import vrf command
2 parents d29b704 + 07f3d0a commit fae0bbc

File tree

5 files changed

+81
-53
lines changed

5 files changed

+81
-53
lines changed

‎bgpd/bgp_mplsvpn.c‎

Lines changed: 22 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -3073,7 +3073,7 @@ void vpn_policy_routemap_event(const char *rmap_name)
30733073
vpn_policy_routemap_update(bgp, rmap_name);
30743074
}
30753075

3076-
void vrf_import_from_vrf(struct bgp *to_bgp, struct bgp *from_bgp,
3076+
void vrf_import_from_vrf(struct bgp *to_bgp, struct bgp *from_bgp, const char *import_name,
30773077
afi_t afi, safi_t safi)
30783078
{
30793079
const char *export_name;
@@ -3097,8 +3097,8 @@ void vrf_import_from_vrf(struct bgp *to_bgp, struct bgp *from_bgp,
30973097
* Cross-ref both VRFs. Also, note if this is the first time
30983098
* any VRF is importing from "import_vrf".
30993099
*/
3100-
vname = (from_bgp->name ? XSTRDUP(MTYPE_TMP, from_bgp->name)
3101-
: XSTRDUP(MTYPE_TMP, VRF_DEFAULT_NAME));
3100+
vname = (import_name ? XSTRDUP(MTYPE_TMP, import_name)
3101+
: XSTRDUP(MTYPE_TMP, VRF_DEFAULT_NAME));
31023102

31033103
/* Check the import_vrf list of destination vrf for the source vrf name,
31043104
* insert otherwise.
@@ -3116,6 +3116,12 @@ void vrf_import_from_vrf(struct bgp *to_bgp, struct bgp *from_bgp,
31163116
else
31173117
XFREE(MTYPE_TMP, vname);
31183118

3119+
SET_FLAG(to_bgp->af_flags[afi][safi], BGP_CONFIG_VRF_TO_VRF_IMPORT);
3120+
3121+
if (!from_bgp)
3122+
/* import vrf VRF context does not exist yet. */
3123+
return;
3124+
31193125
/* Check if the source vrf already exports to any vrf,
31203126
* first time export requires to setup auto derived RD/RT values.
31213127
* Add the destination vrf name to export vrf list if it is
@@ -3170,7 +3176,6 @@ void vrf_import_from_vrf(struct bgp *to_bgp, struct bgp *from_bgp,
31703176
.rtlist[idir], ecom);
31713177
else
31723178
to_bgp->vpn_policy[afi].rtlist[idir] = ecommunity_dup(ecom);
3173-
SET_FLAG(to_bgp->af_flags[afi][safi], BGP_CONFIG_VRF_TO_VRF_IMPORT);
31743179

31753180
if (debug) {
31763181
const char *from_name;
@@ -3206,18 +3211,17 @@ void vrf_import_from_vrf(struct bgp *to_bgp, struct bgp *from_bgp,
32063211
vpn_leak_postchange(idir, afi, bgp_get_default(), to_bgp);
32073212
}
32083213

3209-
void vrf_unimport_from_vrf(struct bgp *to_bgp, struct bgp *from_bgp,
3214+
void vrf_unimport_from_vrf(struct bgp *to_bgp, struct bgp *from_bgp, const char *import_name,
32103215
afi_t afi, safi_t safi)
32113216
{
3212-
const char *export_name, *tmp_name;
3217+
const char *export_name;
32133218
enum vpn_policy_direction idir, edir;
32143219
char *vname;
32153220
struct ecommunity *ecom = NULL;
32163221
struct listnode *node;
32173222
int debug;
32183223

32193224
export_name = to_bgp->name ? to_bgp->name : VRF_DEFAULT_NAME;
3220-
tmp_name = from_bgp->name ? from_bgp->name : VRF_DEFAULT_NAME;
32213225
idir = BGP_VPN_POLICY_DIR_FROMVPN;
32223226
edir = BGP_VPN_POLICY_DIR_TOVPN;
32233227

@@ -3227,7 +3231,7 @@ void vrf_unimport_from_vrf(struct bgp *to_bgp, struct bgp *from_bgp,
32273231
/* Were we importing from "import_vrf"? */
32283232
for (ALL_LIST_ELEMENTS_RO(to_bgp->vpn_policy[afi].import_vrf, node,
32293233
vname)) {
3230-
if (strcmp(vname, tmp_name) == 0)
3234+
if (strcmp(vname, import_name) == 0)
32313235
break;
32323236
}
32333237

@@ -3242,7 +3246,7 @@ void vrf_unimport_from_vrf(struct bgp *to_bgp, struct bgp *from_bgp,
32423246
return;
32433247

32443248
if (debug)
3245-
zlog_debug("%s from %s to %s", __func__, tmp_name, export_name);
3249+
zlog_debug("%s from %s to %s", __func__, import_name, export_name);
32463250

32473251
/* Remove "import_vrf" from our import list. */
32483252
listnode_delete(to_bgp->vpn_policy[afi].import_vrf, vname);
@@ -3260,14 +3264,18 @@ void vrf_unimport_from_vrf(struct bgp *to_bgp, struct bgp *from_bgp,
32603264
BGP_CONFIG_VRF_TO_VRF_IMPORT);
32613265
if (to_bgp->vpn_policy[afi].rtlist[idir])
32623266
ecommunity_free(&to_bgp->vpn_policy[afi].rtlist[idir]);
3263-
} else {
3267+
} else if (from_bgp) {
32643268
ecom = from_bgp->vpn_policy[afi].rtlist[edir];
32653269
if (ecom)
32663270
ecommunity_del_val(to_bgp->vpn_policy[afi].rtlist[idir],
32673271
(struct ecommunity_val *)ecom->val);
32683272
vpn_leak_postchange(idir, afi, bgp_get_default(), to_bgp);
32693273
}
32703274

3275+
if (!from_bgp)
3276+
/* import vrf VRF context not (or no more) existing */
3277+
return;
3278+
32713279
/*
32723280
* What?
32733281
* So SA is assuming that since the ALL_LIST_ELEMENTS_RO
@@ -4107,7 +4115,7 @@ void bgp_vpn_leak_unimport(struct bgp *from_bgp)
41074115
to_bgp->name_pretty, afi2str(afi),
41084116
to_vpolicy->import_vrf->count);
41094117

4110-
vrf_unimport_from_vrf(to_bgp, from_bgp, afi, safi);
4118+
vrf_unimport_from_vrf(to_bgp, from_bgp, tmp_name, afi, safi);
41114119

41124120
/* readd vrf name as unimport removes import vrf name
41134121
* from the destination vrf's import list where the
@@ -4127,8 +4135,8 @@ void bgp_vpn_leak_unimport(struct bgp *from_bgp)
41274135
.export_vrf, node,
41284136
vname)) {
41294137
if (strcmp(vname, tmp_name) == 0) {
4130-
vrf_unimport_from_vrf(from_bgp, to_bgp,
4131-
afi, safi);
4138+
vrf_unimport_from_vrf(from_bgp, to_bgp, tmp_name, afi,
4139+
safi);
41324140
break;
41334141
}
41344142
}
@@ -4210,8 +4218,7 @@ void bgp_vpn_leak_export(struct bgp *from_bgp)
42104218
to_vpolicy->rtlist[idir],
42114219
(struct ecommunity_val *)
42124220
ecom->val);
4213-
vrf_import_from_vrf(to_bgp, from_bgp,
4214-
afi, safi);
4221+
vrf_import_from_vrf(to_bgp, from_bgp, export_name, afi, safi);
42154222
break;
42164223

42174224
}

‎bgpd/bgp_mplsvpn.h‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -89,9 +89,9 @@ extern void ensure_vrf_tovpn_sid_per_af(struct bgp *vpn, struct bgp *vrf,
8989
extern void ensure_vrf_tovpn_sid_per_vrf(struct bgp *vpn, struct bgp *vrf);
9090
extern void transpose_sid(struct in6_addr *sid, uint32_t label, uint8_t offset,
9191
uint8_t size);
92-
extern void vrf_import_from_vrf(struct bgp *to_bgp, struct bgp *from_bgp,
92+
extern void vrf_import_from_vrf(struct bgp *to_bgp, struct bgp *from_bgp, const char *import_name,
9393
afi_t afi, safi_t safi);
94-
void vrf_unimport_from_vrf(struct bgp *to_bgp, struct bgp *from_bgp,
94+
void vrf_unimport_from_vrf(struct bgp *to_bgp, struct bgp *from_bgp, const char *import_name,
9595
afi_t afi, safi_t safi);
9696
bool srv6_sid_compose(struct in6_addr *sid_value, struct srv6_locator *locator, uint32_t sid_func);
9797

‎bgpd/bgp_vty.c‎

Lines changed: 6 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -11107,7 +11107,6 @@ DEFPY(bgp_imexport_vrf, bgp_imexport_vrf_cmd,
1110711107
bool remove = false;
1110811108
int32_t idx = 0;
1110911109
char *vname;
11110-
enum bgp_instance_type bgp_type = BGP_INSTANCE_TYPE_VRF;
1111111110
safi_t safi;
1111211111
afi_t afi;
1111311112

@@ -11156,35 +11155,13 @@ DEFPY(bgp_imexport_vrf, bgp_imexport_vrf_cmd,
1115611155
SET_FLAG(bgp_default->flags, BGP_FLAG_INSTANCE_HIDDEN);
1115711156
}
1115811157

11159-
vrf_bgp = bgp_lookup_by_name_filter(import_name, false);
11160-
if (!vrf_bgp) {
11161-
if (strcmp(import_name, VRF_DEFAULT_NAME) == 0) {
11162-
vrf_bgp = bgp_default;
11163-
} else {
11164-
as = AS_UNSPECIFIED;
11165-
11166-
/* Auto-create with AS_UNSPECIFIED, fill in later */
11167-
ret = bgp_get_vty(&vrf_bgp, &as, import_name, bgp_type,
11168-
NULL, ASNOTATION_UNDEFINED);
11169-
if (ret) {
11170-
vty_out(vty,
11171-
"VRF %s is not configured as a bgp instance\n",
11172-
import_name);
11173-
return CMD_WARNING;
11174-
}
11175-
11176-
SET_FLAG(vrf_bgp->flags, BGP_FLAG_INSTANCE_HIDDEN);
11177-
11178-
/* Auto created VRF instances should be marked
11179-
* properly, otherwise we have a state after bgpd
11180-
* restart where VRF instance has default VRF's ASN.
11181-
*/
11182-
SET_FLAG(vrf_bgp->vrf_flags, BGP_VRF_AUTO);
11183-
}
11184-
}
11158+
if (strcmp(import_name, VRF_DEFAULT_NAME) == 0)
11159+
vrf_bgp = bgp_default;
11160+
else
11161+
vrf_bgp = bgp_lookup_by_name_filter(import_name, false);
1118511162

1118611163
if (remove) {
11187-
vrf_unimport_from_vrf(bgp, vrf_bgp, afi, safi);
11164+
vrf_unimport_from_vrf(bgp, vrf_bgp, import_name, afi, safi);
1118811165
} else {
1118911166
/* Already importing from "import_vrf"? */
1119011167
for (ALL_LIST_ELEMENTS_RO(bgp->vpn_policy[afi].import_vrf, node,
@@ -11193,7 +11170,7 @@ DEFPY(bgp_imexport_vrf, bgp_imexport_vrf_cmd,
1119311170
return CMD_WARNING;
1119411171
}
1119511172

11196-
vrf_import_from_vrf(bgp, vrf_bgp, afi, safi);
11173+
vrf_import_from_vrf(bgp, vrf_bgp, import_name, afi, safi);
1119711174
}
1119811175

1119911176
return CMD_SUCCESS;

‎tests/topotests/bgp_vrf_route_leak_basic/r1/bgpd.conf‎

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -17,13 +17,6 @@ router bgp 65500 vrf EVA
1717
import vrf ZITA
1818
!
1919
!
20-
router bgp 65500 vrf ZITA
21-
no bgp ebgp-requires-policy
22-
no bgp network import-check
23-
address-family ipv4 unicast
24-
network 172.16.101.0/24
25-
!
26-
!
2720
router bgp 65500
2821
bgp router-id 192.0.2.1
2922
no bgp ebgp-requires-policy

‎tests/topotests/bgp_vrf_route_leak_basic/test_bgp-vrf-route-leak-basic.py‎

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,36 @@ def test_router_bgp_as_pretty():
7979
assert "router bgp 65500\n" in output, "router bgp 65500 not found in show run"
8080

8181

82+
def test_vrf_bgp_list():
83+
logger.info("Ensure that BGP list does not include non existing VRFs")
84+
tgen = get_topogen()
85+
# Don't run this test if we have any failure.
86+
if tgen.routers_have_failure():
87+
pytest.skip(tgen.errors)
88+
89+
r1 = tgen.gears["r1"]
90+
91+
# Configure a VRF that is not going to be used
92+
r1.cmd("ip link add MARIA type vrf table 1009")
93+
94+
# Attempt to remove ANNA VRF that is not present
95+
r1.vtysh_cmd(
96+
"""
97+
configure
98+
99+
router bgp 99 vrf DONNA
100+
address-family ipv4 unicast
101+
no import vrf ANNA
102+
"""
103+
)
104+
105+
expect = {"vrfs": {"DONNA": {}, "default": {}, "EVA": {}}, "totalVrfs": 3}
106+
107+
test_func = partial(topotest.router_json_cmp, r1, "show bgp vrfs json", expect)
108+
result, diff = topotest.run_and_expect(test_func, None, count=15, wait=1)
109+
assert result, "BGP VRF list check failed:\n{}".format(diff)
110+
111+
82112
def test_vrf_route_leak_donna():
83113
logger.info("Ensure that routes are leaked back and forth")
84114
tgen = get_topogen()
@@ -382,6 +412,18 @@ def test_vrf_route_leak_donna_add_vrf_zita():
382412
r1 = tgen.gears["r1"]
383413
r1.cmd("ip link add ZITA type vrf table 1003")
384414

415+
r1.vtysh_cmd(
416+
"""
417+
configure
418+
419+
router bgp 65500 vrf ZITA
420+
no bgp ebgp-requires-policy
421+
no bgp network import-check
422+
address-family ipv4 unicast
423+
network 172.16.101.0/24
424+
"""
425+
)
426+
385427
# Test DONNA VRF.
386428
expect = {
387429
"172.16.101.0/24": None,
@@ -393,6 +435,15 @@ def test_vrf_route_leak_donna_add_vrf_zita():
393435
result, diff = topotest.run_and_expect(test_func, None, count=15, wait=1)
394436
assert result, "BGP VRF DONNA check failed:\n{}".format(diff)
395437

438+
expect = {
439+
"vrfs": {"DONNA": {}, "default": {}, "EVA": {}, "ZITA": {}},
440+
"totalVrfs": 4,
441+
}
442+
443+
test_func = partial(topotest.router_json_cmp, r1, "show bgp vrfs json", expect)
444+
result, diff = topotest.run_and_expect(test_func, None, count=15, wait=1)
445+
assert result, "BGP VRF list check failed:\n{}".format(diff)
446+
396447

397448
def test_vrf_route_leak_donna_set_zita_up():
398449
logger.info("Set VRF ZITA up and ensure that the route from VRF ZITA is updated")

0 commit comments

Comments
 (0)