-
Notifications
You must be signed in to change notification settings - Fork 524
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
Add missing neighbor resolution for MPLS route programming #1968
Changes from all commits
d6141c4
c819ef8
0922d83
7029c74
733c84d
83f70a1
bd9d0ae
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -598,46 +598,84 @@ def test_RouteAddRemoveMplsRouteMixedNHG(self, dvs, testlog): | |
|
||
self.teardown_mpls(dvs) | ||
|
||
def test_RouteUnresolvedMplsRouteSwap(self, dvs, testlog): | ||
self.setup_mpls(dvs, False) | ||
def test_RouteAddRemoveMplsRouteResolveNeigh(self, dvs, testlog): | ||
if not self.mpls_appdb_mode(): | ||
dvs.runcmd("modprobe mpls_router") | ||
dvs.runcmd("modprobe mpls_iptunnel") | ||
dvs.runcmd("sysctl -w net.mpls.platform_labels=1000") | ||
|
||
# add route entry | ||
label = "200" | ||
self.setup_db(dvs) | ||
|
||
self.clear_srv_config(dvs) | ||
|
||
# create mpls interface | ||
self.create_mpls_intf("Ethernet0") | ||
self.create_mpls_intf("Ethernet4") | ||
|
||
# set ip address | ||
self.add_ip_address("Ethernet0", "10.0.0.0/31") | ||
self.add_ip_address("Ethernet4", "10.0.0.2/31") | ||
|
||
# bring up interface | ||
self.set_admin_status("Ethernet0", "up") | ||
self.set_admin_status("Ethernet4", "up") | ||
|
||
# set ip address and default route | ||
dvs.servers[0].runcmd("ip address add 10.0.0.1/31 dev eth0") | ||
dvs.servers[0].runcmd("ip route add default via 10.0.0.0") | ||
|
||
dvs.servers[1].runcmd("ip address add 10.0.0.3/31 dev eth0") | ||
dvs.servers[1].runcmd("ip route add default via 10.0.0.2") | ||
time.sleep(2) | ||
|
||
# add route entries. The neighbor entries for 10.0.0.1 and 10.0.0.3 are not yet resolved, so will trigger an ARP request | ||
label = "100" | ||
label2 = "200" | ||
if self.mpls_appdb_mode(): | ||
fieldValues = {"nexthop": "10.0.0.1", "ifname": "Ethernet0", "mpls_nh": "swap201", "mpls_pop": "1"} | ||
fieldValues = {"nexthop": "10.0.0.1", "ifname": "Ethernet0", "mpls_nh": "swap101", "mpls_pop": "1"} | ||
self.create_inseg_entry(label, fieldValues) | ||
fieldValues = {"nexthop": "10.0.0.1,10.0.0.3", "ifname": "Ethernet0,Ethernet4", "mpls_nh": "swap201,swap202", "mpls_pop": "1"} | ||
self.create_inseg_entry(label2, fieldValues) | ||
else: | ||
# dvs.runcmd("ip -f mpls route add 200 as 201 via inet 10.0.0.1 dev Ethernet0") | ||
dvs.runcmd("vtysh -c \"configure terminal\" -c \"mpls lsp 200 10.0.0.1 201\"") | ||
dvs.runcmd("ip -f mpls route add 100 nexthop as 101 via inet 10.0.0.1 dev Ethernet0") | ||
dvs.runcmd("ip -f mpls route add 200 nexthop as 201 via inet 10.0.0.1 dev Ethernet0 nexthop as 202 via inet 10.0.0.3 dev Ethernet4") | ||
|
||
# check application database | ||
self.pdb.wait_for_entry("LABEL_ROUTE_TABLE", label) | ||
self.pdb.wait_for_entry("LABEL_ROUTE_TABLE", label2) | ||
|
||
# check ASIC inseg database. inseg with unresolved NH should not be present. | ||
self.check_inseg_entries(False, [label]) | ||
self.check_nexthop(False, "SAI_NEXT_HOP_TYPE_MPLS", "10.0.0.1", "SAI_OUTSEG_TYPE_SWAP", "1:201") | ||
|
||
# now resolve the NH | ||
dvs.servers[0].runcmd("ping -c 1 10.0.0.3") | ||
dvs.servers[2].runcmd("ping -c 1 10.0.0.3") | ||
# check neighbor got resolved and removed from NEIGH_RESOLVE_TABLE | ||
self.pdb.wait_for_deleted_entry("NEIGH_RESOLVE_TABLE", "Ethernet0:10.0.0.1") | ||
self.pdb.wait_for_deleted_entry("NEIGH_RESOLVE_TABLE", "Ethernet4:10.0.0.3") | ||
|
||
# check ASIC inseg database | ||
self.check_inseg_entries(True, [label]) | ||
self.check_inseg_nexthop(label, "SAI_NEXT_HOP_TYPE_MPLS", "10.0.0.1", "SAI_OUTSEG_TYPE_SWAP", "1:201") | ||
self.check_inseg_nexthop(label, "SAI_NEXT_HOP_TYPE_MPLS", "10.0.0.1", "SAI_OUTSEG_TYPE_SWAP", "1:101") | ||
self.check_inseg_entries(True, [label2]) | ||
self.check_inseg_nexthop_group(label2, 2) | ||
self.check_nexthop(True, "SAI_NEXT_HOP_TYPE_MPLS", "10.0.0.1", "SAI_OUTSEG_TYPE_SWAP", "1:201") | ||
self.check_nexthop(True, "SAI_NEXT_HOP_TYPE_MPLS", "10.0.0.3", "SAI_OUTSEG_TYPE_SWAP", "1:202") | ||
nhg = self.get_inseg_nexthop(label) | ||
|
||
# remove route entry | ||
# remove inseg entry | ||
if self.mpls_appdb_mode(): | ||
self.remove_inseg_entry(label) | ||
self.remove_inseg_entry(label2) | ||
else: | ||
# dvs.runcmd("ip -f mpls route del 200 as 201 via inet 10.0.0.1 dev Ethernet0") | ||
dvs.runcmd("vtysh -c \"configure terminal\" -c \"no mpls lsp 200 10.0.0.1 201\"") | ||
dvs.runcmd("ip -f mpls route del 100 nexthop as 101 via inet 10.0.0.1 dev Ethernet0") | ||
dvs.runcmd("ip -f mpls route del 200 nexthop as 201 via inet 10.0.0.1 dev Ethernet0 nexthop as 202 via inet 10.0.0.3 dev Ethernet4") | ||
|
||
# check application database | ||
self.pdb.wait_for_deleted_entry("LABEL_ROUTE_TABLE", label) | ||
self.pdb.wait_for_deleted_entry("LABEL_ROUTE_TABLE", label2) | ||
|
||
# check ASIC inseg database | ||
# check ASIC route database | ||
self.check_inseg_entries(False, [label]) | ||
self.check_inseg_entries(False, [label2]) | ||
self.check_nexthop_group(False, nhg) | ||
self.check_nexthop(False, "SAI_NEXT_HOP_TYPE_MPLS", "10.0.0.1", "SAI_OUTSEG_TYPE_SWAP", "1:101") | ||
self.check_nexthop(False, "SAI_NEXT_HOP_TYPE_MPLS", "10.0.0.1", "SAI_OUTSEG_TYPE_SWAP", "1:201") | ||
self.check_nexthop(False, "SAI_NEXT_HOP_TYPE_MPLS", "10.0.0.3", "SAI_OUTSEG_TYPE_SWAP", "1:202") | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. the test should indicate pass or fail. I don't see that here. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not sure what you mean here. This finishes the same way as every other test, and I can't see any return codes in other test suites. What should I be adding here? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You should add 'asserts' for whatever condition that is expected for the test to claim pass. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The assert is within the wait_for_result function called by each of the check_ functions (e.g. check_nexthop). This is standard across many of the test suites where it's waiting for programming to be done by orchagent - and I've seen many times that it does produce an assert if the programming doesn't reach the expected state in time. |
||
self.teardown_mpls(dvs) | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
which neighbor is unresolved here? I want to know how the new code in mplsrouteorch.cpp:529 getting exercised.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've extended the comment above this