[developer] Re: [openzfs/openzfs] Native data and metadata encryption for zfs (#124)

[Jorgen Lundman]

Looks like we managed to pass all tests finally. I plan to squash everything 
into one single commit unless there is some other number of commits people 
would prefer..?


-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/openzfs/openzfs/pull/124#issuecomment-336278865
--
openzfs-developer
Archives: 
https://openzfs.topicbox.com/groups/developer/discussions/Ta17fb0edb29e7895-M467009f7541d443069b2c2e3
Powered by Topicbox: https://topicbox.com

Re: [developer] Re: [openzfs/openzfs] Native data and metadata encryption for zfs (#124)

[Udo Grabowski (IMK)]

On 12/10/2017 12:47, Joerg Schilling wrote:

Jorgen Lundman  wrote:


I prefer `print` myself, but there were no uses of `print` in existing 
zfs-tester test files, whereas there was precedent in using `tr -d` in 
`zfs_create_014_pos`.


"print" is non standard and tus non-portable!

"print" is a ksh-ism that was intentionally not adopted by POSIX.

Jörg



But 'printf' is standardized, maybe that's a good replacement.
--
Dr.Udo Grabowski   Inst.f.Meteorology & Climate Research IMK-ASF-SAT
http://www.imk-asf.kit.edu/english/sat.php
KIT - Karlsruhe Institute of Technology   http://www.kit.edu
Postfach 3640,76021 Karlsruhe,Germany T:(+49)721 608-26026 F:-926026



smime.p7s
Description: S/MIME Cryptographic Signature


--
openzfs-developer
Archives: 
https://openzfs.topicbox.com/groups/developer/discussions/Ta17fb0edb29e7895-Mbf3d212c296139e9b4141be3
Powered by Topicbox: https://topicbox.com

Re: [developer] Re: [openzfs/openzfs] Native data and metadata encryption for zfs (#124)

[Joerg Schilling]

Jorgen Lundman  wrote:

> I prefer `print` myself, but there were no uses of `print` in existing 
> zfs-tester test files, whereas there was precedent in using `tr -d` in 
> `zfs_create_014_pos`. 

"print" is non standard and tus non-portable!

"print" is a ksh-ism that was intentionally not adopted by POSIX.

Jörg

-- 
 EMail:jo...@schily.net(home) Jörg Schilling D-13353 Berlin
joerg.schill...@fokus.fraunhofer.de (work) Blog: http://schily.blogspot.com/
 URL: http://cdrecord.org/private/ http://sf.net/projects/schilytools/files/'

--
openzfs-developer
Archives: 
https://openzfs.topicbox.com/groups/developer/discussions/Ta17fb0edb29e7895-M949c42f145b80c314edb133a
Powered by Topicbox: https://topicbox.com

[developer] Re: [openzfs/openzfs] Native data and metadata encryption for zfs (#124)

[Jorgen Lundman]

@lundman pushed 2 commits.

53406a2  More corrections to testers
bf0d5b0  Disable dump on encrypted zvol


-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/openzfs/openzfs/pull/124/files/14424ba0610e4ae07057a630538a1c1860dcc3a0..bf0d5b058d816c14a0934bb67bde6b9aa1b82d92

--
openzfs-developer
Archives: 
https://openzfs.topicbox.com/groups/developer/discussions/Ta17fb0edb29e7895-M00763155b5b92eab3194a23b
Powered by Topicbox: https://topicbox.com

[developer] Re: [openzfs/openzfs] Native data and metadata encryption for zfs (#124)

[Alex Wilson]

I think you missed this from ZOL in the latest update:

```diff
diff --git a/usr/src/uts/common/fs/zfs/bptree.c 
b/usr/src/uts/common/fs/zfs/bptree.c
index c74d072..25c08f6 100644
--- a/usr/src/uts/common/fs/zfs/bptree.c
+++ b/usr/src/uts/common/fs/zfs/bptree.c
@@ -211,7 +211,8 @@ bptree_iterate(objset_t *os, uint64_t obj, boolean_t free, 
bptree_itor_t func,
err = 0;
for (i = ba.ba_phys->bt_begin; i < ba.ba_phys->bt_end; i++) {
bptree_entry_phys_t bte;
-   int flags = TRAVERSE_PREFETCH_METADATA | TRAVERSE_POST;
+   int flags = TRAVERSE_PREFETCH_METADATA | TRAVERSE_POST |
+   TRAVERSE_NO_DECRYPT;
 
err = dmu_read(os, obj, i * sizeof (bte), sizeof (bte),
, DMU_READ_NO_PREFETCH);
```

(without it, after the recent change to turn async destroy back on for 
encrypted datasets, you get a panic)

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/openzfs/openzfs/pull/124#issuecomment-335667214
--
openzfs-developer
Archives: 
https://openzfs.topicbox.com/groups/developer/discussions/Ta17fb0edb29e7895-Md040efcd6b0b2dadef41217b
Powered by Topicbox: https://topicbox.com

[developer] Re: [openzfs/openzfs] Native data and metadata encryption for zfs (#124)

[Jorgen Lundman]

@lundman pushed 1 commit.

599316e  OpenZFS repo runs delphix.run in tests, so update it to match


-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/openzfs/openzfs/pull/124/files/fba7a78847ee76f1c86db525e50e44a710575e15..599316ee7968e43c125cbd1d40bffc91099dbdea

--
openzfs-developer
Archives: 
https://openzfs.topicbox.com/groups/developer/discussions/Ta17fb0edb29e7895-M9d498203c2740a0c13c6ee70
Powered by Topicbox: https://topicbox.com

[developer] Re: [openzfs/openzfs] Native data and metadata encryption for zfs (#124)

[GernotS]

sorry to report that send/receive (see Aug. 18th/19th) still crashes when 
testing with more than 10GB of data:

panic[cpu1]/thread=ff000faf4c40:
BAD TRAP: type=e (#pf Page fault) rp=ff000faf4870 addr=68 occurred in 
module "zfs" due to a NULL pointer dereference
sched:
#pf Page fault
Bad kernel fault at addr=0x68
pid=0, pc=0xf7d49176, sp=0xff000faf4960, eflags=0x10202
cr0: 8005003b cr4: 6f8
cr2: 68
cr3: 1dc0
cr8: c

rdi:   68 rsi: ff03dde12120 rdx: ff000faf4c40
rcx:d  r8: ff03db3662a0  r9:0
rax:0 rbx: ff03dde12120 rbp: ff000faf4970
r10: fb858740 r11: fb800983 r12:0
r13: c001af00 r14: c001e890 r15: ff03dde12120
fsb:0 gsb: ff03c9b89080  ds:   4b
 es:   4b  fs:0  gs:  1c3
trp:e err:0 rip: f7d49176
 cs:   30 rfl:10202 rsp: ff000faf4960
 ss:   38

ff000faf4750 unix:real_mode_stop_cpu_stage2_end+b21c ()
ff000faf4860 unix:trap+e08 ()
ff000faf4870 unix:_cmntrap+e6 ()
ff000faf4970 zfs:arc_buf_remove+16 ()
ff000faf49b0 zfs:arc_buf_destroy_impl+bd ()
ff000faf4a10 zfs:arc_evict_hdr+b3 ()
ff000faf4aa0 zfs:arc_evict_state_impl+154 ()
ff000faf4b40 zfs:arc_evict_state+12e ()
ff000faf4b70 zfs:arc_adjust_impl+44 ()
ff000faf4ba0 zfs:arc_adjust+d9 ()
ff000faf4c20 zfs:arc_reclaim_thread+ae ()
ff000faf4c30 unix:thread_start+8 ()


-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/openzfs/openzfs/pull/124#issuecomment-334371066
--
openzfs-developer
Archives: 
https://openzfs.topicbox.com/groups/developer/discussions/Ta17fb0edb29e7895-Mcf62774af91f4436a8fac9ff
Powered by Topicbox: https://topicbox.com

[developer] Re: [openzfs/openzfs] Native data and metadata encryption for zfs (#124)

[Jorgen Lundman]

This PR has been updated with latest commits up and including Oct 2nd 
"060e4cf33002c4".

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/openzfs/openzfs/pull/124#issuecomment-334077937
--
openzfs-developer
Archives: 
https://openzfs.topicbox.com/groups/developer/discussions/Ta17fb0edb29e7895-M9266d6889a7b0e2abf0b2b3b
Powered by Topicbox: https://topicbox.com

[developer] Re: [openzfs/openzfs] Native data and metadata encryption for zfs (#124)

[ludminer]

Still crashing with send/receive. Not related to RAIDZ, also happens with 
single disk pool.
btw dmu_send.c, patch does not apply cleanly here:
struct drr_object *drro = >rrd->header.drr_u.drr_object;
-   uint32_t size = P2ROUNDUP(drro->drr_bonuslen, 8);
+   uint32_t size = DRR_OBJECT_PAYLOAD_SIZE(drro);

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/openzfs/openzfs/pull/124#issuecomment-324828278
--
openzfs-developer
Archives: 
https://openzfs.topicbox.com/groups/developer/discussions/Ta17fb0edb29e7895-M92dab5ac738c2df1cd1f028b
Powered by Topicbox: https://topicbox.com

[developer] Re: [openzfs/openzfs] Native data and metadata encryption for zfs (#124)

[Jorgen Lundman]

@lundman pushed 1 commit.

807e3db  Minor code style changes


-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/openzfs/openzfs/pull/124/files/0b4ee95274efe6e3103402a8d390e9206cca2669..807e3dbdbda5090243d8d534c010ee6ccb34187a

--
openzfs-developer
Archives: 
https://openzfs.topicbox.com/groups/developer/discussions/Ta17fb0edb29e7895-M9b3463f47a33a0f1de59c2cc
Powered by Topicbox: https://topicbox.com

[developer] Re: [openzfs/openzfs] Native data and metadata encryption for zfs (#124)

[Jorgen Lundman]

@lundman pushed 1 commit.

0b4ee95  prevent panic when non-raidz checksum error detected


-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/openzfs/openzfs/pull/124/files/57291b074f93cd588106df1a5c0147778c600c6e..0b4ee95274efe6e3103402a8d390e9206cca2669

--
openzfs-developer
Archives: 
https://openzfs.topicbox.com/groups/developer/discussions/Ta17fb0edb29e7895-Mc3c3456d80c51e051b09e37f
Powered by Topicbox: https://topicbox.com

[developer] Re: [openzfs/openzfs] Native data and metadata encryption for zfs (#124)

[Paul Dagnelie]

pcd1193182 requested changes on this pull request.

I just looked through the zfs send related code, but that part of it seemed 
pretty good to me.

> @@ -1332,9 +1533,17 @@ recv_begin_check_existing_impl(dmu_recv_begin_arg_t 
> *drba, dsl_dataset_t *ds,
/* if full, then must be forced */
if (!drba->drba_cookie->drc_force)
return (SET_ERROR(EEXIST));
-   /* start from $ORIGIN@$ORIGIN, if supported */
-   drba->drba_snapobj = dp->dp_origin_snap != NULL ?

Do we not still want to start from $ORIGIN@$ORIGIN for non-encrypted receives?

>   return (SET_ERROR(ENODEV));
}
-   dsl_dataset_rele(origin, FTAG);
+   dsl_dataset_rele_flags(origin,
+   dsflags, FTAG);

Unflow this line?

if (drrb->drr_flags & DRR_FLAG_CI_DATA)
crflags |= DS_FLAG_CI_DATASET;
+   if ((featureflags & DMU_BACKUP_FEATURE_RAW) == 0) {

I don't much care either way, but for 1-line if statements we should probably 
try to keep brace/no brace consistency within the same function.

-   VERIFY0(dsl_dataset_own_obj(dp, dsobj, dmu_recv_tag, ));
+   VERIFY0(dsl_dataset_own_obj(dp, dsobj, dsflags, dmu_recv_tag, ));
+   VERIFY0(dmu_objset_from_ds(ds, ));

Why do this here? It doesn't look like we use the os anywhere.

> +
+   /*
+* By default, we assume this block is in our native format
+* (ZFS_HOST_BYTEORDER). We then take into account whether
+* the send stream is byteswapped (rwa->byteswap). Finally,
+* we need to byteswap again if this particular block was
+* in non-native format on the send side.
+*/
+   boolean_t byteorder = ZFS_HOST_BYTEORDER ^ rwa->byteswap ^
+   !!DRR_IS_RAW_BYTESWAPPED(drror->drr_flags);
+
+   /*
+* Since dnode block sizes are constant, we should not need to worry
+* about making sure that the dnode block size is the same on the
+* sending and receiving sides for the time being. For non-raw sends,
+* this does not matter (and in fact we do not send a DRR_OBJECT_RANGE

Given that we should never receive one of these for non-raw sends, do we want 
to add a check for that?

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/openzfs/openzfs/pull/124#pullrequestreview-58530651
--
openzfs-developer
Archives: 
https://openzfs.topicbox.com/groups/developer/discussions/Ta17fb0edb29e7895-Mffadea297adbdd73caa9ff3d
Powered by Topicbox: https://topicbox.com

[developer] Re: [openzfs/openzfs] Native data and metadata encryption for zfs (#124)

[Paul Dagnelie]

pcd1193182 commented on this pull request.



> @@ -1332,9 +1533,17 @@ recv_begin_check_existing_impl(dmu_recv_begin_arg_t 
> *drba, dsl_dataset_t *ds,
/* if full, then must be forced */
if (!drba->drba_cookie->drc_force)
return (SET_ERROR(EEXIST));
-   /* start from $ORIGIN@$ORIGIN, if supported */
-   drba->drba_snapobj = dp->dp_origin_snap != NULL ?

Do we not still want to start from $ORIGIN@$ORIGIN for non-encrypted receives?

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/openzfs/openzfs/pull/124#pullrequestreview-58530618
--
openzfs-developer
Archives: 
https://openzfs.topicbox.com/groups/developer/discussions/Ta17fb0edb29e7895-Mda52c1777eb90043f2d73a52
Powered by Topicbox: https://topicbox.com

[developer] Re: [openzfs/openzfs] Native data and metadata encryption for zfs (#124)

[Jorgen Lundman]

Now contains the send/recv fixes from ZOL. Please test.


-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/openzfs/openzfs/pull/124#issuecomment-324526413
--
openzfs-developer
Archives: 
https://openzfs.topicbox.com/groups/developer/discussions/Ta17fb0edb29e7895-M308c690cb9f77387326d751f
Powered by Topicbox: https://topicbox.com

[developer] Re: [openzfs/openzfs] Native data and metadata encryption for zfs (#124)

[Jorgen Lundman]

@lundman pushed 1 commit.

57291b0  de-linting


-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/openzfs/openzfs/pull/124/files/8e2c3b855a7eb155744586ec610d642c2c46a5c9..57291b074f93cd588106df1a5c0147778c600c6e

--
openzfs-developer
Archives: 
https://openzfs.topicbox.com/groups/developer/discussions/T1625245905c55186-M77ee3f4745b532d33becaa0e
Powered by Topicbox: https://topicbox.com

[developer] Re: [openzfs/openzfs] Native data and metadata encryption for zfs (#124)

[Jorgen Lundman]

I can confirm that a `send --raw plain` into encrypted panics, which shouldn't 
be allowed.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/openzfs/openzfs/pull/124#issuecomment-323559993
--
openzfs-developer
Archives: 
https://openzfs.topicbox.com/groups/developer/discussions/T1625245905c55186-M2a4681e03b79c37c0851fec1
Powered by Topicbox: https://topicbox.com

[developer] Re: [openzfs/openzfs] Native data and metadata encryption for zfs (#124)

[GernotS]

exactly. But you might need more than 10GB data to transfer.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/openzfs/openzfs/pull/124#issuecomment-323513670
--
openzfs-developer
Archives: 
https://openzfs.topicbox.com/groups/developer/discussions/T1625245905c55186-M947dd7ef5aacc17cc2fa75b8
Powered by Topicbox: https://topicbox.com

[developer] Re: [openzfs/openzfs] Native data and metadata encryption for zfs (#124)

[Jorgen Lundman]

Ah volumes, so something like
```
# zpool create -f BOOM c2t3d0
# zfs create BOOM/plain
# zfs create -o encryption=on -o keyformat=passphrase BOOM/ccm
Enter passphrase:
Re-enter passphrase:

# zfs create -V 1G BOOM/vol
# zfs snapshot BOOM/vol@send
# zfs send BOOM/vol@send | zfs recv BOOM/ccm/evol
# zfs get keystatus
NAME PROPERTY   VALUESOURCE
BOOM/ccm keystatus  available-
BOOM/ccm/evolkeystatus  available-
BOOM/ccm/evol@send   keystatus  available-
BOOM/vol keystatus  --
BOOM/vol@sendkeystatus  --
```

?


-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/openzfs/openzfs/pull/124#issuecomment-323487498
--
openzfs-developer
Archives: 
https://openzfs.topicbox.com/groups/developer/discussions/T1625245905c55186-M6c4b93d6e811815416089841
Powered by Topicbox: https://topicbox.com

[developer] Re: [openzfs/openzfs] Native data and metadata encryption for zfs (#124)

[GernotS]

I am sending a unencrypted zvol to an encrypted pool.
Now the crash is different:

panic[cpu0]/thread=ff000fb50c40:
BAD TRAP: type=e (#pf Page fault) rp=ff000fb50540 addr=68 occurred in 
module "zfs" due to a NULL pointer dereference


zpool-zones:
#pf Page fault
Bad kernel fault at addr=0x68
pid=89, pc=0xf7d46a06, sp=0xff000fb50630, eflags=0x10202
cr0: 8005003b cr4: 6f8
cr2: 68
cr3: 1dc0
cr8: c

rdi:   68 rsi: ff03d1b85d38 rdx: ff000fb50c40
rcx:a  r8: ff03efa321d8  r9:0
rax:0 rbx: ff03d1b85d38 rbp: ff000fb50640
r10:3 r11: fb800983 r12:0
r13: ff03d1248b78 r14: ff03ce26b1e8 r15: 5d00
fsb:0 gsb: fbc398e0  ds:   4b
 es:   4b  fs:0  gs:  1c3
trp:e err:0 rip: f7d46a06
 cs:   30 rfl:10202 rsp: ff000fb50630
 ss:   38

ff000fb50420 unix:real_mode_stop_cpu_stage2_end+b1bc ()
ff000fb50530 unix:trap+e08 ()
ff000fb50540 unix:_cmntrap+e6 ()
ff000fb50640 zfs:arc_buf_remove+16 ()
ff000fb50680 zfs:arc_buf_destroy_impl+b9 ()
ff000fb506c0 zfs:arc_hdr_destroy+e5 ()
ff000fb50700 zfs:arc_freed+7b ()
ff000fb507d0 zfs:zio_free_sync+5c ()
ff000fb50830 zfs:zio_free+e4 ()
ff000fb50860 zfs:dsl_free+1c ()
ff000fb508e0 zfs:dsl_dataset_block_kill+25c ()
ff000fb50930 zfs:dbuf_write_done+142 ()
ff000fb509a0 zfs:arc_write_done+1a7 ()
ff000fb50a40 zfs:zio_done+3e2 ()
ff000fb50a70 zfs:zio_execute+7f ()
ff000fb50b30 genunix:taskq_thread+2d0 ()
ff000fb50b40 unix:thread_start+8 ()



-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/openzfs/openzfs/pull/124#issuecomment-323348523
--
openzfs-developer
Archives: 
https://openzfs.topicbox.com/groups/developer/discussions/T1625245905c55186-M9f42c656b3fd9c97da45d3bc
Powered by Topicbox: https://topicbox.com

[developer] Re: [openzfs/openzfs] Native data and metadata encryption for zfs (#124)

[Jorgen Lundman]

@zettabot go


-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/openzfs/openzfs/pull/124#issuecomment-323239573
--
openzfs-developer
Archives: 
https://openzfs.topicbox.com/groups/developer/discussions/T1625245905c55186-Mb3c0113e02ea05630ad85f36
Powered by Topicbox: https://topicbox.com

[developer] Re: [openzfs/openzfs] Native data and metadata encryption for zfs (#124)

[Jorgen Lundman]

@GernotS  are these the commands used? I've tried it with latest commit:

```
# zpool create -f BOOM c2t3d0
# zfs create -o encryption=aes-256-ccm -o keyformat=passphrase BOOM/ccm
# zfs create BOOM/plain
# zfs snapshot BOOM/plain@send
# zfs send BOOM/plain@send | zfs recv BOOM/ccm/new
# zfs get keystatus

BOOM keystatus  --
BOOM/ccm keystatus  available-
BOOM/ccm/new keystatus  available-
BOOM/ccm/new@sendkeystatus  available-
BOOM/plain   keystatus  --
BOOM/plain@send  keystatus  --
```


-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/openzfs/openzfs/pull/124#issuecomment-323229283
--
openzfs-developer
Archives: 
https://openzfs.topicbox.com/groups/developer/discussions/T1625245905c55186-M96a25d65e260e11fb091d8eb
Powered by Topicbox: https://topicbox.com

[developer] Re: [openzfs/openzfs] Native data and metadata encryption for zfs (#124)

[GernotS]

still crashing when trying to zfs send/receive from an unencrypted to an 
encrypted dataset:
panic[cpu1]/thread=ff000feecc40:
assertion failed: spa_do_crypt_abd(B_TRUE, spa, zio->io_bookmark.zb_objset, bp, 
zio->io_txg, psize, zio->io_abd, eabd, iv, mac, salt, _crypt) == 0 (0x5 == 
0x0), file: ../../common/fs/zfs/zio.c, line: 3560


ff000feec950 fba7bcad ()
ff000feeca40 zfs:zio_encrypt+5cf ()
ff000feeca70 zfs:zio_execute+7f ()
ff000feecb30 genunix:taskq_thread+2d0 ()
ff000feecb40 unix:thread_start+8 ()

I can provide a coredump


-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/openzfs/openzfs/pull/124#issuecomment-321761104
--
openzfs-developer
Archives: 
https://openzfs.topicbox.com/groups/developer/discussions/T1625245905c55186-M686609df323268ccbe13118f
Powered by Topicbox: https://topicbox.com

[developer] Re: [openzfs/openzfs] Native data and metadata encryption for zfs (#124)

[Jorgen Lundman]

@lundman pushed 1 commit.

e418fcb  Small bug fixes


-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/openzfs/openzfs/pull/124/files/c26140e60da6f8a885be38fd339e9c5e2781007c..e418fcbb7204fafc5f0b77881739fa0289175352

--
openzfs-developer
Archives: 
https://openzfs.topicbox.com/groups/developer/discussions/T1625245905c55186-M8b134c04c20c6f7b12999f8c
Powered by Topicbox: https://topicbox.com

[developer] Re: [openzfs/openzfs] Native data and metadata encryption for zfs (#124)

[Matthew Ahrens]

@arekinath The panic should be fixed in this PR.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/openzfs/openzfs/pull/124#issuecomment-321363891
--
openzfs-developer
Archives: 
https://openzfs.topicbox.com/groups/developer/discussions/T1625245905c55186-M5ec2c20e6189695c0abebacc
Powered by Topicbox: https://topicbox.com

[developer] Re: [openzfs/openzfs] Native data and metadata encryption for zfs (#124)

[Alex Wilson]

@ahrens Ok. That works for me. Is the change to make that an error going to go 
in this repo and this PR? Or be a separate but we file and integrate just in 
illumos when we merge this up (seeing as it's illumos specific)?

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/openzfs/openzfs/pull/124#issuecomment-321336125
--
openzfs-developer
Archives: 
https://openzfs.topicbox.com/groups/developer/discussions/T1625245905c55186-Mb0d1f103bb29808bf004304b
Powered by Topicbox: https://topicbox.com

[developer] Re: [openzfs/openzfs] Native data and metadata encryption for zfs (#124)

[Matthew Ahrens]

@arekinath Yes, it should be an error to dumpify (i.e. disable 
checksums/encryption on) a zvol that has encryption on.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/openzfs/openzfs/pull/124#issuecomment-321333416
--
openzfs-developer
Archives: 
https://openzfs.topicbox.com/groups/developer/discussions/T1625245905c55186-Mf95713a4e17b3b043bbd17bd
Powered by Topicbox: https://topicbox.com

[developer] Re: [openzfs/openzfs] Native data and metadata encryption for zfs (#124)

[Matthew Ahrens]

I totally understand wanting to have encrypted dumps.

However, the current designs of dump and encryption are incompatible.  
Specifically: when dumping we do not modify any ZFS data structures; instead we 
just overwrite pre-allocated space.  Therefore dump doesn't have checksums, 
doesn't modify indirect blocks, and doesn't write out a TXG.  Encryption 
requires recording the IV in the block pointer, in the indirect block, which 
requires writing out a TXG.

We could implement encrypted dump in a different way than normal encryption, 
but have the administration seamlessly integrate with the current encryption.  
For example, we could record the IV (or IV's - and MAC) at the end of the dump, 
inside the dump zvol itself (in its data blocks, not the block pointers).

Until a special case encrypted dump is implemented, I think it would be 
dangerously confusing to automatically ignore an inherited `encryption=on` 
setting on a dump zvol.  I could see the argument for allowing unencrypted 
datasets under encrypted datasets, but if we do that we should allow it in 
general, not just for dump.

I would suggest that for now, we keep the current semantic (no unencrypted 
datasets under encrypted datasets).  If for Joyent's use case, they are OK with 
unencrypted dump, but need encryption set at the top of the pool, I'd suggest 
that an exception be added to the Joyent/SmartOS code to allow this (and 
hopefully @tcaputi or I can point you to the right place to do that or provide 
a patch).

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/openzfs/openzfs/pull/124#issuecomment-321329521
--
openzfs-developer
Archives: 
https://openzfs.topicbox.com/groups/developer/discussions/T1625245905c55186-M7bf751a6064ac11553e71157
Powered by Topicbox: https://topicbox.com

[developer] Re: [openzfs/openzfs] Native data and metadata encryption for zfs (#124)

[Alex Wilson]

Actually I'm gonna correct myself there a bit, I'm pretty sure after more 
reading that the thing that was too complex to have in dump code was not so 
much computing Fletcher4 itself, as updating the actual pool structure and 
committing a new txg (but hopefully someone who knows more about it will chime 
in here and correct me!). I think that's why ZFS pre-allocates the blocks used 
for dump and gives dump a list of LBAs to write to that have all checksumming 
and parity turned off -- so dump can just write there without updating any of 
the structure elsewhere in the pool.

So if that's true, it does seem plausible that maybe we can have something like 
a fixed dump encryption key that we give to the dump subsystem in a similar 
manner at the time we activate the dump device, and it has some very minimal 
code that does a single mode of encryption (probably unauthenticated?) just for 
this. It almost seems like a separate project though.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/openzfs/openzfs/pull/124#issuecomment-321308336
--
openzfs-developer
Archives: 
https://openzfs.topicbox.com/groups/developer/discussions/T1625245905c55186-Mba46b9174e31611d54d3
Powered by Topicbox: https://topicbox.com

[developer] Re: [openzfs/openzfs] Native data and metadata encryption for zfs (#124)

[Alex Wilson]

> Are you serious? I would definitely not want to expose plaintext dumps of 
> kernel memory if I'm otherwise encrypting the entire pool. (I don't know if 
> encryption keys can be dumped, but if so, that's obviously a huge problem)

I am, unfortunately, serious - there's a big tension here between the fact that 
we have to be able to write to dump during a panic and not wanting to write out 
plaintext data on disk ever. I'm not sure that we can confidently say we're 
always going to be able to use all the crypto bits in panic context - that code 
is extremely special so that it can keep working even in the face of all kinds 
of chaos: even certain kinds of storage driver malfunctions.

The dump code has to be kept super simple or it loses that reliability.

We can shred (overwrite randomly and zero) all that space after we come back 
up, or else maybe we could build some extremely simplified code specifically 
for encrypting dumps, but I mean... as I said, we have previously decided that 
even computing a Fletcher4 is too complex to have in that code.

I really think the easiest option will be to just make dump unencrypted and 
plaster a big warning sign all over it that it might ruin your plans to keep 
all plain data off disk if you include e.g. ARC in your dumped memory (which 
isn't by default as I understand it, but other kinds of transient plaintext in 
kernel memory will be of course)

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/openzfs/openzfs/pull/124#issuecomment-321295975
--
openzfs-developer
Archives: 
https://openzfs.topicbox.com/groups/developer/discussions/T1625245905c55186-M9bf7f25356405e44715098a9
Powered by Topicbox: https://topicbox.com

[developer] Re: [openzfs/openzfs] Native data and metadata encryption for zfs (#124)

[Lauri Tirkkonen]

Because dumps are useful for post-mortem debugging of production failures as 
well; OmniOS for example creates a dump device by default. Even if not, it's 
not nice to have a "trap" like that where the user thinks they're encrypting 
the entire disk but crash dumps happen to be special.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/openzfs/openzfs/pull/124#issuecomment-32160
--
openzfs-developer
Archives: 
https://openzfs.topicbox.com/groups/developer/discussions/T1625245905c55186-M302433b609ea8de37ca0fb46
Powered by Topicbox: https://topicbox.com

[developer] Re: [openzfs/openzfs] Native data and metadata encryption for zfs (#124)

[Lauri Tirkkonen]

@arekinath 
> And I think the dump zvol should not be encrypted

Are you serious? I would definitely not want to expose plaintext dumps of 
kernel memory if I'm otherwise encrypting the entire pool.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/openzfs/openzfs/pull/124#issuecomment-321197140
--
openzfs-developer
Archives: 
https://openzfs.topicbox.com/groups/developer/discussions/T1625245905c55186-M86b7a3656977831283529059
Powered by Topicbox: https://topicbox.com

[developer] Re: [openzfs/openzfs] Native data and metadata encryption for zfs (#124)

[Alex Wilson]

I debugged the crash that I got a bit further, in case it's helpful: When we 
died we're trying to encrypt a dnode, and `zio_crypt_init_uios_dnode` is 
setting `total_len = 0` (as well as `*no_crypt = B_TRUE`). In 
`zio_do_crypt_data` we carry on to call `zio_do_crypt_uio` after this returns, 
with `datalen = 0` (that's the `enc_len` that `init_uios_dnode` set to `0`), 
which then calls `crypto_encrypt` with the `plaindata.cd_length = 0`. This 
causes the AES module inside KCF to return `0xC` (`CRYPTO_DATA_LEN_RANGE`) 
because the length we gave it doesn't seem valid. So, we return `EIO` (the 5 
you can see in the panic message).

I'm not really sure what we should be doing here... the fact that we've set 
`no_crypt = B_TRUE` will mean that `zio_encrypt` is going to do the copy 
anyway, so I don't think we have to call in to try to encrypt in this case at 
all? For now, to keep testing, I just made `zio_do_crypt_data` ignore the 
return of `zio_do_crypt_uio` if `no_crypt` is set and that seems to hack around 
it.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/openzfs/openzfs/pull/124#issuecomment-321176320
--
openzfs-developer
Archives: 
https://openzfs.topicbox.com/groups/developer/discussions/T1625245905c55186-M359ff8053e3a282efb573401
Powered by Topicbox: https://topicbox.com

[developer] Re: [openzfs/openzfs] Native data and metadata encryption for zfs (#124)

[Tom Caputi]

@ahrens care to weigh in on how to handle this at all?

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/openzfs/openzfs/pull/124#issuecomment-321138143
--
openzfs-developer
Archives: 
https://openzfs.topicbox.com/groups/developer/discussions/T1625245905c55186-M18cd1ca4806f349e0ba3dd2c
Powered by Topicbox: https://topicbox.com

[developer] Re: [openzfs/openzfs] Native data and metadata encryption for zfs (#124)

[Alex Wilson]

> Could you create pool/dump, pool/swap and pool/encrypted? Is there a reason 
> this data shouldn't be encrypted in the first place (even though it should 
> only be used by the system)?

Yeah, we probably can do that, there's just a disturbing number of scripts that 
assume things are in the root of the pool. :) I wonder if we should maybe add 
an exclusion specifically and only for dump zvols (we already treat them 
specially all over the place).

And I think the dump zvol should not be encrypted, for the same reason it has 
all checksumming and raidz parity turned off on it: the system needs to be able 
to write to it during a kernel panic. The system dumps the contents of kernel 
memory out into there, and then after reboot the OS picks that data up and 
moves it into the pool proper for permanent storage (it's just a temporary 
holding area before we reboot). `checksum=noparity` is a mode that was 
introduced exclusively for this zvol (it's not really meant to be used for 
anything else).

As for swap, I'm fine with encrypting it, though what I'd really like is to put 
it under a separate key (one that we generate each boot and then throw it away 
so next boot we can't read it any more).

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/openzfs/openzfs/pull/124#issuecomment-321135678
--
openzfs-developer
Archives: 
https://openzfs.topicbox.com/groups/developer/discussions/T1625245905c55186-M360101b0bd655334076c76a3
Powered by Topicbox: https://topicbox.com

[developer] Re: [openzfs/openzfs] Native data and metadata encryption for zfs (#124)

[Alex Wilson]

So, when I do:

```
# zfs create -o encryption=on -o checksum=off -o keyformat=passphrase -V 4G 
tank/testvol
```

I get this panic:

```
> ::status
debugging crash dump ./vmcore.9 (64-bit) from 00-0c-29-5d-4c-f7
operating system: 5.11 joyent_20170807T220431Z (i86pc)
image uuid: (not set)
panic message: assertion failed: spa_do_crypt_abd(B_TRUE, spa, 
zio->io_bookmark.zb_objset, bp, zio->io_txg, psize, zio->io_abd, eabd, iv, mac, 
salt, _crypt) == 0 (0x5 == 0x0), file: ../../common/fs/zfs/zio.c, line: 3558
dump content: kernel pages only
> $C
ff000f613900 vpanic()
ff000f613950 0xfba7bfad()
ff000f613a40 zio_encrypt+0x5cf(ff0378abd7d0)
ff000f613a70 zio_execute+0x7f(ff0378abd7d0)
ff000f613b30 taskq_thread+0x2d0(ff0377006a20)
ff000f613b40 thread_start+8()
```

Happy to provide the dump for further debugging, or look more into it if you 
want. I'm guessing that this is probably not going to work, and should be 
blocked off as a combination of properties (i.e. refuse to set encryption=on 
with checksum=off). I guess this also means that having a dump zvol be inside 
an encrypted pool is not going to work.

So.. anyone have any ideas about what do about dump zvols on an encrypted pool? 
I'm trying to think through how we can use this at Joyent, where we only have 
one pool on the system and our dump and swap need to go on it -- since we can't 
make encrypted clones of an unencrypted dataset, and can't have some things 
unencrypted under an encrypted sub-tree, it would make the most sense I think 
to add encryption at the top of the pool (so all our existing code bits that do 
clones of datasets underneath that continue to work). But then we have the 
problem of what to do about the dump zvol. Would it be possible to allow 
unencrypted children of an encrypted dataset at some point in the future? I 
must admit I haven't understood the design well enough yet to tell that for 
myself, sorry.

I've also noticed some odd rendering issues looking at the new man pages. Might 
be a mis-merge on my part, I'll follow up on them later.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/openzfs/openzfs/pull/124#issuecomment-321112170
--
openzfs-developer
Archives: 
https://openzfs.topicbox.com/groups/developer/discussions/T1625245905c55186-M33abc8e7fc85e15556ce3be6
Powered by Topicbox: https://topicbox.com

[developer] Re: [openzfs/openzfs] Native data and metadata encryption for zfs (#124)

[Attila Fülöp]

@ahrens I tried an earlier version of this patch, which crashed in debug 
builds. The tests ran fine on a non-debug build though. See 
https://github.com/openzfs/openzfs/pull/124#issuecomment-288516088 ff and 
https://github.com/openzfs/openzfs/pull/124#issuecomment-285545199 ff

I'm planing to revive my OmniOS VM and try out the latest version since it 
seems that upstream has stabilized, but that may take some time

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/openzfs/openzfs/pull/124#issuecomment-320783118
--
openzfs-developer
Archives: 
https://openzfs.topicbox.com/groups/developer/discussions/T1625245905c55186-M4fcf63f6b5d2c7b395495ac4
Powered by Topicbox: https://topicbox.com

[developer] Re: [openzfs/openzfs] Native data and metadata encryption for zfs (#124)

[Matthew Ahrens]

@ilovezfs I'd like to see the encryption changes merged to ZoL first, since 
that's where they originated and they've gotten the most testing and code 
review on Linux.  https://github.com/zfsonlinux/zfs/pull/5769

We also need to figure out the failing tests (which might or might not be the 
fault of these changes).

Is anyone running this patch (illumos + encryption) or have an experience to 
offer with it, on illumos?  Given the complexity of this change, it would be 
nice to have a little more testing than "works on linux + the tests don't fail".

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/openzfs/openzfs/pull/124#issuecomment-320718260
--
openzfs-developer
Archives: 
https://openzfs.topicbox.com/groups/developer/discussions/T1625245905c55186-M3d549018a5eb3c665f4a7e7a
Powered by Topicbox: https://topicbox.com

[developer] Re: [openzfs/openzfs] Native data and metadata encryption for zfs (#124)

[ilovezfs]

@ahrens what are the blockers to getting this merged?

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/openzfs/openzfs/pull/124#issuecomment-320549971
--
openzfs-developer
Archives: 
https://openzfs.topicbox.com/groups/developer/discussions/T1625245905c55186-M32f4e338a0b2992f0da2f586
Powered by Topicbox: https://topicbox.com

[developer] Re: [openzfs/openzfs] Native data and metadata encryption for zfs (#124)

[Jorgen Lundman]

A few of the tests fail that might not be related, which tests do I need to 
look at wrt to this PR ?


-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/openzfs/openzfs/pull/124#issuecomment-316238036
--
openzfs-developer
Archives: 
https://openzfs.topicbox.com/groups/developer/discussions/T1625245905c55186-M92f3208ba1482a74cfc4512c
Powered by Topicbox: https://topicbox.com

[developer] Re: [openzfs/openzfs] Native data and metadata encryption for zfs (#124)

[Jorgen Lundman]

@lundman pushed 1 commit.

ed02f22  bug fixes


-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/openzfs/openzfs/pull/124/files/2992551a9e789513fdd9a5c7868771d78c12b8bf..ed02f22933e7c5c9ae31cbd3cbaf1322c302f299

--
openzfs-developer
Archives: 
https://openzfs.topicbox.com/groups/developer/discussions/T1625245905c55186-Mdbc6bb8a10216ac0a621f47a
Powered by Topicbox: https://topicbox.com

[developer] Re: [openzfs/openzfs] Native data and metadata encryption for zfs (#124)

[Tom Caputi]

tcaputi commented on this pull request.



> @@ -1200,6 +1210,18 @@ transactions.
 Not all damaged pools can be recovered by using this option.
 If successful, the data from the discarded transactions is irretrievably lost.
 This option is ignored if the pool is importable or already imported.
+.It Fl l
+Indicates that the zpool command will request encryption keys for all
+encrypted datasets it attempts to mount as it is bringing the pool
+online. This is equivalent to running
+.Nm Cm mount
+on each encrypted dataset immediately after the pool is imported.
+If any datasets have a
+.Sy prompt
+keysource this command will block waiting for the key to be entered.
+Otherwise, encrypted datasets will be left unavailable until the keys are
+loaded.
+>>> 97611e4... Native data and metadata encryption for zfs

This looks like a merge mistake :)

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/openzfs/openzfs/pull/124#pullrequestreview-48158899
--
openzfs-developer
Archives: 
https://openzfs.topicbox.com/groups/developer/discussions/T1625245905c55186-Me048bd654cdd9a6c1bd9a0cb
Powered by Topicbox: https://topicbox.com

[developer] Re: [openzfs/openzfs] Native data and metadata encryption for zfs (#124)

[Jorgen Lundman]

@lundman pushed 1 commit.

a19f544  zfs-tests corrections


-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/openzfs/openzfs/pull/124/files/1045b9c5762c2f0bad1110fe1303f461a1b96aff..a19f54495262141f5a551d525c9d6179276c066e

--
openzfs-developer
Archives: 
https://openzfs.topicbox.com/groups/developer/discussions/T1625245905c55186-M2730f80298ef4209e3e3558e
Powered by Topicbox: https://topicbox.com

[developer] Re: [openzfs/openzfs] Native data and metadata encryption for zfs (#124)

[Jorgen Lundman]

Ok I went through the FAILs I got locally

```
functional/cli_root/zpool_get/zpool_get_002_pos (run as root) [00:00] [FAIL]
```
It seems `zpool_get.cfg` is missing the `bootsize` property, which is outside 
of the scope of this PR, so I will ignore it.

```
14:17:01.36 Assertion failed: nvlist_size(nvl, , 0) == 0, file 
../common/libzfs_util.c, line 791,
 function zcmd_write_nvlist_com 
/opt/zfs-tests/tests/functional/rsend/send-c_incremental[41]: log_must[67]: 
log_pos[178]: eval: line 1: 628767: Abort(coredump)
14:17:01.36 ERROR: eval zfs receive -d -F testpool2 < /backdir-rsend/final 
exited 262
```

```
14:19:04.72 Assertion failed: 0 == nvlist_lookup_string(props, 
zfs_prop_to_name(ZFS_PROP_KEYLOCATION), _keylocation), file 
../common/libzfs_sendrecv.c, line 2670, 
function recv_fix_encryption_heirarchy 
/opt/zfs-tests/tests/functional/rsend/send_encrypted_heirarchy[60]: 
log_must[67]: log_pos[178]: eval: line 1: 636308: Abort(coredump)
```

those two needs inspection. 

Also the runfile had a few stale test names.


-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/openzfs/openzfs/pull/124#issuecomment-310288180
--
openzfs-developer
Archives: 
https://openzfs.topicbox.com/groups/developer/discussions/T1625245905c55186-Ma17eb734fa6c5db55a321c73
Powered by Topicbox: https://topicbox.com

[developer] Re: [openzfs/openzfs] Native data and metadata encryption for zfs (#124)

[GernotS]

found another assert when doing a simple send/recv from an unencrypted to an 
encrypted dataset:

zfs create -o canmount=noauto -o encryption=on -o keyformat=passphrase -o 
keylocation=prompt  backup/test
zfs send zones/testdata@1 |zfs recv backup/test/testdata

panic[cpu0]/thread=ff001079fc40:
assertion failed: spa_do_crypt_abd(B_TRUE, spa, zio->io_bookmark.zb_objset, bp, 
zio->io_txg, psize, zio->io_abd, eabd, iv, mac, salt, _crypt) == 0 (0x5 == 
0x0), file: ../../common/fs/zfs/zio.c, line: 3560


ff001079f950 fba7bcad ()
ff001079fa40 zfs:zio_encrypt+5cf ()
ff001079fa70 zfs:zio_execute+7f ()
ff001079fb30 genunix:taskq_thread+2d0 ()
ff001079fb40 unix:thread_start+8 ()


-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/openzfs/openzfs/pull/124#issuecomment-310277185
--
openzfs-developer
Archives: 
https://openzfs.topicbox.com/groups/developer/discussions/T1625245905c55186-Md9b50742977c5935172a80bc
Powered by Topicbox: https://topicbox.com

[developer] Re: [openzfs/openzfs] Native data and metadata encryption for zfs (#124)

[Jorgen Lundman]

@lundman pushed 1 commit.

1045b9c  Remove rngd from tests


-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/openzfs/openzfs/pull/124/files/7e91e1af19bceb66dcddfc8776e281e7a72d685e..1045b9c5762c2f0bad1110fe1303f461a1b96aff

--
openzfs-developer
Archives: 
https://openzfs.topicbox.com/groups/developer/discussions/T1625245905c55186-Mbcb8c675665a5db1de3569c2
Powered by Topicbox: https://topicbox.com

[developer] Re: [openzfs/openzfs] Native data and metadata encryption for zfs (#124)

[Jorgen Lundman]

`rngd` is a Linux helper for random, it can just be `echo` or similar on 
Illumos. I'll have a look at the tests in the morning.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/openzfs/openzfs/pull/124#issuecomment-310043826
--
openzfs-developer
Archives: 
https://openzfs.topicbox.com/groups/developer/discussions/T1625245905c55186-M41e779dd8ba25743a68680f8
Powered by Topicbox: https://topicbox.com

[developer] Re: [openzfs/openzfs] Native data and metadata encryption for zfs (#124)

[GernotS]

doing a zfs destroy on an encrypted dataset without loading the key causes a 
panic:

panic[cpu0]/thread=ff00116d0c40:
BAD TRAP: type=e (#pf Page fault) rp=ff00116cff50 addr=18 occurred in 
module "zfs" due to a NULL pointer dereference


sched:
#pf Page fault
Bad kernel fault at addr=0x18
pid=0, pc=0xf7d63b07, sp=0xff00116d0040, eflags=0x10246
cr0: 8005003b cr4: 6f8
cr2: 18
cr3: 1dc0
cr8: c

rdi:0 rsi:   20 rdx: ff00116d0c40
rcx: ff00116d00b0  r8: ff03ceb79d38  r9: 1a762c00
rax:0 rbx: ff00116d0170 rbp: ff00116d0110
r10:0 r11:1 r12: ff00116d0710
r13: ff03ceb6c000 r14: ff03dcf99000 r15: ff00116d0770
fsb:0 gsb: fbc398e0  ds:   4b
 es:   4b  fs:0  gs:  1c3
trp:e err:0 rip: f7d63b07
 cs:   30 rfl:10246 rsp: ff00116d0040
 ss:   38

ff00116cfe30 unix:real_mode_stop_cpu_stage2_end+b1bc ()
ff00116cff40 unix:trap+e08 ()
ff00116cff50 unix:_cmntrap+e6 ()
ff00116d0110 zfs:traverse_visitbp+6a7 ()
ff00116d01f0 zfs:traverse_visitbp+22f ()
ff00116d02d0 zfs:traverse_visitbp+22f ()
ff00116d03b0 zfs:traverse_visitbp+22f ()
ff00116d0490 zfs:traverse_visitbp+22f ()
ff00116d0570 zfs:traverse_visitbp+22f ()
ff00116d0600 zfs:traverse_dnode+af ()
ff00116d06e0 zfs:traverse_visitbp+574 ()
ff00116d0840 zfs:traverse_impl+1a9 ()
ff00116d08a0 zfs:traverse_dataset_destroyed+49 ()
ff00116d0a30 zfs:bptree_iterate+1e5 ()
ff00116d0aa0 zfs:dsl_scan_sync+33f ()
ff00116d0b70 zfs:spa_sync+3bb ()
ff00116d0c20 zfs:txg_sync_thread+227 ()
ff00116d0c30 unix:thread_start+8 ()


-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/openzfs/openzfs/pull/124#issuecomment-309668862
--
openzfs-developer
Archives: 
https://openzfs.topicbox.com/groups/developer/discussions/T1625245905c55186-Me7ee472dabd79783b8373b24
Powered by Topicbox: https://topicbox.com

[developer] Re: [openzfs/openzfs] Native data and metadata encryption for zfs (#124)

[Jorgen Lundman]

@lundman pushed 1 commit.

9170e97  Correct code in raidz_checksum_error()


-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/openzfs/openzfs/pull/124/files/84fbf78cb5c75fe5027fb3edcaeacb496e25400c..9170e977ffe1f483173d85dece954b08e2ff

--
openzfs-developer
Archives: 
https://openzfs.topicbox.com/groups/developer/discussions/T1625245905c55186-M5e0c764cd519e09fb966c3ea
Powered by Topicbox: https://topicbox.com

[developer] Re: [openzfs/openzfs] Native data and metadata encryption for zfs (#124)

[Jorgen Lundman]

The test is the one immediate after
```
/opt/zfs-tests/tests/functional/cli_root/zpool_clear/setup 
```
ie
```
zpool_clear_001_pos.ksh
```


-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/openzfs/openzfs/pull/124#issuecomment-309604362
--
openzfs-developer
Archives: 
https://openzfs.topicbox.com/groups/developer/discussions/T1625245905c55186-Maadf3d93ffdac7cde6e42275
Powered by Topicbox: https://topicbox.com

[developer] Re: [openzfs/openzfs] Native data and metadata encryption for zfs (#124)

[Igor K]

@prakashsurya can you identify test what produce panic?
and we can run only this test or just couple of tests in this are for reproduce 
it?
can we identify minimal steps with scenario for reproduce steps for this panic ?

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/openzfs/openzfs/pull/124#issuecomment-309458432
--
openzfs-developer
Archives: 
https://openzfs.topicbox.com/groups/developer/discussions/T1625245905c55186-M964f2daac7b2211922b064c6
Powered by Topicbox: https://topicbox.com

[developer] Re: [openzfs/openzfs] Native data and metadata encryption for zfs (#124)

[Jorgen Lundman]

It was my guess that all the zdb cores (there are many of them) made the tests 
run slow, and ended up in a timeout.. I will take a look at the zdb assert and 
see if it is something obvious. Cheers for your feedback


-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/openzfs/openzfs/pull/124#issuecomment-309337835
--
openzfs-developer
Archives: 
https://openzfs.topicbox.com/groups/developer/discussions/T1625245905c55186-Mc92c8c8fc688c0d44784fc49
Powered by Topicbox: https://topicbox.com

[developer] Re: [openzfs/openzfs] Native data and metadata encryption for zfs (#124)

[Prakash Surya]

re:
```
Cannot contact i-09a7ea9f1f400ab9b: java.lang.InterruptedException
Could not connect to i-09a7ea9f1f400ab9b to send interrupt signal to process
```
we have an 8 hour time limit set on the zfstest runs, so if any run takes 
longer than that, it'll be killed by the Jenkins framework, and the result is a 
message like this.. additionally, I can see that the zfstest script was running 
for nearly 8 hours; the UI shows `7h 59m 15s`:

![jenkins](https://user-images.githubusercontent.com/658081/27268698-2a77467e-5466-11e7-96c9-3636ed24a117.png)

So that's all a good indicator that we hit the time limit. Looking at some of 
the recent "master" branch runs, 
[here](http://jenkins.open-zfs.org/blue/organizations/jenkins/openzfs%2Fopenzfs/activity/?branch=master),
 zfstest appears to usually take between 4 and 6 hours to complete. So, the 
question is, why was it still running after 8 hours of runtime? I have seen 
this timeout trigger before, on code that passed like normal when run a second 
time, so this might not have anything to do with this specific change, but it 
would be good to verify that (e.g. double check that this change isn't making 
some of the tests run longer than normal, and/or run the tests again to see if 
it hits the same timeout).

The `zdb` error is almost certainly due to this change, so that'll need to be 
addressed.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/openzfs/openzfs/pull/124#issuecomment-309332490
--
openzfs-developer
Archives: 
https://openzfs.topicbox.com/groups/developer/discussions/T1625245905c55186-M6d98b35cb0422dd6a7d9ab45
Powered by Topicbox: https://topicbox.com

[developer] Re: [openzfs/openzfs] Native data and metadata encryption for zfs (#124)

[Jorgen Lundman]

@lundman pushed 2 commits.

bbf9bd3  Fixes and improvements after 5th round of review
439cd9c  Fixes after rebase and more review


-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/openzfs/openzfs/pull/124/files/5556d50abd4ba1cbe70cc2599f4af32176d0a39c..439cd9c2550171097fb23c1af7b518f9de7029ed

--
openzfs-developer
Archives: 
https://openzfs.topicbox.com/groups/developer/discussions/T1625245905c55186-M74b65d52096d7eddbc9212e5
Powered by Topicbox: https://topicbox.com

[developer] Re: [openzfs/openzfs] Native data and metadata encryption for zfs (#124)

[Jorgen Lundman]

@lundman pushed 1 commit.

f68ee3c  Attempt to update the manpages


-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/openzfs/openzfs/pull/124/files/79798fac7fde50d17bd90222d13ee0ac3d806be3..f68ee3ce27b2a9758ae41e0addf24b511fcd1cb4

--
openzfs-developer
Archives: 
https://openzfs.topicbox.com/groups/developer/discussions/T1625245905c55186-Mb4de6012d972113ee50d2c9a
Powered by Topicbox: https://topicbox.com

[developer] Re: [openzfs/openzfs] Native data and metadata encryption for zfs (#124)

[Jorgen Lundman]

@lundman pushed 2 commits.

27bc76c  ABD released too early in abd_hmac check
785086e  Corrections in zio_checksum for crypto


-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/openzfs/openzfs/pull/124/files/2980e8ef1b01c9a6b4abe49d67e84bb57019b9d3..785086e61d1b438c27e6b1867af187200674ed11

--
openzfs-developer
Archives: 
https://openzfs.topicbox.com/groups/developer/discussions/T1625245905c55186-Md3d11126a0c72c587f474daa
Powered by Topicbox: https://topicbox.com

[developer] Re: [openzfs/openzfs] Native data and metadata encryption for zfs (#124)

[Matthew Ahrens]

ahrens commented on this pull request.



> @@ -444,7 +448,7 @@ blkptr(uintptr_t addr, uint_t flags, int argc, const 
> mdb_arg_t *argv)
}
 
SNPRINTF_BLKPTR(mdb_snprintf, '\n', buf, sizeof (buf), bp, type,
-   checksum, compress);
+   checksum, 0, compress);

1. don't we need to pass in the crypt_type?

2. dmu_ot[] isn't necessarily filled in here.  We need to read it from the 
target first.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/openzfs/openzfs/pull/124#pullrequestreview-42168801
--
openzfs-developer
Archives: 
https://openzfs.topicbox.com/groups/developer/discussions/T1625245905c55186-M95c72a29084f21e9bcbda1ce
Powered by Topicbox: https://topicbox.com

[developer] Re: [openzfs/openzfs] Native data and metadata encryption for zfs (#124)

[Jorgen Lundman]

@lundman pushed 1 commit.

dc4e629  Fixes


-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/openzfs/openzfs/pull/124/files/6c89a04a59661b4bedc7e4b892c0a100204695da..dc4e629f2d708e7241963ddaa8b94996863075d9

--
openzfs-developer
Archives: 
https://openzfs.topicbox.com/groups/developer/discussions/T1625245905c55186-Me823d3f384045457f952b378
Powered by Topicbox: https://topicbox.com

[developer] Re: [openzfs/openzfs] Native data and metadata encryption for zfs (#124)

[Jorgen Lundman]

@lundman pushed 1 commit.

6c89a04  Delinting


-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/openzfs/openzfs/pull/124/files/ea53ac7930549f31a8d6480ab697c404ffbc05f4..6c89a04a59661b4bedc7e4b892c0a100204695da

--
openzfs-developer
Archives: 
https://openzfs.topicbox.com/groups/developer/discussions/T1625245905c55186-Mae9a8b54c666292357e7ba98
Powered by Topicbox: https://topicbox.com

[developer] Re: [openzfs/openzfs] Native data and metadata encryption for zfs (#124)

[Matthew Ahrens]

@lundman looks like there are some lint errors.  If you click through to 
"Details" of the test failure you'll see them:
```
"../../../common/modules/zfs/zfs.c", line 451: warning: suspicious comparison 
of unsigned with 0: op "<=" (E_SUSPICIOUS_COMPARISON)
"../../../common/modules/zfs/zfs.c", line 70: warning: const object should have 
initializer: dmu_ot (E_CONST_OBJ_SHOULD_HAVE_INITIZR)
"../../common/fs/zfs/dmu_objset.c", line 862: warning: set but not used in 
function: ibs in dmu_objset_create_impl_dnstats (E_FUNC_SET_NOT_USED)
"../../common/fs/zfs/zio_crypt.c", line 1007: warning: constant operand to op: 
"!" (E_CONST_EXPR)
"../../common/fs/zfs/zio_crypt.c", line 1122: warning: constant operand to op: 
"!" (E_CONST_EXPR)
"../common/libzfs_sendrecv.c", line 2592: warning: argument unused in function: 
stream_avl in recv_fix_encryption_heirarchy (E_FUNC_ARG_UNUSED)
"../zdb.c", line 1185: warning: suspicious comparison of unsigned with 0: op 
"<=" (E_SUSPICIOUS_COMPARISON)
"../zdb.c", line 1222: warning: suspicious comparison of unsigned with 0: op 
"<=" (E_SUSPICIOUS_COMPARISON)
"../zdb.c", line 1292: warning: suspicious comparison of unsigned with 0: op 
"<=" (E_SUSPICIOUS_COMPARISON)
"../zdb.c", line 1295: warning: suspicious comparison of unsigned with 0: op 
"<=" (E_SUSPICIOUS_COMPARISON)
"../zdb.c", line 2026: warning: suspicious comparison of unsigned with 0: op 
"<=" (E_SUSPICIOUS_COMPARISON)
"../zdb.c", line 2033: warning: suspicious comparison of unsigned with 0: op 
"<=" (E_SUSPICIOUS_COMPARISON)
"../zdb.c", line 2496: warning: suspicious comparison of unsigned with 0: op 
"<=" (E_SUSPICIOUS_COMPARISON)
"../zdb.c", line 2510: warning: suspicious comparison of unsigned with 0: op 
"<=" (E_SUSPICIOUS_COMPARISON)
"../zdb.c", line 2512: warning: suspicious comparison of unsigned with 0: op 
"<=" (E_SUSPICIOUS_COMPARISON)
"../zdb.c", line 2730: warning: suspicious comparison of unsigned with 0: op 
"<=" (E_SUSPICIOUS_COMPARISON)
"../zdb.c", line 3131: warning: suspicious comparison of unsigned with 0: op 
"<=" (E_SUSPICIOUS_COMPARISON)
"/opt/jenkins/root/workspace/openzfs_openzfs_PR-124-BMYUW6PFZNQM4CULXGIUCOULZFGUBEB7CINIXVRIENRUKP5JHJ2Q/usr/src/lib/libzfs/common/libzfs_crypto.c",
 line 713: warning: function returns value which is sometimes ignored: strcpy 
(E_FUNC_RET_MAYBE_IGNORED2)
```

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/openzfs/openzfs/pull/124#issuecomment-305838291
--
openzfs-developer
Archives: 
https://openzfs.topicbox.com/groups/developer/discussions/T1625245905c55186-M75cd88cc45a580074020f288
Powered by Topicbox: https://topicbox.com

[developer] Re: [openzfs/openzfs] Native data and metadata encryption for zfs (#124)

[Jorgen Lundman]

@lundman pushed 1 commit.

ea53ac7  Compile fixes


-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/openzfs/openzfs/pull/124/files/0f52c029de5953d706bec7b3d288b5cc6d6f0072..ea53ac7930549f31a8d6480ab697c404ffbc05f4

--
openzfs-developer
Archives: 
https://openzfs.topicbox.com/groups/developer/discussions/T1625245905c55186-M02f18c4eb1ed604b5ed81497
Powered by Topicbox: https://topicbox.com

[developer] Re: [openzfs/openzfs] Native data and metadata encryption for zfs (#124)

[Jorgen Lundman]

Pushed the latest commits, I did not do the manpage changes, specifically those 
in
https://github.com/zfsonlinux/zfs/pull/5769/commits/705a2bfc5eabb45facf1e0f7a38a3ee5a69a54ea#diff-42027d93bce823d185f48bd265de4ac0
https://github.com/zfsonlinux/zfs/pull/5769/commits/369ba84201eaa730d6a4d41ef76b14e9b1df02f3#diff-42027d93bce823d185f48bd265de4ac0
https://github.com/zfsonlinux/zfs/pull/5769/commits/369ba84201eaa730d6a4d41ef76b14e9b1df02f3#diff-03c0d83ffa90bcf02b291a55c4a49874

I've only just done bare minimum compile checks, as the VM fails to build due 
to something mdb. When I find a solution, I'll push a fix if one is needed.



-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/openzfs/openzfs/pull/124#issuecomment-302989850
--
openzfs-developer
Archives: https://openzfs.topicbox-beta.com/groups/developer/
Powered by Topicbox: https://topicbox-beta.com

[developer] Re: [openzfs/openzfs] Native data and metadata encryption for zfs (#124)

[Jorgen Lundman]

@lundman pushed 2 commits.

24aa9ff  Various fixes and improvements
0f52c02  Fixes and improvements after 4th round of review


-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/openzfs/openzfs/pull/124/files/2a16cf6821ddb026d7b51181a456d9bfee2ffb93..0f52c029de5953d706bec7b3d288b5cc6d6f0072

--
openzfs-developer
Archives: https://openzfs.topicbox-beta.com/groups/developer/
Powered by Topicbox: https://topicbox-beta.com

[developer] Re: [openzfs/openzfs] Native data and metadata encryption for zfs (#124)

[Jorgen Lundman]

The PR is just 3 weeks away from its first birthday, yaaay \o/


-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/openzfs/openzfs/pull/124#issuecomment-302028603
--
openzfs-developer
Archives: https://openzfs.topicbox.com/groups/developer/
Powered by Topicbox: https://topicbox.com