[MIR] ndctl

Bug #1853506 reported by Rafael David Tinoco
22
This bug affects 1 person
Affects Status Importance Assigned to Milestone
ndctl (Ubuntu)
Fix Released
Undecided
Andreas Hasenack

Bug Description

https://bugs.launchpad.net/ubuntu/+source/ndctl/+bug/1853506

[Availability]

There is an on-going MIR for a package whose ndctl is a dependency:
pmdk (LP: #1790856)

* Package exists since bionic (-updates) in universe:

    61.2-0ubuntu1~18.04.1 | bionic-updates/universe
    63-1.3 | disco/universe
    65-1 | eoan/universe
    67-1 | focal/universe

* Packages:

    ndctl libndctl6 libndctl-dev
    daxctl libdaxctl1 libdaxctl-dev

    ndctl: dctl is utility for managing the "libnvdimm" kernel
    subsystem. The "libnvdimm" subsystem defines a kernel device
    model and control message interface for platform NVDIMM resources
    like those defined by the ACPI 6.0 NFIT (NVDIMM Firmware
    Interface Table).

    Operations supported by the tool include, provisioning capacity
    (namespaces), as well as enumerating/enabling/disabling the
    devices (dimms, regions, namespaces) associated with an NVDIMM
    bus.

    daxctl: The daxctl utility provides enumeration and provisioning
    commands for the Linux kernel Device-DAX facility. This facility
    enables DAX mappings of performance / feature differentiated
    memory without need of a filesystem.

* Architectures:

    source, amd64, arm64, armhf, i386, ppc64el, s390x

PMEM: A system-physical-address range where writes are persistent. A
block device composed of PMEM is capable of DAX. A PMEM address
range may span an interleave of several DIMMs.

BLK: A set of one or more programmable memory mapped apertures
provided by a DIMM to access its media. This indirection precludes
the performance benefit of interleaving, but enables DIMM-bounded
failure modes.

DAX: File system extensions to bypass the page cache and block layer
to mmap persistent memory, from a PMEM block device, directly into a
process address space.

Binary Packages:

    ndctl
    libndctl6
    libndctl-dev
    daxctl
    libdaxctl1
    libdaxctl-dev

[Rationale]

This is part of the MIR activity for all dependencies of pmdk. "ndctl" and "daxctl" are userland tools used to configure NVDIMMs and should be supported and put in main pocket (and server).

The "main" MIR of it is at:
https://bugs.launchpad.net/ubuntu/+source/pmdk/+bug/1790856

Package was introduced in bionic after bionic was released (19.04.1)
in universe and it is still in universe until now.

[Security]

- No related CVEs found at: cve.mitre.org

[Quality assurance]

* Documentation:

 - https://docs.pmem.io/ndctl-users-guide
 - https://nvdimm.wiki.kernel.org/

- Both packages (ndctl and daxctl) and its libraries (libndctl6,
libndctl-dev, libdaxctl1, libdaxctl-dev) install fine and are
operational (check comment #1).

- There are no debconf templates and/or questions in this package.

- The only existing/opened bug affecting the package was LP: #1811785 and I have already provided 2 MRs fixing that issue. No other long-term outstanding bug affecting it. Upstream seems well maintained and isolating fix commits, making them easy to cherry-pick and/or backport.

- There are no current Debian bugs for ndctl (https://bugs.debian.org/cgi-bin/pkgreport.cgi?dist=unstable;package=ndctl)

- Upstream has 25 cases opened (almost all of them with comments and discussions) and 77 cases already closed, leading to a conclusion that is actively maintained and provides good feedback to reporters.

- Package does not deal with exotic hardware, but, indeed, it deals with NEW type of hardware, getting more common every day (NVDIMMs). DAX support is already included in EXT4 and XFS filesystems, allowing NVDIMMs to be transparently used as block devices (without page cache). The userland libraries allow applications to get all benefits with a new memory (non-volatile) layer.

- Package has a minimum DEP8 support (for obvious issues with the binaries, as the utils need NVDIMMs to be present in the running environment).

- Package uses debian/watch.

- No lintian errors (pedantic).

- Package does not rely on obsolete packages:

$ apt-cache rdepends ndctl
ndctl
Reverse Depends:
  daxctl
  ndctl-dbgsym
  daxctl

$ apt-cache depends ndctl
ndctl
  PreDepends: init-system-helpers
  Depends: libc6
  Depends: libdaxctl1
  Depends: libjson-c4
  Depends: libkeyutils1
  Depends: libndctl6
  Depends: libuuid1

[UI standards]

N/A

[Dependencies]

$ apt-cache depends ndctl

ndctl
  PreDepends: init-system-helpers
  Depends: libc6
  Depends: libdaxctl1 (itself)
  Depends: libjson-c4 - main (exists since eoan)
  Depends: libkeyutils1 - main
  Depends: libndctl6 (itself)
  Depends: libuuid1 - main

[Standards compliance]

- Package follows FHS.

- Debian Standards 4.4.1

- Source package is clear and well done.

[Maintenance]

The Server team will subscribe for the package for maintenance

[Background]

The Persistent Memory Development Kit (PMDK) is a collection of
libraries and tools for System Administrators and Application
Developers to simplify managing and accessing persistent memory
devices. The libraries build on the Direct Access (DAX) feature which
allows applications to directly access persistent memory as
memory-mapped files. This is described in detail in the Storage
Network Industry Association (SNIA) NVM Programming Model.

PMDK depends on libndctl.

The Non-Volatile Device Control (ndctl) is a utility for managing the
LIBNVDIMM Linux Kernel subsystem. The LIBNVDIMM subsystem defines a
kernel device model and control message interface for platform NFIT
(NVDIMM Firmware Interface Table). This interface was first defined
by the ACPI v6.0 specification. Later versions may enhance or modify
this specification. The latest ACPI and UEFI specifications can be
found at http://uefi.org/specifications.

The latest ACPI and UEFI specifications can be found at uefi.org.
Operations
supported by ndctl include:

- Provisioning capacity (namespaces)
- Enumerating Devices
- Enabling and Disabling NVDIMMs, Regions, and Namespaces
- Managing NVDIMM Labels

The LIBNVDIMM subsystem provides support for three types of NVDIMMs,
namely, PMEM, BLK, and NVDIMM devices that can simultaneously support
both PMEM and BLK mode access. These three modes of operation are
described by the "NVDIMM Firmware Interface Table" (NFIT) in ACPI
v6.0 or later. Linux Kernel documentation can be found at:
https://www.kernel.org/doc/Documentation/nvdimm/nvdimm.txt.

Related branches

Changed in ndctl (Ubuntu):
milestone: none → ubuntu-20.04
description: updated
Revision history for this message
Rafael David Tinoco (rafaeldtinoco) wrote :
Download full text (3.3 KiB)

(k)rafaeldtinoco@emulated:~$ sudo ndctl enable-dimm all
enabled 2 nmems

(k)rafaeldtinoco@emulated:~$ sudo ndctl list -D
[
  {
    "dev":"nmem1",
    "id":"8680-57341200",
    "handle":2,
    "phys_id":0,
    "flag_failed_arm":true
  },
  {
    "dev":"nmem0",
    "id":"8680-56341200",
    "handle":1,
    "phys_id":0,
    "flag_failed_arm":true
  }
]

(k)rafaeldtinoco@emulated:~$ sudo ndctl enable-region all
enabled 2 regions

(k)rafaeldtinoco@emulated:~$ ndctl list -R
[
  {
    "dev":"region1",
    "size":2145386496,
    "available_size":0,
    "max_available_extent":0,
    "type":"pmem",
    "persistence_domain":"unknown"
  },
  {
    "dev":"region0",
    "size":2145386496,
    "available_size":0,
    "max_available_extent":0,
    "type":"pmem",
    "persistence_domain":"unknown"
  }
]

(k)rafaeldtinoco@emulated:~$ sudo ndctl create-namespace -r region0
{
  "dev":"namespace0.0",
  "mode":"fsdax",
  "map":"dev",
  "size":"2014.00 MiB (2111.83 MB)",
  "uuid":"c3621fd1-6b37-4241-956f-eeffdb498872",
  "sector_size":512,
  "align":2097152,
  "blockdev":"pmem0"
}

(k)rafaeldtinoco@emulated:~$ sudo ndctl create-namespace -r region1
{
  "dev":"namespace1.0",
  "mode":"fsdax",
  "map":"dev",
  "size":"2014.00 MiB (2111.83 MB)",
  "uuid":"ec231ca1-cc4a-41e6-892b-e8afd3e36313",
  "sector_size":512,
  "align":2097152,
  "blockdev":"pmem1"
}

(k)rafaeldtinoco@emulated:~$ sudo cat /proc/iomem
...
240000000-2bfdfffff : Persistent Memory
  240000000-2bfdfffff : namespace0.0
2bfe00000-33fbfffff : Persistent Memory
  2bfe00000-33fbfffff : namespace1.0

(k)rafaeldtinoco@emulated:~$ sudo fdisk -l /dev/pmem0
Disk /dev/pmem0: 1.99 GiB, 2111832064 bytes, 4124672 sectors
Units: sectors of 1 * 512 = 512 bytes
Sector size (logical/physical): 512 bytes / 4096 bytes
I/O size (minimum/optimal): 4096 bytes / 4096 bytes
Disklabel type: dos
Disk identifier: 0x6e3d7965

Device Boot Start End Sectors Size Id Type
/dev/pmem0p1 4096 4124671 4120576 2G 83 Linux

(k)rafaeldtinoco@emulated:~$ sudo mkfs.ext4 -b 4096 -E stride=512 -F /dev/pmem0
mke2fs 1.45.3 (14-Jul-2019)
Found a dos partition table in /dev/pmem0
Creating filesystem with 515584 4k blocks and 129024 inodes
Filesystem UUID: d6d72f36-28bb-40ed-9108-252e3787b08e
Superblock backups stored on blocks:
        32768, 98304, 163840, 229376, 294912

Allocating group tables: done
Writing inode tables: done
Creating journal (8192 blocks): done
Writing superblocks and filesystem accounting information: done

k)rafaeldtinoco@emulated:~$ sudo mkfs.ext4 -b 4096 -E stride=512 /dev/pmem0
pmem0# pmem0p1#
(k)rafaeldtinoco@emulated:~$ sudo mkfs.ext4 -b 4096 -E stride=512 /dev/pmem0p1
mke2fs 1.45.3 (14-Jul-2019)
Creating filesystem with 515072 4k blocks and 128768 inodes
Filesystem UUID: 56e9b056-eea1-41dd-9eaa-3f9e66c712d0
Superblock backups stored on blocks:
        32768, 98304, 163840, 229376, 294912

Allocating group tables: done
Writing inode tables: done
Creating journal (8192 blocks): done
Writing superblocks and filesystem accounting information: done

(k)rafaeldtinoco@emulated:~$ sudo moun...

Read more...

description: updated
Revision history for this message
Rafael David Tinoco (rafaeldtinoco) wrote :

This is a good XML template for a VM emulating nvdimms.

Revision history for this message
Rafael David Tinoco (rafaeldtinoco) wrote :

When using dax:

(k)rafaeldtinoco@emulated:~$ sudo mount -o dax /dev/pmem0p1 /mnt

(k)rafaeldtinoco@emulated:~$ mount | grep -i mnt
/dev/pmem0p1 on /mnt type ext4 (rw,relatime,dax,stripe=512)

(k)rafaeldtinoco@emulated:~$ dd if=/dev/zero of=./teste bs=1M count=1024
1024+0 records in
1024+0 records out
1073741824 bytes (1.1 GB, 1.0 GiB) copied, 0.43958 s, 2.4 GB/s

description: updated
description: updated
Changed in ndctl (Ubuntu):
assignee: nobody → Christian Ehrhardt  (paelzer)
Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

[Summary]
- looks up to date and well packaged
- MIR Team ack (constraint to add the subscription before promotion)
- Needs security review
  - subscribing and assigning to security now
- some minimal optional improvements that should be looked at briefly at least
  - over-linking warnings by dpkg-shlibdeps

[Duplication]
- No duplication issue as there isn't another tool/lib providing the same functionality atm.

[Embedded sources and static linking]
- No embedded sources
- No static linking

[Security]
- No history of CVEs
- does not run a daemon as root
- does not use webkit1,2
- does not use lib*v8 directly
- does not open a port
- does not processes arbitrary web content
- does not use centralized online accounts
- does not integrate arbitrary javascript into the desktop
- does not deal with system authentication (eg, pam), etc)

It does:
- parses data formats (from its CTL programs as well as through the libs API)

Compared to other packages this doesn't have a lot of security exposure at first (network, ...), but
it is responsible to manage your (persistent) memory and therefore can be considered important
to not allow access to areas users should have none.

Also tools known to use the libs could be tricked into bug by modifying the (virtual) system environment.
So the stability of the tools is important for potentially further programs.

Therefore despite not ticking a lot of "security concern" check-boxes I'd ask for a security review to be on the safe side.

[Common blockers]
- builds fine atm
- bug subscriber is clear and will be done at promotion of the package
- no translations available, but this is not facing non-experienced end users
- no python package for extra checks in that regard

- it does have a test suite, but atm all tests are skipped at build time!
  It seems it needs to load modules to test, which won't work in build-env
  One might think those tests might easily be converted to an autopkgtest
  but it is not the main modules that it needs.
  It needs a special test module from tools/testing/nvdimm which isn't part of
  any linux-* package and therefore hard to get right in an autopkgtest
  environment. Therefore consider this ok for now as-is.

[Packaging red flags]
- no Ubuntu delta
- symbols tracking in place
- d/watch is fine
- regular update history
- latest release is packaged
- no issues for MOTUs when promoted
- no massive Lintian warnings
- d/rules is rather clear
- no Built-Using present
- no golang involved

[Upstream red flags]
- no Errors/warnings during the build
- no Incautious use of malloc/sprintf
- no use of sudo, gksu, pkexec, or LD_LIBRARY_PATH
- no use of user nobody
- no use of setuid
- no major bugs in Debian or Ubuntu
- no dependency on webkit, qtwebkit, seed or libgoa-*
- no embedded source copies
- not scope for UI

Changed in ndctl (Ubuntu):
assignee: Christian Ehrhardt  (paelzer) → Ubuntu Security Team (ubuntu-security)
Revision history for this message
Seth Arnold (seth-arnold) wrote : Re: [Bug 1853506] Re: [MIR] ndctl

My first and most pressing concern is hardware availability: certainly
none of us on the security team have nvdimm hardware at the moment and
getting systems for this is probably beyond the usual hardware refresh
budget.

At a minimum we'll need someone else's commitment to test updates as
necessary.

Thanks

Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

@Rafael - Seths comment is correct but I think can be alleviated with instructions how to do that on virtual HW. I know you have testing info on bug 1811785 and that you are working on more aspects of it, so maybe you have even better/extended instructions now.
I'd ask you to)
a) add a comment here how this can be tested via qemu emulating pmem
b) If confirmed below by Seth, add a test script to qa-regression-tools based on (a)

@Seth - I think this could be mostly done virtualized.
Would security be ok if we outline how it works, maybe even add a test to the qa-regression-test report for it?
For real HW it will depend on the manufacturer and even if we'd buy one we wouldn't have all the other variants - as with other special HW (think of the 1001+ different network cards out there) once debugging reaches the assumption that it is HW dependent we will have to ping and wait for the manufacturer.

Revision history for this message
Rafael David Tinoco (rafaeldtinoco) wrote :

On 26/11/2019 04:01, Christian Ehrhardt  wrote:
> @Rafael - Seths comment is correct but I think can be alleviated with instructions how to do that on virtual HW. I know you have testing info on bug 1811785 and that you are working on more aspects of it, so maybe you have even better/extended instructions now.
> I'd ask you to)
> a) add a comment here how this can be tested via qemu emulating pmem
Yes, looks like NVDIMM emulation in QEMU
(https://github.com/qemu/qemu/blob/master/docs/nvdimm.txt) is in good
shape. The direct pass-through gives us no VM_EXITs and direct access to
the host files AND we can use, as I did, the NVDIMM emulation *from* a
host file.

We usually have:

PMEM: the physical addresses that are mapped to one or more nvdimms
(writes are persistent).
DAX: a block device composed of PMEM is DAX capable (fs extension to
bypass cache and block layer)
BW (block window): set of registers (command, status, aperture) to
read/write blocks in any part of NVDIMM
BLK: memory mapped I/O
BTT (block translation table): like a journal data structure defined in
nvdimm namespace specs.
Namespace: Similar to a partition or LUN (or a NVMe namespace)
NFIT (NVDIMM Firmware Interface Table): ACPI table to inform OS about
NVDIMMs
DSM (Device Specific Method): specific methods for particular NVDIMM
implementation.

The QEMU does NOT implement the BW (Block Window Mode). So we would rely
only on the MMIO-like capabilities of the virtual NVDIMM only: Accessing
NVDIMM through NVM libraries AND through the regular VFS <-> Dax Enabled
Filesystem interface (ext4, xfs). I think it is enough for enablement.

> b) If confirmed below by Seth, add a test script to qa-regression-tools based on (a)
IMO, basic DEP8 should include:

1) Add test for configuring regions & namespaces
2) Add test for testing a DAX enabled filesystem
3) Add test to test accessing NVDIMMs through NVM libs

> @Seth - I think this could be mostly done virtualized.
> Would security be ok if we outline how it works, maybe even add a test to the qa-regression-test report for it?
> For real HW it will depend on the manufacturer and even if we'd buy one we wouldn't have all the other variants - as with other special HW (think of the 1001+ different network cards out there) once debugging reaches the assumption that it is HW dependent we will have to ping and wait for the manufacturer.
If we concentrate in the virtual NVDIMMs for enablement, we can use the
public bugs to guarantee better compatibility. Just like we do for
multipath-tools, for example. We usually don't test ALL existing HBAs
and/or iSCSI storage-servers, but we guarantee the multipath-tools core
is good and fix issues as they appear (discovering ALUA issues w/ weird
storage-servers here and there). Just one example that came to my mind.

Revision history for this message
Seth Arnold (seth-arnold) wrote :

Dep-8 tests for what can be tested there, and qrt tests for what can't / shouldn't be tested there sound wonderful and go a very long way towards alleviating my concerns.

Thanks

Revision history for this message
Rafael David Tinoco (rafaeldtinoco) wrote :

hello Seth,

sorry for the delay here, I've been working in making a qa-regression test available for the security team, and will push into your repository, but I had to struggle with things like:

https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1855177

and so. I believe I'm almost finishing the tests I suggested, so we can continue this after addressing your concerns.

tks

Revision history for this message
Seth Arnold (seth-arnold) wrote :

On Wed, Dec 04, 2019 at 08:44:44PM -0000, Rafael David Tinoco wrote:
> sorry for the delay here, I've been working in making a qa-regression
> test available for the security team, and will push into your
> repository, but I had to struggle with things like:
>
> https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1855177
>
> and so. I believe I'm almost finishing the tests I suggested, so we can
> continue this after addressing your concerns.

This is so cool, I didn't expect the tests to pay dividends so quickly!
Thanks so much Rafael.

Revision history for this message
Rafael David Tinoco (rafaeldtinoco) wrote :

Yes Seth!

Unfortunately (or not, as picking up things like this in this phase is very good) there is an on-going issue with nvdimm regions boundaries. I'm documenting everything in another bug:

https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1855177

And the issue is well known upstream (and capable of happening to some arches/firmwares). Since there isn't a clear "spec" on how HW manufactores can force the region alignments, mitigating the issue, I'm keeping documentation in that bug not to block this MIR, as the upstream ndctl also faces the same issue and end-user can use some directions by reading that bug.

In our "test-scenario", for the qa-regressions tests, the mitigation will be to reduce from 2 emulated nvdimms to just 1, and report the alignment issue to QEMU. I'm using some of the most important tests from ndctl/test source code.

Will post the repo pushes soon.

Cheers

Revision history for this message
Rafael David Tinoco (rafaeldtinoco) wrote :

This patch, applied to ndctl code upstream (v67-1-gf6cafcf), or focal devel, creates the testing environment we need. I have also created another set of scripts to create the virtual environment for the nvdimms emulation which I'll inform soon.

TL;DR is:

- a set of scripts will provision a kvm/qemu guest with nvdimm emulation
- this VM will download focal (or upstream) ndctl source package
- it will also download the patch file and apply to the source package
- all the tests will run

Where should I put this ? into qa-regression-testing ? Should I proposed a MR ?
To which folder ?

Thank you very much

Rafael

Revision history for this message
Rafael David Tinoco (rafaeldtinoco) wrote :

A few comments on the test (explained in the git commit log):

----

[PATCH] canonical: convert ndctl/test into qa-regression-ndctl tests

  ## good tests (bad results mean something)

    blk-exhaust.sh
    btt-check.sh
    btt-pad-compat.sh
    label-compat.sh
    multi-dax.sh
    rescan-partitions.sh
    sector-mode.sh
    dax-ext4.sh
    dax-xfs.sh

  ## TODO: needs more work. locking/unlocking nvdimms using keyutils

    security.sh

  ## TODO: do a merge proposal to debian fio pkg enabling pmem engines

    device-dax-fio.sh

  ## TODO: enable this tests by compiling "test/" directory (next phase)

    mmap.sh
    monitor.sh

  ## alignment issues (full ns boundaries not aligned, TODO: smaller ns might work)

    create.sh

  ## needs injection (error or smart) and qemu nvdimm emulation does not support it

    btt-errors.sh
    clear.sh
    daxdev-errors.sh
    inject-error.sh
    inject-smart.sh
    pfn-meta-errors.sh
    pmem-errors.sh

  ## qemu nvdimm is pmem_compat (dax-class instead of dax-mode)

    daxctl-devices.sh

  ## Expected output:

    ~/ndctl/test$ ./regression.sh
    test ./blk-exhaust.sh: succeeded
    test ./btt-check.sh: succeeded
    test ./btt-pad-compat.sh: succeeded
    test ./label-compat.sh: succeeded
    test ./multi-dax.sh: succeeded
    test ./rescan-partitions.sh: succeeded
    test ./sector-mode.sh: succeeded
    test ./dax-ext4.sh: succeeded
    test ./dax-xfs.sh: succeeded

  ## Expected return code:

    0 = All tests have succeeded
    1 = A test has failed (check /tmp/regression.log)

Signed-off-by: Rafael David Tinoco <email address hidden>

----

# security team

Please note that, security wise, you're probably interested in the security.sh test. It uses keys to lock/unlock nvdimm devices (an interesting feature for 20.04 I suppose). I'm not entirely sure the emulated nvdimms will be enough for this test to run BUT I can help you after all this has been placed somewhere and is up and running.

# other todos:

i might be able to fix the TODOs here but I'm trying to be careful on how much time Im spending on this (balancing with other things for 20.04).

@Christian:

Do you think this patch could be put as a delta for ndctl/test directory ? I'm thinking in supportability in the next years to come. Keeping it out of the source package might make it hard to either extend it and/or fix changes.

Note: ndctl/test is not used for anything in binary packages, it would just be in the source packages. I could even add those as debian/tests for autopkgtest, and skip tests if being ran in a machine that doesn't support nvdimms (even suggesting to Debian).

Thoughts ?

PS: I'm pretty sure with those tests we're able to maintain this and catch regressions regarding PMEM-based block devices being accessed with DAX and/or through MMAPs (having the mmap tests fixed would be also beneficial).

For now I'm considering this done, missing only a place to merge this.

Thank you!

Revision history for this message
Rafael David Tinoco (rafaeldtinoco) wrote :

A merge proposal for all the tests I adapted:

https://code.launchpad.net/~rafaeldtinoco/ubuntu/+source/ndctl/+git/ndctl/+merge/376487

This makes much easier to run and support those tests (specially with a xml example file provided on how to create the autopkgtest environment).

Revision history for this message
Adam Borowski (kilobyte) wrote :

Unlike what your commit message says, you _can_ run at least some of the functionality with no HW and no specially configured qemu: append memmap=4G!20G to the kernel's cmdline, where "4G" is the size of an emulated nvdimm, and 20G is its start in physical memory. This stanza can be used multiple times, obviously for non-overlapping regions.

Such an emulated nvdimm has no label. Without a label, you can't partition the nvdimm -- there can be only exactly one namespace per memmap stanza (so you can't delete it, but create-namespace -e works same as delete+create).

ndctl:
* changing namespace's type: ✓
* partitioning: ✗
* error injection: requires "nfit_test" kernel module
* on-chip encryption: ✗
* on-chip scrub: ✗

daxctl:
* switching devdax↔system-ram: ✓

Thus, compared to qemu, you lose labels, gain very clunky error simulation.
This way of emulating nvdimms is even more useful for higher layers of the stack, as you don't need to muck with qemu but can use your regular bare-metal dev environment.

Revision history for this message
Rafael David Tinoco (rafaeldtinoco) wrote :

Yes Adam, my commit should have mentioned that! Maybe even referenced "https://docs.pmem.io/getting-started-guide/creating-development-environments/linux-environments/linux-memmap" or some other doc about it.

Thanks for pointing that out!

I'll rephrase that, so others willing to do autopkgtest may consider testing with memmap as well. If the "-e" option works, I think most of the scripts being included shall work. I might have to "skip" some tests whenever memcg is being used if they're not good.

Since these tests are likely not being ran in bileto, for every new merge/fix, as neither kvm nvdimm emulation nor kernel cmdlines can be provided in that environment, I'll keep things as is for now, if everyone agrees (after changing the commit log).

I haven't tested all scripts I'm including with "memmap=" cmdline because I was also worried about supporting QEMU nvdimm support, so I was killing 2 birds w/ a single stone in this effort.

So, for now, the regressions test would be done manually anyway.

For a next step, when merging this back to Debian (so I can put the package back to syncpackage), I, perhaps, could spend more time into this and split those tests that would work with memmap= , those that would work with both, memmap= and qemu emulation, and those only working with HW.

Sounds good for you ?

Revision history for this message
Rafael David Tinoco (rafaeldtinoco) wrote :

I'm changing it to:

    ubuntu: convert ndctl/test into qa-regression-ndctl tests

    Adding regression tests based in existing upstream tests. While upstream
    is worried about testing the entire pmem/dax stack (including kernel
    libnvdimm using a mocked driver) our need is much simpler: to guarantee
    the userland tool is good. This patch creates an autopkgtests with the
    most important upstream tests to satisfy the need.

    These tests were developed and tested using qemu emulated nvdimms
    (https://github.com/qemu/qemu/blob/master/docs/nvdimm.txt), so a file
    called "vm-example.xml" is provided.

    Tests may also work with the kernel emulation: https://docs.pmem.io/
    getting-started-guide/creating-development-environments/linux-
    environments/linux-memmap), but need more tests, as the kernel emulated
    option does not provide labels AND Ubuntu kernel has by default
    CONFIG_RANDOMIZE_BASE=y and that might cause kernel to interfere in
    memory previously reserved for persistent memory.

So it is more accurate!

Thanks! let me know if you have any other suggestions please.

Revision history for this message
Rafael David Tinoco (rafaeldtinoco) wrote :
Revision history for this message
Rafael David Tinoco (rafaeldtinoco) wrote :

I have changed the merge request as it had the wrong LP # on it (just realized that). I have included Adam's suggestions and will, in a near future, differentiate tests working with kernel cmdline from these working in a virtual environment (both outside the automated bileto scope).

Waiting for the MIR to continue (reviewing this changes).

Joy Latten (j-latten)
Changed in ndctl (Ubuntu):
assignee: Ubuntu Security Team (ubuntu-security) → nobody
Revision history for this message
Joy Latten (j-latten) wrote :
Download full text (4.8 KiB)

I reviewed ndctl as checked into focal. This shouldn't be considered a full audit but rather a quick gauge of maintainability.

ndctl is comprised of utilities and libraries for managing the libnvdimm (non-volatile memory device) sub-system in the Linux kernel

- No CVEs readily found.
  Gleaned the git repository, https://github.com/pmem/ndctl. Appears to be actively maintained.
  Security-wise, noted fixes for a memory leak and non-null terminated strings.
- Build-Depends: debhelper-compat (= 12), pkg-config, libkmod-dev, libudev-dev, uuid-dev,
  libjson-c-dev, bash-completion, systemd, libkeyutils-dev, asciidoctor
- No pre/post inst/rm scripts.
- There is an init script, debian/ndctl.init that is is installed as /etc/init.d/ndctl-monitor.
  All actions are circumvented to systemctl.
- There is a systemd unit file, ndctl-monitor.service, for the ndctl monitor daemon. The daemon
  catches smart events notify from firmware and outputs the notifications (in json format) to a
  logfile.
- No dbus services.
- No setuid binaries.
- 2 binaries, ndctl and daxctl in /usr/bin
- No sudo fragments.
- No udev rules.
- There are unit-tests and autopkgtests. The unit tests were skipped. There has been considerable
  discussion in this bugreport about providing regression testing.
- No cron jobs.
- Build reported following...
  - configure: WARNING: unrecognized options: --disable-maintainer-mode
  - quite a few alignment warnings for "address-of-packed-member",
    i.e.,
nfit.c: In function ‘ndctl_bus_cmd_new_translate_spa’:
nfit.c:65:25: warning: taking address of packed member of ‘struct nd_cmd_translate_spa’ may result in an unaligned pointer value [-Waddress-of-packed-member]
   65 | cmd->firmware_status = &translate_spa->status;
      | ^~~~~~~~~~~~~~~~~~~~~~

  - following lintian warnings,
    - malformed-deb-archive newer compressed control.tar.xz
    - init.d-script-uses-usr-interpreter etc/init.d/ndctl-monitor /usr/bin/env
E: ndctl: init.d-script-does-not-implement-required-option etc/init.d/ndctl-monitor start
E: ndctl: init.d-script-does-not-implement-required-option etc/init.d/ndctl-monitor stop
E: ndctl: init.d-script-does-not-implement-required-option etc/init.d/ndctl-monitor restart
E: ndctl: init.d-script-does-not-implement-required-option etc/init.d/ndctl-monitor force-reload
W: ndctl: unusual-interpreter etc/init.d/ndctl-monitor #!/lib/init/init-d-script
W: ndctl: init.d-script-does-not-source-init-functions etc/init.d/ndctl-monitor

   - following dpkg warnings
dpkg-shlibdeps: warning: package could avoid a useless dependency if debian/daxctl/usr/bin/daxctl was not linked against libndctl.so.6 (it uses none of the library's symbols)
dpkg-shlibdeps: warning: package could avoid a useless dependency if debian/daxctl/usr/bin/daxctl was not linked against libuuid.so.1 (it uses none of the library's symbols)

- execlp() called without an absolute path to bring up help pages. A call to "kfmclient" and
  once to call "man".
- Inspecting a random sampling of memory mgmt routines, the memory allocation looked good;
  memcpy() ok; none of the sprintf() nor asprintf() checked return value.
- File IO looked ok.
- Loggin...

Read more...

Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

bug 1853506 and bug 1790856 are ready (process-wise) when you are @ahasenack.
As checked on IRC, let me know when all is ready to add the dependency pulling it in.

Changed in ndctl (Ubuntu):
assignee: nobody → Andreas Hasenack (ahasenack)
status: New → In Progress
Revision history for this message
Andreas Hasenack (ahasenack) wrote :

I'm having a hard time in getting a VM up and running with the correct support for the ndctl commands, and I'm not sure what is going on. @paelzer also tried, and got the same error. Basically any enable-namespace or create-namespace command fails complaining about "no such device".

I also tried using just one emulated nvdimm device, given comment #11, but it still fails.

I tried both bionic (ndctl 61.2, also the one from Tinoco's ppa; kernel 4.15.0-76-generic #86), and focal-devel (ndctl 67.1, kernel 5.4.0-9-generic #12).

I can enable and disable dimms, regions, mess with labels, but namespaces can only be listed (disabled) and disabled (they are already disabled). I cannot enable or create them.

Revision history for this message
Andreas Hasenack (ahasenack) wrote :

This is the libvirt vm definition: https://pastebin.ubuntu.com/p/tjzwHbJMpT/

Here are some commands I tried and their results: https://pastebin.ubuntu.com/p/3Sn74Kp8KH/

Revision history for this message
Andreas Hasenack (ahasenack) wrote :

I also tried rebuilding qemu with pmem support, but qemu is currently an FTBFS in focal (https://launchpad.net/ubuntu/+source/qemu/1:4.0+dfsg-0ubuntu11)

Revision history for this message
Dan Williams (djbw) wrote :

@andreas, can you paste a full dmesg after turning on driver debug and retrying the create-namespace? Either boot with:

    libnvdimm.dyndbg=+fp

...on the kernel command line or:

    echo "module libnvdimm +fp" > /sys/kernel/debug/dynamic_debug/control

Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

@andreas - you'll have a pmem enabled qemu in PPA: https://launchpad.net/~ci-train-ppa-service/+archive/ubuntu/3883

Revision history for this message
Andreas Hasenack (ahasenack) wrote :
Download full text (4.0 KiB)

I get nothing in dmesg when I run enable-namespace all:
root@focal-nvdimm:~# ndctl enable-namespace all
libndctl: ndctl_namespace_enable: namespace1.0: failed to enable
libndctl: ndctl_namespace_enable: namespace0.0: failed to enable
error enabling namespaces: No such device or address
enabled 0 namespaces

But when I run create-namespace:
root@focal-nvdimm:~# ndctl create-namespace -v
libndctl: ndctl_pfn_enable: pfn1.0: failed to enable
  Error: namespace1.0: failed to enable

failed to create namespace: No such device or address

I get:
[Fri Jan 31 09:24:29 2020] init_dpa_allocation: nd_region region1: nmem1: pmem-reserve: 0x3fe00000 @ 0x0 init 0
[Fri Jan 31 09:24:29 2020] del_labels: nvdimm nmem1: no more active labels
[Fri Jan 31 09:24:29 2020] uuid_store: nd namespace1.0: result: 0 wrote: df662f5c-56d0-46c6-9feb-b8a51aa53e8c
[Fri Jan 31 09:24:29 2020] del_labels: nvdimm nmem1: no more active labels
[Fri Jan 31 09:24:29 2020] alt_name_store: nd namespace1.0: (0)
[Fri Jan 31 09:24:29 2020] init_dpa_allocation: nd_region region1: nmem1: pmem-reserve: 0x3fe00000 @ 0x0 init 0
[Fri Jan 31 09:24:29 2020] init_dpa_allocation: nd_region region1: nmem1: pmem-df662f5c: 0x3fe00000 @ 0x0 init 0
[Fri Jan 31 09:24:29 2020] __pmem_label_update: nvdimm nmem1: allocated: 0
[Fri Jan 31 09:24:29 2020] __pmem_label_update: nd_region region1: nmem1: pmem-df662f5c: 0x3fe00000 @ 0x0
[Fri Jan 31 09:24:29 2020] __pmem_label_update: nvdimm nmem1: allocated: 1
[Fri Jan 31 09:24:29 2020] __pmem_label_update: nd_region region1: nmem1: pmem-df662f5c: 0x3fe00000 @ 0x0
[Fri Jan 31 09:24:29 2020] reap_victim: nvdimm nmem1: free: 0
[Fri Jan 31 09:24:29 2020] size_store: nd namespace1.0: 3fe00000 success (0)
[Fri Jan 31 09:24:29 2020] __pmem_label_update: nvdimm nmem1: allocated: 0
[Fri Jan 31 09:24:29 2020] __pmem_label_update: nd_region region1: nmem1: pmem-df662f5c: 0x3fe00000 @ 0x0
[Fri Jan 31 09:24:29 2020] reap_victim: nvdimm nmem1: free: 1
[Fri Jan 31 09:24:29 2020] __pmem_label_update: nvdimm nmem1: allocated: 1
[Fri Jan 31 09:24:29 2020] __pmem_label_update: nd_region region1: nmem1: pmem-df662f5c: 0x3fe00000 @ 0x0
[Fri Jan 31 09:24:29 2020] reap_victim: nvdimm nmem1: free: 0
[Fri Jan 31 09:24:29 2020] sector_size_store: nd namespace1.0: result: 0 wrote: 512

[Fri Jan 31 09:24:29 2020] __pmem_label_update: nvdimm nmem1: allocated: 0
[Fri Jan 31 09:24:29 2020] __pmem_label_update: nd_region region1: nmem1: pmem-df662f5c: 0x3fe00000 @ 0x0
[Fri Jan 31 09:24:29 2020] reap_victim: nvdimm nmem1: free: 1
[Fri Jan 31 09:24:29 2020] __pmem_label_update: nvdimm nmem1: allocated: 1
[Fri Jan 31 09:24:29 2020] __pmem_label_update: nd_region region1: nmem1: pmem-df662f5c: 0x3fe00000 @ 0x0
[Fri Jan 31 09:24:29 2020] reap_victim: nvdimm nmem1: free: 0
[Fri Jan 31 09:24:29 2020] holder_class_store: nd namespace1.0: (0)
[Fri Jan 31 09:24:29 2020] uuid_store: nd pfn1.0: result: 0 wrote: 3a2b9e27-4b1a-4b60-bc7d-08e21926496b
[Fri Jan 31 09:24:29 2020] mode_store: nd pfn1.0: result: 0 wrote: pmem
[Fri Jan 31 09:24:29 2020] align_store: nd pfn1.0: result: 0 wrote: 2097152

[Fri Jan 31 09:24:29 2020] namespace_store: nd pfn1.0: result: 13 wrote: namespace1.0
[Fri Jan 31 09:24:29 20...

Read more...

Revision history for this message
Dan Williams (djbw) wrote :

@andreas this almost looks like the pmem module is missing. Can you paste the full dmesg on pastebin and verify that the driver is present?

Revision history for this message
Andreas Hasenack (ahasenack) wrote :

@djbw do you mean a kernel module?

Here is the full dmesg. I just logged in and ran "sudo ndctl create-namespace":

https://paste.ubuntu.com/p/Mf2g282JNQ/

lsmod: https://paste.ubuntu.com/p/GqTvskG7RJ/

/proc/iomem (it shows some lines with "Persistent Memory"): https://paste.ubuntu.com/p/K43GVhfQcc/

Revision history for this message
Andreas Hasenack (ahasenack) wrote :

hah, bingo, the default kernel package for VMs doesn't have nd_pmem or nd_btt. I had to install linux-modules-extra-5.4.0-12-generic.

Now it works:
ubuntu@focal-nvdimm:~$ sudo ndctl create-namespace
{
  "dev":"namespace1.0",
  "mode":"fsdax",
  "map":"dev",
  "size":"1006.00 MiB (1054.87 MB)",
  "uuid":"53defad5-c13c-4f3c-abbb-d9152db43c69",
  "sector_size":512,
  "align":2097152,
  "blockdev":"pmem1"
}

Thanks a lot, Dan!

Revision history for this message
Rafael David Tinoco (rafaeldtinoco) wrote :

Oh thats nice to hear that... I was getting worried (coming back from
vacation and reading backlog). Thanks Andreas for testing this and Dan
for the help.

On Fri, Jan 31, 2020 at 10:41 PM Andreas Hasenack <email address hidden> wrote:
>
> hah, bingo, the default kernel package for VMs doesn't have nd_pmem or
> nd_btt. I had to install linux-modules-extra-5.4.0-12-generic.
>
> Now it works:
> ubuntu@focal-nvdimm:~$ sudo ndctl create-namespace
> {
> "dev":"namespace1.0",
> "mode":"fsdax",
> "map":"dev",
> "size":"1006.00 MiB (1054.87 MB)",
> "uuid":"53defad5-c13c-4f3c-abbb-d9152db43c69",
> "sector_size":512,
> "align":2097152,
> "blockdev":"pmem1"
> }
>
>
> Thanks a lot, Dan!
>
> --
> You received this bug notification because you are subscribed to the bug
> report.
> https://bugs.launchpad.net/bugs/1853506
>
> Title:
> [MIR] ndctl
>
> To manage notifications about this bug go to:
> https://bugs.launchpad.net/ubuntu/+source/ndctl/+bug/1853506/+subscriptions

Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

\o/ glad you found this!

@andreas:
While I was at PMDK adding one more was easy, MP suggestion for the seed change: https://code.launchpad.net/~paelzer/ubuntu-seeds/+git/platform/+merge/378436

Revision history for this message
Andreas Hasenack (ahasenack) wrote :

Quick update on progress. Of all these tests that were written:
blk-exhaust
btt-check
btt-pad-compat
dax
dax-ext4
dax-xfs
daxctl-commands
label-compat
multi-dax
ndctl-commands
rescan-partitions
sector-mode

Only multi-dax and sector-mode are failing on bionic, and rescan-partitions is being skipped (too old kernel). In focal, they all passed. This is plenty to continue.

Revision history for this message
Andreas Hasenack (ahasenack) wrote :
Revision history for this message
Steve Beattie (sbeattie) wrote :

Andreas' merge proposal for adding tests to q-r-t has been merged in to master. Thanks Andreas!

Revision history for this message
Sebastien Bacher (seb128) wrote :

$ ./change-override -c main -t ndctl
Override component to main
ndctl 67-1 in focal: universe/libs -> main
Override [y|N]? y
1 publication overridden.

$ ./change-override -c main libdaxctl-dev libdaxctl1 libndctl-dev libndctl6 ndctl
Override component to main
libdaxctl-dev 67-1 in focal amd64: universe/libdevel/optional/100% -> main
libdaxctl-dev 67-1 in focal arm64: universe/libdevel/optional/100% -> main
libdaxctl-dev 67-1 in focal armhf: universe/libdevel/optional/100% -> main
libdaxctl-dev 67-1 in focal i386: universe/libdevel/optional/100% -> main
libdaxctl-dev 67-1 in focal ppc64el: universe/libdevel/optional/100% -> main
libdaxctl-dev 67-1 in focal s390x: universe/libdevel/optional/100% -> main
libdaxctl1 67-1 in focal amd64: universe/libs/optional/100% -> main
libdaxctl1 67-1 in focal arm64: universe/libs/optional/100% -> main
libdaxctl1 67-1 in focal armhf: universe/libs/optional/100% -> main
libdaxctl1 67-1 in focal i386: universe/libs/optional/100% -> main
libdaxctl1 67-1 in focal ppc64el: universe/libs/optional/100% -> main
libdaxctl1 67-1 in focal s390x: universe/libs/optional/100% -> main
libndctl-dev 67-1 in focal amd64: universe/libdevel/optional/100% -> main
libndctl-dev 67-1 in focal arm64: universe/libdevel/optional/100% -> main
libndctl-dev 67-1 in focal armhf: universe/libdevel/optional/100% -> main
libndctl-dev 67-1 in focal i386: universe/libdevel/optional/100% -> main
libndctl-dev 67-1 in focal ppc64el: universe/libdevel/optional/100% -> main
libndctl-dev 67-1 in focal s390x: universe/libdevel/optional/100% -> main
libndctl6 67-1 in focal amd64: universe/libs/optional/100% -> main
libndctl6 67-1 in focal arm64: universe/libs/optional/100% -> main
libndctl6 67-1 in focal armhf: universe/libs/optional/100% -> main
libndctl6 67-1 in focal i386: universe/libs/optional/100% -> main
libndctl6 67-1 in focal ppc64el: universe/libs/optional/100% -> main
libndctl6 67-1 in focal s390x: universe/libs/optional/100% -> main
ndctl 67-1 in focal amd64: universe/misc/optional/100% -> main
ndctl 67-1 in focal arm64: universe/misc/optional/100% -> main
ndctl 67-1 in focal armhf: universe/misc/optional/100% -> main
ndctl 67-1 in focal i386: universe/misc/optional/100% -> main
ndctl 67-1 in focal ppc64el: universe/misc/optional/100% -> main
ndctl 67-1 in focal s390x: universe/misc/optional/100% -> main
Override [y|N]? y
30 publications overridden.

Changed in ndctl (Ubuntu):
status: In Progress → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.