[ovs-dev] dev Dige.st, Vol 45, Iss,,ue 173

jayce lee jaycelq at gmail.com
Wed May 1 07:32:40 UTC 2013


*,
On May 1, 2013 8:49 AM, <dev-request at openvswitch.org> wrote:

> Send dev mailing list submissions to
>         dev at openvswitch.org
>
> To subscribe or unsubscribe via the World Wide Web, visit
>         http://openvswitch.org/mailman/listinfo/dev
> or, via email, send a message with subject or body 'help' to
>         dev-request at openvswitch.org
>
> You can reach the person managing the list at
>         dev-owner at openvswitch.org
>
> When replying, please edit your Subject line so it is more specific
> than "Re: Contents of dev digest..."
>
>
> Today's Topics:
>
>    1. [PATCH v2 2/2] datapath: Move vport init to First port
>       create. (Pravin B Shelar)
>    2. Re: [PATCH v2 1/2] tunneling: Remove struct tnl_vport and
>       tnl_ops. (Jesse Gross)
>    3. Re: [PATCH v2 2/2] datapath: Move vport init to First     port
>       create. (Jesse Gross)
>    4. Re: [PATCH v2 1/2] tunneling: Remove struct tnl_vport and
>       tnl_ops. (Pravin Shelar)
>
>
> ----------------------------------------------------------------------
>
> Message: 1
> Date: Tue, 30 Apr 2013 16:21:09 -0700
> From: Pravin B Shelar <pshelar at nicira.com>
> Subject: [ovs-dev] [PATCH v2 2/2] datapath: Move vport init to First
>         port    create.
> To: dev at openvswitch.org
> Message-ID: <1367364069-4964-1-git-send-email-pshelar at nicira.com>
>
> vport->init and exit() functios are defined by gre and netdev vport
> only and both can be moved to first port create.
>
> Following patch does same, it moves vport init to respectve vport
> create and get rid of vport->init() and vport->exit() fnctions.
>
> Signed-off-by: Pravin B Shelar <pshelar at nicira.com>
> ---
> v1-v2:
> No Change.
> ---
>  datapath/vport-gre.c          |   49 +++++++++++++++++++++++-----------
>  datapath/vport-internal_dev.c |    1 -
>  datapath/vport-netdev.c       |   19 ++++++++++---
>  datapath/vport.c              |   58
> ++++------------------------------------
>  datapath/vport.h              |   11 +-------
>  5 files changed, 54 insertions(+), 84 deletions(-)
>
> diff --git a/datapath/vport-gre.c b/datapath/vport-gre.c
> index 2db4934..59e22be 100644
> --- a/datapath/vport-gre.c
> +++ b/datapath/vport-gre.c
> @@ -290,16 +290,15 @@ static const struct net_protocol
> gre_protocol_handlers = {
>  #endif
>  };
>
> -static bool inited;
> -
> +static int gre_ports;
>  static int gre_init(void)
>  {
>         int err;
>
> -       if (inited)
> +       gre_ports++;
> +       if (gre_ports > 1)
>                 return 0;
>
> -       inited = true;
>         err = inet_add_protocol(&gre_protocol_handlers, IPPROTO_GRE);
>         if (err)
>                 pr_warn("cannot register gre protocol handler\n");
> @@ -309,11 +308,10 @@ static int gre_init(void)
>
>  static void gre_exit(void)
>  {
> -       if (!inited)
> +       gre_ports--;
> +       if (gre_ports > 0)
>                 return;
>
> -       inited = false;
> -
>         inet_del_protocol(&gre_protocol_handlers, IPPROTO_GRE);
>  }
>
> @@ -327,10 +325,17 @@ static struct vport *gre_create(const struct
> vport_parms *parms)
>         struct net *net = ovs_dp_get_net(parms->dp);
>         struct ovs_net *ovs_net;
>         struct vport *vport;
> +       int err;
> +
> +       err = gre_init();
> +       if (err)
> +               return ERR_PTR(err);
>
>         ovs_net = net_generic(net, ovs_net_id);
> -       if (ovsl_dereference(ovs_net->vport_net.gre_vport))
> -               return ERR_PTR(-EEXIST);
> +       if (ovsl_dereference(ovs_net->vport_net.gre_vport)) {
> +               vport = ERR_PTR(-EEXIST);
> +               goto error;
> +       }
>
>         vport = ovs_vport_alloc(IFNAMSIZ, &ovs_gre_vport_ops, parms);
>         if (IS_ERR(vport))
> @@ -339,6 +344,10 @@ static struct vport *gre_create(const struct
> vport_parms *parms)
>         strncpy(vport_priv(vport), parms->name, IFNAMSIZ);
>         rcu_assign_pointer(ovs_net->vport_net.gre_vport, vport);
>         return vport;
> +
> +error:
> +       gre_exit();
> +       return vport;
>  }
>
>  static void gre_tnl_destroy(struct vport *vport)
> @@ -350,6 +359,7 @@ static void gre_tnl_destroy(struct vport *vport)
>
>         rcu_assign_pointer(ovs_net->vport_net.gre_vport, NULL);
>         ovs_vport_deferred_free(vport);
> +       gre_exit();
>  }
>
>  static int gre_tnl_send(struct vport *vport, struct sk_buff *skb)
> @@ -366,8 +376,6 @@ static int gre_tnl_send(struct vport *vport, struct
> sk_buff *skb)
>  const struct vport_ops ovs_gre_vport_ops = {
>         .type           = OVS_VPORT_TYPE_GRE,
>         .flags          = VPORT_F_TUN_ID,
> -       .init           = gre_init,
> -       .exit           = gre_exit,
>         .create         = gre_create,
>         .destroy        = gre_tnl_destroy,
>         .get_name       = gre_get_name,
> @@ -380,18 +388,28 @@ static struct vport *gre64_create(const struct
> vport_parms *parms)
>         struct net *net = ovs_dp_get_net(parms->dp);
>         struct ovs_net *ovs_net;
>         struct vport *vport;
> +       int err;
> +
> +       err = gre_init();
> +       if (err)
> +               return ERR_PTR(err);
>
>         ovs_net = net_generic(net, ovs_net_id);
> -       if (ovsl_dereference(ovs_net->vport_net.gre64_vport))
> -               return ERR_PTR(-EEXIST);
> +       if (ovsl_dereference(ovs_net->vport_net.gre64_vport)) {
> +               vport = ERR_PTR(-EEXIST);
> +               goto error;
> +       }
>
>         vport = ovs_vport_alloc(IFNAMSIZ, &ovs_gre64_vport_ops, parms);
>         if (IS_ERR(vport))
> -               return vport;
> +               goto error;
>
>         strncpy(vport_priv(vport), parms->name, IFNAMSIZ);
>         rcu_assign_pointer(ovs_net->vport_net.gre64_vport, vport);
>         return vport;
> +error:
> +       gre_exit();
> +       return vport;
>  }
>
>  static void gre64_tnl_destroy(struct vport *vport)
> @@ -403,6 +421,7 @@ static void gre64_tnl_destroy(struct vport *vport)
>
>         rcu_assign_pointer(ovs_net->vport_net.gre64_vport, NULL);
>         ovs_vport_deferred_free(vport);
> +       gre_exit();
>  }
>
>  static int gre64_tnl_send(struct vport *vport, struct sk_buff *skb)
> @@ -419,8 +438,6 @@ static int gre64_tnl_send(struct vport *vport, struct
> sk_buff *skb)
>  const struct vport_ops ovs_gre64_vport_ops = {
>         .type           = OVS_VPORT_TYPE_GRE64,
>         .flags          = VPORT_F_TUN_ID,
> -       .init           = gre_init,
> -       .exit           = gre_exit,
>         .create         = gre64_create,
>         .destroy        = gre64_tnl_destroy,
>         .get_name       = gre_get_name,
> diff --git a/datapath/vport-internal_dev.c b/datapath/vport-internal_dev.c
> index 1e8c304..1b19bfc 100644
> --- a/datapath/vport-internal_dev.c
> +++ b/datapath/vport-internal_dev.c
> @@ -294,7 +294,6 @@ static int internal_dev_recv(struct vport *vport,
> struct sk_buff *skb)
>
>  const struct vport_ops ovs_internal_vport_ops = {
>         .type           = OVS_VPORT_TYPE_INTERNAL,
> -       .flags          = VPORT_F_REQUIRED,
>         .create         = internal_dev_create,
>         .destroy        = internal_dev_destroy,
>         .get_name       = ovs_netdev_get_name,
> diff --git a/datapath/vport-netdev.c b/datapath/vport-netdev.c
> index 4558079..434ab4d 100644
> --- a/datapath/vport-netdev.c
> +++ b/datapath/vport-netdev.c
> @@ -117,17 +117,27 @@ static int netdev_frame_hook(struct net_bridge_port
> *p, struct sk_buff **pskb)
>  static int netdev_init(void) { return 0; }
>  static void netdev_exit(void) { }
>  #else
> -static int netdev_init(void)
> +static int port_count;
> +
> +static void netdev_init(void)
>  {
> +       port_count++;
> +       if (port_count > 1)
> +               return;
> +
>         /* Hook into callback used by the bridge to intercept packets.
>          * Parasites we are. */
>         br_handle_frame_hook = netdev_frame_hook;
>
> -       return 0;
> +       return;
>  }
>
>  static void netdev_exit(void)
>  {
> +       port_count--;
> +       if (port_count > 0)
> +               return;
> +
>         br_handle_frame_hook = NULL;
>  }
>  #endif
> @@ -179,6 +189,7 @@ static struct vport *netdev_create(const struct
> vport_parms *parms)
>         netdev_vport->dev->priv_flags |= IFF_OVS_DATAPATH;
>         rtnl_unlock();
>
> +       netdev_init();
>         return vport;
>
>  #ifndef HAVE_RHEL_OVS_HOOK
> @@ -212,6 +223,7 @@ static void netdev_destroy(struct vport *vport)
>  {
>         struct netdev_vport *netdev_vport = netdev_vport_priv(vport);
>
> +       netdev_exit();
>         rtnl_lock();
>         netdev_vport->dev->priv_flags &= ~IFF_OVS_DATAPATH;
>         netdev_rx_handler_unregister(netdev_vport->dev);
> @@ -388,9 +400,6 @@ struct vport *ovs_netdev_get_vport(struct net_device
> *dev)
>
>  const struct vport_ops ovs_netdev_vport_ops = {
>         .type           = OVS_VPORT_TYPE_NETDEV,
> -       .flags          = VPORT_F_REQUIRED,
> -       .init           = netdev_init,
> -       .exit           = netdev_exit,
>         .create         = netdev_create,
>         .destroy        = netdev_destroy,
>         .get_name       = ovs_netdev_get_name,
> diff --git a/datapath/vport.c b/datapath/vport.c
> index b63ed59..2f007ae 100644
> --- a/datapath/vport.c
> +++ b/datapath/vport.c
> @@ -36,7 +36,7 @@
>
>  /* List of statically compiled vport implementations.  Don't forget to
> also
>   * add yours to the list at the bottom of vport.h. */
> -static const struct vport_ops *base_vport_ops_list[] = {
> +static const struct vport_ops *vport_ops_list[] = {
>         &ovs_netdev_vport_ops,
>         &ovs_internal_vport_ops,
>         &ovs_gre_vport_ops,
> @@ -47,9 +47,6 @@ static const struct vport_ops *base_vport_ops_list[] = {
>  #endif
>  };
>
> -static const struct vport_ops **vport_ops_list;
> -static int n_vport_types;
> -
>  /* Protected by RCU read lock for reading, ovs_mutex for writing. */
>  static struct hlist_head *dev_table;
>  #define VPORT_HASH_BUCKETS 1024
> @@ -57,68 +54,25 @@ static struct hlist_head *dev_table;
>  /**
>   *     ovs_vport_init - initialize vport subsystem
>   *
> - * Called at module load time to initialize the vport subsystem and any
> - * compiled in vport types.
> + * Called at module load time to initialize the vport subsystem.
>   */
>  int ovs_vport_init(void)
>  {
> -       int err;
> -       int i;
> -
>         dev_table = kzalloc(VPORT_HASH_BUCKETS * sizeof(struct hlist_head),
>                             GFP_KERNEL);
> -       if (!dev_table) {
> -               err = -ENOMEM;
> -               goto error;
> -       }
> -
> -       vport_ops_list = kmalloc(ARRAY_SIZE(base_vport_ops_list) *
> -                                sizeof(struct vport_ops *), GFP_KERNEL);
> -       if (!vport_ops_list) {
> -               err = -ENOMEM;
> -               goto error_dev_table;
> -       }
> -
> -       for (i = 0; i < ARRAY_SIZE(base_vport_ops_list); i++) {
> -               const struct vport_ops *new_ops = base_vport_ops_list[i];
> -
> -               if (new_ops->init)
> -                       err = new_ops->init();
> -               else
> -                       err = 0;
> -
> -               if (!err)
> -                       vport_ops_list[n_vport_types++] = new_ops;
> -               else if (new_ops->flags & VPORT_F_REQUIRED) {
> -                       ovs_vport_exit();
> -                       goto error;
> -               }
> -       }
> +       if (!dev_table)
> +               return -ENOMEM;
>
>         return 0;
> -
> -error_dev_table:
> -       kfree(dev_table);
> -error:
> -       return err;
>  }
>
>  /**
>   *     ovs_vport_exit - shutdown vport subsystem
>   *
> - * Called at module exit time to shutdown the vport subsystem and any
> - * initialized vport types.
> + * Called at module exit time to shutdown the vport subsystem.
>   */
>  void ovs_vport_exit(void)
>  {
> -       int i;
> -
> -       for (i = 0; i < n_vport_types; i++) {
> -               if (vport_ops_list[i]->exit)
> -                       vport_ops_list[i]->exit();
> -       }
> -
> -       kfree(vport_ops_list);
>         kfree(dev_table);
>  }
>
> @@ -222,7 +176,7 @@ struct vport *ovs_vport_add(const struct vport_parms
> *parms)
>         int err = 0;
>         int i;
>
> -       for (i = 0; i < n_vport_types; i++) {
> +       for (i = 0; i < ARRAY_SIZE(vport_ops_list); i++) {
>                 if (vport_ops_list[i]->type == parms->type) {
>                         struct hlist_head *bucket;
>
> diff --git a/datapath/vport.h b/datapath/vport.h
> index cba578c..56065a7 100644
> --- a/datapath/vport.h
> +++ b/datapath/vport.h
> @@ -94,8 +94,7 @@ struct vport {
>         struct ovs_vport_stats offset_stats;
>  };
>
> -#define VPORT_F_REQUIRED       (1 << 0) /* If init fails, module loading
> fails. */
> -#define VPORT_F_TUN_ID         (1 << 1) /* Sets OVS_CB(skb)->tun_id. */
> +#define VPORT_F_TUN_ID         (1 << 0) /* Sets OVS_CB(skb)->tun_id. */
>
>  /**
>   * struct vport_parms - parameters for creating a new vport
> @@ -124,10 +123,6 @@ struct vport_parms {
>   * @type: %OVS_VPORT_TYPE_* value for this type of virtual port.
>   * @flags: Flags of type VPORT_F_* that influence how the generic vport
> layer
>   * handles this vport.
> - * @init: Called at module initialization.  If VPORT_F_REQUIRED is set
> then the
> - * failure of this function will cause the module to not load.  If the
> flag is
> - * not set and initialzation fails then no vports of this type can be
> created.
> - * @exit: Called at module unload.
>   * @create: Create a new vport configured as specified.  On success
> returns
>   * a new vport allocated with ovs_vport_alloc(), otherwise an ERR_PTR()
> value.
>   * @destroy: Destroys a vport.  Must call vport_free() on the vport but
> not
> @@ -146,10 +141,6 @@ struct vport_ops {
>         enum ovs_vport_type type;
>         u32 flags;
>
> -       /* Called at module init and exit respectively. */
> -       int (*init)(void);
> -       void (*exit)(void);
> -
>         /* Called with ovs_mutex. */
>         struct vport *(*create)(const struct vport_parms *);
>         void (*destroy)(struct vport *);
> --
> 1.7.1
>
>
>
> ------------------------------
>
> Message: 2
> Date: Tue, 30 Apr 2013 17:11:17 -0700
> From: Jesse Gross <jesse at nicira.com>
> Subject: Re: [ovs-dev] [PATCH v2 1/2] tunneling: Remove struct
>         tnl_vport and   tnl_ops.
> To: Pravin B Shelar <pshelar at nicira.com>
> Cc: "dev at openvswitch.org" <dev at openvswitch.org>
> Message-ID:
>         <CAEP_g=
> 8fXsp_G3HpektAskHq9qcXLtEX-fEeMFDCjRQeS63XAA at mail.gmail.com>
> Content-Type: text/plain; charset=UTF-8
>
> 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?
>
>
> ------------------------------
>
> Message: 3
> Date: Tue, 30 Apr 2013 17:38:46 -0700
> From: Jesse Gross <jesse at nicira.com>
> Subject: Re: [ovs-dev] [PATCH v2 2/2] datapath: Move vport init to
>         First   port create.
> To: Pravin B Shelar <pshelar at nicira.com>
> Cc: "dev at openvswitch.org" <dev at openvswitch.org>
> Message-ID:
>         <CAEP_g=-
> 5y8XL7z48czFz6UKrLMUA9OcbyFFo6RV8URqAmA0OFQ at mail.gmail.com>
> Content-Type: text/plain; charset=UTF-8
>
> On Tue, Apr 30, 2013 at 4:21 PM, Pravin B Shelar <pshelar at nicira.com>
> wrote:
> > diff --git a/datapath/vport-gre.c b/datapath/vport-gre.c
> > index 2db4934..59e22be 100644
> > --- a/datapath/vport-gre.c
> > +++ b/datapath/vport-gre.c
> > @@ -327,10 +325,17 @@ static struct vport *gre_create(const struct
> vport_parms *parms)
> >         struct net *net = ovs_dp_get_net(parms->dp);
> >         struct ovs_net *ovs_net;
> >         struct vport *vport;
> > +       int err;
> > +
> > +       err = gre_init();
> > +       if (err)
> > +               return ERR_PTR(err);
> >
> >         ovs_net = net_generic(net, ovs_net_id);
> > -       if (ovsl_dereference(ovs_net->vport_net.gre_vport))
> > -               return ERR_PTR(-EEXIST);
> > +       if (ovsl_dereference(ovs_net->vport_net.gre_vport)) {
> > +               vport = ERR_PTR(-EEXIST);
> > +               goto error;
> > +       }
>
> I think that we need to jump to error in the case that
> ovs_vport_alloc() fails as well.
>
> > diff --git a/datapath/vport.h b/datapath/vport.h
> > index cba578c..56065a7 100644
> > --- a/datapath/vport.h
> > +++ b/datapath/vport.h
> > @@ -94,8 +94,7 @@ struct vport {
> >         struct ovs_vport_stats offset_stats;
> >  };
> >
> > -#define VPORT_F_REQUIRED       (1 << 0) /* If init fails, module
> loading fails. */
> > -#define VPORT_F_TUN_ID         (1 << 1) /* Sets OVS_CB(skb)->tun_id. */
> > +#define VPORT_F_TUN_ID         (1 << 0) /* Sets OVS_CB(skb)->tun_id. */
>
> This is separate but we should probably rename TUN_ID now that it's
> more than just the ID. Maybe we can even just remove it (for example,
> force people to initialize it) now that it's the only remaining flag.
>
>
> ------------------------------
>
> Message: 4
> Date: Tue, 30 Apr 2013 17:49:24 -0700
> From: Pravin Shelar <pshelar at nicira.com>
> Subject: Re: [ovs-dev] [PATCH v2 1/2] tunneling: Remove struct
>         tnl_vport and   tnl_ops.
> To: Jesse Gross <jesse at nicira.com>
> Cc: "dev at openvswitch.org" <dev at openvswitch.org>
> Message-ID:
>         <CALnjE+pjpwLGZRm0pt159Kvai=23F7VN8gaQPeV2i=
> RrfG3Swg at mail.gmail.com>
> Content-Type: text/plain; charset=ISO-8859-1
>
> On Tue, Apr 30, 2013 at 5:11 PM, Jesse Gross <jesse at nicira.com> wrote:
> > 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().
> >
>
> 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 at openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
>
>
> End of dev Digest, Vol 45, Issue 173
> ************************************
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openvswitch.org/pipermail/ovs-dev/attachments/20130501/0f1562ef/attachment-0003.html>


More information about the dev mailing list