[PATCH][ATM] make atm (and clip) modular + try_module_get()

[PATCH][ATM] make atm (and clip) modular + try_module_get()

[chas williams]

the following patch makes atm a module.  this patch also fixes a 
number of other problems:  MOD_INC/MOD_DEC are gone in lec and mpc
(however i needed to use module_put() inside lec, mpc and clip due to
the way the user space 'clients' unregister from the modules), clip
is a module (i wrote the clip module_exit() function -- its *probably*
correct), structs are now exported as pointers (if someone has a
compelling reason it should remain the old way let me know), ipcommon.c
is included by clip.c and doesnt export symbols -- it should probably
just be split eventually with only llc_oui residing in the kernel
(and exported) and skb_migrate moved to clip -- this really depends
on the status of CONFIG_NET_SCH_ATM.

Index: linux/net/atm/Makefile
===================================================================
RCS file: /home/chas/CVSROOT/linux/net/atm/Makefile,v
retrieving revision 1.1.1.1
diff -u -r1.1.1.1 Makefile
--- linux/net/atm/Makefile	20 Feb 2003 13:46:30 -0000	1.1.1.1
+++ linux/net/atm/Makefile	3 Mar 2003 16:51:03 -0000
@@ -3,12 +3,15 @@
 #
 
 mpoa-objs	:= mpc.o mpoa_caches.o mpoa_proc.o
+atm-objs	:= addr.o pvc.o signaling.o svc.o common.o atm_misc.o raw.o resources.o
 
-obj-$(CONFIG_ATM) := addr.o pvc.o signaling.o svc.o common.o atm_misc.o raw.o resources.o
+obj-$(CONFIG_ATM) += atm.o
 
-obj-$(CONFIG_ATM_CLIP) += clip.o ipcommon.o
-obj-$(CONFIG_NET_SCH_ATM) += ipcommon.o
-obj-$(CONFIG_PROC_FS) += proc.o
+obj-$(CONFIG_ATM_CLIP) += clip.o
+# obj-$(CONFIG_NET_SCH_ATM) += ipcommon.o
+ifeq ($(CONFIG_PROC_FS),y)
+atm-objs += proc.o
+endif
 
 obj-$(CONFIG_ATM_LANE) += lec.o
 obj-$(CONFIG_ATM_MPOA) += mpoa.o
Index: linux/net/atm/clip.c
===================================================================
RCS file: /home/chas/CVSROOT/linux/net/atm/clip.c,v
retrieving revision 1.1.1.1
diff -u -r1.1.1.1 clip.c
--- linux/net/atm/clip.c	20 Feb 2003 13:46:30 -0000	1.1.1.1
+++ linux/net/atm/clip.c	3 Mar 2003 19:33:38 -0000
@@ -7,6 +7,7 @@
 #include <linux/string.h>
 #include <linux/errno.h>
 #include <linux/kernel.h> /* for UINT_MAX */
+#include <linux/module.h>
 #include <linux/netdevice.h>
 #include <linux/skbuff.h>
 #include <linux/wait.h>
@@ -48,6 +49,7 @@
 static struct timer_list idle_timer;
 static int start_timer = 1;
 
+#include "ipcommon.c"
 
 static int to_atmarpd(enum atmarp_ctrl_type type,int itf,unsigned long ip)
 {
@@ -696,11 +698,12 @@
 		    "pending\n");
 	skb_queue_purge(&vcc->recvq);
 	DPRINTK("(done)\n");
+	module_put(THIS_MODULE);
 }
 
 
 static struct atmdev_ops atmarpd_dev_ops = {
-	.close =atmarpd_close,
+	.close = atmarpd_close,
 };
 
 
@@ -747,10 +750,50 @@
 	return 0;
 }
 
+static struct atm_clip_ops __atm_clip_ops = {
+	.clip_create =		clip_create,
+	.clip_mkip =		clip_mkip,
+	.clip_setentry =	clip_setentry,
+	.clip_encap =		clip_encap,
+	.clip_push =		clip_push,
+	.atm_init_atmarp =	atm_init_atmarp,
+	.clip_tbl =		&clip_tbl,
+	.owner =		THIS_MODULE
+};
 
-void atm_clip_init(void)
+static int __init atm_clip_init(void)
 {
+	extern struct atm_clip_ops *atm_clip_ops;
+
 	clip_tbl.lock = RW_LOCK_UNLOCKED;
 	clip_tbl.kmem_cachep = kmem_cache_create(clip_tbl.id,
 	    clip_tbl.entry_size, 0, SLAB_HWCACHE_ALIGN, NULL, NULL);
+
+	atm_clip_ops = &__atm_clip_ops;
+
+	return 0;
 }
+
+static void __exit atm_clip_exit(void)
+{
+	extern struct atm_clip_ops *atm_clip_ops;
+	struct net_device *dev, *next;
+
+	atm_clip_ops = NULL;
+
+	next = clip_devs;
+	while (next) {
+		dev = next;
+		next = PRIV(dev)->next;
+		unregister_netdev(dev);
+		kfree(dev);
+	}
+	if (start_timer == 0) del_timer(&idle_timer);
+
+	kmem_cache_destroy(clip_tbl.kmem_cachep);
+}
+
+module_init(atm_clip_init);
+module_exit(atm_clip_exit);
+
+MODULE_LICENSE("GPL");
Index: linux/net/atm/common.c
===================================================================
RCS file: /home/chas/CVSROOT/linux/net/atm/common.c,v
retrieving revision 1.5
diff -u -r1.5 common.c
--- linux/net/atm/common.c	26 Feb 2003 15:52:44 -0000	1.5
+++ linux/net/atm/common.c	3 Mar 2003 19:41:21 -0000
@@ -33,16 +33,16 @@
 #include <linux/atmlec.h>
 #include "lec.h"
 #include "lec_arpc.h"
-struct atm_lane_ops atm_lane_ops;
-#endif
+struct atm_lane_ops *atm_lane_ops = NULL;
 #ifdef CONFIG_ATM_LANE_MODULE
 EXPORT_SYMBOL(atm_lane_ops);
 #endif
+#endif
 
 #if defined(CONFIG_ATM_MPOA) || defined(CONFIG_ATM_MPOA_MODULE)
 #include <linux/atmmpc.h>
 #include "mpc.h"
-struct atm_mpoa_ops atm_mpoa_ops;
+struct atm_mpoa_ops *atm_mpoa_ops = NULL;
 #endif
 #ifdef CONFIG_ATM_MPOA_MODULE
 EXPORT_SYMBOL(atm_mpoa_ops);
@@ -59,6 +59,14 @@
 #endif
 #endif
 
+#if defined(CONFIG_ATM_CLIP) || defined(CONFIG_ATM_CLIP_MODULE)
+#include <net/atmclip.h>
+struct atm_clip_ops *atm_clip_ops = NULL;
+#ifdef CONFIG_ATM_CLIP_MODULE
+EXPORT_SYMBOL(atm_clip_ops);
+#endif
+#endif
+
 #if defined(CONFIG_PPPOATM) || defined(CONFIG_PPPOATM_MODULE)
 int (*pppoatm_ioctl_hook)(struct atm_vcc *, unsigned int, unsigned long);
 EXPORT_SYMBOL(pppoatm_ioctl_hook);
@@ -68,9 +76,6 @@
 #include "common.h"		/* prototypes */
 #include "protocols.h"		/* atm_init_<transport> */
 #include "addr.h"		/* address registry */
-#ifdef CONFIG_ATM_CLIP
-#include <net/atmclip.h>	/* for clip_create */
-#endif
 #include "signaling.h"		/* for WAITING and sigd_attach */
 
 
@@ -642,39 +647,50 @@
 			if (!error) sock->state = SS_CONNECTED;
 			ret_val = error;
 			goto done;
-#ifdef CONFIG_ATM_CLIP
+#if defined(CONFIG_ATM_CLIP) || defined(CONFIG_ATM_CLIP_MODULE)
 		case SIOCMKCLIP:
 			if (!capable(CAP_NET_ADMIN))
 				ret_val = -EPERM;
 			else 
-				ret_val = clip_create(arg);
+				ret_val = atm_clip_ops->clip_create(arg);
 			goto done;
 		case ATMARPD_CTRL:
 			if (!capable(CAP_NET_ADMIN)) {
 				ret_val = -EPERM;
 				goto done;
 			}
-			error = atm_init_atmarp(vcc);
-			if (!error) sock->state = SS_CONNECTED;
+#if defined(CONFIG_ATM_CLIP_MODULE)
+			if (atm_clip_ops == NULL)
+				request_module("clip");
+#endif
+			if (atm_clip_ops && !try_module_get(atm_clip_ops->owner)) {
+				ret_val = -ENOSYS;
+				goto done;
+			}
+			error = atm_clip_ops->atm_init_atmarp(vcc);
+			if (!error)
+				sock->state = SS_CONNECTED;
+			else
+				module_put(atm_clip_ops->owner);
 			ret_val = error;
 			goto done;
 		case ATMARP_MKIP:
 			if (!capable(CAP_NET_ADMIN)) 
 				ret_val = -EPERM;
 			else 
-				ret_val = clip_mkip(vcc,arg);
+				ret_val = atm_clip_ops->clip_mkip(vcc,arg);
 			goto done;
 		case ATMARP_SETENTRY:
 			if (!capable(CAP_NET_ADMIN)) 
 				ret_val = -EPERM;
 			else
-				ret_val = clip_setentry(vcc,arg);
+				ret_val = atm_clip_ops->clip_setentry(vcc,arg);
 			goto done;
 		case ATMARP_ENCAP:
 			if (!capable(CAP_NET_ADMIN)) 
 				ret_val = -EPERM;
 			else
-				ret_val = clip_encap(vcc,arg);
+				ret_val = atm_clip_ops->clip_encap(vcc,arg);
 			goto done;
 #endif
 #if defined(CONFIG_ATM_LANE) || defined(CONFIG_ATM_LANE_MODULE)
@@ -683,30 +699,32 @@
 				ret_val = -EPERM;
 				goto done;
 			}
-                        if (atm_lane_ops.lecd_attach == NULL)
-				atm_lane_init();
-                        if (!try_module_get(atm_lane_ops.owner)) { /* try again */
+#if defined(CONFIG_ATM_LANE_MODULE)
+                        if (atm_lane_ops == NULL)
+				request_module("lec");
+#endif
+                        if (atm_lane_ops && !try_module_get(atm_lane_ops->owner)) {
 				ret_val = -ENOSYS;
 				goto done;
 			}
-			error = atm_lane_ops.lecd_attach(vcc, (int)arg);
+			error = atm_lane_ops->lecd_attach(vcc, (int)arg);
 			if (error >= 0)
 				sock->state = SS_CONNECTED;
 			else
-				module_put(atm_lane_ops.owner);
+				module_put(atm_lane_ops->owner);
 			ret_val =  error;
 			goto done;
                 case ATMLEC_MCAST:
 			if (!capable(CAP_NET_ADMIN))
 				ret_val = -EPERM;
 			else
-				ret_val = atm_lane_ops.mcast_attach(vcc, (int)arg);
+				ret_val = atm_lane_ops->mcast_attach(vcc, (int)arg);
 			goto done;
                 case ATMLEC_DATA:
 			if (!capable(CAP_NET_ADMIN))
 				ret_val = -EPERM;
 			else
-				ret_val = atm_lane_ops.vcc_attach(vcc, (void*)arg);
+				ret_val = atm_lane_ops->vcc_attach(vcc, (void*)arg);
 			goto done;
 #endif
 #if defined(CONFIG_ATM_MPOA) || defined(CONFIG_ATM_MPOA_MODULE)
@@ -715,21 +733,26 @@
 				ret_val = -EPERM;
 				goto done;
 			}
-			if (atm_mpoa_ops.mpoad_attach == NULL)
-                                atm_mpoa_init();
-			if (atm_mpoa_ops.mpoad_attach == NULL) { /* try again */
+#if defined(CONFIG_ATM_MPOA_MODULE)
+			if (atm_mpoa_ops == NULL)
+                                request_module("mpoa");
+#endif
+			if (atm_mpoa_ops && !try_module_get(atm_mpoa_ops->owner)) {
 				ret_val = -ENOSYS;
 				goto done;
 			}
-			error = atm_mpoa_ops.mpoad_attach(vcc, (int)arg);
-			if (error >= 0) sock->state = SS_CONNECTED;
+			error = atm_mpoa_ops->mpoad_attach(vcc, (int)arg);
+			if (error >= 0)
+				sock->state = SS_CONNECTED;
+			else
+				module_put(atm_mpoa_ops->owner);
 			ret_val = error;
 			goto done;
 		case ATMMPC_DATA:
 			if (!capable(CAP_NET_ADMIN)) 
 				ret_val = -EPERM;
 			else
-				ret_val = atm_mpoa_ops.vcc_attach(vcc, arg);
+				ret_val = atm_mpoa_ops->vcc_attach(vcc, arg);
 			goto done;
 #endif
 #if defined(CONFIG_ATM_TCP) || defined(CONFIG_ATM_TCP_MODULE)
@@ -1105,40 +1128,6 @@
 }
 
 
-/*
- * lane_mpoa_init.c: A couple of helper functions
- * to make modular LANE and MPOA client easier to implement
- */
-
-/*
- * This is how it goes:
- *
- * if xxxx is not compiled as module, call atm_xxxx_init_ops()
- *    from here
- * else call atm_mpoa_init_ops() from init_module() within
- *    the kernel when xxxx module is loaded
- *
- * In either case function pointers in struct atm_xxxx_ops
- * are initialized to their correct values. Either they
- * point to functions in the module or in the kernel
- */
- 
-extern struct atm_mpoa_ops atm_mpoa_ops; /* in common.c */
-extern struct atm_lane_ops atm_lane_ops; /* in common.c */
-
-#if defined(CONFIG_ATM_MPOA) || defined(CONFIG_ATM_MPOA_MODULE)
-void atm_mpoa_init(void)
-{
-#ifndef CONFIG_ATM_MPOA_MODULE /* not module */
-        atm_mpoa_init_ops(&atm_mpoa_ops);
-#else
-	request_module("mpoa");
-#endif
-
-        return;
-}
-#endif
-
 #if defined(CONFIG_ATM_LANE) || defined(CONFIG_ATM_LANE_MODULE)
 #if defined(CONFIG_BRIDGE) || defined(CONFIG_BRIDGE_MODULE)
 struct net_bridge_fdb_entry *(*br_fdb_get_hook)(struct net_bridge *br,
@@ -1149,15 +1138,33 @@
 EXPORT_SYMBOL(br_fdb_put_hook);
 #endif /* defined(CONFIG_ATM_LANE_MODULE) || defined(CONFIG_BRIDGE_MODULE) */
 #endif /* defined(CONFIG_BRIDGE) || defined(CONFIG_BRIDGE_MODULE) */
+#endif /* defined(CONFIG_ATM_LANE) || defined(CONFIG_ATM_LANE_MODULE) */
+
 
-void atm_lane_init(void)
+static int __init atm_init(void)
 {
-#ifndef CONFIG_ATM_LANE_MODULE /* not module */
-        atm_lane_init_ops(&atm_lane_ops);
-#else
-	request_module("lec");
+	int error = 0;
+
+	if (atmpvc_init() < 0)
+		return -1;
+	if (atmsvc_init() < 0)
+		return -1;
+#ifdef CONFIG_PROC_FS
+        error = atm_proc_init();
+        if (error) printk("atm_proc_init fails with %d\n",error);
 #endif
+	return error;
+}
 
-        return;
-}        
+static void __exit atm_exit(void)
+{
+#ifdef CONFIG_PROC_FS
+	atm_proc_exit();
 #endif
+	atmsvc_exit();
+	atmpvc_exit();
+}
+
+module_init(atm_init);
+module_exit(atm_exit);
+MODULE_LICENSE("GPL");
Index: linux/net/atm/common.h
===================================================================
RCS file: /home/chas/CVSROOT/linux/net/atm/common.h,v
retrieving revision 1.1.1.1
diff -u -r1.1.1.1 common.h
--- linux/net/atm/common.h	20 Feb 2003 13:46:30 -0000	1.1.1.1
+++ linux/net/atm/common.h	27 Feb 2003 16:47:15 -0000
@@ -28,7 +28,14 @@
 void atm_release_vcc_sk(struct sock *sk,int free_sk);
 void atm_shutdown_dev(struct atm_dev *dev);
 
+int atmsvc_init(void);
+void atmsvc_exit(void);
+
+int atmpvc_init(void);
+void atmpvc_exit(void);
+
 int atm_proc_init(void);
+void atm_proc_exit(void);
 
 /* SVC */
 
Index: linux/net/atm/ipcommon.c
===================================================================
RCS file: /home/chas/CVSROOT/linux/net/atm/ipcommon.c,v
retrieving revision 1.1.1.1
diff -u -r1.1.1.1 ipcommon.c
--- linux/net/atm/ipcommon.c	20 Feb 2003 13:46:30 -0000	1.1.1.1
+++ linux/net/atm/ipcommon.c	3 Mar 2003 16:51:32 -0000
@@ -65,6 +65,3 @@
 	from->qlen = 0;
 	spin_unlock_irqrestore(&from->lock,flags);
 }
-
-
-EXPORT_SYMBOL(skb_migrate);
Index: linux/net/atm/ipcommon.h
===================================================================
RCS file: /home/chas/CVSROOT/linux/net/atm/ipcommon.h,v
retrieving revision 1.1.1.1
diff -u -r1.1.1.1 ipcommon.h
--- linux/net/atm/ipcommon.h	20 Feb 2003 13:46:30 -0000	1.1.1.1
+++ linux/net/atm/ipcommon.h	3 Mar 2003 16:52:24 -0000
@@ -13,8 +13,6 @@
 #include <linux/atmdev.h>
 
 
-extern struct net_device *clip_devs;
-
 /*
  * Appends all skbs from "from" to "to". The operation is atomic with respect
  * to all other skb operations on "from" or "to".
Index: linux/net/atm/lec.c
===================================================================
RCS file: /home/chas/CVSROOT/linux/net/atm/lec.c,v
retrieving revision 1.11
diff -u -r1.11 lec.c
--- linux/net/atm/lec.c	3 Mar 2003 21:25:05 -0000	1.11
+++ linux/net/atm/lec.c	3 Mar 2003 21:27:05 -0000
@@ -541,14 +541,14 @@
                 atm_return(vcc, skb->truesize);
 		dev_kfree_skb(skb);
         }
-  
+
 	printk("%s: Shut down!\n", dev->name);
-        MOD_DEC_USE_COUNT;
+	module_put(THIS_MODULE);
 }
 
 static struct atmdev_ops lecdev_ops = {
         .close	= lec_atm_close,
-        .send	= lec_atm_send
+        .send	= lec_atm_send,
 };
 
 static struct atm_dev lecatm_dev = {
@@ -790,7 +790,6 @@
                 dev_lec[i] = init_etherdev(NULL, size);
                 if (!dev_lec[i])
                         return -ENOMEM;
-
                 priv = dev_lec[i]->priv;
                 priv->is_trdev = is_trdev;
                 sprintf(dev_lec[i]->name, "lec%d", i);
@@ -824,27 +823,23 @@
         if (dev_lec[i]->flags & IFF_UP) {
                 netif_start_queue(dev_lec[i]);
         }
-        MOD_INC_USE_COUNT;
         return i;
 }
 
-void atm_lane_init_ops(struct atm_lane_ops *ops)
-{
-        ops->lecd_attach = lecd_attach;
-        ops->mcast_attach = lec_mcast_attach;
-        ops->vcc_attach = lec_vcc_attach;
-        ops->get_lecs = get_dev_lec;
-
-        printk("lec.c: " __DATE__ " " __TIME__ " initialized\n");
-
-	return;
-}
+static struct atm_lane_ops __atm_lane_ops = {
+        .lecd_attach =	lecd_attach,
+        .mcast_attach =	lec_mcast_attach,
+        .vcc_attach =	lec_vcc_attach,
+        .get_lecs = 	get_dev_lec,
+        .owner =	THIS_MODULE
+};
 
 static int __init lane_module_init(void)
 {
-        extern struct atm_lane_ops atm_lane_ops;
+        extern struct atm_lane_ops *atm_lane_ops;
 
-        atm_lane_init_ops(&atm_lane_ops);
+        atm_lane_ops = &__atm_lane_ops;
+        printk("lec.c: " __DATE__ " " __TIME__ " initialized\n");
 
         return 0;
 }
@@ -852,13 +847,10 @@
 static void __exit lane_module_cleanup(void)
 {
         int i;
-        extern struct atm_lane_ops atm_lane_ops;
+        extern struct atm_lane_ops *atm_lane_ops;
         struct lec_priv *priv;
 
-        atm_lane_ops.lecd_attach = NULL;
-        atm_lane_ops.mcast_attach = NULL;
-        atm_lane_ops.vcc_attach = NULL;
-        atm_lane_ops.get_lecs = NULL;
+        atm_lane_ops = NULL;
 
         for (i = 0; i < MAX_LEC_ITF; i++) {
                 if (dev_lec[i] != NULL) {
@@ -871,7 +863,7 @@
                         unregister_netdev(dev_lec[i]);
                         kfree(dev_lec[i]);
                         dev_lec[i] = NULL;
-                }
+              }
         }
 
         return;                                    
@@ -1021,7 +1013,7 @@
 #include <linux/timer.h>
 #include <asm/param.h>
 #include <asm/atomic.h>
-#include <linux/inetdevice.h>
+#include <linux/netdevice.h>
 #include <net/route.h>
 
 
Index: linux/net/atm/lec.h
===================================================================
RCS file: /home/chas/CVSROOT/linux/net/atm/lec.h,v
retrieving revision 1.5
diff -u -r1.5 lec.h
--- linux/net/atm/lec.h	3 Mar 2003 21:25:05 -0000	1.5
+++ linux/net/atm/lec.h	3 Mar 2003 21:25:17 -0000
@@ -65,6 +65,7 @@
         int (*mcast_attach)(struct atm_vcc *vcc, int arg);
         int (*vcc_attach)(struct atm_vcc *vcc, void *arg);
         struct net_device **(*get_lecs)(void);
+        struct module *owner;
 };
 
 /*
@@ -155,7 +156,5 @@
                  unsigned char *atm_addr, struct sk_buff *data);
 void lec_push(struct atm_vcc *vcc, struct sk_buff *skb);
 
-void atm_lane_init(void);
-void atm_lane_init_ops(struct atm_lane_ops *ops);
 #endif /* _LEC_H_ */
 
Index: linux/net/atm/mpc.c
===================================================================
RCS file: /home/chas/CVSROOT/linux/net/atm/mpc.c,v
retrieving revision 1.3
diff -u -r1.3 mpc.c
--- linux/net/atm/mpc.c	3 Mar 2003 21:25:05 -0000	1.3
+++ linux/net/atm/mpc.c	3 Mar 2003 21:25:19 -0000
@@ -799,7 +799,6 @@
 			send_set_mps_ctrl_addr(mpc->mps_ctrl_addr, mpc);
 	}
 
-	MOD_INC_USE_COUNT;
 	return arg;
 }
 
@@ -848,7 +847,7 @@
 	
 	printk("mpoa: (%s) going down\n",
 		(mpc->dev) ? mpc->dev->name : "<unknown>");
-	MOD_DEC_USE_COUNT;
+	module_put(THIS_MODULE);
 
 	return;
 }
@@ -1390,11 +1389,17 @@
 	return;
 }
 
-void atm_mpoa_init_ops(struct atm_mpoa_ops *ops)
+struct atm_mpoa_ops __atm_mpoa_ops = {
+        .mpoad_attach =	atm_mpoa_mpoad_attach,
+        .vcc_attach =	atm_mpoa_vcc_attach,
+        .owner =	THIS_MODULE
+};
+
+static int __init atm_mpoa_init(void)
 {
-	ops->mpoad_attach = atm_mpoa_mpoad_attach;
-	ops->vcc_attach = atm_mpoa_vcc_attach;
+	extern struct atm_mpoa_ops *atm_mpoa_ops;
 
+	atm_mpoa_ops = &__atm_mpoa_ops;
 #ifdef CONFIG_PROC_FS
 	if(mpc_proc_init() != 0)
 		printk(KERN_INFO "mpoa: failed to initialize /proc/mpoa\n");
@@ -1404,22 +1409,12 @@
 
 	printk("mpc.c: " __DATE__ " " __TIME__ " initialized\n");
 
-	return;
-}
-
-#ifdef MODULE
-int init_module(void)
-{
-	extern struct atm_mpoa_ops atm_mpoa_ops;
-
-	atm_mpoa_init_ops(&atm_mpoa_ops);
-
 	return 0;
 }
 
-void cleanup_module(void)
+static void __exit atm_mpoa_cleanup(void)
 {
-	extern struct atm_mpoa_ops atm_mpoa_ops;
+	extern struct atm_mpoa_ops *atm_mpoa_ops;
 	struct mpoa_client *mpc, *tmp;
 	struct atm_mpoa_qos *qos, *nextqos;
 	struct lec_priv *priv;
@@ -1430,8 +1425,7 @@
 
 	del_timer(&mpc_timer);
 	unregister_netdevice_notifier(&mpoa_notifier);
-	atm_mpoa_ops.mpoad_attach = NULL;
-	atm_mpoa_ops.vcc_attach = NULL;
+	atm_mpoa_ops = NULL;
 
 	mpc = mpcs;
 	mpcs = NULL;
@@ -1463,8 +1457,9 @@
 		kfree(qos);
 		qos = nextqos;
 	}
-
-	return;
 }
-#endif /* MODULE */
+
+module_init(atm_mpoa_init);
+module_exit(atm_mpoa_cleanup);
+
 MODULE_LICENSE("GPL");
Index: linux/net/atm/mpc.h
===================================================================
RCS file: /home/chas/CVSROOT/linux/net/atm/mpc.h,v
retrieving revision 1.1.1.1
diff -u -r1.1.1.1 mpc.h
--- linux/net/atm/mpc.h	20 Feb 2003 13:46:30 -0000	1.1.1.1
+++ linux/net/atm/mpc.h	1 Mar 2003 13:37:05 -0000
@@ -48,11 +48,8 @@
 struct atm_mpoa_ops {
         int (*mpoad_attach)(struct atm_vcc *vcc, int arg);  /* attach mpoa daemon  */
         int (*vcc_attach)(struct atm_vcc *vcc, long arg);   /* attach shortcut vcc */
+        struct module *owner;
 };
-
-/* Boot/module initialization function */
-void atm_mpoa_init(void);
-void atm_mpoa_init_ops(struct atm_mpoa_ops *ops);
 
 /* MPOA QoS operations */
 struct atm_mpoa_qos *atm_mpoa_add_qos(uint32_t dst_ip, struct atm_qos *qos);
Index: linux/net/atm/proc.c
===================================================================
RCS file: /home/chas/CVSROOT/linux/net/atm/proc.c,v
retrieving revision 1.2
diff -u -r1.2 proc.c
--- linux/net/atm/proc.c	26 Feb 2003 15:52:44 -0000	1.2
+++ linux/net/atm/proc.c	1 Mar 2003 14:39:12 -0000
@@ -39,16 +39,15 @@
 #include "common.h" /* atm_proc_init prototype */
 #include "signaling.h" /* to get sigd - ugly too */
 
-#ifdef CONFIG_ATM_CLIP
+#if defined(CONFIG_ATM_CLIP) || defined(CONFIG_ATM_CLIP_MODULE)
 #include <net/atmclip.h>
-#include "ipcommon.h"
-extern void clip_push(struct atm_vcc *vcc,struct sk_buff *skb);
+extern struct atm_clip_ops *atm_clip_ops; /* in common.c */
 #endif
 
 #if defined(CONFIG_ATM_LANE) || defined(CONFIG_ATM_LANE_MODULE)
 #include "lec.h"
 #include "lec_arpc.h"
-extern struct atm_lane_ops atm_lane_ops; /* in common.c */
+extern struct atm_lane_ops *atm_lane_ops; /* in common.c */
 #endif
 
 static ssize_t proc_dev_atm_read(struct file *file,char *buf,size_t count,
@@ -89,7 +88,7 @@
 }
 
 
-#ifdef CONFIG_ATM_CLIP
+#if defined(CONFIG_ATM_CLIP) || defined(CONFIG_ATM_CLIP_MODULE)
 
 
 static int svc_addr(char *buf,struct sockaddr_atmsvc *addr)
@@ -178,8 +177,8 @@
 	    aal_name[vcc->qos.aal],vcc->qos.rxtp.min_pcr,
 	    class_name[vcc->qos.rxtp.traffic_class],vcc->qos.txtp.min_pcr,
 	    class_name[vcc->qos.txtp.traffic_class]);
-#ifdef CONFIG_ATM_CLIP
-	if (vcc->push == clip_push) {
+#if defined(CONFIG_ATM_CLIP) || defined(CONFIG_ATM_CLIP_MODULE)
+	if (atm_clip_ops && (vcc->push == atm_clip_ops->clip_push)) {
 		struct clip_vcc *clip_vcc = CLIP_VCC(vcc);
 		struct net_device *dev;
 
@@ -393,7 +392,7 @@
 	return 0;
 }
 
-#ifdef CONFIG_ATM_CLIP
+#if defined(CONFIG_ATM_CLIP) || defined(CONFIG_ATM_CLIP_MODULE)
 static int atm_arp_info(loff_t pos,char *buf)
 {
 	struct neighbour *n;
@@ -403,28 +402,30 @@
 		return sprintf(buf,"IPitf TypeEncp Idle IP address      "
 		    "ATM address\n");
 	}
+	if (!atm_clip_ops)
+		return 0;
 	count = pos;
-	read_lock_bh(&clip_tbl.lock);
+	read_lock_bh(&atm_clip_ops->clip_tbl->lock);
 	for (i = 0; i <= NEIGH_HASHMASK; i++)
-		for (n = clip_tbl.hash_buckets[i]; n; n = n->next) {
+		for (n = atm_clip_ops->clip_tbl->hash_buckets[i]; n; n = n->next) {
 			struct atmarp_entry *entry = NEIGH2ENTRY(n);
 			struct clip_vcc *vcc;
 
 			if (!entry->vccs) {
 				if (--count) continue;
 				atmarp_info(n->dev,entry,NULL,buf);
-				read_unlock_bh(&clip_tbl.lock);
+				read_unlock_bh(&atm_clip_ops->clip_tbl->lock);
 				return strlen(buf);
 			}
 			for (vcc = entry->vccs; vcc;
 			    vcc = vcc->next) {
 				if (--count) continue;
 				atmarp_info(n->dev,entry,vcc,buf);
-				read_unlock_bh(&clip_tbl.lock);
+				read_unlock_bh(&atm_clip_ops->clip_tbl->lock);
 				return strlen(buf);
 			}
 		}
-	read_unlock_bh(&clip_tbl.lock);
+	read_unlock_bh(&atm_clip_ops->clip_tbl->lock);
 	return 0;
 }
 #endif
@@ -442,13 +443,13 @@
 		    "                          Status            Flags "
 		    "VPI/VCI Recv VPI/VCI\n");
 	}
-	if (atm_lane_ops.get_lecs == NULL)
+	if (!atm_lane_ops)
 		return 0; /* the lane module is not there yet */
 
-	if (!try_module_get(atm_lane_ops.owner))
+	if (!try_module_get(atm_lane_ops->owner))
 		return 0;
 
-	dev_lec = atm_lane_ops.get_lecs();
+	dev_lec = atm_lane_ops->get_lecs();
 
 	count = pos;
 	for(d=0;d<MAX_LEC_ITF;d++) {
@@ -461,7 +462,7 @@
 				e=sprintf(buf,"%s ",
 				    dev_lec[d]->name);
 				lec_info(entry,buf+e);
-				module_put(atm_lane_ops.owner);
+				module_put(atm_lane_ops->owner);
 				return strlen(buf);
 			}
 		}
@@ -470,7 +471,7 @@
 			if (--count) continue;
 			e=sprintf(buf,"%s ",dev_lec[d]->name);
 			lec_info(entry, buf+e);
-			module_put(atm_lane_ops.owner);
+			module_put(atm_lane_ops->owner);
 			return strlen(buf);
 		}
 		for(entry=priv->lec_no_forward; entry;
@@ -478,7 +479,7 @@
 			if (--count) continue;
 			e=sprintf(buf,"%s ",dev_lec[d]->name);
 			lec_info(entry, buf+e);
-			module_put(atm_lane_ops.owner);
+			module_put(atm_lane_ops->owner);
 			return strlen(buf);
 		}
 		for(entry=priv->mcast_fwds; entry;
@@ -486,11 +487,11 @@
 			if (--count) continue;
 			e=sprintf(buf,"%s ",dev_lec[d]->name);
 			lec_info(entry, buf+e);
-			module_put(atm_lane_ops.owner);
+			module_put(atm_lane_ops->owner);
 			return strlen(buf);
 		}
 	}
-	module_put(atm_lane_ops.owner);
+	module_put(atm_lane_ops->owner);
 	return 0;
 }
 #endif
@@ -591,12 +592,11 @@
     name->proc_fops = &proc_spec_atm_operations; \
     name->owner = THIS_MODULE
 
+struct proc_dir_entry *devices = NULL, *pvc = NULL, *svc = NULL;
+struct proc_dir_entry *arp = NULL, *lec = NULL, *vc = NULL;
 
 int __init atm_proc_init(void)
 {
-	struct proc_dir_entry *devices = NULL,*pvc = NULL,*svc = NULL;
-	struct proc_dir_entry *arp = NULL,*lec = NULL,*vc = NULL;
-
 	atm_proc_root = proc_mkdir("net/atm",NULL);
 	if (!atm_proc_root)
 		return -ENOMEM;
@@ -604,7 +604,7 @@
 	CREATE_ENTRY(pvc);
 	CREATE_ENTRY(svc);
 	CREATE_ENTRY(vc);
-#ifdef CONFIG_ATM_CLIP
+#if defined(CONFIG_ATM_CLIP) || defined(CONFIG_ATM_CLIP_MODULE)
 	CREATE_ENTRY(arp);
 #endif
 #if defined(CONFIG_ATM_LANE) || defined(CONFIG_ATM_LANE_MODULE)
@@ -621,4 +621,15 @@
 	if (vc) remove_proc_entry("vc",atm_proc_root);
 	remove_proc_entry("net/atm",NULL);
 	return -ENOMEM;
+}
+
+void __exit atm_proc_exit(void)
+{
+	if (vc) remove_proc_entry("vc",atm_proc_root);
+	if (lec) remove_proc_entry("lec",atm_proc_root);
+	if (arp) remove_proc_entry("arp",atm_proc_root);
+	if (svc) remove_proc_entry("svc",atm_proc_root);
+	if (pvc) remove_proc_entry("pvc",atm_proc_root);
+	if (devices) remove_proc_entry("devices",atm_proc_root);
+	remove_proc_entry("net/atm",NULL);
 }
Index: linux/net/atm/pvc.c
===================================================================
RCS file: /home/chas/CVSROOT/linux/net/atm/pvc.c,v
retrieving revision 1.1.1.1
diff -u -r1.1.1.1 pvc.c
--- linux/net/atm/pvc.c	20 Feb 2003 13:46:30 -0000	1.1.1.1
+++ linux/net/atm/pvc.c	1 Mar 2003 14:11:17 -0000
@@ -15,9 +15,6 @@
 #include <linux/skbuff.h>
 #include <linux/bitops.h>
 #include <net/sock.h>		/* for sock_no_* */
-#ifdef CONFIG_ATM_CLIP
-#include <net/atmclip.h>
-#endif
 
 #include "resources.h"		/* devs and vccs */
 #include "common.h"		/* common for PVCs and SVCs */
@@ -111,8 +108,8 @@
 
 
 static struct net_proto_family pvc_family_ops = {
-	.family =PF_ATMPVC,
-	.create =pvc_create,
+	.family = PF_ATMPVC,
+	.create = pvc_create,
 };
 
 
@@ -121,23 +118,20 @@
  */
 
 
-static int __init atmpvc_init(void)
+int __init atmpvc_init(void)
 {
 	int error;
 
 	error = sock_register(&pvc_family_ops);
 	if (error < 0) {
-		printk(KERN_ERR "ATMPVC: can't register (%d)",error);
+		printk(KERN_ERR "ATMPVC: can't register (%d)", error);
 		return error;
 	}
-#ifdef CONFIG_ATM_CLIP
-	atm_clip_init();
-#endif
-#ifdef CONFIG_PROC_FS
-	error = atm_proc_init();
-	if (error) printk("atm_proc_init fails with %d\n",error);
-#endif
-	return 0;
+
+	return error;
 }
 
-module_init(atmpvc_init);
+void __exit atmpvc_exit(void)
+{
+	sock_unregister(PF_ATMPVC);
+}
Index: linux/net/atm/svc.c
===================================================================
RCS file: /home/chas/CVSROOT/linux/net/atm/svc.c,v
retrieving revision 1.1.1.1
diff -u -r1.1.1.1 svc.c
--- linux/net/atm/svc.c	20 Feb 2003 13:46:30 -0000	1.1.1.1
+++ linux/net/atm/svc.c	27 Feb 2003 16:48:54 -0000
@@ -430,8 +430,8 @@
 
 
 static struct net_proto_family svc_family_ops = {
-	.family =PF_ATMSVC,
-	.create =svc_create,
+	.family = PF_ATMSVC,
+	.create = svc_create,
 };
 
 
@@ -439,13 +439,19 @@
  *	Initialize the ATM SVC protocol family
  */
 
-static int __init atmsvc_init(void)
+int __init atmsvc_init(void)
 {
-	if (sock_register(&svc_family_ops) < 0) {
-		printk(KERN_ERR "ATMSVC: can't register");
+	int error;
+
+	error = sock_register(&svc_family_ops);
+	if (error < 0) {
+		printk(KERN_ERR "ATMSVC: can't register (%d)\n", error);
 		return -1;
 	}
 	return 0;
 }
 
-module_init(atmsvc_init);
+void __exit atmsvc_exit(void)
+{
+	sock_unregister(PF_ATMSVC);
+}
Index: linux/include/net/atmclip.h
===================================================================
RCS file: /home/chas/CVSROOT/linux/include/net/atmclip.h,v
retrieving revision 1.1.1.1
diff -u -r1.1.1.1 atmclip.h
--- linux/include/net/atmclip.h	20 Feb 2003 13:45:58 -0000	1.1.1.1
+++ linux/include/net/atmclip.h	1 Mar 2003 13:59:31 -0000
@@ -62,6 +62,16 @@
 int clip_mkip(struct atm_vcc *vcc,int timeout);
 int clip_setentry(struct atm_vcc *vcc,u32 ip);
 int clip_encap(struct atm_vcc *vcc,int mode);
-void atm_clip_init(void);
+
+struct atm_clip_ops {
+	int (*clip_create)(int number);
+	int (*clip_mkip)(struct atm_vcc *vcc,int timeout);
+	int (*clip_setentry)(struct atm_vcc *vcc,u32 ip);
+	int (*clip_encap)(struct atm_vcc *vcc,int mode);
+	void (*clip_push)(struct atm_vcc *vcc,struct sk_buff *skb);
+	int (*atm_init_atmarp)(struct atm_vcc *vcc);
+	struct neigh_table *clip_tbl;
+	struct module *owner;
+};
 
 #endif

Re: [PATCH][ATM] make atm (and clip) modular + try_module_get()

[chas williams]

oops -- missing just one little bit (the best part?)

Index: linux/net/Kconfig
===================================================================
RCS file: /home/chas/CVSROOT/linux/net/Kconfig,v
retrieving revision 1.1.1.1
diff -u -r1.1.1.1 Kconfig
--- linux/net/Kconfig	20 Feb 2003 13:46:28 -0000	1.1.1.1
+++ linux/net/Kconfig	1 Mar 2003 13:47:27 -0000
@@ -227,7 +227,7 @@
 source "net/sctp/Kconfig"
 
 config ATM
-	bool "Asynchronous Transfer Mode (ATM) (EXPERIMENTAL)"
+	tristate "Asynchronous Transfer Mode (ATM) (EXPERIMENTAL)"
 	depends on EXPERIMENTAL
 	---help---
 	  ATM is a high-speed networking technology for Local Area Networks
@@ -244,7 +244,7 @@
 	  further details.
 
 config ATM_CLIP
-	bool "Classical IP over ATM (EXPERIMENTAL)"
+	tristate "Classical IP over ATM (EXPERIMENTAL)"
 	depends on ATM && INET
 	help
 	  Classical IP over ATM for PVCs and SVCs, supporting InARP and
@@ -264,7 +264,7 @@
 
 config ATM_LANE
 	tristate "LAN Emulation (LANE) support (EXPERIMENTAL)"
-	depends on ATM
+	depends on ATM && INET
 	help
 	  LAN Emulation emulates services of existing LANs across an ATM
 	  network. Besides operating as a normal ATM end station client, Linux

Re: [PATCH][ATM] make atm (and clip) modular + try_module_get()

[Kai Germaschewski]

On Mon, 3 Mar 2003, chas williams wrote:

> Index: linux/net/atm/Makefile
> ===================================================================
> RCS file: /home/chas/CVSROOT/linux/net/atm/Makefile,v
> retrieving revision 1.1.1.1
> diff -u -r1.1.1.1 Makefile
> --- linux/net/atm/Makefile	20 Feb 2003 13:46:30 -0000	1.1.1.1
> +++ linux/net/atm/Makefile	3 Mar 2003 16:51:03 -0000
> @@ -3,12 +3,15 @@
>  #
>  
>  mpoa-objs	:= mpc.o mpoa_caches.o mpoa_proc.o
> +atm-objs	:= addr.o pvc.o signaling.o svc.o common.o atm_misc.o raw.o resources.o
>  
> -obj-$(CONFIG_ATM) := addr.o pvc.o signaling.o svc.o common.o atm_misc.o raw.o resources.o
> +obj-$(CONFIG_ATM) += atm.o
>  
> -obj-$(CONFIG_ATM_CLIP) += clip.o ipcommon.o
> -obj-$(CONFIG_NET_SCH_ATM) += ipcommon.o
> -obj-$(CONFIG_PROC_FS) += proc.o
> +obj-$(CONFIG_ATM_CLIP) += clip.o
> +# obj-$(CONFIG_NET_SCH_ATM) += ipcommon.o
> +ifeq ($(CONFIG_PROC_FS),y)
> +atm-objs += proc.o
> +endif
>  
>  obj-$(CONFIG_ATM_LANE) += lec.o
>  obj-$(CONFIG_ATM_MPOA) += mpoa.o


Not terribly important, but you can write this as:


obj-$(CONFIG_ATM)	+= atm.o

atm-y			+= addr.o pvc.o signaling.o svc.o ...
atm-$(CONFIG_PROC_FS)	+= proc.o


which looks a bit nicer ;)

--Kai

Re: [PATCH][ATM] make atm (and clip) modular + try_module_get()

[Werner Almesberger]

chas williams wrote:
> (and exported) and skb_migrate moved to clip -- this really depends

Actually, skb_migrate is something that I'd always have liked to
see getting moved to net/core/skbuff.c, because it seems to provide
a reasonably generic function.

- Werner

-- 
  _________________________________________________________________________
 / Werner Almesberger, Buenos Aires, Argentina         wa@almesberger.net /
/_http://www.almesberger.net/____________________________________________/

Re: [PATCH][ATM] make atm (and clip) modular + try_module_get()

[David S. Miller]

   From: chas williams <chas@locutus.cmf.nrl.navy.mil>
   Date: Wed, 05 Mar 2003 09:47:38 -0500

   In message <20030303230706.R2791@almesberger.net>,Werner Almesberger writes:
   >see getting moved to net/core/skbuff.c, because it seems to provide
   >a reasonably generic function.
   
   it has been suggested to me that the locking in skb_migrate might not be
   completely correct.  any comments on the following?
   
If you own both objects, why lock anything?

Re: [PATCH][ATM] make atm (and clip) modular + try_module_get()

[chas williams]

In message <Pine.LNX.4.44.0303031825240.16397-100000@chaos.physics.uiowa.edu>,K
ai Germaschewski writes:
>Not terribly important, but you can write this as:
>obj-$(CONFIG_ATM)	+= atm.o
>atm-y			+= addr.o pvc.o signaling.o svc.o ...
>atm-$(CONFIG_PROC_FS)	+= proc.o

after looking at some other examples, i guess i like this even better:


ipcommon-obj-$(subst m,y,$(CONFIG_ATM_CLIP)) := ipcommon.o
ipcommon-obj-$(subst m,y,$(CONFIG_NET_SCH_ATM)) := ipcommon.o

atm-objs        := addr.o pvc.o signaling.o svc.o common.o atm_misc.o raw.o resources.o $(ipcommon-obj-y)

obj-$(CONFIG_ATM) += atm.o
atm-$(CONFIG_PROC_FS) += proc.o

Re: [PATCH][ATM] make atm (and clip) modular + try_module_get()

[chas williams]

In message <20030303230706.R2791@almesberger.net>,Werner Almesberger writes:
>see getting moved to net/core/skbuff.c, because it seems to provide
>a reasonably generic function.

it has been suggested to me that the locking in skb_migrate might not be
completely correct.  any comments on the following?

        spinlock_t *first, *second;
        if ((unsigned long)from < (unsigned long) to)) {
                first = &from->lock;
                second = &to->lock;
        } else {
                first = &to->lock;
                second = &from->lock;
        }

        local_irq_save(flags);
        spin_lock(&first);
        spin_lock(&second);

i imagine this is to prevent deadlocks when you do something silly like
skb_migrate(a,b) and then skb_migrate(b,a) elsewhere.

Re: [PATCH][ATM] make atm (and clip) modular + try_module_get()

[David S. Miller]

   From: chas williams <chas@locutus.cmf.nrl.navy.mil>
   Date: Wed, 05 Mar 2003 10:08:42 -0500

   In message <20030305.063158.53514369.davem@redhat.com>,"David S. Miller" writes
   :
   >If you own both objects, why lock anything?
   
   i believe the original intent was to prevent anyone else from 
   appending to either of the lists while the lists are being joined.
   while it would be slightly less efficient, should it use the
   skb primitives?  this is quite a bit easier to read:

I understand why you think you have to lock, that isn't the issue.

I'm telling you that you should be locking this crap at a much
higher level.

Re: [PATCH][ATM] make atm (and clip) modular + try_module_get()

[chas williams]

In message <20030305.063158.53514369.davem@redhat.com>,"David S. Miller" writes
:
>If you own both objects, why lock anything?

i believe the original intent was to prevent anyone else from 
appending to either of the lists while the lists are being joined.
while it would be slightly less efficient, should it use the
skb primitives?  this is quite a bit easier to read:

void skb_migrate(struct sk_buff_head *from,struct sk_buff_head *to)
{
	struct sk_buff *skb;
	unsigned long flags;

	local_irq_save(flags);
	spin_lock(&to->lock);
	spin_lock(&from->lock);

	while(skb = __skb_dequeue(from))
		__skb_queue_tail(to, skb);

	spin_unlock(&to->lock);
	spin_unlock(&from->lock);
	local_irq_restore(flags);
}

Re: [PATCH][ATM] make atm (and clip) modular + try_module_get()

[David S. Miller]

   From: chas williams <chas@locutus.cmf.nrl.navy.mil>
   Date: Wed, 05 Mar 2003 10:28:52 -0500
   
   i am afraid i dont know much about how the clip driver operates.

As ATM maintainer, now might be a good time to learn this :-)

Re: [PATCH][ATM] make atm (and clip) modular + try_module_get()

[chas williams]

In message <20030305.065341.35361286.davem@redhat.com>,"David S. Miller" writes
:
>I understand why you think you have to lock, that isn't the issue.
>I'm telling you that you should be locking this crap at a much
>higher level.

ok, i see what you are saying now.  since this is used to move the
stuff queued from the old connection to the new connection i suppose
clip might want something like?

	spin_lock_bh(&vcc->recvq.lock)
        skb_migrate(&vcc->recvq,&copy);
	spin_unlock_bh(&vcc->recvq.lock);

that would block any more additions to the recvq (which should be
sk->receieve_queue i suspect -- more on that later) while the skb are
moved to copy.  i am afraid i dont know much about how the clip driver
operates.

Re: [PATCH][ATM] make atm (and clip) modular + try_module_get()

[chas williams]

In message <20030305.071245.92277670.davem@redhat.com>,"David S. Miller" writes
:
>As ATM maintainer, now might be a good time to learn this :-)

i have a religious objection to clip.  ask me about lane :)  seriously
though i know there are a coulple problems with the clip driver:  xoff
handling is really quite 'strange' and there appears to a race with the
neighbor table handling w/ svcs.  i really just want to get things running
again (however poorly) on 2.5 before 'fixing' things.

Re: [PATCH][ATM] make atm (and clip) modular + try_module_get()

[Werner Almesberger]

chas williams wrote:
> sk->receieve_queue i suspect -- more on that later) while the skb are
> moved to copy.  i am afraid i dont know much about how the clip driver
> operates.

What's happening there is that new CLIP connections start as normal
native ATM VCs, owned by atmarpd. When atmarpd clears them for IP
traffic, all the data that has been queued so far (i.e. any ATMARP
messages, and any IP - typically I'd expect to find a SYN there)
is removed from the native ATM VC's queue, and fed into the non-ATM
part of the stack, for de-encapsulation, etc.

Note that ATMARP will be delivered again to the "native ATM" VC's
queue, because that's how atmarpd receives ATMARP messages.

Figure 6 of http://www.almesberger.net/cv/papers/atm_on_linux.ps
shows the overall design (some details have changed since then,
though).

- Werner

-- 
  _________________________________________________________________________
 / Werner Almesberger, Buenos Aires, Argentina         wa@almesberger.net /
/_http://www.almesberger.net/____________________________________________/

Re: [PATCH][ATM] make atm (and clip) modular + try_module_get()

[Kai Germaschewski]

On Wed, 5 Mar 2003, chas williams wrote:

> In message <Pine.LNX.4.44.0303031825240.16397-100000@chaos.physics.uiowa.edu>,K
> ai Germaschewski writes:
> >Not terribly important, but you can write this as:
> >obj-$(CONFIG_ATM)	+= atm.o
> >atm-y			+= addr.o pvc.o signaling.o svc.o ...
> >atm-$(CONFIG_PROC_FS)	+= proc.o
> 
> after looking at some other examples, i guess i like this even better:
> 
> 
> ipcommon-obj-$(subst m,y,$(CONFIG_ATM_CLIP)) := ipcommon.o
> ipcommon-obj-$(subst m,y,$(CONFIG_NET_SCH_ATM)) := ipcommon.o
> 
> atm-objs        := addr.o pvc.o signaling.o svc.o common.o atm_misc.o raw.o resources.o $(ipcommon-obj-y)
> 
> obj-$(CONFIG_ATM) += atm.o
> atm-$(CONFIG_PROC_FS) += proc.o

Well, this is IMO confusing since now you're using two different ways to 
add to atm.o in the same Makefile.

The preferred way would be:

obj-$(CONFIG_ATM) += atm.o

atm-y := addr.o pvc.o signaling.o svc.o common.o atm_misc.o raw.o resources.o
atm-$(subst m,y,$(CONFIG_ATM_CLIP))	+= ipcommon.o
atm-$(subst m,y,$(CONFIG_NET_SCH_ATM))	+= ipcommon.o
atm-$(CONFIG_PROC_FS)			+= proc.o

You could write this as

obj-$(CONFIG_ATM) += atm.o

atm-obj-$(subst m,y,$(CONFIG_ATM_CLIP))		+= ipcommon.o
atm-obj-$(subst m,y,$(CONFIG_NET_SCH_ATM))	+= ipcommon.o
atm-obj-$(CONFIG_PROC_FS)			+= proc.o
atm-objs := addr.o pvc.o signaling.o svc.o common.o atm_misc.o raw.o \
	 resources.o $(atm-obj-y)

But I'd really like you to use the former. I know this is currently 
handled inconsistently throughout the Makefiles, at some point I'll 
hopefully get around to converting things to the "new-style" above.

--Kai

Re: [PATCH][ATM] make atm (and clip) modular + try_module_get()

[chas williams]

In message <Pine.LNX.4.44.0303050952010.31461-100000@chaos.physics.uiowa.edu>,K
ai Germaschewski writes:
>atm-$(subst m,y,$(CONFIG_ATM_CLIP))	+= ipcommon.o
>atm-$(subst m,y,$(CONFIG_NET_SCH_ATM))	+= ipcommon.o

i cant do this because if both CONFIG_ATM_CLIP and CONFIG_NET_SCH_ATM
are configured, ipcommon.o will be in the link list twice twice.

Re: [PATCH][ATM] make atm (and clip) modular + try_module_get()

[Kai Germaschewski]

On Wed, 5 Mar 2003, chas williams wrote:

> In message <Pine.LNX.4.44.0303050952010.31461-100000@chaos.physics.uiowa.edu>,K
> ai Germaschewski writes:
> >atm-$(subst m,y,$(CONFIG_ATM_CLIP))	+= ipcommon.o
> >atm-$(subst m,y,$(CONFIG_NET_SCH_ATM))	+= ipcommon.o
> 
> i cant do this because if both CONFIG_ATM_CLIP and CONFIG_NET_SCH_ATM
> are configured, ipcommon.o will be in the link list twice twice.

The build system actually goes through quite a bit of effort to remove 
those duplicate entries. Try it, if it doesn't work I sure want to know
about it.

--Kai

Re: [PATCH][ATM] make atm (and clip) modular + try_module_get()

[chas williams]

In message <Pine.LNX.4.44.0303051010200.31461-100000@chaos.physics.uiowa.edu>,K
ai Germaschewski writes:
>...quite a bit of effort to remove those duplicate entries...

shocking!  so how about this then:

atm-y   := addr.o pvc.o signaling.o svc.o common.o atm_misc.o raw.o resources.o
mpoa-y  := mpc.o mpoa_caches.o mpoa_proc.o

obj-$(CONFIG_ATM) += atm.o
atm-$(CONFIG_PROC_FS) += proc.o
atm-$(subst m,y,$(CONFIG_ATM_CLIP)) += ipcommon.o
atm-$(subst m,y,$(CONFIG_NET_SCH_ATM)) += ipcommon.o
obj-$(CONFIG_ATM_CLIP) += clip.o
obj-$(CONFIG_ATM_LANE) += lec.o
obj-$(CONFIG_ATM_MPOA) += mpoa.o
obj-$(CONFIG_PPPOATM) += pppoatm.o

Re: [PATCH][ATM] make atm (and clip) modular + try_module_get()

[Roman Zippel]

Hi,

On Wed, 5 Mar 2003, Kai Germaschewski wrote:

> The preferred way would be:
> 
> obj-$(CONFIG_ATM) += atm.o
> 
> atm-y := addr.o pvc.o signaling.o svc.o common.o atm_misc.o raw.o resources.o
> atm-$(subst m,y,$(CONFIG_ATM_CLIP))	+= ipcommon.o
> atm-$(subst m,y,$(CONFIG_NET_SCH_ATM))	+= ipcommon.o
> atm-$(CONFIG_PROC_FS)			+= proc.o

BTW some of this could also be done in the Kconfig:

config ATM_IPCOMMON
	bool
	default y if ATM_CLIP || NET_SCH_ATM

so you can change this into:

atm-$(CONFIG_ATM_IPCOMMON) += ipcommon.o

bye, Roman

Re: [PATCH][ATM] make atm (and clip) modular + try_module_get()

[Kai Germaschewski]

On Wed, 5 Mar 2003, chas williams wrote:

> shocking!  so how about this then:
> 
> atm-y   := addr.o pvc.o signaling.o svc.o common.o atm_misc.o raw.o resources.o
> mpoa-y  := mpc.o mpoa_caches.o mpoa_proc.o
> 
> obj-$(CONFIG_ATM) += atm.o
> atm-$(CONFIG_PROC_FS) += proc.o
> atm-$(subst m,y,$(CONFIG_ATM_CLIP)) += ipcommon.o
> atm-$(subst m,y,$(CONFIG_NET_SCH_ATM)) += ipcommon.o
> obj-$(CONFIG_ATM_CLIP) += clip.o
> obj-$(CONFIG_ATM_LANE) += lec.o
> obj-$(CONFIG_ATM_MPOA) += mpoa.o
> obj-$(CONFIG_PPPOATM) += pppoatm.o

Better, but I still think you should group the statements which define 
what atm.o is composed of, i.e. move the atm-$... up, or the atm-y down ;)

It's up to you, though, of course.

--Kai

Re: [PATCH][ATM] make atm (and clip) modular + try_module_get()

[chas williams]

In message <20030305125230.B525@almesberger.net>,Werner Almesberger writes:
>native ATM VCs, owned by atmarpd. When atmarpd clears them for IP
>traffic, all the data that has been queued so far (i.e. any ATMARP
>messages, and any IP - typically I'd expect to find a SYN there)
>is removed from the native ATM VC's queue, and fed into the non-ATM
>part of the stack, for de-encapsulation, etc.

while you could make skb_migrate() generic, in this case it doesnt
really need to be.  since the dest list is on the stack you wouldn't
need to lock it -- only the source list.  however, this whole migrate
thing makes me a bit nervous.  you copy the pending data and then
requeue the data.  what keeps data from arriving on the vcc while
you are reprocessing -- this could lead to out of order packet
arrival.

perhaps the recvq could be locked (to prevent new arrivals) and then
go through and feed the ip traffic to clip_vcc() leaving the atmarp
traffic in the queue.  then, unlock and wakeup the vcc to force atmarp
to get the data pending for it (if any).  this would prevent get 
around the need to 'migrate' the data back and forth.

Re: [PATCH][ATM] make atm (and clip) modular + try_module_get()

[Werner Almesberger]

chas williams wrote:
> while you could make skb_migrate() generic, in this case it doesnt
> really need to be.

Yes, that's a possible simplification.

> requeue the data.  what keeps data from arriving on the vcc while
> you are reprocessing -- this could lead to out of order packet
> arrival.

I don't think that would be much of a problem: ATMARP basically
aggregates information, but does very little in terms of actual
state transitions based on ATMARP/InARP messages. And IP doesn't
mind a bit of reordering anyway. (Reconnecting an SVC in the
middle of a TCP session may of course cause performance
glitches, but that would be more a problem of detaching the VC
in the first place, than anything happening when re-attaching.)

- Werner

-- 
  _________________________________________________________________________
 / Werner Almesberger, Buenos Aires, Argentina         wa@almesberger.net /
/_http://www.almesberger.net/____________________________________________/

Re: [PATCH][ATM] make atm (and clip) modular + try_module_get()

[chas williams]

In message <20030306180209.C525@almesberger.net>,Werner Almesberger writes:
>I don't think that would be much of a problem: ATMARP basically
>aggregates information, but does very little in terms of actual
>state transitions based on ATMARP/InARP messages. And IP doesn't
>mind a bit of reordering anyway. (Reconnecting an SVC in the

it would be a good idea to preserve the arrival order as much as possible
even though some protocols might not be overly sensitive.  just handling
the list as it appears on the recvq would eliminate the need for skb_migrate
and always keep the packets in the right order.