Bug 602 status

View: New views
20 Messages — Rating Filter:   Alert me  
< Prev | 1 - 2 | Next >

Bug 602 status

by Mirko Banchi :: Rate this Message:

Reply to Author | View Threaded | Show Only this Message




This is a possible patch for bug 602. Please take a look at it.

Thank you all,

Mirko

--
Mirko Banchi

e-mail:    mk.banchi@...
e-mail:    mk.banchi@...
id-jabber: mk.banchi@...

PGP key fingerprint:

308F BFB1 4E67 2522 C88E
DC69 7631 52ED 32A5 6456






QosRetryCounters.patch (19K) Download Attachment
smime.p7s (3K) Download Attachment

Parent Message unknown Re: Bug 602 status

by Mirko Banchi :: Rate this Message:

Reply to Author | View Threaded | Show Only this Message


Il giorno 23/ott/09, alle ore 09:59, Mathieu Lacage ha scritto:

> I think it might be better to just change the signature of
> WifiRemoteStationManager::Lookup and add an enum AccessClass argument.
> Every caller would need to be modified to pass the right value. non-
> qos
> callers would be required to pass AC_BE unconditionally.
>
> Mathieu
>

How could that help us? could you be a little more specific?

Thank you

Mirko

--
Mirko Banchi

e-mail:    mk.banchi@...
e-mail:    mk.banchi@...
id-jabber: mk.banchi@...

PGP key fingerprint:

308F BFB1 4E67 2522 C88E
DC69 7631 52ED 32A5 6456






smime.p7s (3K) Download Attachment

Re: Bug 602 status

by Faker Moatamri :: Rate this Message:

Reply to Author | View Threaded | Show Only this Message

Mirko Banchi wrote:
>
>
> This is a possible patch for bug 602. Please take a look at it.
>
Hi Mirko,
Thanks for submitting the patch. The patch looks good for me but I have
some remarks:
- In mac-low.cc the same piece of code replacing
station->ReportDataFailed() is repeated 3 times using copy & paste, can
you create a small function instead or encapsulate the behavior in
another routine. You can do the same for ReportDataOk, ReportRtsFailed...
-In wifi-remote-station-manager.cc:
    -line 811, please do not use NS_ASSERT (false) and use instead
NS_FATAL_ERROR and include a comprehensive message
    -Also the code if (ac == AC_UNDEF) .... is repeated several times
using copy and paste, sometimes with minor changes like
    *mqosSsrc.find (ac)->second = 0 instead of ++, please encapsulate
this piece of code into a function with comprehensive name
    -Line 927: //How should we use ac parameter here? I have no answer
for that, do you have one? some plans??

Other than that, the patch is cool for me, once you finish fixing the
above you will get +1 from me.
Someone else has comments about the patch?
Best regards
Faker Moatamri

Parent Message unknown Re: Bug 602 status

by Kirill Andreev :: Rate this Message:

Reply to Author | View Threaded | Show Only this Message

>>Mirko Banchi wrote:
>>This is a possible patch for bug 602. Please take a look at it.


>Hi Mirko,
>Thanks for submitting the patch. The patch looks good for me but I have
>some remarks:
>- In mac-low.cc the same piece of code replacing
>station->ReportDataFailed() is repeated 3 times using copy & paste, can
>you create a small function instead or encapsulate the behavior in
>another routine. You can do the same for ReportDataOk, ReportRtsFailed...
>-In wifi-remote-station-manager.cc:
>-line 811, please do not use NS_ASSERT (false) and use instead
>NS_FATAL_ERROR and include a comprehensive message
>-Also the code if (ac == AC_UNDEF) .... is repeated several times
>using copy and paste, sometimes with minor changes like
>*mqosSsrc.find (ac)->second = 0 instead of ++, please encapsulate
>this piece of code into a function with comprehensive name
>-Line 927: //How should we use ac parameter here? I have no answer
>for that, do you have one? some plans??

>Other than that, the patch is cool for me, once you finish fixing the
>above you will get +1 from me.
>Someone else has comments about the patch?
>Best regards
>Faker Moatamri


Hi!, in addition to said above, could you, please, fix
this copy&paste in WifiRemoteStationManager class:

TracedValue<uint32_t> m_ssrc;
TracedValue<uint32_t> m_slrc;
/* voice access class retry counters */
TracedValue<uint32_t> m_voSsrc;
TracedValue<uint32_t> m_voSlrc;
/* video access class retry counters */
TracedValue<uint32_t> m_viSsrc;
TracedValue<uint32_t> m_viSlrc;
/* best-effort access class retry counters */
TracedValue<uint32_t> m_beSsrc;
TracedValue<uint32_t> m_beSlrc;
/* background access class retry counters */
TracedValue<uint32_t> m_bkSsrc;
TracedValue<uint32_t> m_bkSlrc;.

May be is it better to make std::map<AccessClass,
uint32_t> and call traced callback at each change of each
retry counter like the following
TracedCallback<AccessClass, uint32_t> m_slrcCallback; and
call m_slrcCallback(ac, retryCount)?

Is it worth to replace this huge set of traced values (and
trace sources in GetTypeId) with two maps and two traced
callbacks?
It seems to me that fixed number of available access
classes is not good here.

Regards,
Kirill Andreev,
IITP RAS

Re: Bug 602 status

by Mirko Banchi :: Rate this Message:

Reply to Author | View Threaded | Show Only this Message


Il giorno 23/ott/09, alle ore 11:40, Faker Moatamri ha scritto:

> Mirko Banchi wrote:
>>
>>
>> This is a possible patch for bug 602. Please take a look at it.
>>
> Hi Mirko,
> Thanks for submitting the patch. The patch looks good for me but I  
> have some remarks:
> - In mac-low.cc the same piece of code replacing station-
> >ReportDataFailed() is repeated 3 times using copy & paste, can you  
> create a small function instead or encapsulate the behavior in  
> another routine. You can do the same for ReportDataOk,  
> ReportRtsFailed...
Yes, you are right. i'll create two functions ReportDataFailed,  
ReportDataOk that could be useful in order to avoid future copy and  
paste. About ReportRtsFailed and ReportCtsOk i think that there is no  
need to create separate routine because they will use only one time:  
when we'll receive  a Cts and when a CtsTimeout occurs.

> -In wifi-remote-station-manager.cc:
>   -line 811, please do not use NS_ASSERT (false) and use instead  
> NS_FATAL_ERROR and include a comprehensive message

Ok.

>   -Also the code if (ac == AC_UNDEF) .... is repeated several times  
> using copy and paste, sometimes with minor changes like
>   *mqosSsrc.find (ac)->second = 0 instead of ++, please encapsulate  
> this piece of code into a function with comprehensive name

I agree. I'll define two function:

WifiRemoteStation::UpdateSsrcCounter (enum AccessClass ac, bool reset)
WifiRemoteStation::UpdateSlrcCounter (enum AccessClass ac, bool reset)

>   -Line 927: //How should we use ac parameter here? I have no answer  
> for that, do you have one? some plans??

Unfortunately i have no idea. Maybe we should ask Mathieu or other  
authors of wifi managers.

Mirko

--
Mirko Banchi

e-mail:    mk.banchi@...
e-mail:    mk.banchi@...
id-jabber: mk.banchi@...

PGP key fingerprint:

308F BFB1 4E67 2522 C88E
DC69 7631 52ED 32A5 6456






smime.p7s (3K) Download Attachment

Re: Bug 602 status

by Mirko Banchi :: Rate this Message:

Reply to Author | View Threaded | Show Only this Message


Il giorno 23/ott/09, alle ore 12:00, Kirill Andreev ha scritto:

>
> Hi!, in addition to said above, could you, please, fix this  
> copy&paste in WifiRemoteStationManager class:
>
> TracedValue<uint32_t> m_ssrc;
> TracedValue<uint32_t> m_slrc;
> /* voice access class retry counters */
> TracedValue<uint32_t> m_voSsrc;
> TracedValue<uint32_t> m_voSlrc;
> /* video access class retry counters */
> TracedValue<uint32_t> m_viSsrc;
> TracedValue<uint32_t> m_viSlrc;
> /* best-effort access class retry counters */
> TracedValue<uint32_t> m_beSsrc;
> TracedValue<uint32_t> m_beSlrc;
> /* background access class retry counters */
> TracedValue<uint32_t> m_bkSsrc;
> TracedValue<uint32_t> m_bkSlrc;.
>
> May be is it better to make std::map<AccessClass, uint32_t> and call  
> traced callback at each change of each retry counter like the  
> following
> TracedCallback<AccessClass, uint32_t> m_slrcCallback; and call  
> m_slrcCallback(ac, retryCount)?
>
> Is it worth to replace this huge set of traced values (and trace  
> sources in GetTypeId) with two maps and two traced callbacks?
> It seems to me that fixed number of available access classes is not  
> good here.
Maybe the best way is to add in future other counters for the new  
access classes. The total number of possible access classes is 8; 9 if  
we consider AC_BE_NQOS access class. If this number was variable i'd  
agree with you, but in this condition i think that use of traced  
callback is tedius.

Regards,

Mirko

--
Mirko Banchi

e-mail:    mk.banchi@...
e-mail:    mk.banchi@...
id-jabber: mk.banchi@...

PGP key fingerprint:

308F BFB1 4E67 2522 C88E
DC69 7631 52ED 32A5 6456






smime.p7s (3K) Download Attachment

Re: Bug 602 status (patch)

by Mirko Banchi :: Rate this Message:

Reply to Author | View Threaded | Show Only this Message


Here is the patch for bug #602 with the corrections that Faker  
suggested.
If there are no objections or advices i'll apply it.

Thank you all.

Mirko


--
Mirko Banchi

e-mail:    mk.banchi@...
e-mail:    mk.banchi@...
id-jabber: mk.banchi@...

PGP key fingerprint:

308F BFB1 4E67 2522 C88E
DC69 7631 52ED 32A5 6456






QosRetryCounters.patch (35K) Download Attachment
smime.p7s (3K) Download Attachment

Parent Message unknown Re: Bug 602 status (patch)

by Kirill Andreev :: Rate this Message:

Reply to Author | View Threaded | Show Only this Message

>Here is the patch for bug #602 with the corrections that Faker suggested.
>If there are no objections or advices i'll apply it.
>
>Thank you all.
>
>Mirko

To apply a patch you need one more +1 from Pavel Boyko,
but he is away till monday.
or by Nicola Baldo.
I am doing on behalf of Pavel Boyko because he is away
now.
I still do not agree with copy-paste in
WifiRemoteStationManager

Regard,
Kirill Andreev,
IITP RAS

Re: Bug 602 status (patch)

by Nicola Baldo-3 :: Rate this Message:

Reply to Author | View Threaded | Show Only this Message

Hi Mirco & all,

I understand that Bug 602 is an issue and I acknowledge your effort in
trying to solve that; however, I am not very convinced about it. I would
rather propose to change WifiRemoteStationManager so that it maintains
an instance of WifiRemoteStation for every receiver address and access
category (tid). This would require the following modifications:

- add a new member variable to WifiRemoteStation:
uint8_t m_tid;

- change this member method of WifiRemoteStationManager
WifiRemoteStation *Lookup (Mac48Address address)
to the following:
WifiRemoteStation *Lookup (Mac48Address address, uint8_t tid)

- change the implementation of WifiRemoteStationManager::Lookup() in
wifi-remote-station-manager.cc so that it checks for the tid in addition
to the remote station address.

BTW, I've just been re-reading prior emails on this subject, and I get
the impression that this is similar to what Mathieu was proposing.

What do you think?

Regards,

Nicola



Kirill Andreev wrote:

>> Here is the patch for bug #602 with the corrections that Faker suggested.
>> If there are no objections or advices i'll apply it.
>>
>> Thank you all.
>>
>> Mirko
>
> To apply a patch you need one more +1 from Pavel Boyko, but he is away
> till monday.
> or by Nicola Baldo.
> I am doing on behalf of Pavel Boyko because he is away now.
> I still do not agree with copy-paste in WifiRemoteStationManager
>
> Regard,
> Kirill Andreev,
> IITP RAS

Re: Bug 602 status (patch)

by Mirko Banchi :: Rate this Message:

Reply to Author | View Threaded | Show Only this Message


Il giorno 23/ott/09, alle ore 19:26, Nicola Baldo ha scritto:

> Hi Mirco & all,
>
> I understand that Bug 602 is an issue and I acknowledge your effort  
> in trying to solve that; however, I am not very convinced about it.  
> I would rather propose to change WifiRemoteStationManager so that it  
> maintains an instance of WifiRemoteStation for every receiver  
> address and access category (tid). This would require the following  
> modifications:
>
> - add a new member variable to WifiRemoteStation:
> uint8_t m_tid;
>
> - change this member method of WifiRemoteStationManager
> WifiRemoteStation *Lookup (Mac48Address address)
> to the following:
> WifiRemoteStation *Lookup (Mac48Address address, uint8_t tid)
>
> - change the implementation of WifiRemoteStationManager::Lookup() in  
> wifi-remote-station-manager.cc so that it checks for the tid in  
> addition to the remote station address.
>
> BTW, I've just been re-reading prior emails on this subject, and I  
> get the impression that this is similar to what Mathieu was proposing.
>
> What do you think?
I think that it could be a solution but i don't understand the  
advantages in respect to my patch. Moreover, changing signature of  
WifiRemoteStationManager::Lookup method involves a lot of tedious  
changes.

Regards,

Mirko

--
Mirko Banchi

e-mail:    mk.banchi@...
e-mail:    mk.banchi@...
id-jabber: mk.banchi@...

PGP key fingerprint:

308F BFB1 4E67 2522 C88E
DC69 7631 52ED 32A5 6456






smime.p7s (3K) Download Attachment

Re: Bug 602 status (patch)

by Mathieu Lacage :: Rate this Message:

Reply to Author | View Threaded | Show Only this Message

On Sat, 2009-10-24 at 10:54 +0200, Mirko Banchi wrote:

> > I understand that Bug 602 is an issue and I acknowledge your effort  
> > in trying to solve that; however, I am not very convinced about it.  
> > I would rather propose to change WifiRemoteStationManager so that it  
> > maintains an instance of WifiRemoteStation for every receiver  

agreed.

> > address and access category (tid). This would require the following  
> > modifications:
> >
> > - add a new member variable to WifiRemoteStation:
> > uint8_t m_tid;
> >
> > - change this member method of WifiRemoteStationManager
> > WifiRemoteStation *Lookup (Mac48Address address)
> > to the following:
> > WifiRemoteStation *Lookup (Mac48Address address, uint8_t tid)
> >
> > - change the implementation of WifiRemoteStationManager::Lookup() in  
> > wifi-remote-station-manager.cc so that it checks for the tid in  
> > addition to the remote station address.
> >
> > BTW, I've just been re-reading prior emails on this subject, and I  
> > get the impression that this is similar to what Mathieu was proposing.
> >
> > What do you think?
>
> I think that it could be a solution but i don't understand the  
> advantages in respect to my patch. Moreover, changing signature of  
> WifiRemoteStationManager::Lookup method involves a lot of tedious  
> changes.

The compiler will trivially catch all of the needed changes as errors
so, it's just automated work: you can switch off your brain and get it
done in less than 30mins :)

If you need access to a nice build machine, I can give you an account on
the 8-core/16GB box rahan.inria.fr.

Mathieu


Re: Bug 602 status (patch)

by Mirko Banchi :: Rate this Message:

Reply to Author | View Threaded | Show Only this Message


Il giorno 25/ott/09, alle ore 09:19, Mathieu Lacage ha scritto:

> On Sat, 2009-10-24 at 10:54 +0200, Mirko Banchi wrote:
>
>>> I understand that Bug 602 is an issue and I acknowledge your effort
>>> in trying to solve that; however, I am not very convinced about it.
>>> I would rather propose to change WifiRemoteStationManager so that it
>>> maintains an instance of WifiRemoteStation for every receiver
>
> agreed.
>
>>> address and access category (tid). This would require the following
>>> modifications:
>>>
>>> - add a new member variable to WifiRemoteStation:
>>> uint8_t m_tid;
This doens't work. :( The counter must be mainteined for each access  
class. See section 9.9.1.6 in IEEE802.11 standard.
Better a variable like the following:

AccessClass m_ac.

>>>
>>> - change this member method of WifiRemoteStationManager
>>> WifiRemoteStation *Lookup (Mac48Address address)
>>> to the following:
>>> WifiRemoteStation *Lookup (Mac48Address address, uint8_t tid)

The same of above:

WifiRemoteStation *Lookup (Mac48Address address, AccessClass ac)

> The compiler will trivially catch all of the needed changes as errors
> so, it's just automated work: you can switch off your brain and get it
> done in less than 30mins :)
>
> If you need access to a nice build machine, I can give you an  
> account on
> the 8-core/16GB box rahan.inria.fr.
>


Ok! If i need it i'll ask you! Thank you.

Mirko

--
Mirko Banchi

e-mail:    mk.banchi@...
e-mail:    mk.banchi@...
id-jabber: mk.banchi@...

PGP key fingerprint:

308F BFB1 4E67 2522 C88E
DC69 7631 52ED 32A5 6456






smime.p7s (3K) Download Attachment

Re: Bug 602 status (patch)

by Nicola Baldo-3 :: Rate this Message:

Reply to Author | View Threaded | Show Only this Message

Hi Mirko,


>>>> - add a new member variable to WifiRemoteStation:
>>>> uint8_t m_tid;
>
> This doens't work. :( The counter must be mainteined for each access
> class. See section 9.9.1.6 in IEEE802.11 standard.
> Better a variable like the following:
>
> AccessClass m_ac.
>
[...]
>>>> - change this member method of WifiRemoteStationManager
>>>> WifiRemoteStation *Lookup (Mac48Address address)
>>>> to the following:
>>>> WifiRemoteStation *Lookup (Mac48Address address, uint8_t tid)
>
> The same of above:
>
> WifiRemoteStation *Lookup (Mac48Address address, AccessClass ac)
>

I think the two solutions (AC vs. TID) are equivalent since there is a
one-to-one mapping from a TID of an outgoing frame to an AC.
However, after reading section 9.9.1.6 in the standard I agree that
using AC is more appropriate, so I am fine with the above changes that
you proposed.


Nicola

Re: Bug 602 status (patch)

by Mirko Banchi :: Rate this Message:

Reply to Author | View Threaded | Show Only this Message


Il giorno 26/ott/09, alle ore 10:16, Nicola Baldo ha scritto:

> Hi Mirko,
>
>
>>>>> - add a new member variable to WifiRemoteStation:
>>>>> uint8_t m_tid;
>> This doens't work. :( The counter must be mainteined for each  
>> access class. See section 9.9.1.6 in IEEE802.11 standard.
>> Better a variable like the following:
>> AccessClass m_ac.
> [...]
>>>>> - change this member method of WifiRemoteStationManager
>>>>> WifiRemoteStation *Lookup (Mac48Address address)
>>>>> to the following:
>>>>> WifiRemoteStation *Lookup (Mac48Address address, uint8_t tid)
>> The same of above:
>> WifiRemoteStation *Lookup (Mac48Address address, AccessClass ac)
>
> I think the two solutions (AC vs. TID) are equivalent since there is  
> a one-to-one mapping from a TID of an outgoing frame to an AC.
> However, after reading section 9.9.1.6 in the standard I agree that  
> using AC is more appropriate, so I am fine with the above changes  
> that you proposed.
The two solutions aren't equivalent because two instances of  
WifiRemoteStation with different TIDs that are mapped to the same AC  
have to share the same counter.

Regards,

Mirko

--
Mirko Banchi

e-mail:    mk.banchi@...
e-mail:    mk.banchi@...
id-jabber: mk.banchi@...

PGP key fingerprint:

308F BFB1 4E67 2522 C88E
DC69 7631 52ED 32A5 6456






smime.p7s (3K) Download Attachment

Re: Bug 602 status (patch)

by Mirko Banchi :: Rate this Message:

Reply to Author | View Threaded | Show Only this Message


Hi Mathieu,Nicola and all,

i'm analysing your solution for correction of bug 602. I noticed we  
have to also change signature of MacLow::GetStation method:

MacLow::GetStation (Mac48Address, const WifiMacHeader&)

That involves other changes to other functions like:

MacLow::GetAckDuration
MacLow::GetAckTxModeForData
MacLow::GetCtsTxModeForRts
....

However all functions that call MacLow::GetStation.

What do you think?

Regards,

Mirko

--
Mirko Banchi

e-mail:    mk.banchi@...
e-mail:    mk.banchi@...
id-jabber: mk.banchi@...

PGP key fingerprint:

308F BFB1 4E67 2522 C88E
DC69 7631 52ED 32A5 6456






smime.p7s (3K) Download Attachment

Re: Bug 602 status (patch)

by Mathieu Lacage :: Rate this Message:

Reply to Author | View Threaded | Show Only this Message

On Mon, 2009-10-26 at 15:41 +0100, Mirko Banchi wrote:

> That involves other changes to other functions like:
>
> MacLow::GetAckDuration
> MacLow::GetAckTxModeForData
> MacLow::GetCtsTxModeForRts
> ....
>
> However all functions that call MacLow::GetStation.
>
> What do you think?

It's not clear to me what the best solution is but it seems to me that
if you did what nicola suggested first, that is, use the tid instead of
the AC within the Lookup method signature, you could perform the
necessary conversion from tid to AC within Lookup itself instead of
requiring the caller to do it and make MacLow::GetStation take a tid as
second argument which should not be too hard.

Mathieu


Re: Bug 602 status (patch)

by Nicola Baldo-3 :: Rate this Message:

Reply to Author | View Threaded | Show Only this Message

Hi Mirko & Mathieu,

Mathieu Lacage wrote:

> On Mon, 2009-10-26 at 15:41 +0100, Mirko Banchi wrote:
>
>> That involves other changes to other functions like:
>>
>> MacLow::GetAckDuration
>> MacLow::GetAckTxModeForData
>> MacLow::GetCtsTxModeForRts
>> ....
>>
>> However all functions that call MacLow::GetStation.
>>
>> What do you think?
>
> It's not clear to me what the best solution is but it seems to me that
> if you did what nicola suggested first, that is, use the tid instead of
> the AC within the Lookup method signature, you could perform the
> necessary conversion from tid to AC within Lookup itself instead of
> requiring the caller to do it and make MacLow::GetStation take a tid as
> second argument which should not be too hard.
>

Sorry I forgot to state clearly that I agree with Mirko's last argument
on TID vs AC, i.e., that since different TIDs can be mapped to the same
AC the two solutions are not always equivalent, and we need to use AC to
comply with Section 9.9.1.6 in the standard in all possible cases.

So the current "most agreed" modification is this one:

WifiRemoteStation *
WifiRemoteStationManager::Lookup (Mac48Address address, AccessClass ac);


As for the changes in the signature of MacLow::GetStation, what about
doing the following?

WifiRemoteStation *
MacLow::GetStation (const WifiMacHeader& hdr) const
{
   return m_stationManager->Lookup (hdr.GetAddr2 (),
         QosUtilsMapTidToAc (hdr.GetQosTid ()));
}

this means that in mac-low.cc all current calls of the type
GetStation (m_currentHdr.GetAddr1 ());
would be simply changed into
GetStation (m_currentHdr);


Regards,

Nicola



Re: Bug 602 status (patch)

by Mirko Banchi :: Rate this Message:

Reply to Author | View Threaded | Show Only this Message


Il giorno 30/ott/09, alle ore 10:43, Nicola Baldo ha scritto:

> Hi Mirko & Mathieu,
>
> Mathieu Lacage wrote:
>> On Mon, 2009-10-26 at 15:41 +0100, Mirko Banchi wrote:
>>> That involves other changes to other functions like:
>>>
>>> MacLow::GetAckDuration
>>> MacLow::GetAckTxModeForData
>>> MacLow::GetCtsTxModeForRts
>>> ....
>>>
>>> However all functions that call MacLow::GetStation.
>>>
>>> What do you think?
>> It's not clear to me what the best solution is but it seems to me  
>> that
>> if you did what nicola suggested first, that is, use the tid  
>> instead of
>> the AC within the Lookup method signature, you could perform the
>> necessary conversion from tid to AC within Lookup itself instead of
>> requiring the caller to do it and make MacLow::GetStation take a  
>> tid as
>> second argument which should not be too hard.
>
> Sorry I forgot to state clearly that I agree with Mirko's last  
> argument on TID vs AC, i.e., that since different TIDs can be mapped  
> to the same AC the two solutions are not always equivalent, and we  
> need to use AC to comply with Section 9.9.1.6 in the standard in all  
> possible cases.
>
> So the current "most agreed" modification is this one:
>
> WifiRemoteStation *
> WifiRemoteStationManager::Lookup (Mac48Address address, AccessClass  
> ac);
>
It could be.

>
> As for the changes in the signature of MacLow::GetStation, what  
> about doing the following?
>
> WifiRemoteStation *
> MacLow::GetStation (const WifiMacHeader& hdr) const
> {
>  return m_stationManager->Lookup (hdr.GetAddr2 (),
> QosUtilsMapTidToAc (hdr.GetQosTid ()));
> }
>
> this means that in mac-low.cc all current calls of the type
> GetStation (m_currentHdr.GetAddr1 ());
> would be simply changed into
> GetStation (m_currentHdr);
>
I think taht with this change it''s not possible to choose address  
type (addr1 or addr2) for GetStation method. However the problems are  
in MacLow.
For instance, how would you call GetStation in  
MacLow::GetCtsTxModeForRts? I think that in some cases this solution  
is not applicable.


In addiction, i think that there is also another bug in MacLow.  
Function like NormalAckTimeout, FastAckTimeout... call method  
GetStation with argument m_currentHdr.GetAddr1 () but should it be  
m_currentHdr.GetAddr2 () instead?
Tx station should increment its retry counter not Rx station.

What do you think?

best regards,

Mirko

--
Mirko Banchi

e-mail:    mk.banchi@...
e-mail:    mk.banchi@...
id-jabber: mk.banchi@...

PGP key fingerprint:

308F BFB1 4E67 2522 C88E
DC69 7631 52ED 32A5 6456






smime.p7s (3K) Download Attachment

Re: Bug 602 status (patch)

by Nicola Baldo-3 :: Rate this Message:

Reply to Author | View Threaded | Show Only this Message

Hi,

>> this means that in mac-low.cc all current calls of the type
>> GetStation (m_currentHdr.GetAddr1 ());
>> would be simply changed into
>> GetStation (m_currentHdr);
>>
>
> I think taht with this change it''s not possible to choose address type
> (addr1 or addr2) for GetStation method.

Ok so that was just a stupid idea, let's forget it. Sorry for the noise.


However the problems are in MacLow.
> For instance, how would you call GetStation in
> MacLow::GetCtsTxModeForRts? I think that in some cases this solution is
> not applicable.


So what you say is that when a STA receives a RTS it needs to call
GetStation (), but since the TID is not provided within the RTS we
cannot determine the AC and pass it to GetStation (). I agree that's a
problem.

I think that more in general the issue is that WifiRemoteStation handles
many different functionalities which are related to a connection (a
tx-rx pair), however the exact definition of connection varies slightly
for each functionality considered. Examples:

retx count -> connection identified by remote address and AC
STA Association -> connection identified by desination address only?
Rate Adaptation -> standard doesn't define it, but it would be a nice
feature if a connection were identified by remote address and AC or TID

At this point I don't know what could be a good solution to this
problem. Any ideas?

Nicola




Re: Bug 602 status (patch)

by Mirko Banchi :: Rate this Message:

Reply to Author | View Threaded | Show Only this Message


Il giorno 30/ott/09, alle ore 12:31, Nicola Baldo ha scritto:

> Hi,
>
>>> this means that in mac-low.cc all current calls of the type
>>> GetStation (m_currentHdr.GetAddr1 ());
>>> would be simply changed into
>>> GetStation (m_currentHdr);
>>>
>> I think taht with this change it''s not possible to choose address  
>> type (addr1 or addr2) for GetStation method.
>
> Ok so that was just a stupid idea, let's forget it. Sorry for the  
> noise.
No problems :)

>
> However the problems are in MacLow.
>> For instance, how would you call GetStation in  
>> MacLow::GetCtsTxModeForRts? I think that in some cases this  
>> solution is not applicable.
>
>
> So what you say is that when a STA receives a RTS it needs to call  
> GetStation (), but since the TID is not provided within the RTS we  
> cannot determine the AC and pass it to GetStation (). I agree that's  
> a problem.
>
> I think that more in general the issue is that WifiRemoteStation  
> handles many different functionalities which are related to a  
> connection (a tx-rx pair), however the exact definition of  
> connection varies slightly for each functionality considered.  
> Examples:
>
> retx count -> connection identified by remote address and AC
> STA Association -> connection identified by desination address only?
> Rate Adaptation -> standard doesn't define it, but it would be a  
> nice feature if a connection were identified by remote address and  
> AC or TID
>
> At this point I don't know what could be a good solution to this  
> problem. Any ideas?
>
I think the first patch i created it's the best solution because  
access class information is passed to methods  
WifiRemoteStation::ReportDataFailed,  
WifiRemoteStation::ReportRtsFailed... directly with the call. The  
patch is in attachment. Please take a look at it.


.

What do you think about above-mentioned bug in  
MacLow::NormalAckTimeout, MacLow::FastAckTimeout ...?

Thank you,

Mirko

--
Mirko Banchi

e-mail:    mk.banchi@...
e-mail:    mk.banchi@...
id-jabber: mk.banchi@...

PGP key fingerprint:

308F BFB1 4E67 2522 C88E
DC69 7631 52ED 32A5 6456






QosRetryCounters.patch (35K) Download Attachment
smime.p7s (3K) Download Attachment
< Prev | 1 - 2 | Next >