* [PATCH net-next 04/24] net: cdc_ncm: remove redundant endpoint pointers
From: Bjørn Mork @ 2013-11-01 10:16 UTC (permalink / raw)
To: netdev; +Cc: linux-usb, Alexey Orishko, Bjørn Mork
In-Reply-To: <1383301021-16613-1-git-send-email-bjorn@mork.no>
No need to duplicate stuff already in the common usbnet
struct. We still need to keep our special find_endpoints
function because we need explicit control over the selected
altsetting.
Cc: Alexey Orishko <alexey.orishko@gmail.com>
Signed-off-by: Bjørn Mork <bjorn@mork.no>
---
drivers/net/usb/cdc_ncm.c | 38 +++++++++++++++++++-------------------
include/linux/usb/cdc_ncm.h | 3 ---
2 files changed, 19 insertions(+), 22 deletions(-)
diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c
index bfab879..a989bd5 100644
--- a/drivers/net/usb/cdc_ncm.c
+++ b/drivers/net/usb/cdc_ncm.c
@@ -292,9 +292,9 @@ max_dgram_err:
}
static void
-cdc_ncm_find_endpoints(struct cdc_ncm_ctx *ctx, struct usb_interface *intf)
+cdc_ncm_find_endpoints(struct usbnet *dev, struct usb_interface *intf)
{
- struct usb_host_endpoint *e;
+ struct usb_host_endpoint *e, *in = NULL, *out = NULL;
u8 ep;
for (ep = 0; ep < intf->cur_altsetting->desc.bNumEndpoints; ep++) {
@@ -303,18 +303,18 @@ cdc_ncm_find_endpoints(struct cdc_ncm_ctx *ctx, struct usb_interface *intf)
switch (e->desc.bmAttributes & USB_ENDPOINT_XFERTYPE_MASK) {
case USB_ENDPOINT_XFER_INT:
if (usb_endpoint_dir_in(&e->desc)) {
- if (ctx->status_ep == NULL)
- ctx->status_ep = e;
+ if (!dev->status)
+ dev->status = e;
}
break;
case USB_ENDPOINT_XFER_BULK:
if (usb_endpoint_dir_in(&e->desc)) {
- if (ctx->in_ep == NULL)
- ctx->in_ep = e;
+ if (!in)
+ in = e;
} else {
- if (ctx->out_ep == NULL)
- ctx->out_ep = e;
+ if (!out)
+ out = e;
}
break;
@@ -322,6 +322,14 @@ cdc_ncm_find_endpoints(struct cdc_ncm_ctx *ctx, struct usb_interface *intf)
break;
}
}
+ if (in && !dev->in)
+ dev->in = usb_rcvbulkpipe(dev->udev,
+ in->desc.bEndpointAddress &
+ USB_ENDPOINT_NUMBER_MASK);
+ if (out && !dev->out)
+ dev->out = usb_sndbulkpipe(dev->udev,
+ out->desc.bEndpointAddress &
+ USB_ENDPOINT_NUMBER_MASK);
}
static void cdc_ncm_free(struct cdc_ncm_ctx *ctx)
@@ -477,11 +485,9 @@ advance:
if (temp)
goto error2;
- cdc_ncm_find_endpoints(ctx, ctx->data);
- cdc_ncm_find_endpoints(ctx, ctx->control);
-
- if ((ctx->in_ep == NULL) || (ctx->out_ep == NULL) ||
- (ctx->status_ep == NULL))
+ cdc_ncm_find_endpoints(dev, ctx->data);
+ cdc_ncm_find_endpoints(dev, ctx->control);
+ if (!dev->in || !dev->out || !dev->status)
goto error2;
dev->net->ethtool_ops = &cdc_ncm_ethtool_ops;
@@ -496,12 +502,6 @@ advance:
dev_info(&dev->udev->dev, "MAC-Address: %pM\n", dev->net->dev_addr);
}
-
- dev->in = usb_rcvbulkpipe(dev->udev,
- ctx->in_ep->desc.bEndpointAddress & USB_ENDPOINT_NUMBER_MASK);
- dev->out = usb_sndbulkpipe(dev->udev,
- ctx->out_ep->desc.bEndpointAddress & USB_ENDPOINT_NUMBER_MASK);
- dev->status = ctx->status_ep;
dev->rx_urb_size = ctx->rx_max;
/* cdc_ncm_setup will override dwNtbOutMaxSize if it is
diff --git a/include/linux/usb/cdc_ncm.h b/include/linux/usb/cdc_ncm.h
index c14e00f..36e1e15 100644
--- a/include/linux/usb/cdc_ncm.h
+++ b/include/linux/usb/cdc_ncm.h
@@ -100,9 +100,6 @@ struct cdc_ncm_ctx {
struct net_device *netdev;
struct usb_device *udev;
- struct usb_host_endpoint *in_ep;
- struct usb_host_endpoint *out_ep;
- struct usb_host_endpoint *status_ep;
struct usb_interface *control;
struct usb_interface *data;
--
1.7.10.4
^ permalink raw reply related
* [PATCH net-next 00/24] cdc_ncm: many small and mostly trivial fixes
From: Bjørn Mork @ 2013-11-01 10:16 UTC (permalink / raw)
To: netdev; +Cc: linux-usb, Alexey Orishko, Bjørn Mork
This series ended up longer than expected, and it is still not
complete. There is more to come when time allows...
Most changes are trivial. Notable non-trivial changes are
- removed filtering of identical speed notifications
- tx_max calulation is changed to count the pad byte if
necessary, and respect the device limit as an absolute
upper limit even if it is too low according to the spec
- remove the bug preventing SET_MAX_DATAGRAM_SIZE from having
any effect
- drop the pad-to-max if ZLPs are enabled
- the driver specific VERSION is dropped
- dev->hard_mtu is set to tx_max instead of max_datagram_size
causing usbnet to calculate the qlen based on the real max
size of tx skbs
This series has been tested, along with the previously posted
cdc_mbim series, on the NCM and MBIM devices I have:
- Ericsson F5521gw (NCM)
- Huawei E367 (MBIM)
- D-Link DWM-156 A7 (MBIM w/ too low dwNtb{In,Out}MaxSize bug)
- Sierra Wireless MC7710 (MBIM w/ ZLP and CDC Union bugs)
Apart from the D-Link modem dropping a lot less oversized
frames with the fix dedicated to it, there are no end user
noticable functional changes as a result of this series. But
all the non-trivial changes I listed above are of course
detectable by users looking at that specific area (except maybe
the removed speed notification, which requires a device sending
duplicates to be noticable - I don't have any such device).
Bjørn Mork (24):
net: cdc_ncm: simplify and optimize frame padding
net: cdc_ncm: add include protection to cdc_ncm.h
net: cdc_ncm: remove redundant "intf" field
net: cdc_ncm: remove redundant endpoint pointers
net: cdc_ncm: remove redundant netdev field
net: cdc_ncm: remove unused udev field
net: cdc_ncm: remove tx_speed and rx_speed fields
net: cdc_ncm: remove ncm_parm field
net: cdc_ncm: fix SET_MAX_DATAGRAM_SIZE
net: cdc_ncm: remove descriptor pointers
net: cdc_ncm: only the control intf can be probed
net: cdc_ncm: no point in filling up the NTBs if we send ZLPs
net: cdc_ncm: remove probe and disconnect wrappers
net: cdc_ncm: remove ethtool ops
net: cdc_ncm: set correct dev->hard_mtu
net: cdc_ncm: log the length we warn about
net: cdc_ncm: use netif_* and dev_* instead of pr_*
net: cdc_ncm: log signatures in hex
net: cdc_ncm: endian convert constants instead of variables
net: cdc_ncm: drop "extern" from header declarations
net: cdc_ncm: refactoring cdc_ncm_setup
net: cdc_ncm: return proper error if setup fails
net: cdc_ncm: improve bind error debug messages
net: cdc_ncm: no not set tx_max higher than the device supports
drivers/net/usb/cdc_mbim.c | 2 +-
drivers/net/usb/cdc_ncm.c | 490 +++++++++++++++++++------------------------
include/linux/usb/cdc_ncm.h | 30 ++-
3 files changed, 233 insertions(+), 289 deletions(-)
--
1.7.10.4
^ permalink raw reply
* [PATCH net-next 11/24] net: cdc_ncm: only the control intf can be probed
From: Bjørn Mork @ 2013-11-01 10:16 UTC (permalink / raw)
To: netdev; +Cc: linux-usb, Alexey Orishko, Bjørn Mork
In-Reply-To: <1383301021-16613-1-git-send-email-bjorn@mork.no>
The probed interface must be the master/control interface of the
function. Make this explicit and simplify redundant tests.
Cc: Alexey Orishko <alexey.orishko@gmail.com>
Signed-off-by: Bjørn Mork <bjorn@mork.no>
---
drivers/net/usb/cdc_ncm.c | 14 ++++++++------
1 file changed, 8 insertions(+), 6 deletions(-)
diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c
index 435fcc7..5aa3e60 100644
--- a/drivers/net/usb/cdc_ncm.c
+++ b/drivers/net/usb/cdc_ncm.c
@@ -394,6 +394,9 @@ int cdc_ncm_bind_common(struct usbnet *dev, struct usb_interface *intf, u8 data_
/* store ctx pointer in device data field */
dev->data[0] = (unsigned long)ctx;
+ /* only the control interface can be successfully probed */
+ ctx->control = intf;
+
/* get some pointers */
driver = driver_of(intf);
buf = intf->cur_altsetting->extra;
@@ -411,9 +414,10 @@ int cdc_ncm_bind_common(struct usbnet *dev, struct usb_interface *intf, u8 data_
break;
union_desc = (const struct usb_cdc_union_desc *)buf;
-
- ctx->control = usb_ifnum_to_if(dev->udev,
- union_desc->bMasterInterface0);
+ /* the master must be the interface we are probing */
+ if (intf->cur_altsetting->desc.bInterfaceNumber !=
+ union_desc->bMasterInterface0)
+ goto error;
ctx->data = usb_ifnum_to_if(dev->udev,
union_desc->bSlaveInterface0);
break;
@@ -459,14 +463,12 @@ advance:
/* some buggy devices have an IAD but no CDC Union */
if (!union_desc && intf->intf_assoc && intf->intf_assoc->bInterfaceCount == 2) {
- ctx->control = intf;
ctx->data = usb_ifnum_to_if(dev->udev, intf->cur_altsetting->desc.bInterfaceNumber + 1);
dev_dbg(&intf->dev, "CDC Union missing - got slave from IAD\n");
}
/* check if we got everything */
- if ((ctx->control == NULL) || (ctx->data == NULL) ||
- ((!ctx->mbim_desc) && ((ctx->ether_desc == NULL) || (ctx->control != intf))))
+ if (!ctx->data || (!ctx->mbim_desc && !ctx->ether_desc))
goto error;
/* claim data interface, if different from control */
--
1.7.10.4
^ permalink raw reply related
* [PATCH net-next 21/24] net: cdc_ncm: refactoring cdc_ncm_setup
From: Bjørn Mork @ 2013-11-01 10:16 UTC (permalink / raw)
To: netdev; +Cc: linux-usb, Alexey Orishko, Bjørn Mork
In-Reply-To: <1383301021-16613-1-git-send-email-bjorn@mork.no>
Rewriting the "set max datagram" part of dc_ncm_setup to
separate the selection and validatation of the size from
the code which optionally informs the device of this
value. This ensures that we use the correct value
regardless of device support for the get and set commands.
Removing some of the many indent levels while doing this
to make the code more readable.
Cc: Alexey Orishko <alexey.orishko@gmail.com>
Signed-off-by: Bjørn Mork <bjorn@mork.no>
---
drivers/net/usb/cdc_ncm.c | 108 ++++++++++++++++++---------------------------
1 file changed, 44 insertions(+), 64 deletions(-)
diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c
index 83e6d5b..62dcb2e 100644
--- a/drivers/net/usb/cdc_ncm.c
+++ b/drivers/net/usb/cdc_ncm.c
@@ -76,8 +76,7 @@ static u8 cdc_ncm_setup(struct usbnet *dev)
int err;
int eth_hlen;
u16 ntb_fmt_supported;
- u32 min_dgram_size;
- u32 min_hdr_size;
+ __le16 max_datagram_size;
iface_no = ctx->control->cur_altsetting->desc.bInterfaceNumber;
@@ -101,20 +100,29 @@ static u8 cdc_ncm_setup(struct usbnet *dev)
ctx->tx_max_datagrams = le16_to_cpu(ncm_parm.wNtbOutMaxDatagrams);
ntb_fmt_supported = le16_to_cpu(ncm_parm.bmNtbFormatsSupported);
- eth_hlen = ETH_HLEN;
- min_dgram_size = CDC_NCM_MIN_DATAGRAM_SIZE;
- min_hdr_size = CDC_NCM_MIN_HDR_SIZE;
- if (ctx->mbim_desc != NULL) {
- flags = ctx->mbim_desc->bmNetworkCapabilities;
+ /* there are some minor differences in NCM and MBIM defaults */
+ if (cdc_ncm_comm_intf_is_mbim(ctx->control->cur_altsetting)) {
+ if (!ctx->mbim_desc)
+ return -EINVAL;
eth_hlen = 0;
- min_dgram_size = CDC_MBIM_MIN_DATAGRAM_SIZE;
- min_hdr_size = 0;
- } else if (ctx->func_desc != NULL) {
- flags = ctx->func_desc->bmNetworkCapabilities;
+ flags = ctx->mbim_desc->bmNetworkCapabilities;
+ ctx->max_datagram_size = le16_to_cpu(ctx->mbim_desc->wMaxSegmentSize);
+ if (ctx->max_datagram_size < CDC_MBIM_MIN_DATAGRAM_SIZE)
+ ctx->max_datagram_size = CDC_MBIM_MIN_DATAGRAM_SIZE;
} else {
- flags = 0;
+ if (!ctx->func_desc)
+ return -EINVAL;
+ eth_hlen = ETH_HLEN;
+ flags = ctx->func_desc->bmNetworkCapabilities;
+ ctx->max_datagram_size = le16_to_cpu(ctx->ether_desc->wMaxSegmentSize);
+ if (ctx->max_datagram_size < CDC_NCM_MIN_DATAGRAM_SIZE)
+ ctx->max_datagram_size = CDC_NCM_MIN_DATAGRAM_SIZE;
}
+ /* common absolute max for NCM and MBIM */
+ if (ctx->max_datagram_size > CDC_NCM_MAX_DATAGRAM_SIZE)
+ ctx->max_datagram_size = CDC_NCM_MAX_DATAGRAM_SIZE;
+
dev_dbg(&dev->intf->dev,
"dwNtbInMaxSize=%u dwNtbOutMaxSize=%u wNdpOutPayloadRemainder=%u wNdpOutDivisor=%u wNdpOutAlignment=%u wNtbOutMaxDatagrams=%u flags=0x%x\n",
ctx->rx_max, ctx->tx_max, ctx->tx_remainder, ctx->tx_modulus,
@@ -151,8 +159,7 @@ static u8 cdc_ncm_setup(struct usbnet *dev)
}
/* verify maximum size of transmitted NTB in bytes */
- if ((ctx->tx_max <
- (min_hdr_size + min_dgram_size)) ||
+ if ((ctx->tx_max < (CDC_NCM_MIN_HDR_SIZE + ctx->max_datagram_size)) ||
(ctx->tx_max > CDC_NCM_NTB_MAX_SIZE_TX)) {
dev_dbg(&dev->intf->dev, "Using default maximum transmit length=%d\n",
CDC_NCM_NTB_MAX_SIZE_TX);
@@ -229,60 +236,33 @@ static u8 cdc_ncm_setup(struct usbnet *dev)
dev_dbg(&dev->intf->dev, "Setting NTB format to 16-bit failed\n");
}
- ctx->max_datagram_size = min_dgram_size;
+ /* inform the device about the selected Max Datagram Size */
+ if (!(flags & USB_CDC_NCM_NCAP_MAX_DATAGRAM_SIZE))
+ goto out;
- /* set Max Datagram Size (MTU) */
- if (flags & USB_CDC_NCM_NCAP_MAX_DATAGRAM_SIZE) {
- __le16 max_datagram_size;
- u16 eth_max_sz;
- if (ctx->ether_desc != NULL)
- eth_max_sz = le16_to_cpu(ctx->ether_desc->wMaxSegmentSize);
- else if (ctx->mbim_desc != NULL)
- eth_max_sz = le16_to_cpu(ctx->mbim_desc->wMaxSegmentSize);
- else
- goto max_dgram_err;
-
- err = usbnet_read_cmd(dev, USB_CDC_GET_MAX_DATAGRAM_SIZE,
- USB_TYPE_CLASS | USB_DIR_IN
- | USB_RECIP_INTERFACE,
- 0, iface_no, &max_datagram_size, 2);
- if (err < 0) {
- dev_dbg(&dev->intf->dev, "GET_MAX_DATAGRAM_SIZE failed, use size=%u\n",
- min_dgram_size);
- } else {
- ctx->max_datagram_size =
- le16_to_cpu(max_datagram_size);
- /* Check Eth descriptor value */
- if (ctx->max_datagram_size > eth_max_sz)
- ctx->max_datagram_size = eth_max_sz;
-
- if (ctx->max_datagram_size > CDC_NCM_MAX_DATAGRAM_SIZE)
- ctx->max_datagram_size = CDC_NCM_MAX_DATAGRAM_SIZE;
-
- if (ctx->max_datagram_size < min_dgram_size)
- ctx->max_datagram_size = min_dgram_size;
-
- /* if value changed, update device */
- if (ctx->max_datagram_size !=
- le16_to_cpu(max_datagram_size)) {
- max_datagram_size = cpu_to_le16(ctx->max_datagram_size);
- err = usbnet_write_cmd(dev,
- USB_CDC_SET_MAX_DATAGRAM_SIZE,
- USB_TYPE_CLASS | USB_DIR_OUT
- | USB_RECIP_INTERFACE,
- 0,
- iface_no, &max_datagram_size,
- 2);
- if (err < 0)
- dev_dbg(&dev->intf->dev, "SET_MAX_DGRAM_SIZE failed\n");
- }
- }
+ /* read current mtu value from device */
+ err = usbnet_read_cmd(dev, USB_CDC_GET_MAX_DATAGRAM_SIZE,
+ USB_TYPE_CLASS | USB_DIR_IN | USB_RECIP_INTERFACE,
+ 0, iface_no, &max_datagram_size, 2);
+ if (err < 0) {
+ dev_dbg(&dev->intf->dev, "GET_MAX_DATAGRAM_SIZE failed\n");
+ goto out;
}
-max_dgram_err:
- if (dev->net->mtu != (ctx->max_datagram_size - eth_hlen))
- dev->net->mtu = ctx->max_datagram_size - eth_hlen;
+ if (le16_to_cpu(max_datagram_size) == ctx->max_datagram_size)
+ goto out;
+
+ max_datagram_size = cpu_to_le16(ctx->max_datagram_size);
+ err = usbnet_write_cmd(dev, USB_CDC_SET_MAX_DATAGRAM_SIZE,
+ USB_TYPE_CLASS | USB_DIR_OUT | USB_RECIP_INTERFACE,
+ 0, iface_no, &max_datagram_size, 2);
+ if (err < 0)
+ dev_dbg(&dev->intf->dev, "SET_MAX_DATAGRAM_SIZE failed\n");
+out:
+ /* set MTU to max supported by the device if necessary */
+ if (dev->net->mtu > ctx->max_datagram_size - eth_hlen)
+ dev->net->mtu = ctx->max_datagram_size - eth_hlen;
return 0;
}
--
1.7.10.4
^ permalink raw reply related
* [PATCH net-next 01/24] net: cdc_ncm: simplify and optimize frame padding
From: Bjørn Mork @ 2013-11-01 10:16 UTC (permalink / raw)
To: netdev; +Cc: linux-usb, Alexey Orishko, Bjørn Mork
In-Reply-To: <1383301021-16613-1-git-send-email-bjorn@mork.no>
We can avoid the costly division for the common case where
we pad the frame to tx_max size as long as we ensure that
tx_max is either the device specified dwNtbOutMaxSize or not
a multiplum of wMaxPacketSize.
Using the preconverted 'maxpacket' field avoids converting
wMaxPacketSize to CPU endianness for every transmitted frame
And since we only will hit the one byte padding rule for short
frames, we can drop testing the skb for tailroom.
The change means that tx_max now represents the real maximum
skb size, enabling us to allocate the correct size instead of
always making room for one extra byte.
Cc: Alexey Orishko <alexey.orishko@gmail.com>
Signed-off-by: Bjørn Mork <bjorn@mork.no>
---
drivers/net/usb/cdc_ncm.c | 31 +++++++++++++++++++------------
1 file changed, 19 insertions(+), 12 deletions(-)
diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c
index 43afde8..38566bf 100644
--- a/drivers/net/usb/cdc_ncm.c
+++ b/drivers/net/usb/cdc_ncm.c
@@ -506,6 +506,15 @@ advance:
dev->status = ctx->status_ep;
dev->rx_urb_size = ctx->rx_max;
+ /* cdc_ncm_setup will override dwNtbOutMaxSize if it is
+ * outside the sane range. Adding a pad byte here if necessary
+ * simplifies the handling in cdc_ncm_fill_tx_frame, making
+ * tx_max always represent the real skb max size.
+ */
+ if (ctx->tx_max != le32_to_cpu(ctx->ncm_parm.dwNtbOutMaxSize) &&
+ ctx->tx_max % usb_maxpacket(dev->udev, dev->out, 1) == 0)
+ ctx->tx_max++;
+
ctx->tx_speed = ctx->rx_speed = 0;
return 0;
@@ -664,6 +673,7 @@ static struct usb_cdc_ncm_ndp16 *cdc_ncm_ndp(struct cdc_ncm_ctx *ctx, struct sk_
struct sk_buff *
cdc_ncm_fill_tx_frame(struct cdc_ncm_ctx *ctx, struct sk_buff *skb, __le32 sign)
{
+ struct usbnet *dev = netdev_priv(ctx->netdev);
struct usb_cdc_ncm_nth16 *nth16;
struct usb_cdc_ncm_ndp16 *ndp16;
struct sk_buff *skb_out;
@@ -683,7 +693,7 @@ cdc_ncm_fill_tx_frame(struct cdc_ncm_ctx *ctx, struct sk_buff *skb, __le32 sign)
/* allocate a new OUT skb */
if (!skb_out) {
- skb_out = alloc_skb((ctx->tx_max + 1), GFP_ATOMIC);
+ skb_out = alloc_skb(ctx->tx_max, GFP_ATOMIC);
if (skb_out == NULL) {
if (skb != NULL) {
dev_kfree_skb_any(skb);
@@ -788,19 +798,16 @@ cdc_ncm_fill_tx_frame(struct cdc_ncm_ctx *ctx, struct sk_buff *skb, __le32 sign)
/* variables will be reset at next call */
}
- /*
- * If collected data size is less or equal CDC_NCM_MIN_TX_PKT bytes,
- * we send buffers as it is. If we get more data, it would be more
- * efficient for USB HS mobile device with DMA engine to receive a full
- * size NTB, than canceling DMA transfer and receiving a short packet.
+ /* If collected data size is less or equal CDC_NCM_MIN_TX_PKT
+ * bytes, we send buffers as it is. If we get more data, it
+ * would be more efficient for USB HS mobile device with DMA
+ * engine to receive a full size NTB, than canceling DMA
+ * transfer and receiving a short packet.
*/
if (skb_out->len > CDC_NCM_MIN_TX_PKT)
- /* final zero padding */
- memset(skb_put(skb_out, ctx->tx_max - skb_out->len), 0, ctx->tx_max - skb_out->len);
-
- /* do we need to prevent a ZLP? */
- if (((skb_out->len % le16_to_cpu(ctx->out_ep->desc.wMaxPacketSize)) == 0) &&
- (skb_out->len < le32_to_cpu(ctx->ncm_parm.dwNtbOutMaxSize)) && skb_tailroom(skb_out))
+ memset(skb_put(skb_out, ctx->tx_max - skb_out->len), 0,
+ ctx->tx_max - skb_out->len);
+ else if ((skb_out->len % dev->maxpacket) == 0)
*skb_put(skb_out, 1) = 0; /* force short packet */
/* set final frame length */
--
1.7.10.4
^ permalink raw reply related
* [PATCH net-next 07/24] net: cdc_ncm: remove tx_speed and rx_speed fields
From: Bjørn Mork @ 2013-11-01 10:16 UTC (permalink / raw)
To: netdev; +Cc: linux-usb, Alexey Orishko, Bjørn Mork
In-Reply-To: <1383301021-16613-1-git-send-email-bjorn@mork.no>
These fields are only used to prevent printing the same speeds
multiple times if we receive multiple identical speed notifications.
The value of these printk's is questionable, and even more so when
we filter out some of the notifications sent us by the firmware. If
we are going to print any of these, then we should print them all.
Removing little used fields is a bonus.
Cc: Alexey Orishko <alexey.orishko@gmail.com>
Signed-off-by: Bjørn Mork <bjorn@mork.no>
---
drivers/net/usb/cdc_ncm.c | 37 ++++++++++++++-----------------------
include/linux/usb/cdc_ncm.h | 2 --
2 files changed, 14 insertions(+), 25 deletions(-)
diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c
index 9cdd762..fc36a99 100644
--- a/drivers/net/usb/cdc_ncm.c
+++ b/drivers/net/usb/cdc_ncm.c
@@ -510,7 +510,6 @@ advance:
ctx->tx_max % usb_maxpacket(dev->udev, dev->out, 1) == 0)
ctx->tx_max++;
- ctx->tx_speed = ctx->rx_speed = 0;
return 0;
error2:
@@ -1048,7 +1047,6 @@ static void
cdc_ncm_speed_change(struct usbnet *dev,
struct usb_cdc_speed_change *data)
{
- struct cdc_ncm_ctx *ctx = (struct cdc_ncm_ctx *)dev->data[0];
uint32_t rx_speed = le32_to_cpu(data->DLBitRRate);
uint32_t tx_speed = le32_to_cpu(data->ULBitRate);
@@ -1056,25 +1054,20 @@ cdc_ncm_speed_change(struct usbnet *dev,
* Currently the USB-NET API does not support reporting the actual
* device speed. Do print it instead.
*/
- if ((tx_speed != ctx->tx_speed) || (rx_speed != ctx->rx_speed)) {
- ctx->tx_speed = tx_speed;
- ctx->rx_speed = rx_speed;
-
- if ((tx_speed > 1000000) && (rx_speed > 1000000)) {
- printk(KERN_INFO KBUILD_MODNAME
- ": %s: %u mbit/s downlink "
- "%u mbit/s uplink\n",
- dev->net->name,
- (unsigned int)(rx_speed / 1000000U),
- (unsigned int)(tx_speed / 1000000U));
- } else {
- printk(KERN_INFO KBUILD_MODNAME
- ": %s: %u kbit/s downlink "
- "%u kbit/s uplink\n",
- dev->net->name,
- (unsigned int)(rx_speed / 1000U),
- (unsigned int)(tx_speed / 1000U));
- }
+ if ((tx_speed > 1000000) && (rx_speed > 1000000)) {
+ printk(KERN_INFO KBUILD_MODNAME
+ ": %s: %u mbit/s downlink "
+ "%u mbit/s uplink\n",
+ dev->net->name,
+ (unsigned int)(rx_speed / 1000000U),
+ (unsigned int)(tx_speed / 1000000U));
+ } else {
+ printk(KERN_INFO KBUILD_MODNAME
+ ": %s: %u kbit/s downlink "
+ "%u kbit/s uplink\n",
+ dev->net->name,
+ (unsigned int)(rx_speed / 1000U),
+ (unsigned int)(tx_speed / 1000U));
}
}
@@ -1111,8 +1104,6 @@ static void cdc_ncm_status(struct usbnet *dev, struct urb *urb)
dev->net->name, ctx->connected ? "" : "dis");
usbnet_link_change(dev, ctx->connected, 0);
- if (!ctx->connected)
- ctx->tx_speed = ctx->rx_speed = 0;
break;
case USB_CDC_NOTIFY_SPEED_CHANGE:
diff --git a/include/linux/usb/cdc_ncm.h b/include/linux/usb/cdc_ncm.h
index 059dcc9..f14af3d 100644
--- a/include/linux/usb/cdc_ncm.h
+++ b/include/linux/usb/cdc_ncm.h
@@ -110,8 +110,6 @@ struct cdc_ncm_ctx {
u32 tx_timer_pending;
u32 tx_curr_frame_num;
- u32 rx_speed;
- u32 tx_speed;
u32 rx_max;
u32 tx_max;
u32 max_datagram_size;
--
1.7.10.4
^ permalink raw reply related
* [PATCH net-next 09/24] net: cdc_ncm: fix SET_MAX_DATAGRAM_SIZE
From: Bjørn Mork @ 2013-11-01 10:16 UTC (permalink / raw)
To: netdev; +Cc: linux-usb, Alexey Orishko, Bjørn Mork
In-Reply-To: <1383301021-16613-1-git-send-email-bjorn@mork.no>
We need to inform the device about the *new* value, not the
old one.
Cc: Alexey Orishko <alexey.orishko@gmail.com>
Signed-off-by: Bjørn Mork <bjorn@mork.no>
---
drivers/net/usb/cdc_ncm.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c
index 4de3a542..b8de8dd 100644
--- a/drivers/net/usb/cdc_ncm.c
+++ b/drivers/net/usb/cdc_ncm.c
@@ -280,6 +280,7 @@ static u8 cdc_ncm_setup(struct usbnet *dev)
/* if value changed, update device */
if (ctx->max_datagram_size !=
le16_to_cpu(max_datagram_size)) {
+ max_datagram_size = cpu_to_le16(ctx->max_datagram_size);
err = usbnet_write_cmd(dev,
USB_CDC_SET_MAX_DATAGRAM_SIZE,
USB_TYPE_CLASS | USB_DIR_OUT
--
1.7.10.4
^ permalink raw reply related
* [PATCH net-next 10/24] net: cdc_ncm: remove descriptor pointers
From: Bjørn Mork @ 2013-11-01 10:16 UTC (permalink / raw)
To: netdev; +Cc: linux-usb, Alexey Orishko, Bjørn Mork
In-Reply-To: <1383301021-16613-1-git-send-email-bjorn@mork.no>
header_desc was completely unused and union_desc was never used
outside cdc_ncm_bind_common.
Cc: Alexey Orishko <alexey.orishko@gmail.com>
Signed-off-by: Bjørn Mork <bjorn@mork.no>
---
drivers/net/usb/cdc_ncm.c | 12 ++++++------
include/linux/usb/cdc_ncm.h | 4 +---
2 files changed, 7 insertions(+), 9 deletions(-)
diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c
index b8de8dd..435fcc7 100644
--- a/drivers/net/usb/cdc_ncm.c
+++ b/drivers/net/usb/cdc_ncm.c
@@ -372,6 +372,7 @@ static const struct ethtool_ops cdc_ncm_ethtool_ops = {
int cdc_ncm_bind_common(struct usbnet *dev, struct usb_interface *intf, u8 data_altsetting)
{
+ const struct usb_cdc_union_desc *union_desc = NULL;
struct cdc_ncm_ctx *ctx;
struct usb_driver *driver;
u8 *buf;
@@ -406,16 +407,15 @@ int cdc_ncm_bind_common(struct usbnet *dev, struct usb_interface *intf, u8 data_
switch (buf[2]) {
case USB_CDC_UNION_TYPE:
- if (buf[0] < sizeof(*(ctx->union_desc)))
+ if (buf[0] < sizeof(*union_desc))
break;
- ctx->union_desc =
- (const struct usb_cdc_union_desc *)buf;
+ union_desc = (const struct usb_cdc_union_desc *)buf;
ctx->control = usb_ifnum_to_if(dev->udev,
- ctx->union_desc->bMasterInterface0);
+ union_desc->bMasterInterface0);
ctx->data = usb_ifnum_to_if(dev->udev,
- ctx->union_desc->bSlaveInterface0);
+ union_desc->bSlaveInterface0);
break;
case USB_CDC_ETHERNET_TYPE:
@@ -458,7 +458,7 @@ advance:
}
/* some buggy devices have an IAD but no CDC Union */
- if (!ctx->union_desc && intf->intf_assoc && intf->intf_assoc->bInterfaceCount == 2) {
+ if (!union_desc && intf->intf_assoc && intf->intf_assoc->bInterfaceCount == 2) {
ctx->control = intf;
ctx->data = usb_ifnum_to_if(dev->udev, intf->cur_altsetting->desc.bInterfaceNumber + 1);
dev_dbg(&intf->dev, "CDC Union missing - got slave from IAD\n");
diff --git a/include/linux/usb/cdc_ncm.h b/include/linux/usb/cdc_ncm.h
index 89b52a0..cad54ad 100644
--- a/include/linux/usb/cdc_ncm.h
+++ b/include/linux/usb/cdc_ncm.h
@@ -92,9 +92,7 @@ struct cdc_ncm_ctx {
struct tasklet_struct bh;
const struct usb_cdc_ncm_desc *func_desc;
- const struct usb_cdc_mbim_desc *mbim_desc;
- const struct usb_cdc_header_desc *header_desc;
- const struct usb_cdc_union_desc *union_desc;
+ const struct usb_cdc_mbim_desc *mbim_desc;
const struct usb_cdc_ether_desc *ether_desc;
struct usb_interface *control;
--
1.7.10.4
^ permalink raw reply related
* [PATCH net-next 03/24] net: cdc_ncm: remove redundant "intf" field
From: Bjørn Mork @ 2013-11-01 10:16 UTC (permalink / raw)
To: netdev; +Cc: linux-usb, Alexey Orishko, Bjørn Mork
In-Reply-To: <1383301021-16613-1-git-send-email-bjorn@mork.no>
This is always a duplicate of the "control" field. It causes
confusion wrt intf_data updates and cleanups.
Cc: Alexey Orishko <alexey.orishko@gmail.com>
Signed-off-by: Bjørn Mork <bjorn@mork.no>
---
drivers/net/usb/cdc_ncm.c | 4 +---
include/linux/usb/cdc_ncm.h | 1 -
2 files changed, 1 insertion(+), 4 deletions(-)
diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c
index 38566bf..bfab879 100644
--- a/drivers/net/usb/cdc_ncm.c
+++ b/drivers/net/usb/cdc_ncm.c
@@ -382,7 +382,6 @@ int cdc_ncm_bind_common(struct usbnet *dev, struct usb_interface *intf, u8 data_
len = intf->cur_altsetting->extralen;
ctx->udev = dev->udev;
- ctx->intf = intf;
/* parse through descriptors associated with control interface */
while ((len > 0) && (buf[0] > 2) && (buf[0] <= len)) {
@@ -489,7 +488,6 @@ advance:
usb_set_intfdata(ctx->data, dev);
usb_set_intfdata(ctx->control, dev);
- usb_set_intfdata(ctx->intf, dev);
if (ctx->ether_desc) {
temp = usbnet_get_ethernet_addr(dev, ctx->ether_desc->iMACAddress);
@@ -562,7 +560,7 @@ void cdc_ncm_unbind(struct usbnet *dev, struct usb_interface *intf)
ctx->control = NULL;
}
- usb_set_intfdata(ctx->intf, NULL);
+ usb_set_intfdata(intf, NULL);
cdc_ncm_free(ctx);
}
EXPORT_SYMBOL_GPL(cdc_ncm_unbind);
diff --git a/include/linux/usb/cdc_ncm.h b/include/linux/usb/cdc_ncm.h
index 89f0bbc..c14e00f 100644
--- a/include/linux/usb/cdc_ncm.h
+++ b/include/linux/usb/cdc_ncm.h
@@ -103,7 +103,6 @@ struct cdc_ncm_ctx {
struct usb_host_endpoint *in_ep;
struct usb_host_endpoint *out_ep;
struct usb_host_endpoint *status_ep;
- struct usb_interface *intf;
struct usb_interface *control;
struct usb_interface *data;
--
1.7.10.4
^ permalink raw reply related
* [PATCH net-next 23/24] net: cdc_ncm: improve bind error debug messages
From: Bjørn Mork @ 2013-11-01 10:17 UTC (permalink / raw)
To: netdev-u79uwXL29TY76Z2rM5mHXA
Cc: linux-usb-u79uwXL29TY76Z2rM5mHXA, Alexey Orishko, Bjørn Mork
In-Reply-To: <1383301021-16613-1-git-send-email-bjorn-yOkvZcmFvRU@public.gmane.org>
Make it a bit easier for users to figure out what goes
wrong when bind fails.
Cc: Alexey Orishko <alexey.orishko-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Signed-off-by: Bjørn Mork <bjorn-yOkvZcmFvRU@public.gmane.org>
---
drivers/net/usb/cdc_ncm.c | 36 ++++++++++++++++++++++++++----------
1 file changed, 26 insertions(+), 10 deletions(-)
diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c
index f168bc8..4531f38 100644
--- a/drivers/net/usb/cdc_ncm.c
+++ b/drivers/net/usb/cdc_ncm.c
@@ -371,8 +371,10 @@ int cdc_ncm_bind_common(struct usbnet *dev, struct usb_interface *intf, u8 data_
union_desc = (const struct usb_cdc_union_desc *)buf;
/* the master must be the interface we are probing */
if (intf->cur_altsetting->desc.bInterfaceNumber !=
- union_desc->bMasterInterface0)
+ union_desc->bMasterInterface0) {
+ dev_dbg(&intf->dev, "bogus CDC Union\n");
goto error;
+ }
ctx->data = usb_ifnum_to_if(dev->udev,
union_desc->bSlaveInterface0);
break;
@@ -416,45 +418,59 @@ advance:
}
/* check if we got everything */
- if (!ctx->data || (!ctx->mbim_desc && !ctx->ether_desc))
+ if (!ctx->data || (!ctx->mbim_desc && !ctx->ether_desc)) {
+ dev_dbg(&intf->dev, "CDC descriptors missing\n");
goto error;
+ }
/* claim data interface, if different from control */
if (ctx->data != ctx->control) {
temp = usb_driver_claim_interface(driver, ctx->data, dev);
- if (temp)
+ if (temp) {
+ dev_dbg(&intf->dev, "failed to claim data intf\n");
goto error;
+ }
}
iface_no = ctx->data->cur_altsetting->desc.bInterfaceNumber;
/* reset data interface */
temp = usb_set_interface(dev->udev, iface_no, 0);
- if (temp)
+ if (temp) {
+ dev_dbg(&intf->dev, "set interface failed\n");
goto error2;
+ }
/* configure data interface */
temp = usb_set_interface(dev->udev, iface_no, data_altsetting);
- if (temp)
+ if (temp) {
+ dev_dbg(&intf->dev, "set interface failed\n");
goto error2;
+ }
cdc_ncm_find_endpoints(dev, ctx->data);
cdc_ncm_find_endpoints(dev, ctx->control);
- if (!dev->in || !dev->out || !dev->status)
+ if (!dev->in || !dev->out || !dev->status) {
+ dev_dbg(&intf->dev, "failed to collect endpoints\n");
goto error2;
+ }
/* initialize data interface */
- if (cdc_ncm_setup(dev))
+ if (cdc_ncm_setup(dev)) {
+ dev_dbg(&intf->dev, "cdc_ncm_setup() failed\n");
goto error2;
+ }
usb_set_intfdata(ctx->data, dev);
usb_set_intfdata(ctx->control, dev);
if (ctx->ether_desc) {
temp = usbnet_get_ethernet_addr(dev, ctx->ether_desc->iMACAddress);
- if (temp)
+ if (temp) {
+ dev_dbg(&intf->dev, "failed to get mac address\n");
goto error2;
- dev_info(&dev->udev->dev, "MAC-Address: %pM\n", dev->net->dev_addr);
+ }
+ dev_info(&intf->dev, "MAC-Address: %pM\n", dev->net->dev_addr);
}
/* usbnet use these values for sizing tx/rx queues */
@@ -471,7 +487,7 @@ error2:
error:
cdc_ncm_free((struct cdc_ncm_ctx *)dev->data[0]);
dev->data[0] = 0;
- dev_info(&dev->udev->dev, "bind() failure\n");
+ dev_info(&intf->dev, "bind() failure\n");
return -ENODEV;
}
EXPORT_SYMBOL_GPL(cdc_ncm_bind_common);
--
1.7.10.4
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [PATCH net-next 20/24] net: cdc_ncm: drop "extern" from header declarations
From: Bjørn Mork @ 2013-11-01 10:16 UTC (permalink / raw)
To: netdev-u79uwXL29TY76Z2rM5mHXA
Cc: linux-usb-u79uwXL29TY76Z2rM5mHXA, Alexey Orishko, Bjørn Mork
In-Reply-To: <1383301021-16613-1-git-send-email-bjorn-yOkvZcmFvRU@public.gmane.org>
Cc: Alexey Orishko <alexey.orishko-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Signed-off-by: Bjørn Mork <bjorn-yOkvZcmFvRU@public.gmane.org>
---
include/linux/usb/cdc_ncm.h | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/include/linux/usb/cdc_ncm.h b/include/linux/usb/cdc_ncm.h
index cad54ad..2300f74 100644
--- a/include/linux/usb/cdc_ncm.h
+++ b/include/linux/usb/cdc_ncm.h
@@ -119,11 +119,11 @@ struct cdc_ncm_ctx {
u16 connected;
};
-extern u8 cdc_ncm_select_altsetting(struct usbnet *dev, struct usb_interface *intf);
-extern int cdc_ncm_bind_common(struct usbnet *dev, struct usb_interface *intf, u8 data_altsetting);
-extern void cdc_ncm_unbind(struct usbnet *dev, struct usb_interface *intf);
-extern struct sk_buff *cdc_ncm_fill_tx_frame(struct usbnet *dev, struct sk_buff *skb, __le32 sign);
-extern int cdc_ncm_rx_verify_nth16(struct cdc_ncm_ctx *ctx, struct sk_buff *skb_in);
-extern int cdc_ncm_rx_verify_ndp16(struct sk_buff *skb_in, int ndpoffset);
+u8 cdc_ncm_select_altsetting(struct usbnet *dev, struct usb_interface *intf);
+int cdc_ncm_bind_common(struct usbnet *dev, struct usb_interface *intf, u8 data_altsetting);
+void cdc_ncm_unbind(struct usbnet *dev, struct usb_interface *intf);
+struct sk_buff *cdc_ncm_fill_tx_frame(struct usbnet *dev, struct sk_buff *skb, __le32 sign);
+int cdc_ncm_rx_verify_nth16(struct cdc_ncm_ctx *ctx, struct sk_buff *skb_in);
+int cdc_ncm_rx_verify_ndp16(struct sk_buff *skb_in, int ndpoffset);
#endif /* __LINUX_USB_CDC_NCM_H */
--
1.7.10.4
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [PATCH net-next 19/24] net: cdc_ncm: endian convert constants instead of variables
From: Bjørn Mork @ 2013-11-01 10:16 UTC (permalink / raw)
To: netdev-u79uwXL29TY76Z2rM5mHXA
Cc: linux-usb-u79uwXL29TY76Z2rM5mHXA, Alexey Orishko, Bjørn Mork
In-Reply-To: <1383301021-16613-1-git-send-email-bjorn-yOkvZcmFvRU@public.gmane.org>
Converting the constants used in these comparisons at build
time instead of converting the variables for every received
frame at run time.
Cc: Alexey Orishko <alexey.orishko-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Signed-off-by: Bjørn Mork <bjorn-yOkvZcmFvRU@public.gmane.org>
---
drivers/net/usb/cdc_ncm.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c
index d49e4c5..83e6d5b 100644
--- a/drivers/net/usb/cdc_ncm.c
+++ b/drivers/net/usb/cdc_ncm.c
@@ -882,7 +882,7 @@ int cdc_ncm_rx_verify_nth16(struct cdc_ncm_ctx *ctx, struct sk_buff *skb_in)
nth16 = (struct usb_cdc_ncm_nth16 *)skb_in->data;
- if (le32_to_cpu(nth16->dwSignature) != USB_CDC_NCM_NTH16_SIGN) {
+ if (nth16->dwSignature != cpu_to_le32(USB_CDC_NCM_NTH16_SIGN)) {
netif_dbg(dev, rx_err, dev->net,
"invalid NTH16 signature <%#010x>\n",
le32_to_cpu(nth16->dwSignature));
@@ -972,7 +972,7 @@ next_ndp:
ndp16 = (struct usb_cdc_ncm_ndp16 *)(skb_in->data + ndpoffset);
- if (le32_to_cpu(ndp16->dwSignature) != USB_CDC_NCM_NDP16_NOCRC_SIGN) {
+ if (ndp16->dwSignature != cpu_to_le32(USB_CDC_NCM_NDP16_NOCRC_SIGN)) {
netif_dbg(dev, rx_err, dev->net,
"invalid DPT16 signature <%#010x>\n",
le32_to_cpu(ndp16->dwSignature));
--
1.7.10.4
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [PATCH net-next 18/24] net: cdc_ncm: log signatures in hex
From: Bjørn Mork @ 2013-11-01 10:16 UTC (permalink / raw)
To: netdev-u79uwXL29TY76Z2rM5mHXA
Cc: linux-usb-u79uwXL29TY76Z2rM5mHXA, Alexey Orishko, Bjørn Mork
In-Reply-To: <1383301021-16613-1-git-send-email-bjorn-yOkvZcmFvRU@public.gmane.org>
These signatures are well known bit patterns, mostly made up
of ascii characters. Mentally parsing works best if they
are printed in hex.
Cc: Alexey Orishko <alexey.orishko-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Signed-off-by: Bjørn Mork <bjorn-yOkvZcmFvRU@public.gmane.org>
---
drivers/net/usb/cdc_ncm.c | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c
index 73faf05..d49e4c5 100644
--- a/drivers/net/usb/cdc_ncm.c
+++ b/drivers/net/usb/cdc_ncm.c
@@ -883,8 +883,9 @@ int cdc_ncm_rx_verify_nth16(struct cdc_ncm_ctx *ctx, struct sk_buff *skb_in)
nth16 = (struct usb_cdc_ncm_nth16 *)skb_in->data;
if (le32_to_cpu(nth16->dwSignature) != USB_CDC_NCM_NTH16_SIGN) {
- pr_debug("invalid NTH16 signature <%u>\n",
- le32_to_cpu(nth16->dwSignature));
+ netif_dbg(dev, rx_err, dev->net,
+ "invalid NTH16 signature <%#010x>\n",
+ le32_to_cpu(nth16->dwSignature));
goto error;
}
@@ -972,8 +973,9 @@ next_ndp:
ndp16 = (struct usb_cdc_ncm_ndp16 *)(skb_in->data + ndpoffset);
if (le32_to_cpu(ndp16->dwSignature) != USB_CDC_NCM_NDP16_NOCRC_SIGN) {
- pr_debug("invalid DPT16 signature <%u>\n",
- le32_to_cpu(ndp16->dwSignature));
+ netif_dbg(dev, rx_err, dev->net,
+ "invalid DPT16 signature <%#010x>\n",
+ le32_to_cpu(ndp16->dwSignature));
goto err_ndp;
}
dpe16 = ndp16->dpe16;
--
1.7.10.4
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [PATCH net-next 16/24] net: cdc_ncm: log the length we warn about
From: Bjørn Mork @ 2013-11-01 10:16 UTC (permalink / raw)
To: netdev-u79uwXL29TY76Z2rM5mHXA
Cc: linux-usb-u79uwXL29TY76Z2rM5mHXA, Alexey Orishko, Bjørn Mork
In-Reply-To: <1383301021-16613-1-git-send-email-bjorn-yOkvZcmFvRU@public.gmane.org>
Fix cut'n'paste typo. Log the bogus length and not the
irrelevant signature.
Cc: Alexey Orishko <alexey.orishko-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Signed-off-by: Bjørn Mork <bjorn-yOkvZcmFvRU@public.gmane.org>
---
drivers/net/usb/cdc_ncm.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c
index c40f742..b5fdf8f 100644
--- a/drivers/net/usb/cdc_ncm.c
+++ b/drivers/net/usb/cdc_ncm.c
@@ -923,7 +923,7 @@ int cdc_ncm_rx_verify_ndp16(struct sk_buff *skb_in, int ndpoffset)
if (le16_to_cpu(ndp16->wLength) < USB_CDC_NCM_NDP16_LENGTH_MIN) {
pr_debug("invalid DPT16 length <%u>\n",
- le32_to_cpu(ndp16->dwSignature));
+ le16_to_cpu(ndp16->wLength));
goto error;
}
--
1.7.10.4
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [PATCH net-next 14/24] net: cdc_ncm: remove ethtool ops
From: Bjørn Mork @ 2013-11-01 10:16 UTC (permalink / raw)
To: netdev-u79uwXL29TY76Z2rM5mHXA
Cc: linux-usb-u79uwXL29TY76Z2rM5mHXA, Alexey Orishko, Bjørn Mork
In-Reply-To: <1383301021-16613-1-git-send-email-bjorn-yOkvZcmFvRU@public.gmane.org>
No need to keep this code duplicated from usbnet.
Cc: Alexey Orishko <alexey.orishko-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Signed-off-by: Bjørn Mork <bjorn-yOkvZcmFvRU@public.gmane.org>
---
drivers/net/usb/cdc_ncm.c | 26 --------------------------
1 file changed, 26 deletions(-)
diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c
index c90d843..8fc1a06 100644
--- a/drivers/net/usb/cdc_ncm.c
+++ b/drivers/net/usb/cdc_ncm.c
@@ -53,8 +53,6 @@
#include <linux/usb/cdc.h>
#include <linux/usb/cdc_ncm.h>
-#define DRIVER_VERSION "14-Mar-2012"
-
#if IS_ENABLED(CONFIG_USB_NET_CDC_MBIM)
static bool prefer_mbim = true;
#else
@@ -68,18 +66,6 @@ static void cdc_ncm_tx_timeout_start(struct cdc_ncm_ctx *ctx);
static enum hrtimer_restart cdc_ncm_tx_timer_cb(struct hrtimer *hr_timer);
static struct usb_driver cdc_ncm_driver;
-static void
-cdc_ncm_get_drvinfo(struct net_device *net, struct ethtool_drvinfo *info)
-{
- struct usbnet *dev = netdev_priv(net);
-
- strlcpy(info->driver, dev->driver_name, sizeof(info->driver));
- strlcpy(info->version, DRIVER_VERSION, sizeof(info->version));
- strlcpy(info->fw_version, dev->driver_info->description,
- sizeof(info->fw_version));
- usb_make_path(dev->udev, info->bus_info, sizeof(info->bus_info));
-}
-
static u8 cdc_ncm_setup(struct usbnet *dev)
{
struct cdc_ncm_ctx *ctx = (struct cdc_ncm_ctx *)dev->data[0];
@@ -360,16 +346,6 @@ static void cdc_ncm_free(struct cdc_ncm_ctx *ctx)
kfree(ctx);
}
-static const struct ethtool_ops cdc_ncm_ethtool_ops = {
- .get_drvinfo = cdc_ncm_get_drvinfo,
- .get_link = usbnet_get_link,
- .get_msglevel = usbnet_get_msglevel,
- .set_msglevel = usbnet_set_msglevel,
- .get_settings = usbnet_get_settings,
- .set_settings = usbnet_set_settings,
- .nway_reset = usbnet_nway_reset,
-};
-
int cdc_ncm_bind_common(struct usbnet *dev, struct usb_interface *intf, u8 data_altsetting)
{
const struct usb_cdc_union_desc *union_desc = NULL;
@@ -499,8 +475,6 @@ advance:
if (cdc_ncm_setup(dev))
goto error2;
- dev->net->ethtool_ops = &cdc_ncm_ethtool_ops;
^ permalink raw reply related
* [PATCH net-next 06/24] net: cdc_ncm: remove unused udev field
From: Bjørn Mork @ 2013-11-01 10:16 UTC (permalink / raw)
To: netdev-u79uwXL29TY76Z2rM5mHXA
Cc: linux-usb-u79uwXL29TY76Z2rM5mHXA, Alexey Orishko, Bjørn Mork
In-Reply-To: <1383301021-16613-1-git-send-email-bjorn-yOkvZcmFvRU@public.gmane.org>
We already use the usbnet udev field everywhere this could have
been used.
Cc: Alexey Orishko <alexey.orishko-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Signed-off-by: Bjørn Mork <bjorn-yOkvZcmFvRU@public.gmane.org>
---
drivers/net/usb/cdc_ncm.c | 2 --
include/linux/usb/cdc_ncm.h | 1 -
2 files changed, 3 deletions(-)
diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c
index e39e767..9cdd762 100644
--- a/drivers/net/usb/cdc_ncm.c
+++ b/drivers/net/usb/cdc_ncm.c
@@ -388,8 +388,6 @@ int cdc_ncm_bind_common(struct usbnet *dev, struct usb_interface *intf, u8 data_
buf = intf->cur_altsetting->extra;
len = intf->cur_altsetting->extralen;
- ctx->udev = dev->udev;
^ permalink raw reply related
* [PATCH net-next 05/24] net: cdc_ncm: remove redundant netdev field
From: Bjørn Mork @ 2013-11-01 10:16 UTC (permalink / raw)
To: netdev-u79uwXL29TY76Z2rM5mHXA
Cc: linux-usb-u79uwXL29TY76Z2rM5mHXA, Alexey Orishko, Bjørn Mork,
Greg Suarez
In-Reply-To: <1383301021-16613-1-git-send-email-bjorn-yOkvZcmFvRU@public.gmane.org>
Too many pointers back and forth are likely to confuse developers,
creating subtle bugs whenever we forget to syncronize them all.
As a usbnet driver, we should stick with the standard struct
usbnet fields as much as possible. The netdevice is one such
field.
Cc: Greg Suarez <gsuarez-AKjrjAf1O7qe8kRwQpwjMg@public.gmane.org>
Cc: Alexey Orishko <alexey.orishko-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Signed-off-by: Bjørn Mork <bjorn-yOkvZcmFvRU@public.gmane.org>
---
drivers/net/usb/cdc_mbim.c | 2 +-
drivers/net/usb/cdc_ncm.c | 73 ++++++++++++++++++++++---------------------
include/linux/usb/cdc_ncm.h | 3 +-
3 files changed, 39 insertions(+), 39 deletions(-)
diff --git a/drivers/net/usb/cdc_mbim.c b/drivers/net/usb/cdc_mbim.c
index 253472a..af76aaf 100644
--- a/drivers/net/usb/cdc_mbim.c
+++ b/drivers/net/usb/cdc_mbim.c
@@ -175,7 +175,7 @@ static struct sk_buff *cdc_mbim_tx_fixup(struct usbnet *dev, struct sk_buff *skb
}
spin_lock_bh(&ctx->mtx);
- skb_out = cdc_ncm_fill_tx_frame(ctx, skb, sign);
+ skb_out = cdc_ncm_fill_tx_frame(dev, skb, sign);
spin_unlock_bh(&ctx->mtx);
return skb_out;
diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c
index a989bd5..e39e767 100644
--- a/drivers/net/usb/cdc_ncm.c
+++ b/drivers/net/usb/cdc_ncm.c
@@ -80,8 +80,9 @@ cdc_ncm_get_drvinfo(struct net_device *net, struct ethtool_drvinfo *info)
usb_make_path(dev->udev, info->bus_info, sizeof(info->bus_info));
}
-static u8 cdc_ncm_setup(struct cdc_ncm_ctx *ctx)
+static u8 cdc_ncm_setup(struct usbnet *dev)
{
+ struct cdc_ncm_ctx *ctx = (struct cdc_ncm_ctx *)dev->data[0];
u32 val;
u8 flags;
u8 iface_no;
@@ -90,7 +91,6 @@ static u8 cdc_ncm_setup(struct cdc_ncm_ctx *ctx)
u16 ntb_fmt_supported;
u32 min_dgram_size;
u32 min_hdr_size;
- struct usbnet *dev = netdev_priv(ctx->netdev);
iface_no = ctx->control->cur_altsetting->desc.bInterfaceNumber;
@@ -285,8 +285,8 @@ static u8 cdc_ncm_setup(struct cdc_ncm_ctx *ctx)
}
max_dgram_err:
- if (ctx->netdev->mtu != (ctx->max_datagram_size - eth_hlen))
- ctx->netdev->mtu = ctx->max_datagram_size - eth_hlen;
+ if (dev->net->mtu != (ctx->max_datagram_size - eth_hlen))
+ dev->net->mtu = ctx->max_datagram_size - eth_hlen;
return 0;
}
@@ -375,11 +375,10 @@ int cdc_ncm_bind_common(struct usbnet *dev, struct usb_interface *intf, u8 data_
hrtimer_init(&ctx->tx_timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
ctx->tx_timer.function = &cdc_ncm_tx_timer_cb;
- ctx->bh.data = (unsigned long)ctx;
+ ctx->bh.data = (unsigned long)dev;
ctx->bh.func = cdc_ncm_txpath_bh;
atomic_set(&ctx->stop, 0);
spin_lock_init(&ctx->mtx);
- ctx->netdev = dev->net;
/* store ctx pointer in device data field */
dev->data[0] = (unsigned long)ctx;
@@ -477,7 +476,7 @@ advance:
goto error2;
/* initialize data interface */
- if (cdc_ncm_setup(ctx))
+ if (cdc_ncm_setup(dev))
goto error2;
/* configure data interface */
@@ -669,9 +668,9 @@ static struct usb_cdc_ncm_ndp16 *cdc_ncm_ndp(struct cdc_ncm_ctx *ctx, struct sk_
}
struct sk_buff *
-cdc_ncm_fill_tx_frame(struct cdc_ncm_ctx *ctx, struct sk_buff *skb, __le32 sign)
+cdc_ncm_fill_tx_frame(struct usbnet *dev, struct sk_buff *skb, __le32 sign)
{
- struct usbnet *dev = netdev_priv(ctx->netdev);
+ struct cdc_ncm_ctx *ctx = (struct cdc_ncm_ctx *)dev->data[0];
struct usb_cdc_ncm_nth16 *nth16;
struct usb_cdc_ncm_ndp16 *ndp16;
struct sk_buff *skb_out;
@@ -695,7 +694,7 @@ cdc_ncm_fill_tx_frame(struct cdc_ncm_ctx *ctx, struct sk_buff *skb, __le32 sign)
if (skb_out == NULL) {
if (skb != NULL) {
dev_kfree_skb_any(skb);
- ctx->netdev->stats.tx_dropped++;
+ dev->net->stats.tx_dropped++;
}
goto exit_no_skb;
}
@@ -733,12 +732,12 @@ cdc_ncm_fill_tx_frame(struct cdc_ncm_ctx *ctx, struct sk_buff *skb, __le32 sign)
/* won't fit, MTU problem? */
dev_kfree_skb_any(skb);
skb = NULL;
- ctx->netdev->stats.tx_dropped++;
+ dev->net->stats.tx_dropped++;
} else {
/* no room for skb - store for later */
if (ctx->tx_rem_skb != NULL) {
dev_kfree_skb_any(ctx->tx_rem_skb);
- ctx->netdev->stats.tx_dropped++;
+ dev->net->stats.tx_dropped++;
}
ctx->tx_rem_skb = skb;
ctx->tx_rem_sign = sign;
@@ -771,7 +770,7 @@ cdc_ncm_fill_tx_frame(struct cdc_ncm_ctx *ctx, struct sk_buff *skb, __le32 sign)
if (skb != NULL) {
dev_kfree_skb_any(skb);
skb = NULL;
- ctx->netdev->stats.tx_dropped++;
+ dev->net->stats.tx_dropped++;
}
ctx->tx_curr_frame_num = n;
@@ -814,7 +813,7 @@ cdc_ncm_fill_tx_frame(struct cdc_ncm_ctx *ctx, struct sk_buff *skb, __le32 sign)
/* return skb */
ctx->tx_curr_skb = NULL;
- ctx->netdev->stats.tx_packets += ctx->tx_curr_frame_num;
+ dev->net->stats.tx_packets += ctx->tx_curr_frame_num;
return skb_out;
exit_no_skb:
@@ -846,18 +845,19 @@ static enum hrtimer_restart cdc_ncm_tx_timer_cb(struct hrtimer *timer)
static void cdc_ncm_txpath_bh(unsigned long param)
{
- struct cdc_ncm_ctx *ctx = (struct cdc_ncm_ctx *)param;
+ struct usbnet *dev = (struct usbnet *)param;
+ struct cdc_ncm_ctx *ctx = (struct cdc_ncm_ctx *)dev->data[0];
spin_lock_bh(&ctx->mtx);
if (ctx->tx_timer_pending != 0) {
ctx->tx_timer_pending--;
cdc_ncm_tx_timeout_start(ctx);
spin_unlock_bh(&ctx->mtx);
- } else if (ctx->netdev != NULL) {
+ } else if (dev->net != NULL) {
spin_unlock_bh(&ctx->mtx);
- netif_tx_lock_bh(ctx->netdev);
- usbnet_start_xmit(NULL, ctx->netdev);
- netif_tx_unlock_bh(ctx->netdev);
+ netif_tx_lock_bh(dev->net);
+ usbnet_start_xmit(NULL, dev->net);
+ netif_tx_unlock_bh(dev->net);
} else {
spin_unlock_bh(&ctx->mtx);
}
@@ -880,7 +880,7 @@ cdc_ncm_tx_fixup(struct usbnet *dev, struct sk_buff *skb, gfp_t flags)
goto error;
spin_lock_bh(&ctx->mtx);
- skb_out = cdc_ncm_fill_tx_frame(ctx, skb, cpu_to_le32(USB_CDC_NCM_NDP16_NOCRC_SIGN));
+ skb_out = cdc_ncm_fill_tx_frame(dev, skb, cpu_to_le32(USB_CDC_NCM_NDP16_NOCRC_SIGN));
spin_unlock_bh(&ctx->mtx);
return skb_out;
@@ -1047,9 +1047,10 @@ error:
}
static void
-cdc_ncm_speed_change(struct cdc_ncm_ctx *ctx,
+cdc_ncm_speed_change(struct usbnet *dev,
struct usb_cdc_speed_change *data)
{
+ struct cdc_ncm_ctx *ctx = (struct cdc_ncm_ctx *)dev->data[0];
uint32_t rx_speed = le32_to_cpu(data->DLBitRRate);
uint32_t tx_speed = le32_to_cpu(data->ULBitRate);
@@ -1063,18 +1064,18 @@ cdc_ncm_speed_change(struct cdc_ncm_ctx *ctx,
if ((tx_speed > 1000000) && (rx_speed > 1000000)) {
printk(KERN_INFO KBUILD_MODNAME
- ": %s: %u mbit/s downlink "
- "%u mbit/s uplink\n",
- ctx->netdev->name,
- (unsigned int)(rx_speed / 1000000U),
- (unsigned int)(tx_speed / 1000000U));
+ ": %s: %u mbit/s downlink "
+ "%u mbit/s uplink\n",
+ dev->net->name,
+ (unsigned int)(rx_speed / 1000000U),
+ (unsigned int)(tx_speed / 1000000U));
} else {
printk(KERN_INFO KBUILD_MODNAME
- ": %s: %u kbit/s downlink "
- "%u kbit/s uplink\n",
- ctx->netdev->name,
- (unsigned int)(rx_speed / 1000U),
- (unsigned int)(tx_speed / 1000U));
+ ": %s: %u kbit/s downlink "
+ "%u kbit/s uplink\n",
+ dev->net->name,
+ (unsigned int)(rx_speed / 1000U),
+ (unsigned int)(tx_speed / 1000U));
}
}
}
@@ -1091,7 +1092,7 @@ static void cdc_ncm_status(struct usbnet *dev, struct urb *urb)
/* test for split data in 8-byte chunks */
if (test_and_clear_bit(EVENT_STS_SPLIT, &dev->flags)) {
- cdc_ncm_speed_change(ctx,
+ cdc_ncm_speed_change(dev,
(struct usb_cdc_speed_change *)urb->transfer_buffer);
return;
}
@@ -1108,8 +1109,8 @@ static void cdc_ncm_status(struct usbnet *dev, struct urb *urb)
ctx->connected = le16_to_cpu(event->wValue);
printk(KERN_INFO KBUILD_MODNAME ": %s: network connection:"
- " %sconnected\n",
- ctx->netdev->name, ctx->connected ? "" : "dis");
+ " %sconnected\n",
+ dev->net->name, ctx->connected ? "" : "dis");
usbnet_link_change(dev, ctx->connected, 0);
if (!ctx->connected)
@@ -1121,8 +1122,8 @@ static void cdc_ncm_status(struct usbnet *dev, struct urb *urb)
sizeof(struct usb_cdc_speed_change)))
set_bit(EVENT_STS_SPLIT, &dev->flags);
else
- cdc_ncm_speed_change(ctx,
- (struct usb_cdc_speed_change *) &event[1]);
+ cdc_ncm_speed_change(dev,
+ (struct usb_cdc_speed_change *)&event[1]);
break;
default:
diff --git a/include/linux/usb/cdc_ncm.h b/include/linux/usb/cdc_ncm.h
index 36e1e15..5c47bd9 100644
--- a/include/linux/usb/cdc_ncm.h
+++ b/include/linux/usb/cdc_ncm.h
@@ -98,7 +98,6 @@ struct cdc_ncm_ctx {
const struct usb_cdc_union_desc *union_desc;
const struct usb_cdc_ether_desc *ether_desc;
- struct net_device *netdev;
struct usb_device *udev;
struct usb_interface *control;
struct usb_interface *data;
@@ -129,7 +128,7 @@ struct cdc_ncm_ctx {
extern u8 cdc_ncm_select_altsetting(struct usbnet *dev, struct usb_interface *intf);
extern int cdc_ncm_bind_common(struct usbnet *dev, struct usb_interface *intf, u8 data_altsetting);
extern void cdc_ncm_unbind(struct usbnet *dev, struct usb_interface *intf);
-extern struct sk_buff *cdc_ncm_fill_tx_frame(struct cdc_ncm_ctx *ctx, struct sk_buff *skb, __le32 sign);
+extern struct sk_buff *cdc_ncm_fill_tx_frame(struct usbnet *dev, struct sk_buff *skb, __le32 sign);
extern int cdc_ncm_rx_verify_nth16(struct cdc_ncm_ctx *ctx, struct sk_buff *skb_in);
extern int cdc_ncm_rx_verify_ndp16(struct sk_buff *skb_in, int ndpoffset);
--
1.7.10.4
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [PATCH] drivers/net: replace IS_ERR and PTR_ERR with PTR_ERR_OR_ZERO
From: Duan Jiong @ 2013-11-01 10:00 UTC (permalink / raw)
To: acme; +Cc: netdev, linux-kernel, Duan Jiong
This patch fixes coccinelle error regarding usage of IS_ERR and
PTR_ERR instead of PTR_ERR_OR_ZERO.
Signed-off-by: Duan Jiong <duanj.fnst@cn.fujitsu.com>
---
drivers/net/appletalk/ipddp.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/net/appletalk/ipddp.c b/drivers/net/appletalk/ipddp.c
index 10d0dba..78f06cb 100644
--- a/drivers/net/appletalk/ipddp.c
+++ b/drivers/net/appletalk/ipddp.c
@@ -312,9 +312,7 @@ module_param(ipddp_mode, int, 0);
static int __init ipddp_init_module(void)
{
dev_ipddp = ipddp_init();
- if (IS_ERR(dev_ipddp))
- return PTR_ERR(dev_ipddp);
- return 0;
+ return PTR_ERR_OR_ZERO(dev_ipddp);
}
static void __exit ipddp_cleanup_module(void)
--
1.8.3.1
^ permalink raw reply related
* Re: [PATCH net] net: flow_dissector: fail on evil iph->ihl
From: Eric Dumazet @ 2013-11-01 9:48 UTC (permalink / raw)
To: Jason Wang
Cc: davem, edumazet, netdev, linux-kernel, Petr Matousek,
Michael S. Tsirkin, Daniel Borkmann
In-Reply-To: <1383289270-18952-1-git-send-email-jasowang@redhat.com>
On Fri, 2013-11-01 at 15:01 +0800, Jason Wang wrote:
> We don't validate iph->ihl which may lead a dead loop if we meet a IPIP
> skb whose iph->ihl is zero. Fix this by failing immediately when iph->ihl
> is evil (less than 5).
>
> This issue were introduced by commit ec5efe7946280d1e84603389a1030ccec0a767ae
> (rps: support IPIP encapsulation).
>
> Cc: Eric Dumazet <edumazet@google.com>
> Cc: Petr Matousek <pmatouse@redhat.com>
> Cc: Michael S. Tsirkin <mst@redhat.com>
> Cc: Daniel Borkmann <dborkman@redhat.com>
> Signed-off-by: Jason Wang <jasowang@redhat.com>
> ---
> This patch is needed for stable.
> ---
Acked-by: Eric Dumazet <edumazet@google.com>
Thanks Jason !
^ permalink raw reply
* Re: [PATCH net-next V3 2/2] virtio-net: coalesce rx frags when possible during rx
From: Eric Dumazet @ 2013-11-01 9:47 UTC (permalink / raw)
To: Jason Wang
Cc: mwdalton, mst, netdev, linux-kernel, virtualization, kmindg,
edumazet, davem
In-Reply-To: <1383286068-10421-2-git-send-email-jasowang@redhat.com>
On Fri, 2013-11-01 at 14:07 +0800, Jason Wang wrote:
> Commit 2613af0ed18a11d5c566a81f9a6510b73180660a (virtio_net: migrate mergeable
> rx buffers to page frag allocators) try to increase the payload/truesize for
> MTU-sized traffic. But this will introduce the extra overhead for GSO packets
> received because of the frag list. This commit tries to reduce this issue by
> coalesce the possible rx frags when possible during rx. Test result shows the
> about 15% improvement on full size GSO packet receiving (and even better than
> before commit 2613af0ed18a11d5c566a81f9a6510b73180660a).
>
> Before this commit:
> ./netperf -H 192.168.100.4
> MIGRATED TCP STREAM TEST from 0.0.0.0 (0.0.0.0) port 0 AF_INET to 192.168.100.4
> () port 0 AF_INET : demo
> Recv Send Send
> Socket Socket Message Elapsed
> Size Size Size Time Throughput
> bytes bytes bytes secs. 10^6bits/sec
>
> 87380 16384 16384 10.00 20303.87
>
> After this commit:
> ./netperf -H 192.168.100.4
> MIGRATED TCP STREAM TEST from 0.0.0.0 (0.0.0.0) port 0 AF_INET to 192.168.100.4
> () port 0 AF_INET : demo
> Recv Send Send
> Socket Socket Message Elapsed
> Size Size Size Time Throughput
> bytes bytes bytes secs. 10^6bits/sec
>
> 87380 16384 16384 10.00 23841.26
>
> Cc: Rusty Russell <rusty@rustcorp.com.au>
> Cc: Michael S. Tsirkin <mst@redhat.com>
> Cc: Michael Dalton <mwdalton@google.com>
> Cc: Eric Dumazet <edumazet@google.com>
> Acked-by: Michael S. Tsirkin <mst@redhat.com>
> Acked-by: Eric Dumazet <edumazet@google.com>
> Signed-off-by: Jason Wang <jasowang@redhat.com>
> ---
> Changes from V2:
> - call put_page() instead of depending on skb_coalesce_rx_frag()
> ---
Yes, very nice improvement, thanks again !
Acked-by: Eric Dumazet <edumazet@google.com>
^ permalink raw reply
* Re: [PATCH net-next V3 1/2] net: introduce skb_coalesce_rx_frag()
From: Eric Dumazet @ 2013-11-01 9:45 UTC (permalink / raw)
To: Jason Wang
Cc: mwdalton, mst, netdev, linux-kernel, virtualization, kmindg,
edumazet, davem
In-Reply-To: <1383286068-10421-1-git-send-email-jasowang@redhat.com>
On Fri, 2013-11-01 at 14:07 +0800, Jason Wang wrote:
> Sometimes we need to coalesce the rx frags to avoid frag list. One example is
> virtio-net driver which tries to use small frags for both MTU sized packet and
> GSO packet. So this patch introduce skb_coalesce_rx_frag() to do this.
>
> Cc: Rusty Russell <rusty@rustcorp.com.au>
> Cc: Michael S. Tsirkin <mst@redhat.com>
> Cc: Michael Dalton <mwdalton@google.com>
> Cc: Eric Dumazet <edumazet@google.com>
> Acked-by: Michael S. Tsirkin <mst@redhat.com>
> Signed-off-by: Jason Wang <jasowang@redhat.com>
> ---
> Changes from V2:
> - remove the skb_frag_unref() and let the called to put the page reference
> Changes from V1:
> - remove the useless off parameter.
> ---
Acked-by: Eric Dumazet <edumazet@google.com>
^ permalink raw reply
* [PATCH v3] can: c_can: Speed up rx_poll function
From: Markus Pargmann @ 2013-11-01 9:36 UTC (permalink / raw)
To: Marc Kleine-Budde, Wolfgang Grandegger
Cc: Joe Perches, linux-can, netdev, linux-kernel, kernel,
Markus Pargmann
This patch speeds up the rx_poll function by reducing the number of
register reads.
Replace the 32bit register read by a 16bit register read. Currently
the 32bit register read is implemented by using 2 16bit reads. This is
inefficient as we only use the lower 16bit in rx_poll.
The for loop reads the pending interrupts in every iteration. This
leads up to 16 reads of pending interrupts. The patch introduces a new
outer loop to read the pending interrupts as long as 'quota' is above 0.
This reduces the total number of reads.
The third change is to replace the for-loop by a ffs loop.
Tested on AM335x. I removed all 'static' and 'inline' from c_can.c to
see the timings for all functions. I used the function tracer with
trace_stats.
125kbit:
Function Hit Time Avg s^2
-------- --- ---- --- ---
c_can_do_rx_poll 63960 10168178 us 158.977 us 1493056 us
With patch:
c_can_do_rx_poll 63941 3764057 us 58.867 us 776162.2 us
1Mbit:
Function Hit Time Avg s^2
-------- --- ---- --- ---
c_can_do_rx_poll 69489 30049498 us 432.435 us 9271851 us
With patch:
c_can_do_rx_poll 207109 24322185 us 117.436 us 171469047 us
Signed-off-by: Markus Pargmann <mpa@pengutronix.de>
---
Notes:
Changes in v3:
- Update commit message (measurements and ffs)
Changes in v2:
- Small changes, find_next_bit -> ffs and other
drivers/net/can/c_can/c_can.c | 22 ++++++++++++----------
1 file changed, 12 insertions(+), 10 deletions(-)
diff --git a/drivers/net/can/c_can/c_can.c b/drivers/net/can/c_can/c_can.c
index a668cd4..428681e 100644
--- a/drivers/net/can/c_can/c_can.c
+++ b/drivers/net/can/c_can/c_can.c
@@ -798,17 +798,19 @@ static int c_can_do_rx_poll(struct net_device *dev, int quota)
u32 num_rx_pkts = 0;
unsigned int msg_obj, msg_ctrl_save;
struct c_can_priv *priv = netdev_priv(dev);
- u32 val = c_can_read_reg32(priv, C_CAN_INTPND1_REG);
+ u16 val;
+
+ /*
+ * It is faster to read only one 16bit register. This is only possible
+ * for a maximum number of 16 objects.
+ */
+ BUILD_BUG_ON_MSG(C_CAN_MSG_OBJ_RX_LAST > 16,
+ "Implementation does not support more message objects than 16");
+
+ while (quota > 0 && (val = priv->read_reg(priv, C_CAN_INTPND1_REG))) {
+ while ((msg_obj = ffs(val)) && quota > 0) {
+ val &= ~BIT(msg_obj - 1);
- for (msg_obj = C_CAN_MSG_OBJ_RX_FIRST;
- msg_obj <= C_CAN_MSG_OBJ_RX_LAST && quota > 0;
- val = c_can_read_reg32(priv, C_CAN_INTPND1_REG),
- msg_obj++) {
- /*
- * as interrupt pending register's bit n-1 corresponds to
- * message object n, we need to handle the same properly.
- */
- if (val & (1 << (msg_obj - 1))) {
c_can_object_get(dev, 0, msg_obj, IF_COMM_ALL &
~IF_COMM_TXRQST);
msg_ctrl_save = priv->read_reg(priv,
--
1.8.4.rc3
^ permalink raw reply related
* Re: ipv6 fragmentation-related panic in netfilter
From: Patrick McHardy @ 2013-11-01 9:25 UTC (permalink / raw)
To: Steffen Klassert; +Cc: Tomas Hlavacek, netdev, netfilter-devel
In-Reply-To: <20131101084529.GL31491@secunet.com>
On Fri, Nov 01, 2013 at 09:45:29AM +0100, Steffen Klassert wrote:
> On Wed, Oct 30, 2013 at 12:07:11AM +0000, Patrick McHardy wrote:
> >
> > The problem is that the reassembled packet is referenced by the individual
> > fragments, so we trigger the BUG_ON in pskb_expand_head(). In this
> > particular case the case we BUG() on is actually OK, but I'm looking at
> > a way we can fix this without special casing. Hope to have a patch for
> > testing in the next hours.
>
> Just for the record. I'm observing similar, quite reproducable crashes when
> receiving fragmented icmp echo request packets on an IPsec gateway with
> nf_conntrack_ipv6.
>
> Since git commit 58a317f10 ("netfilter: ipv6: add IPv6 NAT support")
> netfilter might insert a reassembled ipv6 packet with a shared skb and
> local_df = 1 to the ok function. In case of xfrm, __xfrm6_output()
> fragments the packet again and when adjusting the headroom later, we
> crash because of a shared skb.
>
> I can fix it by checking for a shared skb in ip6_fragment() and do
> slow path fragmentation then. But we never needed such a check in
> ip6_fragment(), so it's maybe better to fix it in netfilter.
So what seems to be happening is that this case in __ipv6_conntrack_in()
triggers:
/* Conntrack helpers need the entire reassembled packet in the
* POST_ROUTING hook. In case of unconfirmed connections NAT
* might reassign a helper, so the entire packet is also
* required.
*/
ct = nf_ct_get(reasm, &ctinfo);
if (ct != NULL && !nf_ct_is_untracked(ct)) {
help = nfct_help(ct);
if ((help && help->helper) || !nf_ct_is_confirmed(ct)) {
nf_conntrack_get_reasm(reasm);
NF_HOOK_THRESH(NFPROTO_IPV6, hooknum, reasm,
(struct net_device *)in,
(struct net_device *)out,
okfn, NF_IP6_PRI_CONNTRACK + 1);
Since this code is called while walking through the fragment chain, we have
extra references to the reassembled skb. So I think what we need to do is
to release the fragment chain before calling NF_HOOK_THRESH() and indicate
this to nf_ct_frag6_output() so it will stop processing the chain immediately.
I'll give it a try, will let you know when I have a patch for testing.
^ permalink raw reply
* Re: [PATCH] x86: Run checksumming in parallel accross multiple alu's
From: Ingo Molnar @ 2013-11-01 9:21 UTC (permalink / raw)
To: Neil Horman
Cc: Doug Ledford, Eric Dumazet, linux-kernel, netdev, David Laight
In-Reply-To: <20131031183003.GC25894@hmsreliant.think-freely.org>
* Neil Horman <nhorman@tuxdriver.com> wrote:
> Prefetch and simluated adcx/adox from above:
> Performance counter stats for './test.sh' (20 runs):
>
> 35,704,331 L1-dcache-load-misses ( +- 0.07% ) [75.00%]
> 0 L1-dcache-prefetches [75.00%]
> 19,751,409,264 cycles # 0.000 GHz ( +- 0.59% ) [75.00%]
> 34,850,056 branch-misses ( +- 1.29% ) [75.00%]
>
> 7.768602160 seconds time elapsed ( +- 1.38% )
btw., you might also want to try measuring only the basics:
-e cycles -e instructions -e branches -e branch-misses
that should give you 100% in the last column and should also allow
you to double check whether all the PMU counts are correct: is it
the expected number of instructions, expected number of branches,
expected number of branch-misses, etc.
Then you can remove branch stats and add just L1-dcache stats - and
still be 100% covered:
-e cycles -e instructions -e L1-dcache-loads -e L1-dcache-load-misses
etc.
Just so that you can trust what the PMU tells you. Prefetch counts
are sometimes off, they might include speculative activities, etc.
Thanks,
Ingo
^ permalink raw reply
* Re: [PATCH] x86: Run checksumming in parallel accross multiple alu's
From: Ingo Molnar @ 2013-11-01 9:13 UTC (permalink / raw)
To: Neil Horman
Cc: Eric Dumazet, linux-kernel, sebastien.dugue, Thomas Gleixner,
Ingo Molnar, H. Peter Anvin, x86, netdev
In-Reply-To: <20131031143325.GB25894@hmsreliant.think-freely.org>
* Neil Horman <nhorman@tuxdriver.com> wrote:
> On Thu, Oct 31, 2013 at 11:22:00AM +0100, Ingo Molnar wrote:
> >
> > * Neil Horman <nhorman@tuxdriver.com> wrote:
> >
> > > > etc. For such short runtimes make sure the last column displays
> > > > close to 100%, so that the PMU results become trustable.
> > > >
> > > > A nehalem+ PMU will allow 2-4 events to be measured in parallel,
> > > > plus generics like 'cycles', 'instructions' can be added 'for free'
> > > > because they get counted in a separate (fixed purpose) PMU register.
> > > >
> > > > The last colum tells you what percentage of the runtime that
> > > > particular event was actually active. 100% (or empty last column)
> > > > means it was active all the time.
> > > >
> > > > Thanks,
> > > >
> > > > Ingo
> > > >
> > >
> > > Hmm,
> > >
> > > I ran this test:
> > >
> > > for i in `seq 0 1 3`
> > > do
> > > echo $i > /sys/module/csum_test/parameters/module_test_mode
> > > taskset -c 0 perf stat --repeat 20 -C 0 -e L1-dcache-load-misses -e L1-dcache-prefetches -e cycles -e instructions -ddd ./test.sh
> > > done
> >
> > You need to remove '-ddd' which is a shortcut for a ton of useful
> > events, but here you want to use fewer events, to increase the
> > precision of the measurement.
> >
> > Thanks,
> >
> > Ingo
> >
>
> Thank you ingo, that fixed it. I'm trying some other variants of
> the csum algorithm that Doug and I discussed last night, but FWIW,
> the relative performance of the 4 test cases
> (base/prefetch/parallel/both) remains unchanged. I'm starting to
> feel like at this point, theres very little point in doing
> parallel alu operations (unless we can find a way to break the
> dependency on the carry flag, which is what I'm tinkering with
> now).
I would still like to encourage you to pick up the improvements that
Doug measured (mostly via prefetch tweaking?) - that looked like
some significant speedups that we don't want to lose!
Also, trying to stick the in-kernel implementation into 'perf bench'
would be a useful first step as well, for this and future efforts.
See what we do in tools/perf/bench/mem-memcpy-x86-64-asm.S to pick
up the in-kernel assembly memcpy implementations:
#define memcpy MEMCPY /* don't hide glibc's memcpy() */
#define altinstr_replacement text
#define globl p2align 4; .globl
#define Lmemcpy_c globl memcpy_c; memcpy_c
#define Lmemcpy_c_e globl memcpy_c_e; memcpy_c_e
#include "../../../arch/x86/lib/memcpy_64.S"
So it needed a bit of trickery/wrappery for 'perf bench mem memcpy',
but that is a one-time effort - once it's done then the current
in-kernel csum_partial() implementation would be easily measurable
(and any performance regression in it bisectable, etc.) from that
point on.
In user-space it would also be easier to add various parameters and
experimental implementations and background cache-stressing
workloads automatically.
Something similar might be possible for csum_partial(),
csum_partial_copy*(), etc.
Note, if any of you ventures to add checksum-benchmarking to perf
bench, please base any patches on top of tip:perf/core:
git pull git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git perf/core
as there are a couple of perf bench enhancements in the pipeline
already for v3.13.
Thanks,
Ingo
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox