Re: rt_addr & source address selection

[Alexander Bluhm]

On Tue, Jun 14, 2016 at 12:02:34PM +0200, Martin Pieuchot wrote:
> --- netinet/in_pcb.c  19 Apr 2016 22:16:25 -  1.206
> +++ netinet/in_pcb.c  14 Jun 2016 09:57:25 -
> @@ -924,8 +924,10 @@ in_selectsrc(struct in_addr **insrc, str
>* If we found a route, use the address
>* corresponding to the outgoing interface.
>*/
> - if (ro->ro_rt != NULL)
> - ia = ifatoia(ro->ro_rt->rt_ifa);
> + if (ro->ro_rt != NULL) {
> + *insrc = (ro->ro_rt->rt_addr)->sin_addr;
> + return (0);
> + }
>  
>   if (ia == NULL)
>   return (EADDRNOTAVAIL);

A ia is never set, the code below this line is dead.  Perhaps change
the logic to
if (ro->ro_rt == NULL)
return (EADDRNOTAVAIL);

anyway OK bluhm@

> Index: netinet6/in6_src.c
> ===
> RCS file: /cvs/src/sys/netinet6/in6_src.c,v
> retrieving revision 1.72
> diff -u -p -r1.72 in6_src.c
> --- netinet6/in6_src.c5 Dec 2015 13:21:00 -   1.72
> +++ netinet6/in6_src.c14 Jun 2016 09:57:26 -
> @@ -257,8 +257,11 @@ in6_selectsrc(struct in6_addr **in6src, 
>   ia6 = in6_ifawithscope(ifp, dst, rtableid);
>   if_put(ifp);
>   }
> - if (ia6 == NULL) /* xxx scope error ?*/
> - ia6 = ifatoia6(ro->ro_rt->rt_ifa);
> + if (ia6 == NULL) { /* xxx scope error ?*/
> + *in6src =
> + (ro->ro_rt->rt_addr)->sin6_addr;
> + return (0);
> + }
>   }
>   if (ia6 == NULL)
>   return (EHOSTUNREACH);  /* no route */

Re: af-to on pass out should be a parser error

[Mike Belopuhov]

On Mon, Jun 20, 2016 at 00:27 +0200, Sebastian Benoit wrote:
> Mike Belopuhov(m...@belopuhov.com) on 2016.06.20 00:11:03 +0200:
> > On Sun, Jun 19, 2016 at 23:43 +0200, Sebastian Benoit wrote:
> > > manpage documents that af-to does not work on pass out rules, but the
> > > pf.conf parser allows it, which leads a non working configuration being
> > > loaded.
> > > 
> > > this changes the parser to make pass out .. af-to an error.
> > > 
> > > ok?
> > > 
> > 
> > forgot to mention in my previous mail that af-to follows route-to
> > in this regard.  you can say "pass out route-to" but in fact it's
> > sort of pointless since the routing decision has already been made
> > by the forwarding code.  i'm not certain doing route-to at this
> > point produces a working result regarding created states, but that
> > would indeed contrast with af-to where this is not a supported
> > configuration.
> > 
> > to some extent "pass out af-to" also follows "pass out rdr-to" and
> > "pass in nat-to" in a sense that they're not common and might not
> > produce results one would expect, yet are parsed and installed into
> > the kernel successfully.
> 
> yes,
> 
> i thought these were checked, but there is only a check to make sure
> rdr/nat-to have a direction, not which one. i'll look at that tomorrow.
> 
> thanks.

rdr-to/nat-to are not checked on purpose.  i'm not certain about
route-to/reply-to.

as far as i'm concerned, af-to should be restricted to "pass in".
but it would be nice to know if "pass out route-to" and "pass in
reply-to" produce working configurations to restrict them as well
if they don't.

Re: af-to on pass out should be a parser error

[Sebastian Benoit]

Mike Belopuhov(m...@belopuhov.com) on 2016.06.20 00:11:03 +0200:
> On Sun, Jun 19, 2016 at 23:43 +0200, Sebastian Benoit wrote:
> > manpage documents that af-to does not work on pass out rules, but the
> > pf.conf parser allows it, which leads a non working configuration being
> > loaded.
> > 
> > this changes the parser to make pass out .. af-to an error.
> > 
> > ok?
> > 
> 
> forgot to mention in my previous mail that af-to follows route-to
> in this regard.  you can say "pass out route-to" but in fact it's
> sort of pointless since the routing decision has already been made
> by the forwarding code.  i'm not certain doing route-to at this
> point produces a working result regarding created states, but that
> would indeed contrast with af-to where this is not a supported
> configuration.
> 
> to some extent "pass out af-to" also follows "pass out rdr-to" and
> "pass in nat-to" in a sense that they're not common and might not
> produce results one would expect, yet are parsed and installed into
> the kernel successfully.

yes,

i thought these were checked, but there is only a check to make sure
rdr/nat-to have a direction, not which one. i'll look at that tomorrow.

thanks.

Re: af-to on pass out should be a parser error

[Sebastian Benoit]

Mike Belopuhov(m...@belopuhov.com) on 2016.06.20 00:01:28 +0200:
> On Sun, Jun 19, 2016 at 23:43 +0200, Sebastian Benoit wrote:
> > manpage documents that af-to does not work on pass out rules, but the
> > pf.conf parser allows it, which leads a non working configuration being
> > loaded.
> > 
> > this changes the parser to make pass out .. af-to an error.
> >
> 
> what happens if the direction is not specified?

this works better i hope.

diff --git sbin/pfctl/parse.y sbin/pfctl/parse.y
index 934438c..c491b8e 100644
--- sbin/pfctl/parse.y
+++ sbin/pfctl/parse.y
@@ -1518,6 +1518,9 @@ pfrule: action dir logquick interface af 
proto fromto
}
if ($8.marker & FOM_AFTO)
r.rule_flag |= PFRULE_AFTO;
+   if ($8.marker & FOM_AFTO && r.direction != PF_IN)
+   yyerror("af-to can only be used with direction 
in");
+   YYERROR;
r.af = $5;
 
if ($8.tag)

Re: af-to on pass out should be a parser error

[Mike Belopuhov]

On Sun, Jun 19, 2016 at 23:43 +0200, Sebastian Benoit wrote:
> manpage documents that af-to does not work on pass out rules, but the
> pf.conf parser allows it, which leads a non working configuration being
> loaded.
> 
> this changes the parser to make pass out .. af-to an error.
> 
> ok?
> 

forgot to mention in my previous mail that af-to follows route-to
in this regard.  you can say "pass out route-to" but in fact it's
sort of pointless since the routing decision has already been made
by the forwarding code.  i'm not certain doing route-to at this
point produces a working result regarding created states, but that
would indeed contrast with af-to where this is not a supported
configuration.

to some extent "pass out af-to" also follows "pass out rdr-to" and
"pass in nat-to" in a sense that they're not common and might not
produce results one would expect, yet are parsed and installed into
the kernel successfully.

Re: IP_SENDSRCADDR [2/2] : add cmsg support

[Alexander Bluhm]

On Wed, Jun 15, 2016 at 07:43:37PM +0200, Vincent Gross wrote:
> rev3 below.
> 
> I fixed the line length, the useless bzero(), and also the wording in
> ip.4
> 
> Ok ?

OK bluhm@

> 
> Index: sys/netinet/in.h
> ===
> RCS file: /cvs/src/sys/netinet/in.h,v
> retrieving revision 1.115
> diff -u -p -r1.115 in.h
> --- sys/netinet/in.h  20 Oct 2015 20:22:42 -  1.115
> +++ sys/netinet/in.h  15 Jun 2016 17:37:11 -
> @@ -307,6 +307,7 @@ struct ip_opts {
>  #define IP_RECVRTABLE35   /* bool; receive rdomain w/dgram */
>  #define IP_IPSECFLOWINFO 36   /* bool; IPsec flow info for dgram */
>  #define IP_IPDEFTTL  37   /* int; IP TTL system default */
> +#define IP_SENDSRCADDR   38   /* struct in_addr; source address 
> to use */
>  
>  #define IP_RTABLE0x1021  /* int; routing table, see SO_RTABLE */
>  #define IP_DIVERTFL  0x1022  /* int; divert direction flag opt */
> Index: sys/netinet/udp_usrreq.c
> ===
> RCS file: /cvs/src/sys/netinet/udp_usrreq.c,v
> retrieving revision 1.212
> diff -u -p -r1.212 udp_usrreq.c
> --- sys/netinet/udp_usrreq.c  15 Jun 2016 16:06:35 -  1.212
> +++ sys/netinet/udp_usrreq.c  15 Jun 2016 17:37:11 -
> @@ -888,6 +888,7 @@ udp_output(struct inpcb *inp, struct mbu
>   struct sockaddr_in *sin = NULL;
>   struct udpiphdr *ui;
>   u_int32_t ipsecflowinfo = 0;
> + struct sockaddr_in src_sin;
>   int len = m->m_pkthdr.len;
>   struct in_addr *laddr;
>   int error = 0;
> @@ -906,6 +907,8 @@ udp_output(struct inpcb *inp, struct mbu
>   goto release;
>   }
>  
> + memset(_sin, 0, sizeof(src_sin));
> +
>   if (control) {
>   u_int clen;
>   struct cmsghdr *cm;
> @@ -939,9 +942,20 @@ udp_output(struct inpcb *inp, struct mbu
>   cm->cmsg_level == IPPROTO_IP &&
>   cm->cmsg_type == IP_IPSECFLOWINFO) {
>   ipsecflowinfo = *(u_int32_t *)CMSG_DATA(cm);
> - break;
> - }
> + } else
>  #endif
> + if (cm->cmsg_len == CMSG_LEN(sizeof(struct in_addr)) &&
> + cm->cmsg_level == IPPROTO_IP &&
> + cm->cmsg_type == IP_SENDSRCADDR) {
> + memcpy(_sin.sin_addr, CMSG_DATA(cm),
> + sizeof(struct in_addr));
> + src_sin.sin_family = AF_INET;
> + src_sin.sin_len = sizeof(src_sin);
> + /* no check on reuse when sin->sin_port == 0 */
> + if ((error = in_pcbaddrisavail(inp, _sin,
> + 0, curproc)))
> + goto release;
> + }
>   clen -= CMSG_ALIGN(cm->cmsg_len);
>   cmsgs += CMSG_ALIGN(cm->cmsg_len);
>   } while (clen);
> @@ -979,6 +993,17 @@ udp_output(struct inpcb *inp, struct mbu
>   splx(s);
>   if (error)
>   goto release;
> + }
> +
> + if (src_sin.sin_len > 0 &&
> + src_sin.sin_addr.s_addr != INADDR_ANY &&
> + src_sin.sin_addr.s_addr != inp->inp_laddr.s_addr) {
> + src_sin.sin_port = inp->inp_lport;
> + if (inp->inp_laddr.s_addr != INADDR_ANY &&
> + (error =
> + in_pcbaddrisavail(inp, _sin, 0, curproc)))
> + goto release;
> + laddr = _sin.sin_addr;
>   }
>   } else {
>   if (inp->inp_faddr.s_addr == INADDR_ANY) {
> Index: share/man/man4/ip.4
> ===
> RCS file: /cvs/src/share/man/man4/ip.4,v
> retrieving revision 1.38
> diff -u -p -r1.38 ip.4
> --- share/man/man4/ip.4   20 Oct 2015 22:08:19 -  1.38
> +++ share/man/man4/ip.4   15 Jun 2016 17:37:12 -
> @@ -290,6 +290,34 @@ cmsg_len = CMSG_LEN(sizeof(u_int))
>  cmsg_level = IPPROTO_IP
>  cmsg_type = IP_RECVRTABLE
>  .Ed
> +.Pp
> +When sending on a
> +.Dv SOCK_DGRAM
> +socket with
> +.Xr sendmsg 2
> +, the source address to be used can be passed as ancillary data with a type 
> code of
> +.Dv IP_SENDSRCADDR .
> +The
> +.Va msg_control
> +field in the
> +.Vt msghdr
> +structure should point to a buffer that contains a
> +.Vt cmsghdr
> +structure followed by the requested source address.
> +The
> +.Vt cmsghdr
> +fields should have the following values:
> +.Bd -literal -offset indent
> +cmsg_len = CMSG_LEN(sizeof(struct in_addr))
> +cmsg_level = IPPROTO_IP
> +cmsg_type = IP_SENDSRCADDR
> +.Ed
> +.Pp
> +The same checks and 

Re: af-to on pass out should be a parser error

[Mike Belopuhov]

On Sun, Jun 19, 2016 at 23:43 +0200, Sebastian Benoit wrote:
> manpage documents that af-to does not work on pass out rules, but the
> pf.conf parser allows it, which leads a non working configuration being
> loaded.
> 
> this changes the parser to make pass out .. af-to an error.
>

what happens if the direction is not specified?

> ok?
>

i'm not a native speaker but there should be a verb somewhere :-)
how about "af-to can only be applied to inbound rules"?

> diff --git sbin/pfctl/parse.y sbin/pfctl/parse.y
> index 934438c..0fecba8 100644
> --- sbin/pfctl/parse.y
> +++ sbin/pfctl/parse.y
> @@ -1518,6 +1518,9 @@ pfrule  : action dir logquick interface af 
> proto fromto
>   }
>   if ($8.marker & FOM_AFTO)
>   r.rule_flag |= PFRULE_AFTO;
> + if ($8.marker & FOM_AFTO && r.direction == PF_OUT)
> + yyerror("af-to not possible with direction 
> out");
> + YYERROR;
>   r.af = $5;
>  
>   if ($8.tag)

af-to on pass out should be a parser error

[Sebastian Benoit]

manpage documents that af-to does not work on pass out rules, but the
pf.conf parser allows it, which leads a non working configuration being
loaded.

this changes the parser to make pass out .. af-to an error.

ok?

diff --git sbin/pfctl/parse.y sbin/pfctl/parse.y
index 934438c..0fecba8 100644
--- sbin/pfctl/parse.y
+++ sbin/pfctl/parse.y
@@ -1518,6 +1518,9 @@ pfrule: action dir logquick interface af 
proto fromto
}
if ($8.marker & FOM_AFTO)
r.rule_flag |= PFRULE_AFTO;
+   if ($8.marker & FOM_AFTO && r.direction == PF_OUT)
+   yyerror("af-to not possible with direction 
out");
+   YYERROR;
r.af = $5;
 
if ($8.tag)

Re: new feature in pkg_add(1)

[Stuart Henderson]

On 2016/06/18 19:54, frantisek holop wrote:
> does this deprecate -z ?

hopefully not; -z is useful for feeding old pkg_info output into pkg_add
on a new machine, and we've used it in the past for getting past a flag
day bump.

Re: pledge in /usr/bin/ktrace

[Philip Guenther]

On Wed, 15 Jun 2016, Michal Mazurek wrote:
> KTRFAC_PLEDGE on its own isn't very useful, but this change makes it 
> possible to recreate the default string.
> 
> Make the default in the manpage more obvious.
> 
> Also ansify getpoints().

Hmm, there's some deeper problems here.  ltrace(1) needs to be updated 
whenever ktrace(1) is updated...but the description of the -t option in 
ltrace(1) is wrong: the default for ltrace is just 'u'...but '+' would use 
the default of ktrace instead of ltrace.  That's wrong: the whole point of 
'+' is that you can say something like
ltrace -t +t command

to add just 't' to the default, but instead of treating that like "-t ut" 
it was treating it like "-t cinstuxt".

Diff below changes getpoints() to take the default to use for '+' as an 
additional argument, then passes the different value when invoked as 
ltrace.  Also fix up the kdump manpage and usage string, sort the switch 
cases in getpoints(), and remove the pointless 'extern' on function decls.


Look good?

Philip Guenther

Index: ktrace/extern.h
===
RCS file: /data/src/openbsd/src/usr.bin/ktrace/extern.h,v
retrieving revision 1.2
diff -u -p -r1.2 extern.h
--- ktrace/extern.h 16 Feb 2002 21:27:47 -  1.2
+++ ktrace/extern.h 18 Jun 2016 04:52:21 -
@@ -25,5 +25,5 @@
  * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
  */
 
-extern int getpoints(const char *);
-extern const char *ioctlname(unsigned long);
+int getpoints(const char *, int);
+const char *ioctlname(unsigned long);
Index: ktrace/ktrace.1
===
RCS file: /data/src/openbsd/src/usr.bin/ktrace/ktrace.1,v
retrieving revision 1.28
diff -u -p -r1.28 ktrace.1
--- ktrace/ktrace.1 6 Mar 2016 20:25:27 -   1.28
+++ ktrace/ktrace.1 18 Jun 2016 04:40:42 -
@@ -111,16 +111,10 @@ Enable (disable) tracing on the indicate
 flag is permitted).
 .It Fl t Ar trstr
 The string argument represents the kernel trace points, one per letter.
-The default flags are
-.Cm c ,
-.Cm i ,
-.Cm n ,
-.Cm s ,
-.Cm t ,
-.Cm u ,
-and
-.Cm x .
-The following table equates the letters with the tracepoints:
+By default all trace points except for
+.Cm X
+are enabled.
+The following table equates the letters with the trace points:
 .Pp
 .Bl -tag -width flag -offset indent -compact
 .It Cm c
@@ -129,6 +123,10 @@ trace system calls
 trace I/O
 .It Cm n
 trace namei translations
+.It Cm p
+trace violation of
+.Xr pledge 2
+restrictions
 .It Cm s
 trace signal processing
 .It Cm t
Index: ktrace/ktrace.c
===
RCS file: /data/src/openbsd/src/usr.bin/ktrace/ktrace.c,v
retrieving revision 1.32
diff -u -p -r1.32 ktrace.c
--- ktrace/ktrace.c 18 Apr 2015 18:28:37 -  1.32
+++ ktrace/ktrace.c 18 Jun 2016 04:51:53 -
@@ -87,7 +87,7 @@ main(int argc, char *argv[])
inherit = 1;
break;
case 't':
-   trpoints = getpoints(optarg);
+   trpoints = getpoints(optarg, KTRFAC_USER);
if (trpoints < 0) {
warnx("unknown facility in %s", optarg);
usage();
@@ -133,7 +133,7 @@ main(int argc, char *argv[])
pidset = 1;
break;
case 't':
-   trpoints = getpoints(optarg);
+   trpoints = getpoints(optarg, DEF_POINTS);
if (trpoints < 0) {
warnx("unknown facility in %s", optarg);
usage();
Index: ktrace/ltrace.1
===
RCS file: /data/src/openbsd/src/usr.bin/ktrace/ltrace.1,v
retrieving revision 1.9
diff -u -p -r1.9 ltrace.1
--- ktrace/ltrace.1 6 Mar 2016 20:25:27 -   1.9
+++ ktrace/ltrace.1 18 Jun 2016 04:52:04 -
@@ -102,16 +102,9 @@ Inherit; pass the trace flags to all fut
 processes.
 .It Fl t Ar trstr
 The string argument represents the kernel trace points, one per letter.
-The default flags are
-.Cm c ,
-.Cm i ,
-.Cm n ,
-.Cm s ,
-.Cm t ,
-.Cm u ,
-and
-.Cm x .
-The following table equates the letters with the tracepoints:
+The default is just
+.Cm u .
+The following table equates the letters with the trace points:
 .Pp
 .Bl -tag -width flag -offset indent -compact
 .It Cm c
@@ -120,6 +113,10 @@ trace system calls
 trace I/O
 .It Cm n
 trace namei translations
+.It Cm p
+trace violation of
+.Xr pledge 2
+restrictions
 .It Cm s
 trace signal processing
 .It Cm t
Index: ktrace/subr.c
===
RCS file: 

Re: new feature in pkg_add(1)

[Nayden Markatchev]

amd64 snapshots have been flowing again since yesterday. apologies for
the delay.

On Fri, Jun 17, 2016 at 8:02 AM, Marc Espie  wrote:
> I was waiting for snapshots to come up with the new stuff, but it
> looks like amd64 will be a bit late.  Someone is still hiking in
> the mountains...
>
>
> A week ago or so, I committed support for some disambiguating
> filter in pkg_add.
>
> This means that you can now simply install packages for ports which
> have several versions in the tree without having to know the exact
> version.
>
> For instance,
>
> pkg_add python%2.7
> will install a python from the lang/python/2.7 branch.
>
>
> e.g., stem%branch  takes all packages that match stem-* and filters them
> according to a word in their pkgpath, e.g., you can also use it for, say
> postfix%stable  and other things.
>
>
> This is not really a change, but I've checked where I put %m and stuff
> support, it works in pkg.conf, it also works in PKG_PATH and on the
> command-line.
>
> This means that you can, for instance, write something like
> pkg_add http://ftp.fr.openbsd.org/%m/tcl%8.5
> and this will grab tcl on the 8.5 branch from the "appropriate"
> directory of the froggie server, depending on your OpenBSD version
> and architecture.
>

Re: Building installer as cross

[Theo de Raadt]

I am not going to approve a change to distrib that only tries to
affect one architecture.

Sadly there are a very wide variety of build methods for the various
architectures which one day we should simplify and unify.  Adding more
complexity goes the wrong way.

Re: Building installer as cross

[Masao Uebayashi]

Slightly improved by killing stupid .ifdef added under src/distrib by honoring
.OBJDIR.

diff --git a/Makefile.cross b/Makefile.cross
index 8e2afa1..2a686eb 100644
--- a/Makefile.cross
+++ b/Makefile.cross
@@ -1,7 +1,7 @@
 # $OpenBSD: Makefile.cross,v 1.85 2016/05/11 21:52:48 deraadt Exp $
 
 cross-tools:   cross-includes cross-binutils cross-gcc cross-lib
-cross-distrib: cross-tools cross-bin cross-share cross-sys cross-etc-root-var
+cross-build:   cross-tools cross-bin cross-share cross-sys cross-etc-root-var 
cross-distrib
 # cross-gnu \
 
 # Have to compute TARGET_ARCH directly in the Makefile, for .if tests involving
@@ -364,6 +364,21 @@ cross-etc-root-var:${CROSSOBJ}
DESTDIR=${CROSSDIR} \
${MAKE} distribution-etc-root-var)
 
+# XXX Append `-j1' to ${MAKEFLAGS} to force serialized make execution
+cross-distrib:   ${CROSSOBJ}
+   MACHINE=${TARGET} \
+   MACHINE_ARCH=${TARGET_ARCH} MACHINE_CPU=${TARGET_CPU}; \
+   export MACHINE MACHINE_ARCH MACHINE_CPU; \
+   for i in distrib; do \
+   (cd ${.CURDIR}/$$i; \
+   eval ${CROSSENV} MAKEOBJDIR=obj.${MACHINE}.${TARGET} \
+   BSDOBJDIR=${CROSSDIR}/usr/obj \
+   MAKEFLAGS=\"${MAKEFLAGS} -j1\" \
+   SKIPDIR=\"${NO_CROSS}\" \
+   DESTDIR=${CROSSDIR} \
+   ${MAKE} depend all install); \
+   done
+
 cross-depend:
@(cd ${.CURDIR} && \
BSDOBJDIR=${CROSSDIR}/usr/obj \
diff --git a/distrib/amd64/common/Makefile.inc 
b/distrib/amd64/common/Makefile.inc
index 8a2bfea..8fcbc45 100644
--- a/distrib/amd64/common/Makefile.inc
+++ b/distrib/amd64/common/Makefile.inc
@@ -76,12 +76,13 @@ bsd.rd: ${IMAGE} bsd rdsetroot
cp bsd bsd.rd
${.OBJDIR}/rdsetroot bsd.rd ${IMAGE}
 
-
 bsd:
-   cd ${.CURDIR}/../../../sys/arch/amd64/conf && config ${RAMDISK}
-   cd ${.CURDIR}/../../../sys/arch/amd64/compile/${RAMDISK} && \
+   config -s ${.CURDIR}/../../../sys \
+   -b ${.OBJDIR}/../../../sys/arch/amd64/compile/${RAMDISK} \
+   ${.CURDIR}/../../../sys/arch/amd64/conf/${RAMDISK}
+   cd -P ${.OBJDIR}/../../../sys/arch/amd64/compile/${RAMDISK} && \
${MAKE} clean && COPTS=-Os exec ${MAKE}
-   cp ${.CURDIR}/../../../sys/arch/amd64/compile/${RAMDISK}/bsd bsd
+   cp ${.OBJDIR}/../../../sys/arch/amd64/compile/${RAMDISK}/bsd bsd
 
 ${IMAGE}: ${CBIN} rd_setup do_files rd_teardown
 
@@ -123,6 +124,7 @@ install:
 
 ${CBIN}.mk ${CBIN}.cache ${CBIN}.c: ${CRUNCHCONF}
crunchgen -E -D ${BSDSRCDIR} -L ${DESTDIR}/usr/lib \
+   -O ${.OBJDIR} \
-c ${CBIN}.c -e ${CBIN} -m ${CBIN}.mk ${CRUNCHCONF}
 
 ${CBIN}: ${CBIN}.mk ${CBIN}.cache ${CBIN}.c

Re: UPDATE: xkeyboard-config 2.18

[Matthieu Herrb]

On Mon, Jun 13, 2016 at 06:58:51PM +0500, Alexandr Shadchin wrote:
> Hi,
> 
> This diff updates xkeyboard-config to the latest release.
> 
> Test:
> * ftp http://devio.us/~koba/distfiles/xkeyboard-config-2.18.diff (or attach)
> * cd /path/to/xenocara
> * patch < /path/to/xkeyboard-config-2.18.diff
> * cd data/xkeyboard-config
> * make clean
> * make obj
> * make build
> * restart X
> 
> Comments ? OK ?
> 

ok matthieu@

-- 
Matthieu Herrb


signature.asc
Description: PGP signature

Re: simpler audioctl

[Sebastien Marie]

On Sun, Jun 19, 2016 at 04:22:31PM +0200, Alexandre Ratchov wrote:
> This diff reimplements audioctl with less lines, using the recently
> added ioctls and simplifies its output.  audioctl is the last
> remaining user of few old ioctls and this diff would permit to
> simplify the audio(4) driver soon.
> 
> - group all encoding parameters in a signle string, ex.  "s16le",
>   this way we use the same naming scheme as aucat, sndiod and many
>   ports.
> - remove "properties" as they are not used any longer
> - remove the list of encodings as there's no benefit in having it. 
>   We don't have lists for other parameters (sample rates, channel
>   numbers) either.
> - add -q option, to look like sysctl
> - remove unused -a option
> - document all audio driver variables in the man page
> - document difference between /dev/audioctl0 and /dev/audio0
> - give an example of how to test hardware capabilities with
>   audioctl
> - stop using symlinks in /dev, most other software doesn't use
>   them.
> 
> OK?

OK semarie@

I reviewed the audioctl.c result file (and not the diff) as it is a
rewrite.

Just one point about the man page:

 Test if the hardware supports 24-bit encoding and 44100Hz sample rate:

   $ audioctl -f /dev/audio0 encoding=s24 rate=44100

"Test" is somehow incorrect as if the device supports it, the values
will be changed: so it is more than just testing.
  

> Index: audioctl.1
> ===
> RCS file: /cvs/src/usr.bin/audioctl/audioctl.1,v
> retrieving revision 1.28
> diff -u -p -u -p -r1.28 audioctl.1
> --- audioctl.129 Jan 2016 10:45:38 -  1.28
> +++ audioctl.119 Jun 2016 14:19:39 -
> @@ -31,42 +31,34 @@
>  .Os
>  .Sh NAME
>  .Nm audioctl
> -.Nd control audio device
> +.Nd get or set audio driver variables
>  .Sh SYNOPSIS
>  .Nm audioctl
> -.Op Fl an
>  .Op Fl f Ar file
>  .Nm audioctl
>  .Op Fl n
>  .Op Fl f Ar file
>  .Ar name ...
>  .Nm audioctl
> -.Op Fl n
> +.Op Fl nq
>  .Op Fl f Ar file
>  .Ar name Ns = Ns Ar value ...
>  .Sh DESCRIPTION
>  The
>  .Nm
> -command displays or sets various audio system driver variables.
> -If a list of variables is present on the command line,
> -.Nm
> -prints the current value of those variables for the specified device.
> -By default,
> -.Nm
> -operates on the
> -.Pa /dev/audioctl
> -device.
> -.Pp
> +utility retrieves or sets
> +.Xr audio 4
> +driver variables.
>  The options are as follows:
> -.Bl -tag -width "name=valueXX"
> -.It Fl a
> -Print all device variables and their current values.
> -This is the default, if no parameters are given to
> -.Nm .
> +.Bl -tag -width Ds
>  .It Fl f Ar file
> -Specify an alternative audio control device.
> +Specifies the audio control device or the audio device.
> +The default is
> +.Pa /dev/audioctl0 .
>  .It Fl n
>  Suppress printing of the variable name.
> +.It Fl q
> +Suppress all output when setting a variable.
>  .It Ar name Ns = Ns Ar value
>  Attempt to set the specified variable
>  .Ar name
> @@ -74,22 +66,84 @@ to
>  .Ar value .
>  .El
>  .Pp
> +If the audio control device is used, then values are only stored in the
> +.Xr audio 4
> +driver; they will be submitted to the hardware the next time the
> +device is opened for playback or recording.
> +If the audio device is used instead of the control device,
> +then values are negotiated with the hardware immediately; this requires
> +exclusive access to the device.
>  Variables may only be changed if the device is not opened for
>  playback or recording by another process.
> -.Sh ENVIRONMENT
> -.Bl -tag -width AUDIOCTLDEVICE
> -.It Ev AUDIOCTLDEVICE
> -Audio control device to use.
> +.Pp
> +The following variable names are available:
> +.Bl -column "record.channels"
> +.It Sy Name Ta Sy Meaning
> +.It name Ta device name as shown by
> +.Xr dmesg 8
> +.It mode Ta current device mode (
> +.Va play ,
> +.Va record
> +or both)
> +.It pause Ta set if not attempting to start
> +.It active Ta set if playing or recording
> +.It nblks Ta number of blocks (in frames) in the play buffer
> +.It blksz Ta number of frames per block
> +.It rate Ta sample rate in Hz
> +.It encoding Ta current sample format
> +.It play.channels Ta number of play channels
> +.It play.bytes Ta bytes played since playback started
> +.It play.errors Ta bytes inserted during underruns
> +.It record.channels Ta number of recording channels
> +.It record.bytes Ta bytes recorded since device started
> +.It record.errors Ta bytes dropped during overruns
>  .El
> +.Pp
> +Encoding names use the following scheme: signedness
> +.Po
> +.Va s
> +or
> +.Va u
> +.Pc
> +followed
> +by the precision in bits, the byte-order
> +.Po
> +.Va le
> +or
> +.Va be
> +.Pc ,
> +the number of
> +bytes per sample, and the alignment
> +.Po
> +.Va msb
> +or
> +.Va lsb
> +.Pc .
> +Only the signedness and the precision are mandatory.
> +Examples:
> +.Va u8 , s16le , s24le3 , s24le4lsb .
>  .Sh FILES
> -.Bl -tag -width /dev/audioctl

bgpd logging nexthop valid

[Sebastian Benoit]

i would like to make bgpd a bit more quiet.

This type of message

 bgpd[59424]: nexthop 1.2.3.4 now valid: via 192.168.0.1

happens quite often depending on your upstreams. This makes it a debug
message only.

ok?

diff --git usr.sbin/bgpd/bgpd.c usr.sbin/bgpd/bgpd.c
index 8e0031e..8925086 100644
--- usr.sbin/bgpd/bgpd.c
+++ usr.sbin/bgpd/bgpd.c
@@ -771,7 +771,7 @@ send_nexthop_update(struct kroute_nexthop *msg)
quit = 1;
}
 
-   log_info("nexthop %s now %s%s%s", log_addr(>nexthop),
+   log_debug("nexthop %s now %s%s%s", log_addr(>nexthop),
msg->valid ? "valid" : "invalid",
msg->connected ? ": directly connected" : "",
msg->gateway.aid ? gw : "");

Re: new feature in pkg_add(1)

[frantisek holop]

does this deprecate -z ?

-f
-- 
there's no second chance for a good first impression.

simpler audioctl

[Alexandre Ratchov]

This diff reimplements audioctl with less lines, using the recently
added ioctls and simplifies its output.  audioctl is the last
remaining user of few old ioctls and this diff would permit to
simplify the audio(4) driver soon.

- group all encoding parameters in a signle string, ex.  "s16le",
  this way we use the same naming scheme as aucat, sndiod and many
  ports.
- remove "properties" as they are not used any longer
- remove the list of encodings as there's no benefit in having it. 
  We don't have lists for other parameters (sample rates, channel
  numbers) either.
- add -q option, to look like sysctl
- remove unused -a option
- document all audio driver variables in the man page
- document difference between /dev/audioctl0 and /dev/audio0
- give an example of how to test hardware capabilities with
  audioctl
- stop using symlinks in /dev, most other software doesn't use
  them.

OK?

Index: audioctl.1
===
RCS file: /cvs/src/usr.bin/audioctl/audioctl.1,v
retrieving revision 1.28
diff -u -p -u -p -r1.28 audioctl.1
--- audioctl.1  29 Jan 2016 10:45:38 -  1.28
+++ audioctl.1  19 Jun 2016 14:19:39 -
@@ -31,42 +31,34 @@
 .Os
 .Sh NAME
 .Nm audioctl
-.Nd control audio device
+.Nd get or set audio driver variables
 .Sh SYNOPSIS
 .Nm audioctl
-.Op Fl an
 .Op Fl f Ar file
 .Nm audioctl
 .Op Fl n
 .Op Fl f Ar file
 .Ar name ...
 .Nm audioctl
-.Op Fl n
+.Op Fl nq
 .Op Fl f Ar file
 .Ar name Ns = Ns Ar value ...
 .Sh DESCRIPTION
 The
 .Nm
-command displays or sets various audio system driver variables.
-If a list of variables is present on the command line,
-.Nm
-prints the current value of those variables for the specified device.
-By default,
-.Nm
-operates on the
-.Pa /dev/audioctl
-device.
-.Pp
+utility retrieves or sets
+.Xr audio 4
+driver variables.
 The options are as follows:
-.Bl -tag -width "name=valueXX"
-.It Fl a
-Print all device variables and their current values.
-This is the default, if no parameters are given to
-.Nm .
+.Bl -tag -width Ds
 .It Fl f Ar file
-Specify an alternative audio control device.
+Specifies the audio control device or the audio device.
+The default is
+.Pa /dev/audioctl0 .
 .It Fl n
 Suppress printing of the variable name.
+.It Fl q
+Suppress all output when setting a variable.
 .It Ar name Ns = Ns Ar value
 Attempt to set the specified variable
 .Ar name
@@ -74,22 +66,84 @@ to
 .Ar value .
 .El
 .Pp
+If the audio control device is used, then values are only stored in the
+.Xr audio 4
+driver; they will be submitted to the hardware the next time the
+device is opened for playback or recording.
+If the audio device is used instead of the control device,
+then values are negotiated with the hardware immediately; this requires
+exclusive access to the device.
 Variables may only be changed if the device is not opened for
 playback or recording by another process.
-.Sh ENVIRONMENT
-.Bl -tag -width AUDIOCTLDEVICE
-.It Ev AUDIOCTLDEVICE
-Audio control device to use.
+.Pp
+The following variable names are available:
+.Bl -column "record.channels"
+.It Sy Name Ta Sy Meaning
+.It name Ta device name as shown by
+.Xr dmesg 8
+.It mode Ta current device mode (
+.Va play ,
+.Va record
+or both)
+.It pause Ta set if not attempting to start
+.It active Ta set if playing or recording
+.It nblks Ta number of blocks (in frames) in the play buffer
+.It blksz Ta number of frames per block
+.It rate Ta sample rate in Hz
+.It encoding Ta current sample format
+.It play.channels Ta number of play channels
+.It play.bytes Ta bytes played since playback started
+.It play.errors Ta bytes inserted during underruns
+.It record.channels Ta number of recording channels
+.It record.bytes Ta bytes recorded since device started
+.It record.errors Ta bytes dropped during overruns
 .El
+.Pp
+Encoding names use the following scheme: signedness
+.Po
+.Va s
+or
+.Va u
+.Pc
+followed
+by the precision in bits, the byte-order
+.Po
+.Va le
+or
+.Va be
+.Pc ,
+the number of
+bytes per sample, and the alignment
+.Po
+.Va msb
+or
+.Va lsb
+.Pc .
+Only the signedness and the precision are mandatory.
+Examples:
+.Va u8 , s16le , s24le3 , s24le4lsb .
 .Sh FILES
-.Bl -tag -width /dev/audioctl
-.It Pa /dev/audioctl
-default audio control device
+.Bl -tag -width /dev/audioctl0
+.It Pa /dev/audioctlN
+audio control devices
+.It Pa /dev/audioN
+audio devices
 .El
 .Sh EXAMPLES
-To set the playing sampling rate to 11025 you can enter:
+Display the number of bytes of silence inserted during play buffer
+underruns since device started:
+.Bd -literal -offset indent
+$ audioctl play.errors
+.Ed
+.Pp
+Test if the hardware supports 24-bit encoding and 44100Hz sample rate:
+.Bd -literal -offset indent
+$ audioctl -f /dev/audio0 encoding=s24 rate=44100
+.Ed
 .Pp
-.Dl $ audioctl play.rate=11025
+Note the use of
+.Pa /dev/audio0 ,
+to force negotiation with the hardware.
 .Sh SEE ALSO
 .Xr aucat 1 ,
 .Xr cdio 1 ,
Index: audioctl.c

Re: make umb(4) less verbose by default

[Martin Pieuchot]

On 19/06/16(Sun) 14:40, Stefan Sperling wrote:
> This diff hides some verbose messages which umb(4) prints by default
> and shows most of them only with 'ifconfig umb0 debug'.
> 
> Some of these messages appear during normal operation, even if the
> umb device isn't used at all, such as:
> 
> umb0 at uhub0 port 6 configuration 2 interface 12 "Sierra Wireless, 
> Incorporated EM7305" rev 2.00/0.06 addr 3
> umb0: ignoring invalid segment size 1500
> umb0: vers 1.0
> umb0: state going up from 'down' to 'open'
> umb0: SIM not initialized (PIN missing)
> umb0: packet service changed from unknown to detached, class none, speed: 0 
> up / 0 down
> 
> In my opinion, this default verbosity is too high.

I agree, ok mpi@

> As an aside, this driver can't make up its mind between log(9) and printf(9).
> In a future diff I'd like to make it use printf(9) wherever it uses log(9).
> Would anyone disagree with this?

I support this.

> 
> Index: if_umb.c
> ===
> RCS file: /cvs/src/sys/dev/usb/if_umb.c,v
> retrieving revision 1.1
> diff -u -p -r1.1 if_umb.c
> --- if_umb.c  15 Jun 2016 19:39:34 -  1.1
> +++ if_umb.c  19 Jun 2016 12:34:42 -
> @@ -305,7 +305,7 @@ umb_attach(struct device *parent, struct
>   /* Never trust a USB device! Could try to exploit us */
>   if (sc->sc_ctrl_len < MBIM_CTRLMSG_MINLEN ||
>   sc->sc_ctrl_len > MBIM_CTRLMSG_MAXLEN) {
> - printf("%s: control message len %d out of "
> + DPRINTF("%s: control message len %d out of "
>   "bounds [%d .. %d]\n", DEVNAM(sc),
>   sc->sc_ctrl_len, MBIM_CTRLMSG_MINLEN,
>   MBIM_CTRLMSG_MAXLEN);
> @@ -313,8 +313,8 @@ umb_attach(struct device *parent, struct
>   }
>   sc->sc_maxpktlen = UGETW(md->wMaxSegmentSize);
>   if (sc->sc_maxpktlen < MBIM_MAXSEGSZ_MINVAL) {
> - printf("%s: ignoring invalid segment size %d\n",
> - DEVNAM(sc), sc->sc_maxpktlen);
> + DPRINTF("%s: ignoring invalid segment "
> + "size %d\n", DEVNAM(sc), sc->sc_maxpktlen);
>   /* cont. anyway */
>   sc->sc_maxpktlen = 8 * 1024;
>   }
> @@ -475,7 +475,7 @@ umb_attach(struct device *parent, struct
>   umb_open(sc);
>   splx(s);
>  
> - printf("%s: vers %d.%d\n", DEVNAM(sc), sc->sc_ver_maj, sc->sc_ver_min);
> + DPRINTF("%s: vers %d.%d\n", DEVNAM(sc), sc->sc_ver_maj, sc->sc_ver_min);
>   return;
>  
>  fail:
> @@ -774,21 +774,24 @@ umb_statechg_timeout(void *arg)
>  {
>   struct umb_softc *sc = arg;
>  
> - printf("%s: state change time out\n",DEVNAM(sc));
> + printf("%s: state change timeout\n",DEVNAM(sc));
>   usb_add_task(sc->sc_udev, >sc_umb_task);
>  }
>  
>  void
>  umb_newstate(struct umb_softc *sc, enum umb_state newstate, int flags)
>  {
> + struct ifnet *ifp = GET_IFP(sc);
> +
>   if (newstate == sc->sc_state)
>   return;
>   if (((flags & UMB_NS_DONT_DROP) && newstate < sc->sc_state) ||
>   ((flags & UMB_NS_DONT_RAISE) && newstate > sc->sc_state))
>   return;
> - log(LOG_DEBUG, "%s: state going %s from '%s' to '%s'\n", DEVNAM(sc),
> - newstate > sc->sc_state ? "up" : "down",
> - umb_istate(sc->sc_state), umb_istate(newstate));
> + if (ifp->if_flags & IFF_DEBUG)
> + log(LOG_DEBUG, "%s: state going %s from '%s' to '%s'\n",
> + DEVNAM(sc), newstate > sc->sc_state ? "up" : "down",
> + umb_istate(sc->sc_state), umb_istate(newstate));
>   sc->sc_state = newstate;
>   usb_add_task(sc->sc_udev, >sc_umb_task);
>  }
> @@ -811,10 +814,12 @@ umb_state_task(void *arg)
>  
>   state = sc->sc_state == UMB_S_UP ? LINK_STATE_UP : LINK_STATE_DOWN;
>   if (ifp->if_link_state != state) {
> - log(LOG_INFO, "%s: link state changed from %s to %s\n",
> - DEVNAM(sc),
> - LINK_STATE_IS_UP(ifp->if_link_state) ? "up" : "down",
> - LINK_STATE_IS_UP(state) ? "up" : "down");
> + if (ifp->if_flags & IFF_DEBUG)
> + log(LOG_DEBUG, "%s: link state changed from %s to %s\n",
> + DEVNAM(sc),
> + LINK_STATE_IS_UP(ifp->if_link_state)
> + ? "up" : "down",
> + LINK_STATE_IS_UP(state) ? "up" : "down");
>   ifp->if_link_state = state;
>   if (!LINK_STATE_IS_UP(state)) {
>   /*
> @@ -865,8 +870,7 @@ umb_up(struct umb_softc *sc)
>   sc->sc_tx_seq = 0;
>   if (!umb_alloc_xfers(sc)) {
>

umb(4) man page tweaks

[Stefan Sperling]

Some information in the umb(4) man page seems to be outdated (IPV4 gateway
handling), or doesn't really belong in a man page ("please hack the driver
to add a device to a blacklist").

Also try to shorten the page a bit and move information about troublesome
devices to CAVEATS.

Index: umb.4
===
RCS file: /cvs/src/share/man/man4/umb.4,v
retrieving revision 1.1
diff -u -p -r1.1 umb.4
--- umb.4   15 Jun 2016 19:39:34 -  1.1
+++ umb.4   19 Jun 2016 12:58:24 -
@@ -26,44 +26,27 @@
 The
 .Nm
 driver provides support for USB MBIM devices.
-Those devices establish connections via celluar networks such as
-GPRS, UMTS, and LTE.
 .Pp
-The
-.Nm
-device appears as a regular point-to-point network interface,
+MBIM devices establish connections via cellular networks such as
+GPRS, UMTS, and LTE.
+They appear as a regular point-to-point network interface,
 transporting raw IP frames.
 .Pp
 Required configuration parameters like PIN and APN have to be set
-via
+with
 .Xr ifconfig 8 .
 Once the SIM card has been unlocked with the correct PIN, it
-will remain in this state until the device is power-cycled.
+will remain in this state until the MBIM device is power-cycled.
 In case the device is connected to an "always-on" USB port,
-it is possible to connect to a provider without entering the
-PIN again even afer a reboot of the system.
-.Pp
-If a default gateway route is configured for the
-.Nm
-network interface, the driver will modify the destination IP address
-dynamically, according to the information sent by the network provider.
+it may be possible to connect to a provider without entering the
+PIN again even if the system was rebooted.
 .Sh HARDWARE
-The following devices are known to be supported by the
-.Nm
-driver:
+The following devices should work:
 .Pp
 .Bl -tag -width Ds -offset indent -compact
 .It Tn Sierra Wireless EM8805
 .It Tn Sierra Wireless MC8305
 .El
-.Pp
-There are probably a lot more devices that also work flawlessly.
-If some devices fail to provide a confirming MBIM implementation,
-their vendor and product IDs should be added to the driver's blacklist
-manually.
-Since most devices offer multiple interfaces, blacklisted ones will
-probably be attached by some other driver, such as
-.Xr umsm 4 .
 .Sh SEE ALSO
 .Xr intro 4 ,
 .Xr netintro 4 ,
@@ -78,4 +61,8 @@ probably be attached by some other drive
 .Sh CAVEATS
 The
 .Nm
-driver currently does not support IPv6 addresses.
+driver does not support IPv6.
+.Pp
+Devices which fail to provide a confirming MBIM implementation will
+probably be attached by some other driver, such as
+.Xr umsm 4 .

make umb(4) less verbose by default

[Stefan Sperling]

This diff hides some verbose messages which umb(4) prints by default
and shows most of them only with 'ifconfig umb0 debug'.

Some of these messages appear during normal operation, even if the
umb device isn't used at all, such as:

umb0 at uhub0 port 6 configuration 2 interface 12 "Sierra Wireless, 
Incorporated EM7305" rev 2.00/0.06 addr 3
umb0: ignoring invalid segment size 1500
umb0: vers 1.0
umb0: state going up from 'down' to 'open'
umb0: SIM not initialized (PIN missing)
umb0: packet service changed from unknown to detached, class none, speed: 0 up 
/ 0 down

In my opinion, this default verbosity is too high.

As an aside, this driver can't make up its mind between log(9) and printf(9).
In a future diff I'd like to make it use printf(9) wherever it uses log(9).
Would anyone disagree with this?

Index: if_umb.c
===
RCS file: /cvs/src/sys/dev/usb/if_umb.c,v
retrieving revision 1.1
diff -u -p -r1.1 if_umb.c
--- if_umb.c15 Jun 2016 19:39:34 -  1.1
+++ if_umb.c19 Jun 2016 12:34:42 -
@@ -305,7 +305,7 @@ umb_attach(struct device *parent, struct
/* Never trust a USB device! Could try to exploit us */
if (sc->sc_ctrl_len < MBIM_CTRLMSG_MINLEN ||
sc->sc_ctrl_len > MBIM_CTRLMSG_MAXLEN) {
-   printf("%s: control message len %d out of "
+   DPRINTF("%s: control message len %d out of "
"bounds [%d .. %d]\n", DEVNAM(sc),
sc->sc_ctrl_len, MBIM_CTRLMSG_MINLEN,
MBIM_CTRLMSG_MAXLEN);
@@ -313,8 +313,8 @@ umb_attach(struct device *parent, struct
}
sc->sc_maxpktlen = UGETW(md->wMaxSegmentSize);
if (sc->sc_maxpktlen < MBIM_MAXSEGSZ_MINVAL) {
-   printf("%s: ignoring invalid segment size %d\n",
-   DEVNAM(sc), sc->sc_maxpktlen);
+   DPRINTF("%s: ignoring invalid segment "
+   "size %d\n", DEVNAM(sc), sc->sc_maxpktlen);
/* cont. anyway */
sc->sc_maxpktlen = 8 * 1024;
}
@@ -475,7 +475,7 @@ umb_attach(struct device *parent, struct
umb_open(sc);
splx(s);
 
-   printf("%s: vers %d.%d\n", DEVNAM(sc), sc->sc_ver_maj, sc->sc_ver_min);
+   DPRINTF("%s: vers %d.%d\n", DEVNAM(sc), sc->sc_ver_maj, sc->sc_ver_min);
return;
 
 fail:
@@ -774,21 +774,24 @@ umb_statechg_timeout(void *arg)
 {
struct umb_softc *sc = arg;
 
-   printf("%s: state change time out\n",DEVNAM(sc));
+   printf("%s: state change timeout\n",DEVNAM(sc));
usb_add_task(sc->sc_udev, >sc_umb_task);
 }
 
 void
 umb_newstate(struct umb_softc *sc, enum umb_state newstate, int flags)
 {
+   struct ifnet *ifp = GET_IFP(sc);
+
if (newstate == sc->sc_state)
return;
if (((flags & UMB_NS_DONT_DROP) && newstate < sc->sc_state) ||
((flags & UMB_NS_DONT_RAISE) && newstate > sc->sc_state))
return;
-   log(LOG_DEBUG, "%s: state going %s from '%s' to '%s'\n", DEVNAM(sc),
-   newstate > sc->sc_state ? "up" : "down",
-   umb_istate(sc->sc_state), umb_istate(newstate));
+   if (ifp->if_flags & IFF_DEBUG)
+   log(LOG_DEBUG, "%s: state going %s from '%s' to '%s'\n",
+   DEVNAM(sc), newstate > sc->sc_state ? "up" : "down",
+   umb_istate(sc->sc_state), umb_istate(newstate));
sc->sc_state = newstate;
usb_add_task(sc->sc_udev, >sc_umb_task);
 }
@@ -811,10 +814,12 @@ umb_state_task(void *arg)
 
state = sc->sc_state == UMB_S_UP ? LINK_STATE_UP : LINK_STATE_DOWN;
if (ifp->if_link_state != state) {
-   log(LOG_INFO, "%s: link state changed from %s to %s\n",
-   DEVNAM(sc),
-   LINK_STATE_IS_UP(ifp->if_link_state) ? "up" : "down",
-   LINK_STATE_IS_UP(state) ? "up" : "down");
+   if (ifp->if_flags & IFF_DEBUG)
+   log(LOG_DEBUG, "%s: link state changed from %s to %s\n",
+   DEVNAM(sc),
+   LINK_STATE_IS_UP(ifp->if_link_state)
+   ? "up" : "down",
+   LINK_STATE_IS_UP(state) ? "up" : "down");
ifp->if_link_state = state;
if (!LINK_STATE_IS_UP(state)) {
/*
@@ -865,8 +870,7 @@ umb_up(struct umb_softc *sc)
sc->sc_tx_seq = 0;
if (!umb_alloc_xfers(sc)) {
umb_free_xfers(sc);
-   log(LOG_ERR, "%s: allocation of xfers failed\n",
-   DEVNAM(sc));
+ 

Re: ucom(4) ttyU* doesn't EBUSY

[Marcus Glocker]

On Wed, Jun 15, 2016 at 10:45:28PM +0200, Marcus Glocker wrote:

> On Wed, Jun 15, 2016 at 08:26:01PM +0200, Marcus Glocker wrote:
> 
> > On Wed, Jun 15, 2016 at 10:11:19AM -0700, Philip Guenther wrote:
> > 
> > > On Wed, Jun 15, 2016 at 7:22 AM, Martin Pieuchot  wrote:
> > > > On 11/06/16(Sat) 16:44, Marcus Glocker wrote:
> > > >> Currently one can open multiple instances of /dev/ttyU* since ucom(4)
> > > >> just checks 'TS_ISOPEN' against /dev/cuaU* access.  There are quiet a
> > > >> lot of flags in ucom.c so it's a bit difficult to understand what the
> > > >> initial idea was.  But moving the 'TS_ISOPEN' check before the UCOMCUA
> > > >> branch makes /dev/ttyU* access also return EBUSY if already opened.
> > > >>
> > > >> Ok?  Or better ideas to fix this?
> > > >
> > > > No idea how this is supposed to work but com(4) contains the exact same
> > > > code, so if this is a bug it should probably fixed there too.
> > > 
> > > If I'm logged in on /dev/ttyU0, shouldn't I be able to open my tty,
> > > such as with "stty < /dev/ttyU0".  Wouldn't this change break that?
> > 
> > Right, that wouldn't work anymore.
> > 
> > > What's the problem with multiple opens of the incoming call device?
> > 
> > The "problem" I faced is that by mistake I did open two cu(1) sessions
> > to /dev/ttyU0.  If you try this you will notice that afterwards both
> > cu(1) sessions are broken.  The output and input gets garbled.  First I
> > thought something is broken with my cable until I've noticed that I've
> > opened two cu(1) sessions by mistake.
> > 
> > Therefore I thought similar as when one is opening an usb audio or
> > video device, the device node should be busy for others since this would
> > also lead to interferences.
> > 
> > The inconsitent part here is that /dev/cuaU* _will_ spit EBUSY when
> > already opened.  Is this how it should be?
> 
> I have checked behaviour of ttyU on NetBSD and FreeBSD;  Both return
> EBUSY when the device is already open.  But stty(1) still works for both
> also when ttyU is open.  I try to find how they managed that, for a
> bit.

The issue lies within our cu(1) rewrite.  While the original cu(1) code
checks for ports beeing busy with a function called hunt() we don't do
it anymore.  I'm not feeling like adding this back again, so lets be
careful in future with multiple cu(1) sessions :-)