Thx Pravin for the review, please see my comments inline,
> > + */
> > +int ovs_vport_set_upcall_portids(struct vport *vport, struct nlattr
> *ids)
> > +{
> > + struct vport_portids *old, *vport_portids;
> > +
> > + if (!nla_len(ids) || nla_len(ids) % sizeof(u32))
> > + return -EINVAL;
> > +
> > + old = ovsl_dereference(vport->upcall_portids);
> > +
> > + vport_portids = kmalloc(sizeof *vport_portids + nla_len(ids),
> > + GFP_KERNEL);
> need to divide nla_len() by 4.
>
Could you explain more a bit? Isn't nla_len returns the number of bytes in
payload?
> > + *
> > + * Returns the portid of the target socket. Must be called with
> rcu_read_lock.
> > + */
> > +u32 ovs_vport_find_portid(const struct vport *p, struct sk_buff *skb)
> > +{
> > + struct vport_portids *ids;
> > + u32 hash;
> > +
> > + ids = rcu_dereference_ovsl(p->upcall_portids);
> > +
> Is this function ever called under ovs-lock? if not we can just use
> rcu_dereference.
Thx, I'm clear about the usage of rcu_dereference_ovsl() and
ovsl_dereference() now.
Will fix it and the comments.
> > };
> > +/**
> > + * struct vport_portids - array of netlink portids of a vport.
> > + * must be protected by rcu.
> > + * @rcu: RCU callback head for deferred destruction.
> > + * @n_ids: Size of @ids array.
> > + * @rn_ids: The reciprocal value of @n_ids.
> > + * @ids: Array storing the Netlink socket pids to use for packets
> received
> > + * on this port that miss the flow table.
> > + */
> > +struct vport_portids {
> > + u32 n_ids;
> > + struct reciprocal_value rn_ids;
> > + struct rcu_head rcu;
> > + u32 *ids;
> > +};
> >
> if we use u32 ids[], we can save pointer space. Same as we have done
> in struct sw_flow{} for stats.
Thx, I'll adjust accordingly,
> > /**
> > diff --git a/include/linux/openvswitch.h b/include/linux/openvswitch.h
> > index a88f6f1..b3977d1 100644
> > --- a/include/linux/openvswitch.h
> > +++ b/include/linux/openvswitch.h
> > @@ -138,6 +138,9 @@ struct ovs_vport_stats {
> > /* Allow last Netlink attribute to be unaligned */
> > #define OVS_DP_F_UNALIGNED (1 << 0)
> >
> > +/* Allow datapath to associate multiple Netlink PIDs to each vport */
> > +#define OVS_DP_F_VPORT_PIDS (1 << 1)
> > +
> Have you tested downgrade case with this patch?
>
>
Yes, I've tested it,
_______________________________________________
dev mailing list
[email protected]
http://openvswitch.org/mailman/listinfo/dev
