[ovs-dev] [PATCH v2 1/2] tunneling: Remove struct tnl_vport and tnl_ops.

[Jesse Gross]

On Tue, Apr 30, 2013 at 4:20 PM, Pravin B Shelar <pshelar at 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().

> 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?