Re: [RFC][PATCH] TX status reporting with help of an ack queue

Re: [RFC][PATCH] TX status reporting with help of an ack queue

[Ivo van Doorn]

On Monday 15 September 2008, Mikko Virkkil=C3=A4 wrote:
> Modeled after the waiting-for-ack queue in the zd1211rw
> driver, this patch adds a similar queue to the rt2x00. If
> the tx state of a packet is unknown and a tx ack or
> tx fail is needed, then the packet is pushed to a
> waiting-for-ack queue. Each time an ack is received the
> queue is checked and a possible match is reported to
> mac80211 as a tx ack.
>=20
> If the queue gets filled up the oldest waiting packet
> will be discarded and a tx fail will be reported.

I believe it was mentioned it earlier as well, but this code belongs
to mac80211 rather than being copied in all individual drivers.

I think the best solution would be:
 - Drivers set *unknown TX status* flag for mac80211
 - Mac80211 sets the FIF_CONTROL flag for the RX filter (or adds a new
   more specific flag FIF_ACK for hardware that allows this specific fi=
ltering (rt2800))
 - Driver will set a *unknown TX status* flag per send out frame
 - Mac80211 keeps track of ACK responses, and decides what to do when
   no ACK has been received for the frame.

Note that rt61 might be tricky since that does support TX status report=
ing,
but it fails in only 1% of the cases to do this, which means enabling t=
his for rt61
would be far too much overhead.

> This doesn't use timers so a message might in theory be
> stuck in the queue indefinitely.

Well you wouldn't need timers actually, if you have a list of packets
waiting to be acked you can expect the frames to be acked in the order
of which they were send (when using the same queue).

> diff --git a/drivers/net/wireless/rt2x00/rt2x00dev.c b/drivers/net/wi=
reless/rt2x00/rt2x00dev.c
> index 2f3bfc6..57020de 100644
> --- a/drivers/net/wireless/rt2x00/rt2x00dev.c
> +++ b/drivers/net/wireless/rt2x00/rt2x00dev.c
> @@ -495,6 +495,30 @@ void rt2x00lib_beacondone(struct rt2x00_dev *rt2=
x00dev)
>  }
>  EXPORT_SYMBOL_GPL(rt2x00lib_beacondone);
> =20
> +static struct sk_buff *rt2x00lib_get_match_from_ack_queue(
> +	struct sk_buff_head *q, struct ieee80211_hdr *rx_hdr)
> +{
> +	struct sk_buff *skb;
> +	for (skb =3D q->next; skb !=3D (struct sk_buff *)q; skb =3D skb->ne=
xt) {
> +		struct ieee80211_hdr *tx_hdr;
> +		tx_hdr =3D (struct ieee80211_hdr *)skb->data;
> +		if (likely(!compare_ether_addr(tx_hdr->addr2, rx_hdr->addr1))) {

Why is this likely() do we really expect the first ack in the list to b=
e the rigth one?
And is this enough to match a frame to an ack?

> @@ -560,7 +584,22 @@ void rt2x00lib_txdone(struct queue_entry *entry,
>  	 * status report back.
>  	 */
>  	if (tx_info->flags & IEEE80211_TX_CTL_REQ_TX_STATUS)
> -		ieee80211_tx_status_irqsafe(rt2x00dev->hw, entry->skb);
> +		if (test_bit(TXDONE_UNKNOWN, &txdesc->flags)) {
> +			struct sk_buff *failed_skb;
> +			failed_skb =3D rt2x00lib_push_to_ack_queue(
> +				&rt2x00dev->ack_wait_queue, entry->skb);
> +			if (failed_skb) {
> +				struct ieee80211_tx_info *info;
> +				info =3D IEEE80211_SKB_CB(failed_skb);
> +				info->status.excessive_retries =3D 1;
> +				rt2x00dev->link.qual.tx_failed++;
> +				rt2x00dev->low_level_stats.dot11RTSFailureCount++;
> +				DEBUG(rt2x00dev, "ACK queue: fail oldest\n");
> +				ieee80211_tx_status_irqsafe(rt2x00dev->hw,
> +					failed_skb);
> +			}
> +		} else
> +			ieee80211_tx_status_irqsafe(rt2x00dev->hw, entry->skb);
>  	else
>  		dev_kfree_skb_irq(entry->skb);
> =20
> @@ -694,7 +733,26 @@ void rt2x00lib_rxdone(struct rt2x00_dev *rt2x00d=
ev,
>  	 * mac80211 will clean up the skb structure.
>  	 */
>  	rt2x00debug_dump_frame(rt2x00dev, DUMP_FRAME_RXDONE, entry->skb);
> -	ieee80211_rx_irqsafe(rt2x00dev->hw, entry->skb, rx_status);
> +
> +	if (ieee80211_is_ack(hdr->frame_control)) {
> +		struct sk_buff *match =3D NULL;
> +		match =3D rt2x00lib_get_match_from_ack_queue(
> +			&rt2x00dev->ack_wait_queue,	hdr);
> +		if (match) {
> +			struct ieee80211_tx_info *tx_info;
> +			tx_info =3D IEEE80211_SKB_CB(match);
> +			tx_info->flags |=3D IEEE80211_TX_STAT_ACK;
> +			rt2x00dev->low_level_stats.dot11RTSSuccessCount++;

Incorrect, you need to check if this was an RTS frame if you want to in=
crement this counter.

> +			rt2x00dev->link.qual.tx_success++;
> +			DEBUG(rt2x00dev, "ACK hit in queue\n")
> +			ieee80211_tx_status_irqsafe(rt2x00dev->hw, match);
> +		} else {
> +			DEBUG(rt2x00dev, "Got ACK but not in queue\n")
> +		}
> +		/* Unmap done at start of func, so just free */
> +		dev_kfree_skb_irq(entry->skb);
> +    } else
> +		ieee80211_rx_irqsafe(rt2x00dev->hw, entry->skb, rx_status);

This breaks when mac80211 requested FIF_CONTROL, you are now always
filtering out ACK frames no matter what mac80211 requested.

>  	/*
>  	 * Replace the skb with the freshly allocated one.
> @@ -974,6 +1032,11 @@ static int rt2x00lib_initialize(struct rt2x00_d=
ev *rt2x00dev)
>  		return status;
> =20
>  	/*
> +	 * Initialize queue for messages waiting for acks
> +	 */
> +	skb_queue_head_init(&rt2x00dev->ack_wait_queue);
> +
> +	/*
>  	 * Initialize the device.
>  	 */
>  	status =3D rt2x00dev->ops->lib->initialize(rt2x00dev);
> @@ -1107,6 +1170,14 @@ exit:
>  }
>  EXPORT_SYMBOL_GPL(rt2x00lib_probe_dev);
> =20
> +static void rt2x00lib_free_ack_wait_queue(struct rt2x00_dev *rt2x00d=
ev)
> +{
> +	struct sk_buff_head *ack_wait_queue =3D &rt2x00dev->ack_wait_queue;
> +	struct sk_buff *skb;
> +	while ((skb =3D skb_dequeue(ack_wait_queue)))
> +		dev_kfree_skb_any(skb);
> +}
> +
>  void rt2x00lib_remove_dev(struct rt2x00_dev *rt2x00dev)
>  {
>  	clear_bit(DEVICE_STATE_PRESENT, &rt2x00dev->flags);
> @@ -1142,6 +1213,11 @@ void rt2x00lib_remove_dev(struct rt2x00_dev *r=
t2x00dev)
>  	 * Free queue structures.
>  	 */
>  	rt2x00queue_free(rt2x00dev);
> +
> +	/*
> +	 * Free ack wait queue
> +	 */
> +	rt2x00lib_free_ack_wait_queue(rt2x00dev);

	skb_queue_purge(&rt2x00dev->ack_wait_queue) ?

>  }
>  EXPORT_SYMBOL_GPL(rt2x00lib_remove_dev);
> =20
> diff --git a/drivers/net/wireless/rt2x00/rt73usb.c b/drivers/net/wire=
less/rt2x00/rt73usb.c
> index 27dde3e..e082d30 100644
> --- a/drivers/net/wireless/rt2x00/rt73usb.c
> +++ b/drivers/net/wireless/rt2x00/rt73usb.c
> @@ -593,8 +593,7 @@ static void rt73usb_config_filter(struct rt2x00_d=
ev *rt2x00dev,
>  			   !(filter_flags & FIF_FCSFAIL));
>  	rt2x00_set_field32(&reg, TXRX_CSR0_DROP_PHYSICAL,
>  			   !(filter_flags & FIF_PLCPFAIL));
> -	rt2x00_set_field32(&reg, TXRX_CSR0_DROP_CONTROL,
> -			   !(filter_flags & FIF_CONTROL));
> +	rt2x00_set_field32(&reg, TXRX_CSR0_DROP_CONTROL, 0);
>  	rt2x00_set_field32(&reg, TXRX_CSR0_DROP_NOT_TO_ME,
>  			   !(filter_flags & FIF_PROMISC_IN_BSS));
>  	rt2x00_set_field32(&reg, TXRX_CSR0_DROP_TO_DS,
> @@ -604,8 +603,7 @@ static void rt73usb_config_filter(struct rt2x00_d=
ev *rt2x00dev,
>  	rt2x00_set_field32(&reg, TXRX_CSR0_DROP_MULTICAST,
>  			   !(filter_flags & FIF_ALLMULTI));
>  	rt2x00_set_field32(&reg, TXRX_CSR0_DROP_BROADCAST, 0);
> -	rt2x00_set_field32(&reg, TXRX_CSR0_DROP_ACK_CTS,
> -			   !(filter_flags & FIF_CONTROL));
> +	rt2x00_set_field32(&reg, TXRX_CSR0_DROP_ACK_CTS, 0);
>  	rt73usb_register_write(rt2x00dev, TXRX_CSR0, reg);
>  }
> =20


--
To unsubscribe from this list: send the line "unsubscribe linux-wireles=
s" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC][PATCH] TX status reporting with help of an ack queue

[Mattias Nissler]

[ I've added Johannes to the CC, I hope he can provide us with some mor=
e
informed comments regarding mac80211 architecture ]

On Mon, 2008-09-15 at 19:11 +0200, Ivo van Doorn wrote:
> On Monday 15 September 2008, Mikko Virkkil=E4 wrote:
> > Modeled after the waiting-for-ack queue in the zd1211rw
> > driver, this patch adds a similar queue to the rt2x00. If
> > the tx state of a packet is unknown and a tx ack or
> > tx fail is needed, then the packet is pushed to a
> > waiting-for-ack queue. Each time an ack is received the
> > queue is checked and a possible match is reported to
> > mac80211 as a tx ack.
> >=20
> > If the queue gets filled up the oldest waiting packet
> > will be discarded and a tx fail will be reported.
>=20
> I believe it was mentioned it earlier as well, but this code belongs
> to mac80211 rather than being copied in all individual drivers.
>=20
> I think the best solution would be:
>  - Drivers set *unknown TX status* flag for mac80211
>  - Mac80211 sets the FIF_CONTROL flag for the RX filter (or adds a ne=
w
>    more specific flag FIF_ACK for hardware that allows this specific =
filtering (rt2800))
>  - Driver will set a *unknown TX status* flag per send out frame
>  - Mac80211 keeps track of ACK responses, and decides what to do when
>    no ACK has been received for the frame.

I've also spent some time thinking about this. IMHO, he difficult part
is to come up with an interface between mac80211 and the driver which i=
s
convenient to use for driver writers and integrates well with mac80211.
The following ideas came to my mind:

1. Add another callback to struct ieee80211_ops that would be used for
TX frame housekeeping. The standard implementation would just pass the
frame on the regular tx handler, but we could provide an alternate
implementation that keeps track of the packets we are expecting ACKs fo=
r
before handing the packet to the hardware. Similarly, the RX path would
provide new functions in addition to the existing ieee80211_rx[_irqsafe=
]
handlers that would do the matching ACK matching and pass the packet on
to ieee80211_rx() afterwards.

2. Make the ACK queue approach generic and wrap it in some library
functions that become part of mac80211 and are called from the driver's
tx and rx handlers, respectively. This has the advantage that it is les=
s
invasive than 1. It is essentially what Ivo has described above. Driver
writers would be free to use this framework, but are not required to do
so.

3. Make mac80211 unconditionally keep track about the packets it has
passed down to the driver and do TX status matching on all ACKs that ar=
e
received in the mac80211 rx path. (Note that this has the tendency of
duplicating the ring buffers that most drivers probably already have fo=
r
their TX queues. Maybe introduce a generic ring buffer framework and
allow the drivers to store private data in data and that?) The advantag=
e
of this approach is that mac80211 has information about all packets tha=
t
are still in processing (don't know if that actually helps) and it's
more convenient for driver writers.

>=20
> Note that rt61 might be tricky since that does support TX status repo=
rting,
> but it fails in only 1% of the cases to do this, which means enabling=
 this for rt61
> would be far too much overhead.

I'd opt for the overhead. We have seen that the hardware/firmware
doesn't behave as expected, so I'd rather see a clean approach in
software.

>=20
> > This doesn't use timers so a message might in theory be
> > stuck in the queue indefinitely.
>=20
> Well you wouldn't need timers actually, if you have a list of packets
> waiting to be acked you can expect the frames to be acked in the orde=
r
> of which they were send (when using the same queue).

Agreed.

Mattias

--
To unsubscribe from this list: send the line "unsubscribe linux-wireles=
s" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC][PATCH] TX status reporting with help of an ack queue

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 3321 bytes --]

On Mon, 2008-09-15 at 19:56 +0200, Mattias Nissler wrote:
> [ I've added Johannes to the CC, I hope he can provide us with some more
> informed comments regarding mac80211 architecture ]

I'd actually written half a reply but then decided to give up on it for
now, what do you want to know?

we currently use status reporting for three things:
 * hostapd needs to know whether some frames were acked
 * the rate control algorithms use it
 * we only report sent frames on monitor interfaces when we get a status
   report and incorporate information from the status report


> I've also spent some time thinking about this. IMHO, he difficult part
> is to come up with an interface between mac80211 and the driver which is
> convenient to use for driver writers and integrates well with mac80211.
> The following ideas came to my mind:
> 
> 1. Add another callback to struct ieee80211_ops that would be used for
> TX frame housekeeping. The standard implementation would just pass the
> frame on the regular tx handler, but we could provide an alternate
> implementation that keeps track of the packets we are expecting ACKs for
> before handing the packet to the hardware. Similarly, the RX path would
> provide new functions in addition to the existing ieee80211_rx[_irqsafe]
> handlers that would do the matching ACK matching and pass the packet on
> to ieee80211_rx() afterwards.

That seems way too much overhead, but we definitely want to use the
IEEE80211_TX_CTL_REQ_TX_STATUS less in the future (we currently always
set it). I suppose by "expect ACKs for" you mean "expect status
information for"?

> 2. Make the ACK queue approach generic and wrap it in some library
> functions that become part of mac80211 and are called from the driver's
> tx and rx handlers, respectively. This has the advantage that it is less
> invasive than 1. It is essentially what Ivo has described above. Driver
> writers would be free to use this framework, but are not required to do
> so.

I think this is what we want.

> 3. Make mac80211 unconditionally keep track about the packets it has
> passed down to the driver and do TX status matching on all ACKs that are
> received in the mac80211 rx path. (Note that this has the tendency of
> duplicating the ring buffers that most drivers probably already have for
> their TX queues. Maybe introduce a generic ring buffer framework and
> allow the drivers to store private data in data and that?) The advantage
> of this approach is that mac80211 has information about all packets that
> are still in processing (don't know if that actually helps) and it's
> more convenient for driver writers.

I think this punishes hardware that can provide all information
needlessly imho. Also, with drivers that do provide the information we
don't need to process control packets at all which is good (if not
necessary) for drivers on slow busses.

> > > This doesn't use timers so a message might in theory be
> > > stuck in the queue indefinitely.
> > 
> > Well you wouldn't need timers actually, if you have a list of packets
> > waiting to be acked you can expect the frames to be acked in the order
> > of which they were send (when using the same queue).

Yes, hence you have to keep track of things per queue.

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [RFC][PATCH] TX status reporting with help of an ack queue

[Mattias Nissler]

On Mon, 2008-09-15 at 20:03 +0200, Johannes Berg wrote:
> > 2. Make the ACK queue approach generic and wrap it in some library
> > functions that become part of mac80211 and are called from the driver's
> > tx and rx handlers, respectively. This has the advantage that it is less
> > invasive than 1. It is essentially what Ivo has described above. Driver
> > writers would be free to use this framework, but are not required to do
> > so.
> 
> I think this is what we want.

Ok. If we take this path I think we do not need to add more flags to
mac80211 and the drivers that enable special processing w.r.t. the
device packet filters (as suggested by Ivo previously). Instead, the
driver itself should take whatever action is necessary to tell the
hardware to deliver ACK frames. Johannes, could you recap for me
whether the driver should report these additional ACKs to mac80211 or
keep quiet if they weren't requested?

> > > > This doesn't use timers so a message might in theory be
> > > > stuck in the queue indefinitely.
> > > 
> > > Well you wouldn't need timers actually, if you have a list of packets
> > > waiting to be acked you can expect the frames to be acked in the order
> > > of which they were send (when using the same queue).
> 
> Yes, hence you have to keep track of things per queue.

Good point :-) So I suggest we have one function that adds new packets
to the ACK queue (which is represented by a struct an instance of which
a driver can keep in its queue information) and another one that matches
a received ACK with the queue, reports the status to mac80211 and purges
the queue entries up to the match (reporting them as failed).

Mikko: I could hack this up, but since your initial patch is not far
from what's been discussed, I rather wait for a comment on whether you
want to do it.

Mattias

Re: [RFC][PATCH] TX status reporting with help of an ack queue

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 1717 bytes --]

On Mon, 2008-09-15 at 21:00 +0200, Mattias Nissler wrote:

> Ok. If we take this path I think we do not need to add more flags to
> mac80211 and the drivers that enable special processing w.r.t. the
> device packet filters (as suggested by Ivo previously). Instead, the
> driver itself should take whatever action is necessary to tell the
> hardware to deliver ACK frames.

Yes, but we could also make the ack queue processing part of mac80211,
i.e. we stick the packet on the queue before ->tx() and add a few new
flags to the status callback which can then process the queue.

> Johannes, could you recap for me
> whether the driver should report these additional ACKs to mac80211 or
> keep quiet if they weren't requested?

This is largely theoretical right now as mac80211 is _always_ requesting
status reports via IEEE80211_TX_CTL_REQ_TX_STATUS. This is something I
can't talk much about, you helped designed the rate control algorithm
and I think you're in a much better position to evaluate how this should
work with respect to rate control.

Then again, I think I'm misunderstanding you, are you thinking whether
it should hand those frames to mac80211? It doesn't matter to mac80211,
to be honest.

> Good point :-) So I suggest we have one function that adds new packets
> to the ACK queue (which is represented by a struct an instance of which
> a driver can keep in its queue information) and another one that matches
> a received ACK with the queue, reports the status to mac80211 and purges
> the queue entries up to the match (reporting them as failed).

can we stop talking about "ACK queue"? It really is a "unknown status
queue" or something like that.

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [RFC][PATCH] TX status reporting with help of an ack queue

[Mattias Nissler]

On Mon, 2008-09-15 at 21:21 +0200, Johannes Berg wrote:
> On Mon, 2008-09-15 at 21:00 +0200, Mattias Nissler wrote:
> 
> > Ok. If we take this path I think we do not need to add more flags to
> > mac80211 and the drivers that enable special processing w.r.t. the
> > device packet filters (as suggested by Ivo previously). Instead, the
> > driver itself should take whatever action is necessary to tell the
> > hardware to deliver ACK frames.
> 
> Yes, but we could also make the ack queue processing part of mac80211,
> i.e. we stick the packet on the queue before ->tx() and add a few new
> flags to the status callback which can then process the queue.

Just to get this straight: By queue you mean the to-be-introduced
unknown status queue?

Of course you can do it that way, but then you'll have all the flags and
data structures built into mac80211 unconditionally instead of letting
the driver developer decide.

> 
> > Johannes, could you recap for me
> > whether the driver should report these additional ACKs to mac80211 or
> > keep quiet if they weren't requested?
> 
> This is largely theoretical right now as mac80211 is _always_ requesting
> status reports via IEEE80211_TX_CTL_REQ_TX_STATUS. This is something I
> can't talk much about, you helped designed the rate control algorithm
> and I think you're in a much better position to evaluate how this should
> work with respect to rate control.
> 
> Then again, I think I'm misunderstanding you, are you thinking whether
> it should hand those frames to mac80211? It doesn't matter to mac80211,
> to be honest.

Ok, that's what I meant.

> 
> > Good point :-) So I suggest we have one function that adds new packets
> > to the ACK queue (which is represented by a struct an instance of which
> > a driver can keep in its queue information) and another one that matches
> > a received ACK with the queue, reports the status to mac80211 and purges
> > the queue entries up to the match (reporting them as failed).
> 
> can we stop talking about "ACK queue"? It really is a "unknown status
> queue" or something like that.

Ok, I don't really care how it's called as long as the one writing that
stuff chooses an appropriate name in the code :-)

Mattias

Re: [RFC][PATCH] TX status reporting with help of an ack queue

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 1515 bytes --]

On Mon, 2008-09-15 at 21:39 +0200, Mattias Nissler wrote:

> > Yes, but we could also make the ack queue processing part of mac80211,
> > i.e. we stick the packet on the queue before ->tx() and add a few new
> > flags to the status callback which can then process the queue.
> 
> Just to get this straight: By queue you mean the to-be-introduced
> unknown status queue?

Yes.

> Of course you can do it that way, but then you'll have all the flags and
> data structures built into mac80211 unconditionally instead of letting
> the driver developer decide.

Oh, I'm fine with building them in there as long as the code paths
aren't actually used for drivers that don't need them. This isn't a big
thing.

> > > Good point :-) So I suggest we have one function that adds new packets
> > > to the ACK queue (which is represented by a struct an instance of which
> > > a driver can keep in its queue information) and another one that matches
> > > a received ACK with the queue, reports the status to mac80211 and purges
> > > the queue entries up to the match (reporting them as failed).
> > 
> > can we stop talking about "ACK queue"? It really is a "unknown status
> > queue" or something like that.
> 
> Ok, I don't really care how it's called as long as the one writing that
> stuff chooses an appropriate name in the code :-)

:)
But ACK is getting confusing, we're just reporting status based on
reception (or non-reception!) of ACKs :) How about retries in the hw?

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [RFC][PATCH] TX status reporting with help of an ack queue

[Mattias Nissler]

On Mon, 2008-09-15 at 21:46 +0200, Johannes Berg wrote:
> On Mon, 2008-09-15 at 21:39 +0200, Mattias Nissler wrote:
> 
> > > Yes, but we could also make the ack queue processing part of mac80211,
> > > i.e. we stick the packet on the queue before ->tx() and add a few new
> > > flags to the status callback which can then process the queue.
> > 
> > Just to get this straight: By queue you mean the to-be-introduced
> > unknown status queue?
> 
> Yes.
> 
> > Of course you can do it that way, but then you'll have all the flags and
> > data structures built into mac80211 unconditionally instead of letting
> > the driver developer decide.
> 
> Oh, I'm fine with building them in there as long as the code paths
> aren't actually used for drivers that don't need them. This isn't a big
> thing.

Well, I'm a big fan of modularizing everything in a clean way. This
whole mac80211 thingy is complex enough... But I don't really care as
long as everybody here is happy with it. Let's wait what Mikko says,
it's his code so far.

> 
> > > > Good point :-) So I suggest we have one function that adds new packets
> > > > to the ACK queue (which is represented by a struct an instance of which
> > > > a driver can keep in its queue information) and another one that matches
> > > > a received ACK with the queue, reports the status to mac80211 and purges
> > > > the queue entries up to the match (reporting them as failed).
> > > 
> > > can we stop talking about "ACK queue"? It really is a "unknown status
> > > queue" or something like that.
> > 
> > Ok, I don't really care how it's called as long as the one writing that
> > stuff chooses an appropriate name in the code :-)
> 
> :)
> But ACK is getting confusing, we're just reporting status based on
> reception (or non-reception!) of ACKs :) How about retries in the hw?

Retries in the hw? I don't understand? You mean that as a name?

Mattias

Re: [RFC][PATCH] TX status reporting with help of an ack queue

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 882 bytes --]

On Mon, 2008-09-15 at 22:20 +0200, Mattias Nissler wrote:

> Well, I'm a big fan of modularizing everything in a clean way. This
> whole mac80211 thingy is complex enough... But I don't really care as
> long as everybody here is happy with it. Let's wait what Mikko says,
> it's his code so far.

Sure. If you manage to split it out entirely, maybe by some struct that
drivers embed in their private vif struct, that'd be great too.


> > But ACK is getting confusing, we're just reporting status based on
> > reception (or non-reception!) of ACKs :) How about retries in the hw?
> 
> Retries in the hw? I don't understand? You mean that as a name?

Well, with all this we won't know how many times the hardware attempted
to send a frame before it got an ACK, if it got one at all. We'd like to
know this too, I guess, for the rate control algorithm.

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [RFC][PATCH] TX status reporting with help of an ack queue

[Mattias Nissler]

On Mon, 2008-09-15 at 22:33 +0200, Johannes Berg wrote:
> On Mon, 2008-09-15 at 22:20 +0200, Mattias Nissler wrote:
> 
> > Well, I'm a big fan of modularizing everything in a clean way. This
> > whole mac80211 thingy is complex enough... But I don't really care as
> > long as everybody here is happy with it. Let's wait what Mikko says,
> > it's his code so far.
> 
> Sure. If you manage to split it out entirely, maybe by some struct that
> drivers embed in their private vif struct, that'd be great too.
> 
> 
> > > But ACK is getting confusing, we're just reporting status based on
> > > reception (or non-reception!) of ACKs :) How about retries in the hw?
> > 
> > Retries in the hw? I don't understand? You mean that as a name?
> 
> Well, with all this we won't know how many times the hardware attempted
> to send a frame before it got an ACK, if it got one at all. We'd like to
> know this too, I guess, for the rate control algorithm.

That's right, the rate control algorithm might want to know. However,
the PID algorithm doesn't care too much about the number of retries, it
just classifies frames into the categories: successfull, single retry,
multiple retries (IIRC, that is).

Btw. lately I've been thinking about a new RC algo that simply
calculates the throughput achievable by a specific rate (based on the
estimated frame transmission failure ratio and transmission times) and
then selects the one that provides the best throughput. It's still a
very vague idea, but I think it might be interesting to try.

Mattias

Re: [RFC][PATCH] TX status reporting with help of an ack queue

[John W. Linville]

On Mon, Sep 15, 2008 at 11:01:27PM +0200, Mattias Nissler wrote:

> Btw. lately I've been thinking about a new RC algo that simply
> calculates the throughput achievable by a specific rate (based on the
> estimated frame transmission failure ratio and transmission times) and
> then selects the one that provides the best throughput. It's still a
> very vague idea, but I think it might be interesting to try.

This sounds like a reasonable approach.  Isn't this more-or-less what
Minstrel does?

John
-- 
John W. Linville		Linux should be at the core
linville@tuxdriver.com			of your literate lifestyle.

Re: [RFC][PATCH] TX status reporting with help of an ack queue

[Mattias Nissler]

On Tue, 2008-09-23 at 20:59 -0400, John W. Linville wrote:
> On Mon, Sep 15, 2008 at 11:01:27PM +0200, Mattias Nissler wrote:
> 
> > Btw. lately I've been thinking about a new RC algo that simply
> > calculates the throughput achievable by a specific rate (based on the
> > estimated frame transmission failure ratio and transmission times) and
> > then selects the one that provides the best throughput. It's still a
> > very vague idea, but I think it might be interesting to try.
> 
> This sounds like a reasonable approach.  Isn't this more-or-less what
> Minstrel does?

I just checked Minstrel out. Seems quite like what I had in mind :-) Are
there any efforts to port minstrel to mac80211?

Mattias

Re: [RFC][PATCH] TX status reporting with help of an ack queue

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 953 bytes --]

On Wed, 2008-09-24 at 09:40 +0200, Mattias Nissler wrote:
> On Tue, 2008-09-23 at 20:59 -0400, John W. Linville wrote:
> > On Mon, Sep 15, 2008 at 11:01:27PM +0200, Mattias Nissler wrote:
> > 
> > > Btw. lately I've been thinking about a new RC algo that simply
> > > calculates the throughput achievable by a specific rate (based on the
> > > estimated frame transmission failure ratio and transmission times) and
> > > then selects the one that provides the best throughput. It's still a
> > > very vague idea, but I think it might be interesting to try.
> > 
> > This sounds like a reasonable approach.  Isn't this more-or-less what
> > Minstrel does?
> 
> I just checked Minstrel out. Seems quite like what I had in mind :-) Are
> there any efforts to port minstrel to mac80211?

nbd has been promising for ages, but has not had time so far. The full
minstrel will also require API changes between drivers/mac80211/RC.

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [RFC][PATCH] TX status reporting with help of an ack queue

[Ivo van Doorn]

On Monday 15 September 2008, Johannes Berg wrote:
> On Mon, 2008-09-15 at 22:20 +0200, Mattias Nissler wrote:
> 
> > Well, I'm a big fan of modularizing everything in a clean way. This
> > whole mac80211 thingy is complex enough... But I don't really care as
> > long as everybody here is happy with it. Let's wait what Mikko says,
> > it's his code so far.
> 
> Sure. If you manage to split it out entirely, maybe by some struct that
> drivers embed in their private vif struct, that'd be great too.

I think the ACK handling could become quite complex, and although it
would be nice to modularize it a bit, however I am not really sure about
what the best approach would be for the implementation other then that
the driver should do as little as possible. ;)

Perhaps Mikko or the zd1211rw developers will have better ideas on this subject. :)

> > > But ACK is getting confusing, we're just reporting status based on
> > > reception (or non-reception!) of ACKs :) How about retries in the hw?
> > 
> > Retries in the hw? I don't understand? You mean that as a name?
> 
> Well, with all this we won't know how many times the hardware attempted
> to send a frame before it got an ACK, if it got one at all. We'd like to
> know this too, I guess, for the rate control algorithm.

Well this information would definately get lost with this ACK handling,
but at least we get *some* information about the TX status. ;)

Ivo

Re: [RFC][PATCH] TX status reporting with help of an ack queue

[Mattias Nissler]

On Mon, 2008-09-15 at 23:28 +0200, Ivo van Doorn wrote:
> On Monday 15 September 2008, Johannes Berg wrote:
> > On Mon, 2008-09-15 at 22:20 +0200, Mattias Nissler wrote:
> > 
> > > Well, I'm a big fan of modularizing everything in a clean way. This
> > > whole mac80211 thingy is complex enough... But I don't really care as
> > > long as everybody here is happy with it. Let's wait what Mikko says,
> > > it's his code so far.
> > 
> > Sure. If you manage to split it out entirely, maybe by some struct that
> > drivers embed in their private vif struct, that'd be great too.
> 
> I think the ACK handling could become quite complex, and although it
> would be nice to modularize it a bit, however I am not really sure about
> what the best approach would be for the implementation other then that
> the driver should do as little as possible. ;)

Huh? I think a single function call for the matching in the rx path is
enough. You call it in the rx handler for every received frame. It
returns true if it found a match and reported the tx status (in which
case you stop processing) or false and you can go on doing with the
frame whatever you want. Am I missing something?

> 
> Perhaps Mikko or the zd1211rw developers will have better ideas on this subject. :)
> 
> > > > But ACK is getting confusing, we're just reporting status based on
> > > > reception (or non-reception!) of ACKs :) How about retries in the hw?
> > > 
> > > Retries in the hw? I don't understand? You mean that as a name?
> > 
> > Well, with all this we won't know how many times the hardware attempted
> > to send a frame before it got an ACK, if it got one at all. We'd like to
> > know this too, I guess, for the rate control algorithm.
> 
> Well this information would definately get lost with this ACK handling,
> but at least we get *some* information about the TX status. ;)

The (not) failed decision is definitely enough to get the PID algo doing
something useful.

Mattias

Re: [RFC][PATCH] TX status reporting with help of an ack queue

[Ivo van Doorn]

On Monday 15 September 2008, Mattias Nissler wrote:
> On Mon, 2008-09-15 at 23:28 +0200, Ivo van Doorn wrote:
> > On Monday 15 September 2008, Johannes Berg wrote:
> > > On Mon, 2008-09-15 at 22:20 +0200, Mattias Nissler wrote:
> > > 
> > > > Well, I'm a big fan of modularizing everything in a clean way. This
> > > > whole mac80211 thingy is complex enough... But I don't really care as
> > > > long as everybody here is happy with it. Let's wait what Mikko says,
> > > > it's his code so far.
> > > 
> > > Sure. If you manage to split it out entirely, maybe by some struct that
> > > drivers embed in their private vif struct, that'd be great too.
> > 
> > I think the ACK handling could become quite complex, and although it
> > would be nice to modularize it a bit, however I am not really sure about
> > what the best approach would be for the implementation other then that
> > the driver should do as little as possible. ;)
> 
> Huh? I think a single function call for the matching in the rx path is
> enough. You call it in the rx handler for every received frame. It
> returns true if it found a match and reported the tx status (in which
> case you stop processing) or false and you can go on doing with the
> frame whatever you want. Am I missing something?

Although this isn't much overhead for a driver, but if it comes down
to a single function call anyway, why not handle it in mac80211 completely?

Ivo

Re: [RFC][PATCH] TX status reporting with help of an ack queue

[Mattias Nissler]

On Tue, 2008-09-16 at 20:17 +0200, Ivo van Doorn wrote:
> On Monday 15 September 2008, Mattias Nissler wrote:
> > On Mon, 2008-09-15 at 23:28 +0200, Ivo van Doorn wrote:
> > > On Monday 15 September 2008, Johannes Berg wrote:
> > > > On Mon, 2008-09-15 at 22:20 +0200, Mattias Nissler wrote:
> > > > 
> > > > > Well, I'm a big fan of modularizing everything in a clean way. This
> > > > > whole mac80211 thingy is complex enough... But I don't really care as
> > > > > long as everybody here is happy with it. Let's wait what Mikko says,
> > > > > it's his code so far.
> > > > 
> > > > Sure. If you manage to split it out entirely, maybe by some struct that
> > > > drivers embed in their private vif struct, that'd be great too.
> > > 
> > > I think the ACK handling could become quite complex, and although it
> > > would be nice to modularize it a bit, however I am not really sure about
> > > what the best approach would be for the implementation other then that
> > > the driver should do as little as possible. ;)
> > 
> > Huh? I think a single function call for the matching in the rx path is
> > enough. You call it in the rx handler for every received frame. It
> > returns true if it found a match and reported the tx status (in which
> > case you stop processing) or false and you can go on doing with the
> > frame whatever you want. Am I missing something?
> 
> Although this isn't much overhead for a driver, but if it comes down
> to a single function call anyway, why not handle it in mac80211 completely?

Simply because most drivers don't need it and it adds another item to
mac80211's bag of complex stuff to understand ;-)

Mattias

Re: [RFC][PATCH] TX status reporting with help of an ack queue

[Mikko Virkkilä]

On Mon, 2008-09-15 at 22:33 +0200, Johannes Berg wrote:
> On Mon, 2008-09-15 at 22:20 +0200, Mattias Nissler wrote:
> 
> > Well, I'm a big fan of modularizing everything in a clean way. This
> > whole mac80211 thingy is complex enough... But I don't really care as
> > long as everybody here is happy with it. Let's wait what Mikko says,
> > it's his code so far.
> 
> Sure. If you manage to split it out entirely, maybe by some struct that
> drivers embed in their private vif struct, that'd be great too.
> 
> 
> > > But ACK is getting confusing, we're just reporting status based on
> > > reception (or non-reception!) of ACKs :) How about retries in the hw?
> > 
> > Retries in the hw? I don't understand? You mean that as a name?
> 
> Well, with all this we won't know how many times the hardware attempted
> to send a frame before it got an ACK, if it got one at all. We'd like to
> know this too, I guess, for the rate control algorithm.
> 
> johannes

My suggestion would be something along the lines of
awaiting_status_queue because the packets are waiting for status info
about their transmission.

To get the AP mode working (my main interest) hostapd needs to know that
the other party has received the packet so that it can advance in its
state machine without losing sync. 

The rate control algorithms on the other hand are probably interested in
getting a lot more details. Currently drivers seem to keep internal
statistics in addition to the external ones. Maybe some way of exporting
additional statistics is needed and tackling that should perhaps not be
tied to the status reporting? Rate control algorithms will probably also
want to modify re-transmission intervals, contention time settings etc.
Is there already some unified way of doing this?


Mikko

Re: [RFC][PATCH] TX status reporting with help of an ack queue

[Mikko Virkkilä]

On Mon, 2008-09-15 at 21:00 +0200, Mattias Nissler wrote:
> On Mon, 2008-09-15 at 20:03 +0200, Johannes Berg wrote:
> > > 2. Make the ACK queue approach generic and wrap it in some library
> > > functions that become part of mac80211 and are called from the driver's
> > > tx and rx handlers, respectively. This has the advantage that it is less
> > > invasive than 1. It is essentially what Ivo has described above. Driver
> > > writers would be free to use this framework, but are not required to do
> > > so.
> > 
> > I think this is what we want.
I also like this approach. It gives us the flexibility to add driver
specific code to the driver while keeping the mac80211 part clean even
if some driver needs additional packet handling hacks.

> Ok. If we take this path I think we do not need to add more flags to
> mac80211 and the drivers that enable special processing w.r.t. the
> device packet filters (as suggested by Ivo previously). Instead, the
> driver itself should take whatever action is necessary to tell the
> hardware to deliver ACK frames. Johannes, could you recap for me
> whether the driver should report these additional ACKs to mac80211 or
> keep quiet if they weren't requested?
> 
> > > > > This doesn't use timers so a message might in theory be
> > > > > stuck in the queue indefinitely.
> > > > 
> > > > Well you wouldn't need timers actually, if you have a list of packets
> > > > waiting to be acked you can expect the frames to be acked in the order
> > > > of which they were send (when using the same queue).
> > 
> > Yes, hence you have to keep track of things per queue.
> 
> Good point :-) So I suggest we have one function that adds new packets
> to the ACK queue (which is represented by a struct an instance of which
> a driver can keep in its queue information) and another one that matches
> a received ACK with the queue, reports the status to mac80211 and purges
> the queue entries up to the match (reporting them as failed).
> 
> Mikko: I could hack this up, but since your initial patch is not far
> from what's been discussed, I rather wait for a comment on whether you
> want to do it.
> 
> Mattias
> 

Feel free to hack up a patch. The functions 
rt2x00lib_get_match_from_ack_queue and rt2x00lib_push_to_ack_queue are
already generic handlers for the queue. 

>From the messages on this list it seems people want to also add the part
of sending the packet to the mac80211 layer to these two functions. This
is certainly possible but it needs to be done in such a way that the
driver can keep its internal statistics up to date. Since I'm not at all
familiar with what information a driver might want to extract, I opted
to leave the packet forwarding out of the generic functions.

Mikko

Re: [RFC][PATCH] TX status reporting with help of an ack queue

[Ivo van Doorn]

> > Good point :-) So I suggest we have one function that adds new packets
> > to the ACK queue (which is represented by a struct an instance of which
> > a driver can keep in its queue information) and another one that matches
> > a received ACK with the queue, reports the status to mac80211 and purges
> > the queue entries up to the match (reporting them as failed).
> > 
> > Mikko: I could hack this up, but since your initial patch is not far
> > from what's been discussed, I rather wait for a comment on whether you
> > want to do it.
> > 
> > Mattias
> > 
> 
> Feel free to hack up a patch. The functions 
> rt2x00lib_get_match_from_ack_queue and rt2x00lib_push_to_ack_queue are
> already generic handlers for the queue. 

Mattias, please note my comments on Mikko's patch if you are going to copy them. :)

Ivo

Re: [RFC][PATCH] TX status reporting with help of an ack queue

[Mattias Nissler]

On Tue, 2008-09-16 at 20:18 +0200, Ivo van Doorn wrote:
> > > Good point :-) So I suggest we have one function that adds new packets
> > > to the ACK queue (which is represented by a struct an instance of which
> > > a driver can keep in its queue information) and another one that matches
> > > a received ACK with the queue, reports the status to mac80211 and purges
> > > the queue entries up to the match (reporting them as failed).
> > > 
> > > Mikko: I could hack this up, but since your initial patch is not far
> > > from what's been discussed, I rather wait for a comment on whether you
> > > want to do it.
> > > 
> > > Mattias
> > > 
> > 
> > Feel free to hack up a patch. The functions 
> > rt2x00lib_get_match_from_ack_queue and rt2x00lib_push_to_ack_queue are
> > already generic handlers for the queue. 
> 
> Mattias, please note my comments on Mikko's patch if you are going to copy them. :)

Ok, I'm picking this thingy up. I've got some time tomorrow, but I'll be
away for the weekend, so expect something at the beginning of next week.

Mattias

ACK matching [was: TX status reporting with help of an ack queue]

[Mattias Nissler]

[I've added the zd1211rw maintainers to the CC, I hope they can shed
some light on the question whether the ack queue mechanism that driver
has is actually correct]

I've had a closer look at the idea of deciding whether a frame has been
transmitted succesfully by monitoring incoming ACK frames and I've hit a
fundamental problem: How do you correlate incoming ACK frames to the
frames that were actually transmitted? The ACK frame only carries the
address of the station that transmitted the frame being acked, no
further information. Now this means if you have the hardware TX two
frames and receive one ACK frame in the RX path, you cannot know which
TX frame the ACK belongs to, because they will be identical, right?

The hardware, however, knows, because the ACKs are required to be
transmitted directly after the corresponding frame is received, but we
don't know in the driver about this timing information, at least not for
rt2x00.

I wonder whether the ack queue idea Mikko found in the zd1211rw driver
is actually working correctly for that driver? I've only had a short
look, but found that incoming ACKs are only matched against transmitted
frames by means of the address carried within the ACK. So I'd think in
the situation I outlined above, the zd1211rw driver will also be unable
to match the ACK to the correct frame. Any comments on this?

Mattiass

Re: ACK matching [was: TX status reporting with help of an ack queue]

[Mikko Virkkilä]

On Thu, 2008-09-18 at 22:29 +0000, Mattias Nissler wrote:
> [I've added the zd1211rw maintainers to the CC, I hope they can shed
> some light on the question whether the ack queue mechanism that driver
> has is actually correct]
> 
> I've had a closer look at the idea of deciding whether a frame has
> been
> transmitted succesfully by monitoring incoming ACK frames and I've hit
> a
> fundamental problem: How do you correlate incoming ACK frames to the
> frames that were actually transmitted? The ACK frame only carries the
> address of the station that transmitted the frame being acked, no
> further information. Now this means if you have the hardware TX two
> frames and receive one ACK frame in the RX path, you cannot know which
> TX frame the ACK belongs to, because they will be identical, right?
> 
> The hardware, however, knows, because the ACKs are required to be
> transmitted directly after the corresponding frame is received, but we
> don't know in the driver about this timing information, at least not for
> rt2x00.
> 
> I wonder whether the ack queue idea Mikko found in the zd1211rw driver
> is actually working correctly for that driver? I've only had a short
> look, but found that incoming ACKs are only matched against transmitted
> frames by means of the address carried within the ACK. So I'd think in
> the situation I outlined above, the zd1211rw driver will also be unable
> to match the ACK to the correct frame. Any comments on this?
> 
> Mattiass
> 

AFAIK your analysis is correct and I think this is somewhat of a known
problem with the implementation in zd1211rw driver. I haven't come
across a way of getting around the problem. I read some suggestions
about stopping all transmission to a certain address until we get a
status update for the previous packet sent to that address, but iirc
this was deemed impractical. On the other hand the current
implementation in zd1211rw (and hopefully soon in rt2x00) doesn't really
break anything that works without it and on rt2x00 it gets AP-mode in a
somewhat more usable state. 

- Mikko

Re: ACK matching [was: TX status reporting with help of an ack queue]

[Mattias Nissler]

On Fri, 2008-09-19 at 12:08 +0300, Mikko Virkkil=E4 wrote:
> On Thu, 2008-09-18 at 22:29 +0000, Mattias Nissler wrote:
> > [I've added the zd1211rw maintainers to the CC, I hope they can she=
d
> > some light on the question whether the ack queue mechanism that dri=
ver
> > has is actually correct]
> >=20
> > I've had a closer look at the idea of deciding whether a frame has
> > been
> > transmitted succesfully by monitoring incoming ACK frames and I've =
hit
> > a
> > fundamental problem: How do you correlate incoming ACK frames to th=
e
> > frames that were actually transmitted? The ACK frame only carries t=
he
> > address of the station that transmitted the frame being acked, no
> > further information. Now this means if you have the hardware TX two
> > frames and receive one ACK frame in the RX path, you cannot know wh=
ich
> > TX frame the ACK belongs to, because they will be identical, right?
> >=20
> > The hardware, however, knows, because the ACKs are required to be
> > transmitted directly after the corresponding frame is received, but=
 we
> > don't know in the driver about this timing information, at least no=
t for
> > rt2x00.
> >=20
> > I wonder whether the ack queue idea Mikko found in the zd1211rw dri=
ver
> > is actually working correctly for that driver? I've only had a shor=
t
> > look, but found that incoming ACKs are only matched against transmi=
tted
> > frames by means of the address carried within the ACK. So I'd think=
 in
> > the situation I outlined above, the zd1211rw driver will also be un=
able
> > to match the ACK to the correct frame. Any comments on this?
> >=20
> > Mattiass
> >=20
>=20
> AFAIK your analysis is correct and I think this is somewhat of a know=
n
> problem with the implementation in zd1211rw driver. I haven't come
> across a way of getting around the problem. I read some suggestions
> about stopping all transmission to a certain address until we get a
> status update for the previous packet sent to that address, but iirc
> this was deemed impractical. On the other hand the current
> implementation in zd1211rw (and hopefully soon in rt2x00) doesn't rea=
lly
> break anything that works without it and on rt2x00 it gets AP-mode in=
 a
> somewhat more usable state.=20

Well, as long as nothing vitally depends on the transmissions status
(i.e. MAC layer acknowledgements in 802.11 lingo), you are right that i=
t
won't break anything. Nevertheless, it is code that is known to be
working incorrectly and non-fixable, so I'd vote against it (I'd rather
live with the approach of always reporting successful tx status as you
have proposed earlier if I had to choose from the two of them). Maybe w=
e
just have to accept that there is hardware that cannot report MAC layer
acknowledgements back to the driver. Unless somebody comes up with a
feasible idea how to do it. Ivo, do we have a contact at Ralink to ask
about this?

This brings up the question whether we can do without tx status
reporting. Does anyone know why hostapd requires the tx status? As far
as I understand, mac80211 only uses the tx status reporting only for th=
e
tx rate control. Rate control algos that don't use tx status are
definitely feasible (and in fact we'll need one for rt73).

I'll look into hostapd to figure out whether the tx status reporting is
really required when I find some time. However, I've just received
rt2800 hardware, so getting the rt2800 driver going is my top priority
for the next weeks.

Mattias

--
To unsubscribe from this list: send the line "unsubscribe linux-wireles=
s" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: ACK matching [was: TX status reporting with help of an ack queue]

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 872 bytes --]

On Fri, 2008-09-19 at 11:46 +0200, Mattias Nissler wrote:

> This brings up the question whether we can do without tx status
> reporting. Does anyone know why hostapd requires the tx status? As far
> as I understand, mac80211 only uses the tx status reporting only for the
> tx rate control. Rate control algos that don't use tx status are
> definitely feasible (and in fact we'll need one for rt73).

mac80211 also uses it for reporting the sent frame to userspace on
monitor interfaces.

> I'll look into hostapd to figure out whether the tx status reporting is
> really required when I find some time.

hostapd requires this because it needs to know whether a station
acknowledged a frame or not to proceed its state machine, it's just how
it has to work. If the hardware can't deal with that I guess you can't
implement AP mode properly.

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: ACK matching [was: TX status reporting with help of an ack queue]

[Mattias Nissler]

On Fri, 2008-09-19 at 11:54 +0200, Johannes Berg wrote:
> On Fri, 2008-09-19 at 11:46 +0200, Mattias Nissler wrote:
> 
> > This brings up the question whether we can do without tx status
> > reporting. Does anyone know why hostapd requires the tx status? As far
> > as I understand, mac80211 only uses the tx status reporting only for the
> > tx rate control. Rate control algos that don't use tx status are
> > definitely feasible (and in fact we'll need one for rt73).
> 
> mac80211 also uses it for reporting the sent frame to userspace on
> monitor interfaces.

Yeah, that's what hostapd relies on. But mac80211 really does not care
to much how userspace uses the information.

> 
> > I'll look into hostapd to figure out whether the tx status reporting is
> > really required when I find some time.
> 
> hostapd requires this because it needs to know whether a station
> acknowledged a frame or not to proceed its state machine, it's just how
> it has to work. If the hardware can't deal with that I guess you can't
> implement AP mode properly.

Well, maybe we can work around this requirement? I still need to learn
about the details, but what happens for example if the STA sends the ACK
and then resets due to a crash? I guess the AP is able to cope with
that, no? So maybe we can relax the rules a bit (unless we become really
incompliant with the standard of course).

Mattias

Re: ACK matching [was: TX status reporting with help of an ack queue]

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 984 bytes --]

On Fri, 2008-09-19 at 12:19 +0200, Mattias Nissler wrote:

> Well, maybe we can work around this requirement? I still need to learn
> about the details, but what happens for example if the STA sends the ACK
> and then resets due to a crash? I guess the AP is able to cope with
> that, no? So maybe we can relax the rules a bit (unless we become really
> incompliant with the standard of course).

I don't really see how to. We can't just assume the station got the
frame properly and advance our state machine. The case you're citing is
quite different, we'll advance the state machine but it won't work
because the STA crashed; if we advance but the station simply hasn't
gotten the frame we'll get out of sync and stuff will fail for no real
reason.

But you can do workarounds all you want in hostapd, I don't care.
However, I do think that if the hardware just isn't up to the job you
should probably buy new hardware, after all, it's dirt cheap. :)

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: ACK matching [was: TX status reporting with help of an ack queue]

[Mattias Nissler]

On Fri, 2008-09-19 at 12:31 +0200, Johannes Berg wrote:
> On Fri, 2008-09-19 at 12:19 +0200, Mattias Nissler wrote:
> 
> > Well, maybe we can work around this requirement? I still need to learn
> > about the details, but what happens for example if the STA sends the ACK
> > and then resets due to a crash? I guess the AP is able to cope with
> > that, no? So maybe we can relax the rules a bit (unless we become really
> > incompliant with the standard of course).
> 
> I don't really see how to. We can't just assume the station got the
> frame properly and advance our state machine. The case you're citing is
> quite different, we'll advance the state machine but it won't work
> because the STA crashed; if we advance but the station simply hasn't
> gotten the frame we'll get out of sync and stuff will fail for no real
> reason.

Well, I understand that we need synchronization of the state machines,
maybe we can advance optimistically and detect in the next state that
the STA didn't receive the last message? Just some random thoughts, I
see it'll at least be tricky, I just wanted your opinion on whether you
see a chance :-) As I said, I'll look into it when I find some time.

> 
> But you can do workarounds all you want in hostapd, I don't care.

:-)

> However, I do think that if the hardware just isn't up to the job you
> should probably buy new hardware, after all, it's dirt cheap. :)

I totally agree with you. I'm just curious to see whether there is a
chance to help our users. I'm perfectly fine if it turns out there is no
chance to do it properly, and I'll rather accept that fact instead of
trying to introduce workarounds that are known to be incorrect. The sad
thing is that if we controlled the firmware, we could probably arrange
to have the necessary information passed to the driver.

Mattias

Re: ACK matching [was: TX status reporting with help of an ack queue]

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 1506 bytes --]

On Fri, 2008-09-19 at 15:51 +0200, Mattias Nissler wrote:

> Well, I understand that we need synchronization of the state machines,
> maybe we can advance optimistically and detect in the next state that
> the STA didn't receive the last message? Just some random thoughts, I
> see it'll at least be tricky, I just wanted your opinion on whether you
> see a chance :-) As I said, I'll look into it when I find some time.

To be honest, I'm not familiar enough with it to be able to answer that
question. I know it's currently needed and I vaguely remember seeing and
discussing that it's pretty important (and the stuff I already wrote),
but that's about it.

> > However, I do think that if the hardware just isn't up to the job you
> > should probably buy new hardware, after all, it's dirt cheap. :)
> 
> I totally agree with you. I'm just curious to see whether there is a
> chance to help our users. I'm perfectly fine if it turns out there is no
> chance to do it properly, and I'll rather accept that fact instead of
> trying to introduce workarounds that are known to be incorrect. The sad
> thing is that if we controlled the firmware, we could probably arrange
> to have the necessary information passed to the driver.

Indeed; at least for those cards where there is firmware. But I guess
you always only get as much out of the hardware as the vendor wants you
to, rt61pci for example doesn't report frame timestamps so it's fairly
useless for monitoring.

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: ACK matching [was: TX status reporting with help of an ack queue]

[Mikko Virkkilä]

On Fri, 2008-09-19 at 15:51 +0200, Mattias Nissler wrote:
> On Fri, 2008-09-19 at 12:31 +0200, Johannes Berg wrote:
> > On Fri, 2008-09-19 at 12:19 +0200, Mattias Nissler wrote:
> > 
> > > Well, maybe we can work around this requirement? I still need to learn
> > > about the details, but what happens for example if the STA sends the ACK
> > > and then resets due to a crash? I guess the AP is able to cope with
> > > that, no? So maybe we can relax the rules a bit (unless we become really
> > > incompliant with the standard of course).
> > 
> > I don't really see how to. We can't just assume the station got the
> > frame properly and advance our state machine. The case you're citing is
> > quite different, we'll advance the state machine but it won't work
> > because the STA crashed; if we advance but the station simply hasn't
> > gotten the frame we'll get out of sync and stuff will fail for no real
> > reason.
> 
> Well, I understand that we need synchronization of the state machines,
> maybe we can advance optimistically and detect in the next state that
> the STA didn't receive the last message? Just some random thoughts, I
> see it'll at least be tricky, I just wanted your opinion on whether you
> see a chance :-) As I said, I'll look into it when I find some time.
> 

Just as a side note: it's the mac80211 stack which converts a missing
TX_ACK flag to be a IEEE80211_RADIOTAP_F_TX_FAIL flag. The hostapd sees
that TX_FAIL and assumes TX failed.

> > 
> > But you can do workarounds all you want in hostapd, I don't care.
> 
> :-)
> 
> > However, I do think that if the hardware just isn't up to the job you
> > should probably buy new hardware, after all, it's dirt cheap. :)
> 
> I totally agree with you. I'm just curious to see whether there is a
> chance to help our users. I'm perfectly fine if it turns out there is no
> chance to do it properly, and I'll rather accept that fact instead of
> trying to introduce workarounds that are known to be incorrect. The sad
> thing is that if we controlled the firmware, we could probably arrange
> to have the necessary information passed to the driver.
> 
> Mattias
> 

I'd really love for some workaround that would allow AP mode to work. 

It would be great if it could be deemed that the protocol doesn't really
require CTS protection/status information, and can be made reliable and
standards compliant without support for it from the driver.
Unfortunately I don't have high hopes for that, but I wonder what's Mr.
Malinen's opinion?

- Mikko

Re: ACK matching [was: TX status reporting with help of an ack queue]

[Jouni Malinen]

Mikko Virkkil=E4 wrote:
> On Fri, 2008-09-19 at 15:51 +0200, Mattias Nissler wrote:
>> On Fri, 2008-09-19 at 12:31 +0200, Johannes Berg wrote:
>>> But you can do workarounds all you want in hostapd, I don't care.

But I do care.. ;-)  As such, I would change that to "you can do
workarounds in your copy of hostapd". And yes, this would likely be the
best location for the workaround.

>>> However, I do think that if the hardware just isn't up to the job y=
ou
>>> should probably buy new hardware, after all, it's dirt cheap. :)

I have never seen wlan design that does not report frame ACK status
properly for transmitted unicast frames. Is it absolutely clear that
that is indeed the case here or could we should be missing some
documentation explaining how this should be done?

> I'd really love for some workaround that would allow AP mode to work.=
=20

The workaround would be to make hostapd generate bogus TX callback
internally with claim for a successfully received ACK when sending
(re)association response. I'm not very keen on including such
functionality and certainly do not consider dropping the correct
behavior because of a broken hw design.

If someone can come up with a clean patch that allows this to be
configured in hostapd.conf without affecting the default behavior, I
could consider applying the changes. However, please keep in mind that
I'm interested in using TX status reporting to improve reliability of
connection setup, so its use is more likely to increase in the future.

> It would be great if it could be deemed that the protocol doesn't rea=
lly
> require CTS protection/status information, and can be made reliable a=
nd
> standards compliant without support for it from the driver.
> Unfortunately I don't have high hopes for that, but I wonder what's M=
r.
> Malinen's opinion?

IEEE 802.11 association state in the AP is changed when receiving an AC=
K
from the STA for a (re)association response frame with success status
(see IEEE Std 802.11-2007, 11.3.2.2). In order to be able to implement
this correctly, AP mode operation require the TX status callback to
provide information for the ACK status of (re)association response
frames.

I would also like to see this status used to speed up recovery from=20
dropped EAPOL frames during IEEE 802.1X EAP authentication, 4-way
handshake and group handshake. In certain environments, this could
improve stability of connection establishment greatly.

- Jouni

--
To unsubscribe from this list: send the line "unsubscribe linux-wireles=
s" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: ACK matching [was: TX status reporting with help of an ack queue]

[Mikko Virkkilä]

On Fri, 2008-09-19 at 20:50 +0300, Jouni Malinen wrote:
> Mikko Virkkil=C3=A4 wrote:
> > On Fri, 2008-09-19 at 15:51 +0200, Mattias Nissler wrote:
> >> On Fri, 2008-09-19 at 12:31 +0200, Johannes Berg wrote:
> >>> But you can do workarounds all you want in hostapd, I don't care.
>=20
> But I do care.. ;-)  As such, I would change that to "you can do
> workarounds in your copy of hostapd". And yes, this would likely be t=
he
> best location for the workaround.
>=20
> >>> However, I do think that if the hardware just isn't up to the job=
 you
> >>> should probably buy new hardware, after all, it's dirt cheap. :)
>=20
> I have never seen wlan design that does not report frame ACK status
> properly for transmitted unicast frames. Is it absolutely clear that
> that is indeed the case here or could we should be missing some
> documentation explaining how this should be done?
>=20
> > I'd really love for some workaround that would allow AP mode to wor=
k.=20
>=20
> The workaround would be to make hostapd generate bogus TX callback
> internally with claim for a successfully received ACK when sending
> (re)association response. I'm not very keen on including such
> functionality and certainly do not consider dropping the correct
> behavior because of a broken hw design.
>=20
> If someone can come up with a clean patch that allows this to be
> configured in hostapd.conf without affecting the default behavior, I
> could consider applying the changes. However, please keep in mind tha=
t
> I'm interested in using TX status reporting to improve reliability of
> connection setup, so its use is more likely to increase in the future=
=2E
>=20
> > It would be great if it could be deemed that the protocol doesn't r=
eally
> > require CTS protection/status information, and can be made reliable=
 and
> > standards compliant without support for it from the driver.
> > Unfortunately I don't have high hopes for that, but I wonder what's=
 Mr.
> > Malinen's opinion?
>=20
> IEEE 802.11 association state in the AP is changed when receiving an =
ACK
> from the STA for a (re)association response frame with success status
> (see IEEE Std 802.11-2007, 11.3.2.2). In order to be able to implemen=
t
> this correctly, AP mode operation require the TX status callback to
> provide information for the ACK status of (re)association response
> frames.
>=20
> I would also like to see this status used to speed up recovery from=20
> dropped EAPOL frames during IEEE 802.1X EAP authentication, 4-way
> handshake and group handshake. In certain environments, this could
> improve stability of connection establishment greatly.
>=20
> - Jouni

Well, sounds like there are good reasons to keep investigating the
possibility of fixing this a bit more properly in the driver.=20

If we turn off the AUTO_TX_SEQ and/or TXD_W1_HW_SEQUENCE (what'as the
difference?) and assigning sequence numbers manually, could we then
match the ack-frame's sequence number to the ones we have sent?

- Mikko




--
To unsubscribe from this list: send the line "unsubscribe linux-wireles=
s" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: ACK matching [was: TX status reporting with help of an ack queue]

[Mattias Nissler]

On Tue, 2008-09-23 at 09:09 +0300, Mikko Virkkil=E4 wrote:
> On Fri, 2008-09-19 at 20:50 +0300, Jouni Malinen wrote:
> > Mikko Virkkil=E4 wrote:
> > > On Fri, 2008-09-19 at 15:51 +0200, Mattias Nissler wrote:
> > >> On Fri, 2008-09-19 at 12:31 +0200, Johannes Berg wrote:
> > >>> But you can do workarounds all you want in hostapd, I don't car=
e.
> >=20
> > But I do care.. ;-)  As such, I would change that to "you can do
> > workarounds in your copy of hostapd". And yes, this would likely be=
 the
> > best location for the workaround.
> >=20
> > >>> However, I do think that if the hardware just isn't up to the j=
ob you
> > >>> should probably buy new hardware, after all, it's dirt cheap. :=
)
> >=20
> > I have never seen wlan design that does not report frame ACK status
> > properly for transmitted unicast frames. Is it absolutely clear tha=
t
> > that is indeed the case here or could we should be missing some
> > documentation explaining how this should be done?
> >=20
> > > I'd really love for some workaround that would allow AP mode to w=
ork.=20
> >=20
> > The workaround would be to make hostapd generate bogus TX callback
> > internally with claim for a successfully received ACK when sending
> > (re)association response. I'm not very keen on including such
> > functionality and certainly do not consider dropping the correct
> > behavior because of a broken hw design.
> >=20
> > If someone can come up with a clean patch that allows this to be
> > configured in hostapd.conf without affecting the default behavior, =
I
> > could consider applying the changes. However, please keep in mind t=
hat
> > I'm interested in using TX status reporting to improve reliability =
of
> > connection setup, so its use is more likely to increase in the futu=
re.
> >=20
> > > It would be great if it could be deemed that the protocol doesn't=
 really
> > > require CTS protection/status information, and can be made reliab=
le and
> > > standards compliant without support for it from the driver.
> > > Unfortunately I don't have high hopes for that, but I wonder what=
's Mr.
> > > Malinen's opinion?
> >=20
> > IEEE 802.11 association state in the AP is changed when receiving a=
n ACK
> > from the STA for a (re)association response frame with success stat=
us
> > (see IEEE Std 802.11-2007, 11.3.2.2). In order to be able to implem=
ent
> > this correctly, AP mode operation require the TX status callback to
> > provide information for the ACK status of (re)association response
> > frames.
> >=20
> > I would also like to see this status used to speed up recovery from=
=20
> > dropped EAPOL frames during IEEE 802.1X EAP authentication, 4-way
> > handshake and group handshake. In certain environments, this could
> > improve stability of connection establishment greatly.
> >=20
> > - Jouni
>=20
> Well, sounds like there are good reasons to keep investigating the
> possibility of fixing this a bit more properly in the driver.=20
>=20
> If we turn off the AUTO_TX_SEQ and/or TXD_W1_HW_SEQUENCE (what'as the
> difference?) and assigning sequence numbers manually, could we then
> match the ack-frame's sequence number to the ones we have sent?

Unfortunately, ACK frames do not carry sequence numbers. Only managemen=
t
and control frames have them.

Mattias

--
To unsubscribe from this list: send the line "unsubscribe linux-wireles=
s" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: ACK matching [was: TX status reporting with help of an ack queue]

[Ivo van Doorn]

On Friday 19 September 2008, Mattias Nissler wrote:
> On Fri, 2008-09-19 at 12:08 +0300, Mikko Virkkil=E4 wrote:
> > On Thu, 2008-09-18 at 22:29 +0000, Mattias Nissler wrote:
> > > [I've added the zd1211rw maintainers to the CC, I hope they can s=
hed
> > > some light on the question whether the ack queue mechanism that d=
river
> > > has is actually correct]
> > >=20
> > > I've had a closer look at the idea of deciding whether a frame ha=
s
> > > been
> > > transmitted succesfully by monitoring incoming ACK frames and I'v=
e hit
> > > a
> > > fundamental problem: How do you correlate incoming ACK frames to =
the
> > > frames that were actually transmitted? The ACK frame only carries=
 the
> > > address of the station that transmitted the frame being acked, no
> > > further information. Now this means if you have the hardware TX t=
wo
> > > frames and receive one ACK frame in the RX path, you cannot know =
which
> > > TX frame the ACK belongs to, because they will be identical, righ=
t?
> > >=20
> > > The hardware, however, knows, because the ACKs are required to be
> > > transmitted directly after the corresponding frame is received, b=
ut we
> > > don't know in the driver about this timing information, at least =
not for
> > > rt2x00.
> > >=20
> > > I wonder whether the ack queue idea Mikko found in the zd1211rw d=
river
> > > is actually working correctly for that driver? I've only had a sh=
ort
> > > look, but found that incoming ACKs are only matched against trans=
mitted
> > > frames by means of the address carried within the ACK. So I'd thi=
nk in
> > > the situation I outlined above, the zd1211rw driver will also be =
unable
> > > to match the ACK to the correct frame. Any comments on this?
> > >=20
> > > Mattiass
> > >=20
> >=20
> > AFAIK your analysis is correct and I think this is somewhat of a kn=
own
> > problem with the implementation in zd1211rw driver. I haven't come
> > across a way of getting around the problem. I read some suggestions
> > about stopping all transmission to a certain address until we get a
> > status update for the previous packet sent to that address, but iir=
c
> > this was deemed impractical. On the other hand the current
> > implementation in zd1211rw (and hopefully soon in rt2x00) doesn't r=
eally
> > break anything that works without it and on rt2x00 it gets AP-mode =
in a
> > somewhat more usable state.=20
>=20
> Well, as long as nothing vitally depends on the transmissions status
> (i.e. MAC layer acknowledgements in 802.11 lingo), you are right that=
 it
> won't break anything. Nevertheless, it is code that is known to be
> working incorrectly and non-fixable, so I'd vote against it (I'd rath=
er
> live with the approach of always reporting successful tx status as yo=
u
> have proposed earlier if I had to choose from the two of them). Maybe=
 we
> just have to accept that there is hardware that cannot report MAC lay=
er
> acknowledgements back to the driver. Unless somebody comes up with a
> feasible idea how to do it. Ivo, do we have a contact at Ralink to as=
k
> about this?

The Legacy USB drivers don't care about the TX status, if it wants to k=
now
the TX statistics it reads the register which contains the ACK/RTS coun=
ters
and determine with those values if the link is good or not.

Seeing that the PCI drivers also don't really care about the TX status =
(other
then setting some statistics) I think we can assume the TX status isn't=
 important
for the Ralink people.

But I'll send a mail to Ralink to see if they actually do have some ide=
as about it.

> This brings up the question whether we can do without tx status
> reporting. Does anyone know why hostapd requires the tx status? As fa=
r
> as I understand, mac80211 only uses the tx status reporting only for =
the
> tx rate control. Rate control algos that don't use tx status are
> definitely feasible (and in fact we'll need one for rt73).
>=20
> I'll look into hostapd to figure out whether the tx status reporting =
is
> really required when I find some time. However, I've just received
> rt2800 hardware, so getting the rt2800 driver going is my top priorit=
y
> for the next weeks.

Ivo
--
To unsubscribe from this list: send the line "unsubscribe linux-wireles=
s" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: ACK matching [was: TX status reporting with help of an ack queue]

[Daniel Drake]

Mattias Nissler wrote:
> I wonder whether the ack queue idea Mikko found in the zd1211rw driver
> is actually working correctly for that driver? I've only had a short
> look, but found that incoming ACKs are only matched against transmitted
> frames by means of the address carried within the ACK. So I'd think in
> the situation I outlined above, the zd1211rw driver will also be unable
> to match the ACK to the correct frame. Any comments on this?

It is known not-quite-correct and an area of the code that I personally 
dislike. However, without it, rate control does not work at all, so it 
is better than not having it. (my personal preference would be for a RC 
algo which does not require input of successful TX frames, but I have 
not stepped up to design or do the work...)
The code that also keeps the USB TX queue to reasonable size also relies 
on it, and in that case the inaccuracies are also not important.

Daniel