bgpd: fix prefix-sid crash bug and add topotest (step4)

This commit fix bgpd's prefix-sid type4,5 feature which has
miss implementation from https://github.com/FRRouting/frr/pull/5653
was merged. Due to some nessesary lines are not presented.

When bgpd receives multi update message with same service-sid on
prefix-sid type-5 attribute, bgpd will crash arround path-attribute's
values object reference count.

And also, this commit add a topotest to check that feature work fine.

Signed-off-by: Hiroki Shirokura <slank.dev@gmail.com>

8 files changed, 360 insertions, 0 deletions

diff --git a/bgpd/bgp_attr.c b/bgpd/bgp_attr.c
index 71e4b56a00..c20c435003 100644
--- a/bgpd/bgp_attr.c
+++ b/bgpd/bgp_attr.c
@@ -676,6 +676,10 @@ unsigned int attrhash_key_make(const void *p)
 		MIX(transit_hash_key_make(bgp_attr_get_transit(attr)));
 	if (attr->encap_subtlvs)
 		MIX(encap_hash_key_make(attr->encap_subtlvs));
+	if (attr->srv6_l3vpn)
+		MIX(srv6_l3vpn_hash_key_make(attr->srv6_l3vpn));
+	if (attr->srv6_vpn)
+		MIX(srv6_vpn_hash_key_make(attr->srv6_vpn));
 #ifdef ENABLE_BGP_VNC
 	struct bgp_attr_encap_subtlv *vnc_subtlvs =
 		bgp_attr_get_vnc_subtlvs(attr);
@@ -1141,6 +1145,16 @@ void bgp_attr_undup(struct attr *new, struct attr *old)
 
 	if (new->lcommunity != old->lcommunity)
 		lcommunity_free(&new->lcommunity);
+
+	if (new->srv6_l3vpn != old->srv6_l3vpn) {
+		srv6_l3vpn_free(new->srv6_l3vpn);
+		new->srv6_l3vpn = NULL;
+	}
+
+	if (new->srv6_vpn != old->srv6_vpn) {
+		srv6_vpn_free(new->srv6_vpn);
+		new->srv6_vpn = NULL;
+	}
 }
 
 /* Free bgp attribute and aspath. */
@@ -1202,6 +1216,14 @@ void bgp_attr_flush(struct attr *attr)
 		encap_free(attr->encap_subtlvs);
 		attr->encap_subtlvs = NULL;
 	}
+	if (attr->srv6_l3vpn && !attr->srv6_l3vpn->refcnt) {
+		srv6_l3vpn_free(attr->srv6_l3vpn);
+		attr->srv6_l3vpn = NULL;
+	}
+	if (attr->srv6_vpn && !attr->srv6_vpn->refcnt) {
+		srv6_vpn_free(attr->srv6_vpn);
+		attr->srv6_vpn = NULL;
+	}
 #ifdef ENABLE_BGP_VNC
 	struct bgp_attr_encap_subtlv *vnc_subtlvs =
 		bgp_attr_get_vnc_subtlvs(attr);
@@ -2676,6 +2698,7 @@ static bgp_attr_parse_ret_t bgp_attr_psid_sub(uint8_t type, uint16_t length,
 					 sizeof(struct bgp_attr_srv6_vpn));
 		attr->srv6_vpn->sid_flags = sid_flags;
 		sid_copy(&attr->srv6_vpn->sid, &ipv6_sid);
+		attr->srv6_vpn = srv6_vpn_intern(attr->srv6_vpn);
 	}
 
 	/* Placeholder code for the SRv6 L3 Service type */
@@ -2718,6 +2741,7 @@ static bgp_attr_parse_ret_t bgp_attr_psid_sub(uint8_t type, uint16_t length,
 		attr->srv6_l3vpn->sid_flags = sid_flags;
 		attr->srv6_l3vpn->endpoint_behavior = endpoint_behavior;
 		sid_copy(&attr->srv6_l3vpn->sid, &ipv6_sid);
+		attr->srv6_l3vpn = srv6_l3vpn_intern(attr->srv6_l3vpn);
 	}
 
 	/* Placeholder code for Unsupported TLV */
diff --git a/tests/topotests/bgp_prefix_sid2/peer1/exabgp.cfg b/tests/topotests/bgp_prefix_sid2/peer1/exabgp.cfg
new file mode 100644
index 0000000000..ad1b15a26c
--- /dev/null
+++ b/tests/topotests/bgp_prefix_sid2/peer1/exabgp.cfg
@@ -0,0 +1,29 @@
+group controller {
+    neighbor 10.0.0.1 {
+        router-id 10.0.0.101;
+        local-address 10.0.0.101;
+        local-as 2;
+        peer-as 1;
+
+        family {
+            ipv6 mpls-vpn;
+        }
+
+        static {
+            route 2001:1::/64 {
+                rd 2:10;
+                next-hop 2001::2;
+                extended-community [ target:2:10 ];
+                label 3;
+                attribute [0x28 0xc0 0x0500150020010db800010001000000000000000100ffff00 ];
+            }
+            route 2001:2::/64 {
+                rd 2:10;
+                next-hop 2001::2;
+                extended-community [ target:2:10 ];
+                label 3;
+                attribute [0x28 0xc0 0x0500150020010db800010001000000000000000100ffff00 ];
+            }
+        }
+    }
+}
diff --git a/tests/topotests/bgp_prefix_sid2/peer1/exabgp.env b/tests/topotests/bgp_prefix_sid2/peer1/exabgp.env
new file mode 100644
index 0000000000..6c554f5fa8
--- /dev/null
+++ b/tests/topotests/bgp_prefix_sid2/peer1/exabgp.env
@@ -0,0 +1,53 @@
+
+[exabgp.api]
+encoder = text
+highres = false
+respawn = false
+socket = ''
+
+[exabgp.bgp]
+openwait = 60
+
+[exabgp.cache]
+attributes = true
+nexthops = true
+
+[exabgp.daemon]
+daemonize = true
+pid = '/var/run/exabgp/exabgp.pid'
+user = 'exabgp'
+
+[exabgp.log]
+all = false
+configuration = true
+daemon = true
+destination = '/var/log/exabgp.log'
+enable = true
+level = INFO
+message = false
+network = true
+packets = false
+parser = false
+processes = true
+reactor = true
+rib = false
+routes = false
+short = false
+timers = false
+
+[exabgp.pdb]
+enable = false
+
+[exabgp.profile]
+enable = false
+file = ''
+
+[exabgp.reactor]
+speed = 1.0
+
+[exabgp.tcp]
+acl = false
+bind = ''
+delay = 0
+once = false
+port = 179
diff --git a/tests/topotests/bgp_prefix_sid2/r1/bgpd.conf b/tests/topotests/bgp_prefix_sid2/r1/bgpd.conf
new file mode 100644
index 0000000000..ddc1f07e42
--- /dev/null
+++ b/tests/topotests/bgp_prefix_sid2/r1/bgpd.conf
@@ -0,0 +1,26 @@
+log stdout notifications
+log monitor notifications
+!log commands
+!
+!debug bgp zebra
+!debug bgp neighbor-events
+!debug bgp vnc verbose
+!debug bgp update-groups
+!debug bgp updates in
+!debug bgp updates out
+!debug bgp vpn label
+!debug bgp vpn leak-from-vrf
+!debug bgp vpn leak-to-vrf
+!debug bgp vpn rmap-event
+!
+router bgp 1
+ bgp router-id 10.0.0.1
+ no bgp default ipv4-unicast
+ no bgp ebgp-requires-policy
+ neighbor 10.0.0.101 remote-as 2
+ neighbor 10.0.0.101 timers 3 10
+ !
+ address-family ipv6 vpn
+  neighbor 10.0.0.101 activate
+ exit-address-family
+!
diff --git a/tests/topotests/bgp_prefix_sid2/r1/vpnv6_rib_entry1.json b/tests/topotests/bgp_prefix_sid2/r1/vpnv6_rib_entry1.json
new file mode 100644
index 0000000000..42293b1fc7
--- /dev/null
+++ b/tests/topotests/bgp_prefix_sid2/r1/vpnv6_rib_entry1.json
@@ -0,0 +1,50 @@
+{
+  "2:10":{
+    "prefix":"2001:1::\/64",
+    "advertisedTo":{
+      "10.0.0.101":{
+      }
+    },
+    "paths":[
+      {
+        "aspath":{
+          "string":"2",
+          "segments":[
+            {
+              "type":"as-sequence",
+              "list":[
+                2
+              ]
+            }
+          ],
+          "length":1
+        },
+        "origin":"IGP",
+        "valid":true,
+        "bestpath":{
+          "overall":true
+        },
+        "extendedCommunity":{
+          "string":"RT:2:10"
+        },
+        "remoteLabel":3,
+        "remoteSid":"2001:db8:1:1::1",
+        "nexthops":[
+          {
+            "ip":"2001::2",
+            "afi":"ipv6",
+            "scope":"global",
+            "metric":0,
+            "accessible":true,
+            "used":true
+          }
+        ],
+        "peer":{
+          "peerId":"10.0.0.101",
+          "routerId":"10.0.0.101",
+          "type":"external"
+        }
+      }
+    ]
+  }
+}
diff --git a/tests/topotests/bgp_prefix_sid2/r1/vpnv6_rib_entry2.json b/tests/topotests/bgp_prefix_sid2/r1/vpnv6_rib_entry2.json
new file mode 100644
index 0000000000..c9ad8714c1
--- /dev/null
+++ b/tests/topotests/bgp_prefix_sid2/r1/vpnv6_rib_entry2.json
@@ -0,0 +1,50 @@
+{
+  "2:10":{
+    "prefix":"2001:2::\/64",
+    "advertisedTo":{
+      "10.0.0.101":{
+      }
+    },
+    "paths":[
+      {
+        "aspath":{
+          "string":"2",
+          "segments":[
+            {
+              "type":"as-sequence",
+              "list":[
+                2
+              ]
+            }
+          ],
+          "length":1
+        },
+        "origin":"IGP",
+        "valid":true,
+        "bestpath":{
+          "overall":true
+        },
+        "extendedCommunity":{
+          "string":"RT:2:10"
+        },
+        "remoteLabel":3,
+        "remoteSid":"2001:db8:1:1::1",
+        "nexthops":[
+          {
+            "ip":"2001::2",
+            "afi":"ipv6",
+            "scope":"global",
+            "metric":0,
+            "accessible":true,
+            "used":true
+          }
+        ],
+        "peer":{
+          "peerId":"10.0.0.101",
+          "routerId":"10.0.0.101",
+          "type":"external"
+        }
+      }
+    ]
+  }
+}
diff --git a/tests/topotests/bgp_prefix_sid2/r1/zebra.conf b/tests/topotests/bgp_prefix_sid2/r1/zebra.conf
new file mode 100644
index 0000000000..0cd26052f2
--- /dev/null
+++ b/tests/topotests/bgp_prefix_sid2/r1/zebra.conf
@@ -0,0 +1,7 @@
+hostname r1
+!
+interface r1-eth0
+ ip address 10.0.0.1/24
+ no shutdown
+!
+line vty
diff --git a/tests/topotests/bgp_prefix_sid2/test_bgp_prefix_sid2.py b/tests/topotests/bgp_prefix_sid2/test_bgp_prefix_sid2.py
new file mode 100755
index 0000000000..25362530d4
--- /dev/null
+++ b/tests/topotests/bgp_prefix_sid2/test_bgp_prefix_sid2.py
@@ -0,0 +1,121 @@
+#!/usr/bin/env python
+
+#
+# test_bgp_prefix_sid2.py
+# Part of NetDEF Topology Tests
+#
+# Copyright (c) 2020 by LINE Corporation
+# Copyright (c) 2020 by Hiroki Shirokura <slank.dev@gmail.com>
+#
+# Permission to use, copy, modify, and/or distribute this software
+# for any purpose with or without fee is hereby granted, provided
+# that the above copyright notice and this permission notice appear
+# in all copies.
+#
+# THE SOFTWARE IS PROVIDED "AS IS" AND NETDEF DISCLAIMS ALL WARRANTIES
+# WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
+# MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL NETDEF BE LIABLE FOR
+# ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY
+# DAMAGES WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS,
+# WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS
+# ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR PERFORMANCE
+# OF THIS SOFTWARE.
+#
+
+"""
+test_bgp_prefix_sid2.py: Test BGP topology with EBGP on prefix-sid
+"""
+
+import json
+import os
+import sys
+import functools
+import pytest
+
+CWD = os.path.dirname(os.path.realpath(__file__))
+sys.path.append(os.path.join(CWD, "../"))
+
+# pylint: disable=C0413
+from lib import topotest
+from lib.topogen import Topogen, TopoRouter, get_topogen
+from lib.topolog import logger
+from mininet.topo import Topo
+
+
+class TemplateTopo(Topo):
+    def build(self, **_opts):
+        tgen = get_topogen(self)
+        router = tgen.add_router("r1")
+        switch = tgen.add_switch("s1")
+        switch.add_link(router)
+
+        switch = tgen.gears["s1"]
+        peer1 = tgen.add_exabgp_peer(
+            "peer1", ip="10.0.0.101", defaultRoute="via 10.0.0.1"
+        )
+        switch.add_link(peer1)
+
+
+def setup_module(module):
+    tgen = Topogen(TemplateTopo, module.__name__)
+    tgen.start_topology()
+
+    router = tgen.gears["r1"]
+    router.load_config(
+        TopoRouter.RD_ZEBRA,
+        os.path.join(CWD, "{}/zebra.conf".format("r1"))
+    )
+    router.load_config(
+        TopoRouter.RD_BGP,
+        os.path.join(CWD, "{}/bgpd.conf".format("r1"))
+    )
+    router.start()
+
+    logger.info("starting exaBGP")
+    peer_list = tgen.exabgp_peers()
+    for pname, peer in peer_list.items():
+        logger.info("starting exaBGP on {}".format(pname))
+        peer_dir = os.path.join(CWD, pname)
+        env_file = os.path.join(CWD, pname, "exabgp.env")
+        logger.info("Running ExaBGP peer on {}".format(pname))
+        peer.start(peer_dir, env_file)
+        logger.info(pname)
+
+
+def teardown_module(module):
+    tgen = get_topogen()
+    tgen.stop_topology()
+
+
+def open_json_file(filename):
+    try:
+        with open(filename, "r") as f:
+            return json.load(f)
+    except IOError:
+        assert False, "Could not read file {}".format(filename)
+
+
+def test_r1_rib():
+    def _check(name, cmd, expected_file):
+        logger.info("polling")
+        tgen = get_topogen()
+        router = tgen.gears[name]
+        output = json.loads(router.vtysh_cmd(cmd))
+        expected = open_json_file("{}/{}".format(CWD, expected_file))
+        return topotest.json_cmp(output, expected)
+
+    def check(name, cmd, expected_file):
+        logger.info("[+] check {} \"{}\" {}".format(name, cmd, expected_file))
+        tgen = get_topogen()
+        func = functools.partial(_check, name, cmd, expected_file)
+        success, result = topotest.run_and_expect(func, None, count=10, wait=0.5)
+        assert result is None, 'Failed'
+
+    check("r1", "show bgp ipv6 vpn 2001:1::/64 json", "r1/vpnv6_rib_entry1.json")
+    check("r1", "show bgp ipv6 vpn 2001:2::/64 json", "r1/vpnv6_rib_entry2.json")
+
+
+if __name__ == "__main__":
+    args = ["-s"] + sys.argv[1:]
+    ret = pytest.main(args)
+    sys.exit(ret)