On Tue, Sep 05, 2017 at 05:22:55PM +0800, Yuanhan Liu wrote:
> So that we could skip the heavy emc processing, notably, the
> miniflow_extract function. A simple PHY-PHY forwarding testing
> shows 53% performance improvement.
> 
> Note that though the heavy miniflow_extract is skipped, we
> still have to do per packet checking, due to we have to check
> the tcp_flags.
> 
> Co-authored-by: Finn Christensen <f...@napatech.com>
> Signed-off-by: Yuanhan Liu <y...@fridaylinux.org>
> Signed-off-by: Finn Christensen <f...@napatech.com>
> ---
> 
> v2: update tcp_flags, which also fixes the build warnings
> ---
>  lib/dp-packet.h   | 13 ++++++++++
>  lib/dpif-netdev.c | 27 ++++++++++++++-----
>  lib/flow.c        | 78 
> +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  lib/flow.h        |  1 +
>  4 files changed, 113 insertions(+), 6 deletions(-)
> 
> diff --git a/lib/dp-packet.h b/lib/dp-packet.h
> index 046f3ab..a7a062f 100644
> --- a/lib/dp-packet.h
> +++ b/lib/dp-packet.h
> @@ -691,6 +691,19 @@ reset_dp_packet_checksum_ol_flags(struct dp_packet *p)
>  #define reset_dp_packet_checksum_ol_flags(arg)
>  #endif
>  
> +static inline bool
> +dp_packet_has_flow_mark(struct dp_packet *p OVS_UNUSED,
> +                        uint32_t *mark OVS_UNUSED)
> +{
> +#ifdef DPDK_NETDEV
> +    if (p->mbuf.ol_flags & PKT_RX_FDIR_ID) {
> +        *mark = p->mbuf.hash.fdir.hi;
> +        return true;
> +    }
> +#endif
> +    return false;
> +}

It seems odd that p and mark are marked as OVS_UNUSED
but used in the case that DPDK_NETDEV is defined.

Something like this seems cleaner to me.

+#ifdef DPDK_NETDEV
+static inline bool
+dp_packet_has_flow_mark(struct dp_packet *p, uint32_t *mark)
+{
+    if (p->mbuf.ol_flags & PKT_RX_FDIR_ID) {
+        *mark = p->mbuf.hash.fdir.hi;
+        return true;
+    }
+    return false;
+}
+#else
+static inline bool
+dp_packet_has_flow_mark(struct dp_packet *p OVS_UNUSED,
+                        uint32_t *mark OVS_UNUSED)
+{
+    return false;
+}
+#endif

...

> diff --git a/lib/flow.c b/lib/flow.c
> index b2b10aa..912c538 100644
> --- a/lib/flow.c
> +++ b/lib/flow.c
> @@ -991,6 +991,84 @@ parse_dl_type(const struct eth_header *data_, size_t 
> size)
>      return parse_ethertype(&data, &size);
>  }
>  
> +uint16_t
> +parse_tcp_flags(struct dp_packet *packet)
> +{
> +    const void *data = dp_packet_data(packet);
> +    size_t size = dp_packet_size(packet);
> +    ovs_be16 dl_type;
> +    uint8_t nw_frag = 0, nw_proto = 0;
> +
> +    if (packet->packet_type != htonl(PT_ETH)) {
> +        return 0;
> +    }
> +
> +    data_pull(&data, &size, ETH_ADDR_LEN * 2);
> +    dl_type = parse_ethertype(&data, &size);
> +    if (OVS_LIKELY(dl_type == htons(ETH_TYPE_IP))) {
> +        const struct ip_header *nh = data;
> +        int ip_len;
> +
> +        if (OVS_UNLIKELY(size < IP_HEADER_LEN)) {
> +            return 0;
> +        }
> +        ip_len = IP_IHL(nh->ip_ihl_ver) * 4;
> +
> +        if (OVS_UNLIKELY(ip_len < IP_HEADER_LEN)) {
> +            return 0;
> +        }
> +        if (OVS_UNLIKELY(size < ip_len)) {
> +            return 0;
> +        }
> +
> +        if (OVS_UNLIKELY(IP_IS_FRAGMENT(nh->ip_frag_off))) {
> +            nw_frag = FLOW_NW_FRAG_ANY;
> +            if (nh->ip_frag_off & htons(IP_FRAG_OFF_MASK)) {
> +                nw_frag |= FLOW_NW_FRAG_LATER;
> +            }
> +        }
> +        nw_proto = nh->ip_proto;
> +        data_pull(&data, &size, ip_len);
> +    } else if (dl_type == htons(ETH_TYPE_IPV6)) {
> +        const struct ovs_16aligned_ip6_hdr *nh;
> +        uint16_t plen;
> +
> +        if (OVS_UNLIKELY(size < sizeof *nh)) {
> +            return 0;
> +        }
> +        nh = data_pull(&data, &size, sizeof *nh);
> +
> +        plen = ntohs(nh->ip6_plen);
> +        if (OVS_UNLIKELY(plen > size)) {
> +            return 0;
> +        }
> +        /* Jumbo Payload option not supported yet. */
> +        if (OVS_UNLIKELY(size - plen > UINT8_MAX)) {
> +            return 0;
> +        }
> +        size = plen;
> +
> +        nw_proto = nh->ip6_nxt;
> +        if (!parse_ipv6_ext_hdrs__(&data, &size, &nw_proto, &nw_frag)) {
> +            return 0;
> +        }
> +    } else {
> +        return 0;
> +    }
> +
> +    if (OVS_LIKELY(!(nw_frag & FLOW_NW_FRAG_LATER))) {
> +        if (OVS_LIKELY(nw_proto == IPPROTO_TCP)) {
> +            if (OVS_LIKELY(size >= TCP_HEADER_LEN)) {

The following might be nicer:

+    if (OVS_LIKELY(!(nw_frag & FLOW_NW_FRAG_LATER))
+        && OVS_LIKELY(nw_proto == IPPROTO_TCP)
+        && OVS_LIKELY(size >= TCP_HEADER_LEN)) {

> +                const struct tcp_header *tcp = data;
> +
> +                return TCP_FLAGS_BE32(tcp->tcp_ctl);
> +            }
> +        }
> +    }
> +
> +    return 0;
> +}
> +
>  /* For every bit of a field that is wildcarded in 'wildcards', sets the
>   * corresponding bit in 'flow' to zero. */
>  void

...
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to