[ovs-dev] [PATCH/RFC v2 3/8] Support decoding of NMX selection method

Simon Horman simon.horman at netronome.com
Sun Feb 15 19:13:14 UTC 2015


On Sun, Feb 15, 2015 at 08:51:31AM -0800, Ben Pfaff wrote:
> On Fri, Feb 13, 2015 at 06:14:28PM -0500, Simon Horman wrote:
> > On Fri, Feb 13, 2015 at 02:55:58PM -0800, Ben Pfaff wrote:
> > > On Fri, Jan 30, 2015 at 11:41:51AM +0900, Simon Horman wrote:
> > > > This is in preparation for supporting group mod and desc reply
> > > > messages with an NMX selection method group experimenter property.
> > > > 
> > > > NMX selection method
> > > > Signed-off-by: Simon Horman <simon.horman at netronome.com>
> > > > 
> > > > ---
> > > > v2 Use list of struct field_array of TLVs rather than OF1.1 match
> > > >    for fields field of NMX selection method property
> > > 
> > > It might be simpler to use an actual array for struct field_array.  I
> > > guess that we can be sure there will be no more than MFF_N_IDS
> > > elements?
> > 
> > As duplicates are rejected I believe that we can make that assumption.
> > 
> > I think that should simplify the code somewhat at the
> > expense of some memory cost if the property is present.
> > I'm assuming that you are happy about the latter and
> > I'll see about making it so.
> 
> I'm not sure yet as I haven't read the rest of the series yet.  If
> you're unsure then you might want to wait for the rest of the reviews.

Thanks. I'll wait.

After writing my previous email I recalled the original motivation
for the empty string indicating that the existing behaviour should occur.

My original proposal extended the group mod message and as such the fields
to describe the selection method were always present. In that scenario
using a empty field to denote that the feature was turned off made sense
(at least to me).  Later, on your advice IIRC, I reworked things to use a
group mod property which may be present or absent. And at that time I
didn't modify the empty field behaviour which, as you pointed out, seems
unnecessary now.



More information about the dev mailing list