[PATCH 1/3] ieee802154: add security bit check function

[PATCH 1/3] ieee802154: add security bit check function

[web+oss]

From: Alexandre Macabies <web+oss@zopieux.com>

ieee802154_is_secen checks if the 802.15.4 security bit is set in the
frame control field.

Signed-off-by: Alexander Aring <aar@pengutronix.de>
Reported-by: Alexandre Macabies <web+oss@zopieux.com>
Tested-by: Alexandre Macabies <web+oss@zopieux.com>
---
 include/linux/ieee802154.h | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/include/linux/ieee802154.h b/include/linux/ieee802154.h
index d3e4156..56090f1 100644
--- a/include/linux/ieee802154.h
+++ b/include/linux/ieee802154.h
@@ -218,6 +218,7 @@ enum {
 /* frame control handling */
 #define IEEE802154_FCTL_FTYPE		0x0003
 #define IEEE802154_FCTL_ACKREQ		0x0020
+#define IEEE802154_FCTL_SECEN		0x0004
 #define IEEE802154_FCTL_INTRA_PAN	0x0040
 
 #define IEEE802154_FTYPE_DATA		0x0001
@@ -233,6 +234,15 @@ static inline int ieee802154_is_data(__le16 fc)
 }
 
 /**
+ * ieee802154_is_secen - check if Security bit is set
+ * @fc: frame control bytes in little-endian byteorder
+ */
+static inline bool ieee802154_is_secen(__le16 fc)
+{
+	return fc & cpu_to_le16(IEEE802154_FCTL_SECEN);
+}
+
+/**
  * ieee802154_is_ackreq - check if acknowledgment request bit is set
  * @fc: frame control bytes in little-endian byteorder
  */
-- 
2.7.2

[PATCH 2/3] mrf24j40: fix security-enabled processing on inbound frames

[web+oss]

From: Alexandre Macabies <web+oss@zopieux.com>

When receiving a security-enabled IEEE 802.15.4 frame, the MRF24J40
triggers a SECIF interrupt that needs to be handled for RX processing
to keep functioning properly.

This patch enables the SECIF interrupt and makes the MRF ignores all
hardware processing of security-enabled frames, that is handled by the
ieee802154 stack instead.

Signed-off-by: Alexander Aring <aar@pengutronix.de>
Reported-by: Alexandre Macabies <web+oss@zopieux.com>
Tested-by: Alexandre Macabies <web+oss@zopieux.com>
---
 drivers/net/ieee802154/mrf24j40.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ieee802154/mrf24j40.c b/drivers/net/ieee802154/mrf24j40.c
index 764a2bd..96814cf 100644
--- a/drivers/net/ieee802154/mrf24j40.c
+++ b/drivers/net/ieee802154/mrf24j40.c
@@ -85,10 +85,12 @@
 #define REG_INTSTAT	0x31  /* Interrupt Status */
 #define BIT_TXNIF	BIT(0)
 #define BIT_RXIF	BIT(3)
+#define BIT_SECIF	BIT(4)
 
 #define REG_INTCON	0x32  /* Interrupt Control */
 #define BIT_TXNIE	BIT(0)
 #define BIT_RXIE	BIT(3)
+#define BIT_SECIE	BIT(4)
 
 #define REG_GPIO	0x33  /* GPIO */
 #define REG_TRISGPIO	0x34  /* GPIO direction */
@@ -616,7 +618,7 @@ static int mrf24j40_start(struct ieee802154_hw *hw)
 
 	/* Clear TXNIE and RXIE. Enable interrupts */
 	return regmap_update_bits(devrec->regmap_short, REG_INTCON,
-				  BIT_TXNIE | BIT_RXIE, 0);
+				  BIT_TXNIE | BIT_RXIE | BIT_SECIE, 0);
 }
 
 static void mrf24j40_stop(struct ieee802154_hw *hw)
@@ -1025,6 +1027,9 @@ static void mrf24j40_intstat_complete(void *context)
 
 	enable_irq(devrec->spi->irq);
 
+	if (intstat & BIT_SECIF)
+		regmap_write_async(devrec->regmap_short, REG_SECCON0, 0x80);
+
 	/* Check for TX complete */
 	if (intstat & BIT_TXNIF)
 		ieee802154_xmit_complete(devrec->hw, devrec->tx_skb, false);
-- 
2.7.2

Re: [PATCH 2/3] mrf24j40: fix security-enabled processing on inbound frames

[Stefan Schmidt]

Hello.

On 10/03/16 19:14, web+oss@zopieux.com wrote:
> From: Alexandre Macabies <web+oss@zopieux.com>
>
> When receiving a security-enabled IEEE 802.15.4 frame, the MRF24J40
> triggers a SECIF interrupt that needs to be handled for RX processing
> to keep functioning properly.
>
> This patch enables the SECIF interrupt and makes the MRF ignores all
> hardware processing of security-enabled frames, that is handled by the
> ieee802154 stack instead.

Missing SOB again.

> Signed-off-by: Alexander Aring <aar@pengutronix.de>
> Reported-by: Alexandre Macabies <web+oss@zopieux.com>
> Tested-by: Alexandre Macabies <web+oss@zopieux.com>
> ---
>   drivers/net/ieee802154/mrf24j40.c | 7 ++++++-
>   1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/ieee802154/mrf24j40.c b/drivers/net/ieee802154/mrf24j40.c
> index 764a2bd..96814cf 100644
> --- a/drivers/net/ieee802154/mrf24j40.c
> +++ b/drivers/net/ieee802154/mrf24j40.c
> @@ -85,10 +85,12 @@
>   #define REG_INTSTAT	0x31  /* Interrupt Status */
>   #define BIT_TXNIF	BIT(0)
>   #define BIT_RXIF	BIT(3)
> +#define BIT_SECIF	BIT(4)
>   
>   #define REG_INTCON	0x32  /* Interrupt Control */
>   #define BIT_TXNIE	BIT(0)
>   #define BIT_RXIE	BIT(3)
> +#define BIT_SECIE	BIT(4)
>   
>   #define REG_GPIO	0x33  /* GPIO */
>   #define REG_TRISGPIO	0x34  /* GPIO direction */
> @@ -616,7 +618,7 @@ static int mrf24j40_start(struct ieee802154_hw *hw)
>   
>   	/* Clear TXNIE and RXIE. Enable interrupts */
>   	return regmap_update_bits(devrec->regmap_short, REG_INTCON,
> -				  BIT_TXNIE | BIT_RXIE, 0);
> +				  BIT_TXNIE | BIT_RXIE | BIT_SECIE, 0);
>   }
>   
>   static void mrf24j40_stop(struct ieee802154_hw *hw)
> @@ -1025,6 +1027,9 @@ static void mrf24j40_intstat_complete(void *context)
>   
>   	enable_irq(devrec->spi->irq);
>   
> +	if (intstat & BIT_SECIF)
> +		regmap_write_async(devrec->regmap_short, REG_SECCON0, 0x80);

Not sure I like the magic number 0x80 here. Maybe a define for it?
> +
>   	/* Check for TX complete */
>   	if (intstat & BIT_TXNIF)
>   		ieee802154_xmit_complete(devrec->hw, devrec->tx_skb, false);


With the SOB and the magic number fixed you get my:

Reviewed-by: Stefan Schmidt <stefan@osg.samsung.com>

regards
Stefan Schmidt

[PATCH 3/3] mrf24j40: apply the security-enabled bit on secured outbound frames

[web+oss]

From: Alexandre Macabies <web+oss@zopieux.com>

We set the TXNSECEN bit of register TXNCON to on when transmitting a
security-enabled frame, as described in section 3.12.2 of the MRF
datasheet.

Signed-off-by: Alexander Aring <aar@pengutronix.de>
Reported-by: Alexandre Macabies <web+oss@zopieux.com>
Tested-by: Alexandre Macabies <web+oss@zopieux.com>
---
 drivers/net/ieee802154/mrf24j40.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/net/ieee802154/mrf24j40.c b/drivers/net/ieee802154/mrf24j40.c
index 96814cf..bf11cbb 100644
--- a/drivers/net/ieee802154/mrf24j40.c
+++ b/drivers/net/ieee802154/mrf24j40.c
@@ -61,6 +61,7 @@
 #define REG_TXBCON0	0x1A
 #define REG_TXNCON	0x1B  /* Transmit Normal FIFO Control */
 #define BIT_TXNTRIG	BIT(0)
+#define BIT_TXNSECEN	BIT(1)
 #define BIT_TXNACKREQ	BIT(2)
 
 #define REG_TXG1CON	0x1C
@@ -550,6 +551,9 @@ static void write_tx_buf_complete(void *context)
 	u8 val = BIT_TXNTRIG;
 	int ret;
 
+	if (ieee802154_is_secen(fc))
+		val |= BIT_TXNSECEN;
+
 	if (ieee802154_is_ackreq(fc))
 		val |= BIT_TXNACKREQ;
 
-- 
2.7.2

Re: [PATCH 3/3] mrf24j40: apply the security-enabled bit on secured outbound frames

[Stefan Schmidt]

Hello.

On 10/03/16 19:14, web+oss@zopieux.com wrote:
> From: Alexandre Macabies <web+oss@zopieux.com>
>
> We set the TXNSECEN bit of register TXNCON to on when transmitting a
> security-enabled frame, as described in section 3.12.2 of the MRF
> datasheet.

SOB again.

> Signed-off-by: Alexander Aring <aar@pengutronix.de>
> Reported-by: Alexandre Macabies <web+oss@zopieux.com>
> Tested-by: Alexandre Macabies <web+oss@zopieux.com>
> ---
>   drivers/net/ieee802154/mrf24j40.c | 4 ++++
>   1 file changed, 4 insertions(+)
>
> diff --git a/drivers/net/ieee802154/mrf24j40.c b/drivers/net/ieee802154/mrf24j40.c
> index 96814cf..bf11cbb 100644
> --- a/drivers/net/ieee802154/mrf24j40.c
> +++ b/drivers/net/ieee802154/mrf24j40.c
> @@ -61,6 +61,7 @@
>   #define REG_TXBCON0	0x1A
>   #define REG_TXNCON	0x1B  /* Transmit Normal FIFO Control */
>   #define BIT_TXNTRIG	BIT(0)
> +#define BIT_TXNSECEN	BIT(1)
>   #define BIT_TXNACKREQ	BIT(2)
>   
>   #define REG_TXG1CON	0x1C
> @@ -550,6 +551,9 @@ static void write_tx_buf_complete(void *context)
>   	u8 val = BIT_TXNTRIG;
>   	int ret;
>   
> +	if (ieee802154_is_secen(fc))
> +		val |= BIT_TXNSECEN;
> +
>   	if (ieee802154_is_ackreq(fc))
>   		val |= BIT_TXNACKREQ;
>   

Code looks fine. With the SOB fixed you get my:

Reviewed-by: Stefan Schmidt <stefan@osg.samsung.com>

regards
Stefan Schmidt

Re: [PATCH 1/3] ieee802154: add security bit check function

[Stefan Schmidt]

Hello.

On 10/03/16 19:14, web+oss@zopieux.com wrote:
> From: Alexandre Macabies <web+oss@zopieux.com>

Details about the commit author and SOB handling here.
The from line lists you but in the SOB section below Alexander signed it 
off.
This is a mismatch. As far as I understood it Alexander did the initial 
code after your report and you worked with it and tested it.

In that case having you as author (as you are submitting the patch here) 
and _both_ off you as sign offs would be correct imho.
>
> ieee802154_is_secen checks if the 802.15.4 security bit is set in the
> frame control field.
>
> Signed-off-by: Alexander Aring <aar@pengutronix.de>
> Reported-by: Alexandre Macabies <web+oss@zopieux.com>
> Tested-by: Alexandre Macabies <web+oss@zopieux.com>
> ---
>   include/linux/ieee802154.h | 10 ++++++++++
>   1 file changed, 10 insertions(+)
>
> diff --git a/include/linux/ieee802154.h b/include/linux/ieee802154.h
> index d3e4156..56090f1 100644
> --- a/include/linux/ieee802154.h
> +++ b/include/linux/ieee802154.h
> @@ -218,6 +218,7 @@ enum {
>   /* frame control handling */
>   #define IEEE802154_FCTL_FTYPE		0x0003
>   #define IEEE802154_FCTL_ACKREQ		0x0020
> +#define IEEE802154_FCTL_SECEN		0x0004
>   #define IEEE802154_FCTL_INTRA_PAN	0x0040
>   
>   #define IEEE802154_FTYPE_DATA		0x0001
> @@ -233,6 +234,15 @@ static inline int ieee802154_is_data(__le16 fc)
>   }
>   
>   /**
> + * ieee802154_is_secen - check if Security bit is set
> + * @fc: frame control bytes in little-endian byteorder
> + */
> +static inline bool ieee802154_is_secen(__le16 fc)
> +{
> +	return fc & cpu_to_le16(IEEE802154_FCTL_SECEN);
> +}
> +
> +/**
>    * ieee802154_is_ackreq - check if acknowledgment request bit is set
>    * @fc: frame control bytes in little-endian byteorder
>    */

Code looks fine. With the SOB fixed you get my:

Reviewed-by: Stefan Schmidt <stefan@osg.samsung.com>

regards
Stefan Schmidt

Re: [PATCH 1/3] ieee802154: add security bit check function

[Marcel Holtmann]

Hi Alexandre,

> Details about the commit author and SOB handling here.
> The from line lists you but in the SOB section below Alexander signed it off.
> This is a mismatch. As far as I understood it Alexander did the initial code after your report and you worked with it and tested it.
> 
> In that case having you as author (as you are submitting the patch here) and _both_ off you as sign offs would be correct imho.
>> 
>> ieee802154_is_secen checks if the 802.15.4 security bit is set in the
>> frame control field.
>> 
>> Signed-off-by: Alexander Aring <aar@pengutronix.de>
>> Reported-by: Alexandre Macabies <web+oss@zopieux.com>
>> Tested-by: Alexandre Macabies <web+oss@zopieux.com>
>> ---
>>  include/linux/ieee802154.h | 10 ++++++++++
>>  1 file changed, 10 insertions(+)
>> 
>> diff --git a/include/linux/ieee802154.h b/include/linux/ieee802154.h
>> index d3e4156..56090f1 100644
>> --- a/include/linux/ieee802154.h
>> +++ b/include/linux/ieee802154.h
>> @@ -218,6 +218,7 @@ enum {
>>  /* frame control handling */
>>  #define IEEE802154_FCTL_FTYPE		0x0003
>>  #define IEEE802154_FCTL_ACKREQ		0x0020
>> +#define IEEE802154_FCTL_SECEN		0x0004
>>  #define IEEE802154_FCTL_INTRA_PAN	0x0040
>>    #define IEEE802154_FTYPE_DATA		0x0001
>> @@ -233,6 +234,15 @@ static inline int ieee802154_is_data(__le16 fc)
>>  }
>>    /**
>> + * ieee802154_is_secen - check if Security bit is set
>> + * @fc: frame control bytes in little-endian byteorder
>> + */
>> +static inline bool ieee802154_is_secen(__le16 fc)
>> +{
>> +	return fc & cpu_to_le16(IEEE802154_FCTL_SECEN);
>> +}
>> +
>> +/**
>>   * ieee802154_is_ackreq - check if acknowledgment request bit is set
>>   * @fc: frame control bytes in little-endian byteorder
>>   */
> 
> Code looks fine. With the SOB fixed you get my:
> 
> Reviewed-by: Stefan Schmidt <stefan@osg.samsung.com>

can I expect a new series with this fixed?

Regards

Marcel

[PATCH v2 1/3] ieee802154: add security bit check function

[Alexandre Macabies]

ieee802154_is_secen checks if the 802.15.4 security bit is set in the
frame control field.

Signed-off-by: Alexander Aring <aar@pengutronix.de>
Signed-off-by: Alexandre Macabies <web+oss@zopieux.com>
Reviewed-by: Stefan Schmidt <stefan@osg.samsung.com>
---
 include/linux/ieee802154.h | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/include/linux/ieee802154.h b/include/linux/ieee802154.h
index d3e4156..56090f1 100644
--- a/include/linux/ieee802154.h
+++ b/include/linux/ieee802154.h
@@ -218,6 +218,7 @@ enum {
 /* frame control handling */
 #define IEEE802154_FCTL_FTYPE		0x0003
 #define IEEE802154_FCTL_ACKREQ		0x0020
+#define IEEE802154_FCTL_SECEN		0x0004
 #define IEEE802154_FCTL_INTRA_PAN	0x0040
 
 #define IEEE802154_FTYPE_DATA		0x0001
@@ -233,6 +234,15 @@ static inline int ieee802154_is_data(__le16 fc)
 }
 
 /**
+ * ieee802154_is_secen - check if Security bit is set
+ * @fc: frame control bytes in little-endian byteorder
+ */
+static inline bool ieee802154_is_secen(__le16 fc)
+{
+	return fc & cpu_to_le16(IEEE802154_FCTL_SECEN);
+}
+
+/**
  * ieee802154_is_ackreq - check if acknowledgment request bit is set
  * @fc: frame control bytes in little-endian byteorder
  */
-- 
2.7.3

[PATCH v2 2/3] mrf24j40: fix security-enabled processing on inbound frames

[Alexandre Macabies]

When receiving a security-enabled IEEE 802.15.4 frame, the MRF24J40
triggers a SECIF interrupt that needs to be handled for RX processing
to keep functioning properly.

This patch enables the SECIF interrupt and makes the MRF ignores all
hardware processing of security-enabled frames, that is handled by the
ieee802154 stack instead.

Signed-off-by: Alexander Aring <aar@pengutronix.de>
Signed-off-by: Alexandre Macabies <web+oss@zopieux.com>
Reviewed-by: Stefan Schmidt <stefan@osg.samsung.com>
---
 drivers/net/ieee802154/mrf24j40.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ieee802154/mrf24j40.c b/drivers/net/ieee802154/mrf24j40.c
index 764a2bd..adc67be 100644
--- a/drivers/net/ieee802154/mrf24j40.c
+++ b/drivers/net/ieee802154/mrf24j40.c
@@ -85,10 +85,13 @@
 #define REG_INTSTAT	0x31  /* Interrupt Status */
 #define BIT_TXNIF	BIT(0)
 #define BIT_RXIF	BIT(3)
+#define BIT_SECIF	BIT(4)
+#define BIT_SECIGNORE	BIT(7)
 
 #define REG_INTCON	0x32  /* Interrupt Control */
 #define BIT_TXNIE	BIT(0)
 #define BIT_RXIE	BIT(3)
+#define BIT_SECIE	BIT(4)
 
 #define REG_GPIO	0x33  /* GPIO */
 #define REG_TRISGPIO	0x34  /* GPIO direction */
@@ -616,7 +619,7 @@ static int mrf24j40_start(struct ieee802154_hw *hw)
 
 	/* Clear TXNIE and RXIE. Enable interrupts */
 	return regmap_update_bits(devrec->regmap_short, REG_INTCON,
-				  BIT_TXNIE | BIT_RXIE, 0);
+				  BIT_TXNIE | BIT_RXIE | BIT_SECIE, 0);
 }
 
 static void mrf24j40_stop(struct ieee802154_hw *hw)
@@ -1025,6 +1028,11 @@ static void mrf24j40_intstat_complete(void *context)
 
 	enable_irq(devrec->spi->irq);
 
+	/* Ignore Rx security decryption */
+	if (intstat & BIT_SECIF)
+		regmap_write_async(devrec->regmap_short, REG_SECCON0,
+				   BIT_SECIGNORE);
+
 	/* Check for TX complete */
 	if (intstat & BIT_TXNIF)
 		ieee802154_xmit_complete(devrec->hw, devrec->tx_skb, false);
-- 
2.7.3

Re: [PATCH v2 2/3] mrf24j40: fix security-enabled processing on inbound frames

[Stefan Schmidt]

Hello.

Added Alan Ott in CC.

regards
Stefan Schmidt

On 14/03/16 21:46, Alexandre Macabies wrote:
> When receiving a security-enabled IEEE 802.15.4 frame, the MRF24J40
> triggers a SECIF interrupt that needs to be handled for RX processing
> to keep functioning properly.
>
> This patch enables the SECIF interrupt and makes the MRF ignores all
> hardware processing of security-enabled frames, that is handled by the
> ieee802154 stack instead.
>
> Signed-off-by: Alexander Aring <aar@pengutronix.de>
> Signed-off-by: Alexandre Macabies <web+oss@zopieux.com>
> Reviewed-by: Stefan Schmidt <stefan@osg.samsung.com>
> ---
>   drivers/net/ieee802154/mrf24j40.c | 10 +++++++++-
>   1 file changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/ieee802154/mrf24j40.c b/drivers/net/ieee802154/mrf24j40.c
> index 764a2bd..adc67be 100644
> --- a/drivers/net/ieee802154/mrf24j40.c
> +++ b/drivers/net/ieee802154/mrf24j40.c
> @@ -85,10 +85,13 @@
>   #define REG_INTSTAT	0x31  /* Interrupt Status */
>   #define BIT_TXNIF	BIT(0)
>   #define BIT_RXIF	BIT(3)
> +#define BIT_SECIF	BIT(4)
> +#define BIT_SECIGNORE	BIT(7)
>   
>   #define REG_INTCON	0x32  /* Interrupt Control */
>   #define BIT_TXNIE	BIT(0)
>   #define BIT_RXIE	BIT(3)
> +#define BIT_SECIE	BIT(4)
>   
>   #define REG_GPIO	0x33  /* GPIO */
>   #define REG_TRISGPIO	0x34  /* GPIO direction */
> @@ -616,7 +619,7 @@ static int mrf24j40_start(struct ieee802154_hw *hw)
>   
>   	/* Clear TXNIE and RXIE. Enable interrupts */
>   	return regmap_update_bits(devrec->regmap_short, REG_INTCON,
> -				  BIT_TXNIE | BIT_RXIE, 0);
> +				  BIT_TXNIE | BIT_RXIE | BIT_SECIE, 0);
>   }
>   
>   static void mrf24j40_stop(struct ieee802154_hw *hw)
> @@ -1025,6 +1028,11 @@ static void mrf24j40_intstat_complete(void *context)
>   
>   	enable_irq(devrec->spi->irq);
>   
> +	/* Ignore Rx security decryption */
> +	if (intstat & BIT_SECIF)
> +		regmap_write_async(devrec->regmap_short, REG_SECCON0,
> +				   BIT_SECIGNORE);
> +
>   	/* Check for TX complete */
>   	if (intstat & BIT_TXNIF)
>   		ieee802154_xmit_complete(devrec->hw, devrec->tx_skb, false);

[PATCH v2 3/3] mrf24j40: apply the security-enabled bit on secured outbound frames

[Alexandre Macabies]

We set the TXNSECEN bit of register TXNCON to on when transmitting a
security-enabled frame, as described in section 3.12.2 of the MRF
datasheet.

Signed-off-by: Alexander Aring <aar@pengutronix.de>
Signed-off-by: Alexandre Macabies <web+oss@zopieux.com>
Reviewed-by: Stefan Schmidt <stefan@osg.samsung.com>
---
 drivers/net/ieee802154/mrf24j40.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/net/ieee802154/mrf24j40.c b/drivers/net/ieee802154/mrf24j40.c
index adc67be..f446db8 100644
--- a/drivers/net/ieee802154/mrf24j40.c
+++ b/drivers/net/ieee802154/mrf24j40.c
@@ -61,6 +61,7 @@
 #define REG_TXBCON0	0x1A
 #define REG_TXNCON	0x1B  /* Transmit Normal FIFO Control */
 #define BIT_TXNTRIG	BIT(0)
+#define BIT_TXNSECEN	BIT(1)
 #define BIT_TXNACKREQ	BIT(2)
 
 #define REG_TXG1CON	0x1C
@@ -551,6 +552,9 @@ static void write_tx_buf_complete(void *context)
 	u8 val = BIT_TXNTRIG;
 	int ret;
 
+	if (ieee802154_is_secen(fc))
+		val |= BIT_TXNSECEN;
+
 	if (ieee802154_is_ackreq(fc))
 		val |= BIT_TXNACKREQ;
 
-- 
2.7.3

Re: [PATCH v2 3/3] mrf24j40: apply the security-enabled bit on secured outbound frames

[Stefan Schmidt]

Hello.

Added Alan Ott in CC.

regards
Stefan Schmidt

On 14/03/16 21:46, Alexandre Macabies wrote:
> We set the TXNSECEN bit of register TXNCON to on when transmitting a
> security-enabled frame, as described in section 3.12.2 of the MRF
> datasheet.
>
> Signed-off-by: Alexander Aring <aar@pengutronix.de>
> Signed-off-by: Alexandre Macabies <web+oss@zopieux.com>
> Reviewed-by: Stefan Schmidt <stefan@osg.samsung.com>
> ---
>   drivers/net/ieee802154/mrf24j40.c | 4 ++++
>   1 file changed, 4 insertions(+)
>
> diff --git a/drivers/net/ieee802154/mrf24j40.c b/drivers/net/ieee802154/mrf24j40.c
> index adc67be..f446db8 100644
> --- a/drivers/net/ieee802154/mrf24j40.c
> +++ b/drivers/net/ieee802154/mrf24j40.c
> @@ -61,6 +61,7 @@
>   #define REG_TXBCON0	0x1A
>   #define REG_TXNCON	0x1B  /* Transmit Normal FIFO Control */
>   #define BIT_TXNTRIG	BIT(0)
> +#define BIT_TXNSECEN	BIT(1)
>   #define BIT_TXNACKREQ	BIT(2)
>   
>   #define REG_TXG1CON	0x1C
> @@ -551,6 +552,9 @@ static void write_tx_buf_complete(void *context)
>   	u8 val = BIT_TXNTRIG;
>   	int ret;
>   
> +	if (ieee802154_is_secen(fc))
> +		val |= BIT_TXNSECEN;
> +
>   	if (ieee802154_is_ackreq(fc))
>   		val |= BIT_TXNACKREQ;
>   

Re: [PATCH v2 1/3] ieee802154: add security bit check function

[Stefan Schmidt]

Hello.

Added Alan Ott in CC.

Alan, can we get your review on this? Its sit son the list for almost 
three weeks now.
If you are to busy just say so and we can get the ACK from Alex and put 
them in.

regards
Stefan Schmidt

On 14/03/16 21:46, Alexandre Macabies wrote:
> ieee802154_is_secen checks if the 802.15.4 security bit is set in the
> frame control field.
>
> Signed-off-by: Alexander Aring <aar@pengutronix.de>
> Signed-off-by: Alexandre Macabies <web+oss@zopieux.com>
> Reviewed-by: Stefan Schmidt <stefan@osg.samsung.com>
> ---
>   include/linux/ieee802154.h | 10 ++++++++++
>   1 file changed, 10 insertions(+)
>
> diff --git a/include/linux/ieee802154.h b/include/linux/ieee802154.h
> index d3e4156..56090f1 100644
> --- a/include/linux/ieee802154.h
> +++ b/include/linux/ieee802154.h
> @@ -218,6 +218,7 @@ enum {
>   /* frame control handling */
>   #define IEEE802154_FCTL_FTYPE		0x0003
>   #define IEEE802154_FCTL_ACKREQ		0x0020
> +#define IEEE802154_FCTL_SECEN		0x0004
>   #define IEEE802154_FCTL_INTRA_PAN	0x0040
>   
>   #define IEEE802154_FTYPE_DATA		0x0001
> @@ -233,6 +234,15 @@ static inline int ieee802154_is_data(__le16 fc)
>   }
>   
>   /**
> + * ieee802154_is_secen - check if Security bit is set
> + * @fc: frame control bytes in little-endian byteorder
> + */
> +static inline bool ieee802154_is_secen(__le16 fc)
> +{
> +	return fc & cpu_to_le16(IEEE802154_FCTL_SECEN);
> +}
> +
> +/**
>    * ieee802154_is_ackreq - check if acknowledgment request bit is set
>    * @fc: frame control bytes in little-endian byteorder
>    */

Re: [PATCH v2 1/3] ieee802154: add security bit check function

[Alan Ott]

On 03/30/2016 04:45 AM, Stefan Schmidt wrote:
> Added Alan Ott in CC.
>
> Alan, can we get your review on this? Its sit son the list for almost 
> three weeks now.
> If you are to busy just say so and we can get the ACK from Alex and 
> put them in.
>

Hi Stefan,

Sorry I've been slow on this. Thanks for forwarding. I actually went 
looking for v2 of this the other day in my inbox and it wasn't there.

Alexandre, make sure you use scripts/get_maintainer.pl to get the list 
of people you should send patches to. It looks like you put me on v1 but 
left me off v2.

For all three:
Acked-by: Alan Ott <alan@signal11.us>

Alan.

>
> On 14/03/16 21:46, Alexandre Macabies wrote:
>> ieee802154_is_secen checks if the 802.15.4 security bit is set in the
>> frame control field.
>>
>> Signed-off-by: Alexander Aring <aar@pengutronix.de>
>> Signed-off-by: Alexandre Macabies <web+oss@zopieux.com>
>> Reviewed-by: Stefan Schmidt <stefan@osg.samsung.com>
>> ---
>>   include/linux/ieee802154.h | 10 ++++++++++
>>   1 file changed, 10 insertions(+)
>>
>> diff --git a/include/linux/ieee802154.h b/include/linux/ieee802154.h
>> index d3e4156..56090f1 100644
>> --- a/include/linux/ieee802154.h
>> +++ b/include/linux/ieee802154.h
>> @@ -218,6 +218,7 @@ enum {
>>   /* frame control handling */
>>   #define IEEE802154_FCTL_FTYPE        0x0003
>>   #define IEEE802154_FCTL_ACKREQ        0x0020
>> +#define IEEE802154_FCTL_SECEN        0x0004
>>   #define IEEE802154_FCTL_INTRA_PAN    0x0040
>>     #define IEEE802154_FTYPE_DATA        0x0001
>> @@ -233,6 +234,15 @@ static inline int ieee802154_is_data(__le16 fc)
>>   }
>>     /**
>> + * ieee802154_is_secen - check if Security bit is set
>> + * @fc: frame control bytes in little-endian byteorder
>> + */
>> +static inline bool ieee802154_is_secen(__le16 fc)
>> +{
>> +    return fc & cpu_to_le16(IEEE802154_FCTL_SECEN);
>> +}
>> +
>> +/**
>>    * ieee802154_is_ackreq - check if acknowledgment request bit is set
>>    * @fc: frame control bytes in little-endian byteorder
>>    */
>

Re: [PATCH v2 1/3] ieee802154: add security bit check function

[Stefan Schmidt]

Hello.

On 30/03/16 16:05, Alan Ott wrote:
> On 03/30/2016 04:45 AM, Stefan Schmidt wrote:
>> Added Alan Ott in CC.
>>
>> Alan, can we get your review on this? Its sit son the list for almost 
>> three weeks now.
>> If you are to busy just say so and we can get the ACK from Alex and 
>> put them in.
>>
>
> Hi Stefan,
>
> Sorry I've been slow on this. Thanks for forwarding. I actually went 
> looking for v2 of this the other day in my inbox and it wasn't there.

Sorry for pushing this but I wanted to get these patches in a state 
where they can get applied. With my review and your ack they should be 
fine now.

>
> Alexandre, make sure you use scripts/get_maintainer.pl to get the list 
> of people you should send patches to. It looks like you put me on v1 
> but left me off v2.
>

Good point!

> For all three:
> Acked-by: Alan Ott <alan@signal11.us>
>

regards
Stefan Schmidt

> Alan.
>
>>
>> On 14/03/16 21:46, Alexandre Macabies wrote:
>>> ieee802154_is_secen checks if the 802.15.4 security bit is set in the
>>> frame control field.
>>>
>>> Signed-off-by: Alexander Aring <aar@pengutronix.de>
>>> Signed-off-by: Alexandre Macabies <web+oss@zopieux.com>
>>> Reviewed-by: Stefan Schmidt <stefan@osg.samsung.com>
>>> ---
>>>   include/linux/ieee802154.h | 10 ++++++++++
>>>   1 file changed, 10 insertions(+)
>>>
>>> diff --git a/include/linux/ieee802154.h b/include/linux/ieee802154.h
>>> index d3e4156..56090f1 100644
>>> --- a/include/linux/ieee802154.h
>>> +++ b/include/linux/ieee802154.h
>>> @@ -218,6 +218,7 @@ enum {
>>>   /* frame control handling */
>>>   #define IEEE802154_FCTL_FTYPE        0x0003
>>>   #define IEEE802154_FCTL_ACKREQ        0x0020
>>> +#define IEEE802154_FCTL_SECEN        0x0004
>>>   #define IEEE802154_FCTL_INTRA_PAN    0x0040
>>>     #define IEEE802154_FTYPE_DATA        0x0001
>>> @@ -233,6 +234,15 @@ static inline int ieee802154_is_data(__le16 fc)
>>>   }
>>>     /**
>>> + * ieee802154_is_secen - check if Security bit is set
>>> + * @fc: frame control bytes in little-endian byteorder
>>> + */
>>> +static inline bool ieee802154_is_secen(__le16 fc)
>>> +{
>>> +    return fc & cpu_to_le16(IEEE802154_FCTL_SECEN);
>>> +}
>>> +
>>> +/**
>>>    * ieee802154_is_ackreq - check if acknowledgment request bit is set
>>>    * @fc: frame control bytes in little-endian byteorder
>>>    */
>>
>

Re: [PATCH v2 1/3] ieee802154: add security bit check function

[Marcel Holtmann]

Hi Stefan,

>>> Alan, can we get your review on this? Its sit son the list for almost three weeks now.
>>> If you are to busy just say so and we can get the ACK from Alex and put them in.
>>> 
>> 
>> Hi Stefan,
>> 
>> Sorry I've been slow on this. Thanks for forwarding. I actually went looking for v2 of this the other day in my inbox and it wasn't there.
> 
> Sorry for pushing this but I wanted to get these patches in a state where they can get applied. With my review and your ack they should be fine now.

can you please resend these ones with the ack included. I seem to have lost the original thread.

Regards

Marcel

[PATCH v3 1/3] ieee802154: add security bit check function

[Alexandre Macabies]

ieee802154_is_secen checks if the 802.15.4 security bit is set in the
frame control field.

Signed-off-by: Alexander Aring <aar@pengutronix.de>
Signed-off-by: Alexandre Macabies <web+oss@zopieux.com>
Reviewed-by: Stefan Schmidt <stefan@osg.samsung.com>
Acked-by: Alan Ott <alan@signal11.us>
---
 include/linux/ieee802154.h | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/include/linux/ieee802154.h b/include/linux/ieee802154.h
index d3e4156..56090f1 100644
--- a/include/linux/ieee802154.h
+++ b/include/linux/ieee802154.h
@@ -218,6 +218,7 @@ enum {
 /* frame control handling */
 #define IEEE802154_FCTL_FTYPE		0x0003
 #define IEEE802154_FCTL_ACKREQ		0x0020
+#define IEEE802154_FCTL_SECEN		0x0004
 #define IEEE802154_FCTL_INTRA_PAN	0x0040
 
 #define IEEE802154_FTYPE_DATA		0x0001
@@ -233,6 +234,15 @@ static inline int ieee802154_is_data(__le16 fc)
 }
 
 /**
+ * ieee802154_is_secen - check if Security bit is set
+ * @fc: frame control bytes in little-endian byteorder
+ */
+static inline bool ieee802154_is_secen(__le16 fc)
+{
+	return fc & cpu_to_le16(IEEE802154_FCTL_SECEN);
+}
+
+/**
  * ieee802154_is_ackreq - check if acknowledgment request bit is set
  * @fc: frame control bytes in little-endian byteorder
  */
-- 
2.7.3

[PATCH v3 2/3] mrf24j40: fix security-enabled processing on inbound frames

[Alexandre Macabies]

When receiving a security-enabled IEEE 802.15.4 frame, the MRF24J40
triggers a SECIF interrupt that needs to be handled for RX processing
to keep functioning properly.

This patch enables the SECIF interrupt and makes the MRF ignores all
hardware processing of security-enabled frames, that is handled by the
ieee802154 stack instead.

Signed-off-by: Alexander Aring <aar@pengutronix.de>
Signed-off-by: Alexandre Macabies <web+oss@zopieux.com>
Reviewed-by: Stefan Schmidt <stefan@osg.samsung.com>
Acked-by: Alan Ott <alan@signal11.us>
---
 drivers/net/ieee802154/mrf24j40.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ieee802154/mrf24j40.c b/drivers/net/ieee802154/mrf24j40.c
index 764a2bd..adc67be 100644
--- a/drivers/net/ieee802154/mrf24j40.c
+++ b/drivers/net/ieee802154/mrf24j40.c
@@ -85,10 +85,13 @@
 #define REG_INTSTAT	0x31  /* Interrupt Status */
 #define BIT_TXNIF	BIT(0)
 #define BIT_RXIF	BIT(3)
+#define BIT_SECIF	BIT(4)
+#define BIT_SECIGNORE	BIT(7)
 
 #define REG_INTCON	0x32  /* Interrupt Control */
 #define BIT_TXNIE	BIT(0)
 #define BIT_RXIE	BIT(3)
+#define BIT_SECIE	BIT(4)
 
 #define REG_GPIO	0x33  /* GPIO */
 #define REG_TRISGPIO	0x34  /* GPIO direction */
@@ -616,7 +619,7 @@ static int mrf24j40_start(struct ieee802154_hw *hw)
 
 	/* Clear TXNIE and RXIE. Enable interrupts */
 	return regmap_update_bits(devrec->regmap_short, REG_INTCON,
-				  BIT_TXNIE | BIT_RXIE, 0);
+				  BIT_TXNIE | BIT_RXIE | BIT_SECIE, 0);
 }
 
 static void mrf24j40_stop(struct ieee802154_hw *hw)
@@ -1025,6 +1028,11 @@ static void mrf24j40_intstat_complete(void *context)
 
 	enable_irq(devrec->spi->irq);
 
+	/* Ignore Rx security decryption */
+	if (intstat & BIT_SECIF)
+		regmap_write_async(devrec->regmap_short, REG_SECCON0,
+				   BIT_SECIGNORE);
+
 	/* Check for TX complete */
 	if (intstat & BIT_TXNIF)
 		ieee802154_xmit_complete(devrec->hw, devrec->tx_skb, false);
-- 
2.7.3

Re: [PATCH v3 2/3] mrf24j40: fix security-enabled processing on inbound frames

[Marcel Holtmann]

Hi Alexandre,

> When receiving a security-enabled IEEE 802.15.4 frame, the MRF24J40
> triggers a SECIF interrupt that needs to be handled for RX processing
> to keep functioning properly.
> 
> This patch enables the SECIF interrupt and makes the MRF ignores all
> hardware processing of security-enabled frames, that is handled by the
> ieee802154 stack instead.
> 
> Signed-off-by: Alexander Aring <aar@pengutronix.de>
> Signed-off-by: Alexandre Macabies <web+oss@zopieux.com>
> Reviewed-by: Stefan Schmidt <stefan@osg.samsung.com>
> Acked-by: Alan Ott <alan@signal11.us>
> ---
> drivers/net/ieee802154/mrf24j40.c | 10 +++++++++-
> 1 file changed, 9 insertions(+), 1 deletion(-)

patch has been applied to bluetooth-next tree.

Regards

Marcel

[PATCH v3 3/3] mrf24j40: apply the security-enabled bit on secured outbound frames

[Alexandre Macabies]

We set the TXNSECEN bit of register TXNCON to on when transmitting a
security-enabled frame, as described in section 3.12.2 of the MRF
datasheet.

Signed-off-by: Alexander Aring <aar@pengutronix.de>
Signed-off-by: Alexandre Macabies <web+oss@zopieux.com>
Reviewed-by: Stefan Schmidt <stefan@osg.samsung.com>
Acked-by: Alan Ott <alan@signal11.us>
---
 drivers/net/ieee802154/mrf24j40.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/net/ieee802154/mrf24j40.c b/drivers/net/ieee802154/mrf24j40.c
index adc67be..f446db8 100644
--- a/drivers/net/ieee802154/mrf24j40.c
+++ b/drivers/net/ieee802154/mrf24j40.c
@@ -61,6 +61,7 @@
 #define REG_TXBCON0	0x1A
 #define REG_TXNCON	0x1B  /* Transmit Normal FIFO Control */
 #define BIT_TXNTRIG	BIT(0)
+#define BIT_TXNSECEN	BIT(1)
 #define BIT_TXNACKREQ	BIT(2)
 
 #define REG_TXG1CON	0x1C
@@ -551,6 +552,9 @@ static void write_tx_buf_complete(void *context)
 	u8 val = BIT_TXNTRIG;
 	int ret;
 
+	if (ieee802154_is_secen(fc))
+		val |= BIT_TXNSECEN;
+
 	if (ieee802154_is_ackreq(fc))
 		val |= BIT_TXNACKREQ;
 
-- 
2.7.3

Re: [PATCH v3 3/3] mrf24j40: apply the security-enabled bit on secured outbound frames

[Marcel Holtmann]

Hi Alexandre,

> We set the TXNSECEN bit of register TXNCON to on when transmitting a
> security-enabled frame, as described in section 3.12.2 of the MRF
> datasheet.
> 
> Signed-off-by: Alexander Aring <aar@pengutronix.de>
> Signed-off-by: Alexandre Macabies <web+oss@zopieux.com>
> Reviewed-by: Stefan Schmidt <stefan@osg.samsung.com>
> Acked-by: Alan Ott <alan@signal11.us>
> ---
> drivers/net/ieee802154/mrf24j40.c | 4 ++++
> 1 file changed, 4 insertions(+)

patch has been applied to bluetooth-next tree.

Regards

Marcel

Re: [PATCH v3 1/3] ieee802154: add security bit check function

[Marcel Holtmann]

Hi Alexandre,

> ieee802154_is_secen checks if the 802.15.4 security bit is set in the
> frame control field.
> 
> Signed-off-by: Alexander Aring <aar@pengutronix.de>
> Signed-off-by: Alexandre Macabies <web+oss@zopieux.com>
> Reviewed-by: Stefan Schmidt <stefan@osg.samsung.com>
> Acked-by: Alan Ott <alan@signal11.us>
> ---
> include/linux/ieee802154.h | 10 ++++++++++
> 1 file changed, 10 insertions(+)

patch has been applied to bluetooth-next tree.

Regards

Marcel