From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from lists.s-osg.org ([54.187.51.154]:49936 "EHLO lists.s-osg.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751674AbcFVOnF (ORCPT ); Wed, 22 Jun 2016 10:43:05 -0400 Subject: Re: [PATCH bluetooth-next 3/3] ieee802154: 6lowpan: fix intra pan id check References: <20160617163450.3229-1-aar@pengutronix.de> <20160617163450.3229-3-aar@pengutronix.de> From: Stefan Schmidt Message-ID: <576AA3E3.8080904@osg.samsung.com> Date: Wed, 22 Jun 2016 16:42:43 +0200 MIME-Version: 1.0 In-Reply-To: <20160617163450.3229-3-aar@pengutronix.de> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Sender: linux-wpan-owner@vger.kernel.org List-ID: To: Alexander Aring , linux-wpan@vger.kernel.org Cc: kernel@pengutronix.de Hello. On 17/06/16 18:34, Alexander Aring wrote: > The RIOT-OS stack does send intra-pan frames but don't set the intra pan > flag inside the mac header. It seems this is valid frame addressing but > inefficient. Anyway this patch adds a new function for intra pan > addressing, doesn't matter if intra pan flag or source and destination > are the same. The newly introduction function will be used to check on > intra pan addressing for 6lowpan. > > Signed-off-by: Alexander Aring > --- > include/linux/ieee802154.h | 1 + > include/net/mac802154.h | 19 +++++++++++++++++++ > net/ieee802154/6lowpan/rx.c | 2 +- > 3 files changed, 21 insertions(+), 1 deletion(-) > > diff --git a/include/linux/ieee802154.h b/include/linux/ieee802154.h > index fd14815..ddb8901 100644 > --- a/include/linux/ieee802154.h > +++ b/include/linux/ieee802154.h > @@ -50,6 +50,7 @@ > > #define IEEE802154_EXTENDED_ADDR_LEN 8 > #define IEEE802154_SHORT_ADDR_LEN 2 > +#define IEEE802154_PAN_ID_LEN 2 > > #define IEEE802154_LIFS_PERIOD 40 > #define IEEE802154_SIFS_PERIOD 12 > diff --git a/include/net/mac802154.h b/include/net/mac802154.h > index deb90a1..df34187 100644 > --- a/include/net/mac802154.h > +++ b/include/net/mac802154.h > @@ -333,6 +333,25 @@ static inline unsigned char *ieee802154_skb_src_pan(__le16 fc, > } > > /** > + * ieee802154_skb_is_intra_pan_addressing - checks whenever the mac addressing > + * is an intra pan communication > + * @fc: mac header frame control field > + * @skb: skb where the source and destination pan should be get from > + */ > +static inline bool ieee802154_skb_is_intra_pan_addressing(__le16 fc, > + const struct sk_buff *skb) > +{ > + unsigned char *dst_pan = ieee802154_skb_dst_pan(fc, skb), > + *src_pan = ieee802154_skb_src_pan(fc, skb); > + > + /* if one is NULL is no intra pan addressing */ > + if (!dst_pan || !src_pan) > + return false; > + > + return !memcmp(dst_pan, src_pan, IEEE802154_PAN_ID_LEN); > +} This seems like a complicated way to compare two PAN IDs which are basicall le16 types. Why would you could the detour here expressing them as char *? In my opinion the ieee802154_skb_{src,dst}_pan() should just return the PAN ID as le16 value. That is what I would expect. > + > +/** > * ieee802154_be64_to_le64 - copies and convert be64 to le64 > * @le64_dst: le64 destination pointer > * @be64_src: be64 source pointer > diff --git a/net/ieee802154/6lowpan/rx.c b/net/ieee802154/6lowpan/rx.c > index ef185dd..649e7d45 100644 > --- a/net/ieee802154/6lowpan/rx.c > +++ b/net/ieee802154/6lowpan/rx.c > @@ -262,7 +262,7 @@ static inline bool lowpan_rx_h_check(struct sk_buff *skb) > > /* check on ieee802154 conform 6LoWPAN header */ > if (!ieee802154_is_data(fc) || > - !ieee802154_is_intra_pan(fc)) > + !ieee802154_skb_is_intra_pan_addressing(fc, skb)) > return false; > > /* check if we can dereference the dispatch */ regards Stefan Schmidt