From 9d080116b9b439e4bee274af0bf85b07871a269e Mon Sep 17 00:00:00 2001 From: Donald Sharp Date: Sat, 18 Jan 2020 09:25:38 -0500 Subject: [PATCH 1/2] lib: Fix nexthop encoding Commit 68a02e06e5f103048d947262c08c569056f74d1c broke nexthop encoding for nexthop tracking. This code combined the different types of nexthop encoding being done in the zapi protocol. What was missed that resolved nexthops of type NEXTHOP_TYPE_IPV4|6 have an ifindex value that was not being reported. This commit ensures that we always send this data( even if it is 0). The following test commit will ensure that this stays working as is expected by an upper level protocol. Signed-off-by: Donald Sharp --- lib/zclient.c | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/lib/zclient.c b/lib/zclient.c index ec1082807c..b2c74cd0b9 100644 --- a/lib/zclient.c +++ b/lib/zclient.c @@ -911,8 +911,6 @@ int zapi_nexthop_encode(struct stream *s, const struct zapi_nexthop *api_nh, stream_putc(s, api_nh->bh_type); break; case NEXTHOP_TYPE_IPV4: - stream_put_in_addr(s, &api_nh->gate.ipv4); - break; case NEXTHOP_TYPE_IPV4_IFINDEX: stream_put_in_addr(s, &api_nh->gate.ipv4); stream_putl(s, api_nh->ifindex); @@ -921,9 +919,6 @@ int zapi_nexthop_encode(struct stream *s, const struct zapi_nexthop *api_nh, stream_putl(s, api_nh->ifindex); break; case NEXTHOP_TYPE_IPV6: - stream_write(s, (uint8_t *)&api_nh->gate.ipv6, - 16); - break; case NEXTHOP_TYPE_IPV6_IFINDEX: stream_write(s, (uint8_t *)&api_nh->gate.ipv6, 16); @@ -1071,9 +1066,6 @@ static int zapi_nexthop_decode(struct stream *s, struct zapi_nexthop *api_nh, STREAM_GETC(s, api_nh->bh_type); break; case NEXTHOP_TYPE_IPV4: - STREAM_GET(&api_nh->gate.ipv4.s_addr, s, - IPV4_MAX_BYTELEN); - break; case NEXTHOP_TYPE_IPV4_IFINDEX: STREAM_GET(&api_nh->gate.ipv4.s_addr, s, IPV4_MAX_BYTELEN); @@ -1083,8 +1075,6 @@ static int zapi_nexthop_decode(struct stream *s, struct zapi_nexthop *api_nh, STREAM_GETL(s, api_nh->ifindex); break; case NEXTHOP_TYPE_IPV6: - STREAM_GET(&api_nh->gate.ipv6, s, 16); - break; case NEXTHOP_TYPE_IPV6_IFINDEX: STREAM_GET(&api_nh->gate.ipv6, s, 16); STREAM_GETL(s, api_nh->ifindex); From 12b76399a87ebe58689bac14aa71f0963a051651 Mon Sep 17 00:00:00 2001 From: Donald Sharp Date: Sat, 18 Jan 2020 09:16:10 -0500 Subject: [PATCH 2/2] tests: Add another router to the basic pim tests Add an additional router to the basic pim tests. 1) This test will add a link between r1 and a new rp 2) This test will ensure that r1 and rp have the expected who is the rp. 3) This test will ensure that the rp has received the upstream data for the multicast stream that is started. Ostensibly commit 68a02e06e5f103048d947262c08c569056f74d1c is the first bad commit commit 68a02e06e5f103048d947262c08c569056f74d1c Author: Mark Stapp Date: Wed Nov 13 16:06:06 2019 -0500 *: revise zapi nexthop encoding Use a per-nexthop flag to indicate the presence of labels; add some utility zapi encode/decode apis for nexthops; use the zapi apis more consistently. Signed-off-by: Mark Stapp Sparked this commit in that it broke nexthop reporting to upper level protocols. Ensure that this expectation stays working in the future. Signed-off-by: Donald Sharp --- tests/topotests/pim-basic/r1/bgpd.conf | 3 ++ tests/topotests/pim-basic/r1/pimd.conf | 9 ++-- tests/topotests/pim-basic/r1/rp-info.json | 9 ++++ tests/topotests/pim-basic/r1/zebra.conf | 3 ++ tests/topotests/pim-basic/rp/bgpd.conf | 3 ++ tests/topotests/pim-basic/rp/pimd.conf | 9 ++++ tests/topotests/pim-basic/rp/upstream.json | 17 +++++++ tests/topotests/pim-basic/rp/zebra.conf | 8 ++++ tests/topotests/pim-basic/test_pim.py | 54 ++++++++++++++++++++++ 9 files changed, 112 insertions(+), 3 deletions(-) create mode 100644 tests/topotests/pim-basic/r1/bgpd.conf create mode 100644 tests/topotests/pim-basic/r1/rp-info.json create mode 100644 tests/topotests/pim-basic/rp/bgpd.conf create mode 100644 tests/topotests/pim-basic/rp/pimd.conf create mode 100644 tests/topotests/pim-basic/rp/upstream.json create mode 100644 tests/topotests/pim-basic/rp/zebra.conf diff --git a/tests/topotests/pim-basic/r1/bgpd.conf b/tests/topotests/pim-basic/r1/bgpd.conf new file mode 100644 index 0000000000..8acaac96a0 --- /dev/null +++ b/tests/topotests/pim-basic/r1/bgpd.conf @@ -0,0 +1,3 @@ +router bgp 65001 + neighbor 10.0.30.3 remote-as external + redistribute connected diff --git a/tests/topotests/pim-basic/r1/pimd.conf b/tests/topotests/pim-basic/r1/pimd.conf index 5740c66e24..cec765699d 100644 --- a/tests/topotests/pim-basic/r1/pimd.conf +++ b/tests/topotests/pim-basic/r1/pimd.conf @@ -2,9 +2,12 @@ hostname r1 ! interface r1-eth0 ip igmp - ip pim sm + ip pim +! +interface r1-eth1 + ip pim ! interface lo - ip pim sm + ip pim ! -ip pim rp 10.254.0.1 +ip pim rp 10.254.0.3 diff --git a/tests/topotests/pim-basic/r1/rp-info.json b/tests/topotests/pim-basic/r1/rp-info.json new file mode 100644 index 0000000000..1f713c2d28 --- /dev/null +++ b/tests/topotests/pim-basic/r1/rp-info.json @@ -0,0 +1,9 @@ +{ + "10.254.0.3":[ + { + "outboundInterface":"r1-eth1", + "group":"224.0.0.0\/4", + "source":"Static" + } + ] +} diff --git a/tests/topotests/pim-basic/r1/zebra.conf b/tests/topotests/pim-basic/r1/zebra.conf index 2bf71294d0..b0a25f12aa 100644 --- a/tests/topotests/pim-basic/r1/zebra.conf +++ b/tests/topotests/pim-basic/r1/zebra.conf @@ -3,6 +3,9 @@ hostname r1 interface r1-eth0 ip address 10.0.20.1/24 ! +interface r1-eth1 + ip address 10.0.30.1/24 +! interface lo ip address 10.254.0.1/32 ! diff --git a/tests/topotests/pim-basic/rp/bgpd.conf b/tests/topotests/pim-basic/rp/bgpd.conf new file mode 100644 index 0000000000..6b16c067a5 --- /dev/null +++ b/tests/topotests/pim-basic/rp/bgpd.conf @@ -0,0 +1,3 @@ +router bgp 65003 + neighbor 10.0.30.1 remote-as external + redistribute connected diff --git a/tests/topotests/pim-basic/rp/pimd.conf b/tests/topotests/pim-basic/rp/pimd.conf new file mode 100644 index 0000000000..3f1b4d65c9 --- /dev/null +++ b/tests/topotests/pim-basic/rp/pimd.conf @@ -0,0 +1,9 @@ +hostname rp +! +interface rp-eth0 + ip pim +! +interface lo + ip pim +! +ip pim rp 10.254.0.3 diff --git a/tests/topotests/pim-basic/rp/upstream.json b/tests/topotests/pim-basic/rp/upstream.json new file mode 100644 index 0000000000..c33dea49e9 --- /dev/null +++ b/tests/topotests/pim-basic/rp/upstream.json @@ -0,0 +1,17 @@ +{ + "229.1.1.1":{ + "10.0.20.2":{ + "sourceStream":true, + "inboundInterface":"rp-eth0", + "rpfAddress":"10.0.20.2", + "source":"10.0.20.2", + "group":"229.1.1.1", + "state":"NotJ", + "joinState":"NotJoined", + "regState":"RegNoInfo", + "resetTimer":"--:--:--", + "refCount":1, + "sptBit":0 + } + } +} diff --git a/tests/topotests/pim-basic/rp/zebra.conf b/tests/topotests/pim-basic/rp/zebra.conf new file mode 100644 index 0000000000..0a1359ecd0 --- /dev/null +++ b/tests/topotests/pim-basic/rp/zebra.conf @@ -0,0 +1,8 @@ +hostname rp +! +interface rp-eth0 + ip address 10.0.30.3/24 +! +interface lo + ip address 10.254.0.3/32 +! diff --git a/tests/topotests/pim-basic/test_pim.py b/tests/topotests/pim-basic/test_pim.py index 6d54b8f2f0..0e0569e234 100644 --- a/tests/topotests/pim-basic/test_pim.py +++ b/tests/topotests/pim-basic/test_pim.py @@ -28,6 +28,8 @@ test_pim.py: Test pim import os import sys import pytest +import json +from functools import partial CWD = os.path.dirname(os.path.realpath(__file__)) sys.path.append(os.path.join(CWD, '../')) @@ -47,11 +49,27 @@ class PIMTopo(Topo): for routern in range(1, 3): tgen.add_router('r{}'.format(routern)) + tgen.add_router('rp') + + # r1 -> .1 + # r2 -> .2 + # rp -> .3 + # loopback network is 10.254.0.X/32 + # # r1 <- sw1 -> r2 + # r1-eth0 <-> r2-eth0 + # 10.0.20.0/24 sw = tgen.add_switch('sw1') sw.add_link(tgen.gears['r1']) sw.add_link(tgen.gears['r2']) + # r1 <- sw2 -> rp + # r1-eth1 <-> rp-eth0 + # 10.0.30.0/24 + sw = tgen.add_switch('sw2') + sw.add_link(tgen.gears['r1']) + sw.add_link(tgen.gears['rp']) + def setup_module(mod): "Sets up the pytest environment" @@ -68,9 +86,14 @@ def setup_module(mod): TopoRouter.RD_PIM, os.path.join(CWD, '{}/pimd.conf'.format(rname)) ) + router.load_config( + TopoRouter.RD_BGP, + os.path.join(CWD, '{}/bgpd.conf'.format(rname)) + ) # After loading the configurations, this function loads configured daemons. tgen.start_router() + #tgen.mininet_cli() def teardown_module(mod): @@ -80,6 +103,22 @@ def teardown_module(mod): # This function tears down the whole topology. tgen.stop_topology() +def test_pim_rp_setup(): + "Ensure basic routing has come up and the rp has an outgoing interface" + #Ensure rp and r1 establish pim neighbor ship and bgp has come up + #Finally ensure that the rp has an outgoing interface on r1 + tgen = get_topogen() + + r1 = tgen.gears['r1'] + json_file = '{}/{}/rp-info.json'.format(CWD, r1.name) + expected = json.loads(open(json_file).read()) + + test_func = partial(topotest.router_json_cmp, + r1, 'show ip pim rp-info json', expected) + _, result = topotest.run_and_expect(test_func, None, count=15, wait=5) + assertmsg = '"{}" JSON output mismatches'.format(r1.name) + assert result is None, assertmsg + #tgen.mininet_cli() def test_pim_send_mcast_stream(): "Establish a Multicast stream from r2 -> r1 and then ensure S,G is created as appropriate" @@ -90,6 +129,7 @@ def test_pim_send_mcast_stream(): if tgen.routers_have_failure(): pytest.skip(tgen.errors) + rp = tgen.gears['rp'] r2 = tgen.gears['r2'] r1 = tgen.gears['r1'] @@ -111,7 +151,21 @@ def test_pim_send_mcast_stream(): } assert topotest.json_cmp(out, expected) is None, 'failed to converge pim' + #tgen.mininet_cli() +def test_pim_rp_sees_stream(): + "Ensure that the RP sees the stream and has acted accordingly" + tgen = get_topogen() + + rp = tgen.gears['rp'] + json_file = '{}/{}/upstream.json'.format(CWD, rp.name) + expected = json.loads(open(json_file).read()) + + test_func = partial(topotest.router_json_cmp, + rp, 'show ip pim upstream json', expected) + _, result = topotest.run_and_expect(test_func, None, count=20, wait=.5) + assertmsg = '"{}" JSON output mismatches'.format(rp.name) + assert result is None, assertmsg def test_pim_igmp_report(): "Send a igmp report from r2->r1 and ensure that the *,G state is created on r1"