[PATCH bluetooth-next 0/2] 6lowpan: iphc: cleanups

[PATCH bluetooth-next 0/2] 6lowpan: iphc: cleanups

[Alexander Aring]

Hi,

These two patches are from the current RFC of stateful compression support.
It has nothing todo with stateful compression, I send it now as PATCH so
it can be applied before.

- Alex

Alexander Aring (2):
  6lowpan: iphc: add check for reserved values
  6lowpan: iphc: remove handling when dam is zero

 net/6lowpan/iphc.c | 20 +++++++++++++++++---
 1 file changed, 17 insertions(+), 3 deletions(-)

-- 
2.6.1

[PATCH bluetooth-next 1/2] 6lowpan: iphc: add check for reserved values

[Alexander Aring]

This patch adds a check on reserved values for IPHC header. We should at
first check on these fields instead of doing parsing before. Afterwards
we can be sure there are no reserved values anymore. The reserved bits
doesn't contain reserved values for NHC headers. This need to be handled
inside the next layer.

Signed-off-by: Alexander Aring <alex.aring@gmail.com>
---
 net/6lowpan/iphc.c | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/net/6lowpan/iphc.c b/net/6lowpan/iphc.c
index 346b5c1..13f5424 100644
--- a/net/6lowpan/iphc.c
+++ b/net/6lowpan/iphc.c
@@ -455,6 +455,20 @@ static const u8 lowpan_ttl_values[] = {
 	[LOWPAN_IPHC_HLIM_11] = 255,
 };
 
+static inline bool lowpan_iphc_is_reserved(u8 iphc1)
+{
+	switch (iphc1 & (LOWPAN_IPHC_DAC | LOWPAN_IPHC_M |
+			 LOWPAN_IPHC_DAM_MASK)) {
+	case LOWPAN_IPHC_DAC | LOWPAN_IPHC_DAM_00:
+	case LOWPAN_IPHC_DAC | LOWPAN_IPHC_M | LOWPAN_IPHC_DAM_01:
+	case LOWPAN_IPHC_DAC | LOWPAN_IPHC_M | LOWPAN_IPHC_DAM_10:
+	case LOWPAN_IPHC_DAC | LOWPAN_IPHC_M | LOWPAN_IPHC_DAM_11:
+		return true;
+	default:
+		return false;
+	}
+}
+
 int lowpan_header_decompress(struct sk_buff *skb, const struct net_device *dev,
 			     const void *daddr, const void *saddr)
 {
@@ -466,7 +480,8 @@ int lowpan_header_decompress(struct sk_buff *skb, const struct net_device *dev,
 		       skb->data, skb->len);
 
 	if (lowpan_fetch_skb(skb, &iphc0, sizeof(iphc0)) ||
-	    lowpan_fetch_skb(skb, &iphc1, sizeof(iphc1)))
+	    lowpan_fetch_skb(skb, &iphc1, sizeof(iphc1)) ||
+	    lowpan_iphc_is_reserved(iphc1))
 		return -EINVAL;
 
 	/* another if the CID flag is set */
-- 
2.6.1

Re: [PATCH bluetooth-next 1/2] 6lowpan: iphc: add check for reserved values

[Stefan Schmidt]

Hello.

On 17/11/15 19:54, Alexander Aring wrote:
> This patch adds a check on reserved values for IPHC header. We should at
> first check on these fields instead of doing parsing before. Afterwards
> we can be sure there are no reserved values anymore. The reserved bits
> doesn't contain reserved values for NHC headers. This need to be handled
> inside the next layer.
>
> Signed-off-by: Alexander Aring <alex.aring@gmail.com>
> ---
>   net/6lowpan/iphc.c | 17 ++++++++++++++++-
>   1 file changed, 16 insertions(+), 1 deletion(-)
>
> diff --git a/net/6lowpan/iphc.c b/net/6lowpan/iphc.c
> index 346b5c1..13f5424 100644
> --- a/net/6lowpan/iphc.c
> +++ b/net/6lowpan/iphc.c
> @@ -455,6 +455,20 @@ static const u8 lowpan_ttl_values[] = {
>   	[LOWPAN_IPHC_HLIM_11] = 255,
>   };
>   
> +static inline bool lowpan_iphc_is_reserved(u8 iphc1)
> +{
> +	switch (iphc1 & (LOWPAN_IPHC_DAC | LOWPAN_IPHC_M |
> +			 LOWPAN_IPHC_DAM_MASK)) {
> +	case LOWPAN_IPHC_DAC | LOWPAN_IPHC_DAM_00:
> +	case LOWPAN_IPHC_DAC | LOWPAN_IPHC_M | LOWPAN_IPHC_DAM_01:
> +	case LOWPAN_IPHC_DAC | LOWPAN_IPHC_M | LOWPAN_IPHC_DAM_10:
> +	case LOWPAN_IPHC_DAC | LOWPAN_IPHC_M | LOWPAN_IPHC_DAM_11:
> +		return true;
> +	default:
> +		return false;
> +	}
> +}
> +
>   int lowpan_header_decompress(struct sk_buff *skb, const struct net_device *dev,
>   			     const void *daddr, const void *saddr)
>   {
> @@ -466,7 +480,8 @@ int lowpan_header_decompress(struct sk_buff *skb, const struct net_device *dev,
>   		       skb->data, skb->len);
>   
>   	if (lowpan_fetch_skb(skb, &iphc0, sizeof(iphc0)) ||
> -	    lowpan_fetch_skb(skb, &iphc1, sizeof(iphc1)))
> +	    lowpan_fetch_skb(skb, &iphc1, sizeof(iphc1)) ||
> +	    lowpan_iphc_is_reserved(iphc1))
>   		return -EINVAL;
>   
>   	/* another if the CID flag is set */

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

regards
Stefan Schmidt

[PATCH bluetooth-next 2/2] 6lowpan: iphc: remove handling when dam is zero

[Alexander Aring]

This patch removes handling of LOWPAN_IPHC_DAM_00 inside the
lowpan_compress_addr_64 function. The case of LOWPAN_IPHC_DAM_00 can
never occur. there exists a if branch which use LOWPAN_IPHC_DAM_11 and
LOWPAN_IPHC_DAM_10 and inside the else branch LOWPAN_IPHC_DAM_01. So
LOWPAN_IPHC_DAM_00 can never occur.

Signed-off-by: Alexander Aring <alex.aring@gmail.com>
---
 net/6lowpan/iphc.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/net/6lowpan/iphc.c b/net/6lowpan/iphc.c
index 13f5424..f7f175c 100644
--- a/net/6lowpan/iphc.c
+++ b/net/6lowpan/iphc.c
@@ -594,7 +594,6 @@ int lowpan_header_decompress(struct sk_buff *skb, const struct net_device *dev,
 EXPORT_SYMBOL_GPL(lowpan_header_decompress);
 
 static const u8 lowpan_iphc_dam_to_sam_value[] = {
-	[LOWPAN_IPHC_DAM_00] = LOWPAN_IPHC_SAM_00,
 	[LOWPAN_IPHC_DAM_01] = LOWPAN_IPHC_SAM_01,
 	[LOWPAN_IPHC_DAM_10] = LOWPAN_IPHC_SAM_10,
 	[LOWPAN_IPHC_DAM_11] = LOWPAN_IPHC_SAM_11,
@@ -603,7 +602,7 @@ static const u8 lowpan_iphc_dam_to_sam_value[] = {
 static u8 lowpan_compress_addr_64(u8 **hc_ptr, const struct in6_addr *ipaddr,
 				  const unsigned char *lladdr, bool sam)
 {
-	u8 dam = LOWPAN_IPHC_DAM_00;
+	u8 dam;
 
 	if (is_addr_mac_addr_based(ipaddr, lladdr)) {
 		dam = LOWPAN_IPHC_DAM_11; /* 0-bits */
-- 
2.6.1

Re: [PATCH bluetooth-next 2/2] 6lowpan: iphc: remove handling when dam is zero

[Stefan Schmidt]

Hello.

On 17/11/15 19:54, Alexander Aring wrote:
> This patch removes handling of LOWPAN_IPHC_DAM_00 inside the
> lowpan_compress_addr_64 function. The case of LOWPAN_IPHC_DAM_00 can
> never occur. there exists a if branch which use LOWPAN_IPHC_DAM_11 and
> LOWPAN_IPHC_DAM_10 and inside the else branch LOWPAN_IPHC_DAM_01. So
> LOWPAN_IPHC_DAM_00 can never occur.

Agreed the DAM_00 case means no compressions and carry the 128 bit 
address in line. We never call lowpan_compress_addr_64() for this case 
so removing this case which is never used is fine.
> Signed-off-by: Alexander Aring <alex.aring@gmail.com>
> ---
>   net/6lowpan/iphc.c | 3 +--
>   1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/net/6lowpan/iphc.c b/net/6lowpan/iphc.c
> index 13f5424..f7f175c 100644
> --- a/net/6lowpan/iphc.c
> +++ b/net/6lowpan/iphc.c
> @@ -594,7 +594,6 @@ int lowpan_header_decompress(struct sk_buff *skb, const struct net_device *dev,
>   EXPORT_SYMBOL_GPL(lowpan_header_decompress);
>   
>   static const u8 lowpan_iphc_dam_to_sam_value[] = {
> -	[LOWPAN_IPHC_DAM_00] = LOWPAN_IPHC_SAM_00,
>   	[LOWPAN_IPHC_DAM_01] = LOWPAN_IPHC_SAM_01,
>   	[LOWPAN_IPHC_DAM_10] = LOWPAN_IPHC_SAM_10,
>   	[LOWPAN_IPHC_DAM_11] = LOWPAN_IPHC_SAM_11,
> @@ -603,7 +602,7 @@ static const u8 lowpan_iphc_dam_to_sam_value[] = {
>   static u8 lowpan_compress_addr_64(u8 **hc_ptr, const struct in6_addr *ipaddr,
>   				  const unsigned char *lladdr, bool sam)
>   {
> -	u8 dam = LOWPAN_IPHC_DAM_00;
> +	u8 dam;
>   
>   	if (is_addr_mac_addr_based(ipaddr, lladdr)) {
>   		dam = LOWPAN_IPHC_DAM_11; /* 0-bits */

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

regards
Stefan Schmidt

Re: [PATCH bluetooth-next 0/2] 6lowpan: iphc: cleanups

[Alexander Aring]

On Tue, Nov 17, 2015 at 07:53:59PM +0100, Alexander Aring wrote:
> Hi,
> 
> These two patches are from the current RFC of stateful compression support.
> It has nothing todo with stateful compression, I send it now as PATCH so
> it can be applied before.
> 

ping :-)

- Alex