xfrm: Fix kernel panic when flush and dump SPD entries

PATCH Superseded Wei Yongjun 4 participants 15 messages Dec 1, 2008 – Dec 3, 2008

Subsystems (3)

Networking Network drivers Networking IPsec

lore

Share & tools

lore

Wei Yongjun

author

2008-12-01 01:59 UTC #0 lore raw

PATCH 1/1 xfrm: Fix kernel panic when flush and dump SPD entries

After flush the SPD entries, dump the SPD entries will cause kernel painc.

Used the following commands to reproduct:

ah/tunnel/3ffe:501:ffff:ff00:200:ff:fe00:b0b0-3ffe:501:ffff:ff02:200:ff:fe00:a1a1/require;\
spddump;' | setkey -c ah/tunnel/3ffe:501:ffff:ff00:200:ff:fe00:b0b0-3ffe:501:ffff:ff02:200:ff:fe00:a1a1/require;\
spddump;' | setkey -c

This is because when flush the SPD entries, the SPD entry is not remove from the list.

This patch fix the problem by remove the SPD entry from the list.

Signed-off-by Wei Yongjun <[email protected]>

---
 net/xfrm/xfrm_policy.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

net/xfrm/xfrm_policy.c +1 -0

index 058f04f..fb216c9 100644
@@ -817,6 +817,7 @@ int xfrm_policy_flush(u8 type, struct xfrm_audit *audit_info)
 				continue;
 			hlist_del(&pol->bydst);
 			hlist_del(&pol->byidx);
+			list_del(&pol->walk.all);
 			write_unlock_bh(&xfrm_policy_lock);
 
 			xfrm_audit_policy_delete(pol, 1, audit_info->loginuid,

Signature · 1 line

1.6.0.2.530.g67faa

Wei Yongjun

author

2008-12-01 02:09 UTC #1 lore raw

PATCH v2 1/1 xfrm: Fix kernel panic when flush and dump SPD entries

After flush the SPD entries, dump the SPD entries will cause kernel painc.

Used the following commands to reproduct:

- echo 'spdflush;' | setkey -c
- echo 'spdadd 3ffe:501:ffff:ff01::/64 3ffe:501:ffff:ff04::/64 any -P out ipsec \
ah/tunnel/3ffe:501:ffff:ff00:200:ff:fe00:b0b0-3ffe:501:ffff:ff02:200:ff:fe00:a1a1/require;\
spddump;' | setkey -c
- echo 'spdflush; spddump;' | setkey -c
- echo 'spdadd 3ffe:501:ffff:ff01::/64 3ffe:501:ffff:ff04::/64 any -P out ipsec \
ah/tunnel/3ffe:501:ffff:ff00:200:ff:fe00:b0b0-3ffe:501:ffff:ff02:200:ff:fe00:a1a1/require;\
spddump;' | setkey -c

This is because when flush the SPD entries, the SPD entry is not remove from the list.

This patch fix the problem by remove the SPD entry from the list.

Signed-off-by Wei Yongjun <[email protected]>

---
 net/xfrm/xfrm_policy.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

net/xfrm/xfrm_policy.c +1 -0

index 058f04f..fb216c9 100644
@@ -817,6 +817,7 @@ int xfrm_policy_flush(u8 type, struct xfrm_audit *audit_info)
 				continue;
 			hlist_del(&pol->bydst);
 			hlist_del(&pol->byidx);
+			list_del(&pol->walk.all);
 			write_unlock_bh(&xfrm_policy_lock);
 
 			xfrm_audit_policy_delete(pol, 1, audit_info->loginuid,

Signature · 1 line

1.6.0.2.530.g67faa

Yang Hongyang

2008-12-01 02:15 UTC #2 lore raw

Quoted 28 lines · Wei Yongjun

After flush the SPD entries, dump the SPD entries will cause kernel painc.
Used the following commands to reproduct:
- echo 'spdflush;' | setkey -c
- echo 'spdadd 3ffe:501:ffff:ff01::/64 3ffe:501:ffff:ff04::/64 any -P out ipsec \
ah/tunnel/3ffe:501:ffff:ff00:200:ff:fe00:b0b0-3ffe:501:ffff:ff02:200:ff:fe00:a1a1/require;\
spddump;' | setkey -c
- echo 'spdflush; spddump;' | setkey -c
- echo 'spdadd 3ffe:501:ffff:ff01::/64 3ffe:501:ffff:ff04::/64 any -P out ipsec \
ah/tunnel/3ffe:501:ffff:ff00:200:ff:fe00:b0b0-3ffe:501:ffff:ff02:200:ff:fe00:a1a1/require;\
spddump;' | setkey -c
This is because when flush the SPD entries, the SPD entry is not remove from the list.
This patch fix the problem by remove the SPD entry from the list.
Signed-off-by Wei Yongjun <[email protected]>
---
 net/xfrm/xfrm_policy.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)
net/xfrm/xfrm_policy.c +0 -0
index 058f04f..fb216c9 100644

net/xfrm/xfrm_policy.c int xfrm_policy_flush(u8 type, struct xfrm_audit *audit_info) · line 823

Quoted 7 lines

net/xfrm/xfrm_policy.c +1 -0

@@ -817,6 +817,7 @@ int xfrm_policy_flush(u8 type, struct xfrm_audit *audit_info)
 				continue;
 			hlist_del(&pol->bydst);
 			hlist_del(&pol->byidx);
+			list_del(&pol->walk.all);
 			write_unlock_bh(&xfrm_policy_lock);
 
 			xfrm_audit_policy_delete(pol, 1, audit_info->loginuid,

Ack.

Herbert Xu

maintainer

2008-12-01 05:00 UTC #3 lore raw

In reply to Wei Yongjun

net/xfrm/xfrm_policy.c int xfrm_policy_flush(u8 type, struct xfrm_audit *audit_info) · line 820

Quoted 10 lines · Wei Yongjun

net/xfrm/xfrm_policy.c +1 -0

index 058f04f..fb216c9 100644
@@ -817,6 +817,7 @@ int xfrm_policy_flush(u8 type, struct xfrm_audit *audit_info)
 				continue;
 			hlist_del(&pol->bydst);
 			hlist_del(&pol->byidx);
+			list_del(&pol->walk.all);

Good catch. In fact there so man occurrences of these three calls that perhaps we should put them (and any other relevant code) in a helper.

Cheers,

Signature · 4 lines

Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Wei Yongjun

author

2008-12-01 06:25 UTC #4 lore raw

Quoted 17 lines · Herbert Xu

net/xfrm/xfrm_policy.c int xfrm_policy_flush(u8 type, struct xfrm_audit *audit_info) · line 821
Quoted 10 lines · Wei Yongjun
net/xfrm/xfrm_policy.c +1 -0
index 058f04f..fb216c9 100644
@@ -817,6 +817,7 @@ int xfrm_policy_flush(u8 type, struct xfrm_audit *audit_info)
 				continue;
 			hlist_del(&pol->bydst);
 			hlist_del(&pol->byidx);
+			list_del(&pol->walk.all);
 
Good catch. In fact there so man occurrences of these three calls that perhaps we should put them (and any other relevant code) in a helper.

I think you mean used __xfrm_policy_unlink() function to instead those codes.

1098 static struct xfrm_policy *__xfrm_policy_unlink(struct xfrm_policy *pol,
1099 int dir)
1100 {
1101 if (hlist_unhashed(&pol->bydst))
1102 return NULL;
1103

1104         hlist_del(&pol->bydst);
1105         hlist_del(&pol->byidx);
1106         list_del(&pol->walk.all);
1107         xfrm_policy_count[dir]--;

1108
1109 return pol;
1110 }

I will post the patch later.

Wei Yongjun

author

2008-12-01 06:26 UTC #5 lore raw

PATCH v3 1/1 xfrm: Fix kernel panic when flush and dump SPD entries

In reply to Herbert Xu

After flush the SPD entries, dump the SPD entries will cause kernel painc.

Used the following commands to reproduct:

This is because when flush the SPD entries, the SPD entry is not remove from the list.

This patch fix the problem by remove the SPD entry from the list. And also do clean up of remove SPD entry.

Signed-off-by Wei Yongjun <[email protected]>

---
net/xfrm/xfrm_policy.c | 35 ++++++++++-------------------------

1 files changed, 10 insertions(+), 25 deletions(-)

net/xfrm/xfrm_policy.c +0 -0

index 058f04f..54e7efe 100644
@@ -63,6 +63,9 @@ static struct xfrm_policy_afinfo *xfrm_policy_get_afinfo(unsigned short family);

static void xfrm_policy_put_afinfo(struct xfrm_policy_afinfo *afinfo); static void xfrm_init_pmtu(struct dst_entry *dst);

+static struct xfrm_policy *__xfrm_policy_unlink(struct xfrm_policy *pol,
+ int dir);
+
static inline int
__xfrm4_selector_match(struct xfrm_selector *sel, struct flowi *fl)
{
@@ -602,12 +605,8 @@ int xfrm_policy_insert(int dir, struct xfrm_policy *policy, int excl)

	xfrm_pol_hold(policy);
	xfrm_policy_count[dir]++;
	atomic_inc(&flow_cache_genid);

- if (delpol) {
- hlist_del(&delpol->bydst);
- hlist_del(&delpol->byidx);
- list_del(&delpol->walk.all);
- xfrm_policy_count[dir]--;
- }

+	if (delpol) +		__xfrm_policy_unlink(delpol, dir);
	policy->index = delpol ? delpol->index : xfrm_gen_index(policy->type, dir);
	hlist_add_head(&policy->byidx, xfrm_policy_byidx+idx_hash(policy->index));
	policy->curlft.add_time = get_seconds();
@@ -679,10 +678,7 @@ struct xfrm_policy *xfrm_policy_bysel_ctx(u8 type, int dir,
					write_unlock_bh(&xfrm_policy_lock);
					return pol;
				}

- hlist_del(&pol->bydst);
- hlist_del(&pol->byidx);
- list_del(&pol->walk.all);
- xfrm_policy_count[dir]--;

+				__xfrm_policy_unlink(pol, dir);
			}
			ret = pol;
			break;
@@ -723,10 +719,7 @@ struct xfrm_policy *xfrm_policy_byid(u8 type, int dir, u32 id, int delete,
					write_unlock_bh(&xfrm_policy_lock);
					return pol;
				}

- hlist_del(&pol->bydst);
- hlist_del(&pol->byidx);
- list_del(&pol->walk.all);
- xfrm_policy_count[dir]--;

+				__xfrm_policy_unlink(pol, dir);
			}
			ret = pol;
			break;
@@ -807,16 +800,14 @@ int xfrm_policy_flush(u8 type, struct xfrm_audit *audit_info)
	for (dir = 0; dir < XFRM_POLICY_MAX; dir++) {
		struct xfrm_policy *pol;
		struct hlist_node *entry;

- int i, killed;
+ int i;

-		killed = 0;
	again1:
		hlist_for_each_entry(pol, entry,
				     &xfrm_policy_inexact[dir], bydst) {
			if (pol->type != type)
				continue;
-			hlist_del(&pol->bydst);
-			hlist_del(&pol->byidx);
+			__xfrm_policy_unlink(pol, dir);
			write_unlock_bh(&xfrm_policy_lock);

			xfrm_audit_policy_delete(pol, 1, audit_info->loginuid,
@@ -824,7 +815,6 @@ int xfrm_policy_flush(u8 type, struct xfrm_audit *audit_info)
						 audit_info->secid);

			xfrm_policy_kill(pol);

- killed++;

			write_lock_bh(&xfrm_policy_lock);
			goto again1;
@@ -837,9 +827,7 @@ int xfrm_policy_flush(u8 type, struct xfrm_audit *audit_info)
					     bydst) {
				if (pol->type != type)
					continue;
-				hlist_del(&pol->bydst);
-				hlist_del(&pol->byidx);
-				list_del(&pol->walk.all);
+				__xfrm_policy_unlink(pol, dir);
				write_unlock_bh(&xfrm_policy_lock);

				xfrm_audit_policy_delete(pol, 1,
@@ -847,14 +835,11 @@ int xfrm_policy_flush(u8 type, struct xfrm_audit *audit_info)
							 audit_info->sessionid,
							 audit_info->secid);
				xfrm_policy_kill(pol);

- killed++;

				write_lock_bh(&xfrm_policy_lock);
				goto again2;
			}
		}
-
-		xfrm_policy_count[dir] -= killed;
	}
	atomic_inc(&flow_cache_genid);

out:

Signature · 1 line

1.6.0.2.530.g67faa

Wei Yongjun

author

2008-12-01 07:49 UTC #6 lore raw

PATCH v4 1/1 xfrm: Fix kernel panic when flush and dump SPD entries

After flush the SPD entries, dump the SPD entries will cause kernel painc.

Used the following commands to reproduct:

This is because when flush the SPD entries, the SPD entry is not remove from the list.

This patch fix the problem by remove the SPD entry from the list. And also do clean up of remove SPD entry.

Signed-off-by Wei Yongjun <[email protected]>

---
 net/xfrm/xfrm_policy.c |   35 ++++++++++-------------------------
 1 files changed, 10 insertions(+), 25 deletions(-)

net/xfrm/xfrm_policy.c +10 ~~-25~~

index 058f04f..54e7efe 100644
@@ -63,6 +63,9 @@ static struct xfrm_policy_afinfo *xfrm_policy_get_afinfo(unsigned short family);
 static void xfrm_policy_put_afinfo(struct xfrm_policy_afinfo *afinfo);
 static void xfrm_init_pmtu(struct dst_entry *dst);
 
+static struct xfrm_policy *__xfrm_policy_unlink(struct xfrm_policy *pol,
+						int dir);
+
 static inline int
 __xfrm4_selector_match(struct xfrm_selector *sel, struct flowi *fl)
 {

@@ -602,12 +605,8 @@ int xfrm_policy_insert(int dir, struct xfrm_policy *policy, int excl)
 	xfrm_pol_hold(policy);
 	xfrm_policy_count[dir]++;
 	atomic_inc(&flow_cache_genid);
-	if (delpol) {
-		hlist_del(&delpol->bydst);
-		hlist_del(&delpol->byidx);
-		list_del(&delpol->walk.all);
-		xfrm_policy_count[dir]--;
-	}
+	if (delpol)
+		__xfrm_policy_unlink(delpol, dir);
 	policy->index = delpol ? delpol->index : xfrm_gen_index(policy->type, dir);
 	hlist_add_head(&policy->byidx, xfrm_policy_byidx+idx_hash(policy->index));
 	policy->curlft.add_time = get_seconds();

@@ -679,10 +678,7 @@ struct xfrm_policy *xfrm_policy_bysel_ctx(u8 type, int dir,
 					write_unlock_bh(&xfrm_policy_lock);
 					return pol;
 				}
-				hlist_del(&pol->bydst);
-				hlist_del(&pol->byidx);
-				list_del(&pol->walk.all);
-				xfrm_policy_count[dir]--;
+				__xfrm_policy_unlink(pol, dir);
 			}
 			ret = pol;
 			break;

@@ -723,10 +719,7 @@ struct xfrm_policy *xfrm_policy_byid(u8 type, int dir, u32 id, int delete,
 					write_unlock_bh(&xfrm_policy_lock);
 					return pol;
 				}
-				hlist_del(&pol->bydst);
-				hlist_del(&pol->byidx);
-				list_del(&pol->walk.all);
-				xfrm_policy_count[dir]--;
+				__xfrm_policy_unlink(pol, dir);
 			}
 			ret = pol;
 			break;

@@ -807,16 +800,14 @@ int xfrm_policy_flush(u8 type, struct xfrm_audit *audit_info)
 	for (dir = 0; dir < XFRM_POLICY_MAX; dir++) {
 		struct xfrm_policy *pol;
 		struct hlist_node *entry;
-		int i, killed;
+		int i;
 
-		killed = 0;
 	again1:
 		hlist_for_each_entry(pol, entry,
 				     &xfrm_policy_inexact[dir], bydst) {
 			if (pol->type != type)
 				continue;
-			hlist_del(&pol->bydst);
-			hlist_del(&pol->byidx);
+			__xfrm_policy_unlink(pol, dir);
 			write_unlock_bh(&xfrm_policy_lock);
 
 			xfrm_audit_policy_delete(pol, 1, audit_info->loginuid,

@@ -824,7 +815,6 @@ int xfrm_policy_flush(u8 type, struct xfrm_audit *audit_info)
 						 audit_info->secid);
 
 			xfrm_policy_kill(pol);
-			killed++;
 
 			write_lock_bh(&xfrm_policy_lock);
 			goto again1;

@@ -837,9 +827,7 @@ int xfrm_policy_flush(u8 type, struct xfrm_audit *audit_info)
 					     bydst) {
 				if (pol->type != type)
 					continue;
-				hlist_del(&pol->bydst);
-				hlist_del(&pol->byidx);
-				list_del(&pol->walk.all);
+				__xfrm_policy_unlink(pol, dir);
 				write_unlock_bh(&xfrm_policy_lock);
 
 				xfrm_audit_policy_delete(pol, 1,

@@ -847,14 +835,11 @@ int xfrm_policy_flush(u8 type, struct xfrm_audit *audit_info)
 							 audit_info->sessionid,
 							 audit_info->secid);
 				xfrm_policy_kill(pol);
-				killed++;
 
 				write_lock_bh(&xfrm_policy_lock);
 				goto again2;
 			}
 		}
-
-		xfrm_policy_count[dir] -= killed;
 	}
 	atomic_inc(&flow_cache_genid);
 out:

Signature · 1 line

1.6.0.2.530.g67faa

Herbert Xu

maintainer

2008-12-01 08:11 UTC #7 lore raw

Quoted 13 lines · Wei Yongjun

After flush the SPD entries, dump the SPD entries will cause kernel painc.
Used the following commands to reproduct:
- echo 'spdflush;' | setkey -c
- echo 'spdadd 3ffe:501:ffff:ff01::/64 3ffe:501:ffff:ff04::/64 any -P out ipsec \
ah/tunnel/3ffe:501:ffff:ff00:200:ff:fe00:b0b0-3ffe:501:ffff:ff02:200:ff:fe00:a1a1/require;\
spddump;' | setkey -c
- echo 'spdflush; spddump;' | setkey -c
- echo 'spdadd 3ffe:501:ffff:ff01::/64 3ffe:501:ffff:ff04::/64 any -P out ipsec \
ah/tunnel/3ffe:501:ffff:ff00:200:ff:fe00:b0b0-3ffe:501:ffff:ff02:200:ff:fe00:a1a1/require;\
spddump;' | setkey -c

Quoted 5 lines

This is because when flush the SPD entries, the SPD entry is not remove from the list.
This patch fix the problem by remove the SPD entry from the list. And also do clean up of remove SPD entry.

I like the clean-up but it's best if you separate the two changes into two patches. That way it's easy to see exactly what the bug fix patch is trying to fix and to verify that the clean-up patch really doesn't change behaviour.

Thanks,

Signature · 4 lines

Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

David Miller

maintainer

2008-12-01 08:38 UTC #8 lore raw

From: Herbert Xu <[email protected]>
Date: Mon, 1 Dec 2008 16:11:11 +0800

Quoted 17 lines · Wei Yongjun

Quoted 16 lines
After flush the SPD entries, dump the SPD entries will cause kernel painc.
Used the following commands to reproduct:
- echo 'spdflush;' | setkey -c
- echo 'spdadd 3ffe:501:ffff:ff01::/64 3ffe:501:ffff:ff04::/64 any -P out ipsec \
ah/tunnel/3ffe:501:ffff:ff00:200:ff:fe00:b0b0-3ffe:501:ffff:ff02:200:ff:fe00:a1a1/require;\
spddump;' | setkey -c
- echo 'spdflush; spddump;' | setkey -c
- echo 'spdadd 3ffe:501:ffff:ff01::/64 3ffe:501:ffff:ff04::/64 any -P out ipsec \
ah/tunnel/3ffe:501:ffff:ff00:200:ff:fe00:b0b0-3ffe:501:ffff:ff02:200:ff:fe00:a1a1/require;\
spddump;' | setkey -c
This is because when flush the SPD entries, the SPD entry is not remove from the list.

Quoted 7 lines

Quoted 2 lines
This patch fix the problem by remove the SPD entry from the list. And also do clean up of remove SPD entry.
I like the clean-up but it's best if you separate the two changes into two patches. That way it's easy to see exactly what the bug fix patch is trying to fix and to verify that the clean-up patch really doesn't change behaviour.

Agreed. Wei, please do this.

Wei Yongjun

author

2008-12-01 09:49 UTC #9 lore raw

PATCH 1/2 xfrm: Fix kernel panic when flush and dump SPD entries

After flush the SPD entries, dump the SPD entries will cause kernel painc.

Used the following commands to reproduct:

This is because when flush the SPD entries, the SPD entry is not remove from the list.

This patch fix the problem by remove the SPD entry from the list.

Signed-off-by Wei Yongjun <[email protected]>

---
 net/xfrm/xfrm_policy.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

net/xfrm/xfrm_policy.c +1 -0

index 058f04f..fb216c9 100644
@@ -817,6 +817,7 @@ int xfrm_policy_flush(u8 type, struct xfrm_audit *audit_info)
 				continue;
 			hlist_del(&pol->bydst);
 			hlist_del(&pol->byidx);
+			list_del(&pol->walk.all);
 			write_unlock_bh(&xfrm_policy_lock);
 
 			xfrm_audit_policy_delete(pol, 1, audit_info->loginuid,

Signature · 1 line

1.6.0.2.530.g67faa

David Miller

maintainer Applied

2008-12-03 08:28 UTC #10 lore raw

From: Wei Yongjun <[email protected]>
Date: Mon, 01 Dec 2008 17:50:52 +0800

Quoted 1 line

After flush the SPD entries, dump the SPD entries will cause kernel painc.

...

Quoted 6 lines

This is because when flush the SPD entries, the SPD entry is not remove from the list.
This patch fix the problem by remove the SPD entry from the list.
Signed-off-by Wei Yongjun <[email protected]>

Applied to net-2.6, thanks a lot Wei.

Wei Yongjun

author

2008-12-01 09:54 UTC #11 lore raw

PATCH 1/2 xfrm: Cleanup for unlink SPD entry

In reply to David Miller

Used __xfrm_policy_unlink() to instead of the dup codes when unlink SPD entry.

Signed-off-by Wei Yongjun <[email protected]>

---
 net/xfrm/xfrm_policy.c |   36 ++++++++++--------------------------
 1 files changed, 10 insertions(+), 26 deletions(-)

net/xfrm/xfrm_policy.c +10 ~~-26~~

index fb216c9..0de2e1f 100644
@@ -63,6 +63,9 @@ static struct xfrm_policy_afinfo *xfrm_policy_get_afinfo(unsigned short family);
 static void xfrm_policy_put_afinfo(struct xfrm_policy_afinfo *afinfo);
 static void xfrm_init_pmtu(struct dst_entry *dst);
 
+static struct xfrm_policy *__xfrm_policy_unlink(struct xfrm_policy *pol,
+						int dir);
+
 static inline int
 __xfrm4_selector_match(struct xfrm_selector *sel, struct flowi *fl)
 {

@@ -602,12 +605,8 @@ int xfrm_policy_insert(int dir, struct xfrm_policy *policy, int excl)
 	xfrm_pol_hold(policy);
 	xfrm_policy_count[dir]++;
 	atomic_inc(&flow_cache_genid);
-	if (delpol) {
-		hlist_del(&delpol->bydst);
-		hlist_del(&delpol->byidx);
-		list_del(&delpol->walk.all);
-		xfrm_policy_count[dir]--;
-	}
+	if (delpol)
+		__xfrm_policy_unlink(delpol, dir);
 	policy->index = delpol ? delpol->index : xfrm_gen_index(policy->type, dir);
 	hlist_add_head(&policy->byidx, xfrm_policy_byidx+idx_hash(policy->index));
 	policy->curlft.add_time = get_seconds();

@@ -679,10 +678,7 @@ struct xfrm_policy *xfrm_policy_bysel_ctx(u8 type, int dir,
 					write_unlock_bh(&xfrm_policy_lock);
 					return pol;
 				}
-				hlist_del(&pol->bydst);
-				hlist_del(&pol->byidx);
-				list_del(&pol->walk.all);
-				xfrm_policy_count[dir]--;
+				__xfrm_policy_unlink(pol, dir);
 			}
 			ret = pol;
 			break;

@@ -723,10 +719,7 @@ struct xfrm_policy *xfrm_policy_byid(u8 type, int dir, u32 id, int delete,
 					write_unlock_bh(&xfrm_policy_lock);
 					return pol;
 				}
-				hlist_del(&pol->bydst);
-				hlist_del(&pol->byidx);
-				list_del(&pol->walk.all);
-				xfrm_policy_count[dir]--;
+				__xfrm_policy_unlink(pol, dir);
 			}
 			ret = pol;
 			break;

@@ -807,17 +800,14 @@ int xfrm_policy_flush(u8 type, struct xfrm_audit *audit_info)
 	for (dir = 0; dir < XFRM_POLICY_MAX; dir++) {
 		struct xfrm_policy *pol;
 		struct hlist_node *entry;
-		int i, killed;
+		int i;
 
-		killed = 0;
 	again1:
 		hlist_for_each_entry(pol, entry,
 				     &xfrm_policy_inexact[dir], bydst) {
 			if (pol->type != type)
 				continue;
-			hlist_del(&pol->bydst);
-			hlist_del(&pol->byidx);
-			list_del(&pol->walk.all);
+			__xfrm_policy_unlink(pol, dir);
 			write_unlock_bh(&xfrm_policy_lock);
 
 			xfrm_audit_policy_delete(pol, 1, audit_info->loginuid,

@@ -825,7 +815,6 @@ int xfrm_policy_flush(u8 type, struct xfrm_audit *audit_info)
 						 audit_info->secid);
 
 			xfrm_policy_kill(pol);
-			killed++;
 
 			write_lock_bh(&xfrm_policy_lock);
 			goto again1;

@@ -838,9 +827,7 @@ int xfrm_policy_flush(u8 type, struct xfrm_audit *audit_info)
 					     bydst) {
 				if (pol->type != type)
 					continue;
-				hlist_del(&pol->bydst);
-				hlist_del(&pol->byidx);
-				list_del(&pol->walk.all);
+				__xfrm_policy_unlink(pol, dir);
 				write_unlock_bh(&xfrm_policy_lock);
 
 				xfrm_audit_policy_delete(pol, 1,

@@ -848,14 +835,11 @@ int xfrm_policy_flush(u8 type, struct xfrm_audit *audit_info)
 							 audit_info->sessionid,
 							 audit_info->secid);
 				xfrm_policy_kill(pol);
-				killed++;
 
 				write_lock_bh(&xfrm_policy_lock);
 				goto again2;
 			}
 		}
-
-		xfrm_policy_count[dir] -= killed;
 	}
 	atomic_inc(&flow_cache_genid);
 out:

Signature · 1 line

1.6.0.2.530.g67faa

Wei Yongjun

author

2008-12-01 09:56 UTC #12 lore raw

Re: [PATCH 1/2] xfrm: Cleanup for unlink SPD entry

Sorry, the title should be this:
[PATCH 2/2] xfrm: Cleanup for unlink SPD entry

Quoted 116 lines · Wei Yongjun

Used __xfrm_policy_unlink() to instead of the dup codes when unlink SPD entry.

Signed-off-by Wei Yongjun <[email protected]>

---
 net/xfrm/xfrm_policy.c |   36 ++++++++++--------------------------
 1 files changed, 10 insertions(+), 26 deletions(-)

net/xfrm/xfrm_policy.c +10 ~~-26~~

index fb216c9..0de2e1f 100644
@@ -63,6 +63,9 @@ static struct xfrm_policy_afinfo *xfrm_policy_get_afinfo(unsigned short family);
 static void xfrm_policy_put_afinfo(struct xfrm_policy_afinfo *afinfo);
 static void xfrm_init_pmtu(struct dst_entry *dst);
 
+static struct xfrm_policy *__xfrm_policy_unlink(struct xfrm_policy *pol,
+						int dir);
+
 static inline int
 __xfrm4_selector_match(struct xfrm_selector *sel, struct flowi *fl)
 {

@@ -602,12 +605,8 @@ int xfrm_policy_insert(int dir, struct xfrm_policy *policy, int excl)
 	xfrm_pol_hold(policy);
 	xfrm_policy_count[dir]++;
 	atomic_inc(&flow_cache_genid);
-	if (delpol) {
-		hlist_del(&delpol->bydst);
-		hlist_del(&delpol->byidx);
-		list_del(&delpol->walk.all);
-		xfrm_policy_count[dir]--;
-	}
+	if (delpol)
+		__xfrm_policy_unlink(delpol, dir);
 	policy->index = delpol ? delpol->index : xfrm_gen_index(policy->type, dir);
 	hlist_add_head(&policy->byidx, xfrm_policy_byidx+idx_hash(policy->index));
 	policy->curlft.add_time = get_seconds();

@@ -679,10 +678,7 @@ struct xfrm_policy *xfrm_policy_bysel_ctx(u8 type, int dir,
 					write_unlock_bh(&xfrm_policy_lock);
 					return pol;
 				}
-				hlist_del(&pol->bydst);
-				hlist_del(&pol->byidx);
-				list_del(&pol->walk.all);
-				xfrm_policy_count[dir]--;
+				__xfrm_policy_unlink(pol, dir);
 			}
 			ret = pol;
 			break;

@@ -723,10 +719,7 @@ struct xfrm_policy *xfrm_policy_byid(u8 type, int dir, u32 id, int delete,
 					write_unlock_bh(&xfrm_policy_lock);
 					return pol;
 				}
-				hlist_del(&pol->bydst);
-				hlist_del(&pol->byidx);
-				list_del(&pol->walk.all);
-				xfrm_policy_count[dir]--;
+				__xfrm_policy_unlink(pol, dir);
 			}
 			ret = pol;
 			break;

@@ -807,17 +800,14 @@ int xfrm_policy_flush(u8 type, struct xfrm_audit *audit_info)
 	for (dir = 0; dir < XFRM_POLICY_MAX; dir++) {
 		struct xfrm_policy *pol;
 		struct hlist_node *entry;
-		int i, killed;
+		int i;
 
-		killed = 0;
 	again1:
 		hlist_for_each_entry(pol, entry,
 				     &xfrm_policy_inexact[dir], bydst) {
 			if (pol->type != type)
 				continue;
-			hlist_del(&pol->bydst);
-			hlist_del(&pol->byidx);
-			list_del(&pol->walk.all);
+			__xfrm_policy_unlink(pol, dir);
 			write_unlock_bh(&xfrm_policy_lock);
 
 			xfrm_audit_policy_delete(pol, 1, audit_info->loginuid,

@@ -825,7 +815,6 @@ int xfrm_policy_flush(u8 type, struct xfrm_audit *audit_info)
 						 audit_info->secid);
 
 			xfrm_policy_kill(pol);
-			killed++;
 
 			write_lock_bh(&xfrm_policy_lock);
 			goto again1;

@@ -838,9 +827,7 @@ int xfrm_policy_flush(u8 type, struct xfrm_audit *audit_info)
 					     bydst) {
 				if (pol->type != type)
 					continue;
-				hlist_del(&pol->bydst);
-				hlist_del(&pol->byidx);
-				list_del(&pol->walk.all);
+				__xfrm_policy_unlink(pol, dir);
 				write_unlock_bh(&xfrm_policy_lock);
 
 				xfrm_audit_policy_delete(pol, 1,

@@ -848,14 +835,11 @@ int xfrm_policy_flush(u8 type, struct xfrm_audit *audit_info)
 							 audit_info->sessionid,
 							 audit_info->secid);
 				xfrm_policy_kill(pol);
-				killed++;
 
 				write_lock_bh(&xfrm_policy_lock);
 				goto again2;
 			}
 		}
-
-		xfrm_policy_count[dir] -= killed;
 	}
 	atomic_inc(&flow_cache_genid);
 out:

Herbert Xu

maintainer

2008-12-02 06:53 UTC #13 lore raw

Re: [PATCH 1/2] xfrm: Cleanup for unlink SPD entry

In reply to Wei Yongjun

Quoted 4 lines · Wei Yongjun

Used __xfrm_policy_unlink() to instead of the dup codes when unlink SPD entry.
Signed-off-by Wei Yongjun <[email protected]>

Both patches look good to me. Thanks!

Signature · 4 lines

Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

David Miller

maintainer

2008-12-03 13:24 UTC #14 lore raw

Re: [PATCH 1/2] xfrm: Cleanup for unlink SPD entry

In reply to Wei Yongjun

From: Wei Yongjun <[email protected]>
Date: Mon, 01 Dec 2008 17:55:20 +0800

Quoted 4 lines

Used __xfrm_policy_unlink() to instead of the dup codes when unlink SPD entry.
Signed-off-by Wei Yongjun <[email protected]>

I forwarded ported this to net-next-2.6 and added it there.

Thanks!