On Thu, Feb 16, 2012 at 3:26 PM, Jesse Gross <je...@nicira.com> wrote:
> On Thu, Feb 16, 2012 at 1:27 PM, Pravin B Shelar <pshe...@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?
>
ok
>> 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()?
>
ok.
>> 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?
>
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).
right, thats why increment done before assigning to request.port_no.
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
