Skip to content

Conversation

@ozhuraki
Copy link
Contributor

@ozhuraki ozhuraki commented Aug 30, 2021

This PR is adds support for provisioning DSA devices and workqueues with accel-config utility for intel-dsa-plugin
through initcontainer.

Closes: #658

@ozhuraki ozhuraki force-pushed the dsa-idxd-op branch 3 times, most recently from eddd733 to 37b559a Compare August 31, 2021 09:44
rojkov
rojkov previously approved these changes Aug 31, 2021
@mythi
Copy link
Contributor

mythi commented Aug 31, 2021

let's wait with this a bit still.

@codecov-commenter
Copy link

codecov-commenter commented Sep 1, 2021

Codecov Report

Merging #680 (0bcd3b6) into main (3a35a75) will not change coverage.
The diff coverage is n/a.

❗ Current head 0bcd3b6 differs from pull request most recent head 1966090. Consider uploading reports for the commit 1966090 to get more accurate results
Impacted file tree graph

@@           Coverage Diff           @@
##             main     #680   +/-   ##
=======================================
  Coverage   60.06%   60.06%           
=======================================
  Files          32       32           
  Lines        2782     2782           
=======================================
  Hits         1671     1671           
  Misses       1028     1028           
  Partials       83       83           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3a35a75...1966090. Read the comment docs.

@ozhuraki
Copy link
Contributor Author

ozhuraki commented Sep 1, 2021

@mythi

Added a license to DSA initcontainer Dockerfile. Will fix everything as you suggested, thanks for the input.

@ozhuraki
Copy link
Contributor Author

ozhuraki commented Sep 1, 2021

@mythi

  • Switched initcontainer to debian:buster-slim
  • Added checksum for accel-config archive
  • Fixed libaccel-config.so symlink issue
  • Added InitImage validation to dsadeviceplugin_webhook.go

@ozhuraki ozhuraki force-pushed the dsa-idxd-op branch 2 times, most recently from c4da835 to 7bb227f Compare September 3, 2021 06:36
@ozhuraki
Copy link
Contributor Author

ozhuraki commented Sep 3, 2021

@mythi @rojkov

  • InitImage -> ProvisioningImage
  • Updated the description to "ProvisioningImage is an initcontainer image to configure and enable DSA devices and workqueues with accel-config utility"

@ozhuraki
Copy link
Contributor Author

ozhuraki commented Sep 3, 2021

@mythi

Oops, missed your comments. I will update.

Can we do both easily with just one image? I'm also fine with one InitImage entry per CRD that is tasked to do both if what it needs to do can be customized somehow (e.g. not install some tools but provision only vs do both).

+1
I think having multiple images could be justified in cases where it's not possible due to non-technical reasons, etc (or where it would simplify the task). In this specific case, there aren't such yet.

@ozhuraki
Copy link
Contributor Author

ozhuraki commented Sep 3, 2021

@mythi

  • ProvisioningImage -> InitImage

Copy link
Contributor

@mythi mythi left a comment

Choose a reason for hiding this comment

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

a few additional comments

@ozhuraki
Copy link
Contributor Author

Changes from the previous version:

  • Switched DSA initcontainer image to debian:sid-slim
  • Updated DSA initcontainer image to use libjson-c5
  • Upgrated accel-config to 3.4.1

TODO/WIP

  • Check if remount of /sys could be dropped
  • Enable all idle DSA devices

@ozhuraki ozhuraki force-pushed the dsa-idxd-op branch 4 times, most recently from 59b7191 to 38dcbf1 Compare September 30, 2021 05:50
@ozhuraki
Copy link
Contributor Author

Changes:

  • ENV -> ARG
  • accel-config -> 3.4.2
  • debian:sid-slim -> debian:unstable
  • Dropped remount
  • Dropped all volumes, using just /sys/devices
  • Added enabling of all idle DSA devices/workqueues (1 engine / 1 group / 1 wq (user/dedicated) )

@ozhuraki ozhuraki force-pushed the dsa-idxd-op branch 4 times, most recently from fc29cd7 to f4e7e6a Compare September 30, 2021 19:14
@ozhuraki ozhuraki force-pushed the dsa-idxd-op branch 2 times, most recently from 571a0db to 4369336 Compare October 1, 2021 07:32
@ozhuraki
Copy link
Contributor Author

ozhuraki commented Oct 1, 2021

Changes:

  • Dropped "readonlyrootfs" from DSA initcontainer deployment and operator
  • Added /sys/devices volume into DSA initcontainer deployment

@ozhuraki
Copy link
Contributor Author

ozhuraki commented Oct 1, 2021

Changes:

  • Dropped /test from initcontainer

Signed-off-by: Oleg Zhurakivskyy <oleg.zhurakivskyy@intel.com>
Previously idxd kernel module instantiated some
default DSA devices and workqueues on boot.

This is a sample deployment that provisions DSA devices and
workqueues for intel-dsa-plugin with accel-config utility
through initcontainer.

Signed-off-by: Oleg Zhurakivskyy <oleg.zhurakivskyy@intel.com>
@ozhuraki
Copy link
Contributor Author

ozhuraki commented Oct 1, 2021

Changes:

  • Added "WORKDIR /dsa-init" to initcontainer

@ozhuraki
Copy link
Contributor Author

ozhuraki commented Oct 1, 2021

Changes:

  • Added /sys/devices volume into DSA operator

@bart0sh
Copy link
Member

bart0sh commented Oct 1, 2021

@ozhuraki 3 more things would be nice to have:

  • documentation (overall description, deployment instructions, etc)
  • list of known issues
  • github issue(s) for the known issue(s) included into the next project milestone

Signed-off-by: Oleg Zhurakivskyy <oleg.zhurakivskyy@intel.com>
@mythi
Copy link
Contributor

mythi commented Oct 1, 2021

  • list of known issues
  • github issue(s) for the known issue(s) included into the next project milestone

I can cover these in the release notes but a short description and sample

$ kubectl apply -k deployments/dsa_plugin/overlays/dsa_initcontainer/ 

would indeed be good in the README

mythi
mythi previously approved these changes Oct 1, 2021
Copy link
Contributor

@mythi mythi left a comment

Choose a reason for hiding this comment

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

this works and is good IMO. short README update as @bart0sh suggested would be good still.

Signed-off-by: Oleg Zhurakivskyy <oleg.zhurakivskyy@intel.com>
@ozhuraki
Copy link
Contributor Author

ozhuraki commented Oct 1, 2021

@mythi @bart0sh

Added a documentation, please take a look.

@mythi
Copy link
Contributor

mythi commented Oct 1, 2021

LGTM. Let's merge

Copy link
Contributor

@rojkov rojkov left a comment

Choose a reason for hiding this comment

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

LGTM

@mythi mythi merged commit dc1465a into intel:main Oct 1, 2021
@ozhuraki ozhuraki deleted the dsa-idxd-op branch December 17, 2021 10:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Device configuration operator (initcontainer)

5 participants