[ovs-dev] [PATCH 1/2] flow: Factor out flag parsing and formatting routines.

[Jesse Gross]

On Thu, Jul 16, 2015 at 12:23 AM, Ben Pfaff <blp at nicira.com> wrote:
> On Wed, Jul 15, 2015 at 03:53:27PM -0700, Jesse Gross wrote:
>> On Wed, Jul 15, 2015 at 11:35 AM, Ben Pfaff <blp at 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 <jesse at 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.
>
> OK, thanks, I appreciate the explanation.
>
>> > format_flags_masked() will output the empty string if flags == 0 && mask
>> > == 0, and I think that parse_flags() will accept that too, but I don't
>> > know if it's the most user-friendly form.  Don't know if that matters.
>>
>> I agree, it's clearer and less ambiguous to output something in all
>> cases. I added this:
>
> Thanks!

Thanks, I pushed this series to master.