Sorry should have said this should be applied against v3.15 version of 
linux-net kernel.

-Martin
On 10/07/14 16:49, Martin Townsend wrote:
> Alex,
>
> I've bitten the bullet and had an attempt at refactoring the receive path of 
> lowpan to
> 1)Remove all use of error codes like -EINVAL as these are really part of the 
> UAPI and we are not propogating error codes to user space.
> 2)Ensure skb is freed in one place at the top of the call stack, ie lowpan_rcv
> 3)Removed unused parameter in lowpan_give_skb_to_devices
> 4)Only call lowpan_give_skb_to_devices from lowpan_rcv, this means pasing the 
> ipv6_hdr down the call chain
> 5)Refactored the dispatch processing a bit to reduce code duplication.
> 6)lowpan_frag_rcv now returns <0 for errors, 0 for more fragments, and 1 for 
> all fragments received to aid lowpan_rcv
>
>
> Sorry for the large patch file, I'm a bit pushed for time and wanted to get 
> some feedback first.
> I'll roll out a proper patch series if it's acceptable.
>
> -Martin
>
>
> ---
>  include/net/6lowpan.h         |  4 +-
>  net/ieee802154/6lowpan_iphc.c | 91 
> ++++++++++++++-----------------------------
>  net/ieee802154/6lowpan_rtnl.c | 86 +++++++++++++++++++++++++---------------
>  net/ieee802154/reassembly.c   |  4 +-
>  4 files changed, 87 insertions(+), 98 deletions(-)
>
> diff --git a/include/net/6lowpan.h b/include/net/6lowpan.h
> index f7d372b..f187b64 100644
> --- a/include/net/6lowpan.h
> +++ b/include/net/6lowpan.h
> @@ -423,10 +423,10 @@ lowpan_uncompress_size(const struct sk_buff *skb, u16 
> *dgram_offset)
>  
>  typedef int (*skb_delivery_cb)(struct sk_buff *skb, struct net_device *dev);
>  
> -int lowpan_process_data(struct sk_buff *skb, struct net_device *dev,
> +int lowpan_process_data(struct sk_buff *skb, struct ipv6hdr *hdr,
>               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);
> +             u8 iphc0, u8 iphc1);
>  int lowpan_header_compress(struct sk_buff *skb, struct net_device *dev,
>                       unsigned short type, const void *_daddr,
>                       const void *_saddr, unsigned int len);
> diff --git a/net/ieee802154/6lowpan_iphc.c b/net/ieee802154/6lowpan_iphc.c
> index 211b568..00eb15b 100644
> --- a/net/ieee802154/6lowpan_iphc.c
> +++ b/net/ieee802154/6lowpan_iphc.c
> @@ -174,37 +174,6 @@ static int uncompress_context_based_src_addr(struct 
> sk_buff *skb,
>       return 0;
>  }
>  
> -static int skb_deliver(struct sk_buff *skb, struct ipv6hdr *hdr,
> -             struct net_device *dev, skb_delivery_cb deliver_skb)
> -{
> -     struct sk_buff *new;
> -     int stat;
> -
> -     new = skb_copy_expand(skb, sizeof(struct ipv6hdr), skb_tailroom(skb),
> -                                                             GFP_ATOMIC);
> -     kfree_skb(skb);
> -
> -     if (!new)
> -             return -ENOMEM;
> -
> -     skb_push(new, sizeof(struct ipv6hdr));
> -     skb_reset_network_header(new);
> -     skb_copy_to_linear_data(new, hdr, sizeof(struct ipv6hdr));
> -
> -     new->protocol = htons(ETH_P_IPV6);
> -     new->pkt_type = PACKET_HOST;
> -     new->dev = dev;
> -
> -     raw_dump_table(__func__, "raw skb data dump before receiving",
> -                     new->data, new->len);
> -
> -     stat = deliver_skb(new, dev);
> -
> -     kfree_skb(new);
> -
> -     return stat;
> -}
> -
>  /* Uncompress function for multicast destination address,
>   * when M bit is set.
>   */
> @@ -337,12 +306,11 @@ err:
>  /* TTL uncompression values */
>  static const u8 lowpan_ttl_values[] = { 0, 1, 64, 255 };
>  
> -int lowpan_process_data(struct sk_buff *skb, struct net_device *dev,
> +int lowpan_process_data(struct sk_buff *skb, struct ipv6hdr *hdr,
>               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 deliver_skb)
> +             u8 iphc0, u8 iphc1)
>  {
> -     struct ipv6hdr hdr = {};
>       u8 tmp, num_context = 0;
>       int err;
>  
> @@ -356,7 +324,7 @@ int lowpan_process_data(struct sk_buff *skb, struct 
> net_device *dev,
>                       goto drop;
>       }
>  
> -     hdr.version = 6;
> +     hdr->version = 6;
>  
>       /* Traffic Class and Flow Label */
>       switch ((iphc0 & LOWPAN_IPHC_TF) >> 3) {
> @@ -368,11 +336,11 @@ int lowpan_process_data(struct sk_buff *skb, struct 
> net_device *dev,
>               if (lowpan_fetch_skb_u8(skb, &tmp))
>                       goto drop;
>  
> -             memcpy(&hdr.flow_lbl, &skb->data[0], 3);
> +             memcpy(&hdr->flow_lbl, &skb->data[0], 3);
>               skb_pull(skb, 3);
> -             hdr.priority = ((tmp >> 2) & 0x0f);
> -             hdr.flow_lbl[0] = ((tmp >> 2) & 0x30) | (tmp << 6) |
> -                                     (hdr.flow_lbl[0] & 0x0f);
> +             hdr->priority = ((tmp >> 2) & 0x0f);
> +             hdr->flow_lbl[0] = ((tmp >> 2) & 0x30) | (tmp << 6) |
> +                                     (hdr->flow_lbl[0] & 0x0f);
>               break;
>       /*
>        * Traffic class carried in-line
> @@ -382,8 +350,8 @@ int lowpan_process_data(struct sk_buff *skb, struct 
> net_device *dev,
>               if (lowpan_fetch_skb_u8(skb, &tmp))
>                       goto drop;
>  
> -             hdr.priority = ((tmp >> 2) & 0x0f);
> -             hdr.flow_lbl[0] = ((tmp << 6) & 0xC0) | ((tmp >> 2) & 0x30);
> +             hdr->priority = ((tmp >> 2) & 0x0f);
> +             hdr->flow_lbl[0] = ((tmp << 6) & 0xC0) | ((tmp >> 2) & 0x30);
>               break;
>       /*
>        * Flow Label carried in-line
> @@ -393,8 +361,8 @@ int lowpan_process_data(struct sk_buff *skb, struct 
> net_device *dev,
>               if (lowpan_fetch_skb_u8(skb, &tmp))
>                       goto drop;
>  
> -             hdr.flow_lbl[0] = (skb->data[0] & 0x0F) | ((tmp >> 2) & 0x30);
> -             memcpy(&hdr.flow_lbl[1], &skb->data[0], 2);
> +             hdr->flow_lbl[0] = (skb->data[0] & 0x0F) | ((tmp >> 2) & 0x30);
> +             memcpy(&hdr->flow_lbl[1], &skb->data[0], 2);
>               skb_pull(skb, 2);
>               break;
>       /* Traffic Class and Flow Label are elided */
> @@ -407,18 +375,18 @@ int lowpan_process_data(struct sk_buff *skb, struct 
> net_device *dev,
>       /* Next Header */
>       if ((iphc0 & LOWPAN_IPHC_NH_C) == 0) {
>               /* Next header is carried inline */
> -             if (lowpan_fetch_skb_u8(skb, &(hdr.nexthdr)))
> +             if (lowpan_fetch_skb_u8(skb, &(hdr->nexthdr)))
>                       goto drop;
>  
>               pr_debug("NH flag is set, next header carried inline: %02x\n",
> -                      hdr.nexthdr);
> +                      hdr->nexthdr);
>       }
>  
>       /* Hop Limit */
>       if ((iphc0 & 0x03) != LOWPAN_IPHC_TTL_I)
> -             hdr.hop_limit = lowpan_ttl_values[iphc0 & 0x03];
> +             hdr->hop_limit = lowpan_ttl_values[iphc0 & 0x03];
>       else {
> -             if (lowpan_fetch_skb_u8(skb, &(hdr.hop_limit)))
> +             if (lowpan_fetch_skb_u8(skb, &(hdr->hop_limit)))
>                       goto drop;
>       }
>  
> @@ -429,11 +397,11 @@ int lowpan_process_data(struct sk_buff *skb, struct 
> net_device *dev,
>               /* Source address context based uncompression */
>               pr_debug("SAC bit is set. Handle context based source 
> address.\n");
>               err = uncompress_context_based_src_addr(
> -                             skb, &hdr.saddr, tmp);
> +                             skb, &hdr->saddr, tmp);
>       } else {
>               /* Source address uncompression */
>               pr_debug("source address stateless compression\n");
> -             err = uncompress_addr(skb, &hdr.saddr, tmp, saddr,
> +             err = uncompress_addr(skb, &hdr->saddr, tmp, saddr,
>                                       saddr_type, saddr_len);
>       }
>  
> @@ -451,15 +419,15 @@ int lowpan_process_data(struct sk_buff *skb, struct 
> net_device *dev,
>                       /* TODO: implement this */
>               } else {
>                       err = lowpan_uncompress_multicast_daddr(
> -                                             skb, &hdr.daddr, tmp);
> +                                             skb, &hdr->daddr, tmp);
>                       if (err)
>                               goto drop;
>               }
>       } else {
> -             err = uncompress_addr(skb, &hdr.daddr, tmp, daddr,
> +             err = uncompress_addr(skb, &hdr->daddr, tmp, daddr,
>                                       daddr_type, daddr_len);
>               pr_debug("dest: stateless compression mode %d dest %pI6c\n",
> -                     tmp, &hdr.daddr);
> +                     tmp, &hdr->daddr);
>               if (err)
>                       goto drop;
>       }
> @@ -477,10 +445,10 @@ int lowpan_process_data(struct sk_buff *skb, struct 
> net_device *dev,
>                */
>               new = skb_copy_expand(skb, sizeof(struct udphdr),
>                                     skb_tailroom(skb), GFP_ATOMIC);
> -             kfree_skb(skb);
> -
>               if (!new)
> -                     return -ENOMEM;
> +                     goto drop;
> +
> +             kfree_skb(skb);
>  
>               skb = new;
>  
> @@ -491,27 +459,26 @@ int lowpan_process_data(struct sk_buff *skb, struct 
> net_device *dev,
>               raw_dump_table(__func__, "raw UDP header dump",
>                                     (u8 *)&uh, sizeof(uh));
>  
> -             hdr.nexthdr = UIP_PROTO_UDP;
> +             hdr->nexthdr = UIP_PROTO_UDP;
>       }
>  
> -     hdr.payload_len = htons(skb->len);
> +     hdr->payload_len = htons(skb->len);
>  
>       pr_debug("skb headroom size = %d, data length = %d\n",
>                skb_headroom(skb), skb->len);
>  
>       pr_debug("IPv6 header dump:\n\tversion = %d\n\tlength  = %d\n\t"
>                "nexthdr = 0x%02x\n\thop_lim = %d\n\tdest    = %pI6c\n",
> -             hdr.version, ntohs(hdr.payload_len), hdr.nexthdr,
> -             hdr.hop_limit, &hdr.daddr);
> +             hdr->version, ntohs(hdr->payload_len), hdr->nexthdr,
> +             hdr->hop_limit, &hdr->daddr);
>  
>       raw_dump_table(__func__, "raw header dump", (u8 *)&hdr,
>                                                       sizeof(hdr));
>  
> -     return skb_deliver(skb, &hdr, dev, deliver_skb);
> +     return 0;
>  
>  drop:
> -     kfree_skb(skb);
> -     return -EINVAL;
> +     return -1;
>  }
>  EXPORT_SYMBOL_GPL(lowpan_process_data);
>  
> diff --git a/net/ieee802154/6lowpan_rtnl.c b/net/ieee802154/6lowpan_rtnl.c
> index 0f5a69e..2d027b4 100644
> --- a/net/ieee802154/6lowpan_rtnl.c
> +++ b/net/ieee802154/6lowpan_rtnl.c
> @@ -144,8 +144,7 @@ static int lowpan_header_create(struct sk_buff *skb,
>                       type, (void *)&da, (void *)&sa, 0);
>  }
>  
> -static int lowpan_give_skb_to_devices(struct sk_buff *skb,
> -                                     struct net_device *dev)
> +static int lowpan_give_skb_to_devices(struct sk_buff *skb)
>  {
>       struct lowpan_dev_record *entry;
>       struct sk_buff *skb_cp;
> @@ -168,7 +167,8 @@ static int lowpan_give_skb_to_devices(struct sk_buff *skb,
>       return stat;
>  }
>  
> -static int process_data(struct sk_buff *skb, const struct ieee802154_hdr 
> *hdr)
> +static int process_data(struct sk_buff *skb, const struct ieee802154_hdr 
> *hdr,
> +                     struct ipv6hdr *ipv6_hdr)
>  {
>       u8 iphc0, iphc1;
>       struct ieee802154_addr_sa sa, da;
> @@ -198,14 +198,12 @@ static int process_data(struct sk_buff *skb, const 
> struct ieee802154_hdr *hdr)
>       else
>               dap = &da.hwaddr;
>  
> -     return lowpan_process_data(skb, skb->dev, sap, sa.addr_type,
> +     return lowpan_process_data(skb, ipv6_hdr, sap, sa.addr_type,
>                                  IEEE802154_ADDR_LEN, dap, da.addr_type,
> -                                IEEE802154_ADDR_LEN, iphc0, iphc1,
> -                                lowpan_give_skb_to_devices);
> +                                IEEE802154_ADDR_LEN, iphc0, iphc1);
>  
>  drop:
> -     kfree_skb(skb);
> -     return -EINVAL;
> +     return -1;
>  }
>  
>  static int lowpan_set_address(struct net_device *dev, void *p)
> @@ -471,38 +469,64 @@ static int lowpan_rcv(struct sk_buff *skb, struct 
> net_device *dev,
>               /* Pull off the 1-byte of 6lowpan header. */
>               skb_pull(skb, 1);
>  
> -             ret = lowpan_give_skb_to_devices(skb, NULL);
> -             if (ret == NET_RX_DROP)
> -                     goto drop;
>       } else {
> -             switch (skb->data[0] & 0xe0) {
> -             case LOWPAN_DISPATCH_IPHC:      /* ipv6 datagram */
> -                     ret = process_data(skb, &hdr);
> -                     if (ret == NET_RX_DROP)
> -                             goto drop;
> +             u8 dispatch = skb->data[0] & 0xe0;
> +             struct sk_buff *new;
> +             struct ipv6hdr ipv6_hdr = {};
> +
> +             switch (dispatch) {
> +                     ret = process_data(skb, &hdr, &ipv6_hdr);
> +                     if (ret != 0)
> +                             goto drop_skb;
>                       break;
>               case LOWPAN_DISPATCH_FRAG1:     /* first fragment header */
> -                     ret = lowpan_frag_rcv(skb, LOWPAN_DISPATCH_FRAG1);
> -                     if (ret == 1) {
> -                             ret = process_data(skb, &hdr);
> -                             if (ret == NET_RX_DROP)
> -                                     goto drop;
> -                     }
> -                     break;
> +                     /* Intentional fall-through */
>               case LOWPAN_DISPATCH_FRAGN:     /* next fragments headers */
> -                     ret = lowpan_frag_rcv(skb, LOWPAN_DISPATCH_FRAGN);
> -                     if (ret == 1) {
> -                             ret = process_data(skb, &hdr);
> -                             if (ret == NET_RX_DROP)
> -                                     goto drop;
> -                     }
> +                     ret = lowpan_frag_rcv(skb, dispatch);
> +                     if (ret == 0)
> +                             /* more fragments to process */
> +                             return NET_RX_SUCCESS;
> +                     else if(ret < 0) 
> +                             goto drop_skb;
> +                     /* Otherwise fragments received, Intentional 
> +                        fall-through to uncompress IPv6 header */
> +             case LOWPAN_DISPATCH_IPHC:      /* ipv6 datagram */
> +                     ret = process_data(skb, &hdr, &ipv6_hdr);
> +                     if (ret != 0)
> +                             goto drop_skb;
>                       break;
>               default:
> -                     break;
> +                     /* TODO: Mesh/ BC0 not supported currently */
> +                     netdev_warn(skb->dev, "Dispatch value 0x%x not 
> supported\n", dispatch);
> +                     goto drop_skb;
>               }
> +
> +             /* Prepare new skb for the uncompressed IPv6 header */
> +             new = skb_copy_expand(skb, sizeof(struct ipv6hdr), 
> +                                   skb_tailroom(skb), GFP_ATOMIC);
> +             if (!new)
> +                     goto drop_skb;
> +
> +             kfree_skb(skb);
> +
> +             skb_push(new, sizeof(struct ipv6hdr));
> +             skb_reset_network_header(new);
> +             skb_copy_to_linear_data(new, &ipv6_hdr, sizeof(struct ipv6hdr));
> +
> +             new->protocol = htons(ETH_P_IPV6);
> +             new->pkt_type = PACKET_HOST;
> +             new->dev = dev;
> +             skb = new;
> +
> +             raw_dump_table(__func__, "raw skb data dump before receiving",
> +                             skb->data, skb->len);
> +
>       }
>  
> -     return NET_RX_SUCCESS;
> +     /* if we get here we're good to pass the skb which should contain an
> +        uncompressed IPv6 header to all lowpan netdev's */
> +     return lowpan_give_skb_to_devices(skb);
> +
>  drop_skb:
>       kfree_skb(skb);
>  drop:
> diff --git a/net/ieee802154/reassembly.c b/net/ieee802154/reassembly.c
> index ef2d543..4fb05cd 100644
> --- a/net/ieee802154/reassembly.c
> +++ b/net/ieee802154/reassembly.c
> @@ -222,9 +222,8 @@ found:
>       }
>  
>       inet_frag_lru_move(&fq->q);
> -     return -1;
> +     return 0;
>  err:
> -     kfree_skb(skb);
>       return -1;
>  }
>  
> @@ -383,7 +382,6 @@ int lowpan_frag_rcv(struct sk_buff *skb, const u8 
> frag_type)
>       }
>  
>  err:
> -     kfree_skb(skb);
>       return -1;
>  }
>  EXPORT_SYMBOL(lowpan_frag_rcv);
> -- 
> 1.9.1
>
>
>
>
> ------------------------------------------------------------------------------
> Open source business process management suite built on Java and Eclipse
> Turn processes into business applications with Bonita BPM Community Edition
> Quickly connect people, data, and systems into organized workflows
> Winner of BOSSIE, CODIE, OW2 and Gartner awards
> http://p.sf.net/sfu/Bonitasoft
>
>
> _______________________________________________
> Linux-zigbee-devel mailing list
> Linux-zigbee-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/linux-zigbee-devel

------------------------------------------------------------------------------
Open source business process management suite built on Java and Eclipse
Turn processes into business applications with Bonita BPM Community Edition
Quickly connect people, data, and systems into organized workflows
Winner of BOSSIE, CODIE, OW2 and Gartner awards
http://p.sf.net/sfu/Bonitasoft
_______________________________________________
Linux-zigbee-devel mailing list
Linux-zigbee-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-zigbee-devel

Reply via email to