Re: [PATCH take 2] net_sched: Add qdisc __NET_XMIT_BYPASS flag

[Jarek Poplawski <jarkao2@gmail.com>]

On Tue, Aug 05, 2008 at 03:43:50PM +0300, Jussi Kivilinna wrote:
> Quoting "Jarek Poplawski" <jarkao2@gmail.com>:
>
>> On Mon, Aug 04, 2008 at 09:35:35PM +0300, Jussi Kivilinna wrote:
>>> Quoting "Jarek Poplawski" <jarkao2@gmail.com>:
>>>
>>> This seems to also make sure that when returning 'full'
>>> NET_XMIT_SUCCESS, skb pointer is safe(r?) to use now.
>>
>> I guess so... At least theoretically - I was mainly afraid of
>> redirecting, which isn't fully implemented yet. On the other hand,
>> outwards of qdiscs there is still no difference.
>>
>
> How about making skb shared before passing into qdisc tree?
> That would make skb usage safe after qdisc enqueues.

It's a bit costly (atomics), so there should be a good reason for this.
It should be first checked if there is real danger. And if it's only
for more exact stats, I'm not sure it's worth of it.

Jarek P.

>
> ---
>
> diff --git a/include/net/sch_generic.h b/include/net/sch_generic.h
> index c5bb130..9cf06fc 100644
> --- a/include/net/sch_generic.h
> +++ b/include/net/sch_generic.h
> @@ -352,10 +352,16 @@ static inline int qdisc_enqueue(struct sk_buff  
> *skb, struct Qdisc *sch)
>         return sch->enqueue(skb, sch);
>  }
>
> -static inline int qdisc_enqueue_root(struct sk_buff *skb, struct Qdisc *sch)
> +static inline int qdisc_enqueue_root(struct sk_buff *__skb, struct  
> Qdisc *sch)
>  {
> +       struct sk_buff *skb = skb_get(__skb);
> +       int ret;
> +
>         qdisc_skb_cb(skb)->pkt_len = skb->len;
> -       return qdisc_enqueue(skb, sch);
> +       ret = qdisc_enqueue(skb, sch);
> +       kfree_skb(skb);
> +
> +       return ret;
>  }
>
>  static inline int __qdisc_enqueue_tail(struct sk_buff *skb, struct  
> Qdisc *sch,
>