Hey Thomas, Thx a lot for the review, again, comments in line,
On Wed, Mar 19, 2014 at 4:37 AM, Thomas Graf <tg...@redhat.com> wrote: > On 03/18/2014 01:15 AM, Alex Wang wrote: > >> In order to allow handlers directly read upcalls from datapath, >> we need to support per-handler netlink socket for each vport in >> datapath. This commit makes this happen. Also, it is guaranteed >> backward and forward compatibility with previous branch. >> >> Signed-off-by: Alex Wang <al...@nicira.com> >> > > FYI, recent netdev sub thread on the subject of % usage: > http://www.spinics.net/lists/netdev/msg275467.html > Thanks for sharing the link. I'll remember to take advantage of it, when the divisor is constant. > Patch looks good to me now. > > Acked-by: Thomas Graf <tg...@redhat.com> > > Great to see this! > Two very minor comments included below just in case you need to > respin anyway: > > +int ovs_vport_set_upcall_portids(struct vport *vport, struct nlattr >> *ids) >> +{ >> + struct vport_portids *old, *vport_portids; >> + >> > [...] > > + vport_portids->rn_ids = reciprocal_value(vport_portids->n_ids); >> + memcpy(vport_portids->ids, nla_data(ids), nla_len(ids)); >> > > I missed this before, this could be replaced with nla_memcpy(). > Thanks, I'll take your suggestion, > +struct vport_portids { >> + struct rcu_head rcu; >> + u32 n_ids; >> + struct reciprocal_value rn_ids; >> + u32 *ids; >> +}; >> > > rcu_head is typically placed at the end of structs for cachline > efficiency as the struct grows (unlikely in this case ;) > Thx for brining up this issue. I'll placed it on top of 'u32 *ids'.
_______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
