Skip to contentSkip to conversation

: Support to increment packet dropping statistics.

PATCH Applied Masahide NAKAMURA 3 participants 12 messages Dec 21, 2007 – Jan 8, 2008

Subsystems (4)
lore
Share & tools
lore
#0 lore raw

PATCH 2/3 : Support to increment packet dropping statistics.

---
 net/ipv6/xfrm6_input.c |    3 ++
 net/xfrm/xfrm_input.c  |   41 +++++++++++++++++++++++-------
 net/xfrm/xfrm_output.c |    6 ++++-
 net/xfrm/xfrm_policy.c |   63 ++++++++++++++++++++++++++++++++++++++---------
 4 files changed, 90 insertions(+), 23 deletions(-)
net/ipv6/xfrm6_input.c +3 -0
index f835ab4..6644fc6 100644
@@ -72,6 +72,7 @@ int xfrm6_input_addr(struct sk_buff *skb, xfrm_address_t *daddr,
 
 		sp = secpath_dup(skb->sp);
 		if (!sp) {
+			XFRM_INC_STATS(LINUX_MIB_XFRMINERROR);
 			goto drop;
 		}
 		if (skb->sp)
@@ -80,6 +81,7 @@ int xfrm6_input_addr(struct sk_buff *skb, xfrm_address_t *daddr,
 	}
 
 	if (1 + skb->sp->len == XFRM_MAX_DEPTH) {
+		XFRM_INC_STATS(LINUX_MIB_XFRMINBUFFERERROR);
 		goto drop;
 	}
 
@@ -149,6 +151,7 @@ int xfrm6_input_addr(struct sk_buff *skb, xfrm_address_t *daddr,
 	}
 
 	if (!x) {
+		XFRM_INC_STATS(LINUX_MIB_XFRMINNOSTATES);
 		goto drop;
 	}
 
net/xfrm/xfrm_input.c +31 -10

92 lines not on this page · raw message

net/xfrm/xfrm_output.c +5 -1
index 26fa0cb..867484a 100644
@@ -69,10 +69,13 @@ static int xfrm_output_one(struct sk_buff *skb, int err)
 		err = x->type->output(x, skb);
 
 resume:
-		if (err)
+		if (err) {
+			XFRM_INC_STATS(LINUX_MIB_XFRMOUTSTATEPROTOERROR);
 			goto error_nolock;
+		}
 
 		if (!(skb->dst = dst_pop(dst))) {
+			XFRM_INC_STATS(LINUX_MIB_XFRMOUTERROR);
 			err = -EHOSTUNREACH;
 			goto error_nolock;
 		}
@@ -167,6 +170,7 @@ int xfrm_output(struct sk_buff *skb)
 	if (skb->ip_summed == CHECKSUM_PARTIAL) {
 		err = skb_checksum_help(skb);
 		if (err) {
+			XFRM_INC_STATS(LINUX_MIB_XFRMOUTERROR);
 			kfree_skb(skb);
 			return err;
 		}
net/xfrm/xfrm_policy.c +51 -12

193 lines not on this page · raw message

Signature · 1 line
1.4.4.2
#2 lore raw

[XFRM]: Fix outbound statistics.

In reply to Masahide NAKAMURA

David, I failed to include this statistics codes since I didn't notice the conflict with the latest Herbert XFRM fix.

Please apply this, too.

---
 net/xfrm/xfrm_output.c |   12 +++++++++---
 1 files changed, 9 insertions(+), 3 deletions(-)
net/xfrm/xfrm_output.c +9 -3
index 867484a..8dee031 100644
@@ -43,17 +43,23 @@ static int xfrm_output_one(struct sk_buff *skb, int err)
 
 	do {
 		err = xfrm_state_check_space(x, skb);
-		if (err)
+		if (err) {
 			goto error_nolock;
+			XFRM_INC_STATS(LINUX_MIB_XFRMOUTERROR);
+		}
 
 		err = x->outer_mode->output(x, skb);
-		if (err)
+		if (err) {
+			XFRM_INC_STATS(LINUX_MIB_XFRMOUTSTATEMODEERROR);
 			goto error_nolock;
+		}
 
 		spin_lock_bh(&x->lock);
 		err = xfrm_state_check_expire(x);
-		if (err)
+		if (err) {
+			XFRM_INC_STATS(LINUX_MIB_XFRMOUTSTATEEXPIRED);
 			goto error;
+		}
 
 		if (x->type->flags & XFRM_TYPE_REPLAY_PROT) {
 			XFRM_SKB_CB(skb)->seq = ++x->replay.oseq;
Signature · 1 line
1.4.4.2
#3 lore raw

[XFRM]: Fix outbound statistics.

In reply to Masahide NAKAMURA

Hello David,

I'm sorry, the previous mail contains wrong code:

Quoted 7 lines

David, I failed to include this statistics codes since I didn't notice the conflict with the latest Herbert XFRM fix.

Please apply this, too.

---

[snip]

Review

Quoted 8 lines
 
	do {
	   err = xfrm_state_check_space(x, skb);
(quoted diff) +1 -1
-	        if (err)
+		if (err) {
			goto error_nolock;
(quoted diff) +2 -0
+			XFRM_INC_STATS(LINUX_MIB_XFRMOUTERROR);
+		}

Please apply the below patch instead of the previous.

---
 net/xfrm/xfrm_output.c |   12 +++++++++---
 1 files changed, 9 insertions(+), 3 deletions(-)
net/xfrm/xfrm_output.c +9 -3
index 867484a..f8e5961 100644
@@ -43,17 +43,23 @@ static int xfrm_output_one(struct sk_buff *skb, int err)
 
 	do {
 		err = xfrm_state_check_space(x, skb);
-		if (err)
+		if (err) {
+			XFRM_INC_STATS(LINUX_MIB_XFRMOUTERROR);
 			goto error_nolock;
+		}
 
 		err = x->outer_mode->output(x, skb);
-		if (err)
+		if (err) {
+			XFRM_INC_STATS(LINUX_MIB_XFRMOUTSTATEMODEERROR);
 			goto error_nolock;
+		}
 
 		spin_lock_bh(&x->lock);
 		err = xfrm_state_check_expire(x);
-		if (err)
+		if (err) {
+			XFRM_INC_STATS(LINUX_MIB_XFRMOUTSTATEEXPIRED);
 			goto error;
+		}
 
 		if (x->type->flags & XFRM_TYPE_REPLAY_PROT) {
 			XFRM_SKB_CB(skb)->seq = ++x->replay.oseq;
Signature · 1 line
1.4.4.2
maintainer
#4 lore raw

Re: [XFRM]: Fix outbound statistics.

net/xfrm/xfrm_output.c static int xfrm_output_one(struct sk_buff *skb, int err) · line 53

Quoted 13 lines · Masahide NAKAMURA
net/xfrm/xfrm_output.c +5 -2
 	do {
 		err = xfrm_state_check_space(x, skb);
-		if (err)
+		if (err) {
+			XFRM_INC_STATS(LINUX_MIB_XFRMOUTERROR);
 			goto error_nolock;
+		}
 
 		err = x->outer_mode->output(x, skb);
-		if (err)
+		if (err) {
+			XFRM_INC_STATS(LINUX_MIB_XFRMOUTSTATEMODEERROR);

BTW, none of our existing mode output functions actually return an error. I noticed that the description for this field is actually "Transformation mode specific error, e.g. Outer header space is not enough". This is slightly misleading as output header space is checked by xfrm_state_check_space so if there's an error that's where it'll show up.

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
#5 lore raw

(unknown)

Subject: [XFRM] Documentaion: Fix error example at XFRMOUTSTATEMODEERROR. (Re: [XFRM]: Fix outbound statistics.)

Hello,

Quoted 10 lines · On Fri, 21 Dec 2007 23:11:11 +0800 Herbert Xu <[email protected]>…
Quoted 9 lines · Masahide NAKAMURA
	do {
		err = xfrm_state_check_space(x, skb);
(quoted diff) +2 -1
-		if (err)
+		if (err) {
+			XFRM_INC_STATS(LINUX_MIB_XFRMOUTERROR);
			goto error_nolock;
(quoted diff) +1 -0
+		}
Quoted 11 lines

Review

Quoted 4 lines
		err = x->outer_mode->output(x, skb);
(quoted diff) +2 -1
-		if (err)
+		if (err) {
+			XFRM_INC_STATS(LINUX_MIB_XFRMOUTSTATEMODEERROR);

BTW, none of our existing mode output functions actually return an error. I noticed that the description for this field is actually "Transformation mode specific error, e.g. Outer header space is not enough". This is slightly misleading as output header space is checked by xfrm_state_check_space so if there's an error that's where it'll show up.

Thanks for comment, Herbert.

I fix the documentation to remove "e.g. Outer header space is not enough" from XFRMSTATEMODEERROR.
About error code from xfrm_state_check_space(), I still map it XFRMOUTERROR (other errors) this time because I think the error here is not a length error by protocol (e.g MTU related things) but an internal buffer management.

Any comments for the statistics are still welcomed.

David, please apply the following patch, too.

[XFRM] Documentaion: Fix error example at XFRMOUTSTATEMODEERROR.

---
 Documentation/networking/xfrm_proc.txt |    1 -
 1 files changed, 0 insertions(+), 1 deletions(-)
Documentation/networking/xfrm_proc.txt +0 -1
index ec9045b..53c1a58 100644
@@ -60,7 +60,6 @@ XfrmOutStateProtoError:
 	Transformation protocol specific error
 XfrmOutStateModeError:
 	Transformation mode specific error
-	e.g. Outer header space is not enough
 XfrmOutStateExpired:
 	State is expired
 XfrmOutPolBlock:
Signature · 1 line
1.4.4.2
#6 lore raw

PATCH 1/1 Documentaion: Fix error example at XFRMOUTSTATEMODEERROR. (Re: [XFRM]: Fix outbound statistics.)

In reply to Herbert Xu

# This is resent email since the Subject is blank at the first time;

Hello,

Quoted 10 lines · On Fri, 21 Dec 2007 23:11:11 +0800 Herbert Xu <[email protected]>…
Quoted 9 lines · Masahide NAKAMURA
	do {
		err = xfrm_state_check_space(x, skb);
(quoted diff) +2 -1
-		if (err)
+		if (err) {
+			XFRM_INC_STATS(LINUX_MIB_XFRMOUTERROR);
			goto error_nolock;
(quoted diff) +1 -0
+		}
Quoted 11 lines

Review

Quoted 4 lines
		err = x->outer_mode->output(x, skb);
(quoted diff) +2 -1
-		if (err)
+		if (err) {
+			XFRM_INC_STATS(LINUX_MIB_XFRMOUTSTATEMODEERROR);

BTW, none of our existing mode output functions actually return an error. I noticed that the description for this field is actually "Transformation mode specific error, e.g. Outer header space is not enough". This is slightly misleading as output header space is checked by xfrm_state_check_space so if there's an error that's where it'll show up.

Thanks for comment, Herbert.

I fix the documentation to remove "e.g. Outer header space is not enough" from XFRMSTATEMODEERROR.
About error code from xfrm_state_check_space(), I still map it XFRMOUTERROR (other errors) this time because I think the error here is not a length error by protocol (e.g MTU related things) but an internal buffer management.

Any comments for the statistics are still welcomed.

David, please apply the following patch, too.

[XFRM] Documentaion: Fix error example at XFRMOUTSTATEMODEERROR.

---
 Documentation/networking/xfrm_proc.txt |    1 -
 1 files changed, 0 insertions(+), 1 deletions(-)
Documentation/networking/xfrm_proc.txt +0 -1
index ec9045b..53c1a58 100644
@@ -60,7 +60,6 @@ XfrmOutStateProtoError:
 	Transformation protocol specific error
 XfrmOutStateModeError:
 	Transformation mode specific error
-	e.g. Outer header space is not enough
 XfrmOutStateExpired:
 	State is expired
 XfrmOutPolBlock:
Signature · 1 line
1.4.4.2
#9 lore raw

PATCH 1/1 Statistics: Add outbound-dropping error.

Hello,

I found two more points where they should be incremented as XFRM packet dropping counter. Please apply it.

P.S.
I don't touch XFRM_LOOKUP_ICMP related error at __xfrm_lookup() since it may not drop the packet.
Correct me if it is wrong or comments are welcomed.

[PATCH][XFRM] Statistics: Add outbound-dropping error.

o Increment PolError counter when flow_cache_lookup() returns errored pointer.

o Increment NoStates counter at larval-drop.

---
 net/xfrm/xfrm_policy.c |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)
net/xfrm/xfrm_policy.c +4 -1
index 280f8de..d83227b 100644
@@ -1510,8 +1510,10 @@ restart:
 		policy = flow_cache_lookup(fl, dst_orig->ops->family,
 					   dir, xfrm_policy_lookup);
 		err = PTR_ERR(policy);
-		if (IS_ERR(policy))
+		if (IS_ERR(policy)) {
+			XFRM_INC_STATS(LINUX_MIB_XFRMOUTPOLERROR);
 			goto dropdst;
+		}
 	}
 
 	if (!policy)
@@ -1603,6 +1605,7 @@ restart:
 				/* EREMOTE tells the caller to generate
 				 * a one-shot blackhole route.
 				 */
+				XFRM_INC_STATS(LINUX_MIB_XFRMOUTNOSTATES);
 				xfrm_pol_put(policy);
 				return -EREMOTE;
 			}
Signature · 1 line
1.4.4.2
maintainer
#10 lore raw

Re: [PATCH][XFRM] Statistics: Add outbound-dropping error.

Quoted 5 lines · Masahide NAKAMURA

P.S.
I don't touch XFRM_LOOKUP_ICMP related error at __xfrm_lookup() since it may not drop the packet.
Correct me if it is wrong or comments are welcomed.

Right, whether the packet is dropped would be decided by the caller.

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