[ovs-dev] [PATCH 4/4] ofproto: Implement IPFIX exporter

Ben Pfaff blp at nicira.com
Thu Jan 10 23:41:00 UTC 2013


On Thu, Jan 10, 2013 at 03:37:37PM -0800, Romain Lenglet wrote:
> On Jan 9, 2013, at 4:00 PM, Ben Pfaff <blp at nicira.com> wrote:
> > It looks like, in practice, the arguments to
> > ipfix_define_template_entity() are always fixed at compile time.  If
> > I'm reading the code correctly, then this means that each call to
> > ipfix_define_template_entity() is going through a "switch" statement
> > to find out a couple of values whose values could actually be fixed at
> > compile time.  So, how about something like this, instead:
> > 
> >   - at top level, add:
> > 
> > enum ipfix_entity_size {
> > #define IPFIX_ENTITY(ENUM, ID, SIZE, NAME)  IPFIX_ENTITY_SIZE_##ENUM,
> > #include "ofproto/ipfix-entities.def"
> > };
> > 
> >   - Change ipfix_define_template_entity() to just:
> > 
> > static void
> > ipfix_define_template_entity(enum ipfix_entity_id id, int size,
> >                             struct ofpbuf *msg)
> > {
> >    struct ipfix_template_field_specifier *field;
> > 
> >    field = ofpbuf_put_zeros(msg, sizeof *field);
> >    field->element_id = htons(id);
> >    field->field_length = htons(size);
> > }
> > 
> >   - Change DEF to just:
> > 
> > #define DEF(ID)                                                         \
> >    {                                                                   \
> >        ipfix_define_template_entity(IPFIX_ENTITY_ID_##ID,              \
> >                                     IPFIX_ENTITY_SIZE_##ID, msg);      \
> >        count++;                                                        \
> >    }
> > 
> > and thereby avoid runtime work for what we know at compile time.
> 
> How would that work?
> Nothing uses the SIZE argument of the IPFIX_ENTITY in your code
> above, and you're using the enum value in place of the size, which
> seems incorrect.

Oops, my definition was wrong.  Here's a corrected (but I didn't
build-test this version as I did the previous one):

    #define IPFIX_ENTITY(ENUM, ID, SIZE, NAME) \
        enum { IPFIX_ENTITY_SIZE_##ENUM = SIZE };
    #include "ofproto/ipfix-entities.def"

Unless I'm missing something again, the value of
IPFIX_ENTITY_SIZE_<field> is the size of <field>, so it should work
out OK.



More information about the dev mailing list