[ovs-dev] [bond megaflow v2 4/5] dpif-netdev: user space datapath recirculation

Ben Pfaff blp at nicira.com
Tue Mar 18 19:42:25 UTC 2014


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 <azhou at 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.

Is dp_netdev_match_init() still correct?  I think that we dropped the
requirement to always exact-match on metadata, right?

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.

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.

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
 */



More information about the dev mailing list