| 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@xxxxxxxxxxxxxxxxxxxxxxxxxx> |
| References: | <20050514153100.GL3643@xxxxxx> <20050514173947.GA32235@xxxxxxxxxxxxxxxxxxxxxxxxxx> <1116358994.5534.2.camel@xxxxxxxxxxxxx> <20050517212217.GB12936@xxxxxxxxxxxxxxxxxxxxxxxxxx> |
| 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> |
|---|---|---|
| ||
| Previous by Date: | Re: Good (exemplary) network driver?, Francois Romieu |
|---|---|
| Next by Date: | Why /sys/class/net/*/features is read-only?, Catalin(ux aka Dino) BOIE |
| Previous by Thread: | Re: [PATCH 11/12] orinoco: monitor mode support, Francois Romieu |
| Next by Thread: | [PATCH 12/12] orinoco: update changelog and version, Christoph Hellwig |
| Indexes: | [Date] [Thread] [Top] [All Lists] |