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

Bgpafisafi #107

Merged
merged 18 commits into from
Jan 27, 2017
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
15 changes: 8 additions & 7 deletions bgpd/bgp_route.c
Original file line number Diff line number Diff line change
Expand Up @@ -8058,7 +8058,7 @@ DEFUN (show_ip_bgp,
struct bgp *bgp = NULL;
int idx = 0;

idx = bgp_vty_find_and_parse_afi_safi_vrf (vty, argv, argc, idx, &afi, &safi, &vrf);
bgp_vty_find_and_parse_afi_safi_vrf (vty, argv, argc, &idx, &afi, &safi, &vrf);
if (!idx)
return CMD_WARNING;

Expand All @@ -8067,6 +8067,7 @@ DEFUN (show_ip_bgp,

if (vrf != VRF_ALL)
{
vty_out(vty, "VRF-id: %d", vrf);
bgp = bgp_lookup_by_vrf_id (vrf);
if (bgp == NULL)
{
Expand Down Expand Up @@ -8165,7 +8166,7 @@ DEFUN (show_ip_bgp_route,

int idx = 0;

idx = bgp_vty_find_and_parse_afi_safi_vrf (vty, argv, argc, idx, &afi, &safi, &vrf);
bgp_vty_find_and_parse_afi_safi_vrf (vty, argv, argc, &idx, &afi, &safi, &vrf);
if (!idx)
return CMD_WARNING;

Expand Down Expand Up @@ -8235,7 +8236,7 @@ DEFUN (show_ip_bgp_regexp,
safi_t safi = SAFI_UNICAST;

int idx = 0;
idx = bgp_vty_find_and_parse_afi_safi_vrf (vty, argv, argc, idx, &afi, &safi, &vrf);
bgp_vty_find_and_parse_afi_safi_vrf (vty, argv, argc, &idx, &afi, &safi, &vrf);
if (!idx)
return CMD_WARNING;

Expand Down Expand Up @@ -8269,7 +8270,7 @@ DEFUN (show_ip_bgp_instance_all,
safi_t safi = SAFI_UNICAST;

int idx = 0;
idx = bgp_vty_find_and_parse_afi_safi_vrf (vty, argv, argc, idx, &afi, &safi, &vrf);
bgp_vty_find_and_parse_afi_safi_vrf (vty, argv, argc, &idx, &afi, &safi, &vrf);
if (!idx)
return CMD_WARNING;

Expand Down Expand Up @@ -8960,7 +8961,7 @@ DEFUN (show_ip_bgp_instance_neighbor_prefix_counts,
int idx = 0;
struct bgp *bgp = NULL;

idx = bgp_vty_find_and_parse_afi_safi_vrf (vty, argv, argc, idx, &afi, &safi, &vrf);
bgp_vty_find_and_parse_afi_safi_vrf (vty, argv, argc, &idx, &afi, &safi, &vrf);
if (!idx)
return CMD_WARNING;

Expand Down Expand Up @@ -9324,7 +9325,7 @@ DEFUN (show_ip_bgp_instance_neighbor_advertised_route,

int idx = 0;

idx = bgp_vty_find_and_parse_afi_safi_vrf (vty, argv, argc, idx, &afi, &safi, &vrf);
bgp_vty_find_and_parse_afi_safi_vrf (vty, argv, argc, &idx, &afi, &safi, &vrf);
if (!idx)
return CMD_WARNING;

Expand Down Expand Up @@ -9512,7 +9513,7 @@ DEFUN (show_ip_bgp_neighbor_routes,

int idx = 0;

idx = bgp_vty_find_and_parse_afi_safi_vrf (vty, argv, argc, idx, &afi, &safi, &vrf);
bgp_vty_find_and_parse_afi_safi_vrf (vty, argv, argc, &idx, &afi, &safi, &vrf);
if (!idx)
return CMD_WARNING;

Expand Down
19 changes: 11 additions & 8 deletions bgpd/bgp_vty.c
Original file line number Diff line number Diff line change
Expand Up @@ -232,22 +232,22 @@ argv_find_and_parse_safi (struct cmd_token **argv, int argc, int *index, safi_t
* it found the last token.
*/
Copy link
Member

Choose a reason for hiding this comment

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

1 . returns 0 if error.

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed message.

int
bgp_vty_find_and_parse_afi_safi_vrf (struct vty *vty, struct cmd_token **argv, int argc, int idx,
bgp_vty_find_and_parse_afi_safi_vrf (struct vty *vty, struct cmd_token **argv, int argc, int *idx,
afi_t *afi, safi_t *safi, vrf_id_t *vrf)
{
char *vrf_name = NULL;

if (argv_find (argv, argc, "ip", &idx))
if (argv_find (argv, argc, "ip", idx))
*afi = AFI_IP;

if (argv_find (argv, argc, "view", &idx) || argv_find (argv, argc, "vrf", &idx))
if (argv_find (argv, argc, "view", idx) || argv_find (argv, argc, "vrf", idx))
{
vrf_name = argv[idx + 1]->arg;
idx += 2;
vrf_name = argv[*idx + 1]->arg;
*idx += 2;
}

if (argv_find_and_parse_afi (argv, argc, &idx, afi))
argv_find_and_parse_safi (argv, argc, &idx, safi);
if (argv_find_and_parse_afi (argv, argc, idx, afi))
argv_find_and_parse_safi (argv, argc, idx, safi);

if (vrf_name)
{
Expand All @@ -260,9 +260,12 @@ bgp_vty_find_and_parse_afi_safi_vrf (struct vty *vty, struct cmd_token **argv, i
if (*vrf == VRF_UNKNOWN)
{
Copy link
Member

Choose a reason for hiding this comment

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

how do you initialise vrf in the case vrf_name is already null ( commands with no vrf at all)?
I would lie the *vrf value to be set inside that function.

Copy link
Member Author

Choose a reason for hiding this comment

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

I do not believe this is the right thing to do. Only the calling function knows what the right defaults are for the afi/safi/vrf due to we assume different defaults based upon the command. This approach would break the ability for the calling function to do the right thing. As I stated I will update the Pull Request with better documentation for this funciton.

vty_out (vty, "View/Vrf specified is unknown: %s", vrf_name);
*idx = 0;
Copy link
Member

Choose a reason for hiding this comment

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

why set idx to null here? Isn't the return value of zero sufficient? For that matter, why do anything special here, the caller can check each of {afi, safi, vrf} as they see fit....

Copy link
Member Author

Choose a reason for hiding this comment

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

that is not setting it to null. I'm setting the value of what is pointed at to 0. Very different

Copy link
Member

Choose a reason for hiding this comment

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

sorry too many threads going on, was thinking about the next comment I was going to write on this (WRT bgp = NULL). the comment still applies, please read "why set *idx = 0 to here ..."

Copy link
Member Author

Choose a reason for hiding this comment

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

because it's the error case? We were unable to figure out what vrf was specified( the last line in the doc )

return 0;
}
return idx + 1;

*idx += 1;
return *idx;
}

static int
Expand Down
2 changes: 1 addition & 1 deletion bgpd/bgp_vty.h
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,6 @@ extern int
argv_find_and_parse_safi(struct cmd_token **argv, int argc, int *index, safi_t *safi);

extern int
bgp_vty_find_and_parse_afi_safi_vrf (struct vty *vty, struct cmd_token **argv, int argc, int idx,
bgp_vty_find_and_parse_afi_safi_vrf (struct vty *vty, struct cmd_token **argv, int argc, int *idx,
afi_t *afi, safi_t *safi, vrf_id_t *vrf);
#endif /* _QUAGGA_BGP_VTY_H */