On Wed, Jun 01, 2011 at 10:16:47AM -0700, Jesse Gross wrote:
> On Wed, Jun 1, 2011 at 10:13 AM, Ben Pfaff <[email protected]> wrote:
> > On Wed, Jun 01, 2011 at 10:08:00AM -0700, Jesse Gross wrote:
> >> I think we need to be careful about ensuring that the layer pointers
> >> are correctly initialized. ??If we need to execute an action later on
> >> it is assumed that the pointers have been set, as they usually are by
> >> flow_extract(). ??It looks like that is true going into this function
> >> (although not strictly guaranteed) because in order to generate the
> >> flow we would have needed to call flow_extract() in the past.
> >> However, if we go down the 'mutates' branch then we put the data in a
> >> new ofpbuf and lose those pointers.
> >
> > Oops, you're right of course.
> >
> > Do you think it's better to copy the layer pointers or just to find
> > them again with flow_extract()?
>
> I would probably just run flow_extract() again. It's more robust,
> matches what we do in the kernel, and is not performance critical.
OK, revised incremental:
diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 88960ec..5f37197 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -944,7 +944,7 @@ dpif_netdev_flow_dump_done(const struct dpif *dpif
OVS_UNUSED, void *state_)
static int
dpif_netdev_execute(struct dpif *dpif,
- const struct nlattr *k OVS_UNUSED, size_t k_len OVS_UNUSED,
+ const struct nlattr *key_attrs, size_t key_len,
const struct nlattr *actions, size_t actions_len,
const struct ofpbuf *packet)
{
@@ -976,7 +976,10 @@ dpif_netdev_execute(struct dpif *dpif,
* if we don't. */
copy = *packet;
}
+
flow_extract(©, 0, -1, &key);
+ dpif_netdev_flow_from_nlattrs(key_attrs, key_len, &key);
+
error = dp_netdev_execute_actions(dp, ©, &key, actions, actions_len);
if (mutates) {
ofpbuf_uninit(©);
_______________________________________________
dev mailing list
[email protected]
http://openvswitch.org/mailman/listinfo/dev