Skip to contentSkip to conversation

: Avoid useless iterating in netlink dump functions

NET Patrick McHardy 2 participants 6 messages May 6, 2005 – May 7, 2005

Subsystems (3)
Networking Network drivers IPv4/IPv6
lore
Share & tools
lore

Patrick McHardy

author
#0 lore raw

This patch changes a couple of places like this:

       for (h = 0; h <= tbl->hash_mask; h++) {
               if (h < s_h)
                       continue;

to this:

       for (h = s_h; h <= tbl->hash_mask; h++) {

The only difference is that we can now get past the loop with h > tbl->hash_mask if hash_mask was decreased between two callbacks and h was already past the new value. But it still behaves identical, nothing is dumped.

[NET]: Avoid useless iterating in netlink dump functions

Signed-off-by Patrick McHardy <[email protected]>

---
commit 4c2ae3efd96b7c6ed7ae62d820c13e95ba8564f1 tree 9f5362e759864bc9b3185ba39b0441952b0b8468 parent 6a800d456a81a9046634bcd26d868fd537f0c9ae author Patrick McHardy <[email protected]> 1115327686 +0200 committer Patrick McHardy <[email protected]> 1115327686 +0200

net/core/neighbour.c (mode:100644 sha1:71d444de4bb752511ea838710248130d1559eb4e) renamed from net/core/neighbour.c (mode:100644 sha1:43bdc521e20d9564ccec6472b30d95328e7d1329) +1 -3
Index: net/core/neighbour.c
===================================================================
@@ -1598,9 +1598,7 @@
 	int rc, h, s_h = cb->args[1];
 	int idx, s_idx = idx = cb->args[2];
 
-	for (h = 0; h <= tbl->hash_mask; h++) {
-		if (h < s_h)
-			continue;
+	for (h = s_h; h <= tbl->hash_mask; h++) {
 		if (h > s_h)
 			s_idx = 0;
 		read_lock_bh(&tbl->lock);
net/core/rtnetlink.c (mode:100644 sha1:bfb5b8efa710524ba4ee7cd14acf7c49a6fac004) renamed from net/core/rtnetlink.c (mode:100644 sha1:00caf4b318b20831c8fad5226c7c3cd358b4995b) +2 -2
Index: net/core/rtnetlink.c
===================================================================
@@ -419,9 +419,9 @@
 
 	if (s_idx == 0)
 		s_idx = 1;
-	for (idx=1; idx<NPROTO; idx++) {
+	for (idx = s_idx; idx < NPROTO; idx++) {
 		int type = cb->nlh->nlmsg_type-RTM_BASE;
-		if (idx < s_idx || idx == PF_PACKET)
+		if (idx == PF_PACKET)
 			continue;
 		if (rtnetlink_links[idx] == NULL ||
 		    rtnetlink_links[idx][type].dumpit == NULL)
net/decnet/dn_route.c (mode:100644 sha1:2b2bad3e4f902a8fb595596c35263c96e7c10779) renamed from net/decnet/dn_route.c (mode:100644 sha1:1e7b5c3ea2154d94251d04ea0de69fda06f9f58e) +1 -3
Index: net/decnet/dn_route.c
===================================================================
@@ -1631,9 +1631,7 @@
 
 	s_h = cb->args[0];
 	s_idx = idx = cb->args[1];
-	for(h = 0; h <= dn_rt_hash_mask; h++) {
-		if (h < s_h)
-			continue;
+	for(h = s_h; h <= dn_rt_hash_mask; h++) {
 		if (h > s_h)
 			s_idx = 0;
 		rcu_read_lock_bh();
net/decnet/dn_table.c (mode:100644 sha1:23d106be9fdf41e4afd1e83377607084c08e8c68) renamed from net/decnet/dn_table.c (mode:100644 sha1:dad5603912be3ed4959fb4d5565c9df4d5830f25) +1 -3
Index: net/decnet/dn_table.c
===================================================================
@@ -394,9 +394,7 @@
 	int h, s_h;
 
 	s_h = cb->args[2];
-	for(h = 0; h < dz->dz_divisor; h++) {
-		if (h < s_h)
-			continue;
+	for(h = s_h; h < dz->dz_divisor; h++) {
 		if (h > s_h)
 			memset(&cb->args[3], 0, sizeof(cb->args) - 3*sizeof(cb->args[0]));
 		if (dz->dz_hash == NULL || dz->dz_hash[h] == NULL)
net/ipv4/fib_frontend.c (mode:100644 sha1:f398b2fc63c1c48cdbfb20fae330ff3a60e51ed8) renamed from net/ipv4/fib_frontend.c (mode:100644 sha1:563e7d61270612b64a5f67684c5eea1662b66a31) +1 -2
Index: net/ipv4/fib_frontend.c
===================================================================
@@ -344,8 +344,7 @@
 	if (s_t == 0)
 		s_t = cb->args[0] = RT_TABLE_MIN;
 
-	for (t=s_t; t<=RT_TABLE_MAX; t++) {
-		if (t < s_t) continue;
+	for (t = s_t; t <= RT_TABLE_MAX; t++) {
 		if (t > s_t)
 			memset(&cb->args[1], 0, sizeof(cb->args)-sizeof(cb->args[0]));
 		if ((tb = fib_get_table(t))==NULL)
net/ipv4/fib_hash.c (mode:100644 sha1:c1cebf82001b3a2c9d512c75b769baf0d4b6888f) renamed from net/ipv4/fib_hash.c (mode:100644 sha1:6506dcc01b460e7c5af35ff756f4819c23bd5d30) +1 -2
Index: net/ipv4/fib_hash.c
===================================================================
@@ -723,8 +723,7 @@
 	int h, s_h;
 
 	s_h = cb->args[2];
-	for (h=0; h < fz->fz_divisor; h++) {
-		if (h < s_h) continue;
+	for (h = s_h; h < fz->fz_divisor; h++) {
 		if (h > s_h)
 			memset(&cb->args[3], 0,
 			       sizeof(cb->args) - 3*sizeof(cb->args[0]));
net/ipv4/route.c (mode:100644 sha1:6a96ebfb4a1a23f7c6ce8a743160b3ae3e74020e) renamed from net/ipv4/route.c (mode:100644 sha1:199311746932ee3952abebc5d008b8c9daf9c11b) +1 -2
Index: net/ipv4/route.c
===================================================================
@@ -2770,8 +2770,7 @@
 
 	s_h = cb->args[0];
 	s_idx = idx = cb->args[1];
-	for (h = 0; h <= rt_hash_mask; h++) {
-		if (h < s_h) continue;
+	for (h = s_h; h <= rt_hash_mask; h++) {
 		if (h > s_h)
 			s_idx = 0;
 		rcu_read_lock_bh();
#1 lore raw
Quoted 6 lines · Patrick McHardy

This patch changes a couple of places like this:

       for (h = 0; h <= tbl->hash_mask; h++) {
               if (h < s_h)
                       continue;
Quoted 9 lines

to this:

       for (h = s_h; h <= tbl->hash_mask; h++) {

The only difference is that we can now get past the loop with h > tbl->hash_mask if hash_mask was decreased between two callbacks and h was already past the new value. But it still behaves identical, nothing is dumped.

Looks good - you may also wanna treat tcf_dump_walker() and xfrm_dump_sa() the same way then.

cheers,
jamal

author
#2 lore raw
Quoted 3 lines · jamal

Looks good - you may also wanna treat tcf_dump_walker() and xfrm_dump_sa() the same way then.

action dumping is on my list for more treatment than this :) xfrm_dump_sa()/xfrm_state_walk() could be changed to skip smarter, but that is a different kind of change.

Regards
Patrick

#3 lore raw
Quoted 6 lines · Patrick McHardy
Quoted 3 lines · jamal

Looks good - you may also wanna treat tcf_dump_walker() and xfrm_dump_sa() the same way then.

action dumping is on my list for more treatment than this :)

Well, that does sound scary ;-> What do you have in mind. Recall, there are two paths to dumping actions - unlike any other thing in the kernel.

Quoted 3 lines

xfrm_dump_sa()/xfrm_state_walk() could be changed to skip smarter, but that is a different kind of change.

I noticed the policy too is of the the same way. Those are actually a little more trickier than they appear.

cheers,
jamal

author
#4 lore raw
Quoted 7 lines · jamal
Quoted 1 line · Patrick McHardy

action dumping is on my list for more treatment than this :)

Well, that does sound scary ;-> What do you have in mind. Recall, there are two paths to dumping actions - unlike any other thing in the kernel.

Don't worry :) Its actually action walking that is on my list. What I have planed is roughly:
- clean up and move hashing functions to act_common.c
- put data needed for hashing (size, pointer to memory, ...) in struct tc_act_common, which is referenced by struct tc_action_ops
- add callback based walking function so we don't need two of them

This should also get rid of all the large functions contained in headerfiles.

Regards
Patrick

#5 lore raw
Quoted 16 lines · Patrick McHardy
Quoted 7 lines · jamal
Quoted 1 line · Patrick McHardy

action dumping is on my list for more treatment than this :)

Well, that does sound scary ;-> What do you have in mind. Recall, there are two paths to dumping actions - unlike any other thing in the kernel.

Don't worry :) Its actually action walking that is on my list. What I have planed is roughly:
- clean up and move hashing functions to act_common.c
- put data needed for hashing (size, pointer to memory, ...) in struct tc_act_common, which is referenced by struct tc_action_ops
- add callback based walking function so we don't need two of them

Quoted 3 lines

This should also get rid of all the large functions contained in headerfiles.

Ah, ok ;->
Weve discussed this already - I think DaveM already took in the simple action patch that i posted a while back; it doesnt seem like you will collide with that - just double check.

cheers,
jamal