Re: [PATCH] PPPOE can kfree SKB twice (was Re: kernel panic problem. (smp, iptables?))

From: Michal Ostrowski (mostrows@us.ibm.com)
Date: Thu Jul 19 2001 - 13:00:54 EST


kuznet@ms2.inr.ac.ru writes:

> Hello!
>
> SOme short comment on the patch:
>
>
> > - dev_queue_xmit(skb);
> > + /* The skb we are to transmit may be a copy (see above). If
> > + * this fails, then the caller is responsible for the original
> > + * skb, otherwise we must free it. Also if this fails we must
> > + * free the copy that we made.
> > + */
> > +
> > + if (dev_queue_xmit(skb)<0) {
>
> dev_queue_xmit _frees_ frame, not depending on return value.
> Return value is not a criterium to assume anything.
>

My mistake. It seemed perfectly reasonable at 6:00 am. :-)

However, could we not have dev_queue_xmit behave as such (not free
frame on failure)? That is, could we extend dev_queue_xmit to tell it
(optionally) that we want the skb back in case of failure?
dev_queue_xmit unconditionally frees the skb in any failure mode,
which is I would venture to say that we could do this.

The reason why I'm proposing this is that ppp_generic.c assumes that
the skb is still available after a transmission failure via pppoe. To
support the semantics of dev_queue_xmit and ppp_generic we would have
to always copy skb's inside pppoe_xmit. Then, if dev_queue_xmit fails
the original is deleted.

In the common case dev_queue_xmit will not fail, and so in that case
I'd like to have to avoid making a copy of the skb.

Michal Ostrowski
mostrows@speakeasy.net

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/



This archive was generated by hypermail 2b29 : Mon Jul 23 2001 - 21:00:12 EST