Hi Alex,
On 16/09/14 12:36, Alexander Aring wrote:
> On Tue, Sep 16, 2014 at 12:01:59PM +0100, Martin Townsend wrote:
>> Currently there are a number of error paths in the lowpan_rcv function that
>> free the skb before returning, the patch simplifies the receive path by
>> ensuring that the skb is only freed from this function.
>>
>> Passing the skb from 6lowpan up to the higher layers is not a
>> function of IPHC. By moving it out of IPHC we also remove the
>> need to support error code returns with NET_RX codes.
>> It also makes the lowpan_rcv function more extendable as we
>> can support more compression schemes.
>>
>> With the above 2 lowpan_rcv is refacored so eliminate incorrect return values.
>>
>> Signed-off-by: Martin Townsend <martin.townsend@xxxxxxxxxx>
>> ---
>> include/net/6lowpan.h | 9 +++--
>> net/6lowpan/iphc.c | 88 +++++++++++++++++--------------------------
>> net/bluetooth/6lowpan.c | 38 ++++++++++---------
>> net/ieee802154/6lowpan_rtnl.c | 61 ++++++++++++++++--------------
>> 4 files changed, 94 insertions(+), 102 deletions(-)
>>
>> diff --git a/include/net/6lowpan.h b/include/net/6lowpan.h
>> index d184df1..c28fadb 100644
>> --- a/include/net/6lowpan.h
>> +++ b/include/net/6lowpan.h
> ...
>> -static int process_data(struct sk_buff *skb, struct net_device *netdev,
>> - struct l2cap_chan *chan)
>> +static struct sk_buff *
>> +process_data(struct sk_buff *skb, struct net_device *netdev,
>> + struct l2cap_chan *chan)
>> {
>> const u8 *saddr, *daddr;
>> u8 iphc0, iphc1;
>> @@ -230,36 +231,31 @@ static int process_data(struct sk_buff *skb, struct net_device *netdev,
>> peer = peer_lookup_chan(dev, chan);
>> read_unlock_irqrestore(&devices_lock, flags);
>> if (!peer)
>> - goto drop;
>> + return ERR_PTR(-EINVAL);
>>
>> saddr = peer->eui64_addr;
>> daddr = dev->netdev->dev_addr;
>>
>> /* at least two bytes will be used for the encoding */
>> if (skb->len < 2)
>> - goto drop;
>> + return ERR_PTR(-EINVAL);
>>
>> if (lowpan_fetch_skb_u8(skb, &iphc0))
>> - goto drop;
>> + return ERR_PTR(-EINVAL);
>>
>> if (lowpan_fetch_skb_u8(skb, &iphc1))
>> - goto drop;
>> + return ERR_PTR(-EINVAL);
>>
>> return lowpan_process_data(skb, netdev,
>> saddr, IEEE802154_ADDR_LONG, EUI64_ADDR_LEN,
>> daddr, IEEE802154_ADDR_LONG, EUI64_ADDR_LEN,
>> - iphc0, iphc1, give_skb_to_upper);
>> -
>> -drop:
>> - kfree_skb(skb);
>> - return -EINVAL;
>> + iphc0, iphc1);
>> }
>>
>> static int recv_pkt(struct sk_buff *skb, struct net_device *dev,
>> struct l2cap_chan *chan)
>> {
>> struct sk_buff *local_skb;
>> - int ret;
>>
>> if (!netif_running(dev))
>> goto drop;
>> @@ -280,9 +276,6 @@ static int recv_pkt(struct sk_buff *skb, struct net_device *dev,
>> local_skb->protocol = htons(ETH_P_IPV6);
>> local_skb->pkt_type = PACKET_HOST;
>>
>> - skb_reset_network_header(local_skb);
>> - skb_set_transport_header(local_skb, sizeof(struct ipv6hdr));
>> -
>> if (give_skb_to_upper(local_skb, dev) != NET_RX_SUCCESS) {
>> kfree_skb(local_skb);
>> goto drop;
>> @@ -300,9 +293,20 @@ static int recv_pkt(struct sk_buff *skb, struct net_device *dev,
>> if (!local_skb)
>> goto drop;
>>
>> - ret = process_data(local_skb, dev, chan);
>> - if (ret != NET_RX_SUCCESS)
>> + skb = process_data(local_skb, dev, chan);
>> + if (IS_ERR(skb)) {
>> + kfree_skb(local_skb);
>> goto drop;
>> + }
>> +
>> + local_skb->protocol = htons(ETH_P_IPV6);
>> + local_skb->pkt_type = PACKET_HOST;
> this is the wrong skb here, the new one is skb. I don't know maybe there
> is some optimization to call skb = process_data(skb, ...);
yes you are right, my mistake, I'll fix in next series.
>
>> +
>> + if (give_skb_to_upper(local_skb, dev)
> also here local_skb should be skb, or?
>
> - Alex
- Martin.
--
To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
