NetBSD Problem Report #55103

From mlelstv@hoppa.1st.de  Tue Mar 24 07:01:27 2020
Return-Path: <mlelstv@hoppa.1st.de>
Received: from mail.netbsd.org (mail.netbsd.org [199.233.217.200])
	(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits))
	(Client CN "mail.NetBSD.org", Issuer "mail.NetBSD.org CA" (not verified))
	by mollari.NetBSD.org (Postfix) with ESMTPS id AEF2F1A9213
	for <gnats-bugs@gnats.NetBSD.org>; Tue, 24 Mar 2020 07:01:27 +0000 (UTC)
Message-Id: <20200324070059.BC5BF9D@hoppa.1st.de>
Date: Tue, 24 Mar 2020 08:00:59 +0100 (CET)
From: mlelstv@serpens.de
Reply-To: mlelstv@serpens.de
To: gnats-bugs@NetBSD.org
Subject: /dev/wsmouse returns EINVAL
X-Send-Pr-Version: 3.95

>Number:         55103
>Category:       kern
>Synopsis:       wsmouse returns EINVAL
>Confidential:   no
>Severity:       serious
>Priority:       low
>Responsible:    pgoyette
>State:          closed
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Tue Mar 24 07:05:00 +0000 2020
>Closed-Date:    Mon Apr 06 15:06:43 +0000 2020
>Last-Modified:  Mon Apr 06 15:06:43 +0000 2020
>Originator:     Michael van Elst
>Release:        NetBSD 9.0_RC1
>Organization:
-- 
                                Michael van Elst
Internet: mlelstv@serpens.de
                                "A potential Snark may lurk in every tree."
>Environment:


System: NetBSD hoppa 9.0_RC1 NetBSD 9.0_RC1 (HOPPA) #1: Thu Dec 26 21:24:12 CET 2019 mlelstv@gossam:/home/netbsd9/obj.evbarm/home/netbsd9/src/sys/arch/evbarm/compile/HOPPA evbarm
Architecture: earmv6hf
Machine: evbarm
>Description:
Reading from /dev/wsmouse was possible to get mouse events from any program.
You would get legacy events with 32bit time_t unless the program negotiates
the current protocol version.

This now fails and a read returns EINVAL.

Reason is that the legacy handling code was moved into the COMPAT_50 module,
if the module cannot be loaded, you get ENOSYS which is then translated
into EINVAL by the wsmouse driver.

>How-To-Repeat:
hexdump -C /dev/wsmouse

>Fix:
Either move the code out of COMPAT_50 or use the current protocol
version as the default.

>Release-Note:

>Audit-Trail:

Responsible-Changed-From-To: kern-bug-people->pgoyette
Responsible-Changed-By: pgoyette@NetBSD.org
Responsible-Changed-When: Tue, 24 Mar 2020 12:16:30 +0000
Responsible-Changed-Why:
This is obviously my mistake from last year's [pgoyette-compat] merge.


From: Paul Goyette <paul@whooppee.com>
To: gnats-bugs@netbsd.org
Cc: 
Subject: Re: kern/55103: /dev/wsmouse returns EINVAL
Date: Wed, 1 Apr 2020 14:12:31 -0700 (PDT)

 > Reading from /dev/wsmouse was possible to get mouse events from any program.
 > You would get legacy events with 32bit time_t unless the program negotiates
 > the current protocol version.
 >
 > This now fails and a read returns EINVAL.
 >
 > Reason is that the legacy handling code was moved into the COMPAT_50 module,
 > if the module cannot be loaded, you get ENOSYS which is then translated
 > into EINVAL by the wsmouse driver.
 >
 >> How-To-Repeat:
 > hexdump -C /dev/wsmouse
 >
 >> Fix:
 > Either move the code out of COMPAT_50 or use the current protocol
 > version as the default.

 Hmmm, I'm not sure I fully understand this.

 The original code (before merging the [pgoyette-compat] branch) was
 conditional on

  	#if defined(COMPAT_50) || defined(MODULAR)

 (and directly returned EINVAL if those options were not present).

 The compatability code relies on 32-bit time-spec code which is also
 conditional on COMPAT_50.  So I can't see how any of this would do
 anything useful unless the compat_50 module was present (either built-
 in or modload(8)ed).

 I'm also unfamiliar with the "protocol version" so not at all aware
 of what would need to be changed to make it the default.

 The current code should work just fine if either the compat_50 module
 is built-in or has been modload(8)ed.



 +--------------------+--------------------------+-----------------------+
 | Paul Goyette       | PGP Key fingerprint:     | E-mail addresses:     |
 | (Retired)          | FA29 0E3B 35AF E8AE 6651 | paul@whooppee.com     |
 | Software Developer | 0786 F758 55DE 53BA 7731 | pgoyette@netbsd.org   |
 +--------------------+--------------------------+-----------------------+

From: Michael van Elst <mlelstv@serpens.de>
To: gnats-bugs@netbsd.org
Cc: pgoyette@netbsd.org, gnats-admin@netbsd.org, netbsd-bugs@netbsd.org
Subject: Re: kern/55103: /dev/wsmouse returns EINVAL
Date: Thu, 2 Apr 2020 01:23:41 +0200

 On Wed, Apr 01, 2020 at 09:15:02PM +0000, Paul Goyette wrote:
 > 
 >  > Reading from /dev/wsmouse was possible to get mouse events from any program.
 >  The original code (before merging the [pgoyette-compat] branch) was
 >  conditional on
 >   	#if defined(COMPAT_50) || defined(MODULAR)
 >  (and directly returned EINVAL if those options were not present).

 Before COMPAT_50:  reading mouse was possible, no EINVAL failure existed.
 After COMPAT_50: reading mouse was possible because COMPAT_50 was default.
 Now: reading mouse is no longer possible because COMPAT_50 is removed and
 module isn't autoloaded.

 What would you suggest to make "reading mouse is possible" true again?

 - Move compat code back into main code, no option, no optional module.

 - Weaken compatibility and make events with 64bit time_t the default
 unless the module is loaded.

 There is a single user of that interface and the difference between
 "doen't work because no compat code installed" (status quo) and
 "doesn't work because interface isn't compatible" (second choice)
 is small. In either case you only get compatibility (with ancient
 userland!) by loading the module.

 The first choice is also a possibility. Just implementing the old protocol
 version probably needs less code than the module and compat glue.


 Greetings,
 -- 
                                 Michael van Elst
 Internet: mlelstv@serpens.de
                                 "A potential Snark may lurk in every tree."

From: Paul Goyette <paul@whooppee.com>
To: Michael van Elst <mlelstv@serpens.de>
Cc: gnats-bugs@netbsd.org
Subject: Re: kern/55103: /dev/wsmouse returns EINVAL
Date: Wed, 1 Apr 2020 19:38:52 -0700 (PDT)

 On Thu, 2 Apr 2020, Michael van Elst wrote:

 > On Wed, Apr 01, 2020 at 09:15:02PM +0000, Paul Goyette wrote:
 >>
 >> > Reading from /dev/wsmouse was possible to get mouse events from any program.
 >>  The original code (before merging the [pgoyette-compat] branch) was
 >>  conditional on
 >>   	#if defined(COMPAT_50) || defined(MODULAR)
 >>  (and directly returned EINVAL if those options were not present).
 >
 > Before COMPAT_50:  reading mouse was possible, no EINVAL failure existed.
 > After COMPAT_50: reading mouse was possible because COMPAT_50 was default.
 > Now: reading mouse is no longer possible because COMPAT_50 is removed and
 > module isn't autoloaded.
 >
 > What would you suggest to make "reading mouse is possible" true again?
 >
 > - Move compat code back into main code, no option, no optional module.
 >
 > - Weaken compatibility and make events with 64bit time_t the default
 > unless the module is loaded.
 >
 > There is a single user of that interface and the difference between
 > "doen't work because no compat code installed" (status quo) and
 > "doesn't work because interface isn't compatible" (second choice)
 > is small. In either case you only get compatibility (with ancient
 > userland!) by loading the module.
 >
 > The first choice is also a possibility. Just implementing the old protocol
 > version probably needs less code than the module and compat glue.

 Looking at the original code, it seems pretty clear that the intent was
 to restrict use of version-0 to compat code, and that without compat
 code one should use WSEVENT_VERSION.  And that is what the current code
 implements, as far as I can tell.

 I'm not familiar with evbarm systems, so I don't know anything about
 the removal of compat_50 from default kernel(s).  But it seems to me
 that that removal should have updated any "standard" users/callers of
 this interface to use the updated version.

 My only suggestion to resolve this would be to attempt to autoload the
 compat_50 module before calling the compat hook.  If autoload succeeds
 then everything should be fine;  if autoload fails, you get the same
 behaviour as what you get today.  Please start a discussion thread on
 tech-kern if you want me to pursue this approach.

 I do not believe that either "move compat code back ..." or "weaken
 compat ..." are appropriate choices.  But I will attempt to follow
 whatever consensus is reached on tech-kern@ discussion.



 +--------------------+--------------------------+-----------------------+
 | Paul Goyette       | PGP Key fingerprint:     | E-mail addresses:     |
 | (Retired)          | FA29 0E3B 35AF E8AE 6651 | paul@whooppee.com     |
 | Software Developer | 0786 F758 55DE 53BA 7731 | pgoyette@netbsd.org   |
 +--------------------+--------------------------+-----------------------+

From: Michael van Elst <mlelstv@serpens.de>
To: Paul Goyette <paul@whooppee.com>
Cc: gnats-bugs@netbsd.org
Subject: Re: kern/55103: /dev/wsmouse returns EINVAL
Date: Thu, 2 Apr 2020 07:21:24 +0200

 On Wed, Apr 01, 2020 at 07:38:52PM -0700, Paul Goyette wrote:

 > Looking at the original code, it seems pretty clear that the intent was
 > to restrict use of version-0 to compat code, and that without compat
 > code one should use WSEVENT_VERSION.  And that is what the current code
 > implements, as far as I can tell.

 So, in your opinion, everything is correct now because reading the
 mouse without a special program that negotiates the non-default
 protocol version is forbidden. It wasn't forbidden in the beginning,
 it wasn't forbidden when the protocol was updated, but it is now
 forbidden after compat code was removed from the default for
 unrelated reasons but it was still the intent all the time and
 must be honored and that's why autoloading the compat module
 on some platforms, which is the opposite of that intent, also
 exists but is also correct.

 Please understand that I cannot believe this.

 I say that the final consequence wasn't thought about then and
 reading the mouse with simple tools like cat or hexdump is
 something that is useful and that ability should be restored.

From: =?UTF-8?B?SmFyb23DrXIgRG9sZcSNZWs=?= <jaromir.dolecek@gmail.com>
To: "gnats-bugs@NetBSD.org" <gnats-bugs@netbsd.org>
Cc: pgoyette@netbsd.org, Michael van Elst <mlelstv@serpens.de>
Subject: Re: kern/55103: /dev/wsmouse returns EINVAL
Date: Thu, 2 Apr 2020 10:05:36 +0200

 Le jeu. 2 avr. 2020 =C3=A0 07:25, Michael van Elst <mlelstv@serpens.de> a =
 =C3=A9crit :
 >  On Wed, Apr 01, 2020 at 07:38:52PM -0700, Paul Goyette wrote:
 >
 >  > Looking at the original code, it seems pretty clear that the intent wa=
 s
 >  > to restrict use of version-0 to compat code, and that without compat
 >  > code one should use WSEVENT_VERSION.  And that is what the current cod=
 e
 >  > implements, as far as I can tell.
 >
 >  So, in your opinion, everything is correct now because reading the
 >  mouse without a special program that negotiates the non-default
 >  protocol version is forbidden. It wasn't forbidden in the beginning,

 FWIW, I think the most useful behaviour is default to latest version,
 and the COMPAT_05 version would simply force the default to be the
 version-0. If you want to be really nice, it would be cool to add a
 sysctl to set the default version.

 Jaromir

From: mlelstv@serpens.de (Michael van Elst)
To: gnats-bugs@netbsd.org
Cc: 
Subject: Re: kern/55103: /dev/wsmouse returns EINVAL
Date: Thu, 2 Apr 2020 10:57:18 -0000 (UTC)

 jaromir.dolecek@gmail.com (=?UTF-8?B?SmFyb23DrXIgRG9sZcSNZWs=?=) writes:

 > FWIW, I think the most useful behaviour is default to latest version,
 > and the COMPAT_05 version would simply force the default to be the
 > version-0. If you want to be really nice, it would be cool to add a
 > sysctl to set the default version.

 Yes. And if you have a sysctl you don't even need the COMPAT_50 option
 anymore.

 -- 
 -- 
                                 Michael van Elst
 Internet: mlelstv@serpens.de
                                 "A potential Snark may lurk in every tree."

From: Paul Goyette <paul@whooppee.com>
To: gnats-bugs@netbsd.org
Cc: pgoyette@netbsd.org, gnats-admin@netbsd.org, netbsd-bugs@netbsd.org, 
    mlelstv@serpens.de
Subject: Re: kern/55103: /dev/wsmouse returns EINVAL
Date: Thu, 2 Apr 2020 05:40:41 -0700 (PDT)

 On Thu, 2 Apr 2020, Michael van Elst wrote:

 > The following reply was made to PR kern/55103; it has been noted by GNATS.
 >
 > From: Michael van Elst <mlelstv@serpens.de>
 > To: Paul Goyette <paul@whooppee.com>
 > Cc: gnats-bugs@netbsd.org
 > Subject: Re: kern/55103: /dev/wsmouse returns EINVAL
 > Date: Thu, 2 Apr 2020 07:21:24 +0200
 >
 > On Wed, Apr 01, 2020 at 07:38:52PM -0700, Paul Goyette wrote:
 >
 > > Looking at the original code, it seems pretty clear that the intent was
 > > to restrict use of version-0 to compat code, and that without compat
 > > code one should use WSEVENT_VERSION.  And that is what the current code
 > > implements, as far as I can tell.
 >
 > So, in your opinion, everything is correct now because reading the
 > mouse without a special program that negotiates the non-default
 > protocol version is forbidden. It wasn't forbidden in the beginning,
 > it wasn't forbidden when the protocol was updated, but it is now
 > forbidden after compat code was removed from the default for
 > unrelated reasons but it was still the intent all the time and
 > must be honored and that's why autoloading the compat module
 > on some platforms, which is the opposite of that intent, also
 > exists but is also correct.
 >
 > Please understand that I cannot believe this.
 >
 > I say that the final consequence wasn't thought about then and
 > reading the mouse with simple tools like cat or hexdump is
 > something that is useful and that ability should be restored.

 I agree that there is a bug here, but the bug is NOT the requirement
 that use of the old-version protocol is linked to COMPAT_50.  That
 requirement has existed since the new-version protocol was created.

 Rather, the bug is that, when the new protocol was created, caller(s)
 were not updated to use the new protocol.  The caller(s) were left
 using the old protocol, and therefore the caller(s) require that the
 COMPAT_50 option be present.

 As I see it, there are three reasonable paths forward to resolve this
 issue:

 1. Identify and update the caller(s) of this protocol (possibly with
     a sysctl(8) variable to control selection of the protocol), or

 2. Add code in wsevent.c to attempt to autoload the compat_50 module
     if wsevent_copyout_events() is called with the old version, or

 3. Return the evbarm port (and possibly others) to include COMPAT_50
     by default.



 +--------------------+--------------------------+-----------------------+
 | Paul Goyette       | PGP Key fingerprint:     | E-mail addresses:     |
 | (Retired)          | FA29 0E3B 35AF E8AE 6651 | paul@whooppee.com     |
 | Software Developer | 0786 F758 55DE 53BA 7731 | pgoyette@netbsd.org   |
 +--------------------+--------------------------+-----------------------+

From: mlelstv@serpens.de (Michael van Elst)
To: gnats-bugs@netbsd.org
Cc: 
Subject: Re: kern/55103: /dev/wsmouse returns EINVAL
Date: Thu, 2 Apr 2020 12:46:20 -0000 (UTC)

 paul@whooppee.com (Paul Goyette) writes:

 >As I see it, there are three reasonable paths forward to resolve this
 >issue:

 >1. Identify and update the caller(s) of this protocol (possibly with
 >    a sysctl(8) variable to control selection of the protocol), or

 There are no callers that need updating or that even could be updated.
 And mandating an optional compat mode for some platforms is nonsense.

 -- 
 -- 
                                 Michael van Elst
 Internet: mlelstv@serpens.de
                                 "A potential Snark may lurk in every tree."

From: Paul Goyette <paul@whooppee.com>
To: gnats-bugs@netbsd.org
Cc: mlelstv@serpens.de
Subject: Re: kern/55103: /dev/wsmouse returns EINVAL
Date: Thu, 2 Apr 2020 06:07:42 -0700 (PDT)

 On Thu, 2 Apr 2020, Michael van Elst wrote:

 > The following reply was made to PR kern/55103; it has been noted by GNATS.
 >
 > From: mlelstv@serpens.de (Michael van Elst)
 > To: gnats-bugs@netbsd.org
 > Cc:
 > Subject: Re: kern/55103: /dev/wsmouse returns EINVAL
 > Date: Thu, 2 Apr 2020 12:46:20 -0000 (UTC)
 >
 > paul@whooppee.com (Paul Goyette) writes:
 >
 > >As I see it, there are three reasonable paths forward to resolve this
 > >issue:
 >
 > >1. Identify and update the caller(s) of this protocol (possibly with
 > >    a sysctl(8) variable to control selection of the protocol), or
 >
 > There are no callers that need updating or that even could be updated.

 What controls the *ev argument passed to wsevent_copyout_events() via
 wsevent_read()?

 > And mandating an optional compat mode for some platforms is nonsense.

 There is no ""... for some platforms..." intended in my reply.  ALL
 platforms have the same requirement WRT needing the compat_50 module,
 and any platform on which you would like the "old protocol" to work
 would need the module, either built-in or modload(8)ed.



 +--------------------+--------------------------+-----------------------+
 | Paul Goyette       | PGP Key fingerprint:     | E-mail addresses:     |
 | (Retired)          | FA29 0E3B 35AF E8AE 6651 | paul@whooppee.com     |
 | Software Developer | 0786 F758 55DE 53BA 7731 | pgoyette@netbsd.org   |
 +--------------------+--------------------------+-----------------------+

From: matthew green <mrg@eterna.com.au>
To: gnats-bugs@netbsd.org
Cc: pgoyette@netbsd.org, gnats-admin@netbsd.org, netbsd-bugs@netbsd.org,
    mlelstv@serpens.de
Subject: re: kern/55103: /dev/wsmouse returns EINVAL
Date: Fri, 03 Apr 2020 08:04:56 +1100

 the problem is that in the past cat and hexdump were
 useful and usable as "clients".  that no longer works,
 because without the compat code available, there is no
 way, without setting the version, to access these nodes,
 which makes the cat/hexdump mehtods of debugging not work.

 any argument that says "fix your userland" misses the 
 desired (and traditional) operation here and is not a
 helpful answer.

 i think the only real solution we have here is the sysctl
 that changes the default, and having the depend on whether
 there is a path for compat or not -- eg, if it is present,
 then use it, or if it's possibly available (MODULAR) then
 use it, but with no baked in compat or modules, the default
 should be the only working method.

 AFAICT, this avoids breaking old userland with new kernel,
 allows cat/hexdump to work, and keeps everything else
 working in at least the same feature set as before.


 .mrg.

From: Paul Goyette <paul@whooppee.com>
To: matthew green <mrg@eterna.com.au>
Cc: gnats-bugs@netbsd.org, pgoyette@netbsd.org, gnats-admin@netbsd.org, 
    netbsd-bugs@netbsd.org, mlelstv@serpens.de
Subject: re: kern/55103: /dev/wsmouse returns EINVAL
Date: Thu, 2 Apr 2020 16:11:48 -0700 (PDT)

 On Fri, 3 Apr 2020, matthew green wrote:

 > the problem is that in the past cat and hexdump were
 > useful and usable as "clients".  that no longer works,
 > because without the compat code available, there is no
 > way, without setting the version, to access these nodes,
 > which makes the cat/hexdump mehtods of debugging not work.
 >
 > any argument that says "fix your userland" misses the
 > desired (and traditional) operation here and is not a
 > helpful answer.
 >
 > i think the only real solution we have here is the sysctl
 > that changes the default, and having the depend on whether
 > there is a path for compat or not -- eg, if it is present,
 > then use it, or if it's possibly available (MODULAR) then
 > use it, but with no baked in compat or modules, the default
 > should be the only working method.
 >
 > AFAICT, this avoids breaking old userland with new kernel,
 > allows cat/hexdump to work, and keeps everything else
 > working in at least the same feature set as before.

 Let's see if I get this decision-process right...

 Create the hw.wsmouse.default_version sysctl(8) to deal with
 updating the default_version.  (And retain the existing ioctl
 call to change the version used by an opened device.)

 static int default_version =
 #ifdef MODULAR
  					old;
 #else
  					new;
 #endif


 In wsmouse_open()

  	...
  	/*
  	 * The hook returns 0 (for "old version") if the hook is
  	 * set;  otherwise we use the compile-time default version.
  	 * Effectively, we have
  	 *
  	 * if (wsevent_50_available) {
  	 *	version = old;
  	 * } else {
  	 *	version = default_version;
  	 * }
  	 */
  	MODULE_HOOK_CALL(wsevent_50_version_hook(), (),
  		default_version, version);
  	...


 If we do this, then MODULAR kernels will never default to the
 new version, even if the compat_50 module is not present, and
 it will be necessary for "new binaries" to explicitly set the
 version using the ioctl() (or to update the version via the
 sysctl).  "Old binaries" will continue to _fail_ if the compat_50
 module is not present in a MODULAR kernel, but "old binaries"
 _will_ work in non-MODULAR kernels (although the time-stamps in
 the data will be wrong, since we'll set 64-bits).

 Is this what we want?  If so, it should be relatively easy to
 implement.

 Unfortunately due to the introduction of a new compat_hook, it
 would be inappropriate to pull this up to the -9 branch.  :(

 I _think_ this is the decision/result matrix we end up with...

                       Old binary                New binary

 non-MODULAR          Read succeeds, but data   Works fine
                       is wrong - 64-vs-32 bit
                       time-stamps

 MODULAR, and         Read succeeds             Read succeeds, but data
 module present,                                is wrong - 32-vs-64 bit
 sysctl not updated                             time-stamps (can use the
                                                 ioctl to make it work)

 MODULAR, module      Read succeeds, but data   Read succeeds
 present, and sysctl  is wrong
 _is_ updated

 MODULAR, module is   Read fails, as it does    Read fails as it does
 not present, and     today                     today
 sysctl not updated

 MODULAR, module is   Read fails, as it does    Read succeeds
 not present, and     today
 sysctl is updated



 +--------------------+--------------------------+-----------------------+
 | Paul Goyette       | PGP Key fingerprint:     | E-mail addresses:     |
 | (Retired)          | FA29 0E3B 35AF E8AE 6651 | paul@whooppee.com     |
 | Software Developer | 0786 F758 55DE 53BA 7731 | pgoyette@netbsd.org   |
 +--------------------+--------------------------+-----------------------+

From: Martin Husemann <martin@duskware.de>
To: matthew green <mrg@eterna.com.au>
Cc: gnats-bugs@netbsd.org
Subject: Re: kern/55103: /dev/wsmouse returns EINVAL
Date: Fri, 3 Apr 2020 01:45:10 +0200

 On Fri, Apr 03, 2020 at 08:04:56AM +1100, matthew green wrote:
 > the problem is that in the past cat and hexdump were
 > useful and usable as "clients".  that no longer works,
 > because without the compat code available, there is no
 > way, without setting the version, to access these nodes,
 > which makes the cat/hexdump mehtods of debugging not work.

 Isn't (for manual debugging / hexdump) the version of the protocol
 mostly irrelevant, so Jaromir's suggestion should work w/o the sysctl?

 The human reader of the hexdump would need to understand the version
 still, but it is quite easy to guess (both from the dump and the kernel
 version).

 Martin

From: mlelstv@serpens.de (Michael van Elst)
To: gnats-bugs@netbsd.org
Cc: 
Subject: Re: kern/55103: /dev/wsmouse returns EINVAL
Date: Fri, 3 Apr 2020 06:05:08 -0000 (UTC)

 mrg@eterna.com.au (matthew green) writes:

 >i think the only real solution we have here is the sysctl
 >that changes the default, and having the depend on whether
 >there is a path for compat or not -- eg, if it is present,
 >then use it, or if it's possibly available (MODULAR) then
 >use it, but with no baked in compat or modules, the default
 >should be the only working method.

 I have prepared

 http://ftp.netbsd.org/pub/NetBSD/misc/mlelstv/wsevent.diff

 Default is now the new version but you can configure it with sysctl.
 This also means that you now must configure it to get the compat
 behaviour (in addition to building with COMPAT_50 or loading the
 module).

 I thought about chosing the default default version depending on
 build parameters.

 For the COMPAT_50 case that's easy, just set the default based
 on that parameter.

 For the modular case it's difficult because you don't know if the
 module will be loaded later.

 Just using the new version seems to be the least confusing mode
 because its very clear what you get and what you need to do for
 the extremly rare case that you need the old protocol version.

 You can add more complexity to the decision process (i.e. by
 loading the module earlier, even when not needed). But that
 contradicts why the compat code is separated into a module
 in the first place.

 So, if absolute compatibility with the old protocol version
 is wanted, it's probably easier, and safer, to just reintegrate
 the code back (it's a single function of about 20 lines) and
 drop the module.

 -- 
 -- 
                                 Michael van Elst
 Internet: mlelstv@serpens.de
                                 "A potential Snark may lurk in every tree."

From: Paul Goyette <paul@whooppee.com>
To: gnats-bugs@netbsd.org
Cc: mlelstv@serpens.de
Subject: Re: kern/55103: /dev/wsmouse returns EINVAL
Date: Fri, 3 Apr 2020 05:58:56 -0700 (PDT)

 On Fri, 3 Apr 2020, Michael van Elst wrote:

 > The following reply was made to PR kern/55103; it has been noted by GNATS.
 >
 > From: mlelstv@serpens.de (Michael van Elst)
 > To: gnats-bugs@netbsd.org
 > Cc:
 > Subject: Re: kern/55103: /dev/wsmouse returns EINVAL
 > Date: Fri, 3 Apr 2020 06:05:08 -0000 (UTC)
 >
 > mrg@eterna.com.au (matthew green) writes:
 >
 > >i think the only real solution we have here is the sysctl
 > >that changes the default, and having the depend on whether
 > >there is a path for compat or not -- eg, if it is present,
 > >then use it, or if it's possibly available (MODULAR) then
 > >use it, but with no baked in compat or modules, the default
 > >should be the only working method.
 >
 > I have prepared
 >
 > http://ftp.netbsd.org/pub/NetBSD/misc/mlelstv/wsevent.diff
 >
 > Default is now the new version but you can configure it with sysctl.
 > This also means that you now must configure it to get the compat
 > behaviour (in addition to building with COMPAT_50 or loading the
 > module).

 Yes, this looks good to me.


 > I thought about chosing the default default version depending on
 > build parameters.
 >
 > For the COMPAT_50 case that's easy, just set the default based
 > on that parameter.
 >
 > For the modular case it's difficult because you don't know if the
 > module will be loaded later.

 Yes, that's why my changes were going to include a new module_hook,
 to determine if the module was available or not.

 > Just using the new version seems to be the least confusing mode
 > because its very clear what you get and what you need to do for
 > the extremly rare case that you need the old protocol version.

 Agreed.

 > You can add more complexity to the decision process (i.e. by
 > loading the module earlier, even when not needed). But that
 > contradicts why the compat code is separated into a module
 > in the first place.

 Yup.

 > So, if absolute compatibility with the old protocol version
 > is wanted, it's probably easier, and safer, to just reintegrate
 > the code back (it's a single function of about 20 lines) and
 > drop the module.

 It's a bit more complex than that, since using the old protocol
 also means using old 32-bit time-stamps.  In order to reintegrate
 we'd also have to include (some of) the other compat_50 code for
 manipulating 32-bit timevals.

 I'm quite happy with your changes.  Do you want to commit, or
 would you prefer that I do it?


 +--------------------+--------------------------+-----------------------+
 | Paul Goyette       | PGP Key fingerprint:     | E-mail addresses:     |
 | (Retired)          | FA29 0E3B 35AF E8AE 6651 | paul@whooppee.com     |
 | Software Developer | 0786 F758 55DE 53BA 7731 | pgoyette@netbsd.org   |
 +--------------------+--------------------------+-----------------------+

From: mlelstv@serpens.de (Michael van Elst)
To: gnats-bugs@netbsd.org
Cc: 
Subject: Re: kern/55103: /dev/wsmouse returns EINVAL
Date: Fri, 3 Apr 2020 14:10:45 -0000 (UTC)

 paul@whooppee.com (Paul Goyette) writes:

 > It's a bit more complex than that, since using the old protocol
 > also means using old 32-bit time-stamps.

 It's only truncating one 64bit time_t value to a 32bit time_t value.
 While you can put that into an abstraction layer and load the entire
 compat_50 module, you could also just cast the value. :)


 > I'm quite happy with your changes.  Do you want to commit, or
 > would you prefer that I do it?

 I'll commit it later..

 -- 
 -- 
                                 Michael van Elst
 Internet: mlelstv@serpens.de
                                 "A potential Snark may lurk in every tree."

From: Valery Ushakov <uwe@stderr.spb.ru>
To: gnats-bugs@netbsd.org
Cc: pgoyette@netbsd.org, mlelstv@serpens.de
Subject: Re: kern/55103: /dev/wsmouse returns EINVAL
Date: Fri, 3 Apr 2020 17:33:54 +0300

 On Fri, Apr 03, 2020 at 06:10:01 +0000, Michael van Elst wrote:

 >  http://ftp.netbsd.org/pub/NetBSD/misc/mlelstv/wsevent.diff
 >  
 >  Default is now the new version but you can configure it with sysctl.
 >  This also means that you now must configure it to get the compat
 >  behaviour (in addition to building with COMPAT_50 or loading the
 >  module).
 >  
 >  I thought about chosing the default default version depending on
 >  build parameters.
 >  
 >  For the COMPAT_50 case that's easy, just set the default based
 >  on that parameter.
 >  
 >  For the modular case it's difficult because you don't know if the
 >  module will be loaded later.
 >  
 >  Just using the new version seems to be the least confusing mode
 >  because its very clear what you get and what you need to do for
 >  the extremly rare case that you need the old protocol version.
 >  
 >  You can add more complexity to the decision process (i.e. by
 >  loading the module earlier, even when not needed). But that
 >  contradicts why the compat code is separated into a module
 >  in the first place.
 >  
 >  So, if absolute compatibility with the old protocol version
 >  is wanted, it's probably easier, and safer, to just reintegrate
 >  the code back (it's a single function of about 20 lines) and
 >  drop the module.

 Are we over-engineering this?  Previously with compat compiled in
 the behaviour was:

 - if you do not set version, you get old events (compat)
 - if you set the version you get that version

 let's just declare that beahviour standart, not compat.  fold the
 "old" code into the main driver and be done with it.

 "New" clients set teh version anyway, so there's not much need to have
 the new version as the default.

 Also, iirc, the old events were exactly aligned in hexdump output,
 which is nice, so I'd prefer if hexdump output uses the old format :)

 -uwe

From: mlelstv@serpens.de (Michael van Elst)
To: gnats-bugs@netbsd.org
Cc: 
Subject: Re: kern/55103: /dev/wsmouse returns EINVAL
Date: Fri, 3 Apr 2020 15:27:03 -0000 (UTC)

 uwe@stderr.spb.ru (Valery Ushakov) writes:

 > let's just declare that beahviour standart, not compat.  fold the
 > "old" code into the main driver and be done with it.

 That was one of my first suggestions. And if you avoid pulling in
 a library of 32bit time_t functions, it's 20 lines of code.

 > Also, iirc, the old events were exactly aligned in hexdump output,
 > which is nice, so I'd prefer if hexdump output uses the old format :)

 On 64bit machines you don't see any difference between old and new
 format until 2038.

 -- 
 -- 
                                 Michael van Elst
 Internet: mlelstv@serpens.de
                                 "A potential Snark may lurk in every tree."

From: "Michael van Elst" <mlelstv@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/55103 CVS commit: src/sys/dev/wscons
Date: Sat, 4 Apr 2020 07:33:18 +0000

 Module Name:	src
 Committed By:	mlelstv
 Date:		Sat Apr  4 07:33:18 UTC 2020

 Modified Files:
 	src/sys/dev/wscons: wsevent.c

 Log Message:
 Make default protocol version used by wscons selectable and default
 to the current version.

 Fixes PR 55103.


 To generate a diff of this commit:
 cvs rdiff -u -r1.42 -r1.43 src/sys/dev/wscons/wsevent.c

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

State-Changed-From-To: open->needs-pullups
State-Changed-By: pgoyette@NetBSD.org
State-Changed-When: Sat, 04 Apr 2020 13:11:02 +0000
State-Changed-Why:
mlelstv committed a fix - needs pullup to netbsd-9


State-Changed-From-To: needs-pullups->pending-pullups
State-Changed-By: pgoyette@NetBSD.org
State-Changed-When: Sat, 04 Apr 2020 14:11:37 +0000
State-Changed-Why:
Pullup-9 ticket #820


From: "Martin Husemann" <martin@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/55103 CVS commit: [netbsd-9] src/sys/dev/wscons
Date: Mon, 6 Apr 2020 14:44:15 +0000

 Module Name:	src
 Committed By:	martin
 Date:		Mon Apr  6 14:44:15 UTC 2020

 Modified Files:
 	src/sys/dev/wscons [netbsd-9]: wsevent.c

 Log Message:
 Pull up following revision(s) (requested by pgoyette in ticket #820):

 	sys/dev/wscons/wsevent.c: revision 1.43
 	sys/dev/wscons/wsevent.c: revision 1.44

 Make default protocol version used by wscons selectable and default
 to the current version.

 Fixes PR 55103.

 KNF (Format block comment)
 NFCI


 To generate a diff of this commit:
 cvs rdiff -u -r1.42 -r1.42.4.1 src/sys/dev/wscons/wsevent.c

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

State-Changed-From-To: pending-pullups->closed
State-Changed-By: pgoyette@NetBSD.org
State-Changed-When: Mon, 06 Apr 2020 15:06:43 +0000
State-Changed-Why:
Pullup complete


>Unformatted:

Home	PR Database Search