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(&copy, 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()?
_______________________________________________
dev mailing list
[email protected]
http://openvswitch.org/mailman/listinfo/dev

Reply via email to