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

Support bgp l3vpn over srv6 te policy #17609

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

guoguojia2021
Copy link
Contributor

@guoguojia2021 guoguojia2021 commented Dec 9, 2024

Add SID list support IPv6 address.

router2(config-sr-te)# segment-list test_srv6
router2(config-sr-te-segment-list)# index 10 ipv6-address
X:X::X:X IPv6 address
router2(config-sr-te-segment-list)# index 10 ipv6-address 100:1::1
router2(config-sr-te-segment-list)# index 20 ipv6-address 200:1::1

router2(config-sr-te-segment-list)# index 30 mpls label
(16-1048575) Label Value
router2(config-sr-te-segment-list)# index 30 mpls label 100
% Configuration failed.

Error type: validation
Error description: The Segment List(test_srv6) Type must be SRv6!
router2(config-sr-te-segment-list)#
router2(config-sr-te)# segment-list test_mpls
router2(config-sr-te-segment-list)# index 10 mpls label 100
router2(config-sr-te-segment-list)# index 20 mpls label 200
router2(config-sr-te-segment-list)# index 30 ipv6-address 1:1::1
% Configuration failed.

Error type: validation
Error description: The Segment List(test_mpls) Type must be MPLS!
router2(config-sr-te-segment-list)#

router2# configure
router2(config)# seg
router2(config-sr)# traffic-eng
router2(config-sr-te)# policy color 100 endpoint
A.B.C.D SR Policy endpoint IPv4 address
X:X::X:X SR Policy endpoint IPv6 address
router2(config-sr-te)# policy color 100 endpoint 1.1.1.1

router2(config-sr-te)# policy color 100 endpoint 1.1.1.1
router2(config-sr-te-policy)# candidate-path preference 1 name mpls explicit segment-list test_mpls

router2(config-sr-te-policy)# candidate-path preference 1 name mpls explicit segment-list test_mpls
router2(config-sr-te-policy)# candidate-path preference 2 name srv6 explicit segment-list test_srv6
% Configuration failed.

Error type: validation
Error description: The Segment List type(2) and Policy type(1) must match!
router2(config-sr-te-policy)# exit
router2(config-sr-te)# policy color 200 endpoint 1:1::1
router2(config-sr-te-policy)# candidate-path preference 1 name srv6 explicit segment-list test_srv6
router2(config-sr-te-policy)# candidate-path preference 2 name mpls explicit segment-list test_mpls
% Configuration failed.

Error type: validation
Error description: The Segment List type(1) and Policy type(2) must match!

BGP L3vpn over SRv6-TE policy
config:

segment-routing
traffic-eng
segment-list a
index 10 ipv6-address 100:1::100
index 20 ipv6-address 200:1::200
exit
policy color 100 endpoint 1::1
candidate-path preference 1 name a explicit segment-list a
exit
exit
exit
!
end

show:
router2# show sr-te policy detail

Endpoint: 1::1 Color: 100 Name: BSID: - Status: Active Type: SRV6

  • Preference: 1 Name: a Type: explicit Segment-List: a Protocol-Origin: Local

router2# show bgp ipv4 vpn
BGP table version is 2, local router ID is 2.2.2.2, vrf id 0
Default local pref 100, local AS 2
Route Distinguisher: 1:1
*> 192.168.11.0/24 1::1 0 0 1 ?
UN=1::1 EC{1:1 Color:01:100} label=16 sid=2001:db8:1:1:: sid_structure=[40,24,16,0] type=bgp, subtype=0
*> 192.168.15.0/24 1::1 0 0 1 ?
UN=1::1 EC{1:1 Color:01:100} label=16 sid=2001:db8:1:1:: sid_structure=[40,24,16,0] type=bgp, subtype=0

Displayed 2 routes and 2 total paths
router2# show bgp ipv4 vpn 192.168.11.0
BGP routing table entry for 1:1:192.168.11.0/24, version 1
not allocated
Paths: (1 available, best #1)
Advertised to non peer-group peers:
1::1
1
0.0.0.0 (metric 1024) from 1::1 (1.1.1.1)
Origin incomplete, metric 0, valid, external, best (First path received)
Relay-Nexthop(ip): gate 3fff:172:20:20::1, if eth0,
Relay-Nexthop(tunnel): srv6-tunnel:1::1|100(endpoint|color),
Extended Community: RT:1:1 Color:01:100
Remote label: 16
Remote SID: 2001:db8:1:1::
Last update: Sat Dec 7 04:27:16 2024
router2# show ip route vrf Vrf1 192.168.11.0
Routing entry for 192.168.11.0/24
Known via "bgp", distance 20, metric 0, vrf Vrf1, best
Last update 00:01:31 ago
_**1::1(vrf default) (recursive), label 16, seg6 100:1::100,200:1::200, srv6(endpoint|color):1::1|100, weight 1

  • fe80::a8c1:abff:fe5c:7e19, via eth3(vrf default), label 16, seg6 100:1::100,200:1::200, srv6(endpoint|color):fe80::a8c1:abff:fe5c:7e19|0, weight 1
  • fe80::a8c1:abff:feb5:6de9, via eth4(vrf default), label 16, seg6 100:1::100,200:1::200, srv6(endpoint|color):fe80::a8c1:abff:feb5:6de9|0, weight 1**_

@guoguojia2021 guoguojia2021 reopened this Dec 9, 2024
@guoguojia2021 guoguojia2021 changed the title BGP l3vpn Support bgp l3vpn over srv6 te policy Dec 9, 2024
@guoguojia2021 guoguojia2021 force-pushed the srv6_te_policy branch 5 times, most recently from ec9c40d to d507421 Compare December 10, 2024 06:11
Copy link
Member

@ton31337 ton31337 left a comment

Choose a reason for hiding this comment

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

  • Please reuse IPV[46]_* macros to keep the code maintable;
  • Unified config for new topotests is mandatory;
  • Some minor nits/cleanups for the code yet;
  • frrbot + verify check also must be vanished.

tests/topotests/bgp_srv6l3vpn_te_policy/ce1/bgpd.conf Outdated Show resolved Hide resolved
tests/topotests/bgp_srv6l3vpn_te_policy/ce1/bgpd.conf Outdated Show resolved Hide resolved
tests/topotests/bgp_srv6l3vpn_te_policy/p1/bgpd.conf Outdated Show resolved Hide resolved
tests/topotests/bgp_srv6l3vpn_te_policy/p2/bgpd.conf Outdated Show resolved Hide resolved
tests/topotests/bgp_srv6l3vpn_te_policy/pe1/zebra.conf Outdated Show resolved Hide resolved
bgpd/bgp_nht.c Outdated Show resolved Hide resolved
bgpd/bgp_nexthop.c Show resolved Hide resolved
bgpd/bgp_nexthop.c Show resolved Hide resolved
bgpd/bgp_nht.c Outdated Show resolved Hide resolved
if ((path->type == ZEBRA_ROUTE_VNC) ||
(path->type == ZEBRA_ROUTE_VNC_DIRECT))
if (bnc->srte_color) {
LIST_FOREACH (path, &(bnc->paths), te_nh_thread) {
Copy link
Member

Choose a reason for hiding this comment

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

Is this basically the same as LIST_FOREACH (path, &(bnc->paths), nh_thread)? Then can we just move to the separate function and reuse? Because now we have lots of duplicated stuff + the indentation is bad at some level (due to nesting).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sometimes, two BNC are needed, for example, one route has a color while the other route does not, and they have the same nexthop, the route with a color select SRv6 TE policy, and the other route select SRv6 BE

Copy link
Member

Choose a reason for hiding this comment

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

  • for better readability, if you indent a huge part of code, the best would be to add a separate commit: put the code to indent in a separate function. Then, the next commit does the indentation with only the called function.

@guoguojia2021 guoguojia2021 force-pushed the srv6_te_policy branch 4 times, most recently from 784c85a to 4844dc7 Compare December 10, 2024 12:48
@pguibert6WIND pguibert6WIND self-assigned this Dec 10, 2024
@pguibert6WIND pguibert6WIND self-requested a review December 10, 2024 20:36
@pguibert6WIND pguibert6WIND removed their assignment Dec 10, 2024
@pguibert6WIND
Copy link
Member

Hi @guoguojia2021 , thanks for contribution.
Actually, there is a pull request #15057 already present.
It will be nice to compare both implems to see which one is best suited for our needs.

Copy link

This pull request has conflicts, please resolve those before we can evaluate the pull request.

Copy link
Member

@pguibert6WIND pguibert6WIND left a comment

Choose a reason for hiding this comment

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

There are interesting stuff:

  • pathd handling with srv6 segmen-list seems fine to me

however, some doubts

  • the te approach in bgp ( versus the other pull request and the implementation of nht in pathd and zebra). I feel that it complexifies BGP a lot, whereas BGP should be transparent to that. a design document in doc/developer would be welcome.

  • some commit logs are too dense, and should be split to better understand what needs to be done

@@ -121,6 +121,7 @@ void ospf6_zebra_import_default_route(struct ospf6 *ospf6, bool unreg)
{
struct prefix prefix = {};
int command;
uint8_t flags = 0;

Copy link
Member

Choose a reason for hiding this comment

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

use an uint32_t instead, if you want to be consistent with the zclient_send_rnh API

@@ -1473,6 +1473,7 @@ void ospf_zebra_import_default_route(struct ospf *ospf, bool unreg)
{
struct prefix prefix = {};
int command;
uint8_t flags = 0;

Copy link
Member

Choose a reason for hiding this comment

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

same remark: uint32_t

@@ -425,6 +425,7 @@ void pbr_send_rnh(struct nexthop *nhop, bool reg)
{
uint32_t command;
struct prefix p;
uint8_t flags = 0;

Copy link
Member

Choose a reason for hiding this comment

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

idem

@@ -623,13 +623,17 @@ void sharp_zebra_nexthop_watch(struct prefix *p, vrf_id_t vrf_id, bool import, b
{
int command = ZEBRA_NEXTHOP_REGISTER;
safi_t safi = mrib ? SAFI_MULTICAST : SAFI_UNICAST;
uint8_t flags = 0;

Copy link
Member

Choose a reason for hiding this comment

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

uint32

@@ -43,11 +43,13 @@ static void pim_sendmsg_zebra_rnh(struct pim_instance *pim, struct zclient *zcli
{
struct prefix p;
int ret;
uint8_t flags = 0;

Copy link
Member

Choose a reason for hiding this comment

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

uint32

bgpd/bgp_nexthop.c Show resolved Hide resolved
bnc->bgp->name_pretty, bnc->flags,
bnc->ifindex_ipv6_ll, bnc->path_count,
bnc->nht_info, &bnc->resolved_prefix);
}

if (srte_color) {
if (make_prefix(afi, pi, &p, true) < 0)
Copy link
Member

Choose a reason for hiding this comment

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

  1. what about MPLS ? that means srte mpls will bring a new context that we don't need. can we protect it ?

  2. this design puts the intelligence in BGP rather than in ZEBRA. before, MPLS TE was handled in ZEBRA, and now we decide to use bgp nht for srv6.
    not sure I adhere to this approach. (I would need more to know about the advantages of choosing this).

in the separate pull request , we added nexthop tracking to pathd, and in that way, we don't need to modify BGP.

}
if (pi && srv6_route)
SET_FLAG(pi->flags, BGP_PATH_SRV6_TE);
} else {
Copy link
Member

Choose a reason for hiding this comment

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

the flag SRV6_TE is present; then someone can ask : what abut MPLS_TE flag ?

@@ -2691,6 +2699,7 @@ void vpn_leak_no_retain(struct bgp *to_bgp, struct bgp *vpn_from, afi_t afi)
continue;

bgp_unlink_nexthop(bpi);
bgp_unlink_te_nexthop(bpi);
bgp_rib_remove(bn, bpi, bpi->peer, afi, safi);
Copy link
Member

Choose a reason for hiding this comment

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

on a long term, I think SRv6 could be applied to not only L3VPN. so it should be better to put bgp_unlink_te_nexthop into bgp_unlink _nexthop.

if ((path->type == ZEBRA_ROUTE_VNC) ||
(path->type == ZEBRA_ROUTE_VNC_DIRECT))
if (bnc->srte_color) {
LIST_FOREACH (path, &(bnc->paths), te_nh_thread) {
Copy link
Member

Choose a reason for hiding this comment

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

  • for better readability, if you indent a huge part of code, the best would be to add a separate commit: put the code to indent in a separate function. Then, the next commit does the indentation with only the called function.

@guoguojia2021
Copy link
Contributor Author

Thank you very much for your comments @ton31337 @pguibert6WIND

This is the HLD
https://github.com/eddieruan-alibaba/SONiC/blob/eruan-srv6p/doc/srv6/srv6_policy.md#color -only-policy-use-case

We will complete it in two steps,

  1. Support SRv6 TE policy
  2. Support ECMP paths(multiple candidate paths with the same preferences)
    Support ECMP dependency on this PR.
    Add PIC support in the srv6 VPN scenario. #16879

@guoguojia2021 guoguojia2021 force-pushed the srv6_te_policy branch 2 times, most recently from 25e4f19 to c6377d2 Compare December 20, 2024 09:23
command:

router2(config-sr-te)# segment-list test_srv6
router2(config-sr-te-segment-list)# index 10 ipv6-address
  X:X::X:X  IPv6 address
router2(config-sr-te-segment-list)# index 10 ipv6-address 100:1::1
router2(config-sr-te-segment-list)# index 20 ipv6-address 200:1::1
router2(config-sr-te-segment-list)# index 30 mpls label
  (16-1048575)  Label Value
router2(config-sr-te-segment-list)# index 30 mpls label 100
% Configuration failed.

Error type: validation
Error description: The Segment List(test_srv6) Type must be SRv6!
router2(config-sr-te-segment-list)#
router2(config-sr-te)# segment-list test_mpls
router2(config-sr-te-segment-list)# index 10 mpls label 100
router2(config-sr-te-segment-list)# index 20 mpls label 200
router2(config-sr-te-segment-list)# index 30 ipv6-address 1:1::1
% Configuration failed.

Error type: validation
Error description: The Segment List(test_mpls) Type must be MPLS!
router2(config-sr-te-segment-list)#

router2# configure
router2(config)# seg
router2(config-sr)# traffic-eng
router2(config-sr-te)# policy color 100 endpoint
  A.B.C.D   SR Policy endpoint IPv4 address
  X:X::X:X  SR Policy endpoint IPv6 address
router2(config-sr-te)# policy color 100 endpoint 1.1.1.1
  <cr>
router2(config-sr-te)# policy color 100 endpoint 1.1.1.1
router2(config-sr-te-policy)# candidate-path preference 1 name mpls explicit segment-list test_mpls
  <cr>
router2(config-sr-te-policy)# candidate-path preference 1 name mpls explicit segment-list test_mpls
router2(config-sr-te-policy)# candidate-path preference 2 name srv6 explicit segment-list test_srv6
% Configuration failed.

Error type: validation
Error description: The Segment List type(2) and Policy type(1) must match!
router2(config-sr-te-policy)# exit
router2(config-sr-te)# policy color 200 endpoint 1:1::1
router2(config-sr-te-policy)# candidate-path preference 1 name srv6 explicit segment-list test_srv6
router2(config-sr-te-policy)# candidate-path preference 2 name mpls explicit segment-list test_mpls
% Configuration failed.

Error type: validation
Error description: The Segment List type(1) and Policy type(2) must match!

config:

segment-routing
 traffic-eng
  segment-list test_srv6
   index 10 ipv6-address 100:1::1
   index 20 ipv6-address 200:1::1
  exit
  segment-list test_mpls
   index 10 mpls label 100
   index 20 mpls label 200
  exit
  policy color 100 endpoint 1.1.1.1
   candidate-path preference 1 name mpls explicit segment-list test_mpls
  exit
  policy color 200 endpoint 1:1::1
   candidate-path preference 1 name srv6 explicit segment-list test_srv6
  exit
 exit
exit
!
end

show:
router2# show sr-te policy detail

Endpoint: 1.1.1.1  Color: 100  Name:   BSID: -  Status: Inactive Type: MPLS
  * Preference: 1  Name: mpls  Type: explicit  Segment-List: test_mpls  Protocol-Origin: Local

Endpoint: 1:1::1  Color: 200  Name:   BSID: -  Status: Active Type: SRV6
  * Preference: 1  Name: srv6  Type: explicit  Segment-List: test_srv6  Protocol-Origin: Local

router2#

Signed-off-by: guozhongfeng.gzf <[email protected]>
config:

segment-routing
 traffic-eng
  segment-list a
   index 10 ipv6-address 100:1::100
   index 20 ipv6-address 200:1::200
  exit
  policy color 100 endpoint 1::1
   candidate-path preference 1 name a explicit segment-list a
  exit
 exit
exit
!
end

show:
router2# show sr-te policy detail

Endpoint: 1::1  Color: 100  Name:   BSID: -  Status: Active Type: SRV6
  * Preference: 1  Name: a  Type: explicit  Segment-List: a  Protocol-Origin: Local

router2# show bgp ipv4 vpn
BGP table version is 2, local router ID is 2.2.2.2, vrf id 0
Default local pref 100, local AS 2
Status codes:  s suppressed, d damped, h history, u unsorted, * valid, > best, = multipath,
               i internal, r RIB-failure, S Stale, R Removed
Nexthop codes: @NNN nexthop's vrf id, < announce-nh-self
Origin codes:  i - IGP, e - EGP, ? - incomplete
RPKI validation codes: V valid, I invalid, N Not found

     Network          Next Hop            Metric LocPrf Weight Path
Route Distinguisher: 1:1
 *>  192.168.11.0/24  1::1                     0             0 1 ?
    UN=1::1 EC{1:1 Color:01:100} label=16 sid=2001:db8:1:1:: sid_structure=[40,24,16,0] type=bgp, subtype=0
 *>  192.168.15.0/24  1::1                     0             0 1 ?
    UN=1::1 EC{1:1 Color:01:100} label=16 sid=2001:db8:1:1:: sid_structure=[40,24,16,0] type=bgp, subtype=0

Displayed 2 routes and 2 total paths
router2# show bgp ipv4 vpn 192.168.11.0
BGP routing table entry for 1:1:192.168.11.0/24, version 1
not allocated
Paths: (1 available, best #1)
  Advertised to non peer-group peers:
  1::1
  1
    0.0.0.0 (metric 1024) from 1::1 (1.1.1.1)
      Origin incomplete, metric 0, valid, external, best (First path received)
      Relay-Nexthop(ip): gate 3fff:172:20:20::1, if eth0,
      Relay-Nexthop(tunnel): srv6-tunnel:1::1|100(endpoint|color),
      Extended Community: RT:1:1 Color:01:100
      Remote label: 16
      Remote SID: 2001:db8:1:1::
      Last update: Sat Dec  7 04:27:16 2024
router2# show ip route vrf Vrf1 192.168.11.0
Routing entry for 192.168.11.0/24
  Known via "bgp", distance 20, metric 0, vrf Vrf1, best
  Last update 00:01:31 ago
    1::1(vrf default) (recursive), label 16, seg6 100:1::100,200:1::200, srv6(endpoint|color):1::1|100, weight 1
  *   fe80::a8c1:abff:fe5c:7e19, via eth3(vrf default), label 16, seg6 100:1::100,200:1::200, srv6(endpoint|color):fe80::a8c1:abff:fe5c:7e19|0, weight 1
  *   fe80::a8c1:abff:feb5:6de9, via eth4(vrf default), label 16, seg6 100:1::100,200:1::200, srv6(endpoint|color):fe80::a8c1:abff:feb5:6de9|0, weight 1

frr@router2:/$ ip route show vrf Vrf1
192.168.11.0/24 nhid 74 proto bgp metric 20
        nexthop  encap seg6 mode encap segs 2 [ 100:1::100 200:1::200 ] via inet6 fe80::a8c1:abff:fe5c:7e19 dev eth3 weight 1
        nexthop  encap seg6 mode encap segs 2 [ 100:1::100 200:1::200 ] via inet6 fe80::a8c1:abff:feb5:6de9 dev eth4 weight 1
192.168.15.0/24 nhid 74 proto bgp metric 20
        nexthop  encap seg6 mode encap segs 2 [ 100:1::100 200:1::200 ] via inet6 fe80::a8c1:abff:fe5c:7e19 dev eth3 weight 1
        nexthop  encap seg6 mode encap segs 2 [ 100:1::100 200:1::200 ] via inet6 fe80::a8c1:abff:feb5:6de9 dev eth4 weight 1
192.168.23.0/24 dev eth1 proto kernel scope link src 192.168.23.1
192.168.25.0/24 dev eth5 proto kernel scope link src 192.168.25.1
frr@router2:/$

Signed-off-by: guozhongfeng.gzf <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants