kuznet@xxxxxxxxxxxxx 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@xxxxxxxxxxxxx
|