2014-12-21 13:25:34

by Eliad Peller

[permalink] [raw]
Subject: [PATCH] mac80211: fix dot11MulticastTransmittedFrameCount tested address

dot11MulticastTransmittedFrameCount should be updated according
to the DA, which might be different from hdr1.

(Checking hdr1 results in the counter being 0 in case of station,
as TODS data frames use hdr1 for the bssid address)

Signed-off-by: Eliad Peller <[email protected]>
---
net/mac80211/status.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/mac80211/status.c b/net/mac80211/status.c
index d64037c..7d4e930 100644
--- a/net/mac80211/status.c
+++ b/net/mac80211/status.c
@@ -862,7 +862,7 @@ void ieee80211_tx_status(struct ieee80211_hw *hw, struct sk_buff *skb)
(info->flags & IEEE80211_TX_STAT_NOACK_TRANSMITTED)) {
if (ieee80211_is_first_frag(hdr->seq_ctrl)) {
local->dot11TransmittedFrameCount++;
- if (is_multicast_ether_addr(hdr->addr1))
+ if (is_multicast_ether_addr(ieee80211_get_DA(hdr)))
local->dot11MulticastTransmittedFrameCount++;
if (retry_count > 0)
local->dot11RetryCount++;
--
1.8.5.1.109.g3d252a9



2014-12-21 14:19:39

by Fred Chou

[permalink] [raw]
Subject: Re: [PATCH] mac80211: fix dot11MulticastTransmittedFrameCount tested address



On 21/12/2014 9:25 PM, Eliad Peller wrote:
> dot11MulticastTransmittedFrameCount should be updated according
> to the DA, which might be different from hdr1.

Shouldn't address 1 be used to determine whether the MPDU is multicast?
>From Std-2012 definition of multicast: "When applied to a MAC protocol
data unit (MPDU), it is an MPDU with a group address in the Address 1
field."

Fred

2014-12-21 16:37:37

by Eliad Peller

[permalink] [raw]
Subject: Re: [PATCH] mac80211: fix dot11MulticastTransmittedFrameCount tested address

On Sun, Dec 21, 2014 at 6:25 PM, Johannes Berg
<[email protected]> wrote:
> On Sun, 2014-12-21 at 17:27 +0200, Eliad Peller wrote:
>> On Sun, Dec 21, 2014 at 4:19 PM, Fred Chou <[email protected]> wrote:
>> > On 21/12/2014 9:25 PM, Eliad Peller wrote:
>> >> dot11MulticastTransmittedFrameCount should be updated according
>> >> to the DA, which might be different from hdr1.
>> >
>> > Shouldn't address 1 be used to determine whether the MPDU is multicast?
>> > From Std-2012 definition of multicast: "When applied to a MAC protocol
>> > data unit (MPDU), it is an MPDU with a group address in the Address 1
>> > field."
>> >
>> good point. i guess it depends on the meaning of
>> dot11MulticastTransmittedFrameCount -
>> multicast frames sent by sta are not multicast frames per se (as they
>> are sent directly to the AP), but they are destined to multicast
>> group.
>>
>> i tried understanding the meaning of this MIB from here (as i couldn't
>> find clear definition in the spec):
>> http://tools.cisco.com/Support/SNMP/do/BrowseOID.do?local=en&translate=Translate&objectInput=dot11MulticastTransmittedFrameCount
>
> Yeah I can't find a good reference in the spec either - anyone want to
> dig through the flow charts? :)
>
> Btw, since this is ancient code from devicescape, it is possible that
> they only cared about AP mode then - and there there's no difference
> between the addresses.
>
makes sense.

>> where it seems to be similar (IIUC) to the way i interpreted it, but i
>> might got it wrong :)
>
> I think you're probably right - however I wonder if we can stick the
> code elsewhere (like subif xmit?) instead of introducing more
> conditionals here?
>
well, we look for the tx status, so i think it makes sense to leave it here.

Eliad.

2014-12-21 15:27:57

by Eliad Peller

[permalink] [raw]
Subject: Re: [PATCH] mac80211: fix dot11MulticastTransmittedFrameCount tested address

On Sun, Dec 21, 2014 at 4:19 PM, Fred Chou <[email protected]> wrote:
> On 21/12/2014 9:25 PM, Eliad Peller wrote:
>> dot11MulticastTransmittedFrameCount should be updated according
>> to the DA, which might be different from hdr1.
>
> Shouldn't address 1 be used to determine whether the MPDU is multicast?
> From Std-2012 definition of multicast: "When applied to a MAC protocol
> data unit (MPDU), it is an MPDU with a group address in the Address 1
> field."
>
good point. i guess it depends on the meaning of
dot11MulticastTransmittedFrameCount -
multicast frames sent by sta are not multicast frames per se (as they
are sent directly to the AP), but they are destined to multicast
group.

i tried understanding the meaning of this MIB from here (as i couldn't
find clear definition in the spec):
http://tools.cisco.com/Support/SNMP/do/BrowseOID.do?local=en&translate=Translate&objectInput=dot11MulticastTransmittedFrameCount

where it seems to be similar (IIUC) to the way i interpreted it, but i
might got it wrong :)

Eliad.

2014-12-23 09:10:25

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH] mac80211: fix dot11MulticastTransmittedFrameCount tested address

On Sun, 2014-12-21 at 15:25 +0200, Eliad Peller wrote:
> dot11MulticastTransmittedFrameCount should be updated according
> to the DA, which might be different from hdr1.
>
> (Checking hdr1 results in the counter being 0 in case of station,
> as TODS data frames use hdr1 for the bssid address)

I've found this in the spec - see sta_tx_dcf_3.1d(10) (page 2410 of
802.11-2012):

cTmcfrm:=
If (fsdu!grpa or
((toDs(tpdu) = 1) and (isGrp(addr3(tpdu))) and (fsdu!fTot=fsdu!
fCur+1))
)
then inc(cTmcfrm)
else cTmcfrm fi

fsdu!grpa is true if RA is multicast, but the rest is clearly checking
the DA (and for fragmentation, which can happen in that case.)

So I believe this patch is correct and will apply it (with some commit
message fixes - should say A1 not hdr1)

johannes


2014-12-21 16:25:06

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH] mac80211: fix dot11MulticastTransmittedFrameCount tested address

On Sun, 2014-12-21 at 17:27 +0200, Eliad Peller wrote:
> On Sun, Dec 21, 2014 at 4:19 PM, Fred Chou <[email protected]> wrote:
> > On 21/12/2014 9:25 PM, Eliad Peller wrote:
> >> dot11MulticastTransmittedFrameCount should be updated according
> >> to the DA, which might be different from hdr1.
> >
> > Shouldn't address 1 be used to determine whether the MPDU is multicast?
> > From Std-2012 definition of multicast: "When applied to a MAC protocol
> > data unit (MPDU), it is an MPDU with a group address in the Address 1
> > field."
> >
> good point. i guess it depends on the meaning of
> dot11MulticastTransmittedFrameCount -
> multicast frames sent by sta are not multicast frames per se (as they
> are sent directly to the AP), but they are destined to multicast
> group.
>
> i tried understanding the meaning of this MIB from here (as i couldn't
> find clear definition in the spec):
> http://tools.cisco.com/Support/SNMP/do/BrowseOID.do?local=en&translate=Translate&objectInput=dot11MulticastTransmittedFrameCount

Yeah I can't find a good reference in the spec either - anyone want to
dig through the flow charts? :)

Btw, since this is ancient code from devicescape, it is possible that
they only cared about AP mode then - and there there's no difference
between the addresses.

> where it seems to be similar (IIUC) to the way i interpreted it, but i
> might got it wrong :)

I think you're probably right - however I wonder if we can stick the
code elsewhere (like subif xmit?) instead of introducing more
conditionals here?

johannes