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

Issue 1968 defaultvrfonly #2042

Closed
Closed
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
35 changes: 24 additions & 11 deletions zebra/zebra_ns.c
Original file line number Diff line number Diff line change
Expand Up @@ -55,10 +55,15 @@ static struct zebra_ns *dzns;
static inline int zebra_ns_table_entry_compare(const struct zebra_ns_table *e1,
const struct zebra_ns_table *e2)
{
if (e1->tableid == e2->tableid)
return (e1->afi - e2->afi);

return e1->tableid - e2->tableid;
if (e1->tableid < e2->tableid)
return -1;
if (e1->tableid > e2->tableid)
return 1;
if (e1->ns_id < e2->ns_id)
return -1;
if (e1->ns_id > e2->ns_id)
return 1;
return (e1->afi - e2->afi);
}

static int logicalrouter_config_write(struct vty *vty);
Expand Down Expand Up @@ -177,6 +182,7 @@ struct route_table *zebra_ns_find_table(struct zebra_ns *zns, uint32_t tableid,
memset(&finder, 0, sizeof(finder));
finder.afi = afi;
finder.tableid = tableid;
finder.ns_id = zns->ns_id;
znst = RB_FIND(zebra_ns_table_head, &zns->ns_tables, &finder);

if (znst)
Expand All @@ -193,9 +199,11 @@ unsigned long zebra_ns_score_proto(uint8_t proto, unsigned short instance)

zns = zebra_ns_lookup(NS_DEFAULT);

RB_FOREACH (znst, zebra_ns_table_head, &zns->ns_tables)
RB_FOREACH (znst, zebra_ns_table_head, &zns->ns_tables) {
if (znst->ns_id != NS_DEFAULT)
continue;
cnt += rib_score_proto_table(proto, instance, znst->table);

}
return cnt;
}

Expand All @@ -206,8 +214,11 @@ void zebra_ns_sweep_route(void)

zns = zebra_ns_lookup(NS_DEFAULT);

RB_FOREACH (znst, zebra_ns_table_head, &zns->ns_tables)
RB_FOREACH (znst, zebra_ns_table_head, &zns->ns_tables) {
if (znst->ns_id != NS_DEFAULT)
continue;
rib_sweep_table(znst->table);
}
}

struct route_table *zebra_ns_get_table(struct zebra_ns *zns,
Expand All @@ -221,6 +232,7 @@ struct route_table *zebra_ns_get_table(struct zebra_ns *zns,
memset(&finder, 0, sizeof(finder));
finder.afi = afi;
finder.tableid = tableid;
finder.ns_id = zns->ns_id;
znst = RB_FIND(zebra_ns_table_head, &zns->ns_tables, &finder);

if (znst)
Expand All @@ -229,6 +241,7 @@ struct route_table *zebra_ns_get_table(struct zebra_ns *zns,
znst = XCALLOC(MTYPE_ZEBRA_NS, sizeof(*znst));
znst->tableid = tableid;
znst->afi = afi;
znst->ns_id = zns->ns_id;
znst->table =
(afi == AFI_IP6) ? srcdest_table_init() : route_table_init();

Expand Down Expand Up @@ -257,7 +270,7 @@ static void zebra_ns_free_table(struct zebra_ns_table *znst)

int zebra_ns_disable(ns_id_t ns_id, void **info)
{
struct zebra_ns_table *znst;
struct zebra_ns_table *znst, *tmp;
struct zebra_ns *zns = (struct zebra_ns *)(*info);

hash_clean(zns->rules_hash, zebra_pbr_rules_free);
Expand All @@ -271,9 +284,9 @@ int zebra_ns_disable(ns_id_t ns_id, void **info)
zebra_pbr_iptable_free);
hash_free(zns->iptable_hash);

while (!RB_EMPTY(zebra_ns_table_head, &zns->ns_tables)) {
znst = RB_ROOT(zebra_ns_table_head, &zns->ns_tables);

RB_FOREACH_SAFE (znst, zebra_ns_table_head, &zns->ns_tables, tmp) {
if (znst->ns_id != ns_id)
continue;
RB_REMOVE(zebra_ns_table_head, &zns->ns_tables, znst);
zebra_ns_free_table(znst);
}
Expand Down
1 change: 1 addition & 0 deletions zebra/zebra_ns.h
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ struct zebra_ns_table {

uint32_t tableid;
afi_t afi;
ns_id_t ns_id;

struct route_table *table;
};
Expand Down
32 changes: 23 additions & 9 deletions zebra/zebra_static.c
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,9 @@ void static_install_route(afi_t afi, safi_t safi, struct prefix *p,
struct vrf *nh_vrf;

/* Lookup table. */
table = zebra_vrf_table(afi, safi, si->vrf_id);
table = zebra_vrf_table_with_table_id(afi, safi,
si->vrf_id,
si->table_id);
if (!table)
return;

Expand Down Expand Up @@ -170,10 +172,15 @@ void static_install_route(afi_t afi, safi_t safi, struct prefix *p,
re->metric = 0;
re->mtu = 0;
re->vrf_id = si->vrf_id;
re->table =
(si->vrf_id != VRF_DEFAULT)
? (zebra_vrf_lookup_by_id(si->vrf_id))->table_id
: zebrad.rtm_table_default;
if (si->vrf_id != VRF_DEFAULT)
re->table = zebra_vrf_lookup_by_id(si->vrf_id)
->table_id;
else if (si->table_id) /* case default VRF, table used
*/
Copy link
Member

Choose a reason for hiding this comment

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

Please put this comment on its own line above the else, or move the terminating */ onto the same line as /*.

re->table = si->table_id;
else /* case default VRF, default table
*/
Copy link
Member

Choose a reason for hiding this comment

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

Same here

re->table = zebrad.rtm_table_default;
re->nexthop_num = 0;
re->tag = si->tag;

Expand Down Expand Up @@ -290,7 +297,9 @@ void static_uninstall_route(afi_t afi, safi_t safi, struct prefix *p,
struct prefix nh_p;

/* Lookup table. */
table = zebra_vrf_table(afi, safi, si->vrf_id);
table = zebra_vrf_table_with_table_id(afi, safi,
si->vrf_id,
si->table_id);
if (!table)
return;

Expand Down Expand Up @@ -395,7 +404,8 @@ int static_add_route(afi_t afi, safi_t safi, uint8_t type, struct prefix *p,
const char *ifname, enum static_blackhole_type bh_type,
route_tag_t tag, uint8_t distance, struct zebra_vrf *zvrf,
struct zebra_vrf *nh_zvrf,
struct static_nh_label *snh_label)
struct static_nh_label *snh_label,
uint32_t table_id)
{
struct route_node *rn;
struct static_route *si;
Expand Down Expand Up @@ -445,7 +455,7 @@ int static_add_route(afi_t afi, safi_t safi, uint8_t type, struct prefix *p,
if (update)
static_delete_route(afi, safi, type, p, src_p, gate, ifname,
update->tag, update->distance, zvrf,
&update->snh_label);
&update->snh_label, table_id);

/* Make new static route structure. */
si = XCALLOC(MTYPE_STATIC_ROUTE, sizeof(struct static_route));
Expand All @@ -457,6 +467,7 @@ int static_add_route(afi_t afi, safi_t safi, uint8_t type, struct prefix *p,
si->vrf_id = zvrf_id(zvrf);
si->nh_vrf_id = zvrf_id(nh_zvrf);
strcpy(si->nh_vrfname, nh_zvrf->vrf->name);
si->table_id = table_id;

if (ifname)
strlcpy(si->ifname, ifname, sizeof(si->ifname));
Expand Down Expand Up @@ -526,7 +537,8 @@ int static_delete_route(afi_t afi, safi_t safi, uint8_t type, struct prefix *p,
struct prefix_ipv6 *src_p, union g_addr *gate,
const char *ifname, route_tag_t tag, uint8_t distance,
struct zebra_vrf *zvrf,
struct static_nh_label *snh_label)
struct static_nh_label *snh_label,
uint32_t table_id)
{
struct route_node *rn;
struct static_route *si;
Expand All @@ -552,6 +564,8 @@ int static_delete_route(afi_t afi, safi_t safi, uint8_t type, struct prefix *p,
&& IPV6_ADDR_SAME(gate, &si->addr.ipv6))))
&& (!strcmp(ifname ? ifname : "", si->ifname))
&& (!tag || (tag == si->tag))
&& ((table_id && table_id == si->table_id) ||
(table_id == 0))
&& (!snh_label->num_labels
|| !memcmp(&si->snh_label, snh_label,
sizeof(struct static_nh_label))))
Expand Down
9 changes: 7 additions & 2 deletions zebra/zebra_static.h
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,9 @@ struct static_route {

/* Label information */
struct static_nh_label snh_label;

/* Table Information */
uint32_t table_id;
};

extern void static_install_route(afi_t afi, safi_t safi, struct prefix *p,
Expand All @@ -92,14 +95,16 @@ extern int static_add_route(afi_t, safi_t safi, uint8_t type, struct prefix *p,
enum static_blackhole_type bh_type, route_tag_t tag,
uint8_t distance, struct zebra_vrf *zvrf,
struct zebra_vrf *nh_zvrf,
struct static_nh_label *snh_label);
struct static_nh_label *snh_label,
uint32_t table_id);

extern int static_delete_route(afi_t, safi_t safi, uint8_t type,
struct prefix *p, struct prefix_ipv6 *src_p,
union g_addr *gate, const char *ifname,
route_tag_t tag, uint8_t distance,
struct zebra_vrf *zvrf,
struct static_nh_label *snh_label);
struct static_nh_label *snh_label,
uint32_t table_id);

extern void static_ifindex_update(struct interface *ifp, bool up);

Expand Down
7 changes: 3 additions & 4 deletions zebra/zebra_vrf.c
Original file line number Diff line number Diff line change
Expand Up @@ -434,7 +434,8 @@ struct zebra_vrf *zebra_vrf_alloc(void)
zebra_vxlan_init_tables(zvrf);
zebra_mpls_init_tables(zvrf);
zebra_pw_init(zvrf);

zvrf->table_id = RT_TABLE_MAIN;
/* by default table ID is default one */
return zvrf;
}

Expand Down Expand Up @@ -524,9 +525,7 @@ static int vrf_config_write(struct vty *vty)
if (zvrf->l3vni)
vty_out(vty, "vni %u\n", zvrf->l3vni);
vty_out(vty, "!\n");
}

if (vrf_is_user_cfged(vrf)) {
} else if (vrf_is_user_cfged(vrf)) {
Copy link
Member

Choose a reason for hiding this comment

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

This is the wrong fix. We should use the vty_frame() code to display vrf config information.

Copy link
Member Author

Choose a reason for hiding this comment

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

Hi Donald,

the issue is related to default routing table.
please tell me if I am wrong.
I am on VRF netns mode.
if I configure a static route on default VRF, then I should not see default VRF table appear.
I expect static route be on configure terminal.
Actually this is not the case.

#configure terminal
ip route 5.5.5.0/24 tun1
#
#show running-config
...
vrf Default-IP-Routing-Table
ip route 5.5.5.0/24 tun1
!
...

I don't see the point here with vty_frame().
Unless I did not understand how to solve the issue, please refine.
Thanks,
Cheers,Philippe

Copy link
Member

Choose a reason for hiding this comment

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

@pguibert6WIND, Donald (and I) agree with you that it should not show the default VRF. If I understand Donald correctly, he is saying that your approach to fixing this is not ideal (again I agree). We have facilities in lib/vty.c for this exact scenario that allow you to enter a "config frame" by calling vty_frame(). You exit the frame by calling vty_endframe(). Any calls to vty_out() are cached in that frame between entry and exit. If you ended up not calling vty_out() at all, then that entire frame is destroyed and nothing is output (including the very first line that you started the frame with).

Incidentally, I have touched this code to use vty_frame() in an unrelated but overlapping PR:
#2079

Ergo you can just revert this part of the patch.

vty_out(vty, "vrf %s\n", zvrf_name(zvrf));
if (zvrf->l3vni)
vty_out(vty, " vni %u%s\n", zvrf->l3vni,
Expand Down
Loading