Bug 197309 - ggatec broken since r238119 (ioctl(/dev/ggctl): Invalid argument)
Summary: ggatec broken since r238119 (ioctl(/dev/ggctl): Invalid argument)
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: bin (show other bugs)
Version: 10.1-STABLE
Hardware: Any Any
: --- Affects Some People
Assignee: Christian Brueffer
URL: https://svnweb.freebsd.org/base?view=...
Keywords: easy, needs-qa, patch, patch-ready, regression
Depends on:
Blocks:
 
Reported: 2015-02-04 02:40 UTC by ota
Modified: 2015-07-21 14:28 UTC (History)
4 users (show)

See Also:
koobs: mfc-stable10?


Attachments
Easy work arond to unbreak ggatec <-> ggated but break HAST (837 bytes, patch)
2015-02-04 02:40 UTC, ota
no flags Details | Diff
server side reproduce procedure (1.19 KB, text/plain)
2015-03-15 09:09 UTC, ota
no flags Details
ggatec: In g_gatec_create(), don't pass stack garbage to the kernel (1.09 KB, patch)
2015-04-15 10:27 UTC, Fabian Keil
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description ota 2015-02-04 02:40:44 UTC
Created attachment 152537 [details]
Easy work arond to unbreak ggatec <-> ggated but break HAST

ggated disconnects ggatec connections immediately.

The server side prints "ioctl(/dev/ggctl): Invalid argument."

https://svnweb.freebsd.org/base?view=revision&revision=238119 looks to be the cause.

The server side expects gctl_readprov[] to be filled with geom provider but ggatec does not pass such argument.

10.0 RELEASE and thereafter seem to be affected by this.

The attached patch allows ggatec and ggated to communicate again but as the change mention HAST and I don't know how it really works and never used it, the patch may break HAST.
Comment 1 Fabian Keil 2015-03-07 14:39:30 UTC
What are the required steps to reproduce the problem?

ggatec and ggated work as expected on all my systems and given
that r238119 is from 2012, most of them already have it.
Comment 2 ota 2015-03-15 09:09:37 UTC
Created attachment 154318 [details]
server side reproduce procedure

I captured ggated configuration and steps by 'script.'

Once ggated is started, run "ggatec create -o rw 127.0.0.1 /dev/md1" on another terminal.
Comment 3 Fabian Keil 2015-03-19 13:03:07 UTC
The steps work as expected for me. Tested on:

FreeBSD kendra 10.1-STABLE FreeBSD 10.1-STABLE #0 r279989+96d5981(stable/10): Tue Mar 17 19:45:41 CET 2015     fk@kendra:/usr/obj/usr/src/sys/GENERIC  amd64
Comment 4 ota 2015-03-26 02:58:38 UTC
Really?
I wondered if my local changes caused this although I don't have any ggate related changes and tried raw image from the FreeBSD official site.
I did the same commands and failed again.

root@:~ # mdconfig -a -t swap -s 10M
md0
root@:~ # echo '127.0.0.1 RW /dev/md0' > /tmp/gg.exports
root@:~ # ggated /tmp/gg.exports
root@:~ # ggatec create -o rw 127.0.0.1 /dev/md0
Mar 26 02:52:16 ggatec: ggatec: ioctl(/dev/ggctl): Invalid argument.
Mar 26 02:52:16 ggatec: Existing.
root@:~ # uname -a
FreeBSD  10.1-RELEASE FreeBSD 10.1-RELEASE #0 r274401: Tue Nov 11 22:51:51 UTC 2014    root@releng1.nyu.freebsd:/usr/obj/usr/src/sys/GENERIC  i386
Comment 5 Fabian Keil 2015-03-26 10:56:19 UTC
Note that I'm only using ggated on amd64 systems.

Maybe it's an architecture-specific bug, for example a variable size mismatch.
Comment 6 Fabian Keil 2015-04-15 10:27:25 UTC
Created attachment 155621 [details]
ggatec: In g_gatec_create(), don't pass stack garbage to the kernel

I reproduced the problem on i386. Fix attached.
Comment 7 ota 2015-04-29 02:01:07 UTC
Yes, the patch fixed the issue and I confirmed that I can establish a connection.
Comment 8 Kubilay Kocak freebsd_committer freebsd_triage 2015-07-03 05:05:41 UTC
Thank you for the patch Fabian.

CC original committer (pjd) for what appears to be very simple (and confirmed) fix, for resolution in 10.2-RELEASE
Comment 9 commit-hook freebsd_committer freebsd_triage 2015-07-14 10:49:52 UTC
A commit references this bug:

Author: brueffer
Date: Tue Jul 14 10:49:37 UTC 2015
New revision: 285531
URL: https://svnweb.freebsd.org/changeset/base/285531

Log:
  Unbreak ggatec and ggatel on i386 after r238119, which added two more
  'struct g_gate_ctl_create' fields.

  While the behaviour was technically undefined on other architectures
  as well, on the reporter's amd64 systems the uninitialized bytes the
  kernel cares about were always zero so everything worked as expected.

  PR:		197309, 199559
  Submitted by:	ota@j.email.ne.jp, Fabian Keil
  Reviewed by:	pjd
  MFC after:	1 week

Changes:
  head/sbin/ggate/ggatec/ggatec.c
  head/sbin/ggate/ggatel/ggatel.c
Comment 10 Christian Brueffer freebsd_committer freebsd_triage 2015-07-14 10:51:40 UTC
Fixed in HEAD, I'll MFC this in a week.  Thanks!
Comment 11 commit-hook freebsd_committer freebsd_triage 2015-07-21 14:25:34 UTC
A commit references this bug:

Author: brueffer
Date: Tue Jul 21 14:25:22 UTC 2015
New revision: 285748
URL: https://svnweb.freebsd.org/changeset/base/285748

Log:
  MFC: r285531

  Unbreak ggatec and ggatel on i386 after r238119, which added two more
  'struct g_gate_ctl_create' fields.

  While the behaviour was technically undefined on other architectures
  as well, on the reporter's amd64 systems the uninitialized bytes the
  kernel cares about were always zero so everything worked as expected.

  PR:		197309, 199559
  Submitted by:	ota@j.email.ne.jp, Fabian Keil
  Reviewed by:	pjd
  Approved by:	re (gjb)

Changes:
_U  stable/10/
  stable/10/sbin/ggate/ggatec/ggatec.c
  stable/10/sbin/ggate/ggatel/ggatel.c
Comment 12 Christian Brueffer freebsd_committer freebsd_triage 2015-07-21 14:28:37 UTC
Merge done; thanks again!