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

Fix some encap/mpls commands #111

Merged

Conversation

pguibert6WIND
Copy link
Member

This series of fixes is taking into account some problems discovered when testing vpn/encap feature.
Among of those, some problems for configuring and exploiting.

Because the vpn configuration command was duplicate, there was an
ambiguity to raise. This is a fix that permits configuring vpnv4 or
vpnv6 address-families on bgp.

Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
The commit is removed duplicated command show ip bgp ipv4|ipv6 enca|vpn
command that is conflicting between bgp_route.c and
bgp_mplsvpn.c/bgp_encap.c files. The fix is integrating the call to
specific mpls or encap settings from inside bgp_show_route() function.

Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
This bgp_show_type enumerate was duplicated and modified in several
places. The commit takes the enumerate with the biggest enumerate, so
that it can be used by all the functions using this enumerate.

Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
The fix consists in setting the correct safi value.

Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
@NetDEF-CI
Copy link
Collaborator

Continous Integration Result: SUCCESSFUL

Congratulations, this patch passed basic tests

Tested-by: NetDEF / OpenSourceRouting.org CI System

CI System Testrun URL: https://ci1.netdef.org/browse/FRR-FRRPULLREQ-116/

This is a comment from an EXPERIMENTAL automated CI system.
For questions and feedback in regards to this CI system, please feel free to email
Martin Winter - mwinter (at) opensourcerouting.org.

@@ -24,6 +24,14 @@

extern void bgp_encap_init (void);
extern int bgp_nlri_parse_encap (struct peer *, struct attr *, struct bgp_nlri *);
int
Copy link
Member

Choose a reason for hiding this comment

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

Let's stay consistent with the code around this new declaration

Copy link
Member

@donaldsharp donaldsharp left a comment

Choose a reason for hiding this comment

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

This change will be subsumed by the changes in the bgpafisafi pull request.

@@ -5588,28 +5588,6 @@ DEFUN (address_family_vpnv6,
}
#endif /* KEEP_OLD_VPN_COMMANDS */

DEFUN (address_family_ipv4_vpn,
address_family_ipv4_vpn_cmd,
"address-family ipv4 vpn",
Copy link
Member

Choose a reason for hiding this comment

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

Where is this now defined?

@@ -7902,15 +7882,9 @@ DEFUN (show_ip_bgp_ipv4,
afi = strmatch(argv[idx]->text, "ipv6") ? AFI_IP6 : AFI_IP;
if (argv_find (argv, argc, "unicast", &idx) || argv_find (argv, argc, "multicast", &idx))
safi = bgp_vty_safi_from_arg (argv[idx]->text);
else if (argv_find (argv, argc, "encap", &idx) || argv_find (argv, argc, "vpn", &idx))
Copy link
Member

Choose a reason for hiding this comment

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

This should be changed to use bgp_vty_find_and_parse_afi_safi_vrf (PR #107)
or use the pattern:

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

@donaldsharp
Copy link
Member

As a note, in an effort to get this code in, I am working on it on my pr/111 branch to resolve the conflicts. I am also merging in #113 into that branch as well.

@donaldsharp donaldsharp merged commit 75688c4 into FRRouting:master Jan 27, 2017
@louberger louberger mentioned this pull request Jul 28, 2018
cfra referenced this pull request in opensourcerouting/frr Nov 29, 2018
bgp_vrf_netns: swap the order of netns destruction
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants