Question about an assignment in handle_ing()

Question about an assignment in handle_ing()

[Jiri Pirko]

Hi Jamal.

I want to ask you about the following chunk of code of net/core/dev.c in function handle_ing():

2699        if (*pt_prev) {
2700                *ret = deliver_skb(skb, *pt_prev, orig_dev);
2701                *pt_prev = NULL;
2702        } else {
2703                /* Huh? Why does turning on AF_PACKET affect this? */
2704                skb->tc_verd = SET_TC_OK2MUNGE(skb->tc_verd);
2705        }

The assignment (in "else" statement) was added by you here:
http://linux.bkbits.net:8080/linux-2.6.12-stable/?PAGE=cset&REV=40cfc085xjyQLyB1UiTbgaRZSlCN9w

The comment was added after move of this code by Herbert:
http://git.kernel.org/?p=linux/kernel/git/davem/net-next-2.6.git;a=commitdiff;h=f697c3e8b35c18b2698d64137c0fa84b0cdb3d10

Question is if this code is correct here. Maybe I'm missing something but
why is this dependent on a ptype was found previously?

Thanks,

	Jirka

Re: Question about an assignment in handle_ing()

[jamal]

Hi Jiri,

On Mon, 2010-05-24 at 13:22 +0200, Jiri Pirko wrote:

> Question is if this code is correct here. Maybe I'm missing something but
> why is this dependent on a ptype was found previously?

The code is correct. 

Main reason for the else condn is driven by optimization:
If you are running tcpdump or other af packet type code, the "if" condn
is hit and matching actions are not allowed trample on that same packet
data. They have to make a private copy; otherwise, the "else" is hit and
(for optimization reason) you give ok to the actions that follow to
munge the packet. Essentially, you dont want actions to alloc/copy every
single time when you are not running tcpdump for example; reason is that
most of the time you run tcpdump it is for debugging.
[I had seen very observable differences on some old mips board back in
the day on whether you avoided copy every time vs when debugging by
running tcpdump and copied every packet.]

There is a secondary reason for the else stmnt: you want tcpdump to see
the naked packet as it came on say eth0. I am in travel mode at the
moment, so i cant validate this for you via a testcase (which moves the
else outside), but i know this was a problem back then...
Example. If i had a packet sequence path as follows: 
--> eth0-->tcpdump0-->filter match --> action: edit -->action: redirect
to dummy0-->tcpdump1-->dummy0(drop and count)
Then you want the packet on tcpdump0 to be whatever was seen by eth0.
You want to see on tcpdump1 whatever was seen by dummy0 - which is an
edited version of whatever was seen by eth0. 

If this description makes sense to you (and since this has come up more
than once before), would you be kind enough to submit a patch that fixes
the current comment and add my ACK to it?

cheers,
jamal

Re: Question about an assignment in handle_ing()

[Herbert Xu]

On Tue, May 25, 2010 at 05:51:07AM -0400, jamal wrote:
> The code is correct. 
> 
> Main reason for the else condn is driven by optimization:
> If you are running tcpdump or other af packet type code, the "if" condn
> is hit and matching actions are not allowed trample on that same packet
> data. They have to make a private copy; otherwise, the "else" is hit and
> (for optimization reason) you give ok to the actions that follow to
> munge the packet. Essentially, you dont want actions to alloc/copy every
> single time when you are not running tcpdump for example; reason is that
> most of the time you run tcpdump it is for debugging.
> [I had seen very observable differences on some old mips board back in
> the day on whether you avoided copy every time vs when debugging by
> running tcpdump and copied every packet.]

In that case you should be checking whether the skb is cloned.
After all, tcpdump might have simply filtered the packet out.

BTW, this is the case whenever you run a DHCP client/server.  So
on most boxes your optimisation will never kick in as is.  Also
the skb may still be cloned even if there is no AF_PACKET listener.
In that case your optimisation may be incorrect.

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: Question about an assignment in handle_ing()

[jamal]

On Tue, 2010-05-25 at 20:26 +1000, Herbert Xu wrote:

> In that case you should be checking whether the skb is cloned.

That is the general rule used (and what i specify to do in the docs)..
but i recall there were issues if the packet path emanated from ingress
and included multiple netdevices (earlier ex with mirror applies). There
may have been bugs then, eg I could not assume that it i had any ptype
at all that the ptype will clone the packet. Does tcpdump guarantee
skb->clone being set? I will try to test some scenarios when i am back
+settled. 

> After all, tcpdump might have simply filtered the packet out.

True - but i think thats an acceptable compromise.

> BTW, this is the case whenever you run a DHCP client/server.  So
> on most boxes your optimisation will never kick in as is.  

"Most" for people running serious firewalls or routers is not to run
DHCP servers;-> They may client, but thats a short-lived session.

> Also
> the skb may still be cloned even if there is no AF_PACKET listener.
> In that case your optimisation may be incorrect.

Did you mean that as long as there are other ptypes - which may or 
not be doing af packet? 

cheers,
jamal

Re: Question about an assignment in handle_ing()

[Herbert Xu]

On Tue, May 25, 2010 at 08:03:44AM -0400, jamal wrote:
> On Tue, 2010-05-25 at 20:26 +1000, Herbert Xu wrote:
> 
> > In that case you should be checking whether the skb is cloned.
> 
> That is the general rule used (and what i specify to do in the docs)..
> but i recall there were issues if the packet path emanated from ingress
> and included multiple netdevices (earlier ex with mirror applies). There
> may have been bugs then, eg I could not assume that it i had any ptype
> at all that the ptype will clone the packet. Does tcpdump guarantee
> skb->clone being set? I will try to test some scenarios when i am back
> +settled. 

It is guaranteed, it is illegal to hold onto the skb contents
without the reference that comes from cloning an skb.

> "Most" for people running serious firewalls or routers is not to run
> DHCP servers;-> They may client, but thats a short-lived session.

The DHCP client may keep the AF_PACKET open even if there is no
ongoing dialogue.

> > Also
> > the skb may still be cloned even if there is no AF_PACKET listener.
> > In that case your optimisation may be incorrect.
> 
> Did you mean that as long as there are other ptypes - which may or 
> not be doing af packet? 

For ingress, the packet may have looped back from a virtual
interface.  It may have been cloned on its way out of the virtual
interface.

Looking at ptype instead of checking whether the skb is cloned
simply doesn't work in this case.

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: Question about an assignment in handle_ing()

[jamal]

On Tue, 2010-05-25 at 22:12 +1000, Herbert Xu wrote:

> It is guaranteed, it is illegal to hold onto the skb contents
> without the reference that comes from cloning an skb.

As i recall this was an issue. And the example i described earlier
would have tcpdump show edited contents coming out of eth0.

> The DHCP client may keep the AF_PACKET open even if there is no
> ongoing dialogue.

Ok ;-> Not running that dhcp client without looking at how it behaves.

> For ingress, the packet may have looped back from a virtual
> interface.  It may have been cloned on its way out of the virtual
> interface.

and i think this is where you and i ended last time we talked.

> Looking at ptype instead of checking whether the skb is cloned
> simply doesn't work in this case.

I will test in a few days - or if Jiri has the bandwidth i think i can
describe what to test (I have the relevant tests logged somewhere).

I apologize there may be some large latency of about a day before my
next response.

cheers,
jamal

Re: Question about an assignment in handle_ing()

[Herbert Xu]

On Tue, May 25, 2010 at 08:20:38AM -0400, jamal wrote:
> On Tue, 2010-05-25 at 22:12 +1000, Herbert Xu wrote:
> 
> > It is guaranteed, it is illegal to hold onto the skb contents
> > without the reference that comes from cloning an skb.
> 
> As i recall this was an issue. And the example i described earlier
> would have tcpdump show edited contents coming out of eth0.

That's not very surprising as you're not checking whether the
skb is cloned in act_pedit.c:

	if (!(skb->tc_verd & TC_OK2MUNGE)) {
		/* should we set skb->cloned? */
		if (pskb_expand_head(skb, 0, 0, GFP_ATOMIC)) {
			return p->tcf_action;
		}
	}

This code needs to be changed to something like

	if (skb_cloned(skb)) {
		if (pskb_expand_head(skb, 0, 0, GFP_ATOMIC)) {
			return p->tcf_action;
		}
	}

BTW, this error handling also looks a bit suss.  Shouldn't it
drop the packet instead of continuing as if we have successfully
modified it?

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: Question about an assignment in handle_ing()

[jamal]

On Tue, 2010-05-25 at 22:46 +1000, Herbert Xu wrote:

> That's not very surprising as you're not checking whether the
> skb is cloned in act_pedit.c:

I meant the test "if (skb_cloned(skb))" failed in such cases;-> So you
couldnt reliably use it. 
If it turns out it is unnecessary, what you describe is what i had in
mind as well.

> BTW, this error handling also looks a bit suss.  Shouldn't it
> drop the packet instead of continuing as if we have successfully
> modified it?

It is not the responsibility of the action to drop packets in a pipeline
rather the responsibility is that of the caller (ref: rule #3 in
Documentation/networking/tc-actions-env-rules.txt). What to do on a
failure such as above is programmable by the action user/admin.

Anyways, now i am really gone. Dont make me look in again Herbert ;->

cheers,
jamal

Re: Question about an assignment in handle_ing()

[Herbert Xu]

On Tue, May 25, 2010 at 09:13:36AM -0400, jamal wrote:
> On Tue, 2010-05-25 at 22:46 +1000, Herbert Xu wrote:
> 
> > That's not very surprising as you're not checking whether the
> > skb is cloned in act_pedit.c:
> 
> I meant the test "if (skb_cloned(skb))" failed in such cases;-> So you
> couldnt reliably use it. 
> If it turns out it is unnecessary, what you describe is what i had in
> mind as well.

If it did happen like you said then it would be a serious bug
in our stack as everything else (including the TCP stack) relies
on this.

You can't just make up your own rules :)
 
> It is not the responsibility of the action to drop packets in a pipeline
> rather the responsibility is that of the caller (ref: rule #3 in
> Documentation/networking/tc-actions-env-rules.txt). What to do on a
> failure such as above is programmable by the action user/admin.

But how can the caller make that decision when you return exactly
the same value in the error case as the normal case?

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: Question about an assignment in handle_ing()

[jamal]

On Wed, 2010-05-26 at 09:13 +1000, Herbert Xu wrote:

> If it did happen like you said then it would be a serious bug
> in our stack as everything else (including the TCP stack) relies
> on this.

It could have been a bug. Note this was not a simple test, so there
may be other factors involved. If you or Jiri are willing to run the
test i will construct a scenario which will test this out. It will need
a compile of the kernel and a small check in pedit to see if we see
cloned skbs when we run the two tcpdumps (and to make sure the tcpdumps
see the correct bytes). Otherwise i will get to it by weekend.
BTW: Jiri, out of curiosity - what was the issue seen that caused the
original question?

> But how can the caller make that decision when you return exactly
> the same value in the error case as the normal case?

Ok - i see your point Herbert ;-> 
it makes sense to have pedit have an error action code like some of the
others actions which defaults to a drop.
I will do a proper patch sometime this weekend.

cheers,
jamal

Re: Question about an assignment in handle_ing()

[jamal]

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

On Wed, 2010-05-26 at 07:38 -0400, jamal wrote:
> On Wed, 2010-05-26 at 09:13 +1000, Herbert Xu wrote:
> 
> > If it did happen like you said then it would be a serious bug
> > in our stack as everything else (including the TCP stack) relies
> > on this.
> 
> It could have been a bug. Note this was not a simple test, so there
> may be other factors involved. If you or Jiri are willing to run the
> test i will construct a scenario which will test this out. It will need
> a compile of the kernel and a small check in pedit to see if we see
> cloned skbs when we run the two tcpdumps (and to make sure the tcpdumps
> see the correct bytes). Otherwise i will get to it by weekend.

I have constructed a test case (attached) and my fear is unfortunately
still there;-< What am i doing wrong?

The packet path is:
-->eth0-->tcpdump eth0-->pedit-->mirror to dummy0-->tcpdump dummy0

I expect pedit to see a cloned packet. It doesnt. The check is in
tcf_pedit(), just before "if (!(skb->tc_verd & TC_OK2MUNGE))"
added: 
printk("pedit: skb-%p is %s\n",skb,skb_cloned(skb)?"cloned":"!cloned");

Is pf packet not cloning etc? Sorry, I dont have much time today
to dig into the code - but i figure youd know the answer.

> > But how can the caller make that decision when you return exactly
> > the same value in the error case as the normal case?
> 
> Ok - i see your point Herbert ;-> 
> it makes sense to have pedit have an error action code like some of the
> others actions which defaults to a drop.
> I will do a proper patch sometime this weekend.

I will get it done this week.

cheers,
jamal

[-- Attachment #2: jiri-q-test --]
[-- Type: text/plain, Size: 1855 bytes --]


machine running script is 10.0.0.111 receiving on eth0.
We are pinging from 10.0.0.26 to 10.0.0.111.
On 10.0.0.111:
1)Edit the packet when it comes in to change src/dst mac addresses
2)Mirror copy to dummy0 

mirror to dummy0 is useful for debugging (ifconfig shows you stats and you
can run tcpdump to log the copies as we do)

run tcpdump before #1 and after #1 - this way we see the original
packet at eth0 and the modified packet at dummy0.

-------------- start script on 10.0.0.111 -----
tc qdisc del dev eth0 ingress     
tc qdisc add dev eth0 ingress     
ifconfig dummy0 up
tc filter add dev eth0 parent ffff: protocol ip prio 10 u32 \
match ip protocol 1 0xff flowid 1:2 \
action pedit \
munge offset -12 u32 set 0x00010100 \
munge offset -8 u32 set 0x0aaf0100 \
munge offset -4 u32 set 0x00080800 pipe \
action mirred egress mirror dev dummy0
------

To validate you did this right, dumping should look as follows:
----
filter protocol ip pref 10 u32 
filter protocol ip pref 10 u32 fh 800: ht divisor 1 
filter protocol ip pref 10 u32 fh 800::800 order 2048 key ht 800 bkt 0 flowid 1:2 
  match 00010000/00ff0000 at 8
	action order 1:  pedit action pipe keys 3
 	 index 1 ref 1 bind 1
	 key #0  at -12: val 00010100 mask 00000000
	 key #1  at -8: val 0aaf0100 mask 00000000
	 key #2  at -4: val 00080800 mask 00000000
 
	action order 2: mirred (Egress Mirror to device dummy0) pipe
 	index 1 ref 1 bind 1
-----

tcpdump on dummy0 (showing modified macs):
 
0a:af:01:00:00:08 > 52:54:00:01:01:00, ethertype IPv4 (0x0800), length 98: 10.0.0.26 > 10.0.0.111: ICMP echo request, id 5981, seq 1, length 64
	0x0000:  4500 0054 0000 4000 4001 2621 0a00 001a
	0x0010:  0a00 006f 0800 a951 175d 0001 d3c8 fa4b
	0x0020:  0000 0000 9d68 0d00 0000 0000 1011 1213
	0x0030:  1415 1617 1819 1a1b 1c1d 1e1f 2021 2223
	0x0040:  2425 2627 2829 2a2b 2c2d 2e2f

Re: Question about an assignment in handle_ing()

[Herbert Xu]

On Sun, May 30, 2010 at 09:29:10AM -0400, jamal wrote:
>
> I have constructed a test case (attached) and my fear is unfortunately
> still there;-< What am i doing wrong?
> 
> The packet path is:
> -->eth0-->tcpdump eth0-->pedit-->mirror to dummy0-->tcpdump dummy0

Well this doesn't guarantee a cloned packet at all.  Once af_packet
receives the packet it'll wake up any listeners like tcpdump, if
tcpdump gets to it before pedit runs then the packet won't be
cloned anymore.

Anyway, I don't see why actions are special.  Everybody else lives
by the rule that cloned skbs are not writeable.  So if this was
indeed buggy as you say it would have shown up a long time ago.

Case in point, we had a bug in certain NIC drivers where they
modified cloned skbs for TSO.  This quickly showed up as bogus
packets in tcpdump and we fixed it.

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: Question about an assignment in handle_ing()

[jamal]

On Thu, 2010-06-03 at 18:01 +1000, Herbert Xu wrote:
> On Sun, May 30, 2010 at 09:29:10AM -0400, jamal wrote:

> > The packet path is:
> > -->eth0-->tcpdump eth0-->pedit-->mirror to dummy0-->tcpdump dummy0
> 
> Well this doesn't guarantee a cloned packet at all.  Once af_packet
> receives the packet it'll wake up any listeners like tcpdump, if
> tcpdump gets to it before pedit runs then the packet won't be
> cloned anymore.

I may be misreading, but:
This is the point i have been trying to make, Herbert;-> There is no
_guarantee_ that the first tcpdump will see the packet that came out of
eth0 instead of seeing the packet that came out the pedit part of the
pipeline. I need to see the correct packet. I know with my check
this is guaranteed.

> Anyway, I don't see why actions are special.  Everybody else lives
> by the rule that cloned skbs are not writeable.  

Yes, if skb_cloned() is true but it is not as i said in my earlier
email.

> So if this was
> indeed buggy as you say it would have shown up a long time ago.

Things may have been buggy - I dont know; you just validated to me that
it _may_ happen. I will be more than happy to remove it if i can get a
guarantee.
So how do we fix this?
Does af_packet need to always clone? That way i can depend on it. I have
a feeling someone will be unhappy with that. I am avoiding to clone
every packet on my part because afaik this problem doesnt exist if i
dont use tcpdump/af_packet...

> Case in point, we had a bug in certain NIC drivers where they
> modified cloned skbs for TSO.  This quickly showed up as bogus
> packets in tcpdump and we fixed it.

I think this is different.

cheers,
jamal

Re: Question about an assignment in handle_ing()

[Herbert Xu]

On Thu, Jun 03, 2010 at 08:43:50AM -0400, jamal wrote:
> 
> On Thu, 2010-06-03 at 18:01 +1000, Herbert Xu wrote:
> > On Sun, May 30, 2010 at 09:29:10AM -0400, jamal wrote:
> 
> > > The packet path is:
> > > -->eth0-->tcpdump eth0-->pedit-->mirror to dummy0-->tcpdump dummy0
> > 
> > Well this doesn't guarantee a cloned packet at all.  Once af_packet
> > receives the packet it'll wake up any listeners like tcpdump, if
> > tcpdump gets to it before pedit runs then the packet won't be
> > cloned anymore.
> 
> I may be misreading, but:
> This is the point i have been trying to make, Herbert;-> There is no
> _guarantee_ that the first tcpdump will see the packet that came out of
> eth0 instead of seeing the packet that came out the pedit part of the
> pipeline. I need to see the correct packet. I know with my check
> this is guaranteed.

You *are* misreading what I wrote.

As I've repeatedly stated, it is guaranteed that either tcpdump
is finished with the skb, or you will see it as cloned.

Anything else is a serious bug in our stack, regardless of what
pedit does.

In fact pedit is buggy today because you're modifying a packet
that may be cloned.

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: Question about an assignment in handle_ing()

[jamal]

On Thu, 2010-06-03 at 22:47 +1000, Herbert Xu wrote:

> You *are* misreading what I wrote.
> 
> As I've repeatedly stated, it is guaranteed that either tcpdump
> is finished with the skb, or you will see it as cloned.

I dont see the problem of when tcpdump sees the packet first.
pedit doesnt care if it is cloned or not.
What i am worried about - and couldnt parse in your email - is:
What happens when pedit gets to it first? would skb_cloned() be true
always/guaranteed?

cheers,
jamal

Re: Question about an assignment in handle_ing()

[Herbert Xu]

On Thu, Jun 03, 2010 at 08:53:14AM -0400, jamal wrote:
>
> I dont see the problem of when tcpdump sees the packet first.
> pedit doesnt care if it is cloned or not.
> What i am worried about - and couldnt parse in your email - is:
> What happens when pedit gets to it first? would skb_cloned() be true
> always/guaranteed?

Yes that's what I've been saying all along.  As long as the skb
is still held up in af_packet's socket queue, the packet will be
marked as cloned.

Only when tcpdump has done a recv(2) on the socket and liberated
the skb in the queue, will you see the clone flag turned off in
pedit.

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: Question about an assignment in handle_ing()

[Herbert Xu]

On Thu, Jun 03, 2010 at 08:53:14AM -0400, jamal wrote:
>
> I dont see the problem of when tcpdump sees the packet first.
> pedit doesnt care if it is cloned or not.
> What i am worried about - and couldnt parse in your email - is:
> What happens when pedit gets to it first? would skb_cloned() be true
> always/guaranteed?

Yes that's what I've been saying all along.  As long as the skb
is still held in the af_packet socket queue, pedit will see the
skb as cloned.  Only when tcpdump has done a recv(2) and liberated
the skb from the queue, will skb_cloned return false.

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: Question about an assignment in handle_ing()

[jamal]

On Thu, 2010-06-03 at 08:53 -0400, jamal wrote:
> On Thu, 2010-06-03 at 22:47 +1000, Herbert Xu wrote:
> 
> > You *are* misreading what I wrote.
> > 
> > As I've repeatedly stated, it is guaranteed that either tcpdump
> > is finished with the skb, or you will see it as cloned.
> 
> I dont see the problem of when tcpdump sees the packet first.
> pedit doesnt care if it is cloned or not.
> What i am worried about - and couldnt parse in your email - is:
> What happens when pedit gets to it first? would skb_cloned() be true
> always/guaranteed?

Never mind. You did answer this question above. You are saying it is
guaranteed.
I will send patches to remove the checks in both pedit and handle_ing().
At some point i will test it....

cheers,
jamal

Re: Question about an assignment in handle_ing()

[Herbert Xu]

On Thu, Jun 03, 2010 at 08:58:16AM -0400, jamal wrote:
>
> Never mind. You did answer this question above. You are saying it is
> guaranteed.
> I will send patches to remove the checks in both pedit and handle_ing().
> At some point i will test it....

Thanks Jamal!
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: Question about an assignment in handle_ing()

[jamal]

On Thu, 2010-06-03 at 23:00 +1000, Herbert Xu wrote:
> On Thu, Jun 03, 2010 at 08:58:16AM -0400, jamal wrote:
> >
> > Never mind. You did answer this question above. You are saying it is
> > guaranteed.
> > I will send patches to remove the checks in both pedit and handle_ing().
> > At some point i will test it....
> 
> Thanks Jamal!

And thank you as well for you patience Herbert.

cheers,
jamal