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 feedback on this one?
>
> --- linux-2.6/net/sched/sch_generic.c 2006-05-10 12:34:52.000000000 -0700
> +++ linux/net/sched/sch_generic.c 2006-05-10 12:39:38.000000000 -0700
> @@ -136,8 +136,12 @@
>
> if (!netif_queue_stopped(dev)) {
> int ret;
> + struct sk_buff *skbc = NULL;
> + /* Clone the skb so that we hold a reference
> + * to its data and we can trace it after a
> + * successful transmit. */
Like this:
/*
* Clone the skb so that we hold a reference to
* its data and we can trace it after a
* successful transmit
*/
> if (netdev_nit)
> - dev_queue_xmit_nit(skb, dev);
> + skbc = skb_clone(skb, GFP_ATOMIC);
>
> ret = dev->hard_start_xmit(skb, dev);
> if (ret == NETDEV_TX_OK) {
> @@ -145,6 +149,15 @@
> dev->xmit_lock_owner = -1;
> spin_unlock(&dev->xmit_lock);
> }
> + if(skbc) {
Like this:
if (skbc)
> + /* transmit succeeded,
> + * trace the clone. */
> + dev_queue_xmit_nit(skbc,dev);
> + kfree_skb(skbc);
> + }
> + /* Free clone if it exists */
> + if(skbc)
if (skbc)
> + kfree_skb(skbc);
We don't need to test for skbc==NULL - kfree_skb(NULL) is legal.
This code will end up running kfree_skb(skbc) twice. Unless
dev_queue_xmit_nit() takes an additional ref on the skb (I don't think it
does), this will cause corruption of freed memory.
> spin_lock(&dev->queue_lock);
> return -1;
> }
dev_queue_xmit_nit() already clones the skb. It's a bit sad to be taking a
clone of a clone like this. Avoidable?
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
