Re: [PATCH] tcpdump may trace some outbound packets twice.

2006-05-15 Thread David S. Miller
From: Ranjit Manomohan [EMAIL PROTECTED] Date: Mon, 15 May 2006 14:19:06 -0700 (PDT) Heres a new version which does a copy instead of the clone to avoid the double cloning issue. I still very much dislike this patch because it is creating 1 more clone per packet than is actually necessary and

Re: [PATCH] tcpdump may trace some outbound packets twice.

2006-05-15 Thread Patrick McHardy
David S. Miller wrote: From: Ranjit Manomohan [EMAIL PROTECTED] Date: Mon, 15 May 2006 14:19:06 -0700 (PDT) Heres a new version which does a copy instead of the clone to avoid the double cloning issue. I still very much dislike this patch because it is creating 1 more clone per packet

Re: [PATCH] tcpdump may trace some outbound packets twice.

2006-05-15 Thread Stephen Hemminger
On Mon, 15 May 2006 16:11:05 -0700 (PDT) Ranjit Manomohan [EMAIL PROTECTED] wrote: On Mon, 15 May 2006, David S. Miller wrote: From: Ranjit Manomohan [EMAIL PROTECTED] Date: Mon, 15 May 2006 14:19:06 -0700 (PDT) Heres a new version which does a copy instead of the clone to avoid

Re: [PATCH] tcpdump may trace some outbound packets twice.

2006-05-15 Thread David S. Miller
From: Stephen Hemminger [EMAIL PROTECTED] Date: Mon, 15 May 2006 16:41:01 -0700 kfree_skb(NULL) is legal so the conditional here is unneeded. But the increased calls to kfree_skb(NULL) would probably bring the unlikely() hordes descending on kfree_skb, so maybe: And unfortunately as Patrick

Re: [PATCH] tcpdump may trace some outbound packets twice.

2006-05-15 Thread Patrick McHardy
David S. Miller wrote: From: Stephen Hemminger [EMAIL PROTECTED] Date: Mon, 15 May 2006 16:41:01 -0700 kfree_skb(NULL) is legal so the conditional here is unneeded. But the increased calls to kfree_skb(NULL) would probably bring the unlikely() hordes descending on kfree_skb, so maybe:

Re: [PATCH] tcpdump may trace some outbound packets twice.

2006-05-15 Thread Herbert Xu
David S. Miller [EMAIL PROTECTED] wrote: Other implementation possibility suggestions welcome :-) I see two possibilities: 1) Move the af_packet hook into the NIC driver. 2) Rethink the lockless tx setup. If all NICs followed the tg3 and replaced spin_lock_irqsave with spin_lock then we

Re: [PATCH] tcpdump may trace some outbound packets twice.

2006-05-15 Thread Tom Young
On Tue, May 16, 2006 at 02:21:27AM +0200, Patrick McHardy wrote: David S. Miller wrote: From: Stephen Hemminger [EMAIL PROTECTED] Date: Mon, 15 May 2006 16:41:01 -0700 kfree_skb(NULL) is legal so the conditional here is unneeded. But the increased calls to kfree_skb(NULL) would

Re: [PATCH] tcpdump may trace some outbound packets twice.

2006-05-15 Thread Patrick McHardy
Herbert Xu wrote: David S. Miller [EMAIL PROTECTED] wrote: Other implementation possibility suggestions welcome :-) I see two possibilities: 1) Move the af_packet hook into the NIC driver. 2) Rethink the lockless tx setup. If all NICs followed the tg3 and replaced

Re: [PATCH] tcpdump may trace some outbound packets twice.

2006-05-15 Thread Herbert Xu
On Tue, May 16, 2006 at 03:17:59AM +0200, Patrick McHardy wrote: 3) Clone the skb and have dev_queue_xmit_nit() consume it. That should actually be pretty easy. Unfortunately that would mean an unconditional copy for all TSO packets on NICs such as tg3/e1000. These drivers have to modify

Re: [PATCH] tcpdump may trace some outbound packets twice.

2006-05-15 Thread Patrick McHardy
Patrick McHardy wrote: 3) Clone the skb and have dev_queue_xmit_nit() consume it. That should actually be pretty easy. On second thought, thats not so great either. netdev_nit just globally signals that there are some taps, but we don't know if they're interested in a specific packet. - To

Re: [PATCH] tcpdump may trace some outbound packets twice.

2006-05-15 Thread David S. Miller
From: Patrick McHardy [EMAIL PROTECTED] Date: Tue, 16 May 2006 03:22:21 +0200 Patrick McHardy wrote: 3) Clone the skb and have dev_queue_xmit_nit() consume it. That should actually be pretty easy. On second thought, thats not so great either. netdev_nit just globally signals that

Re: [PATCH] tcpdump may trace some outbound packets twice.

2006-05-14 Thread Andrew Morton
Ranjit Manomohan [EMAIL PROTECTED] wrote: This patch fixes the problem where tcpdump shows duplicate packets while tracing outbound packets on drivers which support lockless transmit. The patch changes the current behaviour to tracing the packets only on a successful transmit. There was no

Re: [PATCH] tcpdump may trace some outbound packets twice.

2006-05-14 Thread David S. Miller
From: Andrew Morton [EMAIL PROTECTED] Date: Sun, 14 May 2006 03:10:34 -0700 It's a bit sad to be taking a clone of a clone like this. Avoidable? Besides, clones of clones are illegal, if it's already a clone you must make a copy. - To unsubscribe from this list: send the line unsubscribe

[PATCH] tcpdump may trace some outbound packets twice.

2006-05-10 Thread Ranjit Manomohan
This patch fixes the problem where tcpdump shows duplicate packets while tracing outbound packets on drivers which support lockless transmit. The patch changes the current behaviour to tracing the packets only on a successful transmit. Signed-off-by: Ranjit Manomohan [EMAIL PROTECTED] ---