On Tue, Mar 18, 2014 at 12:42 PM, Ben Pfaff <b...@nicira.com> wrote: > On Tue, Mar 11, 2014 at 04:56:20PM -0700, Andy Zhou wrote: > > Add basic recirculation infrastructure and user space > > data path support for it. The following bond mega flow patch will > > make use of this infrastructure. > > > > Signed-off-by: Andy Zhou <az...@nicira.com> > > It seems odd to put RECIRC_ID_BASE and RECIRC_ID_SIZE into the kernel > header. I think they are only a detail of the userspace > implementation. > Good point, I will move them out.
> > Is dp_netdev_match_init() still correct? I think that we dropped the > requirement to always exact-match on metadata, right? > > It got removed in the next patch. But still needed here in order for code with this patch level to work. I will make sure it is removed in the next version. > In format_odp_recirc_action(), this looks awkward, could we just use > an "if" statement? > > + hash_str > + ? ds_put_format(ds, "recirc(%s(%"PRIu32"), %"PRIu32")", > + hash_str, ntohl(act->hash_bias), > ntohl(act->recirc_id)) > + : ds_put_format(ds, "recirc(%"PRIu32")", ntohl(act->recirc_id)); > > In odp_key_to_pkt_metadata(), I think it's time to use a switch > statement. If not, then let's at least keep adding "else"s to match > the existing style. > O.K. > > In odp_flow_key_to_flow__(), I'm not sure that it's really safe to > always make recirc_id exact match. Maybe all the flows we add the > kernel will exact match on it, but if the kernel gives us back one > that doesn't exact match on recirc_id, then it seems incorrect to > pretend that it did. > I will fix it by only make it exact match when (older datapath) does not provide a mask for rercirc_id. If they do, we will honor it. > > The large new comment in ofproto-dpif.h is useful. Can you format it > the same way as other large comments in the program, that is, like: > /* > * this > */ > Yes.
_______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
