On Mon, Sep 24, 2012 at 10:12:59AM -0700, Ben Pfaff wrote: > On Thu, Sep 13, 2012 at 12:01:44PM +0900, Simon Horman wrote: > > On Wed, Sep 12, 2012 at 12:09:41PM -0700, Ben Pfaff wrote: > > > On Wed, Sep 12, 2012 at 05:44:32PM +0900, Simon Horman wrote: > > > > By passing a flow to the action parser the pre-requisites > > > > of set-feild actions will be checked. If the flow is NULL, > > > > for instance in test code such as ofctl_parse_ofp11_instructions(), > > > > then the check is skiped, as it always was before this change. > > > > > > > > Unfortunately I don't think that this check is correct as > > > > it does not take into account other actions actions that may > > > > be applied before the load action, e.g. vlan push/pop, which may > > > > affect the pre-requisites. > > > > > > > > I believe that in its current form there is scope for both false > > > > positives > > > > (not so bad, perhaps) and false negatives (pretty bad). I would welcome > > > > some input on if these concerns are valid and if so how they may be > > > > overcome. > > > > > > When I've thought about this problem before, I wasn't able to come up > > > with an example of when the prerequisite check would be wrong. Do you > > > have an example? > > > > I would very much like for this not to be a problem. > > But here are examples that I am concerned about. > > > > > > # MPLS false negative > > > > 1. Packet with no MPLS header arrives > > 2. Push MPLS action is applied > > 3. Set-Field:MPLS_LABEL action is applied > > > > The pre-requisite for 3 is Eth Type 0x8847 or 0x8848, > > but that isn't true until 2 is applied. > > I agree that this problem will need to be solved one way or another, > but I don't see how this change helps. I think that it is redundant, > too, because the same check will be done later in ofpacts_check(), > when the flow is always available. (That is why ofpacts_check() > exists, since a flow is not always available at parse time.) > > Also, this does not solve the whole problem anyway. When action #2 is > parsed, it does not change the flow, so the check in #3 will still > fail. (Maybe that is what you meant above when you wrote, > "Unfortunately I don't think this check is correct." I thought when I > read it earlier that you meant the previously missing check.)
Sorry for not having been clearer. What I meant is "I don't think this new check is correct". Which I think we agree on. I am interested in ideas on what a correct check might look like. But I am happy for that to be future work. _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
