* [PATCH] net/usb: Misc. fixes for the LG-VL600 LTE USB modem
@ 2011-11-09 3:10 Mark Kamichoff
2011-11-09 17:51 ` Dan Williams
0 siblings, 1 reply; 4+ messages in thread
From: Mark Kamichoff @ 2011-11-09 3:10 UTC (permalink / raw)
To: oliver, gregkh; +Cc: netdev, linux-kernel, Mark Kamichoff
Add checking for valid magic values (needed for stability in the event
corrupted packets are received) and remove some other unneeded checks.
Also, fix flagging device as WWAN (Bugzilla bug #39952).
Signed-off-by: Mark Kamichoff <prox@prolixium.com>
---
drivers/net/usb/cdc_ether.c | 2 +-
drivers/net/usb/lg-vl600.c | 30 ++++++++++++++----------------
2 files changed, 15 insertions(+), 17 deletions(-)
diff --git a/drivers/net/usb/cdc_ether.c b/drivers/net/usb/cdc_ether.c
index c924ea2..99ed6eb 100644
--- a/drivers/net/usb/cdc_ether.c
+++ b/drivers/net/usb/cdc_ether.c
@@ -567,7 +567,7 @@ static const struct usb_device_id products [] = {
{
USB_DEVICE_AND_INTERFACE_INFO(0x1004, 0x61aa, USB_CLASS_COMM,
USB_CDC_SUBCLASS_ETHERNET, USB_CDC_PROTO_NONE),
- .driver_info = (unsigned long)&wwan_info,
+ .driver_info = 0,
},
/*
diff --git a/drivers/net/usb/lg-vl600.c b/drivers/net/usb/lg-vl600.c
index d43db32..b975a39 100644
--- a/drivers/net/usb/lg-vl600.c
+++ b/drivers/net/usb/lg-vl600.c
@@ -144,10 +144,11 @@ static int vl600_rx_fixup(struct usbnet *dev, struct sk_buff *skb)
}
frame = (struct vl600_frame_hdr *) buf->data;
- /* NOTE: Should check that frame->magic == 0x53544448?
- * Otherwise if we receive garbage at the beginning of the frame
- * we may end up allocating a huge buffer and saving all the
- * future incoming data into it. */
+ /* Yes, check that frame->magic == 0x53544448 (or 0x44544d48),
+ * otherwise we may run out of memory w/a bad packet */
+ if (ntohl(frame->magic) != 0x53544448 &&
+ ntohl(frame->magic) != 0x44544d48)
+ goto error;
if (buf->len < sizeof(*frame) ||
buf->len != le32_to_cpup(&frame->len)) {
@@ -209,8 +210,9 @@ static int vl600_rx_fixup(struct usbnet *dev, struct sk_buff *skb)
* for IPv6 packets, and set the ethertype to IPv6
* (0x86dd) so Linux can understand it.
*/
- if ((buf->data[sizeof(*ethhdr)] & 0xf0) == 0x60)
- ethhdr->h_proto = __constant_htons(ETH_P_IPV6);
+ if ((buf->data[sizeof(*ethhdr)] & 0xf0) == 0x60) {
+ ethhdr->h_proto = htons(ETH_P_IPV6);
+ }
}
if (count) {
@@ -296,6 +298,11 @@ encapsulate:
* overwrite the remaining fields.
*/
packet = (struct vl600_pkt_hdr *) skb->data;
+ /* The VL600 wants IPv6 packets to have an IPv4 ethertype
+ * Since this modem only supports IPv4 and IPv6, just set all
+ * frames to 0x0800 (ETH_P_IP)
+ */
+ packet->h_proto = htons(ETH_P_IP);
memset(&packet->dummy, 0, sizeof(packet->dummy));
packet->len = cpu_to_le32(orig_len);
@@ -308,21 +315,12 @@ encapsulate:
if (skb->len < full_len) /* Pad */
skb_put(skb, full_len - skb->len);
- /* The VL600 wants IPv6 packets to have an IPv4 ethertype
- * Check if this is an IPv6 packet, and set the ethertype
- * to 0x800
- */
- if ((skb->data[sizeof(struct vl600_pkt_hdr *) + 0x22] & 0xf0) == 0x60) {
- skb->data[sizeof(struct vl600_pkt_hdr *) + 0x20] = 0x08;
- skb->data[sizeof(struct vl600_pkt_hdr *) + 0x21] = 0;
- }
-
return skb;
}
static const struct driver_info vl600_info = {
.description = "LG VL600 modem",
- .flags = FLAG_ETHER | FLAG_RX_ASSEMBLE,
+ .flags = FLAG_RX_ASSEMBLE | FLAG_WWAN,
.bind = vl600_bind,
.unbind = vl600_unbind,
.status = usbnet_cdc_status,
--
1.7.5.4
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] net/usb: Misc. fixes for the LG-VL600 LTE USB modem
2011-11-09 3:10 [PATCH] net/usb: Misc. fixes for the LG-VL600 LTE USB modem Mark Kamichoff
@ 2011-11-09 17:51 ` Dan Williams
2011-11-09 18:57 ` Mark Kamichoff
0 siblings, 1 reply; 4+ messages in thread
From: Dan Williams @ 2011-11-09 17:51 UTC (permalink / raw)
To: Mark Kamichoff; +Cc: oliver, gregkh, netdev, linux-kernel
On Tue, 2011-11-08 at 22:10 -0500, Mark Kamichoff wrote:
> Add checking for valid magic values (needed for stability in the event
> corrupted packets are received) and remove some other unneeded checks.
> Also, fix flagging device as WWAN (Bugzilla bug #39952).
>
> Signed-off-by: Mark Kamichoff <prox@prolixium.com>
> ---
> drivers/net/usb/cdc_ether.c | 2 +-
> drivers/net/usb/lg-vl600.c | 30 ++++++++++++++----------------
> 2 files changed, 15 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/net/usb/cdc_ether.c b/drivers/net/usb/cdc_ether.c
> index c924ea2..99ed6eb 100644
> --- a/drivers/net/usb/cdc_ether.c
> +++ b/drivers/net/usb/cdc_ether.c
> @@ -567,7 +567,7 @@ static const struct usb_device_id products [] = {
> {
> USB_DEVICE_AND_INTERFACE_INFO(0x1004, 0x61aa, USB_CLASS_COMM,
> USB_CDC_SUBCLASS_ETHERNET, USB_CDC_PROTO_NONE),
> - .driver_info = (unsigned long)&wwan_info,
> + .driver_info = 0,
> },
>
> /*
> diff --git a/drivers/net/usb/lg-vl600.c b/drivers/net/usb/lg-vl600.c
> index d43db32..b975a39 100644
> --- a/drivers/net/usb/lg-vl600.c
> +++ b/drivers/net/usb/lg-vl600.c
> @@ -144,10 +144,11 @@ static int vl600_rx_fixup(struct usbnet *dev, struct sk_buff *skb)
> }
>
> frame = (struct vl600_frame_hdr *) buf->data;
> - /* NOTE: Should check that frame->magic == 0x53544448?
> - * Otherwise if we receive garbage at the beginning of the frame
> - * we may end up allocating a huge buffer and saving all the
> - * future incoming data into it. */
> + /* Yes, check that frame->magic == 0x53544448 (or 0x44544d48),
> + * otherwise we may run out of memory w/a bad packet */
> + if (ntohl(frame->magic) != 0x53544448 &&
> + ntohl(frame->magic) != 0x44544d48)
> + goto error;
>
> if (buf->len < sizeof(*frame) ||
> buf->len != le32_to_cpup(&frame->len)) {
> @@ -209,8 +210,9 @@ static int vl600_rx_fixup(struct usbnet *dev, struct sk_buff *skb)
> * for IPv6 packets, and set the ethertype to IPv6
> * (0x86dd) so Linux can understand it.
> */
> - if ((buf->data[sizeof(*ethhdr)] & 0xf0) == 0x60)
> - ethhdr->h_proto = __constant_htons(ETH_P_IPV6);
> + if ((buf->data[sizeof(*ethhdr)] & 0xf0) == 0x60) {
> + ethhdr->h_proto = htons(ETH_P_IPV6);
> + }
This change seems somewhat gratuitous; what's the reason for (a) the
switch from __constant_htons() to plain htons() and (b) adding the {} ?
Dan
> }
>
> if (count) {
> @@ -296,6 +298,11 @@ encapsulate:
> * overwrite the remaining fields.
> */
> packet = (struct vl600_pkt_hdr *) skb->data;
> + /* The VL600 wants IPv6 packets to have an IPv4 ethertype
> + * Since this modem only supports IPv4 and IPv6, just set all
> + * frames to 0x0800 (ETH_P_IP)
> + */
> + packet->h_proto = htons(ETH_P_IP);
> memset(&packet->dummy, 0, sizeof(packet->dummy));
> packet->len = cpu_to_le32(orig_len);
>
> @@ -308,21 +315,12 @@ encapsulate:
> if (skb->len < full_len) /* Pad */
> skb_put(skb, full_len - skb->len);
>
> - /* The VL600 wants IPv6 packets to have an IPv4 ethertype
> - * Check if this is an IPv6 packet, and set the ethertype
> - * to 0x800
> - */
> - if ((skb->data[sizeof(struct vl600_pkt_hdr *) + 0x22] & 0xf0) == 0x60) {
> - skb->data[sizeof(struct vl600_pkt_hdr *) + 0x20] = 0x08;
> - skb->data[sizeof(struct vl600_pkt_hdr *) + 0x21] = 0;
> - }
> -
> return skb;
> }
>
> static const struct driver_info vl600_info = {
> .description = "LG VL600 modem",
> - .flags = FLAG_ETHER | FLAG_RX_ASSEMBLE,
> + .flags = FLAG_RX_ASSEMBLE | FLAG_WWAN,
> .bind = vl600_bind,
> .unbind = vl600_unbind,
> .status = usbnet_cdc_status,
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] net/usb: Misc. fixes for the LG-VL600 LTE USB modem
2011-11-09 17:51 ` Dan Williams
@ 2011-11-09 18:57 ` Mark Kamichoff
2011-11-09 21:06 ` David Miller
0 siblings, 1 reply; 4+ messages in thread
From: Mark Kamichoff @ 2011-11-09 18:57 UTC (permalink / raw)
To: Dan Williams; +Cc: oliver, gregkh, netdev, linux-kernel
On Wed, Nov 09, 2011 at 11:51:44AM -0600, Dan Williams wrote:
> On Tue, 2011-11-08 at 22:10 -0500, Mark Kamichoff wrote:
> > Add checking for valid magic values (needed for stability in the event
> > corrupted packets are received) and remove some other unneeded checks.
> > Also, fix flagging device as WWAN (Bugzilla bug #39952).
> >
> > Signed-off-by: Mark Kamichoff <prox@prolixium.com>
> > ---
> > drivers/net/usb/cdc_ether.c | 2 +-
> > drivers/net/usb/lg-vl600.c | 30 ++++++++++++++----------------
> > 2 files changed, 15 insertions(+), 17 deletions(-)
> >
> > diff --git a/drivers/net/usb/cdc_ether.c b/drivers/net/usb/cdc_ether.c
> > index c924ea2..99ed6eb 100644
> > --- a/drivers/net/usb/cdc_ether.c
> > +++ b/drivers/net/usb/cdc_ether.c
> > @@ -567,7 +567,7 @@ static const struct usb_device_id products [] = {
> > {
> > USB_DEVICE_AND_INTERFACE_INFO(0x1004, 0x61aa, USB_CLASS_COMM,
> > USB_CDC_SUBCLASS_ETHERNET, USB_CDC_PROTO_NONE),
> > - .driver_info = (unsigned long)&wwan_info,
> > + .driver_info = 0,
> > },
> >
> > /*
> > diff --git a/drivers/net/usb/lg-vl600.c b/drivers/net/usb/lg-vl600.c
> > index d43db32..b975a39 100644
> > --- a/drivers/net/usb/lg-vl600.c
> > +++ b/drivers/net/usb/lg-vl600.c
> > @@ -144,10 +144,11 @@ static int vl600_rx_fixup(struct usbnet *dev, struct sk_buff *skb)
> > }
> >
> > frame = (struct vl600_frame_hdr *) buf->data;
> > - /* NOTE: Should check that frame->magic == 0x53544448?
> > - * Otherwise if we receive garbage at the beginning of the frame
> > - * we may end up allocating a huge buffer and saving all the
> > - * future incoming data into it. */
> > + /* Yes, check that frame->magic == 0x53544448 (or 0x44544d48),
> > + * otherwise we may run out of memory w/a bad packet */
> > + if (ntohl(frame->magic) != 0x53544448 &&
> > + ntohl(frame->magic) != 0x44544d48)
> > + goto error;
> >
> > if (buf->len < sizeof(*frame) ||
> > buf->len != le32_to_cpup(&frame->len)) {
> > @@ -209,8 +210,9 @@ static int vl600_rx_fixup(struct usbnet *dev, struct sk_buff *skb)
> > * for IPv6 packets, and set the ethertype to IPv6
> > * (0x86dd) so Linux can understand it.
> > */
> > - if ((buf->data[sizeof(*ethhdr)] & 0xf0) == 0x60)
> > - ethhdr->h_proto = __constant_htons(ETH_P_IPV6);
> > + if ((buf->data[sizeof(*ethhdr)] & 0xf0) == 0x60) {
> > + ethhdr->h_proto = htons(ETH_P_IPV6);
> > + }
>
> This change seems somewhat gratuitous; what's the reason for (a) the
> switch from __constant_htons() to plain htons() and (b) adding the {} ?
>
> Dan
For (a), it's my understanding that __constant_htons() should be used
only for initializers and htons() used in other cases, since it handles
checking for constants. I suppose you're right and this is a little
gratuitous, but I wanted to keep things clean.
As far as (b), sorry! That's an error on my part. I must have been
practicing another coding style at the time. The braces certainly
shouldn't be there, let me know if I should resubmit.
- Mark
>
> > }
> >
> > if (count) {
> > @@ -296,6 +298,11 @@ encapsulate:
> > * overwrite the remaining fields.
> > */
> > packet = (struct vl600_pkt_hdr *) skb->data;
> > + /* The VL600 wants IPv6 packets to have an IPv4 ethertype
> > + * Since this modem only supports IPv4 and IPv6, just set all
> > + * frames to 0x0800 (ETH_P_IP)
> > + */
> > + packet->h_proto = htons(ETH_P_IP);
> > memset(&packet->dummy, 0, sizeof(packet->dummy));
> > packet->len = cpu_to_le32(orig_len);
> >
> > @@ -308,21 +315,12 @@ encapsulate:
> > if (skb->len < full_len) /* Pad */
> > skb_put(skb, full_len - skb->len);
> >
> > - /* The VL600 wants IPv6 packets to have an IPv4 ethertype
> > - * Check if this is an IPv6 packet, and set the ethertype
> > - * to 0x800
> > - */
> > - if ((skb->data[sizeof(struct vl600_pkt_hdr *) + 0x22] & 0xf0) == 0x60) {
> > - skb->data[sizeof(struct vl600_pkt_hdr *) + 0x20] = 0x08;
> > - skb->data[sizeof(struct vl600_pkt_hdr *) + 0x21] = 0;
> > - }
> > -
> > return skb;
> > }
> >
> > static const struct driver_info vl600_info = {
> > .description = "LG VL600 modem",
> > - .flags = FLAG_ETHER | FLAG_RX_ASSEMBLE,
> > + .flags = FLAG_RX_ASSEMBLE | FLAG_WWAN,
> > .bind = vl600_bind,
> > .unbind = vl600_unbind,
> > .status = usbnet_cdc_status,
>
>
>
--
Mark Kamichoff
prox@prolixium.com
http://www.prolixium.com/
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] net/usb: Misc. fixes for the LG-VL600 LTE USB modem
2011-11-09 18:57 ` Mark Kamichoff
@ 2011-11-09 21:06 ` David Miller
0 siblings, 0 replies; 4+ messages in thread
From: David Miller @ 2011-11-09 21:06 UTC (permalink / raw)
To: prox; +Cc: dcbw, oliver, gregkh, netdev, linux-kernel
From: Mark Kamichoff <prox@prolixium.com>
Date: Wed, 9 Nov 2011 13:57:14 -0500
> For (a), it's my understanding that __constant_htons() should be used
> only for initializers and htons() used in other cases, since it handles
> checking for constants. I suppose you're right and this is a little
> gratuitous, but I wanted to keep things clean.
>
> As far as (b), sorry! That's an error on my part. I must have been
> practicing another coding style at the time. The braces certainly
> shouldn't be there, let me know if I should resubmit.
Please get rid of the gratuitous htons() etc. changes and keep this
patch purely to the bug fixes and resubmit.
Thank you.
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2011-11-09 21:06 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-11-09 3:10 [PATCH] net/usb: Misc. fixes for the LG-VL600 LTE USB modem Mark Kamichoff
2011-11-09 17:51 ` Dan Williams
2011-11-09 18:57 ` Mark Kamichoff
2011-11-09 21:06 ` David Miller
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).