Hi Martin,

I agree about the issue, we talked a lot time before about this. But I
would not complete remove the errno numbers.

The basic issue is that that mixed function can return errno or
NET_RX_DROP/NET_RX_SUCCESS and we need to decide if we use
NET_RX_DROP/NET_RX_SUCCESS only or using errno's but then we need a
conversion of the errno to NET_RX_DROP at the end of receive call, If
a errno was returned.

If all functions return "-1" now, it's hard to debug what could be
failed, when we add some debug prints in this file.

On Wed, Jul 30, 2014 at 04:25:23PM +0100, Martin Townsend wrote:
> Currently it is up to the functions below lowpan_rcv to free the skb on error
> conditions.  This patch now removes all the UAPI error codes and process data
> now returns -1 if there is a problem.  In this scenario lowpan_rcv will free
> the skb and return NET_RX_DROP.  This also fixes the problem where
> NET_RX_SUCCESS is returned on error
> 
> Signed-off-by: Martin Townsend <martin.towns...@xsilon.com>
> ---
>  include/net/6lowpan.h         |  2 +-
>  net/6lowpan/iphc.c            | 35 ++++++++++++++++++-----------------
>  net/bluetooth/6lowpan.c       | 17 ++++++++---------
>  net/ieee802154/6lowpan_rtnl.c | 39 +++++++++++++++++++--------------------
>  4 files changed, 46 insertions(+), 47 deletions(-)
> 
> diff --git a/include/net/6lowpan.h b/include/net/6lowpan.h
> index 995cce86..561defe 100644
> --- a/include/net/6lowpan.h
> +++ b/include/net/6lowpan.h
> @@ -424,7 +424,7 @@ lowpan_uncompress_size(const struct sk_buff *skb, u16 
> *dgram_offset)
>  
>  typedef int (*skb_delivery_cb)(struct sk_buff *skb);
>  
> -int lowpan_process_data(struct sk_buff *skb, struct net_device *dev,
> +int lowpan_process_data(struct sk_buff **skb_inout, struct net_device *dev,
>               const u8 *saddr, const u8 saddr_type, const u8 saddr_len,
>               const u8 *daddr, const u8 daddr_type, const u8 daddr_len,
>               u8 iphc0, u8 iphc1, skb_delivery_cb skb_deliver);
> diff --git a/net/6lowpan/iphc.c b/net/6lowpan/iphc.c
> index b4bb27c..61b5206 100644
> --- a/net/6lowpan/iphc.c
> +++ b/net/6lowpan/iphc.c
> @@ -119,17 +119,17 @@ static int uncompress_addr(struct sk_buff *skb,
>                       break;
>               default:
>                       pr_debug("Invalid addr_type set\n");
> -                     return -EINVAL;
> +                     return -1;
>               }
>               break;
>       default:
>               pr_debug("Invalid address mode value: 0x%x\n", address_mode);
> -             return -EINVAL;
> +             return -1;
>       }
>  
>       if (fail) {
>               pr_debug("Failed to fetch skb data\n");
> -             return -EIO;
> +             return -1;
>       }
>  
>       raw_dump_inline(NULL, "Reconstructed ipv6 addr is",
> @@ -158,10 +158,10 @@ static int uncompress_context_based_src_addr(struct 
> sk_buff *skb,
>       case LOWPAN_IPHC_ADDR_03:
>               /* TODO */
>               netdev_warn(skb->dev, "SAM value 0x%x not supported\n", sam);
> -             return -EINVAL;
> +             return -1;
>       default:
>               pr_debug("Invalid sam value: 0x%x\n", sam);
> -             return -EINVAL;
> +             return -1;
>       }
>  
>       raw_dump_inline(NULL,
> @@ -179,10 +179,10 @@ static int skb_deliver(struct sk_buff *skb, struct 
> ipv6hdr *hdr,
>  
>       new = skb_copy_expand(skb, sizeof(struct ipv6hdr),
>                             skb_tailroom(skb), GFP_ATOMIC);
> -     kfree_skb(skb);
> -
>       if (!new)
> -             return -ENOMEM;
> +             return -1;
> +
> +     kfree_skb(skb);

This leaks memory here, move kfree_skb above the condition after the
skb_copy_expand. This should be also a consume_skb or dev_kfree_skb, we
should not mix these function in this file and use only one of them.

We need another patch for this.

>  
>       skb_push(new, sizeof(struct ipv6hdr));
>       skb_reset_network_header(new);
> @@ -196,6 +196,8 @@ static int skb_deliver(struct sk_buff *skb, struct 
> ipv6hdr *hdr,
>                      new->data, new->len);
>  
>       stat = deliver_skb(new);
> +     if (stat == -1)

keep all existing errnos and look for function which are called but
returning NET_RX_DROP. Then check for if (stat < 0) to indicate a error.

- Alex

------------------------------------------------------------------------------
Infragistics Professional
Build stunning WinForms apps today!
Reboot your WinForms applications with our WinForms controls. 
Build a bridge from your legacy apps to the future.
http://pubads.g.doubleclick.net/gampad/clk?id=153845071&iu=/4140/ostg.clktrk
_______________________________________________
Linux-zigbee-devel mailing list
Linux-zigbee-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-zigbee-devel

Reply via email to