* Re: [PATCH net-next v13 3/3] net: hisilicon: new hip04 ethernet driver
From: Arnd Bergmann @ 2015-01-20 12:01 UTC (permalink / raw)
To: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
Cc: Ding Tianhong, Alexander Graf, devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-lFZ/pmaqli7XmaaqVzeoHQ,
sergei.shtylyov-M4DtvfQ/ZS1MRgGoP+s0PdBPR1lH4CV8,
eric.dumazet-Re5JQEeQqe8AvxtiuMwx3w,
netdev-u79uwXL29TY76Z2rM5mHXA, xuwei5-C8/M+/jPZTeaMJb+Lgu22Q,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
grant.likely-QSEj5FYQhm4dnm+yROfE0A,
zhangfei.gao-QSEj5FYQhm4dnm+yROfE0A, davem-fT/PcQaiUtIeIZ0/mPfg9Q
In-Reply-To: <54BDBA29.10703-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
On Tuesday 20 January 2015 10:15:05 Ding Tianhong wrote:
> On 2015/1/20 4:34, Arnd Bergmann wrote:
> > On Monday 19 January 2015 19:11:11 Alexander Graf wrote:
> >>
> >> After hammering on the box a bit again, I'm in a situation where I get
> >> lots of
> >>
> >> [302398.232603] hip04-ether e28b0000.ethernet eth0: rx drop
> >> [302398.377309] hip04-ether e28b0000.ethernet eth0: rx drop
> >> [302398.395198] hip04-ether e28b0000.ethernet eth0: rx drop
> >> [302398.466118] hip04-ether e28b0000.ethernet eth0: rx drop
> >> [302398.659009] hip04-ether e28b0000.ethernet eth0: rx drop
> >> [302399.053389] hip04-ether e28b0000.ethernet eth0: rx drop
> >> [302399.122067] hip04-ether e28b0000.ethernet eth0: rx drop
> >> [302399.268192] hip04-ether e28b0000.ethernet eth0: rx drop
> >> [302399.286081] hip04-ether e28b0000.ethernet eth0: rx drop
> >> [302399.594201] hip04-ether e28b0000.ethernet eth0: rx drop
> >> [302399.683416] hip04-ether e28b0000.ethernet eth0: rx drop
> >> [302399.701307] hip04-ether e28b0000.ethernet eth0: rx drop
> >>
> >> and I really am getting a lot of drops - I can't even ping the machine
> >> anymore.
> >>
> >> However, as it is there's a good chance the machine is simply
> >> unreachable because it's busy writing to the UART, and even if not all
> >> useful messages indicating anything have scrolled out. I really don't
> >> think you should emit any message over and over again to the user. Once
> >> or twice is enough.
> >>
> >> Please make sure to rate limit it.
> >
> > I would argue that packet loss is not an error condition at all
> > and you should not print this at netdev_err() level. You could make
> > this a netdev_dbg(), or just make it silent because it's already
> > counted in the statistics.
> >
>
> I think something wrong with Graf's board, I will try to make it happen on my board, and
> in any case I will add rate limit to xx_drop and change to dbg log level.
I've looked at the interrupt handling in more detail and came up
with this patch. Please review, and forward if you are happy with the
changes.
Alex, could you try if this solves your problem?
8<----
Subject: [PATCH] net/hip04: refactor interrupt masking
The hip04 ethernet driver currently acknowledges all interrupts directly
in the interrupt handler, and leaves all interrupts except the RX data
enabled the whole time. This causes multiple problems:
- When more packets come in between the original interrupt and the
NAPI poll function, we will get an extraneous RX interrupt as soon
as interrupts are enabled again.
- The two error interrupts are by definition combining all errors that
may have happened since the last time they were handled, but just
acknowledging the irq without dealing with the cause of the condition
makes it come back immediately. In particular, when NAPI is intentionally
stalling the rx queue, this causes a storm of "rx dropped" messages.
- The access to the 'reg_inten' field in hip_priv is used for serializing
access, but is in fact racy itself.
To deal with these issues, the driver is changed to only acknowledge
the IRQ at the point when it is being handled. The RX interrupts now get
acked right before reading the number of received frames to keep spurious
interrupts to the absolute minimum without losing interrupts.
As there is no tx-complete interrupt, only an informational tx-dropped
one, we now ack that in the tx reclaim handler, hoping that clearing
the descriptors has also eliminated the error condition.
The only place that reads the reg_inten now just relies on
napi_schedule_prep() to return whether NAPI is active or not, and
the poll() function is then going to ack and reenabled the IRQ if
necessary.
Finally, when we disable interrupts, they are now all disabled
together, in order to simplify the logic and to avoid getting
rx-dropped interrupts when NAPI is in control of the rx queue.
Signed-off-by: Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>
diff --git a/drivers/net/ethernet/hisilicon/hip04_eth.c b/drivers/net/ethernet/hisilicon/hip04_eth.c
index 525214ef5984..83773247a368 100644
--- a/drivers/net/ethernet/hisilicon/hip04_eth.c
+++ b/drivers/net/ethernet/hisilicon/hip04_eth.c
@@ -56,6 +56,8 @@
#define RCV_DROP BIT(7)
#define TX_DROP BIT(6)
#define DEF_INT_ERR (RCV_NOBUF | RCV_DROP | TX_DROP)
+#define DEF_INT_RX (RCV_INT | RCV_NOBUF | RCV_DROP)
+#define DEF_INT_TX (TX_DROP)
#define DEF_INT_MASK (RCV_INT | DEF_INT_ERR)
/* TX descriptor config */
@@ -154,7 +156,6 @@ struct hip04_priv {
unsigned int port;
unsigned int speed;
unsigned int duplex;
- unsigned int reg_inten;
struct napi_struct napi;
struct net_device *ndev;
@@ -303,17 +304,15 @@ static void hip04_mac_enable(struct net_device *ndev)
val |= GE_RX_PORT_EN | GE_TX_PORT_EN;
writel_relaxed(val, priv->base + GE_PORT_EN);
- /* clear rx int */
- val = RCV_INT;
- writel_relaxed(val, priv->base + PPE_RINT);
+ /* clear prior interrupts */
+ writel_relaxed(DEF_INT_MASK, priv->base + PPE_RINT);
/* config recv int */
val = GE_RX_INT_THRESHOLD | GE_RX_TIMEOUT;
writel_relaxed(val, priv->base + PPE_CFG_RX_PKT_INT);
/* enable interrupt */
- priv->reg_inten = DEF_INT_MASK;
- writel_relaxed(priv->reg_inten, priv->base + PPE_INTEN);
+ writel_relaxed(DEF_INT_MASK, priv->base + PPE_INTEN);
}
static void hip04_mac_disable(struct net_device *ndev)
@@ -322,8 +321,7 @@ static void hip04_mac_disable(struct net_device *ndev)
u32 val;
/* disable int */
- priv->reg_inten &= ~(DEF_INT_MASK);
- writel_relaxed(priv->reg_inten, priv->base + PPE_INTEN);
+ writel_relaxed(0, priv->base + PPE_INTEN);
/* disable tx & rx */
val = readl_relaxed(priv->base + GE_PORT_EN);
@@ -403,6 +401,8 @@ static int hip04_tx_reclaim(struct net_device *ndev, bool force)
priv->tx_tail = tx_tail;
smp_wmb(); /* Ensure tx_tail visible to xmit */
+ writel_relaxed(DEF_INT_TX, priv->base + PPE_RINT);
+
out:
if (pkts_compl || bytes_compl)
netdev_completed_queue(ndev, pkts_compl, bytes_compl);
@@ -458,9 +458,7 @@ static int hip04_mac_start_xmit(struct sk_buff *skb, struct net_device *ndev)
if (count >= priv->tx_coalesce_frames) {
if (napi_schedule_prep(&priv->napi)) {
/* disable rx interrupt and timer */
- priv->reg_inten &= ~(RCV_INT);
- writel_relaxed(DEF_INT_MASK & ~RCV_INT,
- priv->base + PPE_INTEN);
+ writel_relaxed(0, priv->base + PPE_INTEN);
hrtimer_cancel(&priv->tx_coalesce_timer);
__napi_schedule(&priv->napi);
}
@@ -478,7 +476,7 @@ static int hip04_rx_poll(struct napi_struct *napi, int budget)
struct hip04_priv *priv = container_of(napi, struct hip04_priv, napi);
struct net_device *ndev = priv->ndev;
struct net_device_stats *stats = &ndev->stats;
- unsigned int cnt = hip04_recv_cnt(priv);
+ unsigned int cnt;
struct rx_desc *desc;
struct sk_buff *skb;
unsigned char *buf;
@@ -489,6 +487,10 @@ static int hip04_rx_poll(struct napi_struct *napi, int budget)
u16 len;
u32 err;
+ /* acknowledge interrupts and read status */
+ writel_relaxed(DEF_INT_RX, priv->base + PPE_RINT);
+ cnt = hip04_recv_cnt(priv);
+
while (cnt && !last) {
buf = priv->rx_buf[priv->rx_head];
skb = build_skb(buf, priv->rx_buf_size);
@@ -539,11 +541,8 @@ static int hip04_rx_poll(struct napi_struct *napi, int budget)
cnt = hip04_recv_cnt(priv);
}
- if (!(priv->reg_inten & RCV_INT)) {
- /* enable rx interrupt */
- priv->reg_inten |= RCV_INT;
- writel_relaxed(priv->reg_inten, priv->base + PPE_INTEN);
- }
+ /* enable rx interrupt */
+ writel_relaxed(DEF_INT_MASK, priv->base + PPE_INTEN);
napi_complete(napi);
done:
/* clean up tx descriptors and start a new timer if necessary */
@@ -564,23 +563,21 @@ static irqreturn_t hip04_mac_interrupt(int irq, void *dev_id)
if (!ists)
return IRQ_NONE;
- writel_relaxed(DEF_INT_MASK, priv->base + PPE_RINT);
-
if (unlikely(ists & DEF_INT_ERR)) {
- if (ists & (RCV_NOBUF | RCV_DROP))
+ if (ists & (RCV_NOBUF | RCV_DROP)) {
stats->rx_errors++;
stats->rx_dropped++;
- netdev_err(ndev, "rx drop\n");
+ netdev_dbg(ndev, "rx drop\n");
+ }
if (ists & TX_DROP) {
stats->tx_dropped++;
- netdev_err(ndev, "tx drop\n");
+ netdev_dbg(ndev, "tx drop\n");
}
}
- if (ists & RCV_INT && napi_schedule_prep(&priv->napi)) {
- /* disable rx interrupt */
- priv->reg_inten &= ~(RCV_INT);
- writel_relaxed(DEF_INT_MASK & ~RCV_INT, priv->base + PPE_INTEN);
+ if (napi_schedule_prep(&priv->napi)) {
+ /* disable interrupt */
+ writel_relaxed(0, priv->base + PPE_INTEN);
hrtimer_cancel(&priv->tx_coalesce_timer);
__napi_schedule(&priv->napi);
}
@@ -596,8 +593,7 @@ enum hrtimer_restart tx_done(struct hrtimer *hrtimer)
if (napi_schedule_prep(&priv->napi)) {
/* disable rx interrupt */
- priv->reg_inten &= ~(RCV_INT);
- writel_relaxed(DEF_INT_MASK & ~RCV_INT, priv->base + PPE_INTEN);
+ writel_relaxed(0, priv->base + PPE_INTEN);
__napi_schedule(&priv->napi);
}
--
To unsubscribe from this list: send the line "unsubscribe devicetree" 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
* [net PATCH 1/1] drivers: net: cpsw: discard dual emac default vlan configuration
From: Mugunthan V N @ 2015-01-20 12:25 UTC (permalink / raw)
To: netdev; +Cc: davem, Mugunthan V N, stable
In Dual EMAC, the default vlans are used to segregate rx packets between
the ports, so adding the same default vlan to the switch will affect the
normal packet transfers. So returning error on addition of dual emac
default vlans.
Even if emac 0 default port vlan is added to emac 1, it will lead to
break dual EMAC port seperations.
Fixes: d9ba8f9 (driver: net: ethernet: cpsw: dual emac interface implementation)
Cc: <stable@vger.kernel.org> # v3.9+
Reported-by: Felipe Balbi <balbi@ti.com>
Signed-off-by: Mugunthan V N <mugunthanvnm@ti.com>
---
drivers/net/ethernet/ti/cpsw.c | 22 ++++++++++++++++++++++
1 file changed, 22 insertions(+)
diff --git a/drivers/net/ethernet/ti/cpsw.c b/drivers/net/ethernet/ti/cpsw.c
index e068d48..7c32815 100644
--- a/drivers/net/ethernet/ti/cpsw.c
+++ b/drivers/net/ethernet/ti/cpsw.c
@@ -1683,6 +1683,19 @@ static int cpsw_ndo_vlan_rx_add_vid(struct net_device *ndev,
if (vid == priv->data.default_vlan)
return 0;
+ if (priv->data.dual_emac) {
+ /* In dual EMAC, reserved VLAN id should not be used of
+ * creating vlan interfaces as this can break the dual
+ * EMAC port seperation
+ */
+ int i;
+
+ for (i = 0; i < priv->data.slaves; i++) {
+ if (vid == priv->slaves[i].port_vlan)
+ return -EINVAL;
+ }
+ }
+
dev_info(priv->dev, "Adding vlanid %d to vlan filter\n", vid);
return cpsw_add_vlan_ale_entry(priv, vid);
}
@@ -1696,6 +1709,15 @@ static int cpsw_ndo_vlan_rx_kill_vid(struct net_device *ndev,
if (vid == priv->data.default_vlan)
return 0;
+ if (priv->data.dual_emac) {
+ int i;
+
+ for (i = 0; i < priv->data.slaves; i++) {
+ if (vid == priv->slaves[i].port_vlan)
+ return -EINVAL;
+ }
+ }
+
dev_info(priv->dev, "removing vlanid %d from vlan filter\n", vid);
ret = cpsw_ale_del_vlan(priv->ale, vid, 0);
if (ret != 0)
--
2.2.1.62.g3f15098
^ permalink raw reply related
* [PATCH 00/11] hso: fix some problems with reset/disconnect path
From: Olivier Sobrie @ 2015-01-20 12:29 UTC (permalink / raw)
To: Jan Dumon, Greg Kroah-Hartman
Cc: linux-kernel, linux-usb, netdev, Olivier Sobrie
These patches attempt to fix some problems I observed when the hso
device is disconnected or when a usb reset is queued by the hso driver
Several patches of this serie are fixing crashes or memleaks in hso.
The last patch of this serie fix a race condition occurring when multiple
usb resets are queued on an usb interface.
This serie of patches is based on v3.18.
Olivier Sobrie (11):
hso: remove useless header file timer.h
hso: fix crash when device disappears while serial port is open
hso: fix memory leak when device disconnects
hso: fix memory leak in hso_create_rfkill()
hso: fix small indentation error
hso: rename hso_dev into serial in hso_free_interface()
hso: replace reset_device work by usb_queue_reset_device()
hso: move tty_unregister outside hso_serial_common_free()
hso: update serial_table in usb disconnect method
hso: add missing cancel_work_sync in disconnect()
usb: core: fix a race with usb_queue_reset_device()
drivers/net/usb/hso.c | 88 ++++++++++++++++++++--------------------------
drivers/usb/core/driver.c | 4 ++-
drivers/usb/core/hub.c | 4 +--
drivers/usb/core/message.c | 4 +--
include/linux/usb.h | 2 +-
5 files changed, 46 insertions(+), 56 deletions(-)
--
2.2.0
^ permalink raw reply
* [PATCH 01/11] hso: remove useless header file timer.h
From: Olivier Sobrie @ 2015-01-20 12:29 UTC (permalink / raw)
To: Jan Dumon, Greg Kroah-Hartman
Cc: linux-kernel, linux-usb, netdev, Olivier Sobrie
In-Reply-To: <1421756978-4093-1-git-send-email-olivier@sobrie.be>
No timer related function is used in this driver.
Signed-off-by: Olivier Sobrie <olivier@sobrie.be>
---
drivers/net/usb/hso.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/net/usb/hso.c b/drivers/net/usb/hso.c
index babda7d..cb0bcc1 100644
--- a/drivers/net/usb/hso.c
+++ b/drivers/net/usb/hso.c
@@ -58,7 +58,6 @@
#include <linux/module.h>
#include <linux/ethtool.h>
#include <linux/usb.h>
-#include <linux/timer.h>
#include <linux/tty.h>
#include <linux/tty_driver.h>
#include <linux/tty_flip.h>
--
2.2.0
^ permalink raw reply related
* [PATCH 02/11] hso: fix crash when device disappears while serial port is open
From: Olivier Sobrie @ 2015-01-20 12:29 UTC (permalink / raw)
To: Jan Dumon, Greg Kroah-Hartman
Cc: linux-kernel, linux-usb, netdev, Olivier Sobrie
In-Reply-To: <1421756978-4093-2-git-send-email-olivier@sobrie.be>
When the device disappear, the function hso_disconnect() is called to
perform cleanup. In the cleanup function, hso_free_interface() calls
tty_port_tty_hangup() in view of scheduling a work to hang up the tty if
needed. If the port was not open then hso_serial_ref_free() is called
directly to cleanup everything. Otherwise, hso_serial_ref_free() is called
when the last fd associated to the port is closed.
For each open port, tty_release() will call the close method,
hso_serial_close(), which drops the last kref and call
hso_serial_ref_free() which unregisters, destroys the tty port
and finally frees the structure in which the tty_port structure
is included. Later, in tty_release(), more precisely when release_tty()
is called, the tty_port previously freed is accessed to cancel
the tty buf workqueue and it leads to a crash.
In view of avoiding this crash, we add a cleanup method that is called
at the end of the hangup process and we drop the last kref in this
function when all the ports have been closed, when tty_port is no
more needed and when it is safe to free the structure containing the
tty_port structure.
Signed-off-by: Olivier Sobrie <olivier@sobrie.be>
---
drivers/net/usb/hso.c | 17 +++++++++++++----
1 file changed, 13 insertions(+), 4 deletions(-)
diff --git a/drivers/net/usb/hso.c b/drivers/net/usb/hso.c
index cb0bcc1..3a6c630 100644
--- a/drivers/net/usb/hso.c
+++ b/drivers/net/usb/hso.c
@@ -1270,7 +1270,6 @@ static int hso_serial_open(struct tty_struct *tty, struct file *filp)
goto err_out;
D1("Opening %d", serial->minor);
- kref_get(&serial->parent->ref);
/* setup */
tty->driver_data = serial;
@@ -1289,7 +1288,8 @@ static int hso_serial_open(struct tty_struct *tty, struct file *filp)
if (result) {
hso_stop_serial_device(serial->parent);
serial->port.count--;
- kref_put(&serial->parent->ref, hso_serial_ref_free);
+ } else {
+ kref_get(&serial->parent->ref);
}
} else {
D1("Port was already open");
@@ -1339,8 +1339,6 @@ static void hso_serial_close(struct tty_struct *tty, struct file *filp)
usb_autopm_put_interface(serial->parent->interface);
mutex_unlock(&serial->parent->mutex);
-
- kref_put(&serial->parent->ref, hso_serial_ref_free);
}
/* close the requested serial port */
@@ -1391,6 +1389,16 @@ static int hso_serial_write_room(struct tty_struct *tty)
return room;
}
+static void hso_serial_cleanup(struct tty_struct *tty)
+{
+ struct hso_serial *serial = tty->driver_data;
+
+ if (!serial)
+ return;
+
+ kref_put(&serial->parent->ref, hso_serial_ref_free);
+}
+
/* setup the term */
static void hso_serial_set_termios(struct tty_struct *tty, struct ktermios *old)
{
@@ -3215,6 +3223,7 @@ static const struct tty_operations hso_serial_ops = {
.close = hso_serial_close,
.write = hso_serial_write,
.write_room = hso_serial_write_room,
+ .cleanup = hso_serial_cleanup,
.ioctl = hso_serial_ioctl,
.set_termios = hso_serial_set_termios,
.chars_in_buffer = hso_serial_chars_in_buffer,
--
2.2.0
^ permalink raw reply related
* [PATCH 03/11] hso: fix memory leak when device disconnects
From: Olivier Sobrie @ 2015-01-20 12:29 UTC (permalink / raw)
To: Jan Dumon, Greg Kroah-Hartman
Cc: linux-kernel, linux-usb, netdev, Olivier Sobrie
In-Reply-To: <1421756978-4093-3-git-send-email-olivier@sobrie.be>
In the disconnect path, tx_buffer should freed like tx_data to avoid
a memory leak when the device disconnects.
Signed-off-by: Olivier Sobrie <olivier@sobrie.be>
---
drivers/net/usb/hso.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/net/usb/hso.c b/drivers/net/usb/hso.c
index 3a6c630..470ef9e 100644
--- a/drivers/net/usb/hso.c
+++ b/drivers/net/usb/hso.c
@@ -2253,6 +2253,7 @@ static void hso_serial_common_free(struct hso_serial *serial)
/* unlink and free TX URB */
usb_free_urb(serial->tx_urb);
+ kfree(serial->tx_buffer);
kfree(serial->tx_data);
tty_port_destroy(&serial->port);
}
--
2.2.0
^ permalink raw reply related
* [PATCH 04/11] hso: fix memory leak in hso_create_rfkill()
From: Olivier Sobrie @ 2015-01-20 12:29 UTC (permalink / raw)
To: Jan Dumon, Greg Kroah-Hartman
Cc: linux-kernel, linux-usb, netdev, Olivier Sobrie
In-Reply-To: <1421756978-4093-4-git-send-email-olivier@sobrie.be>
When the rfkill interface was created, a buffer containing the name
of the rfkill node was allocated. This buffer was never freed when the
device disappears.
To fix the problem, we put the name given to rfkill_alloc() in
the hso_net structure.
Signed-off-by: Olivier Sobrie <olivier@sobrie.be>
---
drivers/net/usb/hso.c | 12 +++---------
1 file changed, 3 insertions(+), 9 deletions(-)
diff --git a/drivers/net/usb/hso.c b/drivers/net/usb/hso.c
index 470ef9e..a49ac2e 100644
--- a/drivers/net/usb/hso.c
+++ b/drivers/net/usb/hso.c
@@ -153,6 +153,7 @@ struct hso_net {
struct hso_device *parent;
struct net_device *net;
struct rfkill *rfkill;
+ char name[8];
struct usb_endpoint_descriptor *in_endp;
struct usb_endpoint_descriptor *out_endp;
@@ -2467,27 +2468,20 @@ static void hso_create_rfkill(struct hso_device *hso_dev,
{
struct hso_net *hso_net = dev2net(hso_dev);
struct device *dev = &hso_net->net->dev;
- char *rfkn;
- rfkn = kzalloc(20, GFP_KERNEL);
- if (!rfkn)
- dev_err(dev, "%s - Out of memory\n", __func__);
-
- snprintf(rfkn, 20, "hso-%d",
+ snprintf(hso_net->name, sizeof(hso_net->name), "hso-%d",
interface->altsetting->desc.bInterfaceNumber);
- hso_net->rfkill = rfkill_alloc(rfkn,
+ hso_net->rfkill = rfkill_alloc(hso_net->name,
&interface_to_usbdev(interface)->dev,
RFKILL_TYPE_WWAN,
&hso_rfkill_ops, hso_dev);
if (!hso_net->rfkill) {
dev_err(dev, "%s - Out of memory\n", __func__);
- kfree(rfkn);
return;
}
if (rfkill_register(hso_net->rfkill) < 0) {
rfkill_destroy(hso_net->rfkill);
- kfree(rfkn);
hso_net->rfkill = NULL;
dev_err(dev, "%s - Failed to register rfkill\n", __func__);
return;
--
2.2.0
^ permalink raw reply related
* [PATCH 06/11] hso: rename hso_dev into serial in hso_free_interface()
From: Olivier Sobrie @ 2015-01-20 12:29 UTC (permalink / raw)
To: Jan Dumon, Greg Kroah-Hartman
Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-usb-u79uwXL29TY76Z2rM5mHXA, netdev-u79uwXL29TY76Z2rM5mHXA,
Olivier Sobrie
In-Reply-To: <1421756978-4093-6-git-send-email-olivier-Ui3EtX6WB9GzQB+pC5nmwQ@public.gmane.org>
In other functions of the driver, variables of type "struct hso_serial"
are denoted by "serial" and variables of type "struct hso_device" are
denoted by "hso_dev". This patch makes the hso_free_interface()
consistent with these notations.
Signed-off-by: Olivier Sobrie <olivier-Ui3EtX6WB9GzQB+pC5nmwQ@public.gmane.org>
---
drivers/net/usb/hso.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/net/usb/hso.c b/drivers/net/usb/hso.c
index 2f2343d..484e423 100644
--- a/drivers/net/usb/hso.c
+++ b/drivers/net/usb/hso.c
@@ -3115,17 +3115,17 @@ static void hso_serial_ref_free(struct kref *ref)
static void hso_free_interface(struct usb_interface *interface)
{
- struct hso_serial *hso_dev;
+ struct hso_serial *serial;
int i;
for (i = 0; i < HSO_SERIAL_TTY_MINORS; i++) {
if (serial_table[i] &&
(serial_table[i]->interface == interface)) {
- hso_dev = dev2ser(serial_table[i]);
- tty_port_tty_hangup(&hso_dev->port, false);
- mutex_lock(&hso_dev->parent->mutex);
- hso_dev->parent->usb_gone = 1;
- mutex_unlock(&hso_dev->parent->mutex);
+ serial = dev2ser(serial_table[i]);
+ tty_port_tty_hangup(&serial->port, false);
+ mutex_lock(&serial->parent->mutex);
+ serial->parent->usb_gone = 1;
+ mutex_unlock(&serial->parent->mutex);
kref_put(&serial_table[i]->ref, hso_serial_ref_free);
}
}
--
2.2.0
--
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 07/11] hso: replace reset_device work by usb_queue_reset_device()
From: Olivier Sobrie @ 2015-01-20 12:29 UTC (permalink / raw)
To: Jan Dumon, Greg Kroah-Hartman
Cc: linux-kernel, linux-usb, netdev, Olivier Sobrie
In-Reply-To: <1421756978-4093-7-git-send-email-olivier@sobrie.be>
There is no need for a dedicated reset work in the hso driver since
there is already a reset work foreseen in usb_interface that does
the same.
Signed-off-by: Olivier Sobrie <olivier@sobrie.be>
---
drivers/net/usb/hso.c | 25 +------------------------
1 file changed, 1 insertion(+), 24 deletions(-)
diff --git a/drivers/net/usb/hso.c b/drivers/net/usb/hso.c
index 484e423..55074da 100644
--- a/drivers/net/usb/hso.c
+++ b/drivers/net/usb/hso.c
@@ -274,7 +274,6 @@ struct hso_device {
u8 usb_gone;
struct work_struct async_get_intf;
struct work_struct async_put_intf;
- struct work_struct reset_device;
struct usb_device *usb;
struct usb_interface *interface;
@@ -340,7 +339,6 @@ static void async_put_intf(struct work_struct *data);
static int hso_put_activity(struct hso_device *hso_dev);
static int hso_get_activity(struct hso_device *hso_dev);
static void tiocmget_intr_callback(struct urb *urb);
-static void reset_device(struct work_struct *data);
/*****************************************************************************/
/* Helping functions */
/*****************************************************************************/
@@ -696,7 +694,7 @@ static void handle_usb_error(int status, const char *function,
case -ETIMEDOUT:
explanation = "protocol error";
if (hso_dev)
- schedule_work(&hso_dev->reset_device);
+ usb_queue_reset_device(hso_dev->interface);
break;
default:
explanation = "unknown status";
@@ -2347,7 +2345,6 @@ static struct hso_device *hso_create_device(struct usb_interface *intf,
INIT_WORK(&hso_dev->async_get_intf, async_get_intf);
INIT_WORK(&hso_dev->async_put_intf, async_put_intf);
- INIT_WORK(&hso_dev->reset_device, reset_device);
return hso_dev;
}
@@ -3086,26 +3083,6 @@ out:
return result;
}
-static void reset_device(struct work_struct *data)
-{
- struct hso_device *hso_dev =
- container_of(data, struct hso_device, reset_device);
- struct usb_device *usb = hso_dev->usb;
- int result;
-
- if (hso_dev->usb_gone) {
- D1("No reset during disconnect\n");
- } else {
- result = usb_lock_device_for_reset(usb, hso_dev->interface);
- if (result < 0)
- D1("unable to lock device for reset: %d\n", result);
- else {
- usb_reset_device(usb);
- usb_unlock_device(usb);
- }
- }
-}
-
static void hso_serial_ref_free(struct kref *ref)
{
struct hso_device *hso_dev = container_of(ref, struct hso_device, ref);
--
2.2.0
^ permalink raw reply related
* [PATCH 08/11] hso: move tty_unregister outside hso_serial_common_free()
From: Olivier Sobrie @ 2015-01-20 12:29 UTC (permalink / raw)
To: Jan Dumon, Greg Kroah-Hartman
Cc: linux-kernel, linux-usb, netdev, Olivier Sobrie
In-Reply-To: <1421756978-4093-8-git-send-email-olivier@sobrie.be>
The function hso_serial_common_free() is called either by the cleanup
method of the tty or by the usb disconnect method.
In the former case, the usb_disconnect() has been already called
and the sysfs group associated to the device has been removed.
By calling tty_unregister directly from the usb_disconnect() method,
we avoid a warning due to the removal of the sysfs group of the usb
device.
Example of warning:
------------[ cut here ]------------
WARNING: CPU: 0 PID: 778 at fs/sysfs/group.c:225 sysfs_remove_group+0x50/0x94()
sysfs group c0645a88 not found for kobject 'ttyHS5'
Modules linked in:
CPU: 0 PID: 778 Comm: kworker/0:3 Tainted: G W 3.18.0+ #105
Workqueue: events release_one_tty
[<c000dfe4>] (unwind_backtrace) from [<c000c014>] (show_stack+0x14/0x1c)
[<c000c014>] (show_stack) from [<c0016bac>] (warn_slowpath_common+0x5c/0x7c)
[<c0016bac>] (warn_slowpath_common) from [<c0016c60>] (warn_slowpath_fmt+0x30/0x40)
[<c0016c60>] (warn_slowpath_fmt) from [<c00ddd14>] (sysfs_remove_group+0x50/0x94)
[<c00ddd14>] (sysfs_remove_group) from [<c0221e44>] (device_del+0x30/0x190)
[<c0221e44>] (device_del) from [<c0221fb0>] (device_unregister+0xc/0x18)
[<c0221fb0>] (device_unregister) from [<c0221fec>] (device_destroy+0x30/0x3c)
[<c0221fec>] (device_destroy) from [<c01fe1dc>] (tty_unregister_device+0x2c/0x5c)
[<c01fe1dc>] (tty_unregister_device) from [<c029a428>] (hso_serial_common_free+0x2c/0x88)
[<c029a428>] (hso_serial_common_free) from [<c029a4c0>] (hso_serial_ref_free+0x3c/0xb8)
[<c029a4c0>] (hso_serial_ref_free) from [<c01ff430>] (release_one_tty+0x30/0x84)
[<c01ff430>] (release_one_tty) from [<c00271d4>] (process_one_work+0x21c/0x3c8)
[<c00271d4>] (process_one_work) from [<c0027758>] (worker_thread+0x3d8/0x560)
[<c0027758>] (worker_thread) from [<c002be4c>] (kthread+0xc0/0xcc)
[<c002be4c>] (kthread) from [<c0009630>] (ret_from_fork+0x14/0x24)
---[ end trace cb88537fdc8fa208 ]---
Signed-off-by: Olivier Sobrie <olivier@sobrie.be>
---
drivers/net/usb/hso.c | 12 +++++++++---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/drivers/net/usb/hso.c b/drivers/net/usb/hso.c
index 55074da..5b157ad 100644
--- a/drivers/net/usb/hso.c
+++ b/drivers/net/usb/hso.c
@@ -2234,14 +2234,17 @@ static int hso_stop_serial_device(struct hso_device *hso_dev)
return 0;
}
-static void hso_serial_common_free(struct hso_serial *serial)
+static void hso_serial_tty_unregister(struct hso_serial *serial)
{
- int i;
-
if (serial->parent->dev)
device_remove_file(serial->parent->dev, &dev_attr_hsotype);
tty_unregister_device(tty_drv, serial->minor);
+}
+
+static void hso_serial_common_free(struct hso_serial *serial)
+{
+ int i;
for (i = 0; i < serial->num_rx_urbs; i++) {
/* unlink and free RX URB */
@@ -2323,6 +2326,7 @@ static int hso_serial_common_create(struct hso_serial *serial, int num_urbs,
return 0;
exit:
+ hso_serial_tty_unregister(serial);
hso_serial_common_free(serial);
return -1;
}
@@ -2683,6 +2687,7 @@ static struct hso_device *hso_create_bulk_serial_device(
return hso_dev;
exit2:
+ hso_serial_tty_unregister(serial);
hso_serial_common_free(serial);
exit:
hso_free_tiomget(serial);
@@ -3103,6 +3108,7 @@ static void hso_free_interface(struct usb_interface *interface)
mutex_lock(&serial->parent->mutex);
serial->parent->usb_gone = 1;
mutex_unlock(&serial->parent->mutex);
+ hso_serial_tty_unregister(serial);
kref_put(&serial_table[i]->ref, hso_serial_ref_free);
}
}
--
2.2.0
^ permalink raw reply related
* [PATCH 09/11] hso: update serial_table in usb disconnect method
From: Olivier Sobrie @ 2015-01-20 12:29 UTC (permalink / raw)
To: Jan Dumon, Greg Kroah-Hartman
Cc: linux-kernel, linux-usb, netdev, Olivier Sobrie
In-Reply-To: <1421756978-4093-9-git-send-email-olivier@sobrie.be>
The serial_table is used to map the minor number of the usb serial device
to its associated context. The table is updated in the probe method and
in hso_serial_ref_free() which is called either from the tty cleanup
method or from the usb disconnect method.
This patch ensures that the serial_table is updated in the disconnect
method and no more from the cleanup method to avoid the following
potential race condition.
- hso_disconnect() is called for usb interface "x". Because the serial
port was open and because the cleanup method of the tty_port hasn't
been called yet, hso_serial_ref_free() is not run.
- hso_probe() is called and fails for a new hso serial usb interface
"y". The function hso_free_interface() is called and iterates
over the element of serial_table to find the device associated to
the usb interface context.
If the usb interface context of usb interface "y" has been created
at the same place as for usb interface "x", then the cleanup
functions are called for usb interfaces "x" and "y" and
hso_serial_ref_free() is called for both interfaces.
- release_tty() is called for serial port linked to usb interface "x"
and possibly crash because the tty_port structure contained in the
hso_device structure has been freed.
Signed-off-by: Olivier Sobrie <olivier@sobrie.be>
---
drivers/net/usb/hso.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/usb/hso.c b/drivers/net/usb/hso.c
index 5b157ad..c916ab5 100644
--- a/drivers/net/usb/hso.c
+++ b/drivers/net/usb/hso.c
@@ -2597,7 +2597,6 @@ static void hso_free_serial_device(struct hso_device *hso_dev)
if (!serial)
return;
- set_serial_by_index(serial->minor, NULL);
hso_serial_common_free(serial);
@@ -3110,6 +3109,7 @@ static void hso_free_interface(struct usb_interface *interface)
mutex_unlock(&serial->parent->mutex);
hso_serial_tty_unregister(serial);
kref_put(&serial_table[i]->ref, hso_serial_ref_free);
+ set_serial_by_index(i, NULL);
}
}
--
2.2.0
^ permalink raw reply related
* [PATCH 10/11] hso: add missing cancel_work_sync in disconnect()
From: Olivier Sobrie @ 2015-01-20 12:29 UTC (permalink / raw)
To: Jan Dumon, Greg Kroah-Hartman
Cc: linux-kernel, linux-usb, netdev, Olivier Sobrie
In-Reply-To: <1421756978-4093-10-git-send-email-olivier@sobrie.be>
For hso serial devices, two cancel_work_sync were missing in the
disconnect method.
Signed-off-by: Olivier Sobrie <olivier@sobrie.be>
---
drivers/net/usb/hso.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/net/usb/hso.c b/drivers/net/usb/hso.c
index c916ab5..c14fc80 100644
--- a/drivers/net/usb/hso.c
+++ b/drivers/net/usb/hso.c
@@ -3107,6 +3107,8 @@ static void hso_free_interface(struct usb_interface *interface)
mutex_lock(&serial->parent->mutex);
serial->parent->usb_gone = 1;
mutex_unlock(&serial->parent->mutex);
+ cancel_work_sync(&serial_table[i]->async_put_intf);
+ cancel_work_sync(&serial_table[i]->async_get_intf);
hso_serial_tty_unregister(serial);
kref_put(&serial_table[i]->ref, hso_serial_ref_free);
set_serial_by_index(i, NULL);
--
2.2.0
^ permalink raw reply related
* [PATCH 11/11] usb: core: fix a race with usb_queue_reset_device()
From: Olivier Sobrie @ 2015-01-20 12:29 UTC (permalink / raw)
To: Jan Dumon, Greg Kroah-Hartman
Cc: linux-kernel, linux-usb, netdev, Olivier Sobrie
In-Reply-To: <1421756978-4093-11-git-send-email-olivier@sobrie.be>
When usb_queue_reset() is called it schedules a work in view of
resetting the usb interface. When the reset work is running, it
can be scheduled again (e.g. by the usb disconnect method of
the driver).
Consider that the reset work is queued again while the reset work
is running and that this work leads to a forced unbinding of the
usb interface (e.g. because a driver is bound to the interface
and has no pre/post_reset methods - see usb_reset_device()).
In such condition, usb_unbind_interface() gets called and this
function calls usb_cancel_queued_reset() which does nothing
because the flag "reset_running" is set to 1. The second reset
work that has been scheduled is therefore not cancelled.
Later, the usb_reset_device() tries to rebind the interface.
If it fails, then the usb interface context which contain the
reset work struct is freed and it most likely crash when the
second reset work tries to be run.
The following flow shows the problem:
* usb_queue_reset_device()
* __usb_queue_reset_device() <- If the reset work is queued after here, then
reset_running = 1 it will never be cancelled.
usb_reset_device()
usb_forced_unbind_intf()
usb_driver_release_interface()
usb_unbind_interface()
driver->disconnect()
usb_queue_reset_device() <- second reset
usb_cancel_queued_reset() <- does nothing because
the flag reset_running
is set
usb_unbind_and_rebind_marked_interfaces()
usb_rebind_intf()
device_attach()
driver->probe() <- fails (no more drivers hold a reference to
the usb interface)
reset_running = 0
* hub_event()
usb_disconnect()
usb_disable_device()
kobject_release()
device_release()
usb_release_interface()
kfree(intf) <- usb interface context is released
while we still have a pending reset
work that should be run
To avoid this problem, we use a delayed work so that if the reset
work is currently run, we can avoid further call to
__usb_queue_reset_device() work by using cancel_delayed_work().
Unfortunately it increases the size of the usb_interface structure...
Signed-off-by: Olivier Sobrie <olivier@sobrie.be>
---
drivers/usb/core/driver.c | 4 +++-
drivers/usb/core/hub.c | 4 ++--
drivers/usb/core/message.c | 4 ++--
include/linux/usb.h | 2 +-
4 files changed, 8 insertions(+), 6 deletions(-)
diff --git a/drivers/usb/core/driver.c b/drivers/usb/core/driver.c
index 9bffd26..c93fbbbb 100644
--- a/drivers/usb/core/driver.c
+++ b/drivers/usb/core/driver.c
@@ -287,7 +287,9 @@ static int usb_unbind_device(struct device *dev)
static void usb_cancel_queued_reset(struct usb_interface *iface)
{
if (iface->reset_running == 0)
- cancel_work_sync(&iface->reset_ws);
+ cancel_delayed_work_sync(&iface->reset_ws);
+ else
+ cancel_delayed_work(&iface->reset_ws);
}
/* called from driver core with dev locked */
diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index b649fef..52fba97 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -5604,13 +5604,13 @@ EXPORT_SYMBOL_GPL(usb_reset_device);
* NOTE: We don't do any reference count tracking because it is not
* needed. The lifecycle of the work_struct is tied to the
* usb_interface. Before destroying the interface we cancel the
- * work_struct, so the fact that work_struct is queued and or
+ * delayed_work, so the fact that delayed_work is queued and or
* running means the interface (and thus, the device) exist and
* are referenced.
*/
void usb_queue_reset_device(struct usb_interface *iface)
{
- schedule_work(&iface->reset_ws);
+ schedule_delayed_work(&iface->reset_ws, 0);
}
EXPORT_SYMBOL_GPL(usb_queue_reset_device);
diff --git a/drivers/usb/core/message.c b/drivers/usb/core/message.c
index f7b7713..d9f3f68 100644
--- a/drivers/usb/core/message.c
+++ b/drivers/usb/core/message.c
@@ -1650,7 +1650,7 @@ static void __usb_queue_reset_device(struct work_struct *ws)
{
int rc;
struct usb_interface *iface =
- container_of(ws, struct usb_interface, reset_ws);
+ container_of(ws, struct usb_interface, reset_ws.work);
struct usb_device *udev = interface_to_usbdev(iface);
rc = usb_lock_device_for_reset(udev, iface);
@@ -1847,7 +1847,7 @@ free_interfaces:
intf->dev.type = &usb_if_device_type;
intf->dev.groups = usb_interface_groups;
intf->dev.dma_mask = dev->dev.dma_mask;
- INIT_WORK(&intf->reset_ws, __usb_queue_reset_device);
+ INIT_DELAYED_WORK(&intf->reset_ws, __usb_queue_reset_device);
intf->minor = -1;
device_initialize(&intf->dev);
pm_runtime_no_callbacks(&intf->dev);
diff --git a/include/linux/usb.h b/include/linux/usb.h
index 447a7e2..ffb3da1 100644
--- a/include/linux/usb.h
+++ b/include/linux/usb.h
@@ -187,7 +187,7 @@ struct usb_interface {
struct device dev; /* interface specific device info */
struct device *usb_dev;
atomic_t pm_usage_cnt; /* usage counter for autosuspend */
- struct work_struct reset_ws; /* for resets in atomic context */
+ struct delayed_work reset_ws; /* for resets in atomic context */
};
#define to_usb_interface(d) container_of(d, struct usb_interface, dev)
--
2.2.0
^ permalink raw reply related
* [PATCH 05/11] hso: fix small indentation error
From: Olivier Sobrie @ 2015-01-20 12:29 UTC (permalink / raw)
To: Jan Dumon, Greg Kroah-Hartman
Cc: linux-kernel, linux-usb, netdev, Olivier Sobrie
In-Reply-To: <1421756978-4093-5-git-send-email-olivier@sobrie.be>
Simply remove the useless extra tab.
Signed-off-by: Olivier Sobrie <olivier@sobrie.be>
---
drivers/net/usb/hso.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/usb/hso.c b/drivers/net/usb/hso.c
index a49ac2e..2f2343d 100644
--- a/drivers/net/usb/hso.c
+++ b/drivers/net/usb/hso.c
@@ -2206,8 +2206,8 @@ static int hso_stop_serial_device(struct hso_device *hso_dev)
for (i = 0; i < serial->num_rx_urbs; i++) {
if (serial->rx_urb[i]) {
- usb_kill_urb(serial->rx_urb[i]);
- serial->rx_urb_filled[i] = 0;
+ usb_kill_urb(serial->rx_urb[i]);
+ serial->rx_urb_filled[i] = 0;
}
}
serial->curr_rx_urb_idx = 0;
--
2.2.0
^ permalink raw reply related
* [PATCH net-next] act_connmark: Add missing dependency on NF_CONNTRACK_MARK
From: Thomas Graf @ 2015-01-20 12:44 UTC (permalink / raw)
To: davem; +Cc: netdev, Felix Fietkau, Jamal Hadi Salim
Depending on NETFILTER is not sufficient to ensure the presence of the
'mark' field in nf_conn, also needs to depend on NF_CONNTRACK_MARK.
Fixes: 22a5dc ("net: sched: Introduce connmark action")
Cc: Felix Fietkau <nbd@openwrt.org>
Cc: Jamal Hadi Salim <jhs@mojatatu.com>
Signed-off-by: Thomas Graf <tgraf@suug.ch>
---
net/sched/Kconfig | 1 +
1 file changed, 1 insertion(+)
diff --git a/net/sched/Kconfig b/net/sched/Kconfig
index 475e35e..7a57f66 100644
--- a/net/sched/Kconfig
+++ b/net/sched/Kconfig
@@ -713,6 +713,7 @@ config NET_ACT_BPF
config NET_ACT_CONNMARK
tristate "Netfilter Connection Mark Retriever"
depends on NET_CLS_ACT && NETFILTER && IP_NF_IPTABLES
+ depends on NF_CONNTRACK_MARK
---help---
Say Y here to allow retrieving of conn mark
--
1.9.3
^ permalink raw reply related
* Re: [PATCH 04/11] hso: fix memory leak in hso_create_rfkill()
From: Oliver Neukum @ 2015-01-20 13:13 UTC (permalink / raw)
To: Olivier Sobrie
Cc: Jan Dumon, Greg Kroah-Hartman, linux-kernel, linux-usb, netdev
In-Reply-To: <1421756978-4093-5-git-send-email-olivier@sobrie.be>
On Tue, 2015-01-20 at 13:29 +0100, Olivier Sobrie wrote:
> When the rfkill interface was created, a buffer containing the name
> of the rfkill node was allocated. This buffer was never freed when the
> device disappears.
>
> To fix the problem, we put the name given to rfkill_alloc() in
> the hso_net structure.
>
> Signed-off-by: Olivier Sobrie <olivier@sobrie.be>
> ---
> drivers/net/usb/hso.c | 12 +++---------
> 1 file changed, 3 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/net/usb/hso.c b/drivers/net/usb/hso.c
> index 470ef9e..a49ac2e 100644
> --- a/drivers/net/usb/hso.c
> +++ b/drivers/net/usb/hso.c
> @@ -153,6 +153,7 @@ struct hso_net {
> struct hso_device *parent;
> struct net_device *net;
> struct rfkill *rfkill;
> + char name[8];
>
> struct usb_endpoint_descriptor *in_endp;
> struct usb_endpoint_descriptor *out_endp;
> @@ -2467,27 +2468,20 @@ static void hso_create_rfkill(struct hso_device *hso_dev,
> {
> struct hso_net *hso_net = dev2net(hso_dev);
> struct device *dev = &hso_net->net->dev;
> - char *rfkn;
>
> - rfkn = kzalloc(20, GFP_KERNEL);
> - if (!rfkn)
> - dev_err(dev, "%s - Out of memory\n", __func__);
> -
> - snprintf(rfkn, 20, "hso-%d",
> + snprintf(hso_net->name, sizeof(hso_net->name), "hso-%d",
> interface->altsetting->desc.bInterfaceNumber);
That number is not unique. Indeed it will be identical for all devices.
Regards
Oliver
--
Oliver Neukum <oneukum@suse.de>
^ permalink raw reply
* [PATCH net 0/2] fix napi return value
From: Govindarajulu Varadarajan @ 2015-01-20 13:16 UTC (permalink / raw)
To: davem, netdev, ariel.elior; +Cc: ssujith, benve, Govindarajulu Varadarajan
Since d75b1ade567ffab ("net: less interrupt masking in NAPI") we should return
budget for napi repoll.
In enic, we return 0 when busy_poll is happening. Fix this by returning budget.
Looks like bnx2x needs this fix as well. On bnx2x I performed compile
test only. I do not have hardware.
Govindarajulu Varadarajan (2):
enic: fix rx napi poll return value
bnx2x: fix napi poll return value for repoll
drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c | 4 ++--
drivers/net/ethernet/cisco/enic/enic_main.c | 2 +-
2 files changed, 3 insertions(+), 3 deletions(-)
--
2.2.2
^ permalink raw reply
* [PATCH net 1/2] enic: fix rx napi poll return value
From: Govindarajulu Varadarajan @ 2015-01-20 13:16 UTC (permalink / raw)
To: davem, netdev, ariel.elior; +Cc: ssujith, benve, Govindarajulu Varadarajan
In-Reply-To: <1421759776-376-1-git-send-email-_govind@gmx.com>
With the commit d75b1ade567ffab ("net: less interrupt masking in NAPI") napi repoll
is done only when work_done == budget. When we are in busy_poll we return 0 in
napi_poll. We should return budget.
Signed-off-by: Govindarajulu Varadarajan <_govind@gmx.com>
---
drivers/net/ethernet/cisco/enic/enic_main.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/cisco/enic/enic_main.c b/drivers/net/ethernet/cisco/enic/enic_main.c
index b29e027..e356afa 100644
--- a/drivers/net/ethernet/cisco/enic/enic_main.c
+++ b/drivers/net/ethernet/cisco/enic/enic_main.c
@@ -1335,7 +1335,7 @@ static int enic_poll_msix_rq(struct napi_struct *napi, int budget)
int err;
if (!enic_poll_lock_napi(&enic->rq[rq]))
- return work_done;
+ return budget;
/* Service RQ
*/
--
2.2.2
^ permalink raw reply related
* [PATCH net 2/2] bnx2x: fix napi poll return value for repoll
From: Govindarajulu Varadarajan @ 2015-01-20 13:16 UTC (permalink / raw)
To: davem, netdev, ariel.elior; +Cc: ssujith, benve, Govindarajulu Varadarajan
In-Reply-To: <1421759776-376-1-git-send-email-_govind@gmx.com>
With the commit d75b1ade567ffab ("net: less interrupt masking in NAPI") napi repoll
is done only when work_done == budget. When in busy_poll is we return 0 in
napi_poll. We should return budget. Also do not return workdone > budget.
Signed-off-by: Govindarajulu Varadarajan <_govind@gmx.com>
---
drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c
index 1d1147c..ebcbe92 100644
--- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c
+++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c
@@ -3175,7 +3175,7 @@ static int bnx2x_poll(struct napi_struct *napi, int budget)
}
#endif
if (!bnx2x_fp_lock_napi(fp))
- return work_done;
+ return budget;
for_each_cos_in_tx_queue(fp, cos)
if (bnx2x_tx_queue_has_work(fp->txdata_ptr[cos]))
@@ -3187,7 +3187,7 @@ static int bnx2x_poll(struct napi_struct *napi, int budget)
/* must not complete if we consumed full budget */
if (work_done >= budget) {
bnx2x_fp_unlock_napi(fp);
- break;
+ return budget;
}
}
--
2.2.2
^ permalink raw reply related
* [PATCH 2/3] netlink: Mark dumps as inconsistent which have been interrupted by a resize
From: Thomas Graf @ 2015-01-20 13:20 UTC (permalink / raw)
To: davem, kaber, herbert, paulmck, ying.xue; +Cc: netdev, netfilter-devel
In-Reply-To: <cover.1421759056.git.tgraf@suug.ch>
A deferred resize of nl_table causes the offsets that Netlink diag keeps
to become inaccurate. Mark the dump as inconsistent and have user space
request a new dump.
Signed-off-by: Thomas Graf <tgraf@suug.ch>
---
net/netlink/af_netlink.c | 10 ++++++++++
net/netlink/af_netlink.h | 1 +
net/netlink/diag.c | 1 +
3 files changed, 12 insertions(+)
diff --git a/net/netlink/af_netlink.c b/net/netlink/af_netlink.c
index 7a94185..e214557 100644
--- a/net/netlink/af_netlink.c
+++ b/net/netlink/af_netlink.c
@@ -91,6 +91,9 @@ static inline int netlink_is_kernel(struct sock *sk)
struct netlink_table *nl_table;
EXPORT_SYMBOL_GPL(nl_table);
+atomic_t nl_table_seq;
+EXPORT_SYMBOL_GPL(nl_table_seq);
+
static DECLARE_WAIT_QUEUE_HEAD(nl_table_wait);
static int netlink_dump(struct sock *sk);
@@ -3071,6 +3074,12 @@ static const struct net_proto_family netlink_family_ops = {
.owner = THIS_MODULE, /* for consistency 8) */
};
+static void nl_table_resize_notify(const struct rhashtable *ht,
+ enum rht_resize_op op)
+{
+ atomic_inc(&nl_table_seq);
+}
+
static int __net_init netlink_net_init(struct net *net)
{
#ifdef CONFIG_PROC_FS
@@ -3124,6 +3133,7 @@ static int __init netlink_proto_init(void)
.max_shift = 16, /* 64K */
.grow_decision = rht_grow_above_75,
.shrink_decision = rht_shrink_below_30,
+ .resize_notify = nl_table_resize_notify,
};
if (err != 0)
diff --git a/net/netlink/af_netlink.h b/net/netlink/af_netlink.h
index 7518375..51c23c0 100644
--- a/net/netlink/af_netlink.h
+++ b/net/netlink/af_netlink.h
@@ -74,5 +74,6 @@ struct netlink_table {
extern struct netlink_table *nl_table;
extern rwlock_t nl_table_lock;
+extern atomic_t nl_table_seq;
#endif
diff --git a/net/netlink/diag.c b/net/netlink/diag.c
index 3ee63a3..50aa385 100644
--- a/net/netlink/diag.c
+++ b/net/netlink/diag.c
@@ -112,6 +112,7 @@ static int __netlink_diag_dump(struct sk_buff *skb, struct netlink_callback *cb,
int ret = 0, num = 0, i;
req = nlmsg_data(cb->nlh);
+ cb->seq = atomic_read(&nl_table_seq);
for (i = 0; i < htbl->size; i++) {
struct rhash_head *pos;
--
1.9.3
^ permalink raw reply related
* [PATCH 1/3] rhashtable: Provide notifier for deferred resizes
From: Thomas Graf @ 2015-01-20 13:20 UTC (permalink / raw)
To: davem, kaber, herbert, paulmck, ying.xue; +Cc: netdev, netfilter-devel
In-Reply-To: <cover.1421759056.git.tgraf@suug.ch>
Allowing users of rhashtable to bump a sequence counter and invalidate
Netlink dumps which have been interrupted by a deferred resize.
Signed-off-by: Thomas Graf <tgraf@suug.ch>
---
include/linux/rhashtable.h | 10 +++++++++-
lib/rhashtable.c | 13 ++++++++++---
2 files changed, 19 insertions(+), 4 deletions(-)
diff --git a/include/linux/rhashtable.h b/include/linux/rhashtable.h
index a2562ed..b711d16 100644
--- a/include/linux/rhashtable.h
+++ b/include/linux/rhashtable.h
@@ -1,7 +1,7 @@
/*
* Resizable, Scalable, Concurrent Hash Table
*
- * Copyright (c) 2014 Thomas Graf <tgraf@suug.ch>
+ * Copyright (c) 2014-2015 Thomas Graf <tgraf@suug.ch>
* Copyright (c) 2008-2014 Patrick McHardy <kaber@trash.net>
*
* Based on the following paper by Josh Triplett, Paul E. McKenney
@@ -64,6 +64,11 @@ typedef u32 (*rht_obj_hashfn_t)(const void *data, u32 seed);
struct rhashtable;
+enum rht_resize_op {
+ RHT_GROWING,
+ RHT_SHRINKING,
+};
+
/**
* struct rhashtable_params - Hash table construction parameters
* @nelem_hint: Hint on number of elements, should be 75% of desired size
@@ -79,6 +84,7 @@ struct rhashtable;
* @obj_hashfn: Function to hash object
* @grow_decision: If defined, may return true if table should expand
* @shrink_decision: If defined, may return true if table should shrink
+ * @resize_notify: Called when a table resize is scheduled.
*
* Note: when implementing the grow and shrink decision function, min/max
* shift must be enforced, otherwise, resizing watermarks they set may be
@@ -100,6 +106,8 @@ struct rhashtable_params {
size_t new_size);
bool (*shrink_decision)(const struct rhashtable *ht,
size_t new_size);
+ void (*resize_notify)(const struct rhashtable *ht,
+ enum rht_resize_op op);
};
/**
diff --git a/lib/rhashtable.c b/lib/rhashtable.c
index 84a78e3..a4449c4 100644
--- a/lib/rhashtable.c
+++ b/lib/rhashtable.c
@@ -1,7 +1,7 @@
/*
* Resizable, Scalable, Concurrent Hash Table
*
- * Copyright (c) 2014 Thomas Graf <tgraf@suug.ch>
+ * Copyright (c) 2014-2015 Thomas Graf <tgraf@suug.ch>
* Copyright (c) 2008-2014 Patrick McHardy <kaber@trash.net>
*
* Based on the following paper:
@@ -489,10 +489,17 @@ static void rht_deferred_worker(struct work_struct *work)
mutex_lock(&ht->mutex);
tbl = rht_dereference(ht->tbl, ht);
- if (ht->p.grow_decision && ht->p.grow_decision(ht, tbl->size))
+ if (ht->p.grow_decision && ht->p.grow_decision(ht, tbl->size)) {
+ if (ht->p.resize_notify)
+ ht->p.resize_notify(ht, RHT_GROWING);
+
rhashtable_expand(ht);
- else if (ht->p.shrink_decision && ht->p.shrink_decision(ht, tbl->size))
+ } else if (ht->p.shrink_decision && ht->p.shrink_decision(ht, tbl->size)) {
+ if (ht->p.resize_notify)
+ ht->p.resize_notify(ht, RHT_SHRINKING);
+
rhashtable_shrink(ht);
+ }
mutex_unlock(&ht->mutex);
}
--
1.9.3
^ permalink raw reply related
* [PATCH 3/3] netlink: Lock out table resizes while dumping Netlink sockets
From: Thomas Graf @ 2015-01-20 13:20 UTC (permalink / raw)
To: davem, kaber, herbert, paulmck, ying.xue; +Cc: netdev, netfilter-devel
In-Reply-To: <cover.1421759056.git.tgraf@suug.ch>
Lock out table resizes while dumping Netlink sockets to user space.
This keeps disruptions to a minimum for readers which don't handle
the NLM_F_DUMP_INTR flag.
Signed-off-by: Thomas Graf <tgraf@suug.ch>
---
net/netlink/diag.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/net/netlink/diag.c b/net/netlink/diag.c
index 50aa385..be4ea6e 100644
--- a/net/netlink/diag.c
+++ b/net/netlink/diag.c
@@ -114,6 +114,8 @@ static int __netlink_diag_dump(struct sk_buff *skb, struct netlink_callback *cb,
req = nlmsg_data(cb->nlh);
cb->seq = atomic_read(&nl_table_seq);
+ mutex_lock(&ht->mutex);
+
for (i = 0; i < htbl->size; i++) {
struct rhash_head *pos;
@@ -161,6 +163,7 @@ static int __netlink_diag_dump(struct sk_buff *skb, struct netlink_callback *cb,
num++;
}
done:
+ mutex_unlock(&ht->mutex);
cb->args[0] = num;
cb->args[1] = protocol;
--
1.9.3
^ permalink raw reply related
* [PATCH 0/3 net-next] rhashtable: Notify on resize to allow signaling interrupted dumps
From: Thomas Graf @ 2015-01-20 13:20 UTC (permalink / raw)
To: davem, kaber, herbert, paulmck, ying.xue; +Cc: netdev, netfilter-devel
Netlink dumps have a traditional weakness of being interruptible and
non-atomic. Once the message buffer has been filled, all protective
locks must be released and the buffer is handed over to the user to
copy the data. The dumping then continues from the previously stored
offsets in the data structures. Due to dropping of the locks, mutations
can occur.
This first series introduces a new notification mechanism which allows
users of rhashtable to bump a sequence number on resizes and report
an inconsistent dump to user space.
As discussed in the netdev thread ("[PATCH 7/9] rhashtable: Per bucket
locks & deferred expansion/shrinking").
Thomas Graf (3):
rhashtable: Provide notifier for deferred resizes
netlink: Mark dumps as inconsistent which have been interrupted by a
resize
netlink: Lock out table resizes while dumping Netlink sockets
include/linux/rhashtable.h | 10 +++++++++-
lib/rhashtable.c | 13 ++++++++++---
net/netlink/af_netlink.c | 10 ++++++++++
net/netlink/af_netlink.h | 1 +
net/netlink/diag.c | 4 ++++
5 files changed, 34 insertions(+), 4 deletions(-)
--
1.9.3
^ permalink raw reply
* PHYless ethernet switch MAC-MAC serdes connection
From: Vijay @ 2015-01-20 13:20 UTC (permalink / raw)
To: netdev
Hello All,
I have a custom board with Marvell 88E6046 ethernet switch which is
connected directly to freescale P1010 processor.
+------------+
| | +------------+
| | SGMII | |----- p0
| eth0 |-------------| p9 |
| | | |----- p1
| | +------------+
| | 88e6086 (Switch)
| |
| |
| | SGMII +-------------+
| eth1 |-------------| |
+--------------+ +-------------+
FS P1010 88E1512 ( PHY)
My dts looks like this,
mii_bus0: mdio@24000 {
#address-cells = <1>;
#size-cells = <0>;
compatible = "fsl,etsec2-mdio";
reg = <0x24000 0x1000 0xb0030 0x4>;
/* No PHY on external MDIO for 88e6086 Switch,
PHYless direct connection*/
/*------------*/
/* External PHY 88E1512 for eth1 */
phy1: ethernet-phy@1 {
interrupt-parent = <&mpic>;
interrupts = <3 1>;
reg = <0x1>;
};
};
mdio@25000 {
#address-cells = <1>;
#size-cells = <0>;
compatible = "fsl,etsec2-tbi";
reg = <0x25000 0x1000 0xb1030 0x4>;
tbi0: tbi-phy@11 { /* TBI
needed by gianfar for fixed-link eth0 */
reg = <0x11>;
device_type = "tbi-phy";
};
};
mdio@26000 {
#address-cells = <1>;
#size-cells = <0>;
compatible = "fsl,etsec2-tbi";
reg = <0x26000 0x1000 0xb1030 0x4>;
tbi1: tbi-phy@12 {
reg = <0x12>;
device_type = "tbi-phy";
};
};
enet1: ethernet@b1000 { /*switch
connected here*/
#address-cells = <1>;
#size-cells = <1>;
device_type = "network";
model = "eTSEC";
compatible = "fsl,etsec2";
reg = <0xb1000 0x1000>;
interrupts = <35 2 36 2 40 2>;
fsl,num_rx_queues = <0x1>;
fsl,num_tx_queues = <0x1>;
local-mac-address = [ 00 00 00 00 00 00 ];
interrupt-parent = <&mpic>;
fixed-link = <0 1 1000 0 0>;
tbi-handle = <&tbi0>;
phy-mode = "sgmii";
queue-group@0 {
#address-cells = <1>;
#size-cells = <1>;
reg = <0xb1000 0x1000>;
rx-bit-map = <0xff>;
tx-bit-map = <0xff>;
interrupts = <35 2 36 2 40 2>;
};
};
enet2: ethernet@b2000 {
#address-cells = <1>;
#size-cells = <1>;
device_type = "network";
model = "eTSEC";
compatible = "fsl,etsec2";
fsl,num_rx_queues = <0x1>;
fsl,num_tx_queues = <0x1>;
local-mac-address = [ 00 00 00 00 00 00 ];
interrupt-parent = <&mpic>;
phy-handle = <&phy1>;
tbi-handle = <&tbi1>;
phy-connection-type = "sgmii";
ptimer-handle = < &ptp_timer >;
queue-group@0 {
#address-cells = <1>;
#size-cells = <1>;
reg = <0xb2000 0x1000>;
rx-bit-map = <0xff>;
tx-bit-map = <0xff>;
interrupts = <31 2 32 2 33 2>;
};
};
dsa@0 {
compatible = "marvell,dsa";
#address-cells = <2>;
#size-cells = <0>;
dsa,ethernet = <&enet1>;
dsa,mii-bus = <&mii_bus0>;
switch@0 {
#address-cells = <1>;
#size-cells = <0>;
reg = <31 0>; /* Switch at SMI Add 0x1f */
port@0 {
reg = <0>;
label = "lan1";
};
port@1 {
reg = <1>;
label = "lan2";
};
port@9 {
reg = <9>;
label = "cpu";
};
};
};
I have disabled auto-negotiation in gianfar driver for fixed-link eth0
and associated TBI-PHY, so that SGMII link at 1Gb/ps can be forced with
switch port 9 GMII MAC.
Linux DSA driver is able to detect and configure switch. It discover
switch at external MDIO bus, DSA configure 'eth0' as master and 'lan1'
and 'lan2' as slave port.
After assigning IPs to eth0 (Switch) and eth1 ( with external PHY) like,
ifconfig eth0 up
ifconfig lan1 xx.xx.xx.x1 up
ifconfig lan2 xx.xx.xx.x2 up
ifconfig eth1 xx.xx.xx.x3 up
When I ping from any other host on network, only lan1 is able to
respond, because it was configured first. If any other port is pinged it
will be able to respond
only if 'lan1 is connected to network'. If ethernet cable from lan1 is
removed, rest of ports will not be able to respond, though they will be
receiving packets but
their TX counter will not increase.
It seems whichever port is configured first ( lan1 here ) will be able
to transmit packets. If eth1 ( connected to Phy not switch) is
configured first then only this
will be able to respond. All other ports then depends on eth1.
I tried to move 'eth1' to another mdio bus ( mdio@25000 ) by modifying
dts file above. But it seems 'PHY' and 'Switch' will only be detected if
they are on
mdio@24000.
What could be the reason of such strange behaviour ?
Why only one interface is controlling the bus its either eth0/lan0/lan1
or eth1. Rx counter of every interface increments but only one of these
have Tx counter incremented. If I disable switch ports ( lan0/lan1),
strangely eth1 also does not receive any packets ( though its Rx counter
increments but Tx is 0). Is it happening because dsa@0 also controlling
phy1 on same mdio bus ?
^ permalink raw reply
* Re: Bug: mv643xxx fails with highmem
From: Ezequiel Garcia @ 2015-01-20 13:42 UTC (permalink / raw)
To: Russell King - ARM Linux, David Miller, Linus Torvalds
Cc: Nimrod Andy, Fabio Estevam, netdev, fugang.duan
In-Reply-To: <20150120134104.GA12253@n2100.arm.linux.org.uk>
On 01/20/2015 10:41 AM, Russell King - ARM Linux wrote:
> Ping. What's happening on this?
>
> I guess we don't care about regressions in the Linux kernel?
>
Sorry, a holiday trip got in the middle of this.
I'll try to submit a fix this week.
--
Ezequiel García, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com
^ 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