From patchwork Wed Sep 11 14:10:02 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Michal Obrembski X-Patchwork-Id: 1161020 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (mailfrom) smtp.mailfrom=openvswitch.org (client-ip=140.211.169.12; helo=mail.linuxfoundation.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=) Authentication-Results: ozlabs.org; dmarc=fail (p=none dis=none) header.from=intel.com Received: from mail.linuxfoundation.org (mail.linuxfoundation.org [140.211.169.12]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 46T3nW67GPz9s4Y for ; Thu, 12 Sep 2019 00:14:55 +1000 (AEST) Received: from mail.linux-foundation.org (localhost [127.0.0.1]) by mail.linuxfoundation.org (Postfix) with ESMTP id C438C14AC; Wed, 11 Sep 2019 14:12:02 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@mail.linuxfoundation.org Received: from smtp1.linuxfoundation.org (smtp1.linux-foundation.org [172.17.192.35]) by mail.linuxfoundation.org (Postfix) with ESMTPS id 154C01488 for ; Wed, 11 Sep 2019 14:12:00 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.7.6 Received: from mga07.intel.com (mga07.intel.com [134.134.136.100]) by smtp1.linuxfoundation.org (Postfix) with ESMTPS id 166FB894 for ; Wed, 11 Sep 2019 14:11:59 +0000 (UTC) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by orsmga105.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 11 Sep 2019 07:11:58 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.64,493,1559545200"; d="scan'208";a="200528255" Received: from mobrembx-mobl.ger.corp.intel.com ([10.103.104.26]) by fmsmga001.fm.intel.com with ESMTP; 11 Sep 2019 07:11:57 -0700 From: Michal Obrembski To: dev@openvswitch.org Date: Wed, 11 Sep 2019 16:10:02 +0200 Message-Id: <20190911141005.1346-5-michalx.obrembski@intel.com> X-Mailer: git-send-email 2.23.0.windows.1 In-Reply-To: <20190911141005.1346-1-michalx.obrembski@intel.com> References: <20190911141005.1346-1-michalx.obrembski@intel.com> MIME-Version: 1.0 X-Spam-Status: No, score=-4.2 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_MED autolearn=ham version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on smtp1.linux-foundation.org Subject: [ovs-dev] [PATCH v5 4/7] Performance improvements and native tnl fixes. X-BeenThere: ovs-dev@openvswitch.org X-Mailman-Version: 2.1.12 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: ovs-dev-bounces@openvswitch.org Errors-To: ovs-dev-bounces@openvswitch.org From: Tiago Lam Make miniflow_extract() perform by checking if the header fits in the first mbuf only in a single place, since the multiple checks would bring some performance penalties. Furthermore, in some functions, such as dp_packet_set_size(), to distinguish between multi-segment mbufs and a single segment mbuf only the "b->source == DPBUF_DPDK" was being used. This could lead to the case where a single-segment mbuf would enter a loop where only a single segment needs processing, thus leading to some extra logic and some performance penalty. Finally, fix some tunnel cases to check if the tunneling headers (VXLAN, GRE, ...) fit on the first mbuf. Otherwise return an error. Signed-off-by: Tiago Lam Signed-off-by: Michal Obrembski --- lib/dp-packet.h | 18 +++++------ lib/flow.c | 81 ++++++++++++++++--------------------------------- lib/netdev-dpdk.c | 13 +++----- lib/netdev-native-tnl.c | 10 +++--- 4 files changed, 45 insertions(+), 77 deletions(-) diff --git a/lib/dp-packet.h b/lib/dp-packet.h index 96136ed..68da4e9 100644 --- a/lib/dp-packet.h +++ b/lib/dp-packet.h @@ -258,7 +258,7 @@ dp_packet_copy_common_members(struct dp_packet *new_b, static inline void * dp_packet_at(const struct dp_packet *b, size_t offset, size_t size) { - if (offset + size > dp_packet_size(b)) { + if (OVS_UNLIKELY(offset + size > dp_packet_size(b))) { return NULL; } @@ -266,7 +266,7 @@ dp_packet_at(const struct dp_packet *b, size_t offset, size_t size) if (b->source == DPBUF_DPDK) { const struct rte_mbuf *mbuf = dp_packet_mbuf_from_offset(b, &offset); - if (!mbuf || offset + size > mbuf->data_len) { + if (OVS_UNLIKELY(!mbuf || offset + size > mbuf->data_len)) { return NULL; } @@ -290,7 +290,7 @@ static inline void * dp_packet_tail(const struct dp_packet *b) { #ifdef DPDK_NETDEV - if (b->source == DPBUF_DPDK) { + if (OVS_UNLIKELY(b->source == DPBUF_DPDK && !dp_packet_is_linear(b))) { struct rte_mbuf *buf = CONST_CAST(struct rte_mbuf *, &b->mbuf); /* Find last segment where data ends, meaning the tail of the chained * mbufs must be there */ @@ -308,7 +308,7 @@ static inline void * dp_packet_end(const struct dp_packet *b) { #ifdef DPDK_NETDEV - if (b->source == DPBUF_DPDK) { + if (OVS_UNLIKELY(b->source == DPBUF_DPDK && !dp_packet_is_linear(b))) { struct rte_mbuf *buf = CONST_CAST(struct rte_mbuf *, &(b->mbuf)); buf = rte_pktmbuf_lastseg(buf); @@ -342,7 +342,7 @@ static inline void dp_packet_clear(struct dp_packet *b) { #ifdef DPDK_NETDEV - if (b->source == DPBUF_DPDK) { + if (OVS_UNLIKELY(b->source == DPBUF_DPDK && !dp_packet_is_linear(b))) { /* sets pkt_len and data_len to zero and frees unused mbufs */ dp_packet_set_size(b, 0); rte_pktmbuf_reset(&b->mbuf); @@ -383,7 +383,7 @@ dp_packet_may_pull(const struct dp_packet *b, uint16_t offset, size_t size) } #ifdef DPDK_NETDEV /* Offset needs to be within the first mbuf */ - if (offset + size > b->mbuf.data_len) { + if (OVS_UNLIKELY(offset + size > b->mbuf.data_len)) { return false; } #endif @@ -693,7 +693,7 @@ dp_packet_size(const struct dp_packet *b) static inline void dp_packet_set_size(struct dp_packet *b, uint32_t v) { - if (b->source == DPBUF_DPDK) { + if (OVS_UNLIKELY(b->source == DPBUF_DPDK && !dp_packet_is_linear(b))) { struct rte_mbuf *mbuf = &b->mbuf; uint16_t new_len = v; uint16_t data_len; @@ -906,8 +906,8 @@ dp_packet_copy_from_offset(const struct dp_packet *b, size_t offset, static inline bool dp_packet_is_linear(const struct dp_packet *b) { - if (b->source == DPBUF_DPDK) { - return rte_pktmbuf_is_contiguous(&b->mbuf); + if (OVS_UNLIKELY(b->mbuf.nb_segs > 1)) { + return false; } return true; diff --git a/lib/flow.c b/lib/flow.c index e6019bf..1465d28 100644 --- a/lib/flow.c +++ b/lib/flow.c @@ -711,6 +711,25 @@ ipv6_sanity_check(const struct ovs_16aligned_ip6_hdr *nh, size_t size) return true; } +#define MAX_IPV4_LEN \ + (ETH_HEADER_LEN + IP_HEADER_LEN + \ + MAX(TCP_HEADER_LEN, \ + MAX(UDP_HEADER_LEN, \ + MAX(SCTP_HEADER_LEN, MAX(ICMP_HEADER_LEN, \ + IGMP_HEADER_LEN))))) + +#define MAX_IPV6_LEN \ + (ETH_HEADER_LEN + IPV6_HEADER_LEN + \ + MAX(TCP_HEADER_LEN, \ + MAX(UDP_HEADER_LEN, \ + MAX(SCTP_HEADER_LEN, \ + MAX(sizeof(struct tcp_header), IGMP_HEADER_LEN))))) + +#define MAX_ARP_NSH_LEN (ETH_HEADER_LEN + MAX(ARP_ETH_HEADER_LEN, \ + NSH_BASE_HDR_LEN)) + +#define MAX_HEADER_LEN MAX(MAX_IPV4_LEN, MAX(MAX_IPV6_LEN, MAX_ARP_NSH_LEN)) + /* Initializes 'dst' from 'packet' and 'md', taking the packet type into * account. 'dst' must have enough space for FLOW_U64S * 8 bytes. * @@ -759,6 +778,13 @@ miniflow_extract(struct dp_packet *packet, struct miniflow *dst) uint8_t *ct_nw_proto_p = NULL; ovs_be16 ct_tp_src = 0, ct_tp_dst = 0; + /* Check if header is in first mbuf, otherwise return error */ + if (OVS_UNLIKELY(!dp_packet_is_linear(packet))) { + if (!dp_packet_may_pull(packet, 0, MAX_HEADER_LEN)) { + return -EINVAL; + } + } + /* Metadata. */ if (flow_tnl_dst_is_set(&md->tunnel)) { miniflow_push_words(mf, tunnel, &md->tunnel, @@ -862,13 +888,6 @@ miniflow_extract(struct dp_packet *packet, struct miniflow *dst) int ip_len; uint16_t tot_len; - /* Check if header is in first mbuf, otherwise return error */ - if (!dp_packet_is_linear(packet)) { - if (!dp_packet_may_pull(packet, packet->l3_ofs, sizeof *nh)) { - return -EINVAL; - } - } - if (OVS_UNLIKELY(!ipv4_sanity_check(nh, size, &ip_len, &tot_len))) { goto out; } @@ -899,12 +918,6 @@ miniflow_extract(struct dp_packet *packet, struct miniflow *dst) ovs_be32 tc_flow; uint16_t plen; - if (!dp_packet_is_linear(packet)) { - if (!dp_packet_may_pull(packet, packet->l3_ofs, sizeof *nh)) { - return -EINVAL; - } - } - if (OVS_UNLIKELY(!ipv6_sanity_check(nh, size))) { goto out; } @@ -951,13 +964,6 @@ miniflow_extract(struct dp_packet *packet, struct miniflow *dst) dl_type == htons(ETH_TYPE_RARP)) { struct eth_addr arp_buf[2]; - if (!dp_packet_is_linear(packet)) { - if (!dp_packet_may_pull(packet, packet->l3_ofs, - ARP_ETH_HEADER_LEN)) { - return -EINVAL; - } - } - const struct arp_eth_header *arp = (const struct arp_eth_header *) data_try_pull(&data, &size, ARP_ETH_HEADER_LEN); @@ -1005,13 +1011,6 @@ miniflow_extract(struct dp_packet *packet, struct miniflow *dst) if (OVS_LIKELY(size >= TCP_HEADER_LEN)) { const struct tcp_header *tcp = data; - if (!dp_packet_is_linear(packet)) { - if (!dp_packet_may_pull(packet, packet->l4_ofs, - TCP_HEADER_LEN)) { - return -EINVAL; - } - } - miniflow_push_be32(mf, arp_tha.ea[2], 0); miniflow_push_be32(mf, tcp_flags, TCP_FLAGS_BE32(tcp->tcp_ctl)); @@ -1024,13 +1023,6 @@ miniflow_extract(struct dp_packet *packet, struct miniflow *dst) if (OVS_LIKELY(size >= UDP_HEADER_LEN)) { const struct udp_header *udp = data; - if (!dp_packet_is_linear(packet)) { - if (!dp_packet_may_pull(packet, packet->l4_ofs, - UDP_HEADER_LEN)) { - return -EINVAL; - } - } - miniflow_push_be16(mf, tp_src, udp->udp_src); miniflow_push_be16(mf, tp_dst, udp->udp_dst); miniflow_push_be16(mf, ct_tp_src, ct_tp_src); @@ -1040,13 +1032,6 @@ miniflow_extract(struct dp_packet *packet, struct miniflow *dst) if (OVS_LIKELY(size >= SCTP_HEADER_LEN)) { const struct sctp_header *sctp = data; - if (!dp_packet_is_linear(packet)) { - if (!dp_packet_may_pull(packet, packet->l4_ofs, - SCTP_HEADER_LEN)) { - return -EINVAL; - } - } - miniflow_push_be16(mf, tp_src, sctp->sctp_src); miniflow_push_be16(mf, tp_dst, sctp->sctp_dst); miniflow_push_be16(mf, ct_tp_src, ct_tp_src); @@ -1056,13 +1041,6 @@ miniflow_extract(struct dp_packet *packet, struct miniflow *dst) if (OVS_LIKELY(size >= ICMP_HEADER_LEN)) { const struct icmp_header *icmp = data; - if (!dp_packet_is_linear(packet)) { - if (!dp_packet_may_pull(packet, packet->l4_ofs, - ICMP_HEADER_LEN)) { - return -EINVAL; - } - } - miniflow_push_be16(mf, tp_src, htons(icmp->icmp_type)); miniflow_push_be16(mf, tp_dst, htons(icmp->icmp_code)); miniflow_push_be16(mf, ct_tp_src, ct_tp_src); @@ -1072,13 +1050,6 @@ miniflow_extract(struct dp_packet *packet, struct miniflow *dst) if (OVS_LIKELY(size >= IGMP_HEADER_LEN)) { const struct igmp_header *igmp = data; - if (!dp_packet_is_linear(packet)) { - if (!dp_packet_may_pull(packet, packet->l4_ofs, - IGMP_HEADER_LEN)) { - return -EINVAL; - } - } - miniflow_push_be16(mf, tp_src, htons(igmp->igmp_type)); miniflow_push_be16(mf, tp_dst, htons(igmp->igmp_code)); miniflow_push_be16(mf, ct_tp_src, ct_tp_src); diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index 7552caa..159cd7a 100644 --- a/lib/netdev-dpdk.c +++ b/lib/netdev-dpdk.c @@ -559,17 +559,17 @@ void free_dpdk_buf(struct dp_packet *p) { /* If non-pmd we need to lock on nonpmd_mp_mutex mutex */ - if (!dpdk_thread_is_pmd()) { + if (dpdk_thread_is_pmd()) { + rte_pktmbuf_free(&p->mbuf); + + return; + } else { ovs_mutex_lock(&nonpmd_mp_mutex); rte_pktmbuf_free(&p->mbuf); ovs_mutex_unlock(&nonpmd_mp_mutex); - - return; } - - rte_pktmbuf_free(&p->mbuf); } static void @@ -2159,9 +2159,6 @@ netdev_dpdk_prep_tso_packet(struct rte_mbuf *mbuf, int mtu) mbuf->outer_l2_len = 0; mbuf->outer_l3_len = 0; - /* Reset packet RX RSS flag to reuse in egress. */ - dp_packet_mbuf_rss_flag_reset(pkt); - if (!(mbuf->ol_flags & PKT_TX_TCP_SEG)) { return; } diff --git a/lib/netdev-native-tnl.c b/lib/netdev-native-tnl.c index 285b927..145ae46 100644 --- a/lib/netdev-native-tnl.c +++ b/lib/netdev-native-tnl.c @@ -363,7 +363,7 @@ parse_gre_header(struct dp_packet *packet, } hlen = ulen + gre_header_len(greh->flags); - if (hlen > dp_packet_size(packet)) { + if (!dp_packet_may_pull(packet, packet->l3_ofs, hlen)) { return -EINVAL; } @@ -534,7 +534,7 @@ netdev_erspan_pop_header(struct dp_packet *packet) IPV6_HEADER_LEN : IP_HEADER_LEN; pkt_metadata_init_tnl(md); - if (hlen > dp_packet_size(packet)) { + if (!dp_packet_may_pull(packet, packet->l3_ofs, hlen)) { goto err; } @@ -719,7 +719,7 @@ netdev_vxlan_pop_header(struct dp_packet *packet) ovs_assert(packet->l4_ofs > 0); pkt_metadata_init_tnl(md); - if (VXLAN_HLEN > dp_packet_l4_size(packet)) { + if (!dp_packet_may_pull(packet, packet->l4_ofs, VXLAN_HLEN)) { goto err; } @@ -841,7 +841,7 @@ netdev_geneve_pop_header(struct dp_packet *packet) unsigned int hlen, opts_len, ulen; pkt_metadata_init_tnl(md); - if (GENEVE_BASE_HLEN > dp_packet_l4_size(packet)) { + if (!dp_packet_may_pull(packet, packet->l4_ofs, GENEVE_BASE_HLEN)) { VLOG_WARN_RL(&err_rl, "geneve packet too small: min header=%u packet size=%"PRIuSIZE"\n", (unsigned int)GENEVE_BASE_HLEN, dp_packet_l4_size(packet)); goto err; @@ -854,7 +854,7 @@ netdev_geneve_pop_header(struct dp_packet *packet) opts_len = gnh->opt_len * 4; hlen = ulen + GENEVE_BASE_HLEN + opts_len; - if (hlen > dp_packet_size(packet)) { + if (!dp_packet_may_pull(packet, packet->l4_ofs, hlen)) { VLOG_WARN_RL(&err_rl, "geneve packet too small: header len=%u packet size=%u\n", hlen, dp_packet_size(packet)); goto err;