netdev - Re: [PATCH wpan-next v3 3/4] net: mac802154: Set durations automatically

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <8f32c05e-5830-79b3-1a3a-996ee8b58e52@datenfreihafen.org>
Date:   Thu, 28 Apr 2022 18:02:15 +0200
From:   Stefan Schmidt <stefan@...enfreihafen.org>
To:     Miquel Raynal <miquel.raynal@...tlin.com>,
        Alexander Aring <alex.aring@...il.com>,
        linux-wpan@...r.kernel.org
Cc:     "David S. Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>, netdev@...r.kernel.org,
        Xue Liu <liuxuenetmail@...il.com>,
        Marcel Holtmann <marcel@...tmann.org>,
        Harry Morris <harrymorris12@...il.com>,
        David Girault <david.girault@...vo.com>,
        Romuald Despres <romuald.despres@...vo.com>,
        Frederic Blain <frederic.blain@...vo.com>,
        Nicolas Schodet <nico@...fr.eu.org>,
        Thomas Petazzoni <thomas.petazzoni@...tlin.com>
Subject: Re: [PATCH wpan-next v3 3/4] net: mac802154: Set durations
 automatically


Hello.

On 28.04.22 17:58, Miquel Raynal wrote:
> Hi Stefan,
> 
> miquel.raynal@...tlin.com wrote on Tue,  1 Feb 2022 19:06:28 +0100:
> 
>> As depicted in the IEEE 802.15.4 specification, modulation/bands are
>> tight to a number of page/channels so we can for most of them derive the
>> durations automatically.
>>
>> The two locations that must call this new helper to set the variou
>> symbol durations are:
>> - when manually requesting a channel change though the netlink interface
>> - at PHY creation, once the device driver has set the default
>>    page/channel
>>
>> If an information is missing, the symbol duration is not touched, a
>> debug message is eventually printed. This keeps the compatibility with
>> the unconverted drivers for which it was too complicated for me to find
>> their precise information. If they initially provided a symbol duration,
>> it would be kept. If they don't, the symbol duration value is left
>> untouched.
>>
>> Once the symbol duration derived, the lifs and sifs durations are
>> updated as well.
>>
>> Signed-off-by: Miquel Raynal <miquel.raynal@...tlin.com>
>> ---
>>   include/net/cfg802154.h |  2 ++
>>   net/mac802154/cfg.c     |  1 +
>>   net/mac802154/main.c    | 46 +++++++++++++++++++++++++++++++++++++++++
>>   3 files changed, 49 insertions(+)
>>
>> diff --git a/include/net/cfg802154.h b/include/net/cfg802154.h
>> index 8a4b6a50452f..49b4bcc24032 100644
>> --- a/include/net/cfg802154.h
>> +++ b/include/net/cfg802154.h
>> @@ -405,4 +405,6 @@ static inline const char *wpan_phy_name(struct wpan_phy *phy)
>>   	return dev_name(&phy->dev);
>>   }
>>   
>> +void ieee802154_configure_durations(struct wpan_phy *phy);
>> +
>>   #endif /* __NET_CFG802154_H */
>> diff --git a/net/mac802154/cfg.c b/net/mac802154/cfg.c
>> index fbeebe3bc31d..1e4a9f74ed43 100644
>> --- a/net/mac802154/cfg.c
>> +++ b/net/mac802154/cfg.c
>> @@ -118,6 +118,7 @@ ieee802154_set_channel(struct wpan_phy *wpan_phy, u8 page, u8 channel)
>>   	if (!ret) {
>>   		wpan_phy->current_page = page;
>>   		wpan_phy->current_channel = channel;
>> +		ieee802154_configure_durations(wpan_phy);
>>   	}
>>   
>>   	return ret;
>> diff --git a/net/mac802154/main.c b/net/mac802154/main.c
>> index 53153367f9d0..5546ef86e231 100644
>> --- a/net/mac802154/main.c
>> +++ b/net/mac802154/main.c
>> @@ -113,6 +113,50 @@ ieee802154_alloc_hw(size_t priv_data_len, const struct ieee802154_ops *ops)
>>   }
>>   EXPORT_SYMBOL(ieee802154_alloc_hw);
>>   
>> +void ieee802154_configure_durations(struct wpan_phy *phy)
>> +{
>> +	u32 duration = 0;
>> +
>> +	switch (phy->current_page) {
>> +	case 0:
>> +		if (BIT(phy->current_page) & 0x1)
> 
> I am very sorry to spot this only now but this is wrong.
> 
> all the conditions from here and below should be:
> 
> 		if (BIT(phy->current_channel & <mask>))
> 
> The masks look good, the durations as well, but the conditions are
> wrong.
> 
>> +			/* 868 MHz BPSK 802.15.4-2003: 20 ksym/s */
>> +			duration = 50 * NSEC_PER_USEC;
>> +		else if (phy->current_page & 0x7FE)
> 
> Ditto
> 
>> +			/* 915 MHz BPSK	802.15.4-2003: 40 ksym/s */
>> +			duration = 25 * NSEC_PER_USEC;
>> +		else if (phy->current_page & 0x7FFF800)
> 
> Ditto
> 
>> +			/* 2400 MHz O-QPSK 802.15.4-2006: 62.5 ksym/s */
>> +			duration = 16 * NSEC_PER_USEC;
>> +		break;
>> +	case 2:
>> +		if (BIT(phy->current_page) & 0x1)
> 
> Ditto
> 
>> +			/* 868 MHz O-QPSK 802.15.4-2006: 25 ksym/s */
>> +			duration = 40 * NSEC_PER_USEC;
>> +		else if (phy->current_page & 0x7FE)
> 
> Ditto
> 
>> +			/* 915 MHz O-QPSK 802.15.4-2006: 62.5 ksym/s */
>> +			duration = 16 * NSEC_PER_USEC;
>> +		break;
>> +	case 3:
>> +		if (BIT(phy->current_page) & 0x3FFF)
> 
> Ditto
> 
>> +			/* 2.4 GHz CSS 802.15.4a-2007: 1/6 Msym/s */
>> +			duration = 6 * NSEC_PER_USEC;
>> +		break;
> 
> I see it's "only" in wpan-next (781830c800dd "net: mac802154: Set
> durations automatically") and was not yet pulled in the net-next
> tree so please let me know what you prefer: I can either provide a
> proper patch to fit it (without upstream Fixes reference), or you can
> just apply this diff below and push -f the branch. Let me know what you
> prefer.

No forced push to a public branch in my public repo. Please provide a 
fixup patch that I can apply on top.

regards
Stefan Schmidt