Linux-Wireless Archive on lore.kernel.org
 help / color / Atom feed
From: Johannes Berg <johannes@sipsolutions.net>
To: Aloka Dixit <alokad@codeaurora.org>
Cc: linux-wireless@vger.kernel.org
Subject: Re: [PATCH v4 2/2] mac80211: Add FILS discovery support
Date: Thu, 30 Jul 2020 16:47:21 +0200
Message-ID: <bb2be4ac581487aa9e89d3c75180a1766b112370.camel@sipsolutions.net> (raw)
In-Reply-To: <20200618050427.5891-3-alokad@codeaurora.org>

On Wed, 2020-06-17 at 22:04 -0700, Aloka Dixit wrote:

> +++ b/net/mac80211/cfg.c
> @@ -837,6 +837,33 @@ static int ieee80211_set_probe_resp(struct ieee80211_sub_if_data *sdata,
>  	return 0;
>  }
>  
> +static int ieee80211_set_fils_discovery(struct ieee80211_sub_if_data *sdata,
> +					struct cfg80211_fils_discovery *params)
> +{
> +	struct fils_discovery_data *new, *old = NULL;
> +	struct ieee80211_fils_discovery *fd;
> +
> +	fd = &sdata->vif.bss_conf.fils_discovery;
> +	fd->min_interval = params->min_interval;
> +	fd->max_interval = params->max_interval;
> +
> +	if (!params->tmpl || !params->tmpl_len) /* Optional template */
> +		return 0;

Now I'm even more confused. If the template is optional, then if it's
not given it doesn't mean *everything* should be ignored, does it?

What would be the point of that? OTOH, if the template isn't there, what
would you do?

But it still doesn't make sense - if no template means you shouldn't do
anything then that doesn't mean the template should be optional, that
just means userspace shouldn't even put the NL80211_ATTR_FILS_DISCOVERY
attribute when it doesn't want anything to be done?

So it seems to me that something doesn't match. Either the template is
truly optional and then this shouldn't just return success, or the
template isn't actually optional?

> +	err = ieee80211_set_fils_discovery(sdata, &params->fils_discovery);
> +	if (err < 0) {
> +		ieee80211_vif_release_channel(sdata);
> +		return err;

Is there no goto label for this error case?

> +struct fils_discovery_data {
> +	struct rcu_head rcu_head;
> +	int len;
> +	u8 data[0];

please use [] not [0].

> +struct sk_buff *ieee80211_get_fils_discovery_tmpl(struct ieee80211_hw *hw,
> +						  struct ieee80211_vif *vif)
> +{
> +	struct sk_buff *skb = NULL;
> +	struct fils_discovery_data *tmpl = NULL;
> +	struct ieee80211_sub_if_data *sdata = vif_to_sdata(vif);
> +
> +	if (sdata->vif.type != NL80211_IFTYPE_AP)
> +		return NULL;
> +
> +	rcu_read_lock();
> +	tmpl = rcu_dereference(sdata->u.ap.fils_discovery);
> +	if (!tmpl) {
> +		rcu_read_unlock();
> +		return NULL;
> +	}
> +
> +	skb = dev_alloc_skb(tmpl->len);
> +	if (skb)
> +		skb_put_data(skb, tmpl->data, tmpl->len);

You should consider the headroom that the driver may have requested.

johannes



  reply index

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-06-18  5:04 [PATCH v4 0/2] " Aloka Dixit
2020-06-18  5:04 ` [PATCH v4 1/2] nl80211: " Aloka Dixit
2020-07-30 14:43   ` Johannes Berg
2020-07-30 21:17     ` Aloka Dixit
2020-07-30 21:22       ` Johannes Berg
2020-07-30 21:53         ` Aloka Dixit
2020-06-18  5:04 ` [PATCH v4 2/2] mac80211: " Aloka Dixit
2020-07-30 14:47   ` Johannes Berg [this message]
2020-07-30 21:00     ` Aloka Dixit
2020-07-30 21:26       ` Johannes Berg
2020-07-30 22:08         ` Aloka Dixit
2020-07-31  8:21           ` Johannes Berg

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=bb2be4ac581487aa9e89d3c75180a1766b112370.camel@sipsolutions.net \
    --to=johannes@sipsolutions.net \
    --cc=alokad@codeaurora.org \
    --cc=linux-wireless@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Linux-Wireless Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-wireless/0 linux-wireless/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-wireless linux-wireless/ https://lore.kernel.org/linux-wireless \
		linux-wireless@vger.kernel.org
	public-inbox-index linux-wireless

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-wireless


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git