On Thu, Apr 18, 2013 at 4:51 PM, Pravin B Shelar <pshe...@nicira.com> wrote:
> diff --git a/datapath/vport-vxlan.c b/datapath/vport-vxlan.c
> index 1850fc2..a5ad3fc 100644
> --- a/datapath/vport-vxlan.c
> +++ b/datapath/vport-vxlan.c
> @@ -62,22 +57,28 @@ static inline int vxlan_hdr_len(const struct
> ovs_key_ipv4_tunnel *tun_key)
> * @socket: The socket created for this port number.
> */
> 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 should update the comments to match.
> @@ -230,22 +211,15 @@ static int vxlan_tunnel_setup(struct net *net, struct
> vport *vport,
> }
>
> /* Verify if we already have a socket created for this port */
> - vxlan_port = vxlan_find_port(net, htons(dst_port));
> - if (vxlan_port) {
> + if (vxlan_find_port(net, htons(dst_port))) {
> err = -EEXIST;
> goto out;
> }
>
> - /* Add a new socket for this port */
> - vxlan_port = kzalloc(sizeof(struct vxlan_port), GFP_KERNEL);
> - if (!vxlan_port) {
> - err = -ENOMEM;
> - goto out;
> - }
> -
> - tnl_vport->dst_port = htons(dst_port);
> + vxlan_port->dst_port = htons(dst_port);
> vxlan_port->vport = vport;
I think we could now use vport_from_priv() instead of storing an
explicit pointer to the vport.
> @@ -292,28 +258,41 @@ static struct vport *vxlan_tnl_create(const struct
> vport_parms *parms)
> int err;
> struct vport *vport;
>
> - vport = ovs_tnl_create(parms, &ovs_vxlan_vport_ops,
> &ovs_vxlan_tnl_ops);
> + vport = ovs_vport_alloc(sizeof(struct vxlan_port),
> + &ovs_vxlan_vport_ops, parms);
> if (IS_ERR(vport))
> return vport;
>
> err = vxlan_tunnel_setup(ovs_dp_get_net(parms->dp), vport,
> - parms->options);
> + parms);
I think we can merge together vxlan_tnl_create() and
vxlan_tunnel_setup() since there is now a 1:1 mapping between the two
and they are logically the same operation.
Can factor out the common socket_init from VXLAN and LISP?
Otherwise, similar comments also apply to LISP.
> diff --git a/datapath/vport.h b/datapath/vport.h
> index 8280eba..565ae4f 100644
> --- a/datapath/vport.h
> +++ b/datapath/vport.h
> @@ -51,6 +51,7 @@ int ovs_vport_set_options(struct vport *, struct nlattr
> *options);
> int ovs_vport_get_options(const struct vport *, struct sk_buff *);
>
> int ovs_vport_send(struct vport *, struct sk_buff *);
> +void ovs_vport_deferred_destroy(struct vport *vport);
This should be grouped with ovs_vport_free().
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
