Re: softraid(4) crypto/raid1c refactoring
The few test reports I've received for this are looking fine.
Would any developer dare to OK this?
On Tue, Apr 27, 2021 at 03:46:56PM +0200, Stefan Sperling wrote:
> Refactor softraid crypto code to allow use of a discipline-specific data
> structure for RAID1C volumes, as requested by jsing@ during review of my
> initial RAID1C patch.
>
> This patch should effectively be a cosmetic change.
> The whole point of this patch is to allow the data structure changes
> made here in softraidvar.h.
>
> It works in my testing but more testing would be very welcome, given
> that this touches the disk I/O path of machines using softraid crypto.
>
> ok?
>
> diff d5cea33885618bf7e096efc36fffbecc9b13ed21
> fcfd3d6487eca3ffe994e6a46e37df66b37e80d1
> blob - d143a2398b5aba3070dc25bafd02e38f8f10a0c1
> blob + 48bd613f374bc6ad085ae5d9e0eeef50a6367941
> --- sys/dev/softraid_crypto.c
> +++ sys/dev/softraid_crypto.c
> @@ -54,30 +54,44 @@
>
> #include
>
> -struct sr_crypto_wu *sr_crypto_prepare(struct sr_workunit *, int);
> -int sr_crypto_create_keys(struct sr_discipline *);
> -int sr_crypto_get_kdf(struct bioc_createraid *,
> - struct sr_discipline *);
> +struct sr_crypto_wu *sr_crypto_prepare(struct sr_workunit *,
> + struct sr_crypto *, int);
> int sr_crypto_decrypt(u_char *, u_char *, u_char *, size_t, int);
> int sr_crypto_encrypt(u_char *, u_char *, u_char *, size_t, int);
> -int sr_crypto_decrypt_key(struct sr_discipline *);
> +int sr_crypto_decrypt_key(struct sr_discipline *,
> + struct sr_crypto *);
> int sr_crypto_change_maskkey(struct sr_discipline *,
> - struct sr_crypto_kdfinfo *, struct sr_crypto_kdfinfo *);
> + struct sr_crypto *, struct sr_crypto_kdfinfo *,
> + struct sr_crypto_kdfinfo *);
> int sr_crypto_create(struct sr_discipline *,
> struct bioc_createraid *, int, int64_t);
> int sr_crypto_meta_create(struct sr_discipline *,
> - struct bioc_createraid *);
> + struct sr_crypto *, struct bioc_createraid *);
> +int sr_crypto_set_key(struct sr_discipline *, struct sr_crypto *,
> + struct bioc_createraid *, int, void *);
> int sr_crypto_assemble(struct sr_discipline *,
> struct bioc_createraid *, int, void *);
> +void sr_crypto_free_sessions(struct sr_discipline *,
> + struct sr_crypto *);
> +int sr_crypto_alloc_resources_internal(struct sr_discipline *,
> + struct sr_crypto *);
> int sr_crypto_alloc_resources(struct sr_discipline *);
> +void sr_crypto_free_resources_internal(struct sr_discipline *,
> + struct sr_crypto *);
> void sr_crypto_free_resources(struct sr_discipline *);
> +int sr_crypto_ioctl_internal(struct sr_discipline *,
> + struct sr_crypto *, struct bioc_discipline *);
> int sr_crypto_ioctl(struct sr_discipline *,
> struct bioc_discipline *);
> +int sr_crypto_meta_opt_handler_internal(struct sr_discipline *,
> + struct sr_crypto *, struct sr_meta_opt_hdr *);
> int sr_crypto_meta_opt_handler(struct sr_discipline *,
> struct sr_meta_opt_hdr *);
> void sr_crypto_write(struct cryptop *);
> int sr_crypto_rw(struct sr_workunit *);
> int sr_crypto_dev_rw(struct sr_workunit *, struct sr_crypto_wu *);
> +void sr_crypto_done_internal(struct sr_workunit *,
> + struct sr_crypto *);
> void sr_crypto_done(struct sr_workunit *);
> void sr_crypto_read(struct cryptop *);
> void sr_crypto_calculate_check_hmac_sha1(u_int8_t *, int,
> @@ -85,7 +99,7 @@ void
> sr_crypto_calculate_check_hmac_sha1(u_int8_t *,
> void sr_crypto_hotplug(struct sr_discipline *, struct disk *, int);
>
> #ifdef SR_DEBUG0
> -void sr_crypto_dumpkeys(struct sr_discipline *);
> +void sr_crypto_dumpkeys(struct sr_crypto *);
> #endif
>
> /* Discipline initialisation. */
> @@ -129,7 +143,7 @@ sr_crypto_create(struct sr_discipline *sd, struct bioc
>
> sd->sd_meta->ssdi.ssd_size = coerced_size;
>
> - rv = sr_crypto_meta_create(sd, bc);
> + rv = sr_crypto_meta_create(sd, >mds.mdd_crypto, bc);
> if (rv)
> return (rv);
>
> @@ -138,7 +152,8 @@ sr_crypto_create(struct sr_discipline *sd, struct bioc
> }
>
> int
> -sr_crypto_meta_create(struct sr_discipline *sd, struct bioc_createraid *bc)
> +sr_crypto_meta_create(struct sr_discipline *sd, struct sr_crypto *mdd_crypto,
> +struct bioc_createraid *bc)
> {
> struct sr_meta_opt_item *omi;
> int rv = EINVAL;
> @@ -158,19 +173,19 @@ sr_crypto_meta_create(struct sr_discipline *sd, struct
> omi->omi_som->som_type =
Re: softraid(4) crypto/raid1c refactoring
Hi Stefan, * Stefan Sperling wrote: > Refactor softraid crypto code to allow use of a discipline-specific data > structure for RAID1C volumes, as requested by jsing@ during review of my > initial RAID1C patch. > > This patch should effectively be a cosmetic change. > The whole point of this patch is to allow the data structure changes > made here in softraidvar.h. > > It works in my testing but more testing would be very welcome, given > that this touches the disk I/O path of machines using softraid crypto. I tested the patch on an Thinkpad X250 with two softraid encrypted disks (one containing all partitions except /home, the other one solely for /home mounted with rc.local). Cold and warm reboots work, suspend2ram works). Cheers Matthias OpenBSD 6.9-current (GENERIC.MP) #2: Sat May 1 10:47:45 CEST 2021 x...@epsilon.xosc.org:/usr/src/sys/arch/amd64/compile/GENERIC.MP real mem = 17059979264 (16269MB) avail mem = 16527515648 (15761MB) random: good seed from bootblocks mpath0 at root scsibus0 at mpath0: 256 targets mainbus0 at root bios0 at mainbus0: SMBIOS rev. 2.7 @ 0xacbfd000 (64 entries) bios0: vendor LENOVO version "N10ET61W (1.40 )" date 03/17/2020 bios0: LENOVO 20CM004VMN acpi0 at bios0: ACPI 5.0 acpi0: sleep states S0 S3 S4 S5 acpi0: tables DSDT FACP SLIC ASF! HPET ECDT APIC MCFG SSDT SSDT SSDT SSDT SSDT SSDT SSDT SSDT SSDT PCCT SSDT UEFI MSDM BATB FPDT UEFI BGRT DMAR acpi0: wakeup devices LID_(S4) SLPB(S3) IGBE(S4) EXP2(S4) XHCI(S3) EHC1(S3) acpitimer0 at acpi0: 3579545 Hz, 24 bits acpihpet0 at acpi0: 14318179 Hz acpiec0 at acpi0 acpimadt0 at acpi0 addr 0xfee0: PC-AT compat cpu0 at mainbus0: apid 0 (boot processor) cpu0: Intel(R) Core(TM) i5-5200U CPU @ 2.20GHz, 2095.44 MHz, 06-3d-04 cpu0: FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,SSE3,PCLMUL,DTES64,MWAIT,DS-CPL,VMX,EST,TM2,SSSE3,SDBG,FMA3,CX16,xTPR,PDCM,PCID,SSE4.1,SSE4.2,x2APIC,MOVBE,POPCNT,DEADLINE,AES,XSAVE,AVX,F16C,RDRAND,NXE,PAGE1GB,RDTSCP,LONG,LAHF,ABM,3DNOWP,PERF,ITSC,FSGSBASE,TSC_ADJUST,BMI1,AVX2,SMEP,BMI2,ERMS,INVPCID,RDSEED,ADX,SMAP,PT,SRBDS_CTRL,MD_CLEAR,IBRS,IBPB,STIBP,L1DF,SSBD,SENSOR,ARAT,XSAVEOPT,MELTDOWN cpu0: 256KB 64b/line 8-way L2 cache cpu0: smt 0, core 0, package 0 mtrr: Pentium Pro MTRR support, 10 var ranges, 88 fixed ranges cpu0: apic clock running at 99MHz cpu0: mwait min=64, max=64, C-substates=0.2.1.2.4.1.1.1, IBE cpu1 at mainbus0: apid 2 (application processor) cpu1: Intel(R) Core(TM) i5-5200U CPU @ 2.20GHz, 2095.16 MHz, 06-3d-04 cpu1: FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,SSE3,PCLMUL,DTES64,MWAIT,DS-CPL,VMX,EST,TM2,SSSE3,SDBG,FMA3,CX16,xTPR,PDCM,PCID,SSE4.1,SSE4.2,x2APIC,MOVBE,POPCNT,DEADLINE,AES,XSAVE,AVX,F16C,RDRAND,NXE,PAGE1GB,RDTSCP,LONG,LAHF,ABM,3DNOWP,PERF,ITSC,FSGSBASE,TSC_ADJUST,BMI1,AVX2,SMEP,BMI2,ERMS,INVPCID,RDSEED,ADX,SMAP,PT,SRBDS_CTRL,MD_CLEAR,IBRS,IBPB,STIBP,L1DF,SSBD,SENSOR,ARAT,XSAVEOPT,MELTDOWN cpu1: 256KB 64b/line 8-way L2 cache cpu1: smt 0, core 1, package 0 ioapic0 at mainbus0: apid 2 pa 0xfec0, version 20, 40 pins acpimcfg0 at acpi0 acpimcfg0: addr 0xf800, bus 0-63 acpiprt0 at acpi0: bus 0 (PCI0) acpiprt1 at acpi0: bus -1 (PEG_) acpiprt2 at acpi0: bus 2 (EXP1) acpiprt3 at acpi0: bus 3 (EXP2) acpiprt4 at acpi0: bus -1 (EXP3) acpibtn0 at acpi0: LID_ acpibtn1 at acpi0: SLPB acpipci0 at acpi0 PCI0: 0x 0x0011 0x0001 acpicmos0 at acpi0 acpibat0 at acpi0: BAT0 model "45N1113" serial 1370 type LION oem "LGC" acpibat1 at acpi0: BAT1 model "45N1735" serial 745 type LION oem "LGC" acpiac0 at acpi0: AC unit online acpithinkpad0 at acpi0: version 1.0 "PNP0C14" at acpi0 not configured "PNP0C14" at acpi0 not configured "PNP0C14" at acpi0 not configured "INT340F" at acpi0 not configured acpicpu0 at acpi0: C3(200@233 mwait.1@0x40), C2(200@148 mwait.1@0x33), C1(1000@1 mwait.1), PSS acpicpu1 at acpi0: C3(200@233 mwait.1@0x40), C2(200@148 mwait.1@0x33), C1(1000@1 mwait.1), PSS acpipwrres0 at acpi0: PUBS, resource for XHCI, EHC1 acpipwrres1 at acpi0: NVP3, resource for PEG_ acpipwrres2 at acpi0: NVP2, resource for PEG_ acpitz0 at acpi0: critical temperature is 128 degC acpivideo0 at acpi0: VID_ acpivout0 at acpivideo0: LCD0 acpivideo1 at acpi0: VID_ cpu0: using VERW MDS workaround (except on vmm entry) cpu0: Enhanced SpeedStep 2095 MHz: speeds: 2201, 2200, 2100, 2000, 1800, 1700, 1600, 1500, 1300, 1200, 1100, 1000, 900, 700, 600, 500 MHz pci0 at mainbus0 bus 0 pchb0 at pci0 dev 0 function 0 "Intel Core 5G Host" rev 0x09 inteldrm0 at pci0 dev 2 function 0 "Intel HD Graphics 5500" rev 0x09 drm0 at inteldrm0 inteldrm0: msi, BROADWELL, gen 8 azalia0 at pci0 dev 3 function 0 "Intel Core 5G HD Audio" rev 0x09: msi azalia0: No codecs found xhci0 at pci0 dev 20 function 0 "Intel 9 Series xHCI" rev 0x03: msi, xHCI 1.0 usb0 at xhci0: USB revision 3.0 uhub0 at usb0 configuration 1 interface 0 "Intel xHCI root hub"
Re: softraid(4) crypto/raid1c refactoring
On Tue, Apr 27, 2021 at 03:46:56PM +0200, Stefan Sperling wrote: > Refactor softraid crypto code to allow use of a discipline-specific data > structure for RAID1C volumes, as requested by jsing@ during review of my > initial RAID1C patch. > > This patch should effectively be a cosmetic change. > The whole point of this patch is to allow the data structure changes > made here in softraidvar.h. > > It works in my testing but more testing would be very welcome, given > that this touches the disk I/O path of machines using softraid crypto. It worked in my (limited) testing, too.
softraid(4) crypto/raid1c refactoring
Refactor softraid crypto code to allow use of a discipline-specific data
structure for RAID1C volumes, as requested by jsing@ during review of my
initial RAID1C patch.
This patch should effectively be a cosmetic change.
The whole point of this patch is to allow the data structure changes
made here in softraidvar.h.
It works in my testing but more testing would be very welcome, given
that this touches the disk I/O path of machines using softraid crypto.
ok?
diff d5cea33885618bf7e096efc36fffbecc9b13ed21
fcfd3d6487eca3ffe994e6a46e37df66b37e80d1
blob - d143a2398b5aba3070dc25bafd02e38f8f10a0c1
blob + 48bd613f374bc6ad085ae5d9e0eeef50a6367941
--- sys/dev/softraid_crypto.c
+++ sys/dev/softraid_crypto.c
@@ -54,30 +54,44 @@
#include
-struct sr_crypto_wu *sr_crypto_prepare(struct sr_workunit *, int);
-intsr_crypto_create_keys(struct sr_discipline *);
-intsr_crypto_get_kdf(struct bioc_createraid *,
- struct sr_discipline *);
+struct sr_crypto_wu *sr_crypto_prepare(struct sr_workunit *,
+ struct sr_crypto *, int);
intsr_crypto_decrypt(u_char *, u_char *, u_char *, size_t, int);
intsr_crypto_encrypt(u_char *, u_char *, u_char *, size_t, int);
-intsr_crypto_decrypt_key(struct sr_discipline *);
+intsr_crypto_decrypt_key(struct sr_discipline *,
+ struct sr_crypto *);
intsr_crypto_change_maskkey(struct sr_discipline *,
- struct sr_crypto_kdfinfo *, struct sr_crypto_kdfinfo *);
+ struct sr_crypto *, struct sr_crypto_kdfinfo *,
+ struct sr_crypto_kdfinfo *);
intsr_crypto_create(struct sr_discipline *,
struct bioc_createraid *, int, int64_t);
intsr_crypto_meta_create(struct sr_discipline *,
- struct bioc_createraid *);
+ struct sr_crypto *, struct bioc_createraid *);
+intsr_crypto_set_key(struct sr_discipline *, struct sr_crypto *,
+ struct bioc_createraid *, int, void *);
intsr_crypto_assemble(struct sr_discipline *,
struct bioc_createraid *, int, void *);
+void sr_crypto_free_sessions(struct sr_discipline *,
+ struct sr_crypto *);
+intsr_crypto_alloc_resources_internal(struct sr_discipline *,
+ struct sr_crypto *);
intsr_crypto_alloc_resources(struct sr_discipline *);
+void sr_crypto_free_resources_internal(struct sr_discipline *,
+ struct sr_crypto *);
void sr_crypto_free_resources(struct sr_discipline *);
+intsr_crypto_ioctl_internal(struct sr_discipline *,
+ struct sr_crypto *, struct bioc_discipline *);
intsr_crypto_ioctl(struct sr_discipline *,
struct bioc_discipline *);
+intsr_crypto_meta_opt_handler_internal(struct sr_discipline *,
+ struct sr_crypto *, struct sr_meta_opt_hdr *);
intsr_crypto_meta_opt_handler(struct sr_discipline *,
struct sr_meta_opt_hdr *);
void sr_crypto_write(struct cryptop *);
intsr_crypto_rw(struct sr_workunit *);
intsr_crypto_dev_rw(struct sr_workunit *, struct sr_crypto_wu *);
+void sr_crypto_done_internal(struct sr_workunit *,
+ struct sr_crypto *);
void sr_crypto_done(struct sr_workunit *);
void sr_crypto_read(struct cryptop *);
void sr_crypto_calculate_check_hmac_sha1(u_int8_t *, int,
@@ -85,7 +99,7 @@ void sr_crypto_calculate_check_hmac_sha1(u_int8_t *,
void sr_crypto_hotplug(struct sr_discipline *, struct disk *, int);
#ifdef SR_DEBUG0
-voidsr_crypto_dumpkeys(struct sr_discipline *);
+voidsr_crypto_dumpkeys(struct sr_crypto *);
#endif
/* Discipline initialisation. */
@@ -129,7 +143,7 @@ sr_crypto_create(struct sr_discipline *sd, struct bioc
sd->sd_meta->ssdi.ssd_size = coerced_size;
- rv = sr_crypto_meta_create(sd, bc);
+ rv = sr_crypto_meta_create(sd, >mds.mdd_crypto, bc);
if (rv)
return (rv);
@@ -138,7 +152,8 @@ sr_crypto_create(struct sr_discipline *sd, struct bioc
}
int
-sr_crypto_meta_create(struct sr_discipline *sd, struct bioc_createraid *bc)
+sr_crypto_meta_create(struct sr_discipline *sd, struct sr_crypto *mdd_crypto,
+struct bioc_createraid *bc)
{
struct sr_meta_opt_item *omi;
int rv = EINVAL;
@@ -158,19 +173,19 @@ sr_crypto_meta_create(struct sr_discipline *sd, struct
omi->omi_som->som_type = SR_OPT_CRYPTO;
omi->omi_som->som_length = sizeof(struct sr_meta_crypto);
SLIST_INSERT_HEAD(>sd_meta_opt, omi, omi_link);
- sd->mds.mdd_crypto.scr_meta = (struct sr_meta_crypto *)omi->omi_som;
+ mdd_crypto->scr_meta = (struct
