Question on total_tbtt_count

[Aditya Kumar Singh]

On 6/2/24 22:34, Ben Greear wrote:
> Hello,
> 
> While rebasing some patches, I noticed that total_tbtt_count wasn't used
> as I expected.
> 
> Shouldn't it be incremented before tbtt_count is set to 0 below?

Where exactly you are referring to?

> 
> static size_t
> hostapd_eid_rnr_iface_len(struct hostapd_data *hapd,
>                            struct hostapd_data *reporting_hapd,
>                            size_t *current_len,
>                            struct mbssid_ie_profiles *skip_profiles,
>                            bool mld_update)
> {
>          size_t total_len = 0, len = *current_len;
>          int tbtt_count, total_tbtt_count = 0;
>          size_t i, start;
>          u8 tbtt_info_len = mld_update ? RNR_TBTT_INFO_MLD_LEN :
>                  RNR_TBTT_INFO_LEN;
> 
> repeat_rnr_len:
>          start = 0;
>          tbtt_count = 0;

Here?

> 
>          while (start < hapd->iface->num_bss) {
>                  if (!len ||
>                      len + RNR_TBTT_HEADER_LEN + tbtt_info_len > 255 ||
>                      tbtt_count >= RNR_TBTT_INFO_COUNT_MAX) {
>                          len = RNR_HEADER_LEN;
>                          total_len += RNR_HEADER_LEN;
>                          tbtt_count = 0;

Or here?

>                  }
> 
> 

In any case, Why it should be incremented? After the iteration of the 
loop (outer while loop), we would have the no. of TBTTs being considered 
in the RNR in tbtt_count. So before we jump back again to label 
repeat_rnr_len for rebuilding in case of colocation + MLD info, we keep 
the total count in total_tbtt_count and build RNR again considering MLD 
info this time. In order to see how many TBTT we considered in next go, 
we reset the tbtt_count back to 0.


> And farther down, I think maybe it should then not be incremented after 
> the loop, but maybe
> incremented before this code?
> 
>        /* This is possible when in the re-built case and no suitable 
> TBTT was
>           * found. Adjust the length accordingly. */
>          if (!tbtt_count && total_tbtt_count) {
>                  len -= RNR_TBTT_HEADER_LEN;
>                  total_len -= RNR_TBTT_HEADER_LEN;
>          }
> 

Nope not really. When we have to leverage the re-building in case of 
colocation + MLD info, then before jumping to that label, we need to 
store the current already considered TBTTs no.