Skip to content

router: fix routing table for marked packets - #2579

Merged
yadvr merged 1 commit into
apache:4.11from
shapeblue:mangle-free-vpc-nat
Apr 20, 2018
Merged

router: fix routing table for marked packets#2579
yadvr merged 1 commit into
apache:4.11from
shapeblue:mangle-free-vpc-nat

Conversation

@yadvr

@yadvr yadvr commented Apr 18, 2018

Copy link
Copy Markdown
Member

This fixes python code that applies NAT/address rules. This fixes routing rules based on ip addresses received to ensure that marked packets will have a routing rule. Thereby, this fixes the issue of
public IP NAT access from other VMs in the VPC.

Types of changes

  • Breaking change (fix or feature that would cause existing functionality to change)
  • New feature (non-breaking change which adds functionality)
  • Bug fix (non-breaking change which fixes an issue)
  • Enhancement (improves an existing feature and functionality)
  • Cleanup (Code refactoring and cleanup, that may add test cases)

GitHub Issue/PRs

Screenshots (if appropriate):

How Has This Been Tested?

  • Create a VPC and deployed two VMs in a single tier in 10.1.1.1/24, allow_all policy (egress/ingress)
  • Added a public IP for the VPC enabled SNAT of that public IP to the VM1, similarly for VM2
  • Before the fix, I was unable to SSH from VM1 to VM2's public SNAT IP
  • After the fix ssh/ping/access via public IP works

Checklist:

  • I have read the CONTRIBUTING document.
  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
    Testing
  • I have added tests to cover my changes.
  • All relevant new and existing integration tests have passed.
  • A full integration testsuite with all test that can run on my environment has passed.

@blueorangutan package

This fixes routing table rule setup regression to correctly router
marked packets based on interface related ip route tables. This thereby
fixes the access of VMs in the same VPC using NAT/SNAT public IPs.

Signed-off-by: Rohit Yadav <rohit.yadav@shapeblue.com>
@yadvr
yadvr force-pushed the mangle-free-vpc-nat branch from d61eed0 to a20d05a Compare April 18, 2018 19:53
@yadvr yadvr changed the title router: don't mark packets permanently on SNAT router: fix routing table for marked packets Apr 18, 2018
@yadvr

yadvr commented Apr 18, 2018

Copy link
Copy Markdown
Member Author

@rafaelweingartner based on @ustcweizhou 's comment, I've updated and tested the code to add routing rules for fwmarked (MARK-ed) packets. This will fix routing issues that you observed in your env.

For additional commentary, see #2514 (comment)

@yadvr

yadvr commented Apr 18, 2018

Copy link
Copy Markdown
Member Author

@blueorangutan package

@blueorangutan

Copy link
Copy Markdown

@rhtyd a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.

@blueorangutan

Copy link
Copy Markdown

Packaging result: ✔centos6 ✔centos7 ✔debian. JID-1945

@yadvr

yadvr commented Apr 18, 2018

Copy link
Copy Markdown
Member Author

@blueorangutan test matrix

@blueorangutan

Copy link
Copy Markdown

@rhtyd a Trillian-Jenkins matrix job (centos6 mgmt + xs71, centos7 mgmt + vmware65, centos7 mgmt + kvmcentos7) has been kicked to run smoke tests

@rafaelweingartner rafaelweingartner left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@rhtyd I tested this PR again, and now everything (SNAT and normal NAT) are working!

Good job!

@yadvr

yadvr commented Apr 19, 2018

Copy link
Copy Markdown
Member Author

Great, thanks for the review and testing @rafaelweingartner

yadvr pushed a commit that referenced this pull request Apr 19, 2018
…ript (#2586)

While debugging the VR for #2579, I noticed that one of the scripts were breaking. The variable RROUTER was not set and this broke a conditional.

Signed-off-by: Rohit Yadav <rohit.yadav@shapeblue.com>
@ustcweizhou

Copy link
Copy Markdown
Contributor

nice, code looks good to me!
not tested yet.

@blueorangutan

Copy link
Copy Markdown

Trillian test result (tid-2524)
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 101681 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr2579-t2524-kvm-centos7.zip
Intermitten failure detected: /marvin/tests/smoke/test_privategw_acl.py
Intermitten failure detected: /marvin/tests/smoke/test_routers.py
Intermitten failure detected: /marvin/tests/smoke/test_host_maintenance.py
Smoke tests completed. 65 look OK, 2 have error(s)
Only failed tests results shown below:

Test Result Time (s) Test File
test_02_vpc_privategw_static_routes Failure 229.43 test_privategw_acl.py
test_03_vpc_privategw_restart_vpc_cleanup Failure 200.63 test_privategw_acl.py
test_04_restart_network_wo_cleanup Failure 4.13 test_routers.py

@blueorangutan

Copy link
Copy Markdown

Trillian test result (tid-2525)
Environment: vmware-65 (x2), Advanced Networking with Mgmt server 7
Total time taken: 120639 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr2579-t2525-vmware-65.zip
Intermitten failure detected: /marvin/tests/smoke/test_deploy_vm_root_resize.py
Intermitten failure detected: /marvin/tests/smoke/test_routers_network_ops.py
Intermitten failure detected: /marvin/tests/smoke/test_routers.py
Intermitten failure detected: /marvin/tests/smoke/test_service_offerings.py
Smoke tests completed. 64 look OK, 3 have error(s)
Only failed tests results shown below:

Test Result Time (s) Test File
test_01_RVR_Network_FW_PF_SSH_default_routes_egress_true Failure 2111.64 test_routers_network_ops.py
test_04_restart_network_wo_cleanup Failure 4.10 test_routers.py
ContextSuite context=TestCpuCapServiceOfferings>:teardown Error 0.00 test_service_offerings.py

@DaanHoogland

Copy link
Copy Markdown
Contributor

code lgtm, just wondering about the test_04_restart_network_wo_cleanup failures above. looking into the vmware one now

@blueorangutan

Copy link
Copy Markdown

Trillian test result (tid-2523)
Environment: xenserver-71 (x2), Advanced Networking with Mgmt server 6
Total time taken: 124451 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr2579-t2523-xenserver-71.zip
Intermitten failure detected: /marvin/tests/smoke/test_deploy_vm_iso.py
Intermitten failure detected: /marvin/tests/smoke/test_network.py
Intermitten failure detected: /marvin/tests/smoke/test_projects.py
Intermitten failure detected: /marvin/tests/smoke/test_routers.py
Intermitten failure detected: /marvin/tests/smoke/test_scale_vm.py
Intermitten failure detected: /marvin/tests/smoke/test_service_offerings.py
Intermitten failure detected: /marvin/tests/smoke/test_ssvm.py
Intermitten failure detected: /marvin/tests/smoke/test_vpc_redundant.py
Smoke tests completed. 61 look OK, 6 have error(s)
Only failed tests results shown below:

Test Result Time (s) Test File
test_reboot_router Error 2509.71 test_network.py
test_10_project_activation Error 3679.29 test_projects.py
test_04_restart_network_wo_cleanup Failure 6.94 test_routers.py
test_08_start_router Error 1363.18 test_routers.py
test_09_reboot_router Error 1.11 test_routers.py
test_01_scale_vm Error 23.64 test_scale_vm.py
ContextSuite context=TestCpuCapServiceOfferings>:teardown Error 0.00 test_service_offerings.py
test_05_stop_ssvm Failure 915.19 test_ssvm.py
test_06_stop_cpvm Failure 916.28 test_ssvm.py
test_08_reboot_cpvm Failure 0.03 test_ssvm.py

@DaanHoogland

Copy link
Copy Markdown
Contributor

the failure on test_04_restart_network_wo_cleanup is genuine 14 mins uptime instead of <3 mins. I am not sure that logic is valid though. I can not find a timestamp for the start of the test and that would be the real test (had it been restarted since)

@yadvr

yadvr commented Apr 20, 2018

Copy link
Copy Markdown
Member Author

@DaanHoogland I checked test_04_restart_network_wo_cleanup and others test failures are not related to this PR, I'm seeing them on 4.11 - #2376. The router related failures on xenserver are not seen but not caused by this PR.

I'll go ahead and merge this based on your reviews from Daan, Wei and Rafael and try to address test failures in a separate PR. In future, reviewers are requested to use Github's review feature that makes it easy to count/reference in a long pr.

@yadvr
yadvr merged commit 561630e into apache:4.11 Apr 20, 2018
julien-vaz pushed a commit to scclouds/cloudstack that referenced this pull request Feb 18, 2025
Correção na configuração de CD/DVD _drives_ existentes

Closes apache#2579

See merge request scclouds/scclouds!1130
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.

5 participants