Skip to contentSkip to conversation

net/decnet/: misc possible cleanups

PATCH In review Adrian Bunk 3 participants 4 messages Dec 14, 2004 – Dec 18, 2004

Subsystems (2)
Networking Network drivers
lore
Share & tools
lore

Adrian Bunk

author
#0 lore raw

PATCH 1/1 net/decnet/: misc possible cleanups

The patch below contains the following possible cleanups:
- make needlessly global code static
- dn_fib.c: remove the write-only global variable dn_fib_info_cnt
- dn_fib.c: remove the unused global function dn_fib_rt_message
- dn_neigh.c: remove the unused global function dn_neigh_pointopoint_notify
- dn_timer.c: remove the fast timer code that isn't used

Please review and comment on this patch.

diffstat output:

 include/net/dn.h       |    2 -
 include/net/dn_fib.h   |    1 
 net/decnet/af_decnet.c |    6 +----
 net/decnet/dn_fib.c    |   15 -------------
 net/decnet/dn_neigh.c  |    8 ------
 net/decnet/dn_route.c  |    6 ++---
 net/decnet/dn_timer.c  |   47 -----------------------------------------
 7 files changed, 5 insertions(+), 80 deletions(-)
Signed-off-by Adrian Bunk <[email protected]>
net/decnet/af_decnet.c renamed from net/decnet/af_decnet.c.old +2 -4
@@ -246,7 +246,7 @@
 	write_unlock_bh(&dn_hash_lock);
 }
 
-struct hlist_head *listen_hash(struct sockaddr_dn *addr)
+static struct hlist_head *listen_hash(struct sockaddr_dn *addr)
 {
 	int i;
 	unsigned hash = addr->sdn_objnum;

@@ -447,7 +447,7 @@
 	dst_release(xchg(&sk->sk_dst_cache, NULL));
 }
 
-struct sock *dn_alloc_sock(struct socket *sock, int gfp)
+static struct sock *dn_alloc_sock(struct socket *sock, int gfp)
 {
 	struct dn_scp *scp;
 	struct sock *sk = sk_alloc(PF_DECnet, gfp, sizeof(struct dn_sock),

@@ -578,7 +578,6 @@
 	if (sk->sk_socket)
 		return 0;
 
-	dn_stop_fast_timer(sk); /* unlikely, but possible that this is runninng */
 	if ((jiffies - scp->stamp) >= (HZ * decnet_time_wait)) {
 		dn_unhash_sock(sk);
 		sock_put(sk);

@@ -631,7 +630,6 @@
 		default:
 			printk(KERN_DEBUG "DECnet: dn_destroy_sock passed socket in invalid state\n");
 		case DN_O:
-			dn_stop_fast_timer(sk);
 			dn_stop_slow_timer(sk);
 
 			dn_unhash_sock_bh(sk);
include/net/dn_fib.h renamed from include/net/dn_fib.h.old +0 -1
@@ -117,7 +117,6 @@
 extern void dn_fib_init(void);
 extern void dn_fib_cleanup(void);
 
-extern int dn_fib_rt_message(struct sk_buff *skb);
 extern int dn_fib_ioctl(struct socket *sock, unsigned int cmd, 
 			unsigned long arg);
 extern struct dn_fib_info *dn_fib_create_info(const struct rtmsg *r,
net/decnet/dn_fib.c renamed from net/decnet/dn_fib.c.old +0 -15
@@ -60,7 +60,6 @@
 static spinlock_t dn_fib_multipath_lock = SPIN_LOCK_UNLOCKED;
 static struct dn_fib_info *dn_fib_info_list;
 static rwlock_t dn_fib_info_lock = RW_LOCK_UNLOCKED;
-int dn_fib_info_cnt;
 
 static struct
 {

@@ -93,7 +92,6 @@
 			dev_put(nh->nh_dev);
 		nh->nh_dev = NULL;
 	} endfor_nexthops(fi);
-	dn_fib_info_cnt--;
 	kfree(fi);
 }
 

@@ -388,7 +386,6 @@
 	if (dn_fib_info_list)
 		dn_fib_info_list->fib_prev = fi;
 	dn_fib_info_list = fi;
-	dn_fib_info_cnt++;
 	write_unlock(&dn_fib_info_lock);
 	return fi;
 

@@ -486,18 +483,6 @@
 }
 
 
-/*
- * Punt to user via netlink for example, but for now
- * we just drop it.
- */
-int dn_fib_rt_message(struct sk_buff *skb)
-{
-	kfree_skb(skb);
-
-	return 0;
-}
-
-
 static int dn_fib_check_attr(struct rtmsg *r, struct rtattr **rta)
 {
 	int i;
net/decnet/dn_neigh.c renamed from net/decnet/dn_neigh.c.old +0 -8
@@ -355,14 +355,6 @@
  * basically does a neigh_lookup(), but without comparing the device
  * field. This is required for the On-Ethernet cache
  */
-/*
- * Any traffic on a pointopoint link causes the timer to be reset
- * for the entry in the neighbour table.
- */
-void dn_neigh_pointopoint_notify(struct sk_buff *skb)
-{
-	return;
-}
 
 /*
  * Pointopoint link receives a hello message
net/decnet/dn_route.c renamed from net/decnet/dn_route.c.old +3 -3
@@ -99,9 +99,9 @@
 
 static unsigned char dn_hiord_addr[6] = {0xAA,0x00,0x04,0x00,0x00,0x00};
 
-int dn_rt_min_delay = 2 * HZ;
-int dn_rt_max_delay = 10 * HZ;
-int dn_rt_mtu_expires = 10 * 60 * HZ;
+static const int dn_rt_min_delay = 2 * HZ;
+static const int dn_rt_max_delay = 10 * HZ;
+static const int dn_rt_mtu_expires = 10 * 60 * HZ;
 
 static unsigned long dn_rt_deadline;
include/net/dn.h renamed from include/net/dn.h.old +0 -2
@@ -220,8 +220,6 @@
 
 extern void dn_start_slow_timer(struct sock *sk);
 extern void dn_stop_slow_timer(struct sock *sk);
-extern void dn_start_fast_timer(struct sock *sk);
-extern void dn_stop_fast_timer(struct sock *sk);
 
 extern dn_address decnet_address;
 extern int decnet_debug_level;
net/decnet/dn_timer.c renamed from net/decnet/dn_timer.c.old +0 -47

59 lines not on this page · raw message

#1 lore raw

Hi,

In general this looks fine to me. Patrick has the last word though. I would suggest hanging on to the fast timer code though for now... its there so that delayed acks can be used and the rest of the code is very close to actually allowing that to work. The DECnet code has a habit of sending lots of (rather small) ack packets and it would be nice if that could be fixed at some stage.

The issues at stake are that it would interact with the code thats working out what the correct send window is to a certain extent. I know there is also the argument that it can always be added back should someone have time to add delayed acks, so I've no strong opinion either way.

Also, when I was writing the routing code - a lot of the design was "borrowed" from the ipv4 routing code. It might be worth doing a comparison to see where the two have diverged (something I used to do now and again) to pick up any bugs I'd inadvertently copied over, if you are working on clean ups in this area,

Steve.

Quoted 8 lines · Adrian Bunk

The patch below contains the following possible cleanups:
- make needlessly global code static
- dn_fib.c: remove the write-only global variable dn_fib_info_cnt
- dn_fib.c: remove the unused global function dn_fib_rt_message
- dn_neigh.c: remove the unused global function dn_neigh_pointopoint_notify
- dn_timer.c: remove the fast timer code that isn't used

Please review and comment on this patch.

author
#2 lore raw
Quoted 1 line · Steven Whitehouse

Hi,

Hi Steven,

Quoted 6 lines

...
Also, when I was writing the routing code - a lot of the design was "borrowed" from the ipv4 routing code. It might be worth doing a comparison to see where the two have diverged (something I used to do now and again) to pick up any bugs I'd inadvertently copied over, if you are working on clean ups in this area,

unfortunately, I'm not working especially in this area.

I'm currently going through the complete kernel sources searching for global code that can be made static or even removed.

Quoted 1 line

Steve.

cu
Adrian

Signature · 4 lines
       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed
#3 lore raw
In reply to Adrian Bunk
Quoted 9 lines · Adrian Bunk

The patch below contains the following possible cleanups:
- make needlessly global code static
- dn_fib.c: remove the write-only global variable dn_fib_info_cnt
- dn_fib.c: remove the unused global function dn_fib_rt_message
- dn_neigh.c: remove the unused global function dn_neigh_pointopoint_notify
- dn_timer.c: remove the fast timer code that isn't used

Please review and comment on this patch.

Looks fine to me. I'm quite happy to lose the fast ack code - unused code is only a confusion to those reading it IMHO. If we do the delayed-ack code in future then it's easy enough to reinstate.

Thanks.

Signature · 1 line
patrick