Re: [RFCv3 bluetooth-next 2/6] ieee802154: add helpers for frame control checks

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

 



Hello.

On 04/08/15 19:44, Alexander Aring wrote:
On Tue, Aug 04, 2015 at 06:29:13PM +0200, Stefan Schmidt wrote:
Hello.

On 30/07/15 10:55, Alexander Aring wrote:
This patch introduce two static inline functions. The first to get the
frame control field from an sk_buff. The second is for checking on the
acknowledgment request bit on the frame control field. Later we can
introduce more functions to check on the frame control fields.

These will deprecate the current behaviour which requires a
host-byteorder conversion and manually bit handling.

Signed-off-by: Alexander Aring<alex.aring@xxxxxxxxx>
---
Some language suggestions inside.
ok.
  include/linux/ieee802154.h | 29 +++++++++++++++++++++++++++++
  1 file changed, 29 insertions(+)

diff --git a/include/linux/ieee802154.h b/include/linux/ieee802154.h
index 1dc1f4e..4f26c01 100644
--- a/include/linux/ieee802154.h
+++ b/include/linux/ieee802154.h
@@ -25,6 +25,8 @@
  #include <linux/types.h>
  #include <linux/random.h>
+#include <linux/skbuff.h>
+#include <linux/unaligned/memmove.h>
  #include <asm/byteorder.h>
  #define IEEE802154_MTU			127
@@ -205,6 +207,33 @@ enum {
  	IEEE802154_SCAN_IN_PROGRESS = 0xfc,
  };
+/* frame control handling */
+#define IEEE802154_FCTL_ACKREQ	0x0020
+
+/**
+ * ieee802154_is_ackreq - check if acknowledgment request bit is set
+ * @fc: frame control bytes in little-endian byteorder
+ */
+static inline bool ieee802154_is_ackreq(__le16 fc)
+{
+	return fc & cpu_to_le16(IEEE802154_FCTL_ACKREQ);
+}
+
+/**
+ * ieee802154_get_fc_from_skb - get the frame control field from an skb
... from a skb
ok.
+ * @skb: skb where the frame control field will be get from
Maybe:

skb which contains the frame control field

ok.
+ */
+static inline __le16 ieee802154_get_fc_from_skb(const struct sk_buff *skb)
+{
+	/* return some invalid fc on failure */
Maybe:

return on invalid fc

ok.
+	if (unlikely(skb->mac_len < 2)) {
+		WARN_ON(1);
+		return cpu_to_le16(0);
+	}
+
+	return (__force __le16)__get_unaligned_memmove16(skb_mac_header(skb));
Just to make sure we don't run into problems like we did with the 6lowpan
stack. __get_unaligned_memmove16 is not pulling the fc bytes out of the skb,
right? The skb stays as it is.

right it doesn't manipulate the skb. For the "problems like we did with
the 6lowpan" you need to decide which problems, I see several:

  - running skb_pull (which removes) buffer and we don't have the room to
    pull out the bytes of skb, example: skb->len = 3, skb_pull size is 4
    which ends in a BUG(), we need to check it with skb_may_pull before.

Sorry, I should have been a bit more verbose here to make it clearer. I meant the above.

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