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: >> On Wed, Jun 1, 2011 at 9:43 AM, Ben Pfaff <[email protected]> wrote: >> > @@ -963,6 +963,8 @@ dpif_netdev_execute(struct dpif *dpif, >> > ?? ?? ?? ?? return error; >> > ?? ?? } >> > >> > + ?? ??dpif_netdev_flow_from_nlattrs(key_attrs, key_len, &key); >> > + >> > ?? ?? if (mutates) { >> > ?? ?? ?? ?? /* We need a deep copy of 'packet' since we're going to modify >> > its >> > ?? ?? ?? ?? ??* data. */ >> > @@ -976,7 +978,7 @@ dpif_netdev_execute(struct dpif *dpif, >> > ?? ?? ?? ?? ??* if we don't. */ >> > ?? ?? ?? ?? copy = *packet; >> > ?? ?? } >> > - ?? ??flow_extract(©, 0, -1, &key); >> >> 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. _______________________________________________ dev mailing list [email protected] http://openvswitch.org/mailman/listinfo/dev
