[ovs-dev] [PATCH v2 2/8] dpif-netdev: retrieve flow directly from the flow mark
Darrell Ball
dball at vmware.com
Fri Sep 8 17:38:07 UTC 2017
On 9/8/17, 9:44 AM, "ovs-dev-bounces at openvswitch.org on behalf of Simon Horman" <ovs-dev-bounces at openvswitch.org on behalf of simon.horman at netronome.com> wrote:
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 <fc at napatech.com>
> Signed-off-by: Yuanhan Liu <yliu at fridaylinux.org>
> Signed-off-by: Finn Christensen <fc at 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.
[Darrell] OVS_UNUSED means ‘’may be unused”
Typically, for a trivial alternative, we would do a slight variation of the above
rather than writing two versions of the functions.
+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;
+ }
+#else
+ return false;
+#endif
+}
//////////////////////////////
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
dev at openvswitch.org
https://urldefense.proofpoint.com/v2/url?u=https-3A__mail.openvswitch.org_mailman_listinfo_ovs-2Ddev&d=DwICAg&c=uilaK90D4TOVoH58JNXRgQ&r=BVhFA09CGX7JQ5Ih-uZnsw&m=gPQ6bI2kxK_3KX17GsXuiCRl8YFpi7zTpZOiIkM9bgw&s=kqeotzN7bKj_MV8xg5c-r9pc5hrfgTttYknrR2c0pQs&e=
More information about the dev
mailing list