On Tue, Apr 30, 2013 at 5:11 PM, Jesse Gross <[email protected]> wrote:
> On Tue, Apr 30, 2013 at 4:20 PM, Pravin B Shelar <[email protected]> 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
[email protected]
http://openvswitch.org/mailman/listinfo/dev
