[ovs-dev] [PATCH 05/12] dpif: Use explicit packet metadata.

Ben Pfaff blp at nicira.com
Mon Nov 11 23:44:26 UTC 2013


On Fri, Nov 08, 2013 at 10:54:37AM -0800, Jarno Rajahalme wrote:
> This helps reduce confusion about when a flow is a flow
> and when it is just metadata.
> 
> Signed-off-by: Jarno Rajahalme <jrajahalme at nicira.com>

Clang says:

    ../lib/dpif-netdev.c:1186:39: error: missing field 'ip_src' initializer [-Werror,-Wmissing-field-initializers]
                struct dpif_metadata md = DPIF_METADATA_INITIALIZER(port->port_no);
                                          ^
    ../lib/dpif.h:503:47: note: expanded from macro 'DPIF_METADATA_INITIALIZER'
    #define DPIF_METADATA_INITIALIZER(PORT) { { 0 }, 0, 0, (PORT) }

sparse says:

    ../lib/dpif-netdev.c:1301:63: warning: incorrect type in argument 3 (different base types)
    ../lib/dpif-netdev.c:1301:63:    expected restricted odp_port_t [usertype] out_port
    ../lib/dpif-netdev.c:1301:63:    got unsigned int

Do you think that it is worthwhile to use a pointer in struct
dpif_execute?  It is more straightforward to just embed an struct
dpif_metadata, and I am not sure that there is a big penalty to doing
so.

odp_key_to_dpif_metadata() duplicates some code in
odp_flow_key_to_flow__(), but it is a bit difficult to remove the
duplication because of the difference between struct flow and struct
dpif_metadata.  I guess the best one could do is write a common helper
that takes individual pointers to a struct flow_tnl, an skb_priority,
a pkt_mark, and an in_port.  I don't know whether that is better.

What does "9 * 8" come from, for ODP_KEY_METADATA_SIZE?  (Is it
intended as an estimate or an exact value?)

The parameters to flow_extract() seem to be very closely related to
dpif_metadata, which suggests that this concept might be higher level
than dpif.

Thanks,

Ben.



More information about the dev mailing list