On Nov 11, 2013, at 3:44 PM, Ben Pfaff <b...@nicira.com> wrote:
> 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 <jrajaha...@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.
>
Done.
> 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.
>
Yes, there is some duplication, but factoring that out is not very
straightforward, so I kept this as-is.
> What does "9 * 8" come from, for ODP_KEY_METADATA_SIZE? (Is it
> intended as an estimate or an exact value?)
It’s an estimate, clarified that in a comment.
>
> 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.
I renamed it as pkt_metadata and moved the definition to lib/packets.h.
Will send updated patches shortly.
Thanks,
Jarno
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
