netdev
[Top] [All Lists]

Re: [PATCH 11/12] orinoco: monitor mode support

To: Francois Romieu <romieu@xxxxxxxxxxxxx>
Subject: Re: [PATCH 11/12] orinoco: monitor mode support
From: Pavel Roskin <proski@xxxxxxx>
Date: Sun, 22 May 2005 23:40:39 -0400
Cc: Christoph Hellwig <hch@xxxxxx>, jgarzik@xxxxxxxxx, hermes@xxxxxxxxxxxxxxxxxxxxx, netdev@xxxxxxxxxxx
In-reply-to: <20050517212217.GB12936@electric-eye.fr.zoreil.com>
References: <20050514153100.GL3643@lst.de> <20050514173947.GA32235@electric-eye.fr.zoreil.com> <1116358994.5534.2.camel@dv.roinet.com> <20050517212217.GB12936@electric-eye.fr.zoreil.com>
Sender: netdev-bounce@xxxxxxxxxxx
Hello, Francois!

On Tue, 2005-05-17 at 23:22 +0200, Francois Romieu wrote:

> I am not fond of unneeded/hidden state variable. What about:
> 
> +       /* sanity check the length */
> +       if (datalen > IEEE80211_DATA_LEN + 12) {
> +               printk(KERN_DEBUG "%s: oversized monitor frame, "
> +                      "data length = %d\n", dev->name, datalen);
> +               err = -EIO;
> +               goto drop;
>                 ^^^^^^^^^^ -> let's replace by 'goto update_stats;'
> And turn:
> +   drop:
> +       stats->rx_errors++;
> +       stats->rx_dropped++;
> 
> into:
> 
> +   drop:
> +       dev_kfree_skb_irq(skb);     
> +   update_stats:
> +       stats->rx_errors++;
> +       stats->rx_dropped++;
> 
> This way 'goto drop' really drops and the code does not issue a 'goto drop'
> when it actually want to update the stats.

I agree.  Closer look shows that orinoco needs a complete overhaul of
stats in the rx path.  I'm going to fix it in CVS, and I'll post the
patch to netdev after it receives some testing.

-- 
Regards,
Pavel Roskin


<Prev in Thread] Current Thread [Next in Thread>