On Tue, Apr 30, 2013 at 5:11 PM, Jesse Gross <je...@nicira.com> wrote:
> On Tue, Apr 30, 2013 at 4:20 PM, Pravin B Shelar <pshe...@nicira.com> wrote:
>> diff --git a/datapath/tunnel.c b/datapath/tunnel.c
>> index 057aaed..fb430f2 100644
>> --- a/datapath/tunnel.c
>> +++ b/datapath/tunnel.c
>> @@ -234,17 +221,32 @@ int ovs_tnl_send(struct vport *vport, struct sk_buff 
>> *skb)
>>         rt = find_route(ovs_dp_get_net(vport->dp),
>>                         &saddr,
>>                         OVS_CB(skb)->tun_key->ipv4_dst,
>> -                       tnl_vport->tnl_ops->ipproto,
>> +                       ipproto,
>>                         OVS_CB(skb)->tun_key->ipv4_tos,
>>                         skb_get_mark(skb));
>>         if (IS_ERR(rt))
>>                 goto error_free;
>>
>> -       /* Offloading */
>> -       tunnel_hlen = tnl_vport->tnl_ops->hdr_len(OVS_CB(skb)->tun_key);
>>         tunnel_hlen += sizeof(struct iphdr);
>>
>> -       skb = handle_offloads(skb, rt, tunnel_hlen);
>> +       min_headroom = LL_RESERVED_SPACE(rt_dst(rt).dev) + 
>> rt_dst(rt).header_len
>> +                       + tunnel_hlen
>> +                       + (vlan_tx_tag_present(skb) ? VLAN_HLEN : 0);
>> +
>> +       if (skb_headroom(skb) < min_headroom || skb_header_cloned(skb)) {
>> +               int err;
>> +               int head_delta = SKB_DATA_ALIGN(min_headroom -
>> +                                               skb_headroom(skb) +
>> +                                               16);
>> +
>> +               err = pskb_expand_head(skb, max_t(int, head_delta, 0),
>> +                                       0, GFP_ATOMIC);
>> +               if (unlikely(err))
>> +                       goto error_free;
>> +       }
>> +
>> +       /* Offloading */
>> +       skb = handle_offloads(skb, rt);
>
> I'm not sure I understand why the code block that expands the headroom
> was moved out of handle_offloads(). However, I see a couple of issues:
>  * It leaks the rt in the case of an error.
>  * rt is not used in handle_offloads().
>

I wanted to keep only offloading bits in handle offload, Thats why I
moved headroom related code out. I will fix both issues.

>> diff --git a/datapath/vport-vxlan.c b/datapath/vport-vxlan.c
>> index 1850fc2..92b1666 100644
>> --- a/datapath/vport-vxlan.c
>> +++ b/datapath/vport-vxlan.c
>>  /**
>>   * struct vxlan_port - Keeps track of open UDP ports
>> - * @list: list element.
>> - * @vport: vport for the tunnel.
>> + * @dst_port: vxlan UDP port no.
>> + * @list: list element in @vxlan_ports.
>>   * @socket: The socket created for this port number.
>> + * @name: vport name.
>> + * @rcu: RCU callback head for deferred destruction.
>>   */
>>  struct vxlan_port {
>> +       __be16 dst_port;
>>         struct list_head list;
>> -       struct vport *vport;
>>         struct socket *vxlan_rcv_socket;
>> +       char name[IFNAMSIZ];
>>         struct rcu_head rcu;
>
> We're not using the 'rcu' element anymore, are we?

right, It is not required.
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Reply via email to