Re: [RFCv3 bluetooth-next 4/6] mac802154: change max_frame_retries behaviour

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hello.

On 04/08/15 20:00, Alexander Aring wrote:
On Tue, Aug 04, 2015 at 06:40:46PM +0200, Stefan Schmidt wrote:
Hello.

On 30/07/15 10:55, Alexander Aring wrote:
This patch changes the default min value of max_frame_retries to 0 and
changes the max_frame_retries default value to 3 which is also 802.15.4
default.
You mean min_frame_retries to 0 here?
No, you mixed "frame_retries" ranges with "frame_retries" value.

So one is meant as a range, got it.


We don't use max_frame_retries "-1" as indicator for no-aret mode
anymore, instead we checking on the ack request bit inside the 802.15.4
frame control field.

Signed-off-by: Alexander Aring <alex.aring@xxxxxxxxx>
---
  net/mac802154/iface.c | 3 +--
  net/mac802154/main.c  | 9 +++------
  2 files changed, 4 insertions(+), 8 deletions(-)

diff --git a/net/mac802154/iface.c b/net/mac802154/iface.c
index 416de90..8837c5a 100644
--- a/net/mac802154/iface.c
+++ b/net/mac802154/iface.c
@@ -483,8 +483,7 @@ ieee802154_setup_sdata(struct ieee802154_sub_if_data *sdata,
  	wpan_dev->min_be = 3;
  	wpan_dev->max_be = 5;
  	wpan_dev->csma_retries = 4;
-	/* for compatibility, actual default is 3 */
-	wpan_dev->frame_retries = -1;
+	wpan_dev->frame_retries = 3;
Here is meant the frame_retries value, 802.15.4-2011 describe this value
as 3 for default.

  	wpan_dev->pan_id = cpu_to_le16(IEEE802154_PANID_BROADCAST);
  	wpan_dev->short_addr = cpu_to_le16(IEEE802154_ADDR_BROADCAST);
diff --git a/net/mac802154/main.c b/net/mac802154/main.c
index 91f1208..6e1764b 100644
--- a/net/mac802154/main.c
+++ b/net/mac802154/main.c
@@ -109,7 +109,7 @@ ieee802154_alloc_hw(size_t priv_data_len, const struct ieee802154_ops *ops)
  	phy->supported.max_minbe = 8;
  	phy->supported.min_maxbe = 3;
  	phy->supported.max_maxbe = 8;
-	phy->supported.min_frame_retries = -1;
+	phy->supported.min_frame_retries = 0;
  	phy->supported.max_frame_retries = 7;
Maybe also set it to 3 here if we want to have this as default and letting
it to userspace to set it to another value if needed?

Here is the frame_retries range by 802.15.4 which is "0..7". So now
there comes more internal mechanism, this range is set ONLY when you the
driver supports IEEE802154_HW_FRAME_RETRIES. If a transceiver supports
IEEE802154_HW_FRAME_RETRIES but not the default of range "0..7" then the
driver can overwrite this range.

Allowed range, fine.

  	phy->supported.max_csma_backoffs = 5;
  	phy->supported.lbt = NL802154_SUPPORTED_BOOL_FALSE;
@@ -175,11 +175,8 @@ int ieee802154_register_hw(struct ieee802154_hw *hw)
  	}
  	if (!(hw->flags & IEEE802154_HW_FRAME_RETRIES)) {
-		/* TODO should be 3, but our default value is -1 which means
-		 * no ARET handling.
-		 */
-		local->phy->supported.min_frame_retries = -1;
-		local->phy->supported.max_frame_retries = -1;
+		local->phy->supported.min_frame_retries = 3;
+		local->phy->supported.max_frame_retries = 3;
This confuses me. During alloc_hw we set min to 0 which makes sense for me
but afterwards we set both, min and max, to 3? Should it not only be max to
three here? Am I missing something?

Here is also meant the range. But in this case the transceiver doesn't
support IEEE802154_HW_FRAME_RETRIES. Then the stack assumes the 802.15.4
default value and the range is "3..3".

Hmm, would it not make sense to assume that a transceiver which does not support IEEE802154_HW_FRAME_RETRIES does not support ARET at all?

For me it sounds more as if we would want to set the range to 0..0 here because the transceiver does not support IEEE802154_HW_FRAME_RETRIES andthus no ARET.

regards
Stefan Schmidt
--
To unsubscribe from this list: send the line "unsubscribe linux-wpan" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Linux Audio Users]     [Photo]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux