diff mbox series

[RFC,1/2] wifi: mac80211: add a new field in ieee80211_rx_status for link id

Message ID 20220802065019.20791-2-quic_vthiagar@quicinc.com (mailing list archive)
State RFC
Delegated to: Johannes Berg
Headers show
Series wifi: mac80211: extend rx API with link_id for MLO connection | expand

Commit Message

Vasanthakumar Thiagarajan Aug. 2, 2022, 6:50 a.m. UTC
In MLO, when the address translation from link to MLD is done
in fw/hw, it is necessary to be able to have some information
on the link on which the frame has been received. Extend the
rx API to include link_id in ieee80211_rx_status.

Signed-off-by: Vasanthakumar Thiagarajan <quic_vthiagar@quicinc.com>
---
 include/net/mac80211.h | 4 ++++
 1 file changed, 4 insertions(+)

Comments

Jeff Johnson Aug. 2, 2022, 3:28 p.m. UTC | #1
On 8/1/2022 11:50 PM, Vasanthakumar Thiagarajan wrote:
> In MLO, when the address translation from link to MLD is done
> in fw/hw, it is necessary to be able to have some information
> on the link on which the frame has been received. Extend the
> rx API to include link_id in ieee80211_rx_status.
> 
> Signed-off-by: Vasanthakumar Thiagarajan <quic_vthiagar@quicinc.com>
> ---
>   include/net/mac80211.h | 4 ++++
>   1 file changed, 4 insertions(+)
> 
> diff --git a/include/net/mac80211.h b/include/net/mac80211.h
> index f198af600b5e..23bc34657b16 100644
> --- a/include/net/mac80211.h
> +++ b/include/net/mac80211.h
> @@ -1480,6 +1480,9 @@ enum mac80211_rx_encoding {
>    *	each A-MPDU but the same for each subframe within one A-MPDU
>    * @ampdu_delimiter_crc: A-MPDU delimiter CRC
>    * @zero_length_psdu_type: radiotap type of the 0-length PSDU
> + * @link_id: id of the link used to receive the packet. Applicable only with
> + *	MLO connection, valid link ids are in 0-14, link_id 15 means either the
> + *	link id is not known or it is a non-MLO connection.
>    */
>   struct ieee80211_rx_status {
>   	u64 mactime;
> @@ -1504,6 +1507,7 @@ struct ieee80211_rx_status {
>   	s8 chain_signal[IEEE80211_MAX_CHAINS];
>   	u8 ampdu_delimiter_crc;
>   	u8 zero_length_psdu_type;
> +	u8 link_id;
>   };
>   
>   static inline u32

in other parts of the MLO code the link_id is defined as int and a value 
of -1 is used for a non-MLO link. but I don't know if that is currently 
universally true.

if that is curently universally true, do we want to now have divergent 
definitions of a link_id?
Vasanthakumar Thiagarajan Aug. 3, 2022, 4:46 p.m. UTC | #2
On 8/2/2022 8:58 PM, Jeff Johnson wrote:
> On 8/1/2022 11:50 PM, Vasanthakumar Thiagarajan wrote:
>> In MLO, when the address translation from link to MLD is done
>> in fw/hw, it is necessary to be able to have some information
>> on the link on which the frame has been received. Extend the
>> rx API to include link_id in ieee80211_rx_status.
>>
>> Signed-off-by: Vasanthakumar Thiagarajan <quic_vthiagar@quicinc.com>
>> ---
>>   include/net/mac80211.h | 4 ++++
>>   1 file changed, 4 insertions(+)
>>
>> diff --git a/include/net/mac80211.h b/include/net/mac80211.h
>> index f198af600b5e..23bc34657b16 100644
>> --- a/include/net/mac80211.h
>> +++ b/include/net/mac80211.h
>> @@ -1480,6 +1480,9 @@ enum mac80211_rx_encoding {
>>    *    each A-MPDU but the same for each subframe within one A-MPDU
>>    * @ampdu_delimiter_crc: A-MPDU delimiter CRC
>>    * @zero_length_psdu_type: radiotap type of the 0-length PSDU
>> + * @link_id: id of the link used to receive the packet. Applicable 
>> only with
>> + *    MLO connection, valid link ids are in 0-14, link_id 15 means 
>> either the
>> + *    link id is not known or it is a non-MLO connection.
>>    */
>>   struct ieee80211_rx_status {
>>       u64 mactime;
>> @@ -1504,6 +1507,7 @@ struct ieee80211_rx_status {
>>       s8 chain_signal[IEEE80211_MAX_CHAINS];
>>       u8 ampdu_delimiter_crc;
>>       u8 zero_length_psdu_type;
>> +    u8 link_id;
>>   };
>>   static inline u32
> 
> in other parts of the MLO code the link_id is defined as int and a value 
> of -1 is used for a non-MLO link. but I don't know if that is currently 
> universally true.
> 
> if that is curently universally true, do we want to now have divergent 
> definitions of a link_id?

Good point, i see link_id is used both as unsigned and int based on
their usage. The reason I preferred unsigned is that we can make use of 
the remaining 4-bits for some other purpose in future, 
ieee80211_rx_status has size limitation.


Vasanth
Johannes Berg Aug. 9, 2022, 6:12 p.m. UTC | #3
On Wed, 2022-08-03 at 22:16 +0530, Vasanthakumar Thiagarajan wrote:
> 
> > > +    u8 link_id;
> > >   };
> > >   static inline u32
> > 
> > in other parts of the MLO code the link_id is defined as int and a value 
> > of -1 is used for a non-MLO link. but I don't know if that is currently 
> > universally true.
> > 
> > if that is curently universally true, do we want to now have divergent 
> > definitions of a link_id?
> 
> Good point, i see link_id is used both as unsigned and int based on
> their usage. The reason I preferred unsigned is that we can make use of 
> the remaining 4-bits for some other purpose in future, 
> ieee80211_rx_status has size limitation.
> 

It's a bit tricky though - do we want to have 0 for all the drivers that
don't support MLO? Might not really be an issue, but OTOH not
initializing it should probably not result in a valid value otherwise
you might test something, think it's fine, and it really isn't.

I think we should spend a bit to have a validity indication in this
case. Even -1 wouldn't address it since you'd have to initialize to
that.

It works better the other way around in APIs etc. where we have a few
producers and many consumers (cfg80211 down to drivers), but less well
this way where we'd have to change drivers...

johannes
Vasanthakumar Thiagarajan Aug. 11, 2022, 9:31 a.m. UTC | #4
On 8/9/2022 11:42 PM, Johannes Berg wrote:
> On Wed, 2022-08-03 at 22:16 +0530, Vasanthakumar Thiagarajan wrote:
>>
>>>> +    u8 link_id;
>>>>    };
>>>>    static inline u32
>>>
>>> in other parts of the MLO code the link_id is defined as int and a value
>>> of -1 is used for a non-MLO link. but I don't know if that is currently
>>> universally true.
>>>
>>> if that is curently universally true, do we want to now have divergent
>>> definitions of a link_id?
>>
>> Good point, i see link_id is used both as unsigned and int based on
>> their usage. The reason I preferred unsigned is that we can make use of
>> the remaining 4-bits for some other purpose in future,
>> ieee80211_rx_status has size limitation.
>>
> 
> It's a bit tricky though - do we want to have 0 for all the drivers that
> don't support MLO? Might not really be an issue, but OTOH not
> initializing it should probably not result in a valid value otherwise
> you might test something, think it's fine, and it really isn't. >
> I think we should spend a bit to have a validity indication in this
> case. Even -1 wouldn't address it since you'd have to initialize to
> that.
> 

Sure, a new valid bit looks cleaner. Ill make the changes.

> It works better the other way around in APIs etc. where we have a few
> producers and many consumers (cfg80211 down to drivers), but less well
> this way where we'd have to change drivers...

I agree.

Thanks!

Vasanth
diff mbox series

Patch

diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index f198af600b5e..23bc34657b16 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -1480,6 +1480,9 @@  enum mac80211_rx_encoding {
  *	each A-MPDU but the same for each subframe within one A-MPDU
  * @ampdu_delimiter_crc: A-MPDU delimiter CRC
  * @zero_length_psdu_type: radiotap type of the 0-length PSDU
+ * @link_id: id of the link used to receive the packet. Applicable only with
+ *	MLO connection, valid link ids are in 0-14, link_id 15 means either the
+ *	link id is not known or it is a non-MLO connection.
  */
 struct ieee80211_rx_status {
 	u64 mactime;
@@ -1504,6 +1507,7 @@  struct ieee80211_rx_status {
 	s8 chain_signal[IEEE80211_MAX_CHAINS];
 	u8 ampdu_delimiter_crc;
 	u8 zero_length_psdu_type;
+	u8 link_id;
 };
 
 static inline u32