Netdev List
 help / color / mirror / Atom feed
* [PATCH 6/8] ethoc: rework mdio read/write
From: Jonas Bonn @ 2010-11-25 12:30 UTC (permalink / raw)
  To: netdev; +Cc: Jonas Bonn
In-Reply-To: <1290688232-25142-1-git-send-email-jonas@southpole.se>

MDIO read and write were checking whether a timeout had expired to determine
whether to recheck the result of the MDIO operation.  Under heavy CPU usage,
however, it was possible for the timeout to expire before the routine got
around to be able to check a second time even, thus erroneousy returning an
-EBUSY.

This patch changes the the MDIO IO routines to try up to five times to complete
the operation before giving up, thus lessening the dependency on CPU load.

This resolves a problem whereby a ping flood would keep the CPU so busy that
the above problem would manifest itself; the MDIO command to check link status
would fail and the interface would erroneously be shut down.

Signed-off-by: Jonas Bonn <jonas@southpole.se>
---
 drivers/net/ethoc.c |   14 ++++++--------
 1 files changed, 6 insertions(+), 8 deletions(-)

diff --git a/drivers/net/ethoc.c b/drivers/net/ethoc.c
index 43431ff..f3048fa 100644
--- a/drivers/net/ethoc.c
+++ b/drivers/net/ethoc.c
@@ -611,13 +611,13 @@ static int ethoc_poll(struct napi_struct *napi, int budget)
 
 static int ethoc_mdio_read(struct mii_bus *bus, int phy, int reg)
 {
-	unsigned long timeout = jiffies + ETHOC_MII_TIMEOUT;
 	struct ethoc *priv = bus->priv;
+	int i;
 
 	ethoc_write(priv, MIIADDRESS, MIIADDRESS_ADDR(phy, reg));
 	ethoc_write(priv, MIICOMMAND, MIICOMMAND_READ);
 
-	while (time_before(jiffies, timeout)) {
+	for (i=0; i < 5; i++) {
 		u32 status = ethoc_read(priv, MIISTATUS);
 		if (!(status & MIISTATUS_BUSY)) {
 			u32 data = ethoc_read(priv, MIIRX_DATA);
@@ -625,8 +625,7 @@ static int ethoc_mdio_read(struct mii_bus *bus, int phy, int reg)
 			ethoc_write(priv, MIICOMMAND, 0);
 			return data;
 		}
-
-		schedule();
+		usleep_range(100,200);
 	}
 
 	return -EBUSY;
@@ -634,22 +633,21 @@ static int ethoc_mdio_read(struct mii_bus *bus, int phy, int reg)
 
 static int ethoc_mdio_write(struct mii_bus *bus, int phy, int reg, u16 val)
 {
-	unsigned long timeout = jiffies + ETHOC_MII_TIMEOUT;
 	struct ethoc *priv = bus->priv;
+	int i;
 
 	ethoc_write(priv, MIIADDRESS, MIIADDRESS_ADDR(phy, reg));
 	ethoc_write(priv, MIITX_DATA, val);
 	ethoc_write(priv, MIICOMMAND, MIICOMMAND_WRITE);
 
-	while (time_before(jiffies, timeout)) {
+	for (i=0; i < 5; i++) {
 		u32 stat = ethoc_read(priv, MIISTATUS);
 		if (!(stat & MIISTATUS_BUSY)) {
 			/* reset MII command register */
 			ethoc_write(priv, MIICOMMAND, 0);
 			return 0;
 		}
-
-		schedule();
+		usleep_range(100,200);
 	}
 
 	return -EBUSY;
-- 
1.7.1


^ permalink raw reply related

* [PATCH 8/8] ethoc: remove division from loops
From: Jonas Bonn @ 2010-11-25 12:30 UTC (permalink / raw)
  To: netdev; +Cc: Jonas Bonn
In-Reply-To: <1290688232-25142-1-git-send-email-jonas@southpole.se>

Calculating the BD entry using a modulus operation isn't optimal, especially
inside the loop.  This patch removes the modulus operations in favour of:

i)  simply checking for wrapping in the case of cur_rx
ii) forcing num_tx to be a power of two and using it to mask out the
    entry from cur_tx

The also prevents possible issues related overflow of the cur_rx and cur_tx
counters.

Signed-off-by: Jonas Bonn <jonas@southpole.se>
---
 drivers/net/ethoc.c |   17 +++++++++++++----
 1 files changed, 13 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethoc.c b/drivers/net/ethoc.c
index 93b50d6..b79d7e1 100644
--- a/drivers/net/ethoc.c
+++ b/drivers/net/ethoc.c
@@ -412,7 +412,7 @@ static int ethoc_rx(struct net_device *dev, int limit)
 		unsigned int entry;
 		struct ethoc_bd bd;
 
-		entry = priv->num_tx + (priv->cur_rx % priv->num_rx);
+		entry = priv->num_tx + priv->cur_rx;
 		ethoc_read_bd(priv, entry, &bd);
 		if (bd.stat & RX_BD_EMPTY) {
 			ethoc_ack_irq(priv, INT_MASK_RX);
@@ -456,7 +456,8 @@ static int ethoc_rx(struct net_device *dev, int limit)
 		bd.stat &= ~RX_BD_STATS;
 		bd.stat |=  RX_BD_EMPTY;
 		ethoc_write_bd(priv, entry, &bd);
-		priv->cur_rx++;
+		if (++priv->cur_rx == priv->num_rx)
+			priv->cur_rx = 0;
 	}
 
 	return count;
@@ -503,7 +504,7 @@ static int ethoc_tx(struct net_device *dev, int limit)
 	for (count = 0; count < limit; ++count) {
 		unsigned int entry;
 
-		entry = priv->dty_tx % priv->num_tx;
+		entry = priv->dty_tx & (priv->num_tx-1);
 
 		ethoc_read_bd(priv, entry, &bd);
 
@@ -1000,9 +1001,17 @@ static int __devinit ethoc_probe(struct platform_device *pdev)
 	/* calculate the number of TX/RX buffers, maximum 128 supported */
 	num_bd = min_t(unsigned int,
 		128, (netdev->mem_end - netdev->mem_start + 1) / ETHOC_BUFSIZ);
-	priv->num_tx = max(2, num_bd / 4);
+	if (num_bd < 4) {
+		ret = -ENODEV;
+		goto error;
+	}
+	/* num_tx must be a power of two */
+	priv->num_tx = rounddown_pow_of_two(num_bd >> 1);
 	priv->num_rx = num_bd - priv->num_tx;
 
+	dev_dbg(&pdev->dev, "ethoc: num_tx: %d num_rx: %d\n",
+		priv->num_tx, priv->num_rx);
+
 	priv->vma = devm_kzalloc(&pdev->dev, num_bd*sizeof(void*), GFP_KERNEL);
 	if (!priv->vma) {
 		ret = -ENOMEM;
-- 
1.7.1


^ permalink raw reply related

* [PATCH 5/8] ethoc: rework interrupt handling
From: Jonas Bonn @ 2010-11-25 12:30 UTC (permalink / raw)
  To: netdev; +Cc: Jonas Bonn
In-Reply-To: <1290688232-25142-1-git-send-email-jonas@southpole.se>

The old interrupt handling was incorrect in that it did not account for the
fact that the interrupt source bits get set irregardless of whether or not
their corresponding mask is set.  This patch fixes that by masking off the
source bits for masked interrupts.

Furthermore, the handling of transmission events is moved to the NAPI polling
handler alongside the reception handler, thus preventing a whole bunch of
interrupts during heavy traffic.

Signed-off-by: Jonas Bonn <jonas@southpole.se>
---
 drivers/net/ethoc.c |   76 +++++++++++++++++++++++++++++++++------------------
 1 files changed, 49 insertions(+), 27 deletions(-)

diff --git a/drivers/net/ethoc.c b/drivers/net/ethoc.c
index a12a07e..43431ff 100644
--- a/drivers/net/ethoc.c
+++ b/drivers/net/ethoc.c
@@ -495,29 +495,42 @@ static int ethoc_update_tx_stats(struct ethoc *dev, struct ethoc_bd *bd)
 	return 0;
 }
 
-static void ethoc_tx(struct net_device *dev)
+static int ethoc_tx(struct net_device *dev, int limit)
 {
 	struct ethoc *priv = netdev_priv(dev);
+	int count;
+	struct ethoc_bd bd;
 
-	spin_lock(&priv->lock);
+	for (count = 0; count < limit; ++count) {
+		unsigned int entry;
 
-	while (priv->dty_tx != priv->cur_tx) {
-		unsigned int entry = priv->dty_tx % priv->num_tx;
-		struct ethoc_bd bd;
+		entry = priv->dty_tx % priv->num_tx;
 
 		ethoc_read_bd(priv, entry, &bd);
-		if (bd.stat & TX_BD_READY)
-			break;
 
-		entry = (++priv->dty_tx) % priv->num_tx;
+		if (bd.stat & TX_BD_READY || (priv->dty_tx == priv->cur_tx)) {
+			ethoc_ack_irq(priv, INT_MASK_TX);
+			/* If interrupt came in between reading in the BD
+			 * and clearing the interrupt source, then we risk
+			 * missing the event as the TX interrupt won't trigger
+			 * right away when we reenable it; hence, check
+			 * BD_EMPTY here again to make sure there isn't such an
+			 * event pending...
+			 */
+			ethoc_read_bd(priv, entry, &bd);
+			if (bd.stat & TX_BD_READY ||
+			    (priv->dty_tx == priv->cur_tx))
+				break;
+		}
+
 		(void)ethoc_update_tx_stats(priv, &bd);
+		priv->dty_tx++;
 	}
 
 	if ((priv->cur_tx - priv->dty_tx) <= (priv->num_tx / 2))
 		netif_wake_queue(dev);
 
-	ethoc_ack_irq(priv, INT_MASK_TX);
-	spin_unlock(&priv->lock);
+	return count;
 }
 
 static irqreturn_t ethoc_interrupt(int irq, void *dev_id)
@@ -525,32 +538,38 @@ static irqreturn_t ethoc_interrupt(int irq, void *dev_id)
 	struct net_device *dev = dev_id;
 	struct ethoc *priv = netdev_priv(dev);
 	u32 pending;
-
-	ethoc_disable_irq(priv, INT_MASK_ALL);
+	u32 mask;
+
+	/* Figure out what triggered the interrupt...
+	 * The tricky bit here is that the interrupt source bits get
+	 * set in INT_SOURCE for an event irregardless of whether that
+	 * event is masked or not.  Thus, in order to figure out what
+	 * triggered the interrupt, we need to remove the sources
+	 * for all events that are currently masked.  This behaviour
+	 * is not particularly well documented but reasonable...
+	 */
+	mask = ethoc_read(priv, INT_MASK);
 	pending = ethoc_read(priv, INT_SOURCE);
+	pending &= mask;
+
 	if (unlikely(pending == 0)) {
-		ethoc_enable_irq(priv, INT_MASK_ALL);
 		return IRQ_NONE;
 	}
 
 	ethoc_ack_irq(priv, pending);
 
+	/* We always handle the dropped packet interrupt */
 	if (pending & INT_MASK_BUSY) {
 		dev_err(&dev->dev, "packet dropped\n");
 		dev->stats.rx_dropped++;
 	}
 
-	if (pending & INT_MASK_RX) {
-		if (napi_schedule_prep(&priv->napi))
-			__napi_schedule(&priv->napi);
-	} else {
-		ethoc_enable_irq(priv, INT_MASK_RX);
+	/* Handle receive/transmit event by switching to polling */
+	if (pending & (INT_MASK_TX | INT_MASK_RX)) {
+		ethoc_disable_irq(priv, INT_MASK_TX | INT_MASK_RX);
+		napi_schedule(&priv->napi);
 	}
 
-	if (pending & INT_MASK_TX)
-		ethoc_tx(dev);
-
-	ethoc_enable_irq(priv, INT_MASK_ALL & ~INT_MASK_RX);
 	return IRQ_HANDLED;
 }
 
@@ -576,15 +595,18 @@ static int ethoc_get_mac_address(struct net_device *dev, void *addr)
 static int ethoc_poll(struct napi_struct *napi, int budget)
 {
 	struct ethoc *priv = container_of(napi, struct ethoc, napi);
-	int work_done = 0;
+	int rx_work_done = 0;
+	int tx_work_done = 0;
+
+	rx_work_done = ethoc_rx(priv->netdev, budget);
+	tx_work_done = ethoc_tx(priv->netdev, budget);
 
-	work_done = ethoc_rx(priv->netdev, budget);
-	if (work_done < budget) {
+	if (rx_work_done < budget && tx_work_done < budget) {
 		napi_complete(napi);
-		ethoc_enable_irq(priv, INT_MASK_RX);
+		ethoc_enable_irq(priv, INT_MASK_TX | INT_MASK_RX);
 	}
 
-	return work_done;
+	return rx_work_done;
 }
 
 static int ethoc_mdio_read(struct mii_bus *bus, int phy, int reg)
-- 
1.7.1


^ permalink raw reply related

* [PATCH 7/8] ethoc: fix function return type
From: Jonas Bonn @ 2010-11-25 12:30 UTC (permalink / raw)
  To: netdev; +Cc: Jonas Bonn
In-Reply-To: <1290688232-25142-1-git-send-email-jonas@southpole.se>

update_ethoc_tx_stats doesn't need to return anything so make its return
type void in order to avoid an unnecessary cast when the function is called.

Signed-off-by: Jonas Bonn <jonas@southpole.se>
---
 drivers/net/ethoc.c |    5 ++---
 1 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethoc.c b/drivers/net/ethoc.c
index f3048fa..93b50d6 100644
--- a/drivers/net/ethoc.c
+++ b/drivers/net/ethoc.c
@@ -462,7 +462,7 @@ static int ethoc_rx(struct net_device *dev, int limit)
 	return count;
 }
 
-static int ethoc_update_tx_stats(struct ethoc *dev, struct ethoc_bd *bd)
+static void ethoc_update_tx_stats(struct ethoc *dev, struct ethoc_bd *bd)
 {
 	struct net_device *netdev = dev->netdev;
 
@@ -492,7 +492,6 @@ static int ethoc_update_tx_stats(struct ethoc *dev, struct ethoc_bd *bd)
 	netdev->stats.collisions += (bd->stat >> 4) & 0xf;
 	netdev->stats.tx_bytes += bd->stat >> 16;
 	netdev->stats.tx_packets++;
-	return 0;
 }
 
 static int ethoc_tx(struct net_device *dev, int limit)
@@ -523,7 +522,7 @@ static int ethoc_tx(struct net_device *dev, int limit)
 				break;
 		}
 
-		(void)ethoc_update_tx_stats(priv, &bd);
+		ethoc_update_tx_stats(priv, &bd);
 		priv->dty_tx++;
 	}
 
-- 
1.7.1


^ permalink raw reply related

* [PATCH 3/8] ethoc: enable interrupts after napi_complete
From: Jonas Bonn @ 2010-11-25 12:30 UTC (permalink / raw)
  To: netdev; +Cc: Adam Edvardsson, Jonas Bonn
In-Reply-To: <1290688232-25142-1-git-send-email-jonas@southpole.se>

From: Adam Edvardsson <adam.edvardsson@orsoc.se>

Occasionally, it seems that some race is causing the interrupts to not be
reenabled otherwise with the end result that networking just stops working.
Enabling interrupts after calling napi_complete is more in line with what
other drivers do.

Signed-off-by: Jonas Bonn <jonas@southpole.se>
---
 drivers/net/ethoc.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/net/ethoc.c b/drivers/net/ethoc.c
index e9e712e..db444a7 100644
--- a/drivers/net/ethoc.c
+++ b/drivers/net/ethoc.c
@@ -569,8 +569,8 @@ static int ethoc_poll(struct napi_struct *napi, int budget)
 
 	work_done = ethoc_rx(priv->netdev, budget);
 	if (work_done < budget) {
-		ethoc_enable_irq(priv, INT_MASK_RX);
 		napi_complete(napi);
+		ethoc_enable_irq(priv, INT_MASK_RX);
 	}
 
 	return work_done;
-- 
1.7.1


^ permalink raw reply related

* [PATCH 4/8] ethoc: Double check pending RX packet
From: Jonas Bonn @ 2010-11-25 12:30 UTC (permalink / raw)
  To: netdev; +Cc: Jonas Bonn
In-Reply-To: <1290688232-25142-1-git-send-email-jonas@southpole.se>

An interrupt may occur between checking bd.stat and clearing the
interrupt source register which would result in the packet going totally
unnoticed as the interrupt will be missed.  Double check bd.stat after
clearing the interrupt source register to guard against such an
occurrence.

Signed-off-by: Jonas Bonn <jonas@southpole.se>
---
 drivers/net/ethoc.c |   15 +++++++++++++--
 1 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethoc.c b/drivers/net/ethoc.c
index db444a7..a12a07e 100644
--- a/drivers/net/ethoc.c
+++ b/drivers/net/ethoc.c
@@ -414,8 +414,19 @@ static int ethoc_rx(struct net_device *dev, int limit)
 
 		entry = priv->num_tx + (priv->cur_rx % priv->num_rx);
 		ethoc_read_bd(priv, entry, &bd);
-		if (bd.stat & RX_BD_EMPTY)
-			break;
+		if (bd.stat & RX_BD_EMPTY) {
+			ethoc_ack_irq(priv, INT_MASK_RX);
+			/* If packet (interrupt) came in between checking
+			 * BD_EMTPY and clearing the interrupt source, then we
+			 * risk missing the packet as the RX interrupt won't
+			 * trigger right away when we reenable it; hence, check
+			 * BD_EMTPY here again to make sure there isn't such a
+			 * packet waiting for us...
+			 */
+			ethoc_read_bd(priv, entry, &bd);
+			if (bd.stat & RX_BD_EMPTY)
+				break;
+		}
 
 		if (ethoc_update_rx_stats(priv, &bd) == 0) {
 			int size = bd.stat >> 16;
-- 
1.7.1


^ permalink raw reply related

* [PATCH 1/8] ethoc: Add device tree configuration
From: Jonas Bonn @ 2010-11-25 12:30 UTC (permalink / raw)
  To: netdev; +Cc: Jonas Bonn
In-Reply-To: <1290688232-25142-1-git-send-email-jonas@southpole.se>

This patch adds the ability to describe ethernet devices via a flattened
device tree.  As device tree remains an optional feature, these bits all
need to be guarded by CONFIG_OF ifdefs.

MAC address is settable via the device tree parameter "local-mac-address";
however, the selection of the phy id is limited to probing, for now.

Signed-off-by: Jonas Bonn <jonas@southpole.se>
---
 drivers/net/ethoc.c |   32 ++++++++++++++++++++++++++++++--
 1 files changed, 30 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethoc.c b/drivers/net/ethoc.c
index c5a2fe0..9ea3c54 100644
--- a/drivers/net/ethoc.c
+++ b/drivers/net/ethoc.c
@@ -19,6 +19,7 @@
 #include <linux/platform_device.h>
 #include <linux/sched.h>
 #include <linux/slab.h>
+#include <linux/of.h>
 #include <net/ethoc.h>
 
 static int buffer_size = 0x8000; /* 32 KBytes */
@@ -982,10 +983,23 @@ static int __devinit ethoc_probe(struct platform_device *pdev)
 
 	/* Allow the platform setup code to pass in a MAC address. */
 	if (pdev->dev.platform_data) {
-		struct ethoc_platform_data *pdata =
-			(struct ethoc_platform_data *)pdev->dev.platform_data;
+		struct ethoc_platform_data *pdata = pdev->dev.platform_data;
 		memcpy(netdev->dev_addr, pdata->hwaddr, IFHWADDRLEN);
 		priv->phy_id = pdata->phy_id;
+	} else {
+		priv->phy_id = -1;
+
+#ifdef CONFIG_OF
+		{
+		const uint8_t* mac;
+
+		mac = of_get_property(pdev->dev.of_node,
+				      "local-mac-address",
+				      NULL);
+		if (mac)
+			memcpy(netdev->dev_addr, mac, IFHWADDRLEN);
+		}
+#endif
 	}
 
 	/* Check that the given MAC address is valid. If it isn't, read the
@@ -1113,6 +1127,16 @@ static int ethoc_resume(struct platform_device *pdev)
 # define ethoc_resume  NULL
 #endif
 
+#ifdef CONFIG_OF
+static struct of_device_id ethoc_match[] = {
+	{
+		.compatible = "opencores,ethoc",
+	},
+	{},
+};
+MODULE_DEVICE_TABLE(of, ethoc_match);
+#endif
+
 static struct platform_driver ethoc_driver = {
 	.probe   = ethoc_probe,
 	.remove  = __devexit_p(ethoc_remove),
@@ -1120,6 +1144,10 @@ static struct platform_driver ethoc_driver = {
 	.resume  = ethoc_resume,
 	.driver  = {
 		.name = "ethoc",
+		.owner = THIS_MODULE,
+#ifdef CONFIG_OF
+		.of_match_table = ethoc_match,
+#endif
 	},
 };
 
-- 
1.7.1


^ permalink raw reply related

* [PATCH 2/8] ethoc: remove unused spinlock
From: Jonas Bonn @ 2010-11-25 12:30 UTC (permalink / raw)
  To: netdev; +Cc: Jonas Bonn
In-Reply-To: <1290688232-25142-1-git-send-email-jonas@southpole.se>

Signed-off-by: Jonas Bonn <jonas@southpole.se>
---
 drivers/net/ethoc.c |    3 ---
 1 files changed, 0 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethoc.c b/drivers/net/ethoc.c
index 9ea3c54..e9e712e 100644
--- a/drivers/net/ethoc.c
+++ b/drivers/net/ethoc.c
@@ -185,7 +185,6 @@ MODULE_PARM_DESC(buffer_size, "DMA buffer allocation size");
  * @netdev:	pointer to network device structure
  * @napi:	NAPI structure
  * @msg_enable:	device state flags
- * @rx_lock:	receive lock
  * @lock:	device lock
  * @phy:	attached PHY
  * @mdio:	MDIO bus for PHY access
@@ -210,7 +209,6 @@ struct ethoc {
 	struct napi_struct napi;
 	u32 msg_enable;
 
-	spinlock_t rx_lock;
 	spinlock_t lock;
 
 	struct phy_device *phy;
@@ -1060,7 +1058,6 @@ static int __devinit ethoc_probe(struct platform_device *pdev)
 	/* setup NAPI */
 	netif_napi_add(netdev, &priv->napi, ethoc_poll, 64);
 
-	spin_lock_init(&priv->rx_lock);
 	spin_lock_init(&priv->lock);
 
 	ret = register_netdev(netdev);
-- 
1.7.1


^ permalink raw reply related

* ethoc driver changes (version 2)
From: Jonas Bonn @ 2010-11-25 12:30 UTC (permalink / raw)
  To: netdev

This series incorporates the changes requested in the review of the original
set.  The patch to "prevent overflow of cur_rx" has been dropped
completely and the comments concerning the usage of division in the
calculation of the BD entry have been incorporated into a new patch, 
number 8 "remove division from loops"; this new patch should take care of
the potential overflow issues that existed previously, too.

/Jonas


^ permalink raw reply

* [GIT PULL net-2.6] vhost-net: rcu fixup
From: Michael S. Tsirkin @ 2010-11-25 12:23 UTC (permalink / raw)
  To: David Miller; +Cc: kvm, virtualization, netdev, linux-kernel

Please merge the following fix for 2.6.36.
Thanks!

The following changes since commit a27e13d370415add3487949c60810e36069a23a6:

  econet: fix CVE-2010-3848 (2010-11-24 11:51:47 -0800)

are available in the git repository at:
  git://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git vhost-net

Michael S. Tsirkin (1):
      vhost/net: fix rcu check usage

 drivers/vhost/net.c |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)
-- 
MST

^ permalink raw reply

* Re: [PATCH] hso: fix disable_net
From: Sergei Shtylyov @ 2010-11-25 12:17 UTC (permalink / raw)
  To: Filip Aben; +Cc: davem, linux-usb, netdev, jhovold, pki, j.dumon
In-Reply-To: <1290627352.1739.19.camel@filip-linux>

On 24-11-2010 22:35, Filip Aben wrote:

> The HSO driver incorrectly creates a serial device instead of a net
> device when disable_net is set. It shouldn't create anything for the
> network interface.

> Signed-off-by: Filip Aben <f.aben@option.com>
> ---

> diff --git a/drivers/net/usb/hso.c b/drivers/net/usb/hso.c
> index b154a94..b05c235 100644
> --- a/drivers/net/usb/hso.c
> +++ b/drivers/net/usb/hso.c
> @@ -2994,10 +2994,10 @@ static int hso_probe(struct usb_interface *interface,
>
>   	case HSO_INTF_BULK:
>   		/* It's a regular bulk interface */
> -		if (((port_spec&  HSO_PORT_MASK) == HSO_PORT_NETWORK)&&
> -		    !disable_net)
> +		if ((port_spec&  HSO_PORT_MASK) == HSO_PORT_NETWORK) {
> +		    if(!disable_net)
>   			hso_dev = hso_create_net_device(interface, port_spec);
> -		else
> +		} else

    There should now be {} on the *else* branch too, according to CodingStyle.

>   			hso_dev =
>   			    hso_create_bulk_serial_device(interface, port_spec);
>   		if (!hso_dev)

WBR, Sergei

^ permalink raw reply

* [PATCH net-next 5/5] X25 remove bkl in call user data length ioctl
From: Andrew Hendry @ 2010-11-25 12:18 UTC (permalink / raw)
  To: netdev


Signed-off-by: Andrew Hendry <andrew.hendry@gmail.com>

---
 net/x25/af_x25.c |   11 ++++++-----
 1 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/net/x25/af_x25.c b/net/x25/af_x25.c
index 8cfc419..ad96ee9 100644
--- a/net/x25/af_x25.c
+++ b/net/x25/af_x25.c
@@ -1562,19 +1562,20 @@ out_dtefac_release:
 		case SIOCX25SCUDMATCHLEN: {
 			struct x25_subaddr sub_addr;
 			rc = -EINVAL;
-			lock_kernel();
+			lock_sock(sk);
 			if(sk->sk_state != TCP_CLOSE)
-				break;
+				goto out_cud_release;
 			rc = -EFAULT;
 			if (copy_from_user(&sub_addr, argp,
 					sizeof(sub_addr)))
-				break;
+				goto out_cud_release;
 			rc = -EINVAL;
 			if(sub_addr.cudmatchlength > X25_MAX_CUD_LEN)
-				break;
+				goto out_cud_release;
 			x25->cudmatchlength = sub_addr.cudmatchlength;
-			unlock_kernel();
 			rc = 0;
+out_cud_release:
+			release_sock(sk);
 			break;
 		}
 
-- 
1.7.1




^ permalink raw reply related

* [PATCH net-next 4/5] X25 remove bkl from causediag ioctls
From: Andrew Hendry @ 2010-11-25 12:18 UTC (permalink / raw)
  To: netdev


Signed-off-by: Andrew Hendry <andrew.hendry@gmail.com>

---
 net/x25/af_x25.c |   15 +++++++--------
 1 files changed, 7 insertions(+), 8 deletions(-)

diff --git a/net/x25/af_x25.c b/net/x25/af_x25.c
index e2eea0a..8cfc419 100644
--- a/net/x25/af_x25.c
+++ b/net/x25/af_x25.c
@@ -1538,23 +1538,22 @@ out_dtefac_release:
 		}
 
 		case SIOCX25GCAUSEDIAG: {
-			struct x25_causediag causediag;
-			lock_kernel();
-			causediag = x25->causediag;
-			rc = copy_to_user(argp, &causediag,
-					  sizeof(causediag)) ? -EFAULT : 0;
-			unlock_kernel();
+			lock_sock(sk);
+			rc = copy_to_user(argp, &x25->causediag,
+					sizeof(x25->causediag))
+					? -EFAULT : 0;
+			release_sock(sk);
 			break;
 		}
 
 		case SIOCX25SCAUSEDIAG: {
 			struct x25_causediag causediag;
 			rc = -EFAULT;
-			lock_kernel();
 			if (copy_from_user(&causediag, argp, sizeof(causediag)))
 				break;
+			lock_sock(sk);
 			x25->causediag = causediag;
-			unlock_kernel();
+			release_sock(sk);
 			rc = 0;
 			break;
 
-- 
1.7.1




^ permalink raw reply related

* [PATCH net-next 3/5] X25 remove bkl from calluserdata ioctls
From: Andrew Hendry @ 2010-11-25 12:18 UTC (permalink / raw)
  To: netdev


Signed-off-by: Andrew Hendry <andrew.hendry@gmail.com>

---
 net/x25/af_x25.c |   14 +++++++-------
 1 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/net/x25/af_x25.c b/net/x25/af_x25.c
index 2518efa..e2eea0a 100644
--- a/net/x25/af_x25.c
+++ b/net/x25/af_x25.c
@@ -1512,11 +1512,11 @@ out_dtefac_release:
 		}
 
 		case SIOCX25GCALLUSERDATA: {
-			struct x25_calluserdata cud = x25->calluserdata;
-			lock_kernel();
-			rc = copy_to_user(argp, &cud,
-					  sizeof(cud)) ? -EFAULT : 0;
-			unlock_kernel();
+			lock_sock(sk);
+			rc = copy_to_user(argp, &x25->calluserdata,
+					sizeof(x25->calluserdata))
+					? -EFAULT : 0;
+			release_sock(sk);
 			break;
 		}
 
@@ -1524,15 +1524,15 @@ out_dtefac_release:
 			struct x25_calluserdata calluserdata;
 
 			rc = -EFAULT;
-			lock_kernel();
 			if (copy_from_user(&calluserdata, argp,
 					   sizeof(calluserdata)))
 				break;
 			rc = -EINVAL;
 			if (calluserdata.cudlength > X25_MAX_CUD_LEN)
 				break;
+			lock_sock(sk);
 			x25->calluserdata = calluserdata;
-			unlock_kernel();
+			release_sock(sk);
 			rc = 0;
 			break;
 		}
-- 
1.7.1




^ permalink raw reply related

* [PATCH net-next 2/5] X25 remove bkl in facility ioctls
From: Andrew Hendry @ 2010-11-25 12:18 UTC (permalink / raw)
  To: netdev


Signed-off-by: Andrew Hendry <andrew.hendry@gmail.com>

---
 net/x25/af_x25.c |   48 +++++++++++++++++++++++++-----------------------
 1 files changed, 25 insertions(+), 23 deletions(-)

diff --git a/net/x25/af_x25.c b/net/x25/af_x25.c
index 45be72c..2518efa 100644
--- a/net/x25/af_x25.c
+++ b/net/x25/af_x25.c
@@ -1424,34 +1424,34 @@ static int x25_ioctl(struct socket *sock, unsigned int cmd, unsigned long arg)
 			rc = x25_subscr_ioctl(cmd, argp);
 			break;
 		case SIOCX25GFACILITIES: {
-			struct x25_facilities fac = x25->facilities;
-			lock_kernel();
-			rc = copy_to_user(argp, &fac,
-					  sizeof(fac)) ? -EFAULT : 0;
-			unlock_kernel();
+			lock_sock(sk);
+			rc = copy_to_user(argp, &x25->facilities,
+						sizeof(x25->facilities))
+						? -EFAULT : 0;
+			release_sock(sk);
 			break;
 		}
 
 		case SIOCX25SFACILITIES: {
 			struct x25_facilities facilities;
 			rc = -EFAULT;
-			lock_kernel();
 			if (copy_from_user(&facilities, argp,
 					   sizeof(facilities)))
 				break;
 			rc = -EINVAL;
+			lock_sock(sk);
 			if (sk->sk_state != TCP_LISTEN &&
 			    sk->sk_state != TCP_CLOSE)
-				break;
+				goto out_fac_release;
 			if (facilities.pacsize_in < X25_PS16 ||
 			    facilities.pacsize_in > X25_PS4096)
-				break;
+				goto out_fac_release;
 			if (facilities.pacsize_out < X25_PS16 ||
 			    facilities.pacsize_out > X25_PS4096)
-				break;
+				goto out_fac_release;
 			if (facilities.winsize_in < 1 ||
 			    facilities.winsize_in > 127)
-				break;
+				goto out_fac_release;
 			if (facilities.throughput) {
 				int out = facilities.throughput & 0xf0;
 				int in  = facilities.throughput & 0x0f;
@@ -1459,27 +1459,28 @@ static int x25_ioctl(struct socket *sock, unsigned int cmd, unsigned long arg)
 					facilities.throughput |=
 						X25_DEFAULT_THROUGHPUT << 4;
 				else if (out < 0x30 || out > 0xD0)
-					break;
+					goto out_fac_release;
 				if (!in)
 					facilities.throughput |=
 						X25_DEFAULT_THROUGHPUT;
 				else if (in < 0x03 || in > 0x0D)
-					break;
+					goto out_fac_release;
 			}
 			if (facilities.reverse &&
 				(facilities.reverse & 0x81) != 0x81)
-				break;
+				goto out_fac_release;
 			x25->facilities = facilities;
 			rc = 0;
-			unlock_kernel();
+out_fac_release:
+			release_sock(sk);
 			break;
 		}
 
 		case SIOCX25GDTEFACILITIES: {
-			lock_kernel();
+			lock_sock(sk);
 			rc = copy_to_user(argp, &x25->dte_facilities,
 						sizeof(x25->dte_facilities));
-			unlock_kernel();
+			release_sock(sk);
 			if (rc)
 				rc = -EFAULT;
 			break;
@@ -1488,24 +1489,25 @@ static int x25_ioctl(struct socket *sock, unsigned int cmd, unsigned long arg)
 		case SIOCX25SDTEFACILITIES: {
 			struct x25_dte_facilities dtefacs;
 			rc = -EFAULT;
-			lock_kernel();
 			if (copy_from_user(&dtefacs, argp, sizeof(dtefacs)))
 				break;
 			rc = -EINVAL;
+			lock_sock(sk);
 			if (sk->sk_state != TCP_LISTEN &&
 					sk->sk_state != TCP_CLOSE)
-				break;
+				goto out_dtefac_release;
 			if (dtefacs.calling_len > X25_MAX_AE_LEN)
-				break;
+				goto out_dtefac_release;
 			if (dtefacs.calling_ae == NULL)
-				break;
+				goto out_dtefac_release;
 			if (dtefacs.called_len > X25_MAX_AE_LEN)
-				break;
+				goto out_dtefac_release;
 			if (dtefacs.called_ae == NULL)
-				break;
+				goto out_dtefac_release;
 			x25->dte_facilities = dtefacs;
 			rc = 0;
-			unlock_kernel();
+out_dtefac_release:
+			release_sock(sk);
 			break;
 		}
 
-- 
1.7.1




^ permalink raw reply related

* Re: [PATCH] hso: fix disable_net
From: Sergei Shtylyov @ 2010-11-25 12:16 UTC (permalink / raw)
  To: Filip Aben; +Cc: davem, linux-usb, netdev, jhovold, pki, j.dumon
In-Reply-To: <1290627352.1739.19.camel@filip-linux>

Hello.

On 24-11-2010 22:35, Filip Aben wrote:

> The HSO driver incorrectly creates a serial device instead of a net
> device when disable_net is set. It shouldn't create anything for the
> network interface.

> Signed-off-by: Filip Aben <f.aben@option.com>
> ---

> diff --git a/drivers/net/usb/hso.c b/drivers/net/usb/hso.c
> index b154a94..b05c235 100644
> --- a/drivers/net/usb/hso.c
> +++ b/drivers/net/usb/hso.c
> @@ -2994,10 +2994,10 @@ static int hso_probe(struct usb_interface *interface,
>
>   	case HSO_INTF_BULK:
>   		/* It's a regular bulk interface */
> -		if (((port_spec&  HSO_PORT_MASK) == HSO_PORT_NETWORK)&&
> -		    !disable_net)
> +		if ((port_spec&  HSO_PORT_MASK) == HSO_PORT_NETWORK) {
> +		    if(!disable_net)

    Don't use spaces for indentation.

WBR, Sergei

^ permalink raw reply

* [PATCH net-next 1/5] X25 remove bkl in subscription ioctls
From: Andrew Hendry @ 2010-11-25 12:18 UTC (permalink / raw)
  To: netdev


Signed-off-by: Andrew Hendry <andrew.hendry@gmail.com>

---
 include/net/x25.h  |    2 ++
 net/x25/af_x25.c   |   12 ++++--------
 net/x25/x25_link.c |    8 ++++++--
 3 files changed, 12 insertions(+), 10 deletions(-)

diff --git a/include/net/x25.h b/include/net/x25.h
index 1479cb4..a06119a 100644
--- a/include/net/x25.h
+++ b/include/net/x25.h
@@ -315,6 +315,8 @@ extern struct list_head x25_route_list;
 extern rwlock_t x25_route_list_lock;
 extern struct list_head x25_forward_list;
 extern rwlock_t x25_forward_list_lock;
+extern struct list_head x25_neigh_list;
+extern rwlock_t x25_neigh_list_lock;
 
 extern int x25_proc_init(void);
 extern void x25_proc_exit(void);
diff --git a/net/x25/af_x25.c b/net/x25/af_x25.c
index 2351ace..45be72c 100644
--- a/net/x25/af_x25.c
+++ b/net/x25/af_x25.c
@@ -1415,17 +1415,13 @@ static int x25_ioctl(struct socket *sock, unsigned int cmd, unsigned long arg)
 			rc = x25_route_ioctl(cmd, argp);
 			break;
 		case SIOCX25GSUBSCRIP:
-			lock_kernel();
 			rc = x25_subscr_ioctl(cmd, argp);
-			unlock_kernel();
 			break;
 		case SIOCX25SSUBSCRIP:
 			rc = -EPERM;
 			if (!capable(CAP_NET_ADMIN))
 				break;
-			lock_kernel();
 			rc = x25_subscr_ioctl(cmd, argp);
-			unlock_kernel();
 			break;
 		case SIOCX25GFACILITIES: {
 			struct x25_facilities fac = x25->facilities;
@@ -1646,16 +1642,20 @@ static int compat_x25_subscr_ioctl(unsigned int cmd,
 	dev_put(dev);
 
 	if (cmd == SIOCX25GSUBSCRIP) {
+		read_lock_bh(&x25_neigh_list_lock);
 		x25_subscr.extended = nb->extended;
 		x25_subscr.global_facil_mask = nb->global_facil_mask;
+		read_unlock_bh(&x25_neigh_list_lock);
 		rc = copy_to_user(x25_subscr32, &x25_subscr,
 				sizeof(*x25_subscr32)) ? -EFAULT : 0;
 	} else {
 		rc = -EINVAL;
 		if (x25_subscr.extended == 0 || x25_subscr.extended == 1) {
 			rc = 0;
+			write_lock_bh(&x25_neigh_list_lock);
 			nb->extended = x25_subscr.extended;
 			nb->global_facil_mask = x25_subscr.global_facil_mask;
+			write_unlock_bh(&x25_neigh_list_lock);
 		}
 	}
 	x25_neigh_put(nb);
@@ -1711,17 +1711,13 @@ static int compat_x25_ioctl(struct socket *sock, unsigned int cmd,
 		rc = x25_route_ioctl(cmd, argp);
 		break;
 	case SIOCX25GSUBSCRIP:
-		lock_kernel();
 		rc = compat_x25_subscr_ioctl(cmd, argp);
-		unlock_kernel();
 		break;
 	case SIOCX25SSUBSCRIP:
 		rc = -EPERM;
 		if (!capable(CAP_NET_ADMIN))
 			break;
-		lock_kernel();
 		rc = compat_x25_subscr_ioctl(cmd, argp);
-		unlock_kernel();
 		break;
 	case SIOCX25GFACILITIES:
 	case SIOCX25SFACILITIES:
diff --git a/net/x25/x25_link.c b/net/x25/x25_link.c
index 73e7b95..4c81f6a 100644
--- a/net/x25/x25_link.c
+++ b/net/x25/x25_link.c
@@ -31,8 +31,8 @@
 #include <linux/init.h>
 #include <net/x25.h>
 
-static LIST_HEAD(x25_neigh_list);
-static DEFINE_RWLOCK(x25_neigh_list_lock);
+LIST_HEAD(x25_neigh_list);
+DEFINE_RWLOCK(x25_neigh_list_lock);
 
 static void x25_t20timer_expiry(unsigned long);
 
@@ -360,16 +360,20 @@ int x25_subscr_ioctl(unsigned int cmd, void __user *arg)
 	dev_put(dev);
 
 	if (cmd == SIOCX25GSUBSCRIP) {
+		read_lock_bh(&x25_neigh_list_lock);
 		x25_subscr.extended	     = nb->extended;
 		x25_subscr.global_facil_mask = nb->global_facil_mask;
+		read_unlock_bh(&x25_neigh_list_lock);
 		rc = copy_to_user(arg, &x25_subscr,
 				  sizeof(x25_subscr)) ? -EFAULT : 0;
 	} else {
 		rc = -EINVAL;
 		if (!(x25_subscr.extended && x25_subscr.extended != 1)) {
 			rc = 0;
+			write_lock_bh(&x25_neigh_list_lock);
 			nb->extended	     = x25_subscr.extended;
 			nb->global_facil_mask = x25_subscr.global_facil_mask;
+			write_unlock_bh(&x25_neigh_list_lock);
 		}
 	}
 	x25_neigh_put(nb);
-- 
1.7.1




^ permalink raw reply related

* Re: [PATCH net-next-2.6 v3] can: Topcliff: PCH_CAN driver: Add Flow control,
From: Marc Kleine-Budde @ 2010-11-25 12:08 UTC (permalink / raw)
  To: Tomoya MORINAGA
  Cc: andrew.chih.howe.khor-ral2JQCrhuEAvxtiuMwx3w, Samuel Ortiz,
	margie.foster-ral2JQCrhuEAvxtiuMwx3w,
	netdev-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	yong.y.wang-ral2JQCrhuEAvxtiuMwx3w,
	socketcan-core-0fE9KPoRgkgATYTw5x5z8w,
	kok.howg.ewe-ral2JQCrhuEAvxtiuMwx3w,
	joel.clark-ral2JQCrhuEAvxtiuMwx3w, qi.wang-ral2JQCrhuEAvxtiuMwx3w,
	David S. Miller, Christian Pellegrin, Wolfgang Grandegger
In-Reply-To: <001b01cb8c98$d06aaa00$66f8800a-a06+6cuVnkTSQfdrb5gaxUEOCMrvLtNR@public.gmane.org>


[-- Attachment #1.1: Type: text/plain, Size: 2247 bytes --]

On 11/25/2010 01:03 PM, Tomoya MORINAGA wrote:
> On Wednesday, November 24, 2010 9:34 PM, Marc Kleine-Budde wrote :
>> On 11/24/2010 01:09 AM, Tomoya MORINAGA wrote:
>>> On Monday, November 22, 2010 5:27 PM, Marc Kleine-Budde wrote:
>>>>>>>> Still we have the busy waiting in the TX path. Maybe you can move the
>>>>>>>> waiting before accessing the if[1] and remove the busy waiting here.
>>>>>>> I can't understand your saying.
>>>>>>> For transmitting data, calling pch_can_rw_msg_obj is mandatory.
>>>>>> Yes, but the busy wait is not needed. It should be enough to do the
>>>>>> busy-waiting _before_ accessing the if[1].
>>>>>
>>>>> Do you mean we should create other pch_can_rw_msg_obj which doesn't have busy wait ?
>>>> ACK, and this non busy waiting is use in the TX path. But you add a busy
>>>> wait only function before accessing the if[1] in the TX path.
>>>
>>> The "busy waiting" of pch_can_rw_msg_obj is for next processing accesses to Message object.
>>> If deleting this busy waiting, next processing can access to Message object, regardless previous transfer doesn't
>>> complete yet.
>>> Thus, I think, the "busy waiting" is necessary.
>>
>> Yes, it's necessary, but not where it is done currently.
>> Let me outline how I think the TX path should look like:
>>
>> pch_xmit() {
>> take_care_about_flow_control();
>> prepare_can_frame_to_be_copied_to_tx_if();
>>
>> /* most likely we don't have to wait here */
>> wait_until_tx_if_is_ready();
>>
>> copy_can_frame_to_tx_if();
>>
>> /* trigger tx in hardware */
>> send_tx_if_but_dont_do_busywait();
>>
>> /* tx_if is busy now, but before we access it, we'll check tx_if is ready */
>> }
> 
> This Tx path also has Read-Modify-Write for MessageRAM access.
> Do you mean Tx path shouldn't have Read-Modify-Write ?

Why do you Read-Modify-Write for TX? Naively speaking you just need to
push your Data into a Mail/IF/Whatever and push the send button.

Marc

-- 
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]

[-- Attachment #2: Type: text/plain, Size: 188 bytes --]

_______________________________________________
Socketcan-core mailing list
Socketcan-core-0fE9KPoRgkgATYTw5x5z8w@public.gmane.org
https://lists.berlios.de/mailman/listinfo/socketcan-core

^ permalink raw reply

* Re: [PATCH 1/1] NET: wan/x25_asy, move lapb_unregister to x25_asy_close_tty
From: Jiri Slaby @ 2010-11-25 12:07 UTC (permalink / raw)
  To: Andrew Hendry; +Cc: davem, netdev, slapin, linux-kernel
In-Reply-To: <AANLkTi=DEHOKkrvzHUBnW+rAkGz-uhizRqZb2PAuh4k-@mail.gmail.com>

On 11/25/2010 12:37 PM, Andrew Hendry wrote:
> Sorry I haven't used this driver so can't fully test it. Looks
> straightforward and compile tested ok.
> 
> On Thu, Nov 25, 2010 at 10:54 AM, Jiri Slaby <jslaby@suse.cz> wrote:
>> We register lapb when tty is created, but unregister it only when the
>> device is UP. So move the lapb_unregister to x25_asy_close_tty after
>> the device is down.

I forgot to mention what it causes. The commit message should add:
The old behaviour causes ldisc switching to fail each second attempt,
because we noted for us that the device is unused, so we use it the
second time, but labp layer still have it registered, so it fails
obviously.

thanks,
-- 
js

^ permalink raw reply

* Re: [PATCH net-next-2.6 v3] can: Topcliff: PCH_CAN driver: Add Flow control,
From: Tomoya MORINAGA @ 2010-11-25 12:03 UTC (permalink / raw)
  To: Marc Kleine-Budde
  Cc: andrew.chih.howe.khor-ral2JQCrhuEAvxtiuMwx3w, Samuel Ortiz,
	margie.foster-ral2JQCrhuEAvxtiuMwx3w,
	netdev-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	yong.y.wang-ral2JQCrhuEAvxtiuMwx3w,
	socketcan-core-0fE9KPoRgkgATYTw5x5z8w,
	kok.howg.ewe-ral2JQCrhuEAvxtiuMwx3w,
	joel.clark-ral2JQCrhuEAvxtiuMwx3w, qi.wang-ral2JQCrhuEAvxtiuMwx3w,
	David S. Miller, Christian Pellegrin, Wolfgang Grandegger
In-Reply-To: <4CED0650.7030004@pengutronix.de>

On Wednesday, November 24, 2010 9:34 PM, Marc Kleine-Budde wrote :
>On 11/24/2010 01:09 AM, Tomoya MORINAGA wrote:
>> On Monday, November 22, 2010 5:27 PM, Marc Kleine-Budde wrote:
>>>>>>> Still we have the busy waiting in the TX path. Maybe you can move the
>>>>>>> waiting before accessing the if[1] and remove the busy waiting here.
>>>>>> I can't understand your saying.
>>>>>> For transmitting data, calling pch_can_rw_msg_obj is mandatory.
>>>>> Yes, but the busy wait is not needed. It should be enough to do the
>>>>> busy-waiting _before_ accessing the if[1].
>>>>
>>>> Do you mean we should create other pch_can_rw_msg_obj which doesn't have busy wait ?
>>> ACK, and this non busy waiting is use in the TX path. But you add a busy
>>> wait only function before accessing the if[1] in the TX path.
>> 
>> The "busy waiting" of pch_can_rw_msg_obj is for next processing accesses to Message object.
>> If deleting this busy waiting, next processing can access to Message object, regardless previous transfer doesn't
>> complete yet.
>> Thus, I think, the "busy waiting" is necessary.
>
>Yes, it's necessary, but not where it is done currently.
>Let me outline how I think the TX path should look like:
>
>pch_xmit() {
> take_care_about_flow_control();
> prepare_can_frame_to_be_copied_to_tx_if();
>
> /* most likely we don't have to wait here */
> wait_until_tx_if_is_ready();
>
> copy_can_frame_to_tx_if();
>
> /* trigger tx in hardware */
> send_tx_if_but_dont_do_busywait();
>
> /* tx_if is busy now, but before we access it, we'll check tx_if is ready */
>}

This Tx path also has Read-Modify-Write for MessageRAM access.
Do you mean Tx path shouldn't have Read-Modify-Write ?


---
Thanks,

Tomoya MORINAGA
OKI SEMICONDUCTOR CO., LTD.

^ permalink raw reply

* Re: [PATCH net-next-2.6 6/17 v3] can: EG20T PCH: Fix endianness issue
From: Marc Kleine-Budde @ 2010-11-25 11:47 UTC (permalink / raw)
  To: Tomoya MORINAGA
  Cc: andrew.chih.howe.khor-ral2JQCrhuEAvxtiuMwx3w, Samuel Ortiz,
	margie.foster-ral2JQCrhuEAvxtiuMwx3w,
	netdev-u79uwXL29TY76Z2rM5mHXA, Christian Pellegrin,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	yong.y.wang-ral2JQCrhuEAvxtiuMwx3w,
	socketcan-core-0fE9KPoRgkgATYTw5x5z8w,
	kok.howg.ewe-ral2JQCrhuEAvxtiuMwx3w,
	joel.clark-ral2JQCrhuEAvxtiuMwx3w, David S. Miller,
	Wolfgang Grandegger, qi.wang-ral2JQCrhuEAvxtiuMwx3w
In-Reply-To: <001c01cb8c91$6e93aef0$66f8800a-a06+6cuVnkTSQfdrb5gaxUEOCMrvLtNR@public.gmane.org>


[-- Attachment #1.1: Type: text/plain, Size: 748 bytes --]

On 11/25/2010 12:10 PM, Tomoya MORINAGA wrote:
> On Wednesday, November 24, 2010 10:31 PM,  Marc Kleine-Budde wrote :
> 
>>> For easy to read, some sub-functions are created.
>>>
>>>
>>> Modify complex "goto" to do~while.
>> What about the next_flag in the rx_normal function? Can you get rid of
>> it, too?
> Subsequent Patch (11/17 Delete unnecessary ...) deletes "next_flag" in the rx_normal function.

Thanks, I've found the patch myself.

cheers, Marc

-- 
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]

[-- Attachment #2: Type: text/plain, Size: 188 bytes --]

_______________________________________________
Socketcan-core mailing list
Socketcan-core-0fE9KPoRgkgATYTw5x5z8w@public.gmane.org
https://lists.berlios.de/mailman/listinfo/socketcan-core

^ permalink raw reply

* Re: [PATCH 1/1] NET: wan/x25_asy, move lapb_unregister to x25_asy_close_tty
From: Andrew Hendry @ 2010-11-25 11:37 UTC (permalink / raw)
  To: Jiri Slaby; +Cc: davem, netdev, slapin, linux-kernel, jirislaby
In-Reply-To: <1290642894-4577-1-git-send-email-jslaby@suse.cz>

Sorry I haven't used this driver so can't fully test it. Looks
straightforward and compile tested ok.

On Thu, Nov 25, 2010 at 10:54 AM, Jiri Slaby <jslaby@suse.cz> wrote:
> We register lapb when tty is created, but unregister it only when the
> device is UP. So move the lapb_unregister to x25_asy_close_tty after
> the device is down.
>
> Signed-off-by: Jiri Slaby <jslaby@suse.cz>
> Reported-by: Sergey Lapin <slapin@ossfans.org>
> Cc: Andrew Hendry <andrew.hendry@gmail.com>
> ---
>  drivers/net/wan/x25_asy.c |   11 ++++++-----
>  1 files changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/net/wan/x25_asy.c b/drivers/net/wan/x25_asy.c
> index 66cda25..24297b2 100644
> --- a/drivers/net/wan/x25_asy.c
> +++ b/drivers/net/wan/x25_asy.c
> @@ -498,7 +498,6 @@ norbuff:
>  static int x25_asy_close(struct net_device *dev)
>  {
>        struct x25_asy *sl = netdev_priv(dev);
> -       int err;
>
>        spin_lock(&sl->lock);
>        if (sl->tty)
> @@ -507,10 +506,6 @@ static int x25_asy_close(struct net_device *dev)
>        netif_stop_queue(dev);
>        sl->rcount = 0;
>        sl->xleft  = 0;
> -       err = lapb_unregister(dev);
> -       if (err != LAPB_OK)
> -               printk(KERN_ERR "x25_asy_close: lapb_unregister error -%d\n",
> -                       err);
>        spin_unlock(&sl->lock);
>        return 0;
>  }
> @@ -595,6 +590,7 @@ static int x25_asy_open_tty(struct tty_struct *tty)
>  static void x25_asy_close_tty(struct tty_struct *tty)
>  {
>        struct x25_asy *sl = tty->disc_data;
> +       int err;
>
>        /* First make sure we're connected. */
>        if (!sl || sl->magic != X25_ASY_MAGIC)
> @@ -605,6 +601,11 @@ static void x25_asy_close_tty(struct tty_struct *tty)
>                dev_close(sl->dev);
>        rtnl_unlock();
>
> +       err = lapb_unregister(sl->dev);
> +       if (err != LAPB_OK)
> +               printk(KERN_ERR "x25_asy_close: lapb_unregister error -%d\n",
> +                       err);
> +
>        tty->disc_data = NULL;
>        sl->tty = NULL;
>        x25_asy_free(sl);
> --
> 1.7.3.1
>
>
>

^ permalink raw reply

* Re: [PATCH net-next-2.6 6/17 v3] can: EG20T PCH: Fix endianness issue
From: Tomoya MORINAGA @ 2010-11-25 11:10 UTC (permalink / raw)
  To: Marc Kleine-Budde
  Cc: andrew.chih.howe.khor-ral2JQCrhuEAvxtiuMwx3w, Samuel Ortiz,
	margie.foster-ral2JQCrhuEAvxtiuMwx3w,
	netdev-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	socketcan-core-0fE9KPoRgkgATYTw5x5z8w,
	yong.y.wang-ral2JQCrhuEAvxtiuMwx3w,
	kok.howg.ewe-ral2JQCrhuEAvxtiuMwx3w, Wolfgang Grandegger,
	joel.clark-ral2JQCrhuEAvxtiuMwx3w, David S. Miller,
	Christian Pellegrin, qi.wang-ral2JQCrhuEAvxtiuMwx3w
In-Reply-To: <4CED13B5.5020309@pengutronix.de>

On Wednesday, November 24, 2010 10:31 PM,  Marc Kleine-Budde wrote :

>> For easy to read, some sub-functions are created.
>>
>>
>> Modify complex "goto" to do~while.
>What about the next_flag in the rx_normal function? Can you get rid of
>it, too?
Subsequent Patch (11/17 Delete unnecessary ...) deletes "next_flag" in the rx_normal function.

---
Thanks,

Tomoya MORINAGA
OKI SEMICONDUCTOR CO., LTD.

----- Original Message ----- 
From: "Marc Kleine-Budde" <mkl-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
To: "Tomoya MORINAGA" <tomoya-linux-ECg8zkTtlr0C6LszWs/t0g@public.gmane.org>
Cc: "Wolfgang Grandegger" <wg-5Yr1BZd7O62+XT7JhA+gdA@public.gmane.org>; "Wolfram Sang" <w.sang-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>; "Christian Pellegrin"
<chripell-VaTbYqLCNhc@public.gmane.org>; "Barry Song" <21cnbao-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>; "Samuel Ortiz" <sameo-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>;
<socketcan-core-0fE9KPoRgkgATYTw5x5z8w@public.gmane.org>; <netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>; <linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>; "David S. Miller"
<davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>; <andrew.chih.howe.khor-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>; <qi.wang-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>; <margie.foster-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>;
<yong.y.wang-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>; <kok.howg.ewe-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>; <joel.clark-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Sent: Wednesday, November 24, 2010 10:31 PM
Subject: Re: [PATCH net-next-2.6 6/17 v3] can: EG20T PCH: Fix endianness issue

^ permalink raw reply

* [PATCH net-next] bnx2x: Use skb_is_gso_v6(skb) instead of accessing the skb_shinfo(skb) directly
From: Vladislav Zolotarov @ 2010-11-25  9:52 UTC (permalink / raw)
  To: Dave Miller; +Cc: Eilon Greenstein, netdev list
In-Reply-To: <1290612078.27220.2.camel@lb-tlvb-vladz>

Signed-off-by: Vladislav Zolotarov <vladz@broadcom.com>
Signed-off-by: Eilon Greenstein <eilong@broadcom.com>
---
 drivers/net/bnx2x/bnx2x_cmn.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/net/bnx2x/bnx2x_cmn.c b/drivers/net/bnx2x/bnx2x_cmn.c
index 94d5f59..fa8e9ee 100644
--- a/drivers/net/bnx2x/bnx2x_cmn.c
+++ b/drivers/net/bnx2x/bnx2x_cmn.c
@@ -1695,7 +1695,7 @@ static inline u32 bnx2x_xmit_type(struct bnx2x *bp, struct sk_buff *skb)
 	if (skb_shinfo(skb)->gso_type & SKB_GSO_TCPV4)
 		rc |= (XMIT_GSO_V4 | XMIT_CSUM_V4 | XMIT_CSUM_TCP);
 
-	else if (skb_shinfo(skb)->gso_type & SKB_GSO_TCPV6)
+	else if (skb_is_gso_v6(skb))
 		rc |= (XMIT_GSO_V6 | XMIT_CSUM_TCP | XMIT_CSUM_V6);
 
 	return rc;
-- 
1.7.0.4





^ permalink raw reply related

* Re: Fwd: Simple kernel attack using socketpair. easy, 100% reproductiblle, works under guest. no way to protect :(
From: Марк Коренберг @ 2010-11-25  8:35 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: netdev
In-Reply-To: <1290672978.2798.151.camel@edumazet-laptop>

quick and dirty fix will be not to allow to pass unix socket inside
unix socket. I think it would not break much applications.

2010/11/25 Eric Dumazet <eric.dumazet@gmail.com>:
> Le jeudi 25 novembre 2010 à 12:52 +0500, Марк Коренберг a écrit :
>> ---------- Forwarded message ----------
>> From: Марк Коренберг <socketpair@gmail.com>
>> Date: 2010/11/25
>> Subject: Re: Simple kernel attack using socketpair. easy, 100%
>> reproductiblle, works under guest. no way to protect :(
>> To: Eric Dumazet <eric.dumazet@gmail.com>
>>
>>
>> 2010/11/25 Eric Dumazet <eric.dumazet@gmail.com>:
>> > Le jeudi 25 novembre 2010 à 11:52 +0500, Марк Коренберг a écrit :
>> >> Well, It seems, that patch likely will fix 100% CPU usage.
>> >>
>> >> But what about eating all available descriptors in kernel ? vulnerability ?
>> >>
>> >
>> > It doesnt fix cpu usage actually, your program eats 100% of one cpu,
>> > like the following one :
>> >
>> > for (;;) ;
>> >
>> > If you want not eat 100% cpu, I suggest you add some blocking calls,
>> > like usleep(10000)
>> >
>> > for (;;) usleep(10000);
>> >
>> > Patch only makes sure kernel wont eat too much memory to store inflight
>> > unix sockets.
>>
>> I have attached source which proove, that loop not inside this
>> program, but inside kernel.
>>
>> --
>
> Better send a fix, now that thousand of people know how to kill a linux
> machine.
>
> Congrats.
>
>
>
>



-- 
Segmentation fault

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox