Skip to content
This repository was archived by the owner on Aug 15, 2025. It is now read-only.

T7658: Fix default max-map-count and add smoketest for this parameter#54

Closed
natali-rs1985 wants to merge 1 commit intovyos-legacy:currentfrom
natali-rs1985:T7658-fix-default-max_map_count
Closed

T7658: Fix default max-map-count and add smoketest for this parameter#54
natali-rs1985 wants to merge 1 commit intovyos-legacy:currentfrom
natali-rs1985:T7658-fix-default-max_map_count

Conversation

@natali-rs1985
Copy link
Contributor

@natali-rs1985 natali-rs1985 commented Jul 25, 2025

In the code we have the restriction not to decrease values in host-resources section. We need to skip this smoketest for now until we remove the restriction because it will always fail

Change Summary

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Code style update (formatting, renaming)
  • Refactoring (no functional changes)
  • Other (please describe):

Related Task(s)

Related PR(s)

Proposed changes

How to test

vyos@vyos#  /usr/libexec/vyos/tests/smoke/cli/test_vpp.py
test_01_vpp_basic (__main__.TestVPP.test_01_vpp_basic) ... ok
test_02_vpp_vxlan (__main__.TestVPP.test_02_vpp_vxlan) ... ok
test_03_vpp_gre (__main__.TestVPP.test_03_vpp_gre) ... ok
test_04_vpp_geneve (__main__.TestVPP.test_04_vpp_geneve) ... skipped 'Skipping this test geneve index always is 0'
test_05_vpp_loopback (__main__.TestVPP.test_05_vpp_loopback) ... ok
test_06_vpp_bonding (__main__.TestVPP.test_06_vpp_bonding) ... skipped 'Skipping temporary bonding, sometimes get recursion T7117'
test_07_vpp_bridge (__main__.TestVPP.test_07_vpp_bridge) ... ok
test_08_vpp_ipip (__main__.TestVPP.test_08_vpp_ipip) ... ok
test_09_vpp_xconnect (__main__.TestVPP.test_09_vpp_xconnect) ... ok
test_10_vpp_driver_options (__main__.TestVPP.test_10_vpp_driver_options) ... ok
test_11_vpp_cpu_settings (__main__.TestVPP.test_11_vpp_cpu_settings) ... ok
test_12_vpp_cpu_corelist_workers (__main__.TestVPP.test_12_vpp_cpu_corelist_workers) ... ok
test_13_1_buffer_page_size (__main__.TestVPP.test_13_1_buffer_page_size) ... ok
test_13_2_statseg_page_size (__main__.TestVPP.test_13_2_statseg_page_size) ... ok
test_13_3_mem_page_size (__main__.TestVPP.test_13_3_mem_page_size) ... skipped 'Skipping'
test_14_mem_default_hugepage (__main__.TestVPP.test_14_mem_default_hugepage) ... ok
test_15_vpp_ipsec_xfrm_nl (__main__.TestVPP.test_15_vpp_ipsec_xfrm_nl) ... ok
test_16_vpp_cgnat (__main__.TestVPP.test_16_vpp_cgnat) ... ok
test_17_vpp_nat (__main__.TestVPP.test_17_vpp_nat) ... ok
test_18_vpp_host_resources (__main__.TestVPP.test_18_vpp_host_resources) ... ok

----------------------------------------------------------------------
Ran 20 tests in 579.954s

OK (skipped=3)

Checklist:

  • I have read the CONTRIBUTING document
  • I have linked this PR to one or more Phabricator Task(s)
  • My commit headlines contain a valid Task id
  • My change requires a change to the documentation
  • I have updated the documentation accordingly

Copy link
Contributor

@sever-sever sever-sever left a comment

Choose a reason for hiding this comment

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

Please add a smoketest to check this feature and actual settings of the sysctl.
As it wasn't working for months.

@natali-rs1985 natali-rs1985 force-pushed the T7658-fix-default-max_map_count branch from 8d7d9fa to 0810db8 Compare July 25, 2025 14:13
@natali-rs1985
Copy link
Contributor Author

Please add a smoketest to check this feature and actual settings of the sysctl. As it wasn't working for months.

Added smoketest

@natali-rs1985 natali-rs1985 changed the title T7658: Fix default max-map-count T7658: Fix default max-map-count and add smoketest this parameter Jul 25, 2025
Copy link
Contributor

@dmbaturin dmbaturin left a comment

Choose a reason for hiding this comment

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

The change is sensible and there's a test, no objections.

Comment on lines 1411 to 1414
self.cli_delete(hr_path + ['max-map-count'])
self.cli_commit()

self.assertEqual(sysctl_read('vm.max_map_count'), max_map_count)
Copy link
Contributor

Choose a reason for hiding this comment

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

If we delete the max-map-count option, the expected default value is 65530 and not 100_000

@natali-rs1985 natali-rs1985 force-pushed the T7658-fix-default-max_map_count branch from 0810db8 to f0708b2 Compare July 28, 2025 08:54
@natali-rs1985 natali-rs1985 changed the title T7658: Fix default max-map-count and add smoketest this parameter T7658: Fix default max-map-count and add smoketest for this parameter Jul 28, 2025
In the code we have the restriction not to decrease values in host-resources section. We need to skip this smoketest for now until we remove the restriction because it will always fail
@natali-rs1985 natali-rs1985 force-pushed the T7658-fix-default-max_map_count branch from f0708b2 to e5ff51a Compare July 28, 2025 08:56
@natali-rs1985
Copy link
Contributor Author

Close this PR due to moving functionality to "system option host-resources"
#57
vyos/vyos-1x#4647

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants