zebra: Don't display the vrf if not using namespace based vrfs #16750
Conversation
| @@ -942,11 +942,17 @@ static void do_show_route_helper(struct vty *vty, struct zebra_vrf *zvrf, | |||
| if (!tableid) | |||
| vty_out(vty, "VRF %s:\n", | |||
| zvrf_name(zvrf)); | |||
| else | |||
| vty_out(vty, | |||
| "VRF %s table %u:\n", | |||
There was a problem hiding this comment.
hmm - maybe I'm naive, but ... when I was poking around with this stuff, I thought it was nice that the vrf/table relationship was being displayed - it was just wrong sometimes. would it be bad to show the vrf each time?
There was a problem hiding this comment.
tables have a dual nature in linux. It's a table and that table also can be a vrf. I don't think that it's a distinction that we should be making, though. If I have table 99 and it also is being used by vrf GREEN. I don't think when I say display table 99 that we should say it's vrf GREEN in that case, the operator didn't ask for that data. I don't think it makes sense in the l3mdev based vrf case that we should be displaying anything about a vrf when you display a specific table.
There was a problem hiding this comment.
as I say, maybe I'm being naive - but aren't there just two possibilities? either a table is in the default vrf, or it's in a different vrf. why not show that association? we're willing to show the vrf and table in the output of show vrf, and that seems ... fine. why not here too?
There was a problem hiding this comment.
I kinda like the fact that if the table happens to be a vrf, then show me that info too.
There was a problem hiding this comment.
It is an important piece of info. I am with @mjstapp here, I think we should keep it.
There was a problem hiding this comment.
it doesn't do that Jafar. If using l3mdev it always displays Default. My point is that default is nonsensical. The fact that we are asking about a table means that we are not talking about vrfs at all. In linux they just happen to have taken the l3mdev device and overlayed it over the table id space. But table 3939 is not in the default vrf, as that it has no relation to the default vrf at all, hence my removal
There was a problem hiding this comment.
gosh - we seem to be crossed-up here.
none of us thinks it's correct to print "default vrf" when ... it's not - but it would be nice to print the correct vrf, whether default or not (imo).
There was a problem hiding this comment.
I disagree, the operator has asked for a table not a vrf in this case. Don't display anything about a vrf then
There was a problem hiding this comment.
dropping default, and keeping the correct vrf when there is actually one is the right approach IMO.
donaldsharp
force-pushed
the
table_display_is_not_vrf_based_in_some_cases
branch
from
619d5d6 to
bd6c179
Compare
zebra/zebra_vrf.c
Outdated
| @@ -398,6 +398,8 @@ vrf_id_t zebra_vrf_lookup_by_table(uint32_t table_id, ns_id_t ns_id) | |||
|
|
|||
| RB_FOREACH (vrf, vrf_id_head, &vrfs_by_id) { | |||
| zvrf = vrf->info; | |||
|
|
|||
| zlog_debug("Looking at %s for table_id", zvrf_name(zvrf)); | |||
donaldsharp
force-pushed
the
table_display_is_not_vrf_based_in_some_cases
branch
2 times, most recently
from
c368d77 to
53b1243
Compare
|
Fixed |
donaldsharp
force-pushed
the
table_display_is_not_vrf_based_in_some_cases
branch
from
53b1243 to
e700638
Compare
donaldsharp
force-pushed
the
table_display_is_not_vrf_based_in_some_cases
branch
from
e700638 to
e34bc77
Compare
Currently when doing a `show ip route table XXXX`, zebra is displaying the current default vrf as the vrf we are in. We are displaying a table not a vrf. This is only true if you are not using namespace based vrf's, so modify the output to display accordingly. Signed-off-by: Donald Sharp <[email protected]>
CI is complaining about the large level of indentation. Make it a bit better. Signed-off-by: Donald Sharp <[email protected]>
donaldsharp
force-pushed
the
table_display_is_not_vrf_based_in_some_cases
branch
from
e34bc77 to
e88cbd6
Compare
Currently when doing a
show ip route table XXXX, zebra is displaying the current default vrf as the vrf we are in. We are displaying a table not a vrf. This is only true if you are not using namespace based vrf's, so modify the output to display accordingly.