lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Thu, 14 Nov 2019 16:47:02 +0100
From:   Ilya Dryomov <idryomov@...il.com>
To:     Luis Henriques <lhenriques@...e.com>
Cc:     Sage Weil <sweil@...hat.com>, Jeff Layton <jlayton@...nel.org>,
        Gregory Farnum <gfarnum@...hat.com>,
        "Yan, Zheng" <zyan@...hat.com>,
        Ceph Development <ceph-devel@...r.kernel.org>,
        LKML <linux-kernel@...r.kernel.org>
Subject: Re: [RFC PATCH v2 0/4] ceph: safely use 'copy-from' Op on Octopus OSDs

On Thu, Nov 14, 2019 at 4:24 PM Luis Henriques <lhenriques@...e.com> wrote:
>
> On Thu, Nov 14, 2019 at 02:17:44PM +0000, Sage Weil wrote:
> > On Thu, 14 Nov 2019, Ilya Dryomov wrote:
> > > > > I'm just getting caught up on the discussion here, but why was it
> > > > > decided to do it this way instead of just adding a new OSD
> > > > > "copy-from-no-truncseq" operation? Once you tried it once and an OSD
> > > > > didn't support it, you could just give up on using it any longer? That
> > > > > seems a lot simpler than trying to monkey with feature bits.
> > > >
> > > > I don't remember the original discussion either, but in retrospect that
> > > > does seem much simpler--especially since hte client is conditioning
> > > > sending this based on the the require_osd_release.  It seems like passing
> > > > a flag to the copy-from op would be more reasonable instead of conditional
> > > > feature-based behavior.
> > >
> > > Yeah, I suggested adding require_osd_release to the client portion just
> > > because we are running into it more and more: Objecter relies on it for
> > > RESEND_ON_SPLIT for example.  It needs to be accessible so that patches
> > > like that can be carried over to the kernel client without workarounds.
> > >
> > > copy-from in its existing form is another example.  AFAIU the problem
> > > is that copy-from op doesn't reject unknown flags.  Luis added a flag
> > > in https://github.com/ceph/ceph/pull/25374, but it is simply ignored on
> > > nautilus and older releases, potentially leading to data corruption.
> > >
> > > Adding a new op that would be an alias for CEPH_OSD_OP_COPY_FROM with
> > > CEPH_OSD_COPY_FROM_FLAG_TRUNCATE_SEQ like Jeff is suggesting, or a new
> > > copy-from2 op that would behave just like copy-from, but reject unknown
> > > flags to avoid similar compatibility issues in the future is probably
> > > the best thing we can do from the client perspective.
> >
> > Yeah, I think copy-from2 is the best path.  I think that means we should
> > revert what we merged to ceph.git a few weeks back, Luis!  Sorry we didn't
> > put all the pieces together before...
>
> Well, that's an unexpected turn.  I'm not disagreeing with that decision
> but since my initial pull request was done one year ago (almost to the
> day!), it's a bit disappointing to see that in the end I'm back to
> square one :-)

Well, I think literally every line from that PR will still go in, just
wrapped in a new OSD op.  Backwards compatibility is hard...

>
> I guess that the PR I mentioned in the cover letter can also be dropped,
> as it's not really usable by the kernel client (at least not until it
> fully supports all the features up to SERVER_OCTOPUS).

No, some form of https://github.com/ceph/ceph/pull/31611 should go in.
I'm pretty certain it will come up at some point in the future, even if
the new field isn't immediately usable today.  Someone porting a change
to the kernel client a couple years from now will thank you for it ;)

Thanks,

                Ilya

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ