2004-04-14 22:25:13

by Jean Tourrilhes

[permalink] [raw]
Subject: [PATCH 2.6 IrDA] irlan - irlan_eth cleanup

ir265_irlan_eth_cleanup-2.diff :
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
o [FEATURE] Use IrTTP flow control to stop/wake netif
<Patch from Stephen Hemminger>
o [CORRECT] Change irlan_eth device initialization:
*bug* address never set in DIRECT mode because access not set
in alloc_netdev -> irlan_eth_setup path
+ make eth_XXX handles static and provide alloc_irlandev hook
+ use netdev_priv (and get rid of truly impossible ASSERT's)
+ use skb_queue_purge


diff -u -p linux/include/net/irda/irlan_eth.d0.h linux/include/net/irda/irlan_eth.h
--- linux/include/net/irda/irlan_eth.d0.h Thu Apr 8 11:06:54 2004
+++ linux/include/net/irda/irlan_eth.h Thu Apr 8 11:16:16 2004
@@ -25,16 +25,9 @@
#ifndef IRLAN_ETH_H
#define IRLAN_ETH_H

-void irlan_eth_setup(struct net_device *dev);
-int irlan_eth_open(struct net_device *dev);
-int irlan_eth_close(struct net_device *dev);
+struct net_device *alloc_irlandev(const char *name);
int irlan_eth_receive(void *instance, void *sap, struct sk_buff *skb);
-int irlan_eth_xmit(struct sk_buff *skb, struct net_device *dev);

void irlan_eth_flow_indication( void *instance, void *sap, LOCAL_FLOW flow);
void irlan_eth_send_gratuitous_arp(struct net_device *dev);
-
-void irlan_eth_set_multicast_list( struct net_device *dev);
-struct net_device_stats *irlan_eth_get_stats(struct net_device *dev);
-
#endif
diff -u -p linux/net/irda/irlan/irlan_common.d0.c linux/net/irda/irlan/irlan_common.c
--- linux/net/irda/irlan/irlan_common.d0.c Thu Apr 8 11:06:08 2004
+++ linux/net/irda/irlan/irlan_common.c Thu Apr 8 14:03:27 2004
@@ -32,6 +32,7 @@
#include <linux/errno.h>
#include <linux/proc_fs.h>
#include <linux/seq_file.h>
+#include <linux/random.h>
#include <linux/netdevice.h>
#include <linux/etherdevice.h>
#include <linux/rtnetlink.h>
@@ -120,7 +121,7 @@ static int __init irlan_init(void)
struct irlan_cb *new;
__u16 hints;

- IRDA_DEBUG(0, "%s()\n", __FUNCTION__ );
+ IRDA_DEBUG(2, "%s()\n", __FUNCTION__ );

#ifdef CONFIG_PROC_FS
{ struct proc_dir_entry *proc;
@@ -191,9 +192,7 @@ struct irlan_cb *irlan_open(__u32 saddr,
IRDA_DEBUG(2, "%s()\n", __FUNCTION__ );

/* Create network device with irlan */
- dev = alloc_netdev(sizeof(*self),
- eth ? "eth%d" : "irlan%d",
- irlan_eth_setup);
+ dev = alloc_irlandev(eth ? "eth%d" : "irlan%d");
if (!dev)
return NULL;

@@ -209,6 +208,19 @@ struct irlan_cb *irlan_open(__u32 saddr,

/* Provider access can only be PEER, DIRECT, or HOSTED */
self->provider.access_type = access;
+ if (access == ACCESS_DIRECT) {
+ /*
+ * Since we are emulating an IrLAN sever we will have to
+ * give ourself an ethernet address!
+ */
+ dev->dev_addr[0] = 0x40;
+ dev->dev_addr[1] = 0x00;
+ dev->dev_addr[2] = 0x00;
+ dev->dev_addr[3] = 0x00;
+ get_random_bytes(dev->dev_addr+4, 1);
+ get_random_bytes(dev->dev_addr+5, 1);
+ }
+
self->media = MEDIA_802_3;
self->disconnect_reason = LM_USER_REQUEST;
init_timer(&self->watchdog_timer);
@@ -248,8 +260,8 @@ static void __irlan_close(struct irlan_c
ASSERT(self != NULL, return;);
ASSERT(self->magic == IRLAN_MAGIC, return;);

- del_timer(&self->watchdog_timer);
- del_timer(&self->client.kick_timer);
+ del_timer_sync(&self->watchdog_timer);
+ del_timer_sync(&self->client.kick_timer);

/* Close all open connections and remove TSAPs */
irlan_close_tsaps(self);
@@ -300,7 +312,7 @@ void irlan_connect_indication(void *inst
self->max_sdu_size = max_sdu_size;
self->max_header_size = max_header_size;

- IRDA_DEBUG(0, "IrLAN, We are now connected!\n");
+ IRDA_DEBUG(0, "%s: We are now connected!\n", __FUNCTION__);

del_timer(&self->watchdog_timer);

@@ -342,7 +354,7 @@ void irlan_connect_confirm(void *instanc

/* TODO: we could set the MTU depending on the max_sdu_size */

- IRDA_DEBUG(2, "IrLAN, We are now connected!\n");
+ IRDA_DEBUG(0, "%s: We are now connected!\n", __FUNCTION__);
del_timer(&self->watchdog_timer);

/*
@@ -448,7 +460,7 @@ void irlan_open_data_tsap(struct irlan_c
notify.udata_indication = irlan_eth_receive;
notify.connect_indication = irlan_connect_indication;
notify.connect_confirm = irlan_connect_confirm;
- /*notify.flow_indication = irlan_eth_flow_indication;*/
+ notify.flow_indication = irlan_eth_flow_indication;
notify.disconnect_indication = irlan_disconnect_indication;
notify.instance = self;
strlcpy(notify.name, "IrLAN data", sizeof(notify.name));
diff -u -p linux/net/irda/irlan/irlan_eth.d0.c linux/net/irda/irlan/irlan_eth.c
--- linux/net/irda/irlan/irlan_eth.d0.c Thu Apr 8 11:06:17 2004
+++ linux/net/irda/irlan/irlan_eth.c Thu Apr 8 14:47:54 2004
@@ -30,7 +30,6 @@
#include <linux/etherdevice.h>
#include <linux/inetdevice.h>
#include <linux/if_arp.h>
-#include <linux/random.h>
#include <linux/module.h>
#include <net/arp.h>

@@ -41,20 +40,20 @@
#include <net/irda/irlan_event.h>
#include <net/irda/irlan_eth.h>

+static int irlan_eth_open(struct net_device *dev);
+static int irlan_eth_close(struct net_device *dev);
+static int irlan_eth_xmit(struct sk_buff *skb, struct net_device *dev);
+static void irlan_eth_set_multicast_list( struct net_device *dev);
+static struct net_device_stats *irlan_eth_get_stats(struct net_device *dev);
+
/*
- * Function irlan_eth_init (dev)
+ * Function irlan_eth_setup (dev)
*
* The network device initialization function.
*
*/
-void irlan_eth_setup(struct net_device *dev)
+static void irlan_eth_setup(struct net_device *dev)
{
- struct irlan_cb *self;
-
- IRDA_DEBUG(2, "%s()\n", __FUNCTION__ );
-
- self = (struct irlan_cb *) dev->priv;
-
dev->open = irlan_eth_open;
dev->stop = irlan_eth_close;
dev->hard_start_xmit = irlan_eth_xmit;
@@ -71,20 +70,30 @@ void irlan_eth_setup(struct net_device *
* Queueing here as well can introduce some strange latency
* problems, which we will avoid by setting the queue size to 0.
*/
- dev->tx_queue_len = 0;
+ /*
+ * The bugs in IrTTP and IrLAN that created this latency issue
+ * have now been fixed, and we can propagate flow control properly
+ * to the network layer. However, this requires a minimal queue of
+ * packets for the device.
+ * Without flow control, the Tx Queue is 14 (ttp) + 0 (dev) = 14
+ * With flow control, the Tx Queue is 7 (ttp) + 4 (dev) = 11
+ * See irlan_eth_flow_indication()...
+ * Note : this number was randomly selected and would need to
+ * be adjusted.
+ * Jean II */
+ dev->tx_queue_len = 4;
+}

- if (self->provider.access_type == ACCESS_DIRECT) {
- /*
- * Since we are emulating an IrLAN sever we will have to
- * give ourself an ethernet address!
- */
- dev->dev_addr[0] = 0x40;
- dev->dev_addr[1] = 0x00;
- dev->dev_addr[2] = 0x00;
- dev->dev_addr[3] = 0x00;
- get_random_bytes(dev->dev_addr+4, 1);
- get_random_bytes(dev->dev_addr+5, 1);
- }
+/*
+ * Function alloc_irlandev
+ *
+ * Allocate network device and control block
+ *
+ */
+struct net_device *alloc_irlandev(const char *name)
+{
+ return alloc_netdev(sizeof(struct irlan_cb), name,
+ irlan_eth_setup);
}

/*
@@ -93,18 +102,12 @@ void irlan_eth_setup(struct net_device *
* Network device has been opened by user
*
*/
-int irlan_eth_open(struct net_device *dev)
+static int irlan_eth_open(struct net_device *dev)
{
- struct irlan_cb *self;
+ struct irlan_cb *self = netdev_priv(dev);

IRDA_DEBUG(2, "%s()\n", __FUNCTION__ );

- ASSERT(dev != NULL, return -1;);
-
- self = (struct irlan_cb *) dev->priv;
-
- ASSERT(self != NULL, return -1;);
-
/* Ready to play! */
netif_stop_queue(dev); /* Wait until data link is ready */

@@ -126,10 +129,9 @@ int irlan_eth_open(struct net_device *de
* close timer, so that the instance will be removed if we are unable
* to discover the remote device after the disconnect.
*/
-int irlan_eth_close(struct net_device *dev)
+static int irlan_eth_close(struct net_device *dev)
{
- struct irlan_cb *self = (struct irlan_cb *) dev->priv;
- struct sk_buff *skb;
+ struct irlan_cb *self = netdev_priv(dev);

IRDA_DEBUG(2, "%s()\n", __FUNCTION__ );

@@ -143,8 +145,7 @@ int irlan_eth_close(struct net_device *d
irlan_do_provider_event(self, IRLAN_LMP_DISCONNECT, NULL);

/* Remove frames queued on the control channel */
- while ((skb = skb_dequeue(&self->client.txq)))
- dev_kfree_skb(skb);
+ skb_queue_purge(&self->client.txq);

self->client.tx_busy = 0;

@@ -157,16 +158,11 @@ int irlan_eth_close(struct net_device *d
* Transmits ethernet frames over IrDA link.
*
*/
-int irlan_eth_xmit(struct sk_buff *skb, struct net_device *dev)
+static int irlan_eth_xmit(struct sk_buff *skb, struct net_device *dev)
{
- struct irlan_cb *self;
+ struct irlan_cb *self = netdev_priv(dev);
int ret;

- self = (struct irlan_cb *) dev->priv;
-
- ASSERT(self != NULL, return 0;);
- ASSERT(self->magic == IRLAN_MAGIC, return 0;);
-
/* skb headroom large enough to contain all IrDA-headers? */
if ((skb_headroom(skb) < self->max_header_size) || (skb_shared(skb))) {
struct sk_buff *new_skb =
@@ -220,9 +216,7 @@ int irlan_eth_xmit(struct sk_buff *skb,
*/
int irlan_eth_receive(void *instance, void *sap, struct sk_buff *skb)
{
- struct irlan_cb *self;
-
- self = (struct irlan_cb *) instance;
+ struct irlan_cb *self = instance;

if (skb == NULL) {
++self->stats.rx_dropped;
@@ -251,6 +245,14 @@ int irlan_eth_receive(void *instance, vo
*
* Do flow control between IP/Ethernet and IrLAN/IrTTP. This is done by
* controlling the queue stop/start.
+ *
+ * The IrDA link layer has the advantage to have flow control, and
+ * IrTTP now properly handles that. Flow controlling the higher layers
+ * prevent us to drop Tx packets in here (up to 15% for a TCP socket,
+ * more for UDP socket).
+ * Also, this allow us to reduce the overall transmit queue, which means
+ * less latency in case of mixed traffic.
+ * Jean II
*/
void irlan_eth_flow_indication(void *instance, void *sap, LOCAL_FLOW flow)
{
@@ -266,37 +268,25 @@ void irlan_eth_flow_indication(void *ins

ASSERT(dev != NULL, return;);

+ IRDA_DEBUG(0, "%s() : flow %s ; running %d\n", __FUNCTION__,
+ flow == FLOW_STOP ? "FLOW_STOP" : "FLOW_START",
+ netif_running(dev));
+
switch (flow) {
case FLOW_STOP:
+ /* IrTTP is full, stop higher layers */
netif_stop_queue(dev);
break;
case FLOW_START:
default:
/* Tell upper layers that its time to transmit frames again */
/* Schedule network layer */
- netif_start_queue(dev);
+ netif_wake_queue(dev);
break;
}
}

/*
- * Function irlan_eth_rebuild_header (buff, dev, dest, skb)
- *
- * If we don't want to use ARP. Currently not used!!
- *
- */
-void irlan_eth_rebuild_header(void *buff, struct net_device *dev,
- unsigned long dest, struct sk_buff *skb)
-{
- struct ethhdr *eth = (struct ethhdr *) buff;
-
- memcpy(eth->h_source, dev->dev_addr, dev->addr_len);
- memcpy(eth->h_dest, dev->dev_addr, dev->addr_len);
-
- /* return 0; */
-}
-
-/*
* Function irlan_etc_send_gratuitous_arp (dev)
*
* Send gratuitous ARP to announce that we have changed
@@ -336,17 +326,12 @@ void irlan_eth_send_gratuitous_arp(struc
*
*/
#define HW_MAX_ADDRS 4 /* Must query to get it! */
-void irlan_eth_set_multicast_list(struct net_device *dev)
+static void irlan_eth_set_multicast_list(struct net_device *dev)
{
- struct irlan_cb *self;
-
- self = dev->priv;
+ struct irlan_cb *self = netdev_priv(dev);

IRDA_DEBUG(2, "%s()\n", __FUNCTION__ );

- ASSERT(self != NULL, return;);
- ASSERT(self->magic == IRLAN_MAGIC, return;);
-
/* Check if data channel has been connected yet */
if (self->client.state != IRLAN_DATA) {
IRDA_DEBUG(1, "%s(), delaying!\n", __FUNCTION__ );
@@ -388,12 +373,9 @@ void irlan_eth_set_multicast_list(struct
* Get the current statistics for this device
*
*/
-struct net_device_stats *irlan_eth_get_stats(struct net_device *dev)
+static struct net_device_stats *irlan_eth_get_stats(struct net_device *dev)
{
- struct irlan_cb *self = (struct irlan_cb *) dev->priv;
-
- ASSERT(self != NULL, return NULL;);
- ASSERT(self->magic == IRLAN_MAGIC, return NULL;);
+ struct irlan_cb *self = netdev_priv(dev);

return &self->stats;
}