openvswitch: Compact sw_flow_key.

Minimize padding in sw_flow_key and move 'tp' top the main struct.
These changes simplify code when accessing the transport port numbers
and the tcp flags, and makes the sw_flow_key 8 bytes smaller on 64-bit
systems (128->120 bytes).  These changes also make the keys for IPv4
packets to fit in one cache line.

There is a valid concern for safety of packing the struct
ovs_key_ipv4_tunnel, as it would be possible to take the address of
the tun_id member as a __be64 * which could result in unaligned access
in some systems. However:

- sw_flow_key itself is 64-bit aligned, so the tun_id within is
  always
  64-bit aligned.
- We never make arrays of ovs_key_ipv4_tunnel (which would force
  every
  second tun_key to be misaligned).
- We never take the address of the tun_id in to a __be64 *.
- Whereever we use struct ovs_key_ipv4_tunnel outside the
  sw_flow_key,
  it is in stack (on tunnel input functions), where compiler has full
  control of the alignment.

Signed-off-by: Jarno Rajahalme <jrajahalme@nicira.com>
Signed-off-by: Pravin B Shelar <pshelar@nicira.com>

diff --git a/net/openvswitch/flow.c b/net/openvswitch/flow.c
index e0fc12bbeeb165e89ce1a844a9df0a75910bcd12..6d8d2da0a8ec7cf01de3e30e12efaf16e36988f6 100644 (file)
--- a/net/openvswitch/flow.c
+++ b/net/openvswitch/flow.c
@@ -64,17 +64,11 @@ u64 ovs_flow_used_time(unsigned long flow_jiffies)
 void ovs_flow_stats_update(struct sw_flow *flow, struct sk_buff *skb)
 {
        struct flow_stats *stats;
-       __be16 tcp_flags = 0;
+       __be16 tcp_flags = flow->key.tp.flags;
        int node = numa_node_id();
 
        stats = rcu_dereference(flow->stats[node]);
 
-       if (likely(flow->key.ip.proto == IPPROTO_TCP)) {
-               if (likely(flow->key.eth.type == htons(ETH_P_IP)))
-                       tcp_flags = flow->key.ipv4.tp.flags;
-               else if (likely(flow->key.eth.type == htons(ETH_P_IPV6)))
-                       tcp_flags = flow->key.ipv6.tp.flags;
-       }
        /* Check if already have node-specific stats. */
        if (likely(stats)) {
                spin_lock(&stats->lock);
@@ -357,8 +351,8 @@ static int parse_icmpv6(struct sk_buff *skb, struct sw_flow_key *key,
        /* The ICMPv6 type and code fields use the 16-bit transport port
         * fields, so we need to store them in 16-bit network byte order.
         */
-       key->ipv6.tp.src = htons(icmp->icmp6_type);
-       key->ipv6.tp.dst = htons(icmp->icmp6_code);
+       key->tp.src = htons(icmp->icmp6_type);
+       key->tp.dst = htons(icmp->icmp6_code);
 
        if (icmp->icmp6_code == 0 &&
            (icmp->icmp6_type == NDISC_NEIGHBOUR_SOLICITATION ||
@@ -520,21 +514,21 @@ int ovs_flow_extract(struct sk_buff *skb, u16 in_port, struct sw_flow_key *key)
                if (key->ip.proto == IPPROTO_TCP) {
                        if (tcphdr_ok(skb)) {
                                struct tcphdr *tcp = tcp_hdr(skb);
-                               key->ipv4.tp.src = tcp->source;
-                               key->ipv4.tp.dst = tcp->dest;
-                               key->ipv4.tp.flags = TCP_FLAGS_BE16(tcp);
+                               key->tp.src = tcp->source;
+                               key->tp.dst = tcp->dest;
+                               key->tp.flags = TCP_FLAGS_BE16(tcp);
                        }
                } else if (key->ip.proto == IPPROTO_UDP) {
                        if (udphdr_ok(skb)) {
                                struct udphdr *udp = udp_hdr(skb);
-                               key->ipv4.tp.src = udp->source;
-                               key->ipv4.tp.dst = udp->dest;
+                               key->tp.src = udp->source;
+                               key->tp.dst = udp->dest;
                        }
                } else if (key->ip.proto == IPPROTO_SCTP) {
                        if (sctphdr_ok(skb)) {
                                struct sctphdr *sctp = sctp_hdr(skb);
-                               key->ipv4.tp.src = sctp->source;
-                               key->ipv4.tp.dst = sctp->dest;
+                               key->tp.src = sctp->source;
+                               key->tp.dst = sctp->dest;
                        }
                } else if (key->ip.proto == IPPROTO_ICMP) {
                        if (icmphdr_ok(skb)) {
@@ -542,8 +536,8 @@ int ovs_flow_extract(struct sk_buff *skb, u16 in_port, struct sw_flow_key *key)
                                /* The ICMP type and code fields use the 16-bit
                                 * transport port fields, so we need to store
                                 * them in 16-bit network byte order. */
-                               key->ipv4.tp.src = htons(icmp->type);
-                               key->ipv4.tp.dst = htons(icmp->code);
+                               key->tp.src = htons(icmp->type);
+                               key->tp.dst = htons(icmp->code);
                        }
                }
 
@@ -589,21 +583,21 @@ int ovs_flow_extract(struct sk_buff *skb, u16 in_port, struct sw_flow_key *key)
                if (key->ip.proto == NEXTHDR_TCP) {
                        if (tcphdr_ok(skb)) {
                                struct tcphdr *tcp = tcp_hdr(skb);
-                               key->ipv6.tp.src = tcp->source;
-                               key->ipv6.tp.dst = tcp->dest;
-                               key->ipv6.tp.flags = TCP_FLAGS_BE16(tcp);
+                               key->tp.src = tcp->source;
+                               key->tp.dst = tcp->dest;
+                               key->tp.flags = TCP_FLAGS_BE16(tcp);
                        }
                } else if (key->ip.proto == NEXTHDR_UDP) {
                        if (udphdr_ok(skb)) {
                                struct udphdr *udp = udp_hdr(skb);
-                               key->ipv6.tp.src = udp->source;
-                               key->ipv6.tp.dst = udp->dest;
+                               key->tp.src = udp->source;
+                               key->tp.dst = udp->dest;
                        }
                } else if (key->ip.proto == NEXTHDR_SCTP) {
                        if (sctphdr_ok(skb)) {
                                struct sctphdr *sctp = sctp_hdr(skb);
-                               key->ipv6.tp.src = sctp->source;
-                               key->ipv6.tp.dst = sctp->dest;
+                               key->tp.src = sctp->source;
+                               key->tp.dst = sctp->dest;
                        }
                } else if (key->ip.proto == NEXTHDR_ICMP) {
                        if (icmp6hdr_ok(skb)) {

diff --git a/net/openvswitch/flow.h b/net/openvswitch/flow.h
index ddcebc53224f0d1c91ad0c7a8158db8236aafd41..a292bf8ad75cbc62b2447a7d96288da85c1b9703 100644 (file)
--- a/net/openvswitch/flow.h
+++ b/net/openvswitch/flow.h
@@ -47,7 +47,7 @@ struct ovs_key_ipv4_tunnel {
        __be16 tun_flags;
        u8   ipv4_tos;
        u8   ipv4_ttl;
-};
+} __packed __aligned(4); /* Minimize padding. */
 
 static inline void ovs_flow_tun_key_init(struct ovs_key_ipv4_tunnel *tun_key,
                                         const struct iphdr *iph, __be64 tun_id,
@@ -71,7 +71,7 @@ struct sw_flow_key {
                u32     priority;       /* Packet QoS priority. */
                u32     skb_mark;       /* SKB mark. */
                u16     in_port;        /* Input switch port (or DP_MAX_PORTS). */
-       } phy;
+       } __packed phy; /* Safe when right after 'tun_key'. */
        struct {
                u8     src[ETH_ALEN];   /* Ethernet source address. */
                u8     dst[ETH_ALEN];   /* Ethernet destination address. */
@@ -84,23 +84,21 @@ struct sw_flow_key {
                u8     ttl;             /* IP TTL/hop limit. */
                u8     frag;            /* One of OVS_FRAG_TYPE_*. */
        } ip;
+       struct {
+               __be16 src;             /* TCP/UDP/SCTP source port. */
+               __be16 dst;             /* TCP/UDP/SCTP destination port. */
+               __be16 flags;           /* TCP flags. */
+       } tp;
        union {
                struct {
                        struct {
                                __be32 src;     /* IP source address. */
                                __be32 dst;     /* IP destination address. */
                        } addr;
-                       union {
-                               struct {
-                                       __be16 src;             /* TCP/UDP/SCTP source port. */
-                                       __be16 dst;             /* TCP/UDP/SCTP destination port. */
-                                       __be16 flags;           /* TCP flags. */
-                               } tp;
-                               struct {
-                                       u8 sha[ETH_ALEN];       /* ARP source hardware address. */
-                                       u8 tha[ETH_ALEN];       /* ARP target hardware address. */
-                               } arp;
-                       };
+                       struct {
+                               u8 sha[ETH_ALEN];       /* ARP source hardware address. */
+                               u8 tha[ETH_ALEN];       /* ARP target hardware address. */
+                       } arp;
                } ipv4;
                struct {
                        struct {
@@ -108,11 +106,6 @@ struct sw_flow_key {
                                struct in6_addr dst;    /* IPv6 destination address. */
                        } addr;
                        __be32 label;                   /* IPv6 flow label. */
-                       struct {
-                               __be16 src;             /* TCP/UDP/SCTP source port. */
-                               __be16 dst;             /* TCP/UDP/SCTP destination port. */
-                               __be16 flags;           /* TCP flags. */
-                       } tp;
                        struct {
                                struct in6_addr target; /* ND target address. */
                                u8 sll[ETH_ALEN];       /* ND source link layer address. */

diff --git a/net/openvswitch/flow_netlink.c b/net/openvswitch/flow_netlink.c
index 32a725cfeb0e83d06b812a13ae3f827a0d65b36c..d757848da89ca734ac95cef806c2dd314f881bf6 100644 (file)
--- a/net/openvswitch/flow_netlink.c
+++ b/net/openvswitch/flow_netlink.c
@@ -204,11 +204,11 @@ static bool match_validate(const struct sw_flow_match *match,
                                if (match->mask && (match->mask->key.ip.proto == 0xff))
                                        mask_allowed |= 1 << OVS_KEY_ATTR_ICMPV6;
 
-                               if (match->key->ipv6.tp.src ==
+                               if (match->key->tp.src ==
                                                htons(NDISC_NEIGHBOUR_SOLICITATION) ||
-                                   match->key->ipv6.tp.src == htons(NDISC_NEIGHBOUR_ADVERTISEMENT)) {
+                                   match->key->tp.src == htons(NDISC_NEIGHBOUR_ADVERTISEMENT)) {
                                        key_expected |= 1 << OVS_KEY_ATTR_ND;
-                                       if (match->mask && (match->mask->key.ipv6.tp.src == htons(0xffff)))
+                                       if (match->mask && (match->mask->key.tp.src == htons(0xffff)))
                                                mask_allowed |= 1 << OVS_KEY_ATTR_ND;
                                }
                        }
@@ -630,27 +630,18 @@ static int ovs_key_from_nlattrs(struct sw_flow_match *match, u64 attrs,
                const struct ovs_key_tcp *tcp_key;
 
                tcp_key = nla_data(a[OVS_KEY_ATTR_TCP]);
-               if (orig_attrs & (1 << OVS_KEY_ATTR_IPV4)) {
-                       SW_FLOW_KEY_PUT(match, ipv4.tp.src,
-                                       tcp_key->tcp_src, is_mask);
-                       SW_FLOW_KEY_PUT(match, ipv4.tp.dst,
-                                       tcp_key->tcp_dst, is_mask);
-               } else {
-                       SW_FLOW_KEY_PUT(match, ipv6.tp.src,
-                                       tcp_key->tcp_src, is_mask);
-                       SW_FLOW_KEY_PUT(match, ipv6.tp.dst,
-                                       tcp_key->tcp_dst, is_mask);
-               }
+               SW_FLOW_KEY_PUT(match, tp.src, tcp_key->tcp_src, is_mask);
+               SW_FLOW_KEY_PUT(match, tp.dst, tcp_key->tcp_dst, is_mask);
                attrs &= ~(1 << OVS_KEY_ATTR_TCP);
        }
 
        if (attrs & (1 << OVS_KEY_ATTR_TCP_FLAGS)) {
                if (orig_attrs & (1 << OVS_KEY_ATTR_IPV4)) {
-                       SW_FLOW_KEY_PUT(match, ipv4.tp.flags,
+                       SW_FLOW_KEY_PUT(match, tp.flags,
                                        nla_get_be16(a[OVS_KEY_ATTR_TCP_FLAGS]),
                                        is_mask);
                } else {
-                       SW_FLOW_KEY_PUT(match, ipv6.tp.flags,
+                       SW_FLOW_KEY_PUT(match, tp.flags,
                                        nla_get_be16(a[OVS_KEY_ATTR_TCP_FLAGS]),
                                        is_mask);
                }
@@ -661,17 +652,8 @@ static int ovs_key_from_nlattrs(struct sw_flow_match *match, u64 attrs,
                const struct ovs_key_udp *udp_key;
 
                udp_key = nla_data(a[OVS_KEY_ATTR_UDP]);
-               if (orig_attrs & (1 << OVS_KEY_ATTR_IPV4)) {
-                       SW_FLOW_KEY_PUT(match, ipv4.tp.src,
-                                       udp_key->udp_src, is_mask);
-                       SW_FLOW_KEY_PUT(match, ipv4.tp.dst,
-                                       udp_key->udp_dst, is_mask);
-               } else {
-                       SW_FLOW_KEY_PUT(match, ipv6.tp.src,
-                                       udp_key->udp_src, is_mask);
-                       SW_FLOW_KEY_PUT(match, ipv6.tp.dst,
-                                       udp_key->udp_dst, is_mask);
-               }
+               SW_FLOW_KEY_PUT(match, tp.src, udp_key->udp_src, is_mask);
+               SW_FLOW_KEY_PUT(match, tp.dst, udp_key->udp_dst, is_mask);
                attrs &= ~(1 << OVS_KEY_ATTR_UDP);
        }
 
@@ -679,17 +661,8 @@ static int ovs_key_from_nlattrs(struct sw_flow_match *match, u64 attrs,
                const struct ovs_key_sctp *sctp_key;
 
                sctp_key = nla_data(a[OVS_KEY_ATTR_SCTP]);
-               if (orig_attrs & (1 << OVS_KEY_ATTR_IPV4)) {
-                       SW_FLOW_KEY_PUT(match, ipv4.tp.src,
-                                       sctp_key->sctp_src, is_mask);
-                       SW_FLOW_KEY_PUT(match, ipv4.tp.dst,
-                                       sctp_key->sctp_dst, is_mask);
-               } else {
-                       SW_FLOW_KEY_PUT(match, ipv6.tp.src,
-                                       sctp_key->sctp_src, is_mask);
-                       SW_FLOW_KEY_PUT(match, ipv6.tp.dst,
-                                       sctp_key->sctp_dst, is_mask);
-               }
+               SW_FLOW_KEY_PUT(match, tp.src, sctp_key->sctp_src, is_mask);
+               SW_FLOW_KEY_PUT(match, tp.dst, sctp_key->sctp_dst, is_mask);
                attrs &= ~(1 << OVS_KEY_ATTR_SCTP);
        }
 
@@ -697,9 +670,9 @@ static int ovs_key_from_nlattrs(struct sw_flow_match *match, u64 attrs,
                const struct ovs_key_icmp *icmp_key;
 
                icmp_key = nla_data(a[OVS_KEY_ATTR_ICMP]);
-               SW_FLOW_KEY_PUT(match, ipv4.tp.src,
+               SW_FLOW_KEY_PUT(match, tp.src,
                                htons(icmp_key->icmp_type), is_mask);
-               SW_FLOW_KEY_PUT(match, ipv4.tp.dst,
+               SW_FLOW_KEY_PUT(match, tp.dst,
                                htons(icmp_key->icmp_code), is_mask);
                attrs &= ~(1 << OVS_KEY_ATTR_ICMP);
        }
@@ -708,9 +681,9 @@ static int ovs_key_from_nlattrs(struct sw_flow_match *match, u64 attrs,
                const struct ovs_key_icmpv6 *icmpv6_key;
 
                icmpv6_key = nla_data(a[OVS_KEY_ATTR_ICMPV6]);
-               SW_FLOW_KEY_PUT(match, ipv6.tp.src,
+               SW_FLOW_KEY_PUT(match, tp.src,
                                htons(icmpv6_key->icmpv6_type), is_mask);
-               SW_FLOW_KEY_PUT(match, ipv6.tp.dst,
+               SW_FLOW_KEY_PUT(match, tp.dst,
                                htons(icmpv6_key->icmpv6_code), is_mask);
                attrs &= ~(1 << OVS_KEY_ATTR_ICMPV6);
        }
@@ -1024,19 +997,11 @@ int ovs_nla_put_flow(const struct sw_flow_key *swkey,
                        if (!nla)
                                goto nla_put_failure;
                        tcp_key = nla_data(nla);
-                       if (swkey->eth.type == htons(ETH_P_IP)) {
-                               tcp_key->tcp_src = output->ipv4.tp.src;
-                               tcp_key->tcp_dst = output->ipv4.tp.dst;
-                               if (nla_put_be16(skb, OVS_KEY_ATTR_TCP_FLAGS,
-                                                output->ipv4.tp.flags))
-                                       goto nla_put_failure;
-                       } else if (swkey->eth.type == htons(ETH_P_IPV6)) {
-                               tcp_key->tcp_src = output->ipv6.tp.src;
-                               tcp_key->tcp_dst = output->ipv6.tp.dst;
-                               if (nla_put_be16(skb, OVS_KEY_ATTR_TCP_FLAGS,
-                                                output->ipv6.tp.flags))
-                                       goto nla_put_failure;
-                       }
+                       tcp_key->tcp_src = output->tp.src;
+                       tcp_key->tcp_dst = output->tp.dst;
+                       if (nla_put_be16(skb, OVS_KEY_ATTR_TCP_FLAGS,
+                                        output->tp.flags))
+                               goto nla_put_failure;
                } else if (swkey->ip.proto == IPPROTO_UDP) {
                        struct ovs_key_udp *udp_key;
 
@@ -1044,13 +1009,8 @@ int ovs_nla_put_flow(const struct sw_flow_key *swkey,
                        if (!nla)
                                goto nla_put_failure;
                        udp_key = nla_data(nla);
-                       if (swkey->eth.type == htons(ETH_P_IP)) {
-                               udp_key->udp_src = output->ipv4.tp.src;
-                               udp_key->udp_dst = output->ipv4.tp.dst;
-                       } else if (swkey->eth.type == htons(ETH_P_IPV6)) {
-                               udp_key->udp_src = output->ipv6.tp.src;
-                               udp_key->udp_dst = output->ipv6.tp.dst;
-                       }
+                       udp_key->udp_src = output->tp.src;
+                       udp_key->udp_dst = output->tp.dst;
                } else if (swkey->ip.proto == IPPROTO_SCTP) {
                        struct ovs_key_sctp *sctp_key;
 
@@ -1058,13 +1018,8 @@ int ovs_nla_put_flow(const struct sw_flow_key *swkey,
                        if (!nla)
                                goto nla_put_failure;
                        sctp_key = nla_data(nla);
-                       if (swkey->eth.type == htons(ETH_P_IP)) {
-                               sctp_key->sctp_src = output->ipv4.tp.src;
-                               sctp_key->sctp_dst = output->ipv4.tp.dst;
-                       } else if (swkey->eth.type == htons(ETH_P_IPV6)) {
-                               sctp_key->sctp_src = output->ipv6.tp.src;
-                               sctp_key->sctp_dst = output->ipv6.tp.dst;
-                       }
+                       sctp_key->sctp_src = output->tp.src;
+                       sctp_key->sctp_dst = output->tp.dst;
                } else if (swkey->eth.type == htons(ETH_P_IP) &&
                           swkey->ip.proto == IPPROTO_ICMP) {
                        struct ovs_key_icmp *icmp_key;
@@ -1073,8 +1028,8 @@ int ovs_nla_put_flow(const struct sw_flow_key *swkey,
                        if (!nla)
                                goto nla_put_failure;
                        icmp_key = nla_data(nla);
-                       icmp_key->icmp_type = ntohs(output->ipv4.tp.src);
-                       icmp_key->icmp_code = ntohs(output->ipv4.tp.dst);
+                       icmp_key->icmp_type = ntohs(output->tp.src);
+                       icmp_key->icmp_code = ntohs(output->tp.dst);
                } else if (swkey->eth.type == htons(ETH_P_IPV6) &&
                           swkey->ip.proto == IPPROTO_ICMPV6) {
                        struct ovs_key_icmpv6 *icmpv6_key;
@@ -1084,8 +1039,8 @@ int ovs_nla_put_flow(const struct sw_flow_key *swkey,
                        if (!nla)
                                goto nla_put_failure;
                        icmpv6_key = nla_data(nla);
-                       icmpv6_key->icmpv6_type = ntohs(output->ipv6.tp.src);
-                       icmpv6_key->icmpv6_code = ntohs(output->ipv6.tp.dst);
+                       icmpv6_key->icmpv6_type = ntohs(output->tp.src);
+                       icmpv6_key->icmpv6_code = ntohs(output->tp.dst);
 
                        if (icmpv6_key->icmpv6_type == NDISC_NEIGHBOUR_SOLICITATION ||
                            icmpv6_key->icmpv6_type == NDISC_NEIGHBOUR_ADVERTISEMENT) {
@@ -1263,13 +1218,10 @@ static int validate_and_copy_sample(const struct nlattr *attr,
 
 static int validate_tp_port(const struct sw_flow_key *flow_key)
 {
-       if (flow_key->eth.type == htons(ETH_P_IP)) {
-               if (flow_key->ipv4.tp.src || flow_key->ipv4.tp.dst)
-                       return 0;
-       } else if (flow_key->eth.type == htons(ETH_P_IPV6)) {
-               if (flow_key->ipv6.tp.src || flow_key->ipv6.tp.dst)
-                       return 0;
-       }
+       if ((flow_key->eth.type == htons(ETH_P_IP) ||
+            flow_key->eth.type == htons(ETH_P_IPV6)) &&
+           (flow_key->tp.src || flow_key->tp.dst))
+               return 0;
 
        return -EINVAL;
 }