All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH bluetooth-next 0/3] ATUSB driver updates
@ 2015-05-21 14:51 Stefan Schmidt
  2015-05-21 14:51 ` [PATCH bluetooth-next 1/3] ieee802154/atusb: Warn about outdated device firmware Stefan Schmidt
                   ` (4 more replies)
  0 siblings, 5 replies; 15+ messages in thread
From: Stefan Schmidt @ 2015-05-21 14:51 UTC (permalink / raw)
  To: linux-wpan; +Cc: Alexander Aring, Stefan Schmidt

Hello.

Some updates for the atusb driver.

Checking for minimum firmware version now that we have a public release. It
only warns if an old version is used for now but updating in encouraged.

The newest firmware also handles AACK handling so we can indicate this to the
stack.

The last patch just sets the default ed level like the other drivers. Better
ed handling for atusb is planned for the future.

Stefan Schmidt (3):
  ieee802154/atusb: Warn about outdated device firmware.
  ieee802154/atusb: Mark driver as AACK enabled in hardware.
  ieee802154/atusb: Set default ed level to 0xbe like the rest of these
    drivers

 drivers/net/ieee802154/atusb.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

regards
Stefan Schmidt

-- 
2.1.0


^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH bluetooth-next 1/3] ieee802154/atusb: Warn about outdated device firmware.
  2015-05-21 14:51 [PATCH bluetooth-next 0/3] ATUSB driver updates Stefan Schmidt
@ 2015-05-21 14:51 ` Stefan Schmidt
  2015-05-21 14:51 ` [PATCH bluetooth-next 2/3] ieee802154/atusb: Mark driver as AACK enabled in hardware Stefan Schmidt
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 15+ messages in thread
From: Stefan Schmidt @ 2015-05-21 14:51 UTC (permalink / raw)
  To: linux-wpan; +Cc: Alexander Aring, Stefan Schmidt, Stefan Schmidt

From: Stefan Schmidt <s.schmidt@samsung.com>

Together with mainlining the driver we released a first public binary version
of the device firmware. This is version 0.2. With this change we warn users
who run outdated versions of the firmware. While we have not seen problems
with it yet (thus no error, but a warning only) it would be better to run the
released and tested firmware. You can find released versions here:

http://downloads.qi-hardware.com/people/werner/wpan/web/
Signed-off-by: Stefan Schmidt <stefan@osg.samsung.com>
---
 drivers/net/ieee802154/atusb.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/net/ieee802154/atusb.c b/drivers/net/ieee802154/atusb.c
index ea1259e..9d07dd7 100644
--- a/drivers/net/ieee802154/atusb.c
+++ b/drivers/net/ieee802154/atusb.c
@@ -469,6 +469,12 @@ static int atusb_get_and_show_revision(struct atusb *atusb)
 		dev_info(&usb_dev->dev,
 			 "Firmware: major: %u, minor: %u, hardware type: %u\n",
 			 buffer[0], buffer[1], buffer[2]);
+	if (buffer[0] == 0 && buffer[1] < 2) {
+		dev_info(&usb_dev->dev,
+			 "Firmware version (%u.%u) is predates our first public release.",
+			 buffer[0], buffer[1]);
+		dev_info(&usb_dev->dev, "Please update to version 0.2 or newer");
+	}
 
 	return ret;
 }
-- 
2.1.0


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH bluetooth-next 2/3] ieee802154/atusb: Mark driver as AACK enabled in hardware.
  2015-05-21 14:51 [PATCH bluetooth-next 0/3] ATUSB driver updates Stefan Schmidt
  2015-05-21 14:51 ` [PATCH bluetooth-next 1/3] ieee802154/atusb: Warn about outdated device firmware Stefan Schmidt
@ 2015-05-21 14:51 ` Stefan Schmidt
  2015-05-25  7:41   ` Lennert Buytenhek
  2015-05-21 14:51 ` [PATCH bluetooth-next 3/3] ieee802154/atusb: Set default ed level to 0xbe like the rest of these drivers Stefan Schmidt
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 15+ messages in thread
From: Stefan Schmidt @ 2015-05-21 14:51 UTC (permalink / raw)
  To: linux-wpan; +Cc: Alexander Aring, Stefan Schmidt, Stefan Schmidt

From: Stefan Schmidt <s.schmidt@samsung.com>

Since firmware version 0.2 we use AACK handling directly in the firmware.
Inform the stack that the hardware supports and uses it.

Signed-off-by: Stefan Schmidt <stefan@osg.samsung.com>
---
 drivers/net/ieee802154/atusb.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ieee802154/atusb.c b/drivers/net/ieee802154/atusb.c
index 9d07dd7..eef1d8a 100644
--- a/drivers/net/ieee802154/atusb.c
+++ b/drivers/net/ieee802154/atusb.c
@@ -568,7 +568,8 @@ static int atusb_probe(struct usb_interface *interface,
 		goto fail;
 
 	hw->parent = &usb_dev->dev;
-	hw->flags = IEEE802154_HW_TX_OMIT_CKSUM | IEEE802154_HW_AFILT;
+	hw->flags = IEEE802154_HW_TX_OMIT_CKSUM | IEEE802154_HW_AFILT |
+		    IEEE802154_HW_AACK;
 
 	hw->phy->current_page = 0;
 	hw->phy->current_channel = 11;	/* reset default */
-- 
2.1.0


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH bluetooth-next 3/3] ieee802154/atusb: Set default ed level to 0xbe like the rest of these drivers
  2015-05-21 14:51 [PATCH bluetooth-next 0/3] ATUSB driver updates Stefan Schmidt
  2015-05-21 14:51 ` [PATCH bluetooth-next 1/3] ieee802154/atusb: Warn about outdated device firmware Stefan Schmidt
  2015-05-21 14:51 ` [PATCH bluetooth-next 2/3] ieee802154/atusb: Mark driver as AACK enabled in hardware Stefan Schmidt
@ 2015-05-21 14:51 ` Stefan Schmidt
  2015-05-21 15:36 ` [PATCH bluetooth-next 0/3] ATUSB driver updates Alexander Aring
  2015-05-21 15:55 ` Marcel Holtmann
  4 siblings, 0 replies; 15+ messages in thread
From: Stefan Schmidt @ 2015-05-21 14:51 UTC (permalink / raw)
  To: linux-wpan; +Cc: Alexander Aring, Stefan Schmidt, Stefan Schmidt

From: Stefan Schmidt <s.schmidt@samsung.com>

Signed-off-by: Stefan Schmidt <stefan@osg.samsung.com>
---
 drivers/net/ieee802154/atusb.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ieee802154/atusb.c b/drivers/net/ieee802154/atusb.c
index eef1d8a..1e18774 100644
--- a/drivers/net/ieee802154/atusb.c
+++ b/drivers/net/ieee802154/atusb.c
@@ -365,8 +365,8 @@ static int atusb_channel(struct ieee802154_hw *hw, u8 page, u8 channel)
 
 static int atusb_ed(struct ieee802154_hw *hw, u8 *level)
 {
-	/* @@@ not used by the stack yet */
-	*level = 0;
+	BUG_ON(!level);
+	*level = 0xbe;
 	return 0;
 }
 
-- 
2.1.0


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [PATCH bluetooth-next 0/3] ATUSB driver updates
  2015-05-21 14:51 [PATCH bluetooth-next 0/3] ATUSB driver updates Stefan Schmidt
                   ` (2 preceding siblings ...)
  2015-05-21 14:51 ` [PATCH bluetooth-next 3/3] ieee802154/atusb: Set default ed level to 0xbe like the rest of these drivers Stefan Schmidt
@ 2015-05-21 15:36 ` Alexander Aring
  2015-05-21 15:55 ` Marcel Holtmann
  4 siblings, 0 replies; 15+ messages in thread
From: Alexander Aring @ 2015-05-21 15:36 UTC (permalink / raw)
  To: Stefan Schmidt; +Cc: linux-wpan

Hi,

On Thu, May 21, 2015 at 04:51:34PM +0200, Stefan Schmidt wrote:
> Hello.
> 
> Some updates for the atusb driver.
> 
> Checking for minimum firmware version now that we have a public release. It
> only warns if an old version is used for now but updating in encouraged.
> 
> The newest firmware also handles AACK handling so we can indicate this to the
> stack.
> 
> The last patch just sets the default ed level like the other drivers. Better
> ed handling for atusb is planned for the future.
> 
> Stefan Schmidt (3):
>   ieee802154/atusb: Warn about outdated device firmware.
>   ieee802154/atusb: Mark driver as AACK enabled in hardware.
>   ieee802154/atusb: Set default ed level to 0xbe like the rest of these
>     drivers
> 
>  drivers/net/ieee802154/atusb.c | 13 ++++++++++---
>  1 file changed, 10 insertions(+), 3 deletions(-)

Acked-by: Alexander Aring <alex.aring@gmail.com>

on the complete series.

- Alex


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH bluetooth-next 0/3] ATUSB driver updates
  2015-05-21 14:51 [PATCH bluetooth-next 0/3] ATUSB driver updates Stefan Schmidt
                   ` (3 preceding siblings ...)
  2015-05-21 15:36 ` [PATCH bluetooth-next 0/3] ATUSB driver updates Alexander Aring
@ 2015-05-21 15:55 ` Marcel Holtmann
  4 siblings, 0 replies; 15+ messages in thread
From: Marcel Holtmann @ 2015-05-21 15:55 UTC (permalink / raw)
  To: Stefan Schmidt; +Cc: linux-wpan, Alexander Aring

Hi Stefan,

> Some updates for the atusb driver.
> 
> Checking for minimum firmware version now that we have a public release. It
> only warns if an old version is used for now but updating in encouraged.
> 
> The newest firmware also handles AACK handling so we can indicate this to the
> stack.
> 
> The last patch just sets the default ed level like the other drivers. Better
> ed handling for atusb is planned for the future.
> 
> Stefan Schmidt (3):
>  ieee802154/atusb: Warn about outdated device firmware.
>  ieee802154/atusb: Mark driver as AACK enabled in hardware.
>  ieee802154/atusb: Set default ed level to 0xbe like the rest of these
>    drivers
> 
> drivers/net/ieee802154/atusb.c | 13 ++++++++++---
> 1 file changed, 10 insertions(+), 3 deletions(-)

all 3 patches have been applied to bluetooth-next tree.

Regards

Marcel


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH bluetooth-next 2/3] ieee802154/atusb: Mark driver as AACK enabled in hardware.
  2015-05-21 14:51 ` [PATCH bluetooth-next 2/3] ieee802154/atusb: Mark driver as AACK enabled in hardware Stefan Schmidt
@ 2015-05-25  7:41   ` Lennert Buytenhek
  2015-05-26  8:56     ` Alexander Aring
  0 siblings, 1 reply; 15+ messages in thread
From: Lennert Buytenhek @ 2015-05-25  7:41 UTC (permalink / raw)
  To: Stefan Schmidt; +Cc: linux-wpan, Alexander Aring, Stefan Schmidt

On Thu, May 21, 2015 at 04:51:36PM +0200, Stefan Schmidt wrote:

> Since firmware version 0.2 we use AACK handling directly in the firmware.
> Inform the stack that the hardware supports and uses it.
> 
> Signed-off-by: Stefan Schmidt <stefan@osg.samsung.com>
> ---
>  drivers/net/ieee802154/atusb.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ieee802154/atusb.c b/drivers/net/ieee802154/atusb.c
> index 9d07dd7..eef1d8a 100644
> --- a/drivers/net/ieee802154/atusb.c
> +++ b/drivers/net/ieee802154/atusb.c
> @@ -568,7 +568,8 @@ static int atusb_probe(struct usb_interface *interface,
>  		goto fail;
>  
>  	hw->parent = &usb_dev->dev;
> -	hw->flags = IEEE802154_HW_TX_OMIT_CKSUM | IEEE802154_HW_AFILT;
> +	hw->flags = IEEE802154_HW_TX_OMIT_CKSUM | IEEE802154_HW_AFILT |
> +		    IEEE802154_HW_AACK;
>  
>  	hw->phy->current_page = 0;
>  	hw->phy->current_channel = 11;	/* reset default */

I'm wondering about this patch...

The IEEE802154_HW_AACK flag is defined in the core:

include/net/mac802154.h:
	/* Indicates that receiver will autorespond with ACK frames. */
	#define IEEE802154_HW_AACK              0x00000002

And is set by various drivers:

drivers/net/ieee802154/at86rf230.c:     lp->hw->flags = IEEE802154_HW_TX_OMIT_CKSUM | IEEE802154_HW_AACK |
drivers/net/ieee802154/atusb.c:             IEEE802154_HW_AACK;
drivers/net/ieee802154/cc2520.c:        priv->hw->flags = IEEE802154_HW_OMIT_CKSUM | IEEE802154_HW_AACK |
drivers/net/ieee802154/mrf24j40.c:      devrec->hw->flags = IEEE802154_HW_OMIT_CKSUM | IEEE802154_HW_AACK |

But there's no code anywhere in the tree that tests for this flag, and
if I think about it for a bit, I'm not sure what the core code could do
with this information, as I don't think it's feasible to generate ACKs
in software if the hardware doesn't support auto-ACKing?  (Is hardware
that doesn't support this useful or usable at all?  Maybe just remove
the flag altogether?)

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH bluetooth-next 2/3] ieee802154/atusb: Mark driver as AACK enabled in hardware.
  2015-05-25  7:41   ` Lennert Buytenhek
@ 2015-05-26  8:56     ` Alexander Aring
  2015-05-27 12:45       ` Lennert Buytenhek
  0 siblings, 1 reply; 15+ messages in thread
From: Alexander Aring @ 2015-05-26  8:56 UTC (permalink / raw)
  To: Lennert Buytenhek; +Cc: Stefan Schmidt, linux-wpan, Stefan Schmidt

Hi Lennert,

On Mon, May 25, 2015 at 10:41:33AM +0300, Lennert Buytenhek wrote:
> On Thu, May 21, 2015 at 04:51:36PM +0200, Stefan Schmidt wrote:
> 
> > Since firmware version 0.2 we use AACK handling directly in the firmware.
> > Inform the stack that the hardware supports and uses it.
> > 
> > Signed-off-by: Stefan Schmidt <stefan@osg.samsung.com>
> > ---
> >  drivers/net/ieee802154/atusb.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/net/ieee802154/atusb.c b/drivers/net/ieee802154/atusb.c
> > index 9d07dd7..eef1d8a 100644
> > --- a/drivers/net/ieee802154/atusb.c
> > +++ b/drivers/net/ieee802154/atusb.c
> > @@ -568,7 +568,8 @@ static int atusb_probe(struct usb_interface *interface,
> >  		goto fail;
> >  
> >  	hw->parent = &usb_dev->dev;
> > -	hw->flags = IEEE802154_HW_TX_OMIT_CKSUM | IEEE802154_HW_AFILT;
> > +	hw->flags = IEEE802154_HW_TX_OMIT_CKSUM | IEEE802154_HW_AFILT |
> > +		    IEEE802154_HW_AACK;
> >  
> >  	hw->phy->current_page = 0;
> >  	hw->phy->current_channel = 11;	/* reset default */
> 
> I'm wondering about this patch...
> 
> The IEEE802154_HW_AACK flag is defined in the core:
> 
> include/net/mac802154.h:
> 	/* Indicates that receiver will autorespond with ACK frames. */
> 	#define IEEE802154_HW_AACK              0x00000002
> 
> And is set by various drivers:
> 
> drivers/net/ieee802154/at86rf230.c:     lp->hw->flags = IEEE802154_HW_TX_OMIT_CKSUM | IEEE802154_HW_AACK |
> drivers/net/ieee802154/atusb.c:             IEEE802154_HW_AACK;
> drivers/net/ieee802154/cc2520.c:        priv->hw->flags = IEEE802154_HW_OMIT_CKSUM | IEEE802154_HW_AACK |
> drivers/net/ieee802154/mrf24j40.c:      devrec->hw->flags = IEEE802154_HW_OMIT_CKSUM | IEEE802154_HW_AACK |
> 
> But there's no code anywhere in the tree that tests for this flag, and
> if I think about it for a bit, I'm not sure what the core code could do
> with this information, as I don't think it's feasible to generate ACKs
> in software if the hardware doesn't support auto-ACKing?  (Is hardware
> that doesn't support this useful or usable at all?  Maybe just remove
> the flag altogether?)

yes this is true, this flag isn't evaluated and we can't never
supporting ack handling in software. I never saw a transceiver which
doesn't support auto-ACK handling also.

One use case would be that we check on this flag when we receive an ack
frame in mac802154 frame parsing. Then we could drop a WARN_ONCE which
means "hey care that you support aack handling in your driver".

But our logic is for _all_ drivers now is that they should _always_
support AACK handling by default, there is no feature disable or enable.
So maybe removing this flag and then making always WARN_ONCE if we get an
ACK frame in mac802154 (except monitor interface).

The reason is that a sending node which using max_frame_retries above or
equal zero, needs to get an ACK frame after tranmsitting. Otherwise for
frame_retries e.g. 3 the receiving node will receive the frame three
times contiguoues.

So AACK frame handling is always enabled (in mac header is a bit to
enable ack request, this should be configureable somehow (also for
6LoWPAN, it's currently hard coded always enabled)). And ARET handling
can be configureable by max_frame_retries parameters, (-1) means in this
case don't waiting for a ack frame while tranmsit. (0) Means sending
frame but wait for ack frame without retransmit, but possible further
error counting could check on "why transmit failed" and this there
should some counters "no ack frame". All setting above 0 is the same just
with retransmit support, which is can't be done by software but we have
the option to disable/enable it.


btw: I having some plans to rework the frame parsing/creation. The
parsing is based on mac802154 frame parsing [0]. There I have implement
the WARN_ONCE [1]. The frame parsing isn't mainline yet, because I can't
test the crypto layer. For doing this draft I simple removed the crypto
layer which makes the parts easier. The current frame parsing is much
oriented to support data frames only to deal with 6LoWPAN. It's a mess
currently to support new things into the frame parsing and we should
really look how mac802154 dealing with frame parsing and doing the same
in mac802154 and ieee802154 6lowpan, just other hex values and static
inline functions. Then we also don't need to parse always the whole
frame when we just want the addresses for example. Also the wireless
community (if they want) getting easier familar with the code.

- Alex

[0] https://github.com/linux-wpan/linux-wpan-next/blob/wpan_rework_rfc/net/mac802154/rx.c
[1] https://github.com/linux-wpan/linux-wpan-next/blob/wpan_rework_rfc/net/mac802154/rx.c#L171

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH bluetooth-next 2/3] ieee802154/atusb: Mark driver as AACK enabled in hardware.
  2015-05-26  8:56     ` Alexander Aring
@ 2015-05-27 12:45       ` Lennert Buytenhek
  2015-05-27 13:46         ` Alexander Aring
  0 siblings, 1 reply; 15+ messages in thread
From: Lennert Buytenhek @ 2015-05-27 12:45 UTC (permalink / raw)
  To: Alexander Aring; +Cc: Stefan Schmidt, linux-wpan, Stefan Schmidt

On Tue, May 26, 2015 at 10:56:18AM +0200, Alexander Aring wrote:

> Hi Lennert,

Hi!


> > > Since firmware version 0.2 we use AACK handling directly in the firmware.
> > > Inform the stack that the hardware supports and uses it.
> > > 
> > > Signed-off-by: Stefan Schmidt <stefan@osg.samsung.com>
> > > ---
> > >  drivers/net/ieee802154/atusb.c | 3 ++-
> > >  1 file changed, 2 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/net/ieee802154/atusb.c b/drivers/net/ieee802154/atusb.c
> > > index 9d07dd7..eef1d8a 100644
> > > --- a/drivers/net/ieee802154/atusb.c
> > > +++ b/drivers/net/ieee802154/atusb.c
> > > @@ -568,7 +568,8 @@ static int atusb_probe(struct usb_interface *interface,
> > >  		goto fail;
> > >  
> > >  	hw->parent = &usb_dev->dev;
> > > -	hw->flags = IEEE802154_HW_TX_OMIT_CKSUM | IEEE802154_HW_AFILT;
> > > +	hw->flags = IEEE802154_HW_TX_OMIT_CKSUM | IEEE802154_HW_AFILT |
> > > +		    IEEE802154_HW_AACK;
> > >  
> > >  	hw->phy->current_page = 0;
> > >  	hw->phy->current_channel = 11;	/* reset default */
> > 
> > I'm wondering about this patch...
> > 
> > The IEEE802154_HW_AACK flag is defined in the core:
> > 
> > include/net/mac802154.h:
> > 	/* Indicates that receiver will autorespond with ACK frames. */
> > 	#define IEEE802154_HW_AACK              0x00000002
> > 
> > And is set by various drivers:
> > 
> > drivers/net/ieee802154/at86rf230.c:     lp->hw->flags = IEEE802154_HW_TX_OMIT_CKSUM | IEEE802154_HW_AACK |
> > drivers/net/ieee802154/atusb.c:             IEEE802154_HW_AACK;
> > drivers/net/ieee802154/cc2520.c:        priv->hw->flags = IEEE802154_HW_OMIT_CKSUM | IEEE802154_HW_AACK |
> > drivers/net/ieee802154/mrf24j40.c:      devrec->hw->flags = IEEE802154_HW_OMIT_CKSUM | IEEE802154_HW_AACK |
> > 
> > But there's no code anywhere in the tree that tests for this flag, and
> > if I think about it for a bit, I'm not sure what the core code could do
> > with this information, as I don't think it's feasible to generate ACKs
> > in software if the hardware doesn't support auto-ACKing?  (Is hardware
> > that doesn't support this useful or usable at all?  Maybe just remove
> > the flag altogether?)
> 
> yes this is true, this flag isn't evaluated and we can't never
> supporting ack handling in software. I never saw a transceiver which
> doesn't support auto-ACK handling also.
> 
> One use case would be that we check on this flag when we receive an ack
> frame in mac802154 frame parsing. Then we could drop a WARN_ONCE which
> means "hey care that you support aack handling in your driver".
> 
> But our logic is for _all_ drivers now is that they should _always_
> support AACK handling by default, there is no feature disable or enable.

Yep, and so I think the flag should just go.


> So maybe removing this flag and then making always WARN_ONCE if we get an
> ACK frame in mac802154 (except monitor interface).

How do you mean, get an ACK frame in mac802154?  The AACK flag is
documented to be about automatically responding to received packets
addressed to us by transmitting an ACK frame in hardware, but I
think you are saying here that non-AACK hardware would also pass
received ACK packets through to software?  (Is that how e.g. atusb
behaves if you don't enable AACK mode in hardware?)


> btw: I having some plans to rework the frame parsing/creation. The
> parsing is based on mac802154 frame parsing [0]. There I have implement
> the WARN_ONCE [1]. The frame parsing isn't mainline yet, because I can't
> test the crypto layer. For doing this draft I simple removed the crypto
> layer which makes the parts easier. The current frame parsing is much
> oriented to support data frames only to deal with 6LoWPAN. It's a mess
> currently to support new things into the frame parsing and we should
> really look how mac802154 dealing with frame parsing and doing the same
> in mac802154 and ieee802154 6lowpan, just other hex values and static
> inline functions. Then we also don't need to parse always the whole
> frame when we just want the addresses for example. Also the wireless
> community (if they want) getting easier familar with the code.

I'll look into this.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH bluetooth-next 2/3] ieee802154/atusb: Mark driver as AACK enabled in hardware.
  2015-05-27 12:45       ` Lennert Buytenhek
@ 2015-05-27 13:46         ` Alexander Aring
  2015-05-27 16:12           ` Alexander Aring
  2015-05-28 10:29           ` Lennert Buytenhek
  0 siblings, 2 replies; 15+ messages in thread
From: Alexander Aring @ 2015-05-27 13:46 UTC (permalink / raw)
  To: Lennert Buytenhek; +Cc: Stefan Schmidt, linux-wpan, Stefan Schmidt

On Wed, May 27, 2015 at 03:45:50PM +0300, Lennert Buytenhek wrote:
> On Tue, May 26, 2015 at 10:56:18AM +0200, Alexander Aring wrote:
> 
> > Hi Lennert,
> 
> Hi!
> 
> 
> > > > Since firmware version 0.2 we use AACK handling directly in the firmware.
> > > > Inform the stack that the hardware supports and uses it.
> > > > 
> > > > Signed-off-by: Stefan Schmidt <stefan@osg.samsung.com>
> > > > ---
> > > >  drivers/net/ieee802154/atusb.c | 3 ++-
> > > >  1 file changed, 2 insertions(+), 1 deletion(-)
> > > > 
> > > > diff --git a/drivers/net/ieee802154/atusb.c b/drivers/net/ieee802154/atusb.c
> > > > index 9d07dd7..eef1d8a 100644
> > > > --- a/drivers/net/ieee802154/atusb.c
> > > > +++ b/drivers/net/ieee802154/atusb.c
> > > > @@ -568,7 +568,8 @@ static int atusb_probe(struct usb_interface *interface,
> > > >  		goto fail;
> > > >  
> > > >  	hw->parent = &usb_dev->dev;
> > > > -	hw->flags = IEEE802154_HW_TX_OMIT_CKSUM | IEEE802154_HW_AFILT;
> > > > +	hw->flags = IEEE802154_HW_TX_OMIT_CKSUM | IEEE802154_HW_AFILT |
> > > > +		    IEEE802154_HW_AACK;
> > > >  
> > > >  	hw->phy->current_page = 0;
> > > >  	hw->phy->current_channel = 11;	/* reset default */
> > > 
> > > I'm wondering about this patch...
> > > 
> > > The IEEE802154_HW_AACK flag is defined in the core:
> > > 
> > > include/net/mac802154.h:
> > > 	/* Indicates that receiver will autorespond with ACK frames. */
> > > 	#define IEEE802154_HW_AACK              0x00000002
> > > 
> > > And is set by various drivers:
> > > 
> > > drivers/net/ieee802154/at86rf230.c:     lp->hw->flags = IEEE802154_HW_TX_OMIT_CKSUM | IEEE802154_HW_AACK |
> > > drivers/net/ieee802154/atusb.c:             IEEE802154_HW_AACK;
> > > drivers/net/ieee802154/cc2520.c:        priv->hw->flags = IEEE802154_HW_OMIT_CKSUM | IEEE802154_HW_AACK |
> > > drivers/net/ieee802154/mrf24j40.c:      devrec->hw->flags = IEEE802154_HW_OMIT_CKSUM | IEEE802154_HW_AACK |
> > > 
> > > But there's no code anywhere in the tree that tests for this flag, and
> > > if I think about it for a bit, I'm not sure what the core code could do
> > > with this information, as I don't think it's feasible to generate ACKs
> > > in software if the hardware doesn't support auto-ACKing?  (Is hardware
> > > that doesn't support this useful or usable at all?  Maybe just remove
> > > the flag altogether?)
> > 
> > yes this is true, this flag isn't evaluated and we can't never
> > supporting ack handling in software. I never saw a transceiver which
> > doesn't support auto-ACK handling also.
> > 
> > One use case would be that we check on this flag when we receive an ack
> > frame in mac802154 frame parsing. Then we could drop a WARN_ONCE which
> > means "hey care that you support aack handling in your driver".
> > 
> > But our logic is for _all_ drivers now is that they should _always_
> > support AACK handling by default, there is no feature disable or enable.
> 
> Yep, and so I think the flag should just go.
> 
> 
> > So maybe removing this flag and then making always WARN_ONCE if we get an
> > ACK frame in mac802154 (except monitor interface).
> 
> How do you mean, get an ACK frame in mac802154?  The AACK flag is
> documented to be about automatically responding to received packets
> addressed to us by transmitting an ACK frame in hardware, but I
> think you are saying here that non-AACK hardware would also pass
> received ACK packets through to software?  (Is that how e.g. atusb
> behaves if you don't enable AACK mode in hardware?)

Yes, I think I was wrong here. We need to talk about how the ack
handling is really used. (I got confused because AACK handling in
at86rf2xx means that ack frames are never delivered to the next layer).

AACK = automatically ack frame transmitting, when we receive a frame we
       create an ack frame. _IF_ the ack request bit in 802.15.4 mac is
       set.

ARET = this is enabled when max_frame_retries is 0 or above, e.g. 3
       means we try to transmit and if no ack is received then this will
       be retransmit at maximum value of 3 times.

This means if you using ARET handling (max_frame_retries >= 0) in a
network with nodes that doesn't support AACK handling you have a bigger
issue and need to turn off ARET handling (that's currently the reason
why we have as default value for max_frame_retries = -1). The normal
802.15.4 default is max_frame_retries = 3.


I got confused now, because if the at86rf2xx are in RX_AACK mode (which
is the AACK handling mode), then the transceiver doesn't send the ACK
frames to the next upper layer. In normal RX mode (without AACK) then
the transceiver would send ack frames to the next layer which is
mac802154, but the mac802154 can't handle them.

What I suggest is that we always drop a WARN_ONCE when we receive an ACK
frame, because the network is a possible ARET enabled network and we
can't handle the ack frames, so when a node speaks with a ARET handling
and max_frame_retries = 3 we get the same frame 3 times and the sending
node think that the receiving node has never got the frame, because the
receiving node never send an ack frame.

Is this more clear now? Or maybe I am wrong here.

I recently send also a patch that the ack request bit is only set when
the transceiver operates in ARET mode, do you think that's a good idea
for the default behaviour of 802.15.4 6LoWPAN frames?


In monitor mode (promiscuous mode) then the phy doesn't filter anything
and delivers to userspace directly. This is not 100% that what's it is
on air because the ack handling is sometimes too fast and the
transceiver doesn't get all of them, but this is okay for a monitor interface.
When I can put another question to this mail is "it's a good idea that a
monitor interface can transmit frames", they don't can handle never ack
frames, it should only a passiv listen mode, except somebody wants to
make some mess into the network.

- Alex

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH bluetooth-next 2/3] ieee802154/atusb: Mark driver as AACK enabled in hardware.
  2015-05-27 13:46         ` Alexander Aring
@ 2015-05-27 16:12           ` Alexander Aring
  2015-05-28 10:36             ` Lennert Buytenhek
  2015-05-28 10:29           ` Lennert Buytenhek
  1 sibling, 1 reply; 15+ messages in thread
From: Alexander Aring @ 2015-05-27 16:12 UTC (permalink / raw)
  To: Lennert Buytenhek; +Cc: Stefan Schmidt, linux-wpan, Stefan Schmidt

On Wed, May 27, 2015 at 03:46:09PM +0200, Alexander Aring wrote:
> On Wed, May 27, 2015 at 03:45:50PM +0300, Lennert Buytenhek wrote:
> > On Tue, May 26, 2015 at 10:56:18AM +0200, Alexander Aring wrote:
> > 
> > > Hi Lennert,
> > 
> > Hi!
> > 
> > 
> > > > > Since firmware version 0.2 we use AACK handling directly in the firmware.
> > > > > Inform the stack that the hardware supports and uses it.
> > > > > 
> > > > > Signed-off-by: Stefan Schmidt <stefan@osg.samsung.com>
> > > > > ---
> > > > >  drivers/net/ieee802154/atusb.c | 3 ++-
> > > > >  1 file changed, 2 insertions(+), 1 deletion(-)
> > > > > 
> > > > > diff --git a/drivers/net/ieee802154/atusb.c b/drivers/net/ieee802154/atusb.c
> > > > > index 9d07dd7..eef1d8a 100644
> > > > > --- a/drivers/net/ieee802154/atusb.c
> > > > > +++ b/drivers/net/ieee802154/atusb.c
> > > > > @@ -568,7 +568,8 @@ static int atusb_probe(struct usb_interface *interface,
> > > > >  		goto fail;
> > > > >  
> > > > >  	hw->parent = &usb_dev->dev;
> > > > > -	hw->flags = IEEE802154_HW_TX_OMIT_CKSUM | IEEE802154_HW_AFILT;
> > > > > +	hw->flags = IEEE802154_HW_TX_OMIT_CKSUM | IEEE802154_HW_AFILT |
> > > > > +		    IEEE802154_HW_AACK;
> > > > >  
> > > > >  	hw->phy->current_page = 0;
> > > > >  	hw->phy->current_channel = 11;	/* reset default */
> > > > 
> > > > I'm wondering about this patch...
> > > > 
> > > > The IEEE802154_HW_AACK flag is defined in the core:
> > > > 
> > > > include/net/mac802154.h:
> > > > 	/* Indicates that receiver will autorespond with ACK frames. */
> > > > 	#define IEEE802154_HW_AACK              0x00000002
> > > > 
> > > > And is set by various drivers:
> > > > 
> > > > drivers/net/ieee802154/at86rf230.c:     lp->hw->flags = IEEE802154_HW_TX_OMIT_CKSUM | IEEE802154_HW_AACK |
> > > > drivers/net/ieee802154/atusb.c:             IEEE802154_HW_AACK;
> > > > drivers/net/ieee802154/cc2520.c:        priv->hw->flags = IEEE802154_HW_OMIT_CKSUM | IEEE802154_HW_AACK |
> > > > drivers/net/ieee802154/mrf24j40.c:      devrec->hw->flags = IEEE802154_HW_OMIT_CKSUM | IEEE802154_HW_AACK |
> > > > 
> > > > But there's no code anywhere in the tree that tests for this flag, and
> > > > if I think about it for a bit, I'm not sure what the core code could do
> > > > with this information, as I don't think it's feasible to generate ACKs
> > > > in software if the hardware doesn't support auto-ACKing?  (Is hardware
> > > > that doesn't support this useful or usable at all?  Maybe just remove
> > > > the flag altogether?)
> > > 
> > > yes this is true, this flag isn't evaluated and we can't never
> > > supporting ack handling in software. I never saw a transceiver which
> > > doesn't support auto-ACK handling also.
> > > 
> > > One use case would be that we check on this flag when we receive an ack
> > > frame in mac802154 frame parsing. Then we could drop a WARN_ONCE which
> > > means "hey care that you support aack handling in your driver".
> > > 
> > > But our logic is for _all_ drivers now is that they should _always_
> > > support AACK handling by default, there is no feature disable or enable.
> > 
> > Yep, and so I think the flag should just go.
> > 
> > 
> > > So maybe removing this flag and then making always WARN_ONCE if we get an
> > > ACK frame in mac802154 (except monitor interface).
> > 
> > How do you mean, get an ACK frame in mac802154?  The AACK flag is
> > documented to be about automatically responding to received packets
> > addressed to us by transmitting an ACK frame in hardware, but I
> > think you are saying here that non-AACK hardware would also pass
> > received ACK packets through to software?  (Is that how e.g. atusb
> > behaves if you don't enable AACK mode in hardware?)
> 
> Yes, I think I was wrong here. We need to talk about how the ack
> handling is really used. (I got confused because AACK handling in
> at86rf2xx means that ack frames are never delivered to the next layer).
> 
> AACK = automatically ack frame transmitting, when we receive a frame we
>        create an ack frame. _IF_ the ack request bit in 802.15.4 mac is
>        set.
> 
> ARET = this is enabled when max_frame_retries is 0 or above, e.g. 3
>        means we try to transmit and if no ack is received then this will
>        be retransmit at maximum value of 3 times.
> 
> This means if you using ARET handling (max_frame_retries >= 0) in a
> network with nodes that doesn't support AACK handling you have a bigger
> issue and need to turn off ARET handling (that's currently the reason
> why we have as default value for max_frame_retries = -1). The normal
> 802.15.4 default is max_frame_retries = 3.
> 

I think I need also to say that a max_frame_retries which is 0 or above,
then the transceiver do also a CSMA-CA handling before.

The max_frame_retries = -1 is only a "transmit only" mode. Then the
driver need to tell the transceiver do not CSMA-CA handling before and
don't wait for acks afterwards when doing frame transmit.

All other values are: do CSMA-CA handling before and wait for ack frame
after transmit. This handling must done completely on phy which have
this mac functionality. We simple can't run this by software at
mac802154 layer because timing constraints.

If a phy doesn't support max_frame_retries = -1 then it can manipulate
the capabilities values so you never can set the transceiver out of this
mode with nl802154. If a phy support max_frame_retries = -1 then this is
simple the same that the driver need to manipulate the capabilities.

But one of these modes (max_frame_retries = -1 OR max_frame_retries >=
0) should be supported and AACK handling should also always supported.


Maybe I should draw some graphics to display what the meaning of
max_frame_retries really is. At the end each driver should follow all
the same meaning of this value.

- Alex

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH bluetooth-next 2/3] ieee802154/atusb: Mark driver as AACK enabled in hardware.
  2015-05-27 13:46         ` Alexander Aring
  2015-05-27 16:12           ` Alexander Aring
@ 2015-05-28 10:29           ` Lennert Buytenhek
  2015-05-28 11:25             ` Alexander Aring
  1 sibling, 1 reply; 15+ messages in thread
From: Lennert Buytenhek @ 2015-05-28 10:29 UTC (permalink / raw)
  To: Alexander Aring; +Cc: Stefan Schmidt, linux-wpan, Stefan Schmidt

On Wed, May 27, 2015 at 03:46:13PM +0200, Alexander Aring wrote:

> > > > > Since firmware version 0.2 we use AACK handling directly in the firmware.
> > > > > Inform the stack that the hardware supports and uses it.
> > > > > 
> > > > > Signed-off-by: Stefan Schmidt <stefan@osg.samsung.com>
> > > > > ---
> > > > >  drivers/net/ieee802154/atusb.c | 3 ++-
> > > > >  1 file changed, 2 insertions(+), 1 deletion(-)
> > > > > 
> > > > > diff --git a/drivers/net/ieee802154/atusb.c b/drivers/net/ieee802154/atusb.c
> > > > > index 9d07dd7..eef1d8a 100644
> > > > > --- a/drivers/net/ieee802154/atusb.c
> > > > > +++ b/drivers/net/ieee802154/atusb.c
> > > > > @@ -568,7 +568,8 @@ static int atusb_probe(struct usb_interface *interface,
> > > > >  		goto fail;
> > > > >  
> > > > >  	hw->parent = &usb_dev->dev;
> > > > > -	hw->flags = IEEE802154_HW_TX_OMIT_CKSUM | IEEE802154_HW_AFILT;
> > > > > +	hw->flags = IEEE802154_HW_TX_OMIT_CKSUM | IEEE802154_HW_AFILT |
> > > > > +		    IEEE802154_HW_AACK;
> > > > >  
> > > > >  	hw->phy->current_page = 0;
> > > > >  	hw->phy->current_channel = 11;	/* reset default */
> > > > 
> > > > I'm wondering about this patch...
> > > > 
> > > > The IEEE802154_HW_AACK flag is defined in the core:
> > > > 
> > > > include/net/mac802154.h:
> > > > 	/* Indicates that receiver will autorespond with ACK frames. */
> > > > 	#define IEEE802154_HW_AACK              0x00000002
> > > > 
> > > > And is set by various drivers:
> > > > 
> > > > drivers/net/ieee802154/at86rf230.c:     lp->hw->flags = IEEE802154_HW_TX_OMIT_CKSUM | IEEE802154_HW_AACK |
> > > > drivers/net/ieee802154/atusb.c:             IEEE802154_HW_AACK;
> > > > drivers/net/ieee802154/cc2520.c:        priv->hw->flags = IEEE802154_HW_OMIT_CKSUM | IEEE802154_HW_AACK |
> > > > drivers/net/ieee802154/mrf24j40.c:      devrec->hw->flags = IEEE802154_HW_OMIT_CKSUM | IEEE802154_HW_AACK |
> > > > 
> > > > But there's no code anywhere in the tree that tests for this flag, and
> > > > if I think about it for a bit, I'm not sure what the core code could do
> > > > with this information, as I don't think it's feasible to generate ACKs
> > > > in software if the hardware doesn't support auto-ACKing?  (Is hardware
> > > > that doesn't support this useful or usable at all?  Maybe just remove
> > > > the flag altogether?)
> > > 
> > > yes this is true, this flag isn't evaluated and we can't never
> > > supporting ack handling in software. I never saw a transceiver which
> > > doesn't support auto-ACK handling also.
> > > 
> > > One use case would be that we check on this flag when we receive an ack
> > > frame in mac802154 frame parsing. Then we could drop a WARN_ONCE which
> > > means "hey care that you support aack handling in your driver".
> > > 
> > > But our logic is for _all_ drivers now is that they should _always_
> > > support AACK handling by default, there is no feature disable or enable.
> > 
> > Yep, and so I think the flag should just go.
> > 
> > 
> > > So maybe removing this flag and then making always WARN_ONCE if we get an
> > > ACK frame in mac802154 (except monitor interface).
> > 
> > How do you mean, get an ACK frame in mac802154?  The AACK flag is
> > documented to be about automatically responding to received packets
> > addressed to us by transmitting an ACK frame in hardware, but I
> > think you are saying here that non-AACK hardware would also pass
> > received ACK packets through to software?  (Is that how e.g. atusb
> > behaves if you don't enable AACK mode in hardware?)
> 
> Yes, I think I was wrong here. We need to talk about how the ack
> handling is really used. (I got confused because AACK handling in
> at86rf2xx means that ack frames are never delivered to the next layer).
> 
> AACK = automatically ack frame transmitting, when we receive a frame we
>        create an ack frame. _IF_ the ack request bit in 802.15.4 mac is
>        set.
> 
> ARET = this is enabled when max_frame_retries is 0 or above, e.g. 3
>        means we try to transmit and if no ack is received then this will
>        be retransmit at maximum value of 3 times.
> 
> This means if you using ARET handling (max_frame_retries >= 0) in a
> network with nodes that doesn't support AACK handling you have a bigger
> issue and need to turn off ARET handling (that's currently the reason
> why we have as default value for max_frame_retries = -1). The normal
> 802.15.4 default is max_frame_retries = 3.

The 802.15.4 spec requires auto ACK handling, so why is this behaviour
in there?  Is there a lot of non-spec-compliant hardware out there that
never ACKs frames?  (If so, do we care about such hardware?)


> I got confused now, because if the at86rf2xx are in RX_AACK mode (which
> is the AACK handling mode), then the transceiver doesn't send the ACK
> frames to the next upper layer. In normal RX mode (without AACK) then
> the transceiver would send ack frames to the next layer which is
> mac802154, but the mac802154 can't handle them.
> 
> What I suggest is that we always drop a WARN_ONCE when we receive an ACK
> frame, because the network is a possible ARET enabled network and we
> can't handle the ack frames, so when a node speaks with a ARET handling
> and max_frame_retries = 3 we get the same frame 3 times and the sending
> node think that the receiving node has never got the frame, because the
> receiving node never send an ack frame.

Even if the sending node doesn't retry the transmission it can be
important to the sending node to know whether or not the packet
was ACKed (this would be max_frame_retries == 0), as some MLME state
machine actions depend on your packet having been ACKed by the other end.


> I recently send also a patch that the ack request bit is only set when
> the transceiver operates in ARET mode, do you think that's a good idea
> for the default behaviour of 802.15.4 6LoWPAN frames?

I think it's a good idea for all frames, not just 6LoWPAN ones, and
it should probably be the default behavior -- but then the question
of what the default value of max_frame_retries should be is the
harder question, IMHO.


> In monitor mode (promiscuous mode) then the phy doesn't filter anything
> and delivers to userspace directly. This is not 100% that what's it is
> on air because the ack handling is sometimes too fast and the
> transceiver doesn't get all of them, but this is okay for a monitor interface.
> When I can put another question to this mail is "it's a good idea that a
> monitor interface can transmit frames", they don't can handle never ack
> frames, it should only a passiv listen mode, except somebody wants to
> make some mess into the network.

I'm undecided about this right now -- I can find arguments for why
it should be possible to do this and for why it shouldn't and I'm
not sure which are more convincing.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH bluetooth-next 2/3] ieee802154/atusb: Mark driver as AACK enabled in hardware.
  2015-05-27 16:12           ` Alexander Aring
@ 2015-05-28 10:36             ` Lennert Buytenhek
  2015-05-28 11:30               ` Alexander Aring
  0 siblings, 1 reply; 15+ messages in thread
From: Lennert Buytenhek @ 2015-05-28 10:36 UTC (permalink / raw)
  To: Alexander Aring; +Cc: Stefan Schmidt, linux-wpan, Stefan Schmidt

On Wed, May 27, 2015 at 06:12:06PM +0200, Alexander Aring wrote:

> > > > > > Since firmware version 0.2 we use AACK handling directly in the firmware.
> > > > > > Inform the stack that the hardware supports and uses it.
> > > > > > 
> > > > > > Signed-off-by: Stefan Schmidt <stefan@osg.samsung.com>
> > > > > > ---
> > > > > >  drivers/net/ieee802154/atusb.c | 3 ++-
> > > > > >  1 file changed, 2 insertions(+), 1 deletion(-)
> > > > > > 
> > > > > > diff --git a/drivers/net/ieee802154/atusb.c b/drivers/net/ieee802154/atusb.c
> > > > > > index 9d07dd7..eef1d8a 100644
> > > > > > --- a/drivers/net/ieee802154/atusb.c
> > > > > > +++ b/drivers/net/ieee802154/atusb.c
> > > > > > @@ -568,7 +568,8 @@ static int atusb_probe(struct usb_interface *interface,
> > > > > >  		goto fail;
> > > > > >  
> > > > > >  	hw->parent = &usb_dev->dev;
> > > > > > -	hw->flags = IEEE802154_HW_TX_OMIT_CKSUM | IEEE802154_HW_AFILT;
> > > > > > +	hw->flags = IEEE802154_HW_TX_OMIT_CKSUM | IEEE802154_HW_AFILT |
> > > > > > +		    IEEE802154_HW_AACK;
> > > > > >  
> > > > > >  	hw->phy->current_page = 0;
> > > > > >  	hw->phy->current_channel = 11;	/* reset default */
> > > > > 
> > > > > I'm wondering about this patch...
> > > > > 
> > > > > The IEEE802154_HW_AACK flag is defined in the core:
> > > > > 
> > > > > include/net/mac802154.h:
> > > > > 	/* Indicates that receiver will autorespond with ACK frames. */
> > > > > 	#define IEEE802154_HW_AACK              0x00000002
> > > > > 
> > > > > And is set by various drivers:
> > > > > 
> > > > > drivers/net/ieee802154/at86rf230.c:     lp->hw->flags = IEEE802154_HW_TX_OMIT_CKSUM | IEEE802154_HW_AACK |
> > > > > drivers/net/ieee802154/atusb.c:             IEEE802154_HW_AACK;
> > > > > drivers/net/ieee802154/cc2520.c:        priv->hw->flags = IEEE802154_HW_OMIT_CKSUM | IEEE802154_HW_AACK |
> > > > > drivers/net/ieee802154/mrf24j40.c:      devrec->hw->flags = IEEE802154_HW_OMIT_CKSUM | IEEE802154_HW_AACK |
> > > > > 
> > > > > But there's no code anywhere in the tree that tests for this flag, and
> > > > > if I think about it for a bit, I'm not sure what the core code could do
> > > > > with this information, as I don't think it's feasible to generate ACKs
> > > > > in software if the hardware doesn't support auto-ACKing?  (Is hardware
> > > > > that doesn't support this useful or usable at all?  Maybe just remove
> > > > > the flag altogether?)
> > > > 
> > > > yes this is true, this flag isn't evaluated and we can't never
> > > > supporting ack handling in software. I never saw a transceiver which
> > > > doesn't support auto-ACK handling also.
> > > > 
> > > > One use case would be that we check on this flag when we receive an ack
> > > > frame in mac802154 frame parsing. Then we could drop a WARN_ONCE which
> > > > means "hey care that you support aack handling in your driver".
> > > > 
> > > > But our logic is for _all_ drivers now is that they should _always_
> > > > support AACK handling by default, there is no feature disable or enable.
> > > 
> > > Yep, and so I think the flag should just go.
> > > 
> > > 
> > > > So maybe removing this flag and then making always WARN_ONCE if we get an
> > > > ACK frame in mac802154 (except monitor interface).
> > > 
> > > How do you mean, get an ACK frame in mac802154?  The AACK flag is
> > > documented to be about automatically responding to received packets
> > > addressed to us by transmitting an ACK frame in hardware, but I
> > > think you are saying here that non-AACK hardware would also pass
> > > received ACK packets through to software?  (Is that how e.g. atusb
> > > behaves if you don't enable AACK mode in hardware?)
> > 
> > Yes, I think I was wrong here. We need to talk about how the ack
> > handling is really used. (I got confused because AACK handling in
> > at86rf2xx means that ack frames are never delivered to the next layer).
> > 
> > AACK = automatically ack frame transmitting, when we receive a frame we
> >        create an ack frame. _IF_ the ack request bit in 802.15.4 mac is
> >        set.
> > 
> > ARET = this is enabled when max_frame_retries is 0 or above, e.g. 3
> >        means we try to transmit and if no ack is received then this will
> >        be retransmit at maximum value of 3 times.
> > 
> > This means if you using ARET handling (max_frame_retries >= 0) in a
> > network with nodes that doesn't support AACK handling you have a bigger
> > issue and need to turn off ARET handling (that's currently the reason
> > why we have as default value for max_frame_retries = -1). The normal
> > 802.15.4 default is max_frame_retries = 3.
> > 
> 
> I think I need also to say that a max_frame_retries which is 0 or above,
> then the transceiver do also a CSMA-CA handling before.
> 
> The max_frame_retries = -1 is only a "transmit only" mode. Then the
> driver need to tell the transceiver do not CSMA-CA handling before and
> don't wait for acks afterwards when doing frame transmit.
> 
> All other values are: do CSMA-CA handling before and wait for ack frame
> after transmit. This handling must done completely on phy which have
> this mac functionality. We simple can't run this by software at
> mac802154 layer because timing constraints.
> 
> If a phy doesn't support max_frame_retries = -1 then it can manipulate
> the capabilities values so you never can set the transceiver out of this
> mode with nl802154. If a phy support max_frame_retries = -1 then this is
> simple the same that the driver need to manipulate the capabilities.
> 
> But one of these modes (max_frame_retries = -1 OR max_frame_retries >=
> 0) should be supported and AACK handling should also always supported.

Hardware that only supports max_frame_retries = -1 is clearly not
802.15.4 compliant -- how much of such hardware is there out there?


> Maybe I should draw some graphics to display what the meaning of
> max_frame_retries really is. At the end each driver should follow all
> the same meaning of this value.

I think it's clear to me.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH bluetooth-next 2/3] ieee802154/atusb: Mark driver as AACK enabled in hardware.
  2015-05-28 10:29           ` Lennert Buytenhek
@ 2015-05-28 11:25             ` Alexander Aring
  0 siblings, 0 replies; 15+ messages in thread
From: Alexander Aring @ 2015-05-28 11:25 UTC (permalink / raw)
  To: Lennert Buytenhek; +Cc: Stefan Schmidt, linux-wpan, Stefan Schmidt

On Thu, May 28, 2015 at 01:29:14PM +0300, Lennert Buytenhek wrote:
...
> > 
> > Yes, I think I was wrong here. We need to talk about how the ack
> > handling is really used. (I got confused because AACK handling in
> > at86rf2xx means that ack frames are never delivered to the next layer).
> > 
> > AACK = automatically ack frame transmitting, when we receive a frame we
> >        create an ack frame. _IF_ the ack request bit in 802.15.4 mac is
> >        set.
> > 
> > ARET = this is enabled when max_frame_retries is 0 or above, e.g. 3
> >        means we try to transmit and if no ack is received then this will
> >        be retransmit at maximum value of 3 times.
> > 
> > This means if you using ARET handling (max_frame_retries >= 0) in a
> > network with nodes that doesn't support AACK handling you have a bigger
> > issue and need to turn off ARET handling (that's currently the reason
> > why we have as default value for max_frame_retries = -1). The normal
> > 802.15.4 default is max_frame_retries = 3.
> 
> The 802.15.4 spec requires auto ACK handling, so why is this behaviour
> in there?  Is there a lot of non-spec-compliant hardware out there that
> never ACKs frames?  (If so, do we care about such hardware?)
> 

Yes, that's why max_frame_retries = -1 is introduced, if we do a
max_frame_retries = 0 and receive no ACK frames then the transmission
logic of transceiver thinks the transmission was failed.

The point is here, there exists _a_ _lot_ of contiki etc... sensor nodes which
doesn't support AACK handling and then you need to be sure that all
(which can reach the non AACK node) don't use ARET handling. With ARET I
also means max_frame_retries = 0, in this case.

Since there still exists hardware which don't use AACK handling then we
need a option to disable ARET handling and this is currently our default (-1).
I tried to change it at [0], with a BIG note that it could be that other
nodes receives then the same frame three times (this indicates mostly
that the sensor node doesn't support AACK handling) I have myself also
some nodes which _could_ support AACK handling but contiki doesn't use
it.

The patch at [0] never reached mainline. 

> 
> > I got confused now, because if the at86rf2xx are in RX_AACK mode (which
> > is the AACK handling mode), then the transceiver doesn't send the ACK
> > frames to the next upper layer. In normal RX mode (without AACK) then
> > the transceiver would send ack frames to the next layer which is
> > mac802154, but the mac802154 can't handle them.
> > 
> > What I suggest is that we always drop a WARN_ONCE when we receive an ACK
> > frame, because the network is a possible ARET enabled network and we
> > can't handle the ack frames, so when a node speaks with a ARET handling
> > and max_frame_retries = 3 we get the same frame 3 times and the sending
> > node think that the receiving node has never got the frame, because the
> > receiving node never send an ack frame.
> 
> Even if the sending node doesn't retry the transmission it can be
> important to the sending node to know whether or not the packet
> was ACKed (this would be max_frame_retries == 0), as some MLME state
> machine actions depend on your packet having been ACKed by the other end.
> 

yes, some MLME options required acks. In case of at86rf2xx we need to
take care then about the trac status which indicates if ACK was received
or not. Currently we doesn't care about this information. There is also
no way to get this information, I thought about to creating some stats
about that (which represents some 802.15.4 generic stats information).

I am not 100% how to deal with this currently, but I am sure this will
be a lot of fun.


In case of at86rf2xx if we are in RX_AACK mode then we doesn't get any
ack frames anymore to the next layer. The transceiver simple doesn't
react on ack frames anymore while receiving. Maybe because when it's in
RX_AACK and react on acks, then the RX_AACK mode would also send then
ack frames for acks, but this should not happen then.

I think that other transceivers (I currently has the atmel ones only)
react the same, so if we get ack frames inside of mac802154 then this
indicates some nodes operates _possible_ in ARET mode and the used
transceiver doesn't support AACK (because ack frames are delivered to
mac802154) and we should never deliver ack frames to this layer.

> 
> > I recently send also a patch that the ack request bit is only set when
> > the transceiver operates in ARET mode, do you think that's a good idea
> > for the default behaviour of 802.15.4 6LoWPAN frames?
> 
> I think it's a good idea for all frames, not just 6LoWPAN ones, and
> it should probably be the default behavior -- but then the question
> of what the default value of max_frame_retries should be is the
> harder question, IMHO.
> 

Yea, I would say we should use 802.15.4 default. If somebody expierence
that some node receives the same frame type 3 times, then it should turn
-1 on _all_ nodes in the network.

> 
> > In monitor mode (promiscuous mode) then the phy doesn't filter anything
> > and delivers to userspace directly. This is not 100% that what's it is
> > on air because the ack handling is sometimes too fast and the
> > transceiver doesn't get all of them, but this is okay for a monitor interface.
> > When I can put another question to this mail is "it's a good idea that a
> > monitor interface can transmit frames", they don't can handle never ack
> > frames, it should only a passiv listen mode, except somebody wants to
> > make some mess into the network.
> 
> I'm undecided about this right now -- I can find arguments for why
> it should be possible to do this and for why it shouldn't and I'm
> not sure which are more convincing.

okay. Maybe we talk this in another thread.

- Alex

[0] http://www.spinics.net/lists/linux-wpan/msg01604.html

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH bluetooth-next 2/3] ieee802154/atusb: Mark driver as AACK enabled in hardware.
  2015-05-28 10:36             ` Lennert Buytenhek
@ 2015-05-28 11:30               ` Alexander Aring
  0 siblings, 0 replies; 15+ messages in thread
From: Alexander Aring @ 2015-05-28 11:30 UTC (permalink / raw)
  To: Lennert Buytenhek; +Cc: Stefan Schmidt, linux-wpan, Stefan Schmidt

On Thu, May 28, 2015 at 01:36:14PM +0300, Lennert Buytenhek wrote:
...
> > 
> > But one of these modes (max_frame_retries = -1 OR max_frame_retries >=
> > 0) should be supported and AACK handling should also always supported.
> 
> Hardware that only supports max_frame_retries = -1 is clearly not
> 802.15.4 compliant -- how much of such hardware is there out there?
> 

This is correct, because 802.15.4 default is max_frame_retries = 3. I
never saw a transceiver which doesn't support ARET handling in hardware
by transceiver.

What we do currently is:

If a transceiver doesn't support handling of setting max_frame_retries
parameters then we assume the 802.15.4 defaults. We can't say if it's
really supported or not, depends on driver implementation.

And currently for max_frame_retries we assume "-1" which is not correct,
because 802.15.4 describes "3" as default. But again, this is only to
hold backwardscompability with nodes which doesn't support AACK.

> 
> > Maybe I should draw some graphics to display what the meaning of
> > max_frame_retries really is. At the end each driver should follow all
> > the same meaning of this value.
> 
> I think it's clear to me.

ok.

- Alex

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2015-05-28 11:30 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-05-21 14:51 [PATCH bluetooth-next 0/3] ATUSB driver updates Stefan Schmidt
2015-05-21 14:51 ` [PATCH bluetooth-next 1/3] ieee802154/atusb: Warn about outdated device firmware Stefan Schmidt
2015-05-21 14:51 ` [PATCH bluetooth-next 2/3] ieee802154/atusb: Mark driver as AACK enabled in hardware Stefan Schmidt
2015-05-25  7:41   ` Lennert Buytenhek
2015-05-26  8:56     ` Alexander Aring
2015-05-27 12:45       ` Lennert Buytenhek
2015-05-27 13:46         ` Alexander Aring
2015-05-27 16:12           ` Alexander Aring
2015-05-28 10:36             ` Lennert Buytenhek
2015-05-28 11:30               ` Alexander Aring
2015-05-28 10:29           ` Lennert Buytenhek
2015-05-28 11:25             ` Alexander Aring
2015-05-21 14:51 ` [PATCH bluetooth-next 3/3] ieee802154/atusb: Set default ed level to 0xbe like the rest of these drivers Stefan Schmidt
2015-05-21 15:36 ` [PATCH bluetooth-next 0/3] ATUSB driver updates Alexander Aring
2015-05-21 15:55 ` Marcel Holtmann

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.