Received-SPF: pass (google.com: domain of linux-wireless-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <df1eda8a2e788800f71e77af60d3ae899a6d3d5d.camel@sipsolutions.net>
Subject: Re: [PATCH v3 04/12] nl80211: support setting S1G short beacon
 period
From:   Johannes Berg <johannes@sipsolutions.net>
To:     Kieran Frewen <kieran.frewen@morsemicro.com>
Cc:     linux-wireless@vger.kernel.org, quic_jjohnson@quicinc.com
Date:   Tue, 06 Sep 2022 11:42:32 +0200
In-Reply-To: <20220906044812.7609-5-kieran.frewen@morsemicro.com>
References: <20220906044812.7609-1-kieran.frewen@morsemicro.com>
         <20220906044812.7609-5-kieran.frewen@morsemicro.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
User-Agent: Evolution 3.44.4 (3.44.4-1.fc36) 
MIME-Version: 1.0
Precedence: bulk

On Tue, 2022-09-06 at 16:48 +1200, Kieran Frewen wrote:
>=20
> @@ -805,6 +805,7 @@ static const struct nla_policy nl80211_policy[NUM_NL8=
0211_ATTR] =3D {
>  	[NL80211_ATTR_MLD_ADDR] =3D NLA_POLICY_EXACT_LEN(ETH_ALEN),
>  	[NL80211_ATTR_MLO_SUPPORT] =3D { .type =3D NLA_FLAG },
>  	[NL80211_ATTR_MAX_NUM_AKM_SUITES] =3D { .type =3D NLA_REJECT },
> +	[NL80211_ATTR_SHORT_BEACON_PERIOD] =3D { .type =3D NLA_U16 },

You probably want _some_ kind of range validation here, like "don't set
it to zero" at least?

> +	if (info->attrs[NL80211_ATTR_SHORT_BEACON_PERIOD])
> +		params->short_beacon_period =3D
> +			nla_get_u32(info->attrs[NL80211_ATTR_SHORT_BEACON_PERIOD]);
>=20

And maybe it should be < the long beacon period?

Actually the mac80211 code makes it sound like what you really should
have is a "long beacon period" or something, so that every N beacons you
send a long one?

Or are these really completely independent? It feels like probably the
intention would be to have a (short) beacon period, and then every N
beacons you send a long one?

johannes