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_minor = htole32(1);
+ msg->rm_ver_minor = 0;
msg->rm_max_xfersz = htole32(RNDIS_BUFSZ);
DPRINTF(("%s: urndis_ctrl_init send: type %u len %u rid %u ver_major %u
"
@@ -627,6 +627,7 @@ urndis_ctrl_set(struct urndis_softc *sc, u_int32_t oid,
void *buf, size_t len)
return rval;
}
+#if 0
u_int32_t
urndis_ctrl_set_param(struct urndis_softc *sc,
const char *name,
@@ -680,7 +681,6 @@ urndis_ctrl_set_param(struct urndis_softc *sc,
return rval;
}
-#if 0
/* XXX : adrreset, get it from response */
u_int32_t
urndis_ctrl_reset(struct urndis_softc *sc)
@@ -821,12 +821,15 @@ urndis_decap(struct urndis_softc *sc, struct urndis_chain
*c, u_int32_t len)
struct rndis_packet_msg *msg;
struct ifnet *ifp;
int s;
+ int got;
int offset;
ifp = GET_IFP(sc);
+ got = 0;
offset = 0;
- while (len > 0) {
+ /* >1 to guard against "one-byte zero packet", as written in MS docs */
+ while (len > 1) { /* XXX should use ep wMaxPacketSize, but minimal.. */
msg = (struct rndis_packet_msg *)((char*)c->sc_buf + offset);
m = c->sc_mbuf;
@@ -839,7 +842,7 @@ urndis_decap(struct urndis_softc *sc, struct urndis_chain
*c, u_int32_t len)
DEVNAME(sc),
len,
sizeof(*msg));
- return;
+ break;
}
DPRINTF(("%s: urndis_decap len %u data(off:%u len:%u) "
@@ -859,14 +862,14 @@ urndis_decap(struct urndis_softc *sc, struct urndis_chain
*c, u_int32_t len)
DEVNAME(sc),
letoh32(msg->rm_type),
REMOTE_NDIS_PACKET_MSG);
- return;
+ break;
}
if (letoh32(msg->rm_len) < sizeof(*msg)) {
printf("%s: urndis_decap invalid msg len %u < %zu\n",
DEVNAME(sc),
letoh32(msg->rm_len),
sizeof(*msg));
- return;
+ break;
}
if (letoh32(msg->rm_len) > len) {
printf("%s: urndis_decap invalid msg len %u > buffer "
@@ -874,7 +877,7 @@ urndis_decap(struct urndis_softc *sc, struct urndis_chain
*c, u_int32_t len)
DEVNAME(sc),
letoh32(msg->rm_len),
len);
- return;
+ break;
}
if (letoh32(msg->rm_dataoffset) +
@@ -889,7 +892,7 @@ urndis_decap(struct urndis_softc *sc, struct urndis_chain
*c, u_int32_t len)
letoh32(msg->rm_dataoffset) +
letoh32(msg->rm_datalen) + RNDIS_HEADER_OFFSET,
letoh32(msg->rm_len));
- return;
+ break;
}
if (letoh32(msg->rm_datalen) < sizeof(struct ether_header)) {
@@ -899,7 +902,7 @@ urndis_decap(struct urndis_softc *sc, struct urndis_chain
*c, u_int32_t len)
DEVNAME(sc),
letoh32(msg->rm_datalen),
sizeof(struct ether_header)));
- return;
+ break;
}
memcpy(mtod(m, char*),
@@ -911,11 +914,14 @@ urndis_decap(struct urndis_softc *sc, struct urndis_chain
*c, u_int32_t len)
ifp->if_ierrors++;
} else {
ml_enqueue(&ml, m);
+ got++;
}
offset += letoh32(msg->rm_len);
len -= letoh32(msg->rm_len);
}
+ if (!got)
+ return;
s = splnet();
if_input(ifp, &ml);
