Re: urndis(4) fix
On Mon, Sep 24, 2018 at 03:34:59AM +0300, Artturi Alm wrote: > On Sat, Nov 29, 2014 at 09:42:51PM +0100, Mark Kettenis wrote: > > Recent Oracle SPARC machines have a USB gadget to talk to the Service > > Processor (ILOM). This gadget supports both RNDIS and CDC Ethernet. > > The RNDIS bits uncovered a bug in urndis(4). When urndis_ctrl_set() > > sets up the REMOTE_NDIS_SET_MSG command it sets up msg->rm_infobuflen, > > it subsequently overwrites its contents. Apparently many RNDIS > > devices don't care, but this hardware sends a response back that > > indicates the command wasn't accepted. Diff below fixes this problem. > > It matches what Linux does. > > > > Unfortunately, this isn't enough to make the gadget work in RNDIS mode. > > > > I'd appreciate it if people using urndis(4) could test this diff. > > > > Hi, > > just found your mail, i think urndis(4) is not entirely written with > these in mind: > defined(__STRICT_ALIGNMENT) || _BYTE_ORDER != _LITTLE_ENDIAN > > did you ever try with a diff like below? > > -Artturi > sorry for the 'spam', on a second read of your message, i think it might be just more strict than ie. android, not allowing the _SET_MSG from urndis_attach(), because it's "out-of-order" with regards to msg sent by urndis_ctrl_init(). if it's not in the "spec", atleast MS[0] does show things done differently, == init first before setting data packet filter. -Artturi [0]: https://docs.microsoft.com/en-us/windows-hardware/drivers/network/example-connectionless--802-3--initialization-sequence
libnv
Does OpenBSD have anything similar to FreeBSD's libnv? It looks interesting. Thanks, Edgar (please cc me)
Re: urndis0: urndis_decap invalid buffer len 1 < minimum header 44
On Tue, Sep 18, 2018 at 07:13:15PM +0300, Artturi Alm wrote:
> On Sat, Aug 18, 2018 at 11:05:04PM +0300, Artturi Alm wrote:
> > On Thu, Jan 11, 2018 at 12:41:29AM +0200, Artturi Alm wrote:
> > > On Wed, Sep 13, 2017 at 05:51:27AM +0300, Artturi Alm wrote:
> > > > Hi,
> > > >
> > > > even after having recently updated the phone to a newer version of
> > > > android,
> > > > i'm still spammed by urndis w/msg on subject.
> > > >
> > > > doesn't really matter to me what you do to silence it, but something
> > > > like
> > > > below does work for me, and thanks in advacne:)
> > > > -Artturi
> > > >
> > >
> > > ping?
> > > i was told i don't reason my diffs, so here's sorry attempt:
> > > $ dmesg | wc -l
> > > 1040
> > > $ dmesg | grep urndis_decap | wc -l
> > > 1039
> > >
> > > either of the diffs below would work for me.
> > > -Artturi
> > >
> > >
> > > ... this ...
> > >
> > > diff --git a/sys/dev/usb/if_urndis.c b/sys/dev/usb/if_urndis.c
> > > index 5d148da4ab5..7dc12573c0d 100644
> > > --- a/sys/dev/usb/if_urndis.c
> > > +++ b/sys/dev/usb/if_urndis.c
> > > @@ -834,11 +834,11 @@ urndis_decap(struct urndis_softc *sc, struct
> > > urndis_chain *c, u_int32_t len)
> > > len));
> > >
> > > if (len < sizeof(*msg)) {
> > > - printf("%s: urndis_decap invalid buffer len %u < "
> > > + DPRINTF(("%s: urndis_decap invalid buffer len %u < "
> > > "minimum header %zu\n",
> > > DEVNAME(sc),
> > > len,
> > > - sizeof(*msg));
> > > + sizeof(*msg)));
> > > return;
> > > }
> > >
> > >
> > >
> > > ... or this ...
> > >
> > > diff --git a/sys/dev/usb/if_urndis.c b/sys/dev/usb/if_urndis.c
> > > index 5d148da4ab5..4b2c6e89ec9 100644
> > > --- a/sys/dev/usb/if_urndis.c
> > > +++ b/sys/dev/usb/if_urndis.c
> > > @@ -834,6 +834,8 @@ urndis_decap(struct urndis_softc *sc, struct
> > > urndis_chain *c, u_int32_t len)
> > > len));
> > >
> > > if (len < sizeof(*msg)) {
> > > + if (len == 1) /* workaround for spamming androids */
> > > + return;
> > > printf("%s: urndis_decap invalid buffer len %u < "
> > > "minimum header %zu\n",
> > > DEVNAME(sc),
> >
> > Hi,
> >
> > this should fix the urndis_decap() spam more properly by allowing operation
> > as is needed by the linux(android) gadget/function code for usb rndis too,
> > which is explained there to be
> > "zlp framing on tx for strict CDC-Ether conformance",
> > and our cdce(4) does have somewhat similar if (total_len <= 1) goto done;.
> >
> > also, i think the _decap spam is as bad as it is because of those returns,
> > dropping valid packet met before "error", leading to retries likely being
> > the same; triggering the spam loop..
> >
>
> pong? the spam and dropping of a valid packet is not correct.
> this is far from making the driver bug-free/following the spec,
> but definately worth fixing as the only user-visible annoyance.
>
> i don't think i can explain why any better than is done here:
> https://docs.microsoft.com/en-us/windows-hardware/drivers/network/usb-short-packets
>
> above is specifically about/for rndis, even if the url doesn't hint so,
> and there is a note it'll take two minutes to read..
>
new diff below, with some other minor fixes to balance my addition to
urndis_decap.
- urndis_ctrl_set_param() is unused, shouldn't end up in kernel binary,
but atleast should balance compile-time wise :]
- in urndis_ctrl_init(), don't htole32(0) (like is not done in other
urndis_ctrl_xxx either), and setting rm_ver_minor to 1 is against the
"spec", while atleast old androids ignore this, so it hasn't possibly
caused many problems in real life.. see rndis.h for _VERSION_ defines.
and rest is the decap fix, for which i added a comment too.
the version in my previous mail was OK'ed off-list by armani@.
-Artturi
diff --git sys/dev/usb/if_urndis.c sys/dev/usb/if_urndis.c
index 5d148da4ab5..af56ef3cdda 100644
--- sys/dev/usb/if_urndis.c
+++ sys/dev/usb/if_urndis.c
@@ -98,9 +98,9 @@ u_int32_t urndis_ctrl_halt(struct urndis_softc *);
u_int32_t urndis_ctrl_query(struct urndis_softc *, u_int32_t, void *, size_t,
void **, size_t *);
u_int32_t urndis_ctrl_set(struct urndis_softc *, u_int32_t, void *, size_t);
+#if 0
u_int32_t urndis_ctrl_set_param(struct urndis_softc *, const char *, u_int32_t,
void *, size_t);
-#if 0
u_int32_t urndis_ctrl_reset(struct urndis_softc *);
u_int32_t urndis_ctrl_keepalive(struct urndis_softc *);
#endif
@@ -454,9 +454,9 @@ urndis_ctrl_init(struct urndis_softc *sc)
msg->rm_type = htole32(REMOTE_NDIS_INITIALIZE_MSG);
msg->rm_len = htole32(sizeof(*msg));
- msg->rm_rid = htole32(0);
+ msg->rm_rid = 0;
msg->rm_ver_major = htole32(1);
- msg->rm_ver_m
Re: urndis(4) fix
On Sat, Nov 29, 2014 at 09:42:51PM +0100, Mark Kettenis wrote:
> Recent Oracle SPARC machines have a USB gadget to talk to the Service
> Processor (ILOM). This gadget supports both RNDIS and CDC Ethernet.
> The RNDIS bits uncovered a bug in urndis(4). When urndis_ctrl_set()
> sets up the REMOTE_NDIS_SET_MSG command it sets up msg->rm_infobuflen,
> it subsequently overwrites its contents. Apparently many RNDIS
> devices don't care, but this hardware sends a response back that
> indicates the command wasn't accepted. Diff below fixes this problem.
> It matches what Linux does.
>
> Unfortunately, this isn't enough to make the gadget work in RNDIS mode.
>
> I'd appreciate it if people using urndis(4) could test this diff.
>
Hi,
just found your mail, i think urndis(4) is not entirely written with
these in mind:
defined(__STRICT_ALIGNMENT) || _BYTE_ORDER != _LITTLE_ENDIAN
did you ever try with a diff like below?
-Artturi
diff --git sys/dev/rndis.h sys/dev/rndis.h
index 140793990e9..7036d198d4e 100644
--- sys/dev/rndis.h
+++ sys/dev/rndis.h
@@ -98,7 +98,7 @@
struct rndis_msghdr {
uint32_trm_type;
uint32_trm_len;
-};
+} __packed;
/*
* RNDIS data message
@@ -118,7 +118,7 @@ struct rndis_packet_msg {
uint32_trm_pktinfolen;
uint32_trm_vchandle;
uint32_trm_reserved;
-};
+} __packed;
/* Per-packet-info for RNDIS data message */
struct rndis_pktinfo {
@@ -126,7 +126,7 @@ struct rndis_pktinfo {
uint32_trm_type;/* NDIS_PKTINFO_TYPE_ */
uint32_trm_pktinfooffset;
uint8_t rm_data[0];
-};
+} __packed;
#define NDIS_PKTINFO_TYPE_CSUM 0
#define NDIS_PKTINFO_TYPE_IPSEC1
@@ -149,7 +149,7 @@ struct rndis_comp_hdr {
uint32_trm_len;
uint32_trm_rid;
uint32_trm_status;
-};
+} __packed;
/* Initialize the device. */
#define REMOTE_NDIS_INITIALIZE_MSG 0x0002
@@ -162,7 +162,7 @@ struct rndis_init_req {
uint32_trm_ver_major;
uint32_trm_ver_minor;
uint32_trm_max_xfersz;
-};
+} __packed;
struct rndis_init_comp {
uint32_trm_type;
@@ -178,7 +178,7 @@ struct rndis_init_comp {
uint32_trm_align;
uint32_trm_aflistoffset;
uint32_trm_aflistsz;
-};
+} __packed;
/* Halt the device. No response sent. */
#define REMOTE_NDIS_HALT_MSG 0x0003
@@ -187,7 +187,7 @@ struct rndis_halt_req {
uint32_trm_type;
uint32_trm_len;
uint32_trm_rid;
-};
+} __packed;
/* Send a query object. */
#define REMOTE_NDIS_QUERY_MSG 0x0004
@@ -201,7 +201,7 @@ struct rndis_query_req {
uint32_trm_infobuflen;
uint32_trm_infobufoffset;
uint32_trm_devicevchdl;
-};
+} __packed;
struct rndis_query_comp {
uint32_trm_type;
@@ -210,7 +210,7 @@ struct rndis_query_comp {
uint32_trm_status;
uint32_trm_infobuflen;
uint32_trm_infobufoffset;
-};
+} __packed;
/* Send a set object request. */
#define REMOTE_NDIS_SET_MSG0x0005
@@ -224,14 +224,14 @@ struct rndis_set_req {
uint32_trm_infobuflen;
uint32_trm_infobufoffset;
uint32_trm_devicevchdl;
-};
+} __packed;
struct rndis_set_comp {
uint32_trm_type;
uint32_trm_len;
uint32_trm_rid;
uint32_trm_status;
-};
+} __packed;
/* Parameter used by OID_GEN_RNDIS_CONFIG_PARAMETER. */
#define REMOTE_NDIS_SET_PARAM_NUMERIC 0x
@@ -243,7 +243,7 @@ struct rndis_set_parameter {
uint32_trm_type;
uint32_trm_valueoffset;
uint32_trm_valuelen;
-};
+} __packed;
/* Perform a soft reset on the device. */
#define REMOTE_NDIS_RESET_MSG 0x0006
@@ -253,14 +253,14 @@ struct rndis_reset_req {
uint32_trm_type;
uint32_trm_len;
uint32_trm_rid;
-};
+} __packed;
struct rndis_reset_comp {
uint32_trm_type;
uint32_trm_len;
uint32_trm_status;
uint32_trm_adrreset;
-};
+} __packed;
/* 802.3 link-state or undefined message error. Sent by device. */
#define REMOTE_NDIS_INDICATE_STATUS_MSG0x0007
@@ -272,7 +272,7 @@ struct rndis_status_msg {
uint32_trm_stbuflen;
uint32_trm_stbufoffset;
/* rndis_diag_info */
-};
+} __packed;
/*
* Immediately after rndis_status_msg.rm_stbufoffset, if a control
@@ -282,7 +282,7 @@ struct rndis_status_msg {
struct rndis_diag_info {
uint32_trm_diagstatus;
uint32_trm_erroffset;
-};
+} __packed;
/* Keepalive messsage. May be sent by device. */
#define REMOTE_NDIS_KEEPALIVE_MSG 0x000
Minor clarification in ttys(5) man page
You can reload the ttys file by sending SIGHUP to the init process. init(8) mentions this but ttys(5) does not, which can be confusing for users who don't know that. I've added a note to this effect to ttys(5) referencing init(8). Index: ttys.5 === RCS file: /cvs/src/libexec/getty/ttys.5,v retrieving revision 1.11 diff -u -p -r1.11 ttys.5 --- ttys.5 22 Oct 2008 22:16:16 - 1.11 +++ ttys.5 23 Sep 2018 20:21:23 - @@ -129,6 +129,15 @@ may be followed by a quoted command stri will execute .Em before starting the command specified by the second field. +.El +.Pp +Changes to the ttys file will not take effect until it is reloaded by +.Xr init 8 , +which can be triggered without a reboot by sending it a terminal line hangup +(HUP) signal. See the +.Xr init 8 +man page for more information. + .Sh FILES .Bl -tag -width /etc/ttys -compact .It Pa /etc/ttys
csh: quoted string treated as a glob in switch
Hello,
As an exercise I tried writing a csh script and I found strange
behaviour in its parsing of switch statements.
The following snippet causes a syntax error (switch: Missing ].):
set c="["
switch ("$c")
case "[":
echo match
breaksw
endsw
The call tree is main() -> process() -> execute() -> execute() ->
func() -> doswitch() -> search() -> Gmatch() -> pmatch().
Gmatch() is performing a glob match and pmatch() is looking for
matching []s in a glob. In csh a switch case can be a glob
pattern, but the "case" argument is treated as a glob even if
it is quoted. This is at least inconsistent with other glob
usages in csh, where quoted strings are *not* treated as globs.
works as a glob:
ls [ab]*.c
does not work as a glob:
ls "[ab]*.c"
The patch below removes a call to strip(), which was causing
the string (Char *) returned by Dfix1() to lose its QUOTE flag.
When Gmatch()/pmatch() sees the QUOTE flag it knows to not treat
the case argument as a glob.
Thanks to Prashant Satish for helping to debug this.
OK?
- Michael
Index: func.c
===
RCS file: /cvs/src/bin/csh/func.c,v
retrieving revision 1.38
diff -u -p -u -r1.38 func.c
--- func.c 8 Sep 2018 01:28:39 - 1.38
+++ func.c 23 Sep 2018 14:51:25 -
@@ -657,7 +657,7 @@ search(int type, int level, Char *goal)
(void) getword(aword);
if (lastchr(aword) == ':')
aword[Strlen(aword) - 1] = 0;
- cp = strip(Dfix1(aword));
+ cp = Dfix1(aword);
if (Gmatch(goal, cp))
level = -1;
free(cp);
Re: Add bufferevent_setwatermark(3) to manual
On Sat, Sep 22, 2018 at 11:41:05AM -0700, Geoff Hill wrote: > New patch included. > > On Sat, Sep 22, 2018 at 07:09:44AM +0100, Jason McIntyre wrote: > > does the "ok" request mean you have commit access? (sorry, find it hard > > to keep track) > > Nope, no commit access, just a user sending up a documentation fix. > > > regarding the markup: there is no need to quote your argument. actually > > it would be inconsistent to do so. if the intent was to actually display > > quotes in the formatted page (which would be wrong too), you would have > > to use a macro such as Dq. > > Fixed these. Changed to end up leaving the previous paragraph as-is. > > > you need a space between the arg and punctuation > > Fixed. > > > you can find trivial errors like that if you run your changes through > > mandoc: > > > > $ mandoc -Tlint event.3 > > Thanks, that is good to know. Should have kept mdoc(7) up while writing. > > Geoff Thanks! Committed with some additional tweaks. > > > Index: event.3 > === > RCS file: /cvs/src/lib/libevent/event.3,v > retrieving revision 1.54 > diff -u -p -u -r1.54 event.3 > --- event.3 26 Jul 2018 12:50:04 - 1.54 > +++ event.3 22 Sep 2018 17:16:18 - > @@ -68,6 +68,7 @@ > .Nm bufferevent_enable , > .Nm bufferevent_disable , > .Nm bufferevent_settimeout , > +.Nm bufferevent_setwatermark , > .Nm EVBUFFER_INPUT , > .Nm EVBUFFER_OUTPUT > .Nd execute a function when a specific event occurs > @@ -156,6 +157,8 @@ > .Fn "bufferevent_disable" "struct bufferevent *bufev" "short event" > .Ft void > .Fn "bufferevent_settimeout" "struct bufferevent *bufev" "int timeout_read" > "int timeout_write" > +.Ft void > +.Fn "bufferevent_setwatermark" "struct bufferevent *bufev" "short events" > "size_t lowmark" "size_t highmark" > .Ft "struct evbuffer *" > .Fn "EVBUFFER_INPUT" "struct bufferevent *bufev" > .Ft "struct evbuffer *" > @@ -496,6 +499,30 @@ whenever the output buffer is drained be > which is > .Va 0 > by default. > +.Pp > +The > +.Fn bufferevent_setwatermark > +function can set the low and high watermarks > +for read and write events. > +.Fa "events" > +can be > +.Va EV_READ , > +.Va EV_WRITE > +or both via bitwise-OR. > +When used with > +.Va EV_READ , > +a bufferevent does not invoke the user read callback > +unless there is at least > +.Fa lowmark > +data in the buffer. > +If the read buffer is beyond > +.Fa highmark , > +the bufferevent stops reading from the file descriptor. > +When used with > +.Va EV_WRITE , > +the user write callback is invoked whenever the buffered data > +falls below > +.Fa lowmark . > .Pp > The > .Fn bufferevent_write
Re: Add bufferevent_setwatermark(3) to manual
ok nicm On Sat, Sep 22, 2018 at 10:15:21AM +0200, Anton Lindqvist wrote: > On Fri, Sep 21, 2018 at 06:36:54PM -0700, Geoff Hill wrote: > > Hello tech, > > > > I noticed the event(3) manual pages don't mention the > > bufferevent_setwatermark(3) function and glosses over the details of > > watermarks, even though there's a few programs in userland that set both > > read and write watermarks. Looks like there was an effort in 2017 > > to add some documentation but it stalled. > > > > Here's a patch that adds the function synopsis and a brief description > > of how watermarks work separately for read and write. Mostly copied from > > the function declaration comments in event.h. > > > > ok? > > Few nits below, otherwise it looks good. Anyone else willing to ok? > > > > > Geoff Hill > > > > > > Index: event.3 > > === > > RCS file: /cvs/src/lib/libevent/event.3,v > > retrieving revision 1.54 > > diff -u -p -u -r1.54 event.3 > > --- event.3 26 Jul 2018 12:50:04 - 1.54 > > +++ event.3 22 Sep 2018 01:26:56 - > > @@ -68,6 +68,7 @@ > > .Nm bufferevent_enable , > > .Nm bufferevent_disable , > > .Nm bufferevent_settimeout , > > +.Nm bufferevent_setwatermark , > > .Nm EVBUFFER_INPUT , > > .Nm EVBUFFER_OUTPUT > > .Nd execute a function when a specific event occurs > > @@ -156,6 +157,8 @@ > > .Fn "bufferevent_disable" "struct bufferevent *bufev" "short event" > > .Ft void > > .Fn "bufferevent_settimeout" "struct bufferevent *bufev" "int > > timeout_read" "int timeout_write" > > +.Ft void > > +.Fn "bufferevent_setwatermark" "struct bufferevent *bufev" "short events" > > "size_t lowmark" "size_t highmark" > > .Ft "struct evbuffer *" > > .Fn "EVBUFFER_INPUT" "struct bufferevent *bufev" > > .Ft "struct evbuffer *" > > @@ -492,10 +495,35 @@ and > > When read enabled the bufferevent will try to read from the file > > descriptor and call the read callback. > > The write callback is executed > > -whenever the output buffer is drained below the write low watermark, > > +whenever the output buffer is drained below the write > > +.Fa "lowmark" , > > which is > > .Va 0 > > by default. > > +.Pp > > +The > > +.Fn bufferevent_setwatermark > > +function can set the the low and high watermarks > > One `the' is enough. > > > +for read and write events. > > +.Fa "events" > > +can be > > +.Va EV_READ , > > +.Va EV_WRITE > > +or both. > > +When used with > > +.Va EV_READ , > > +a bufferevent does not invoke the user read callback > > +unless there is at least > > +.Fa "lowmark" > > +data in the buffer. > > +If the read buffer is beyond > > +.Fa "highmark" , > > +the bufferevent stops reading from the file descriptor. > > +When used with > > +.Va EV_WRITE, > > Missing space between EV_WRITE and comma. > > > +the user write callback is invoked whenever the buffered data > > +falls below > > +.Fa "lowmark" . > > .Pp > > The > > .Fn bufferevent_write
