On 15 July 2015 at 15:53, Jesse Gross <je...@nicira.com> wrote: > On Wed, Jul 15, 2015 at 11:35 AM, Ben Pfaff <b...@nicira.com> wrote: >> On Mon, Jul 13, 2015 at 02:53:56PM -0700, Jesse Gross wrote: >>> There are several implementations of functions that parse/format >>> flags and their binary representation. This factors them out into >>> common routines. In addition to reducing code, it also makes things >>> more consistent across different parts of OVS. >>> >>> Signed-off-by: Jesse Gross <je...@nicira.com> >> >> Thanks, I like reducing code size! >> >> This patch deletes a test and part of a test in ovs-ofctl.at. Why? > > I'm not sure that these tests really make sense and arguably the > behavior there were checking wasn't even right in the first place. In > theory they are verifying that the correct OpenFlow version is > selected based on the fields provided (in this case the correct > version is "none") but it is being done using names that weren't > supposed to be public, though the old parsing code allowed them to > leak through. The new parsing code is enforcing the same invariants as > before but more carefully and now rejects these commands as a parse > error before it even gets to the OpenFlow layer that is supposed to be > exercised. The next patch makes this field valid anyways and verifies > the correct OpenFlow behavior, so it didn't seem like it made sense to > keep the test around.
Hmm, interesting. One of the bugs found by STACK recently also changes the output of this test -- that bug was long-lived, in parse_ofp_str__(). I plan to send the patches soonish. As I understand, these tests are also meant to pick up on fields that aren't supposed to be exposed, and make sure that ovs-ofctl doesn't try to pass them on. That said, if the field will be allowed after the next patch then I don't object. _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
