Enable eswitch mode setting on SmartNICs by slyon · Pull Request #253 · canonical/netplan

slyon · 2022-01-10T13:52:31Z

Description

Certain types of network cards/SmartNICs allow toggling their low-level (devlink) embedded switch mode PCI setting in order to utilize the full hardware offloading capabilities. With this PR we're exposing this functionality to the netplan YAML schema:

embedded-switch-mode: [switchdev|legacy]
- choose the devlink setting for the embedded switch (eswitch)
- sticks to the hardware's preset by default
delay-virtual-functions-rebind: [false|true]
- False by default: SR-IOV virtual functions are re-bind to their driver right after setting the eswitch mode
- Can be delayed to a later stage during the boot process; useful when OVS/OVN bonding (VF LAG) is to be used

Example:

network:
  version: 2
  ethernets:
    engreen:
      embedded-switch-mode: "switchdev"
      delay-virtual-functions-rebind: true
    enblue:
      match:
        driver: "mlx5_core"
      set-name: enblue
      embedded-switch-mode: "legacy"
      virtual-function-count: 4
    sriov_vf0:
      link: engreen

This PR builds upon the new libnetplan YAML parser (#250) for accessing the new settings.

Checklist

Runs make check successfully.
Retains 100% code coverage (make check-coverage).
New/changed keys in YAML format are documented.
(Optional) Adds example YAML for new feature.
(Optional) Closes an open bug in Launchpad.

codecov-commenter · 2022-01-12T10:30:11Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 99.16%. Comparing base (ed7550d) to head (34c4da9).
Report is 676 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #253      +/-   ##
==========================================
+ Coverage   99.14%   99.16%   +0.02%     
==========================================
  Files          60       61       +1     
  Lines       10511    10818     +307     
==========================================
+ Hits        10421    10728     +307     
  Misses         90       90

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

netplan/cli/sriov.py

doc/netplan.md

slyon · 2022-02-07T11:17:36Z

We got some positive feedback from Canonical's OpenStack team about the new functionality, especially that the VF LAG (bonding of SR-IOV physical functions) is working as expected.

slyon · 2022-02-07T14:56:55Z

autopkgtest [14:41:28]: @@@@@@@@@@@@@@@@@@@@ summary
ovs                  PASS
ethernets            PASS
bridges              PASS
bonds                PASS
routing              PASS
vlans                PASS
wifi                 PASS
tunnels              PASS
scenarios            PASS
regressions          PASS
autostart            PASS
cloud-init           PASS

schopin-pro

This PR is in great shape, and the code looks overall solid 🙂

I have a few items, most of them non-blocking, but there's one in particular regarding potential ABI break that needs answering before merging.

tests/test_sriov.py

schopin-pro · 2022-02-08T16:54:49Z

src/netplan.c

        YAML_STRING(def, event, emitter, "link", def->sriov_link->id);
    YAML_UINT_DEFAULT(def, event, emitter, "virtual-function-count", def->sriov_explicit_vf_count, G_MAXUINT);
+    YAML_STRING(def, event, emitter, "embedded-switch-mode", def->embedded_switch_mode);
+    YAML_BOOL_TRUE(def, event, emitter, "delay-virtual-functions-rebind",


thought (later-pr): the YAML_BOOL_TRUE name has the opposite semantics to YAML_UINT_0. I should fix that, I guess?

src/parse.c

netplan/cli/sriov.py

src/sriov.h

src/generate.c

tests/generator/base.py

netplan/cli/sriov.py

…F/VF mocks dynamically

…ns-rebind'

Taking lots of (especially the PCIDevice class) from: https://github.com/openstack-charmers/mlnx-switchdev-mode/blob/master/mlnx_switchdev_mode/sriovify.py V2: update copyright notice in comment of PCIDevice class

V2: avoid "# pragma: nocover" by having relevant test cases

slyon · 2022-02-09T15:34:55Z

Integration tests are still good, even if the CI is doing funny things by skipping most (all?) tests, due to some package dependency issue. I re-ran it locally:

autopkgtest [16:07:51]: @@@@@@@@@@@@@@@@@@@@ summary
ovs                  PASS
ethernets            PASS
bridges              PASS
bonds                PASS
routing              PASS
vlans                PASS
wifi                 PASS
tunnels              PASS
scenarios            PASS
regressions          PASS
autostart            PASS
cloud-init           PASS

src/generate.c

netplan/cli/sriov.py

schopin-pro

LGTM!
I have a small suggestion for the new API, but feel free to merge if you don't want to pick it up.

V2: * simplify self.assert_sriov() in generator/base.py * add new netplan_state_finish_sriov_write() and netplan_sriov_cleanup() API * move SR-IOV rebind service generation into netplan_state_finish_sriov_write() V3: * move 'any_sriov' udev rules logic into netplan_state_finish_sriov_write()

slyon · 2022-02-09T17:18:11Z

Thanks for the re-review! Turned out the write_sriov_conf_finish() and cleanup_sriov_conf() functions have been part of the generate binary itself, so it didn't break ABI in the first place. But still it's nicer to have all this handling of SR-IOV logic in the sriov module and call into the library in future version.

I will be merging this.

dshcherb · 2022-02-09T17:59:05Z

Thanks a lot for this, this is a much needed feature to utilize SR-IOV 👍

In case setting num_vfs needs to be done via systemd in the future, there was some recent work on that too:

systemd/systemd#21865
systemd/systemd@41ce9d7

slyon · 2022-02-09T18:08:11Z

In case setting num_vfs needs to be done via systemd in the future, there was some recent work on that too:

Nice! That's good to know. We should probably switch to that upstream solution in the future, once it lands in stable series (maybe in parallel to our manual way of doing it).

slyon added the schema change This PR includes a change to netplan's YAML schema, which needs sign-off label Jan 10, 2022

slyon force-pushed the slyon/mlnx-switchdev branch 2 times, most recently from 33c1468 to 9d69bb0 Compare January 12, 2022 10:30

slyon force-pushed the slyon/mlnx-switchdev branch 3 times, most recently from e836b9c to 46ab0ba Compare January 13, 2022 10:36

slyon commented Jan 13, 2022

View reviewed changes

netplan/cli/sriov.py Show resolved Hide resolved

slyon force-pushed the slyon/mlnx-switchdev branch 2 times, most recently from 13d9f33 to 1ba1e1c Compare January 13, 2022 11:17

slyon force-pushed the slyon/mlnx-switchdev branch 6 times, most recently from bbb49e2 to 3e73abb Compare January 27, 2022 15:04

slyon requested a review from vorlonofportland January 31, 2022 17:11

vorlonofportland reviewed Feb 1, 2022

View reviewed changes

doc/netplan.md Show resolved Hide resolved

doc/netplan.md Outdated Show resolved Hide resolved

slyon force-pushed the slyon/mlnx-switchdev branch from 3e73abb to fba8f88 Compare February 2, 2022 09:09

slyon added the schema ok label Feb 2, 2022

slyon force-pushed the slyon/mlnx-switchdev branch 6 times, most recently from 9f1cdb6 to 8db3301 Compare February 2, 2022 11:41

slyon changed the title ~~Enable eswitch mode setting for certain devices~~ Enable eswitch mode setting on SmartNICs Feb 2, 2022

slyon marked this pull request as ready for review February 2, 2022 12:05

slyon requested a review from schopin-pro February 3, 2022 16:22

schopin-pro reviewed Feb 8, 2022

View reviewed changes

slyon added 9 commits February 9, 2022 11:58

test:sriov: virtfns should be starting at index 0

8e393cb

test:sriov: update _prepare_sysfs_dir_structure() to make it create P…

b5bae1c

…F/VF mocks dynamically

doc: add sriov eswitch mode field descriptions

b020e8a

parse:netplan: handle 'embedded-switch-mode' & 'delay-virtual-functio…

c6183de

…ns-rebind'

test:sriov: fix tests when using libnetplan parser

275647f

V2: avoid "# pragma: nocover" by having relevant test cases

test:sriov: add get_pci_slot_name & PCIDevice unit tests

e1e5103

test:sriov: add eswitch_mode test

35a13eb

cli: add sriov rebind method

c19fe2c

slyon force-pushed the slyon/mlnx-switchdev branch from 8db3301 to 9c99162 Compare February 9, 2022 14:04

slyon requested a review from schopin-pro February 9, 2022 14:12

schopin-pro reviewed Feb 9, 2022

View reviewed changes

src/generate.c Outdated Show resolved Hide resolved

netplan/cli/sriov.py Show resolved Hide resolved

schopin-pro approved these changes Feb 9, 2022

View reviewed changes

slyon force-pushed the slyon/mlnx-switchdev branch from 9c99162 to 34c4da9 Compare February 9, 2022 17:12

slyon merged commit ef56b3a into main Feb 9, 2022

slyon deleted the slyon/mlnx-switchdev branch February 9, 2022 17:22

slyon mentioned this pull request Jan 25, 2024

cli/sriov: remove unused code #435

Merged

5 tasks

Conversation

slyon commented Jan 10, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Checklist

Uh oh!

codecov-commenter commented Jan 12, 2022 • edited by codecov bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Uh oh!

Uh oh!

slyon commented Feb 7, 2022

Uh oh!

slyon commented Feb 7, 2022

Uh oh!

schopin-pro left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

schopin-pro Feb 8, 2022

Choose a reason for hiding this comment

Uh oh!

slyon Feb 9, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

slyon commented Feb 9, 2022

Uh oh!

Uh oh!

Uh oh!

schopin-pro left a comment

Choose a reason for hiding this comment

Uh oh!

slyon commented Feb 9, 2022

Uh oh!

dshcherb commented Feb 9, 2022

Uh oh!

slyon commented Feb 9, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

slyon commented Jan 10, 2022 •

edited

Loading

codecov-commenter commented Jan 12, 2022 •

edited by codecov bot

Loading