On Thu, Feb 2, 2012 at 6:58 AM, Pravin B Shelar <[email protected]> wrote:
> diff --git a/datapath/actions.c b/datapath/actions.c
> index 824791d..cc238c4 100644
> --- a/datapath/actions.c
> +++ b/datapath/actions.c
> @@ -248,7 +248,7 @@ static int do_output(struct datapath *dp, struct sk_buff
> *skb, int out_port)
> if (unlikely(!skb))
> return -ENOMEM;
>
> - vport = rcu_dereference(dp->ports[out_port]);
> + vport = ovs_vport(dp, out_port);
ovs_vport() and rcu_dereference() aren't equivalent because the latter
verifies that rcu_read_lock is held.
> @@ -1391,6 +1404,11 @@ static int ovs_dp_cmd_new(struct sk_buff *skb, struct
> genl_info *info)
> }
> ovs_dp_set_net(dp, hold_net(sock_net(skb->sk)));
>
> + err = flex_array_init(&dp->ports, sizeof(struct vport *),
> + DP_MAX_PORTS, GFP_KERNEL|__GFP_ZERO);
Please put spaces around the pipe.
> diff --git a/datapath/datapath.h b/datapath/datapath.h
> index b012a76..05f6de8 100644
> --- a/datapath/datapath.h
> +++ b/datapath/datapath.h
> @@ -34,7 +34,10 @@
> #include "vlan.h"
> #include "vport.h"
>
> -#define DP_MAX_PORTS 1024
> +/* Flex array limit. */
> +#define DP_MAX_PORTS (FLEX_ARRAY_ELEMENTS_PER_PART(sizeof(struct vport *)) *
> \
> + FLEX_ARRAY_NR_BASE_PTRS)
In most situations the number of ports is actually small enough that
it's possible to fit them all in the single page version of flex
array. It would be nice if we could take advantage of that, although
that depends on what the other changes look like since we'll need to
be able to pivot from the single page to multiple pages safely if the
need arises.
> +static inline struct vport *ovs_vport(const struct datapath *dp, int id)
> +{
> + return flex_array_get_ptr(&dp->ports, id);
> +}
> +
> +static inline struct vport *ovs_vport_check(const struct datapath *dp, int
> id)
> +{
> + WARN_ON(!rcu_read_lock_held() && !rtnl_is_locked());
> + return ovs_vport(dp, id);
> +}
> +
> +static inline struct vport *ovs_vport_protected(const struct datapath *dp,
> int id)
> +{
> + ASSERT_RTNL();
> + return ovs_vport(dp, id);
> +}
These function names aren't as descriptive as the ones they replace -
it would be good to include rcu or rtnl in the name to indicate what
condition is verified.
_______________________________________________
dev mailing list
[email protected]
http://openvswitch.org/mailman/listinfo/dev
