ucc_geth driver: add ioctl

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

---
 drivers/net/ucc_geth.c |   14 ++++++++++++++
 1 files changed, 14 insertions(+), 0 deletions(-)

index 4a34833..538148a 100644
@@ -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

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

---
 drivers/net/ucc_geth.c |    9 +++++++--
 1 files changed, 7 insertions(+), 2 deletions(-)

index 538148a..033b7d6 100644
@@ -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

From: Sergey Matyukevich <[email protected]>
Date: Mon, 7 Jun 2010 22:38:14 +0400

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.

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.

---
 drivers/net/ucc_geth.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

index 4a34833..807470e 100644
@@ -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

From: Sergey Matyukevich <[email protected]>
Date: Mon, 14 Jun 2010 20:35:20 +0400

Applied, thanks.

From: Sergey Matyukevich <[email protected]>
Date: Mon, 7 Jun 2010 22:38:13 +0400

Applied to net-next-2.6, thanks.