interfaces/builtin: add opencl interface #1201

claudioandre · May 20, 2016

allows a snap to access and use the OpenCL libraries.
I signed the Canonical contributor licence agreement.

It is only for discussion. This two tests are failing (can't see why)

=== RUN   TestRunnerPipesOutput
OOPS: 1 passed, 1 FAILED
--- PASS: TestRunnerPipesOutput (0.00s)
=== RUN   TestRunnerAcceptsStreamFlag
OOPS: 1 passed, 1 FAILED
--- PASS: TestRunnerAcceptsStreamFlag (0.00s)
=== RUN   TestRunnerAcceptsFilterFlag

claudioandre · May 20, 2016

$ ~/bin/work/bin/snap interfaces
Slot                 Plug
:firewall-control    -
:home                -
:locale-control      -
:log-observe         -
:mount-observe       -
:network             namebench
:network-bind        -
:network-control     -
:network-manager     -
:network-observe     -
:opencl              john-the-ripper
:opengl              -
:snapd-control       -
:system-observe      -
:timeserver-control  -
:timezone-control    -
:unity7              -
:x11                 namebench

niemeyer · May 21, 2016

Thanks! Let's have a look a this next week.

niemeyer · May 21, 2016

niemeyer reviewed May 21, 2016

View changes

This looks a bit problematic. It pretends there's some content in this directory which may not actually be there.

Perhaps we should have Sanitize checking if the content is there, and rejecting the connection if there's nothing.

What do you think @zyga, @jdstrand?

@niemeyer - this is a weird area. I'm assuming this interface is only for classic since this isn't part of the os snap and as you mentioned, this directory may be empty or not exist on the system at all since it requires additional debs to be installed on the classic system. Doing a dynamic check to see if the opencl binaries are installed and if /etc/OpenCL/vendors/* could be a way to do this, but then I worry about the precedent that would set. We might end up with hundreds of little interfaces to optional debs which seems very uncontrolled and not designed.... Why aren't these files and whatever binaries are needed not part of the shipping snap? (sorry if this is obvious-- I'm not familiar with opencl).

Can we have a new method on the interface, TryConnect or Connect, that takes a plug and a slot and lets us do any dynamic checks required? For all current interfaces it would just return true.

This way the interface would exist but may not be connectable. Later on we could flag it as "potential" or something similar. It seems like a nice middle ground between dynamic interfaces and something that is very rigid.

How's that different from Sanitize?

@jdstrand I'm assuming that these files are closely related to the local hardware, which would justify them not being shipped inside the snap. I'm not sure either, though.

@claudioandre Can you shed some light on why these files are not inside the snap itself?

I see, so definitely sounds arch-dependent. That said, as far as I can see, this interface does not give you access to the libraries. So how is that working? Did you try it out in practice?

It fails. But, please, try to see my point (I'm reading your comments trying to find what I did wrong as I pointed out in my first message).

$ cat /var/lib/snapd/apparmor/profiles/snap.john-the-ripper.opencl | grep vendors
  /etc/OpenCL/vendors/** r,

$ john-the-ripper.opencl --list=opencl-devices
[ERROR]

$ dmesg
[28834.360820] audit: type=1400 audit(1464031699.616:33): apparmor="DENIED" operation="open" profile="snap.john-the-ripper.opencl" name="/etc/OpenCL/vendors/" pid=7880 comm="john" requested_mask="r" denied_mask="r" fsuid=1002 ouid=0

I already tried to be more permissive to be able to load the real library. But the error message is puzzling me.

@jdstrand Any ideas?

Yes, please also add this rule:
/etc/OpenCL/vendors/ r,

AppArmor differentiates between directories (paths ending with '/') and files. Your glob says 'all files and subdirectories in /etc/OpenCL/vendors/, but it does not give readdir to the directory itself.

Thanks @jdstrand. All, please, don't worry about the text below.

I am running AMD OpenCL multi-core CPU (auto scale to use all cores). I need try it on 100% clean GPU hardware. It hangs and ps u STAT says:
S interruptible sleep (waiting for an event to complete)
l is multi-threaded (using CLONE_THREAD, like NPTL pthreads do)

is in the foreground process group.

The interface tested

  # OpenCL Installable Client Driver (ICD) Loader
  /etc/OpenCL/vendors/ r,
  /etc/OpenCL/vendors/** r,

  # AMD
  /opt/AMDAPP/lib/x86_64/ r,
  /opt/AMDAPP/lib/x86/ r,
  /opt/AMDAPP/lib/x86_64/** r,
  /opt/AMDAPP/lib/x86/** r,

dmesg

May 23 17:22:32 jucelia-notebook-HP kernel: [32087.007206] audit: type=1326 audit(1464034952.227:65): auid=4294967295 uid=1002 gid=1002 ses=4294967295 pid=9998 comm="john" exe="/snap/john-the-ripper/x1/opencl/john" sig=31 arch=c000003e syscall=203 compat=0 ip=0x7fdd24de1720 code=0x0
May 23 17:23:17 jucelia-notebook-HP kernel: [32132.309280] audit: type=1326 audit(1464034997.532:66): auid=4294967295 uid=1002 gid=1002 ses=4294967295 pid=10013 comm="john" exe="/snap/john-the-ripper/x1/opencl/john" sig=31 arch=c000003e syscall=203 compat=0 ip=0x7f83baa08720 code=0x0

niemeyer · May 21, 2016

niemeyer reviewed May 21, 2016

View changes

What about every other driver?

We might have this in for now, but can we do better easily?

I agree. I'm trying AMD's CPU OpenCL; but remember that GPU AMD OpenCL was postponed in 16.04.

This isn't a blocker, and it's better to have one working than none, but do we at least know what the plan looks like next? Is AMD taking a very similar approach but only with a different device name? Is the device name also well known? Anything else?

I have access to a machine with AMD, NVIDIA and Intel OpenCL drivers installed (not 16.04).

I am not in the dark, but I have the felling we will be surprised by the journey.

That sounds okay.. as long as we have some good idea of what we want, it's better to have something that tends to work now than something that might be perfect in a theoretical future.

I'll also note that this is going to overlap with the opengl interface (which isn't a problem technically, but isn't particularly clean).

Also, it should be noted that direct access to graphics devices is a security hole, but unfortunately not one that can be easily solved (it is something X, mir and wayland would like to solve but no one has been able to yet in any practical way yet).

OpenGL and OpenCL are closely related but actually OpenCL has a much better userspace API->real userspace implementation story. There's a standardized interface that lets you see opencl implementations and pick one dynamically at runtime.

In reality, this doesn't help interfaces much as those userspace bits will still need driver/vendor specific permissions but we can tackle those with data on the slot side down the line. For now I'd really go ahead with broad permissions that work on common vendors (nvidia, amd).

How's that better than OpenGL more specifically? OpenGL also has very well defined user space APIs.

niemeyer · May 21, 2016

niemeyer reviewed May 21, 2016

View changes

I'm not sure we want to auto-connect if it gives unrestricted access to the video device. @jdstrand?

I think we should check, I generally prefer auto-connect and restricted than not connected (similar to how we deal with x11). If a snap is useless without connecting this, people will not use snaps.

zyga · May 23, 2016

Can you please add an entry to docs/interfaces.md and, if possible, a snapcraft yaml (even as a comment) for testing this. I suspect you will also need a few small tweaks around the code to pass unit tests but I can help with that.

zyga · May 23, 2016

zyga reviewed May 23, 2016

View changes

I will remove the requirement to patch this code so we may end up removing it from this branch.

This is no longer needed

claudioandre · May 23, 2016

Ok @zyga, will do it. But I will wait you guys to think more about the problems.

Please, anyone. Let me know if I should create a new commit or if I should use --amend.

niemeyer · May 23, 2016

Please create new commits, as that allows us to follow your changes incrementally.

zyga · May 30, 2016

Can you please merge master into this

snappy-m-o · May 30, 2016

Can one of the admins verify this patch?

snappy-m-o · May 30, 2016

Can one of the admins verify this patch?

snappy-m-o · May 30, 2016

Can one of the admins verify this patch?

claudioandre · May 30, 2016

ASAP I will take a look at this.

Thank you!

zyga · May 30, 2016

whitelist this please

zyga · Jun 14, 2016

Hi, do you have any update on this interface?

snappy-m-o · Jun 14, 2016

Can one of the admins verify this patch?

snappy-m-o · Jun 14, 2016

Can one of the admins verify this patch?

snappy-m-o · Jun 14, 2016

Can one of the admins verify this patch?

claudioandre · Jun 14, 2016

I'm sorry for the delay. I'm fighting against a few problems I'm seeing.

I'm going to finish it (or close the PR) soon.

Thanks.

2016-06-14 18:52 GMT-03:00 Zygmunt Krynicki notifications@github.com:

Hi, do you have any update on this interface?

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#1201 (comment), or mute
the thread
https://github.com/notifications/unsubscribe/ABn8C_loxMi3N97_kq4E_Osc7PSAmUMgks5qLyKSgaJpZM4IjfbQ
.

niemeyer · Jul 4, 2016

I'm closing this for the time being. Please do reopen if you have something we should look at.

snappy-m-o · Jul 4, 2016

Can one of the admins verify this patch?

snappy-m-o · Jul 4, 2016

Can one of the admins verify this patch?

claudioandre changed the title from First version of the opencl interface interfaces. to First version of the opencl interface. May 20, 2016

interfaces: opencl interface added.
- allows a snap to access the OpenCL libraries.

All checks have passed
2 successful checks

continuous-integration/travis-ci/pr — The Travis CI build passed

Details

coverage/coveralls — Coverage increased (+0.01%) to 79.408%

Details

f9db289

niemeyer changed the title from First version of the opencl interface. to interfaces/builtin: add opencl interface. May 21, 2016

niemeyer changed the title from interfaces/builtin: add opencl interface. to interfaces/builtin: add opencl interface May 21, 2016

zyga added the Reviewed label May 23, 2016

niemeyer closed this Jul 4, 2016

+        +		connectedPlugAppArmor: openclConnectedPlugAppArmor,
+        +		connectedPlugSecComp:  openclConnectedPlugSecComp,
+        +		reservedForOS:         true,
+        +		autoConnect:           true,

@@ -451,7 +451,7 @@ func (s *interfaceManagerSuite) TestDoSetupProfilesAddsImplicitSlots(c *C) {
          	// Ensure that we have slots on the OS snap.
          	repo := mgr.Repository()
          	slots := repo.Slots(snapInfo.Name())
-        -	c.Assert(slots, HasLen, 17)
+        +	c.Assert(slots, HasLen, 18)

snapcore/snapd

interfaces/builtin: add opencl interface #1201

claudioandre commented May 20, 2016

claudioandre commented May 20, 2016

claudioandre changed the title from First version of the opencl interface interfaces. to First version of the opencl interface. May 20, 2016

niemeyer commented May 21, 2016

niemeyer reviewed May 21, 2016

View changes

niemeyer reviewed May 21, 2016

View changes

niemeyer changed the title from First version of the opencl interface. to interfaces/builtin: add opencl interface. May 21, 2016

niemeyer changed the title from interfaces/builtin: add opencl interface. to interfaces/builtin: add opencl interface May 21, 2016

niemeyer reviewed May 21, 2016

View changes

zyga commented May 23, 2016

zyga added the Reviewed label May 23, 2016

zyga reviewed May 23, 2016

View changes

claudioandre commented May 23, 2016

niemeyer commented May 23, 2016

zyga commented May 30, 2016

snappy-m-o commented May 30, 2016

snappy-m-o commented May 30, 2016

snappy-m-o commented May 30, 2016

claudioandre commented May 30, 2016

zyga commented May 30, 2016

zyga commented Jun 14, 2016

snappy-m-o commented Jun 14, 2016

snappy-m-o commented Jun 14, 2016

snappy-m-o commented Jun 14, 2016

claudioandre commented Jun 14, 2016

niemeyer commented Jul 4, 2016

niemeyer closed this Jul 4, 2016

snappy-m-o commented Jul 4, 2016

snappy-m-o commented Jul 4, 2016

snapcore/snapd

Join GitHub today

interfaces/builtin: add opencl interface #1201

Conversation

claudioandre commented May 20, 2016

claudioandre commented May 20, 2016

claudioandre changed the title from First version of the opencl interface interfaces. to First version of the opencl interface. May 20, 2016

All checks have passed

niemeyer commented May 21, 2016

niemeyer reviewed May 21, 2016 View changes

zyga May 23, 2016 • edited Edited 1 time zyga May 23, 2016

niemeyer reviewed May 21, 2016 View changes

niemeyer changed the title from First version of the opencl interface. to interfaces/builtin: add opencl interface. May 21, 2016

niemeyer changed the title from interfaces/builtin: add opencl interface. to interfaces/builtin: add opencl interface May 21, 2016

niemeyer reviewed May 21, 2016 View changes

zyga commented May 23, 2016

zyga added the Reviewed label May 23, 2016

zyga reviewed May 23, 2016 View changes

claudioandre commented May 23, 2016

niemeyer commented May 23, 2016

zyga commented May 30, 2016

snappy-m-o commented May 30, 2016

snappy-m-o commented May 30, 2016

snappy-m-o commented May 30, 2016

claudioandre commented May 30, 2016

zyga commented May 30, 2016

zyga commented Jun 14, 2016

snappy-m-o commented Jun 14, 2016

snappy-m-o commented Jun 14, 2016

snappy-m-o commented Jun 14, 2016

claudioandre commented Jun 14, 2016

niemeyer commented Jul 4, 2016

niemeyer closed this Jul 4, 2016

snappy-m-o commented Jul 4, 2016

snappy-m-o commented Jul 4, 2016

niemeyer reviewed May 21, 2016

View changes

zyga May 23, 2016 •

Edited 1 time

zyga
May 23, 2016

niemeyer reviewed May 21, 2016

View changes

niemeyer reviewed May 21, 2016

View changes

zyga reviewed May 23, 2016

View changes