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(&copy, 0, -1, &key);
+    dpif_netdev_flow_from_nlattrs(key_attrs, key_len, &key);
+
     error = dp_netdev_execute_actions(dp, &copy, &key, actions, actions_len);
     if (mutates) {
         ofpbuf_uninit(&copy);
_______________________________________________
dev mailing list
[email protected]
http://openvswitch.org/mailman/listinfo/dev

Reply via email to