Received-SPF: pass (google.com: domain of linux-wireless-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Message-ID: <56500fd2b7c6b54e904567ba27171542027f2aea.camel@sipsolutions.net>
Subject: Re: [PATCH 03/10] nl80211: add HE 6 GHz Band Capability support
From:   Johannes Berg <johannes@sipsolutions.net>
To:     Rajkumar Manoharan <rmanohar@codeaurora.org>
Cc:     linux-wireless@vger.kernel.org
Date:   Wed, 29 Apr 2020 16:28:31 +0200
In-Reply-To: <1587768108-25248-4-git-send-email-rmanohar@codeaurora.org>
References: <1587768108-25248-1-git-send-email-rmanohar@codeaurora.org>
         <1587768108-25248-4-git-send-email-rmanohar@codeaurora.org>
Content-Type: text/plain; charset="UTF-8"
User-Agent: Evolution 3.34.4 (3.34.4-1.fc31) 
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Sender: linux-wireless-owner@vger.kernel.org
Precedence: bulk

On Fri, 2020-04-24 at 15:41 -0700, Rajkumar Manoharan wrote:
> Define new structures for HE 6 GHz band capabilities as per

per?

>  include/linux/ieee80211.h    | 57 ++++++++++++++++++++++++++++++++++++++++++++
>  include/net/cfg80211.h       |  2 ++
>  include/uapi/linux/nl80211.h |  6 +++++
>  net/wireless/nl80211.c       | 14 +++++++++++

This is a bit mixed up, I'd prefer the raw ieee80211.h definitions in a
separate patch.

> +	[NL80211_ATTR_HE_6GHZ_CAPABILITY] = {
> +		.type = NLA_EXACT_LEN_WARN,
> +		.len = NL80211_HE_6GHZ_CAPABILITY_LEN,
> +	},

Shouldn't use _WARN for a new attribute. Just reject it if userspace
gets it wrong.

> @@ -6177,6 +6185,12 @@ static int nl80211_new_station(struct sk_buff *skb, struct genl_info *info)
>  			return -EINVAL;
>  	}
>  
> +	/* Ensure that HT/VHT capabilities are not set for 6 GHz HE STA */
> +	if (params.he_6ghz_capa) {
> +		params.ht_capa = NULL;
> +		params.vht_capa = NULL;
> +	}

IMHO better to reject (with a message)

johannes