Hi Jarno, Thanks for the suggestion, I will try to get a performance number and submit it into Linux first. :)
Thanks Zhenyu Gao 2017-04-22 4:24 GMT+08:00 Jarno Rajahalme <[email protected]>: > As a policy, Linux kernel datapath changes other than backports need to go > to upstream Linux first, new features to net-next tree, and bug fixes to > net tree. See the documentation file ‘backporting-patches.rst’ in directory > 'Documentation/internals/contributing/‘ of the OVS tree for more detailed > description of the process. > > Also, the commit message should contain a clear motivation for the change. > Changes that enhance readability may adversely affect datapath performance, > so having a report on performance testing would be helpful in determining > whether to apply the change. > > Regards, > > Jarno > > > On Apr 21, 2017, at 12:21 AM, Zhenyu Gao <[email protected]> > wrote: > > > > 1. Consume switch/case to judge type of frame instead of using if/else. > > 2. Add parse_ipv4hdr for ipv4 frame header parsing. > > > > Signed-off-by: Zhenyu Gao <[email protected]> > > --- > > datapath/flow.c | 230 ++++++++++++++++++++++++++++-- > -------------------------- > > 1 file changed, 117 insertions(+), 113 deletions(-) > > > > diff --git a/datapath/flow.c b/datapath/flow.c > > index 2bc1ad0..0b35de6 100644 > > --- a/datapath/flow.c > > +++ b/datapath/flow.c > > @@ -250,6 +250,46 @@ static bool icmphdr_ok(struct sk_buff *skb) > > sizeof(struct icmphdr)); > > } > > > > +/** > > + * Parse ipv4 header from an Ethernet frame. > > + * Return ipv4 header length if successful, otherwise a negative errno > value. > > + */ > > +static int parse_ipv4hdr(struct sk_buff *skb, struct sw_flow_key *key) > > +{ > > + int err; > > + struct iphdr *nh; > > + __be16 offset; > > + > > + err = check_iphdr(skb); > > + if (unlikely(err)) > > + return err; > > + > > + nh = ip_hdr(skb); > > + key->ipv4.addr.src = nh->saddr; > > + key->ipv4.addr.dst = nh->daddr; > > + > > + key->ip.proto = nh->protocol; > > + key->ip.tos = nh->tos; > > + key->ip.ttl = nh->ttl; > > + > > + offset = nh->frag_off & htons(IP_OFFSET); > > + if (offset) { > > + key->ip.frag = OVS_FRAG_TYPE_LATER; > > + } else { > > + if (nh->frag_off & htons(IP_MF) || > > + skb_shinfo(skb)->gso_type & SKB_GSO_UDP) { > > + key->ip.frag = OVS_FRAG_TYPE_FIRST; > > + } else { > > + key->ip.frag = OVS_FRAG_TYPE_NONE; > > + } > > + } > > + return ip_hdrlen(skb); > > +} > > + > > +/** > > + * Parse ipv6 header from an Ethernet frame. > > + * Return ipv6 header length if successful, otherwise a negative errno > value. > > + */ > > static int parse_ipv6hdr(struct sk_buff *skb, struct sw_flow_key *key) > > { > > unsigned int nh_ofs = skb_network_offset(skb); > > @@ -283,7 +323,10 @@ static int parse_ipv6hdr(struct sk_buff *skb, > struct sw_flow_key *key) > > else > > key->ip.frag = OVS_FRAG_TYPE_FIRST; > > } else { > > - key->ip.frag = OVS_FRAG_TYPE_NONE; > > + if (skb_shinfo(skb)->gso_type & SKB_GSO_UDP) > > + key->ip.frag = OVS_FRAG_TYPE_FIRST; > > + else > > + key->ip.frag = OVS_FRAG_TYPE_NONE; > > } > > > > /* Delayed handling of error in ipv6_skip_exthdr() as it > > @@ -561,42 +604,43 @@ static int key_extract(struct sk_buff *skb, struct > sw_flow_key *key) > > key->eth.type = skb->protocol; > > > > /* Network layer. */ > > - if (key->eth.type == htons(ETH_P_IP)) { > > - struct iphdr *nh; > > - __be16 offset; > > + switch(key->eth.type) { > > + case htons(ETH_P_IP): > > + case htons(ETH_P_IPV6): { > > + int nh_len; > > + if (key->eth.type == htons(ETH_P_IP)) { > > + nh_len = parse_ipv4hdr(skb, key); > > + } else { > > + nh_len = parse_ipv6hdr(skb, key); > > + } > > > > - error = check_iphdr(skb); > > - if (unlikely(error)) { > > - memset(&key->ip, 0, sizeof(key->ip)); > > - memset(&key->ipv4, 0, sizeof(key->ipv4)); > > - if (error == -EINVAL) { > > + if (unlikely(nh_len < 0)) { > > + switch (nh_len) { > > + case -EINVAL: > > + memset(&key->ip, 0, sizeof(key->ip)); > > + if (key->eth.type == htons(ETH_P_IP)) { > > + memset(&key->ipv4.addr, 0, > sizeof(key->ipv4.addr)); > > + } else { > > + memset(&key->ipv6.addr, 0, > sizeof(key->ipv6.addr)); > > + } > > + /* fall-through */ > > + case -EPROTO: > > skb->transport_header = > skb->network_header; > > error = 0; > > + break; > > + default: > > + error = nh_len; > > } > > return error; > > } > > > > - nh = ip_hdr(skb); > > - key->ipv4.addr.src = nh->saddr; > > - key->ipv4.addr.dst = nh->daddr; > > - > > - key->ip.proto = nh->protocol; > > - key->ip.tos = nh->tos; > > - key->ip.ttl = nh->ttl; > > - > > - offset = nh->frag_off & htons(IP_OFFSET); > > - if (offset) { > > - key->ip.frag = OVS_FRAG_TYPE_LATER; > > + if (key->ip.frag == OVS_FRAG_TYPE_LATER) { > > return 0; > > } > > - if (nh->frag_off & htons(IP_MF) || > > - skb_shinfo(skb)->gso_type & SKB_GSO_UDP) > > - key->ip.frag = OVS_FRAG_TYPE_FIRST; > > - else > > - key->ip.frag = OVS_FRAG_TYPE_NONE; > > > > /* Transport layer. */ > > - if (key->ip.proto == IPPROTO_TCP) { > > + switch(key->ip.proto) { > > + case IPPROTO_TCP: > > if (tcphdr_ok(skb)) { > > struct tcphdr *tcp = tcp_hdr(skb); > > key->tp.src = tcp->source; > > @@ -605,8 +649,8 @@ static int key_extract(struct sk_buff *skb, struct > sw_flow_key *key) > > } else { > > memset(&key->tp, 0, sizeof(key->tp)); > > } > > - > > - } else if (key->ip.proto == IPPROTO_UDP) { > > + break; > > + case IPPROTO_UDP: > > if (udphdr_ok(skb)) { > > struct udphdr *udp = udp_hdr(skb); > > key->tp.src = udp->source; > > @@ -614,7 +658,8 @@ static int key_extract(struct sk_buff *skb, struct > sw_flow_key *key) > > } else { > > memset(&key->tp, 0, sizeof(key->tp)); > > } > > - } else if (key->ip.proto == IPPROTO_SCTP) { > > + break; > > + case IPPROTO_SCTP: > > if (sctphdr_ok(skb)) { > > struct sctphdr *sctp = sctp_hdr(skb); > > key->tp.src = sctp->source; > > @@ -622,7 +667,8 @@ static int key_extract(struct sk_buff *skb, struct > sw_flow_key *key) > > } else { > > memset(&key->tp, 0, sizeof(key->tp)); > > } > > - } else if (key->ip.proto == IPPROTO_ICMP) { > > + break; > > + case IPPROTO_ICMP: > > if (icmphdr_ok(skb)) { > > struct icmphdr *icmp = icmp_hdr(skb); > > /* The ICMP type and code fields use the > 16-bit > > @@ -634,10 +680,23 @@ static int key_extract(struct sk_buff *skb, struct > sw_flow_key *key) > > } else { > > memset(&key->tp, 0, sizeof(key->tp)); > > } > > + break; > > + case NEXTHDR_ICMP: > > + if (icmp6hdr_ok(skb)) { > > + error = parse_icmpv6(skb, key, nh_len); > > + if (error) > > + return error; > > + } else { > > + memset(&key->tp, 0, sizeof(key->tp)); > > + } > > + break; > > + default: > > + break; > > } > > - > > - } else if (key->eth.type == htons(ETH_P_ARP) || > > - key->eth.type == htons(ETH_P_RARP)) { > > + break; > > + } > > + case htons(ETH_P_ARP): > > + case htons(ETH_P_RARP): { > > struct arp_eth_header *arp; > > bool arp_available = arphdr_ok(skb); > > > > @@ -663,94 +722,39 @@ static int key_extract(struct sk_buff *skb, struct > sw_flow_key *key) > > memset(&key->ip, 0, sizeof(key->ip)); > > memset(&key->ipv4, 0, sizeof(key->ipv4)); > > } > > - } else if (eth_p_mpls(key->eth.type)) { > > - size_t stack_len = MPLS_HLEN; > > - > > - /* In the presence of an MPLS label stack the end of the L2 > > - * header and the beginning of the L3 header differ. > > - * > > - * Advance network_header to the beginning of the L3 > > - * header. mac_len corresponds to the end of the L2 header. > > - */ > > - while (1) { > > - __be32 lse; > > + break; > > + } > > + default: > > + if (eth_p_mpls(key->eth.type)) { > > + size_t stack_len = MPLS_HLEN; > > + > > + /* In the presence of an MPLS label stack the end > of the L2 > > + * header and the beginning of the L3 header > differ. > > + * > > + * Advance network_header to the beginning of the > L3 > > + * header. mac_len corresponds to the end of the > L2 header. > > + */ > > + while (1) { > > + __be32 lse; > > > > - error = check_header(skb, skb->mac_len + > stack_len); > > - if (unlikely(error)) > > - return 0; > > + error = check_header(skb, skb->mac_len + > stack_len); > > + if (unlikely(error)) > > + return 0; > > > > - memcpy(&lse, skb_network_header(skb), MPLS_HLEN); > > + memcpy(&lse, skb_network_header(skb), > MPLS_HLEN); > > > > - if (stack_len == MPLS_HLEN) > > - memcpy(&key->mpls.top_lse, &lse, > MPLS_HLEN); > > + if (stack_len == MPLS_HLEN) > > + memcpy(&key->mpls.top_lse, &lse, > MPLS_HLEN); > > > > - skb_set_network_header(skb, skb->mac_len + > stack_len); > > - if (lse & htonl(MPLS_LS_S_MASK)) > > - break; > > + skb_set_network_header(skb, skb->mac_len + > stack_len); > > + if (lse & htonl(MPLS_LS_S_MASK)) > > + break; > > > > - stack_len += MPLS_HLEN; > > - } > > - } else if (key->eth.type == htons(ETH_P_IPV6)) { > > - int nh_len; /* IPv6 Header + Extensions */ > > - > > - nh_len = parse_ipv6hdr(skb, key); > > - if (unlikely(nh_len < 0)) { > > - switch (nh_len) { > > - case -EINVAL: > > - memset(&key->ip, 0, sizeof(key->ip)); > > - memset(&key->ipv6.addr, 0, > sizeof(key->ipv6.addr)); > > - /* fall-through */ > > - case -EPROTO: > > - skb->transport_header = > skb->network_header; > > - error = 0; > > - break; > > - default: > > - error = nh_len; > > - } > > - return error; > > - } > > - > > - if (key->ip.frag == OVS_FRAG_TYPE_LATER) > > - return 0; > > - if (skb_shinfo(skb)->gso_type & SKB_GSO_UDP) > > - key->ip.frag = OVS_FRAG_TYPE_FIRST; > > - > > - /* Transport layer. */ > > - if (key->ip.proto == NEXTHDR_TCP) { > > - if (tcphdr_ok(skb)) { > > - struct tcphdr *tcp = tcp_hdr(skb); > > - key->tp.src = tcp->source; > > - key->tp.dst = tcp->dest; > > - key->tp.flags = TCP_FLAGS_BE16(tcp); > > - } else { > > - memset(&key->tp, 0, sizeof(key->tp)); > > - } > > - } else if (key->ip.proto == NEXTHDR_UDP) { > > - if (udphdr_ok(skb)) { > > - struct udphdr *udp = udp_hdr(skb); > > - key->tp.src = udp->source; > > - key->tp.dst = udp->dest; > > - } else { > > - memset(&key->tp, 0, sizeof(key->tp)); > > - } > > - } else if (key->ip.proto == NEXTHDR_SCTP) { > > - if (sctphdr_ok(skb)) { > > - struct sctphdr *sctp = sctp_hdr(skb); > > - key->tp.src = sctp->source; > > - key->tp.dst = sctp->dest; > > - } else { > > - memset(&key->tp, 0, sizeof(key->tp)); > > - } > > - } else if (key->ip.proto == NEXTHDR_ICMP) { > > - if (icmp6hdr_ok(skb)) { > > - error = parse_icmpv6(skb, key, nh_len); > > - if (error) > > - return error; > > - } else { > > - memset(&key->tp, 0, sizeof(key->tp)); > > + stack_len += MPLS_HLEN; > > } > > } > > } > > + > > return 0; > > } > > > > -- > > 1.9.1 > > > > _______________________________________________ > > dev mailing list > > [email protected] > > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
