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

[Romain Lenglet]

On Jan 17, 2013, at 2:45 PM, Ben Pfaff <blp at nicira.com> wrote:

Thanks for your comments!
I have updated patches 1/4, 2/4, and 4/4 according to your comments.

I really appreciate your patch for the sparse warnings.

[...]

> /* Cf. IETF RFC 5101 Section 3.1. */
> struct ipfix_header {
> -    uint16_t version;  /* IPFIX_VERSION. */
> -    uint16_t length;  /* Length in bytes including this header. */
> -    uint32_t export_time;  /* Seconds since the epoch. */
> -    uint32_t seq_number;  /* Message sequence number. */
> -    uint32_t obs_domain_id;  /* Observation Domain ID. */
> +    ovs_be16 version;  /* IPFIX_VERSION. */
> +    ovs_be16 length;  /* Length in bytes including this header. */
> +    ovs_be32 export_time;  /* Seconds since the epoch. */
> +    ovs_be32 seq_number;  /* Message sequence number. */
> +    ovs_be32 obs_domain_id;  /* Observation Domain ID. */
> } __attribute__((packed));
> BUILD_ASSERT_DECL(sizeof(struct ipfix_header) == 16);

Sorry for having missed this one.
That's weird, I checked with sparse. I must do it wrong. :/

[...]

> 
> I'm also getting a build failure due to:
>    The distribution is missing the following files:
>    ofproto/ipfix-gen-entities
> so I think you need to use EXTRA_DIST (or add a new
> dist_noinst_SCRIPTS) instead of noinst_SCRIPTS.

dist_noinst_SCRIPTS does the trick. make distcheck passes now.

> This is going to require some rethinking in a bit, because we are
> moving from a model where the in_port uniquely identifies the ofproto
> associated with an action to one where it doesn't: because patch ports
> are going to be implemented in userspace, an action might be
> associated with an ofproto other than the one where the packet
> arrived.  So we need to get a bit more information from the kernel to
> userspace somehow.

I guess that will require increasing the cookie size or use the cookie as an index?

[...]
--
Romain Lenglet