On Thu, 2025-05-15 at 23:05 +0530, Sarika Sharma wrote:
> On 5/15/2025 5:00 PM, Johannes Berg wrote:
> > On Thu, 2025-05-15 at 11:19 +0530, Sarika Sharma wrote:
> > >
> > > +void sta_set_accumulated_removed_links_sinfo(struct sta_info *sta,
> > > + struct station_info *sinfo)
> > > +{
> > > + /* Resetting the MLO statistics for accumulated fields, to
> > > + * avoid duplication.
> > > + */
> > > + sinfo->tx_packets = 0;
> > > + sinfo->rx_packets = 0;
> > > + sinfo->tx_bytes = 0;
> > > + sinfo->rx_bytes = 0;
> > > + sinfo->tx_retries = 0;
> > > + sinfo->tx_failed = 0;
> > > + sinfo->rx_dropped_misc = 0;
> > > + sinfo->beacon_loss_count = 0;
> > > + sinfo->expected_throughput = 0;
> > > + sinfo->rx_mpdu_count = 0;
> > > + sinfo->fcs_err_count = 0;
> > > + sinfo->rx_beacon = 0;
> > > + sinfo->rx_duration = 0;
> > > + sinfo->tx_duration = 0;
> > > +
> > > + /* Accumulating the removed link statistics. */
> > > + sinfo->tx_packets += sta->rem_link_stats.tx_packets;
> > > + sinfo->rx_packets += sta->rem_link_stats.rx_packets;
> > > + sinfo->tx_bytes += sta->rem_link_stats.tx_bytes;
> > > + sinfo->rx_bytes += sta->rem_link_stats.rx_bytes;
> > > + sinfo->tx_retries += sta->rem_link_stats.tx_retries;
> > > + sinfo->tx_failed += sta->rem_link_stats.tx_failed;
> > > + sinfo->rx_dropped_misc += sta->rem_link_stats.rx_dropped_misc;
> >
> > Setting something to 0 just to += it seems silly?
> >
> > However I think it also needs a bit more explanation - it's sinfo, so
> > it's zeroed at allocation, where would non-zero numbers come from?
>
> Currently, the station information for MLO is populated with some values
> from sta->deflink, as the sta_set_sinfo() call is common for both
> non-MLO and MLO.
OK but deflink will not actually _have_ any values. And again, mac80211
shouldn't be doing work the results of which are then only discarded...
> When updating the station_info structure in
> cfg80211_sta_set_mld_sinfo(), the accumulated fields (such as packets,
> bytes, etc.) will already contain values set by mac80211 (from the
> deflink fields).
Which arguably is the problem? But also not because those are zero
anyway? I still don't think this makes any sense. Either it's not filled
and remains zero, or it should be filled only with some sensible values
like the accumulated stats from removed links.
johannes
