[PATCH 1/2] [PATCH] ucc_geth driver: add ioctl

[PATCH 1/2] [PATCH] ucc_geth driver: add ioctl

[Sergey Matyukevich]

ioctl operation (ndo_do_ioctl) is added to make mii-tools work

Signed-off-by: Sergey Matyukevich <geomatsi@gmail.com>
---
 drivers/net/ucc_geth.c |   14 ++++++++++++++
 1 files changed, 14 insertions(+), 0 deletions(-)

diff --git a/drivers/net/ucc_geth.c b/drivers/net/ucc_geth.c
index 4a34833..538148a 100644
--- a/drivers/net/ucc_geth.c
+++ b/drivers/net/ucc_geth.c
@@ -3702,6 +3702,19 @@ static phy_interface_t to_phy_interface(const char *phy_connection_type)
 	return PHY_INTERFACE_MODE_MII;
 }
 
+static int ucc_geth_ioctl(struct net_device *dev, struct ifreq *rq, int cmd)
+{
+	struct ucc_geth_private *ugeth = netdev_priv(dev);
+
+	if (!netif_running(dev))
+		return -EINVAL;
+
+	if (!ugeth->phydev)
+		return -ENODEV;
+
+	return phy_mii_ioctl(ugeth->phydev, if_mii(rq), cmd);
+}
+
 static const struct net_device_ops ucc_geth_netdev_ops = {
 	.ndo_open		= ucc_geth_open,
 	.ndo_stop		= ucc_geth_close,
@@ -3711,6 +3724,7 @@ static const struct net_device_ops ucc_geth_netdev_ops = {
 	.ndo_change_mtu		= eth_change_mtu,
 	.ndo_set_multicast_list	= ucc_geth_set_multi,
 	.ndo_tx_timeout		= ucc_geth_timeout,
+	.ndo_do_ioctl		= ucc_geth_ioctl,
 #ifdef CONFIG_NET_POLL_CONTROLLER
 	.ndo_poll_controller	= ucc_netpoll,
 #endif
-- 
1.6.2.5

[PATCH 2/2] [PATCH] ucc_geth: fix for RX skb buffers recycling

[Sergey Matyukevich]

This patch implements a proper recycling of skb buffers belonging to RX error
path. The suggested fix actually follows the recycling scheme implemented for
TX skb buffers in the same driver (see 'ucc_geth_tx' function): skb buffers
are checked by 'skb_recycle_check' function and deleted if can't be recycled.

This problem in recycling of skb buffers was discovered by accident in a setup
when ethernet interface on one link end was full-duplex while another was
half-duplex. In this case numerous corrupted frames were received by
full-duplex interface due to late collisions. RX skb buffers with error
frames were not properly recycled, that is why overflow occured from time to
time on the next use of those buffers. Here is example of crush dump:

[    2.587886] Freeing unused kernel memory: 148k init
[    3.563785] PHY: mdio@80103720:00 - Link is Up - 100/Full
[    5.440474] skb_over_panic: text:c01bf710 len:1514 put:1514 head:cf9d4000 data:cf9d4040 tail:0xcf9d466a end:0xcf9d4660 dev:eth0
[    5.452042] ------------[ cut here ]------------
[    5.456654] kernel BUG at net/core/skbuff.c:127!
[    5.461270] Oops: Exception in kernel mode, sig: 5 [#1]
[    5.469099] Modules linked in:
[    5.472155] NIP: c01cd4f4 LR: c01cd4f4 CTR: c01919a4
[    5.477117] REGS: c0325cf0 TRAP: 0700   Not tainted  (2.6.33)
[    5.482854] MSR: 00029032 <EE,ME,CE,IR,DR>  CR: 22048022  XER: 20000000
[    5.489514] TASK = c030b3e8[0] 'swapper' THREAD: c0324000
[    5.494730] GPR00: c01cd4f4 c0325da0 c030b3e8 00000089 00002e7d ffffffff c018ef88 00002e7d
[    5.503126] GPR08: 00000030 c0323290 00002e7d c032b5a8 82048022 1001a100 c02c0340 c026dd84
[    5.511519] GPR16: 00000000 c033c4a8 00000000 00000000 00000001 00000000 cf88357c 0000003f
[    5.519915] GPR24: cf8832e0 cf883000 cf8832e0 cf883550 1c0005ee 000005ea cf96eaf0 cf9d4080
[    5.528518] NIP [c01cd4f4] skb_over_panic+0x48/0x5c
[    5.533395] LR [c01cd4f4] skb_over_panic+0x48/0x5c
[    5.538177] Call Trace:
[    5.540628] [c0325da0] [c01cd4f4] skb_over_panic+0x48/0x5c (unreliable)
[    5.547251] [c0325db0] [c01cec10] skb_put+0x5c/0x60
[    5.552145] [c0325dc0] [c01bf710] ucc_geth_poll+0x404/0x478
[    5.557735] [c0325e20] [c01daef4] net_rx_action+0x9c/0x1a4

Signed-off-by: Sergey Matyukevich <geomatsi@gmail.com>
---
 drivers/net/ucc_geth.c |    9 +++++++--
 1 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ucc_geth.c b/drivers/net/ucc_geth.c
index 538148a..033b7d6 100644
--- a/drivers/net/ucc_geth.c
+++ b/drivers/net/ucc_geth.c
@@ -3214,8 +3214,13 @@ static int ucc_geth_rx(struct ucc_geth_private *ugeth, u8 rxQ, int rx_work_limit
 				ugeth_err("%s, %d: ERROR!!! skb - 0x%08x",
 					   __func__, __LINE__, (u32) skb);
 			if (skb) {
-				skb->data = skb->head + NET_SKB_PAD;
-				__skb_queue_head(&ugeth->rx_recycle, skb);
+				if (skb_queue_len(&ugeth->rx_recycle) < RX_BD_RING_LEN &&
+						skb_recycle_check(skb,
+							ugeth->ug_info->uf_info.max_rx_buf_length +
+							UCC_GETH_RX_DATA_BUF_ALIGNMENT))
+					__skb_queue_head(&ugeth->rx_recycle, skb);
+				else
+					dev_kfree_skb(skb);
 			}
 
 			ugeth->rx_skbuff[rxQ][ugeth->skb_currx[rxQ]] = NULL;
-- 
1.6.2.5

Re: [PATCH 2/2] [PATCH] ucc_geth: fix for RX skb buffers recycling

[David Miller]

From: Sergey Matyukevich <geomatsi@gmail.com>
Date: Mon,  7 Jun 2010 22:38:14 +0400

> This patch implements a proper recycling of skb buffers belonging to RX error
> path. The suggested fix actually follows the recycling scheme implemented for
> TX skb buffers in the same driver (see 'ucc_geth_tx' function): skb buffers
> are checked by 'skb_recycle_check' function and deleted if can't be recycled.
> 
> This problem in recycling of skb buffers was discovered by accident in a setup
> when ethernet interface on one link end was full-duplex while another was
> half-duplex. In this case numerous corrupted frames were received by
> full-duplex interface due to late collisions. RX skb buffers with error
> frames were not properly recycled, that is why overflow occured from time to
> time on the next use of those buffers. Here is example of crush dump:

The lack of skb_recycle_check() is not the true cause of this bug.
You should never, ever, need to make skb_recycle_check() tests on
packets in this situation.  Once the skb pointers are properly adjusted
it will have sufficient room.

And that points to what the real problem is, the problem is the
skb->data assignment.  It's trying to get the SKB data pointers back
into the same state they are in when dev_alloc_skb() returns a packet
buffer.

But this assignment isn't accomplishing that, in fact it's corrupting
the SKB because after adjusting skb->data, skb->tail and skb->len will
become incorrect.  And this is what you need to fix.

That's why you get the skb_put() over panics, not because you lack
a skb_recycle_check() call here.

In fact, what your patch makes happen is that the error packets will
never get recycled.  The skb_recycle_check() will always fail.

Please fix this bug properly by correctly restoring the SKB pointers
and lengths to their initial state, then you can retain the
unconditional queueing of the error packet onto the recycle list.

Once you do that, all of the checks done by skb_recycle_check() are
superfluous and will always pass, and we know this.  The buffer is
not fragmented, there aren't any clones or external references to it,
and once you fix up the data pointers properly it will have enough
room as necessary for the RX buffer size the driver is currently using.

There are numerous helper routines in linux/skbuff.h that can be used
to do this properly, which will adjust a pointer and make the
corresponding adjustment to skb->len as well when necessary.

Re: [PATCH 1/2] [PATCH] ucc_geth driver: add ioctl

[David Miller]

From: Sergey Matyukevich <geomatsi@gmail.com>
Date: Mon,  7 Jun 2010 22:38:13 +0400

> ioctl operation (ndo_do_ioctl) is added to make mii-tools work
> 
> Signed-off-by: Sergey Matyukevich <geomatsi@gmail.com>

Applied to net-next-2.6, thanks.

[PATCH v2] ucc_geth: fix for RX skb buffers recycling

[Sergey Matyukevich]

Hello David,

Could you please consider the second, simplified, version of the patch
for ucc_geth driver regarding proper RX error skb buffer recycling.


This patch implements a proper modification of RX skb buffers before
recycling. Adjusting only skb->data is not enough because after that
skb->tail and skb->len become incorrect.

Signed-off-by: Sergey Matyukevich <geomatsi@gmail.com>
---
 drivers/net/ucc_geth.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/drivers/net/ucc_geth.c b/drivers/net/ucc_geth.c
index 4a34833..807470e 100644
--- a/drivers/net/ucc_geth.c
+++ b/drivers/net/ucc_geth.c
@@ -3215,6 +3215,8 @@ static int ucc_geth_rx(struct ucc_geth_private *ugeth, u8 rxQ, int rx_work_limit
 					   __func__, __LINE__, (u32) skb);
 			if (skb) {
 				skb->data = skb->head + NET_SKB_PAD;
+				skb->len = 0;
+				skb_reset_tail_pointer(skb);
 				__skb_queue_head(&ugeth->rx_recycle, skb);
 			}
 
-- 
1.6.2.5

Re: [PATCH v2] ucc_geth: fix for RX skb buffers recycling

[David Miller]

From: Sergey Matyukevich <geomatsi@gmail.com>
Date: Mon, 14 Jun 2010 20:35:20 +0400

> This patch implements a proper modification of RX skb buffers before
> recycling. Adjusting only skb->data is not enough because after that
> skb->tail and skb->len become incorrect.
> 
> Signed-off-by: Sergey Matyukevich <geomatsi@gmail.com>

Applied, thanks.