2012-05-04 04:38:27

by Chun-Yeow Yeoh

Subject: [PATCH] fix the increment of unicast/multicast counters for forwarded PREQ

Forwarded PREQ is either unicast or multicast. The appropriate counters
should be incremented accordingly.

Signed-off-by: Chun-Yeow Yeoh <[email protected]>
---
net/mac80211/mesh_hwmp.c | 5 ++++-
1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/net/mac80211/mesh_hwmp.c b/net/mac80211/mesh_hwmp.c
index 503016f..f0695e5 100644
--- a/net/mac80211/mesh_hwmp.c
+++ b/net/mac80211/mesh_hwmp.c
@@ -603,7 +603,10 @@ static void hwmp_preq_frame_process(struct ieee80211_sub_if_data *sdata,
hopcount, ttl, cpu_to_le32(lifetime),
cpu_to_le32(metric), cpu_to_le32(preq_id),
sdata);
- ifmsh->mshstats.fwded_mcast++;
+ if (da != broadcast_addr)
+ ifmsh->mshstats.fwded_unicast++;
+ else
+ ifmsh->mshstats.fwded_mcast++;
ifmsh->mshstats.fwded_frames++;
}
}
--
1.7.0.4

2012-05-07 07:43:28

by Johannes Berg

Subject: Re: [PATCH] fix the increment of unicast/multicast counters for forwarded PREQ

On Fri, 2012-05-04 at 12:36 +0800, Chun-Yeow Yeoh wrote:
> Forwarded PREQ is either unicast or multicast. The appropriate counters
> should be incremented accordingly.
>
> Signed-off-by: Chun-Yeow Yeoh <[email protected]>
> ---
> net/mac80211/mesh_hwmp.c | 5 ++++-
> 1 files changed, 4 insertions(+), 1 deletions(-)
>
> diff --git a/net/mac80211/mesh_hwmp.c b/net/mac80211/mesh_hwmp.c
> index 503016f..f0695e5 100644
> --- a/net/mac80211/mesh_hwmp.c
> +++ b/net/mac80211/mesh_hwmp.c
> @@ -603,7 +603,10 @@ static void hwmp_preq_frame_process(struct ieee80211_sub_if_data *sdata,
> hopcount, ttl, cpu_to_le32(lifetime),
> cpu_to_le32(metric), cpu_to_le32(preq_id),
> sdata);
> - ifmsh->mshstats.fwded_mcast++;
> + if (da != broadcast_addr)

In addition to what Javier said. Err. Think about this comparison again.

johannes

2012-05-07 09:21:07

by Johannes Berg

Subject: Re: [PATCH] fix the increment of unicast/multicast counters for forwarded PREQ

Hi,

> If the destination address is not broadcast frame, the number of
> forwarded unicast frame is increased. And the other way round. Not
> correct?
>
> + if (!is_multicast_ether_addr(da))
> + ifmsh->mshstats.fwded_unicast++;
> + else
> + ifmsh->mshstats.fwded_mcast++;

No, your new code is fine, but this

> >> + if (da != broadcast_addr)

will never be false.

johannes

2012-05-07 11:02:31

by Chun-Yeow Yeoh

Subject: Re: [PATCH] fix the increment of unicast/multicast counters for forwarded PREQ

Hi, Kalle,

Appreciate your explanation. I understand my mistake.

Thanks

Regards,
Chun-Yeow

On Mon, May 7, 2012 at 6:41 PM, Kalle Valo <[email protected]> wrote:
> Yeoh Chun-Yeow <[email protected]> writes:
>
>> Hi, Johannes
>>
>>>> >> + ? ? ? ? ? ? if (da != broadcast_addr)
>>>
>>> will never be false.
>>
>> Hmm, but I have a check on this and it works. The previous line will
>> set the da either as broadcast addr or unicast addr.
>
> You are comparing pointers, not addresses. If you do that on purpose
> then it's very confusing.
>
> --
> Kalle Valo

2012-05-07 10:41:36

by Kalle Valo

Subject: Re: [PATCH] fix the increment of unicast/multicast counters for forwarded PREQ

Yeoh Chun-Yeow <[email protected]> writes:

> Hi, Johannes
>
>>> >> +             if (da != broadcast_addr)
>>
>> will never be false.
>
> Hmm, but I have a check on this and it works. The previous line will
> set the da either as broadcast addr or unicast addr.

You are comparing pointers, not addresses. If you do that on purpose
then it's very confusing.

--
Kalle Valo

2012-05-07 08:58:12

by Chun-Yeow Yeoh

Subject: Re: [PATCH] fix the increment of unicast/multicast counters for forwarded PREQ

Hi, Johannes

If the destination address is not broadcast frame, the number of
forwarded unicast frame is increased. And the other way round. Not
correct?

+ if (!is_multicast_ether_addr(da))
+ ifmsh->mshstats.fwded_unicast++;
+ else
+ ifmsh->mshstats.fwded_mcast++;

Regards,
Chun-Yeow

On Mon, May 7, 2012 at 3:43 PM, Johannes Berg <[email protected]> wrote:
> On Fri, 2012-05-04 at 12:36 +0800, Chun-Yeow Yeoh wrote:
>> Forwarded PREQ is either unicast or multicast. The appropriate counters
>> should be incremented accordingly.
>>
>> Signed-off-by: Chun-Yeow Yeoh <[email protected]>
>> ---
>> ?net/mac80211/mesh_hwmp.c | ? ?5 ++++-
>> ?1 files changed, 4 insertions(+), 1 deletions(-)
>>
>> diff --git a/net/mac80211/mesh_hwmp.c b/net/mac80211/mesh_hwmp.c
>> index 503016f..f0695e5 100644
>> --- a/net/mac80211/mesh_hwmp.c
>> +++ b/net/mac80211/mesh_hwmp.c
>> @@ -603,7 +603,10 @@ static void hwmp_preq_frame_process(struct ieee80211_sub_if_data *sdata,
>> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? hopcount, ttl, cpu_to_le32(lifetime),
>> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? cpu_to_le32(metric), cpu_to_le32(preq_id),
>> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? sdata);
>> - ? ? ? ? ? ? ifmsh->mshstats.fwded_mcast++;
>> + ? ? ? ? ? ? if (da != broadcast_addr)
>
> In addition to what Javier said. Err. Think about this comparison again.
>
> johannes
>

2012-05-07 10:19:51

by Chun-Yeow Yeoh

Subject: Re: [PATCH] fix the increment of unicast/multicast counters for forwarded PREQ

Hi, Johannes

>> >> + ? ? ? ? ? ? if (da != broadcast_addr)
>
> will never be false.

Hmm, but I have a check on this and it works. The previous line will
set the da either as broadcast addr or unicast addr.

da = (mpath && mpath->is_root) ?
mpath->rann_snd_addr : broadcast_addr;

Chun-Yeow