[ovs-dev] [Bug 2462] [PATCH v4] datapath: Increase maximum number of datapath ports.

Jesse Gross jesse at nicira.com
Thu Feb 16 15:26:20 PST 2012


On Thu, Feb 16, 2012 at 1:27 PM, Pravin B Shelar <pshelar at nicira.com> wrote:
> diff --git a/datapath/datapath.c b/datapath/datapath.c
> index 220c7dd..9d3c997 100644
> --- a/datapath/datapath.c
> +++ b/datapath/datapath.c
> +static struct hlist_head *vport_hash_bucket(const struct datapath *dp, u16 port_no)

Some of the added lines are pretty long.  Can you wrap them where it
makes sense?

> diff --git a/datapath/datapath.h b/datapath/datapath.h
> index b012a76..8b8bd00 100644
> --- a/datapath/datapath.h
> +++ b/datapath/datapath.h
> +static inline struct vport *ovs_vport_rcu(const struct datapath *dp, int id)
> +{
> +       WARN_ON_ONCE(!rcu_read_lock_held());
> +       return ovs_lookup_vport(dp, id);
> +}
> +
> +static inline struct vport *ovs_vport_rtnl_rcu(const struct datapath *dp, int id)
> +{
> +       WARN_ON_ONCE(!rcu_read_lock_held() && !rtnl_is_locked());
> +       return ovs_lookup_vport(dp, id);
> +}
> +
> +static inline struct vport *ovs_vport_rtnl(const struct datapath *dp, int id)
> +{
> +       ASSERT_RTNL();
> +       return ovs_lookup_vport(dp, id);
> +}

Can you make the same changes to the id argument that you did in
ovs_lookup_vport()?

> diff --git a/lib/dpif-linux.c b/lib/dpif-linux.c
> index c06bb89..43e816f 100644
> --- a/lib/dpif-linux.c
> +++ b/lib/dpif-linux.c
> @@ -59,10 +59,8 @@
>  #include "vlog.h"
>
>  VLOG_DEFINE_THIS_MODULE(dpif_linux);
> -
> -enum { LRU_MAX_PORTS = 1024 };
> -enum { LRU_MASK = LRU_MAX_PORTS - 1};
> -BUILD_ASSERT_DECL(IS_POW2(LRU_MAX_PORTS));
> +enum { MAX_PORTS = USHRT_MAX };
> +BUILD_ASSERT_DECL(IS_POW2(MAX_PORTS + 1));

There's no longer any reason that MAX_PORTS has to be a power of 2, right?

> @@ -409,9 +373,14 @@ dpif_linux_port_add(struct dpif *dpif_, struct netdev *netdev,
>             *port_nop = reply.port_no;
>             VLOG_DBG("%s: assigning port %"PRIu32" to netlink pid %"PRIu32,
>                      dpif_name(dpif_), request.port_no, upcall_pid);
> +        } else if (error == EFBIG) {
> +            /* Older datapath has lower limit. */
> +            max_ports = dpif->alloc_port_no;
> +            dpif->alloc_port_no = 0;

Port number zero will never be available since it is the local port.
It would be better if we started at 1 (both initially and on wrap
around).



More information about the dev mailing list