Skip to content

Conversation

natali-rs1985
Copy link
Contributor

@natali-rs1985 natali-rs1985 commented Oct 8, 2025

Remove no-syscall-lock from CLI and enable this option if workers are configured.
Also forbid interrupt mode for ixgbevf driver

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)
  • Migration from an old Vyatta component to vyos-1x, please link to related PR inside obsoleted component
  • Other (please describe):

Related Task(s)

Related PR(s)

vyos/vyos-documentation#1691

How to test / Smoketest result

set vpp settings cpu main-core '1'
set vpp settings interface eth1 driver 'xdp'
set vpp settings interface eth1 rx-mode 'interrupt'

set vpp settings cpu workers '1'

vyos@vyos# commit
[edit]
vyos@vyos# sudo vppctl show hardware-interfaces
              Name                Idx   Link  Hardware
eth1                               1     up   eth1
  Link speed: unknown
  RX Queues:
    queue thread         mode
    0     main (0)       interrupt
  TX Queues:
    TX Hash: [name: hash-eth-l34 priority: 50 description: Hash ethernet L34 headers]
    queue shared thread(s)
    0     no     0
  Ethernet address 0c:f2:f4:cc:00:01
  netdev defunct_eth1
  flags: admin-up
...

Checklist:

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

Copy link

github-actions bot commented Oct 8, 2025

👍
No issues in PR Title / Commit Title

Copy link
Member

@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.

Sorry, mis-click.

Copy link
Member

@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.

I'm a bit confused by this PR. It has four distinct parts:

  1. A fix for a typo in the flag name, s/no_systcall_lock/no_syscall_log/ — no objections here.
  2. A fix for the internal flag name for xdp-options, s/no-syscall-lock/zero-copy/ — probably also fine.
  3. A complete removal of ixgbevf from the list of drivers that support the interrupt mode — probably fine, if it doesn't.
  4. A removal of the no-syscall-lock option from the CLI.

I don't understand the point #4 — if there's no option anymore, where would the no_syscall_lock flag be coming from? Also, a while ago we agreed that VPP is no longer experimental and config syntax changes now require migration scripts, if they are necessary.

@natali-rs1985
Copy link
Contributor Author

I don't understand the point #4 — if there's no option anymore, where would the no_syscall_lock flag be coming from? Also, a while ago we agreed that VPP is no longer experimental and config syntax changes now require migration scripts, if they are necessary.

We decided to remove this option from the CLI because it must be used with interrupt/adaptive mode with vpp workers (if it doesn't vpp fails to start). If this conditions are met we set this flag automatically. In other cases it is not needed

I'll add a migration script for this option

Copy link
Member

@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.

Remove no-syscall-lock for config
Use this flag internally

Remove no-syscall-lock from CLI and enable this option if interrupt/adaptive mode is enabled and workers are configured.
Also forbid interrupt mode for ixgbevf driver
Copy link
Contributor

@zdc zdc left a comment

Choose a reason for hiding this comment

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

The code looks logical to me. I have not tested it.

Copy link

CI integration ❌ failed!

Details

CI logs

  • CLI Smoketests (no interfaces) 👍 passed
  • CLI Smoketests VPP 👍 passed
  • CLI Smoketests (interfaces only) ❌ failed
  • Config tests 👍 passed
  • Config tests VPP 👍 passed
  • RAID1 tests 👍 passed
  • TPM tests 👍 passed

@natali-rs1985 natali-rs1985 added the bp/circinus Create automatic backport for circinus label Oct 16, 2025
@sever-sever sever-sever merged commit d8dc670 into vyos:current Oct 16, 2025
28 of 30 checks passed
@vyosbot vyosbot added mirror-initiated This PR initiated for mirror sync workflow mirror-completed and removed mirror-initiated This PR initiated for mirror sync workflow labels Oct 16, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bp/circinus Create automatic backport for circinus current mirror-completed

Development

Successfully merging this pull request may close these issues.

5 participants