Date: Fri, 10 Jul 2015 17:28:37 -0500 From: "Matthew D. Fuller" <fullermd@over-yonder.net> To: Pawel Jakub Dawidek <pjd@FreeBSD.org> Cc: freebsd-geom@freebsd.org Subject: Re: RFC: Pass TRIM through GELI Message-ID: <20150710222837.GE96394@over-yonder.net> In-Reply-To: <20150710200055.GB1270@garage.freebsd.pl> References: <20150308000131.GP1742@over-yonder.net> <20150324021924.GQ52331@over-yonder.net> <20150502125220.GS78376@over-yonder.net> <20150629013841.GO50491@over-yonder.net> <20150710200055.GB1270@garage.freebsd.pl>
next in thread | previous in thread | raw e-mail | index | archive | help
On Fri, Jul 10, 2015 at 10:01:01PM +0200 I heard the voice of
Pawel Jakub Dawidek, and lo! it spake thus:
>
> The good news is that I like the patch - it looks clean and
> complete. The bad news is that I like it a bit too much:) I think
> I'd prefer that BIO_DELETE is passed through by default and there is
> an option to turn it off. [...] OR... just removing the ability to
> ignore BIO_DELETEs.
Well, if we removed the configurability and just made it pass 'em
through unconditionally, the patch gets REAL simple[0]. 98% of the
patch and probably 99.5 of the work is handling the configuration ;)
(though we'd still want to pull out the bit for fixing "geli
configure' on onetime providers; I tried pulling that out into a
separate patch/PR, but it's entwined enough with the other changes
in this that the patches would conflict with each other, so I just
left it combined)
I'm in favor of having it on by default. That's a small adjustment to
the patch to swap the default, a larger to do it by inverting the
meaning of the flag. Probably worth the extra effort to make it a
NO_DELETE flag, rather than just having the bit almost-always set.
Should be able to find time to do it within the next couple days.
> Thinking about it some more, I believe that if someone doesn't want
> TRIM/UNMAP to hit his SSDs it should be configurable on per-SSD
> basis and not on every layer above SSD.
Well, I figure there are 2 reasons somebody might want to shut it off.
1) Functionality. The SSD, or controller, or something along the
chain, gets indigestion when TRIM's happen. In this case, yeah,
you want to shut it off at a level way below GELI.
2) Security. For whatever your threat model is, leaking the "how much
space is in use" datum is unacceptable. I suspect this is a
miniscule fraction of the userbase (probably a miniscule fraction
even of those who think of it as an attack). But insofar as it's a
concern that needs handled, doing it in GELI would be the right
place, since you'd still want to TRIM on the cleartext partition
covering the other 3/4 of your SSD.
Case (2) seems a little borderline in general. By no means entirely
impossible, but perhaps niche enough to be beyond what we care to put
effort into supporting. OTOH, in a "Customer Is Always Right" sense,
whether they _really_ need it or not has little to do with whether
they'll demand it and refuse and badmouth any solution that doesn't
provide it.
And since I've already done the work (well, almost all, since I'll
have to go through and invert the sense), and since it included a
little preening in the configuration code paths that could make things
a little simpler for the next time somebody has to add flags (e.g., in
the 'configure' code path, which had to be reshuffled a fair bit to
handle more than 1 possible flag)... well, why not?
[0] e.g., (untested, but)
Index: g_eli.c
===================================================================
--- g_eli.c (revision 285364)
+++ g_eli.c (working copy)
@@ -309,13 +309,8 @@
case BIO_WRITE:
case BIO_GETATTR:
case BIO_FLUSH:
+ case BIO_DELETE:
break;
- case BIO_DELETE:
- /*
- * We could eventually support BIO_DELETE request.
- * It could be done by overwritting requested sector with
- * random data g_eli_overwrites number of times.
- */
default:
g_io_deliver(bp, EOPNOTSUPP);
return;
@@ -342,6 +337,7 @@
break;
case BIO_GETATTR:
case BIO_FLUSH:
+ case BIO_DELETE:
cbp->bio_done = g_std_done;
cp = LIST_FIRST(&sc->sc_geom->consumer);
cbp->bio_to = cp->provider;
--
Matthew Fuller (MF4839) | fullermd@over-yonder.net
Systems/Network Administrator | http://www.over-yonder.net/~fullermd/
On the Internet, nobody can hear you scream.
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20150710222837.GE96394>