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]
Message-ID: <20101213184057.GB18736@sirena.org.uk>
Date:	Mon, 13 Dec 2010 18:40:57 +0000
From:	Mark Brown <broonie@...ena.org.uk>
To:	Luben Tuikov <ltuikov@...oo.com>
Cc:	Alan Cox <alan@...rguk.ukuu.org.uk>, Greg KH <greg@...ah.com>,
	linux-kernel@...r.kernel.org, linux-usb@...r.kernel.org
Subject: Re: [PATCH] [USB] UASP: USB Attached SCSI (UAS) protocol driver

On Fri, Dec 10, 2010 at 06:26:56PM -0800, Luben Tuikov wrote:

[Reflowed your text into 80 columns, you might want to look at your MUA
configuration here.]

> --- On Fri, 12/10/10, Alan Cox <alan@...rguk.ukuu.org.uk> wrote:

> > "Do you not see HOW DIFFERENT the two drivers are? Do you
> > not see the
> > difference in quality, presentation, etc, etc."
> > 
> > I find the presentation *very* different. I'm rather
> > worried about the
> > manner in which it is being presented.

> Wait a minute... So a commit patch is not enough any more? Code is not
> enough anymore? Quick and knowledgeable responses are not enough
> anymore?

The issue here is with the kernel change and risk management processes
rather than the code.

Your new code adds a driver which replicates the functionality of an
existing driver.  We've had multiple implementations of the same
functionality in the past.  Usually what happens is that users and
distros get confused and end up swapping randomly between the two
different implemenations trading off the different bug and feature sets,
which doesn't make anyone happy and there's a general idea that we
should try to avoid that.

This means that the 1000 foot review comment is what Greg has been
telling you - the standard approach is to work on the existing code in
place, incrementally making it better.  This avoids the problem with bug
tradeoff (as there's only ever one version in the kernel at once) and
makes it much easier to isolate any new problems if they are introduced.

Sometimes this isn't possible or a good idea for some reason, in which
case the change should really explain that in the changelog (usually
everyone involved will have some awareness of the issues already but a
summary is useful for people picking up a new kernel release or
similar).  At the very least proposing such changes needs to involve
some discussion of why a rewrite is required, and there needs to be some
sort of plan for how everything should converge back onto a single
implementation again.

> http://marc.info/?l=linux-usb&m=129185378612218&w=2

This is a good summary of what improvements the new driver brings
(ideally more of it would have gone in the changelog), the missing bit
is an explanation of why these issues can't be addressed with the usual
process of incremental improvements to the existing code, discussion
of how the existence of the two separate implementations would be
resolved and discussion of the user visible impact of swapping to a new
implementation.

Bear in mind that all your changelog said originally was:

| UASP: USB Attached SCSI (UAS) protocol driver

| This driver allows you to connect to UAS devices
| and use them as SCSI devices

which doesn't say much more than that there's a new implementation of
this.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ