Re: [PATCH][XFRM] Optimize policy dumping

[Patrick McHardy <kaber@trash.net>]

[-- Attachment #1: Type: text/plain, Size: 831 bytes --]

Patrick McHardy wrote:
> jamal wrote:
> 
>>All the way down, you call func with a NULL entry. You could add a check
>>to make sure it only gets invoked when last is not null, but the result
>>is in such a case, you will never send a 0 count element. I am sure
>>there could be other tricky scenarios like this that could be
>>constructed.
>>
>>Thoughts.
> 
> 
> Double sending can't happen, but you're right about potentially
> sending a NULL ptr when after setting it to NULL we don't find
> any other matching elements.
> 
> This patch should fix it (and is even simpler), by moving the
> check for pol->type != type before sending, we make sure that
> last always contains a valid element unless count == 0.
> 
> Also fixed an incorrect gcc warning about last_dir potentially
> being used uninitialized.


And again for SAs ..


[-- Attachment #2: x --]
[-- Type: text/plain, Size: 1193 bytes --]

diff --git a/net/xfrm/xfrm_state.c b/net/xfrm/xfrm_state.c
index 864962b..a5877f8 100644
--- a/net/xfrm/xfrm_state.c
+++ b/net/xfrm/xfrm_state.c
@@ -1099,7 +1099,7 @@ int xfrm_state_walk(u8 proto, int (*func
 		    void *data)
 {
 	int i;
-	struct xfrm_state *x;
+	struct xfrm_state *x, *last = NULL;
 	struct hlist_node *entry;
 	int count = 0;
 	int err = 0;
@@ -1107,24 +1107,21 @@ int xfrm_state_walk(u8 proto, int (*func
 	spin_lock_bh(&xfrm_state_lock);
 	for (i = 0; i <= xfrm_state_hmask; i++) {
 		hlist_for_each_entry(x, entry, xfrm_state_bydst+i, bydst) {
-			if (xfrm_id_proto_match(x->id.proto, proto))
-				count++;
+			if (!xfrm_id_proto_match(x->id.proto, proto))
+				continue;
+			if (last) {
+				err = func(last, ++count, data);
+				if (err)
+					goto out;
+			}
+			last = x;
 		}
 	}
 	if (count == 0) {
 		err = -ENOENT;
 		goto out;
 	}
-
-	for (i = 0; i <= xfrm_state_hmask; i++) {
-		hlist_for_each_entry(x, entry, xfrm_state_bydst+i, bydst) {
-			if (!xfrm_id_proto_match(x->id.proto, proto))
-				continue;
-			err = func(x, --count, data);
-			if (err)
-				goto out;
-		}
-	}
+	err = func(last, 0, data);
 out:
 	spin_unlock_bh(&xfrm_state_lock);
 	return err;