[ovs-dev] [PATCH] ofp-print: Make port "config" and "state" output easier to read.

Ben Pfaff blp at nicira.com
Wed Jul 27 00:05:57 UTC 2011


On Tue, Jul 26, 2011 at 03:17:45PM -0700, Ethan Jackson wrote:
> I don't like the
> 
> > + ? ?static const struct bit_name feature_bits[] = {
> > + ? ? ? ?{ OFPPF_10MB_HD, ? ?"10MB-HD" },
> > + ? ? ? ?{ OFPPF_10MB_FD, ? ?"10MB-FD" },
> > + ? ? ? ?{ OFPPF_100MB_HD, ? "100MB-HD" },
> > + ? ? ? ?{ OFPPF_100MB_FD, ? "100MB-FD" },
> > + ? ? ? ?{ OFPPF_1GB_HD, ? ? "1GB-HD" },
> > + ? ? ? ?{ OFPPF_1GB_FD, ? ? "1GB-FD" },
> > + ? ? ? ?{ OFPPF_10GB_FD, ? ?"10GB-FD" },
> > + ? ? ? ?{ OFPPF_COPPER, ? ? "COPPER" },
> > + ? ? ? ?{ OFPPF_FIBER, ? ? ?"FIBER" },
> > + ? ? ? ?{ OFPPF_AUTONEG, ? ?"AUTO_NEG" },
> > + ? ? ? ?{ OFPPF_PAUSE, ? ? ?"AUTO_PAUSE" },
> > + ? ? ? ?{ OFPPF_PAUSE_ASYM, "AUTO_PAUSE_ASYM" },
> > + ? ? ? ?{ 0, ? ? ? ? ? ? ? ?NULL },
> 
> I don't like this kind of thing a great deal, because it's hard to
> remember to update when a new OFPPF appears.  It's a bigger deal in
> the case of nxast_actions[] in the ofp-util code.  It's really easy to
> forget to add new actions there, or update their sizes for that
> matter.

I agree.  The nxast_actions[] problem is written on my whiteboard as
something to fix.  I took a stab at it a couple of weeks ago when you
first mentioned it, but I didn't come up with a good solution, so I need
to take another look.

I think that it is less of a problem here because these constants are
part of OpenFlow 1.0 and haven't changed in ages.

I'll push this soon, thanks.



More information about the dev mailing list