Re: [PATCH 4/10] d80211: reduce mdev usage, fix ieee80211_rx_mgmt

[David Kimdon <david.kimdon@devicescape.com>]

Reply-To: 
In-Reply-To: <1163802439.3392.47.camel@johannes.berg>


> 
> --- wireless-dev.orig/net/d80211/ieee80211.c	2006-11-17 20:01:54.999703408 +0100
> +++ wireless-dev/net/d80211/ieee80211.c	2006-11-17 20:01:55.659703408 +0100
> @@ -210,9 +210,16 @@ static void ieee80211_key_threshold_noti
>  					   struct ieee80211_key *key,
>  					   struct sta_info *sta)
>  {
> +	struct ieee80211_local *local = dev->ieee80211_ptr;
>  	struct sk_buff *skb;
>  	struct ieee80211_msg_key_notification *msg;
>  
> +	/* if no one will get it anyway, don't even allocate it.
> +	 * unlikely because this is only relevant for APs
> +	 * where the device must be open... */
> +	if (unlikely(!local->apdev))
> +		return;

Why not just let it Oops and show the bug?  In what cases is it ok
for apdev to not be set?

>  	skb = dev_alloc_skb(sizeof(struct ieee80211_frame_info) +
>  			    sizeof(struct ieee80211_msg_key_notification));
>  	if (!skb)


Can we get whitespace changes like this in a separate patch?

>  
> -                jiffies_to_timespec(status->hosttime, &ts);
> +		jiffies_to_timespec(status->hosttime, &ts);
>  		fi->hosttime = cpu_to_be64((u64) ts.tv_sec * 1000000 +
>  					   ts.tv_nsec / 1000);
>  		fi->mactime = cpu_to_be64(status->mactime);
>  		switch (status->phymode) {
> -                case MODE_IEEE80211A:
> -                        fi->phytype = htonl(ieee80211_phytype_ofdm_dot11_a);
> -                        break;
> -                case MODE_IEEE80211B:
> -                        fi->phytype = htonl(ieee80211_phytype_dsss_dot11_b);
> -                        break;
> -                case MODE_IEEE80211G:
> -                        fi->phytype = htonl(ieee80211_phytype_pbcc_dot11_g);
> -                        break;
> -                case MODE_ATHEROS_TURBO:
> +		case MODE_IEEE80211A:
> +			fi->phytype = htonl(ieee80211_phytype_ofdm_dot11_a);
> +			break;
> +		case MODE_IEEE80211B:
> +			fi->phytype = htonl(ieee80211_phytype_dsss_dot11_b);
> +			break;
> +		case MODE_IEEE80211G:
> +			fi->phytype = htonl(ieee80211_phytype_pbcc_dot11_g);
> +			break;
> +		case MODE_ATHEROS_TURBO:
>  			fi->phytype =
>  				htonl(ieee80211_phytype_dsss_dot11_turbo);
>                          break;
>                  default:
> -                        fi->phytype = 0xAAAAAAAA;
> +			fi->phytype = 0xAAAAAAAA;
>  			break;
> -                }
> -                fi->channel = htonl(status->channel);
> +		}
> +		fi->channel = htonl(status->channel);


. . . 


>  int ieee80211_set_aid_for_sta(struct net_device *dev, u8 *peer_address,
>  			      u16 aid)
>  {
> +	struct ieee80211_local *local = dev->ieee80211_ptr;
>  	struct sk_buff *skb;
>  	struct ieee80211_msg_set_aid_for_sta *msg;
>  
> +	/* unlikely because if this event only happens for APs,
> +	 * which require an open ap device. */
> +	if (unlikely(!local->apdev))
> +		return 0;

again, so should we just let the Oops happen?

-David