[ovs-dev] [PATCH 1/2] datapath: Rearrange struct sw_flow_key to put optional information last.

Jesse Gross jesse at nicira.com
Tue Apr 26 22:02:42 UTC 2011


On Mon, Apr 25, 2011 at 6:11 PM, Andrew Evans <aevans at nicira.com> wrote:
> IPv4/IPv6-specific fields are currently scattered about struct
> sw_flow_table. Group them together in a union at the end to make it possible to

This should be struct sw_flow_key, right?

> diff --git a/datapath/flow.h b/datapath/flow.h
> index 5c23279..a7c1f69 100644
> --- a/datapath/flow.h
> +++ b/datapath/flow.h
>  struct sw_flow_key {
[...]
> +       u8     nw_proto;                /* IP protocol or lower 8 bits of
> +                                        * ARP opcode. */
> +       u8     nw_tos;                  /* IP ToS (DSCP field, 6 bits). */

It might be worth putting these two fields in a union with ARP opcode.
 That would allow us to match the full opcode (not that it really
matters in practice but it's nice to not have artificial limitations
where it's unnecessary) and make things a little easier to read.

I guess until userspace can either handle the full opcode or
mismatched kernel/userspace flows we probably shouldn't do it though.

> +                       union {
> +                               struct {
> +                                       __be16 src; /* TCP/UDP source port. */
> +                                       __be16 dst; /* TCP/UDP destination port.
> +                                                    */

I don't think you need to be quite so aggressive about line wrapping.
Here I think it decreases readability.

> +                               } tp;
> +                               struct in6_addr nd_target; /* ND target address.
> +                                                           */
> +                       };

I don't think that these can be in a union together because IPv6
neighbor discovery packets are ICMP messages so they have both a
type/code and the ND addresses (unlike IPv4).

> +                       u8 nd_sha[ETH_ALEN]; /* ND source hardware address. */
> +                       u8 nd_tha[ETH_ALEN]; /* ND target hardware address. */

IPv6 calls these SLL/TLL (source/target link layer) so we should
probably use the correct names now that they are broken out.



More information about the dev mailing list