From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 13A1442C05; Thu, 1 Jun 2023 18:23:30 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 9CD8740EF0; Thu, 1 Jun 2023 18:23:29 +0200 (CEST) Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by mails.dpdk.org (Postfix) with ESMTP id 0302540DDC for ; Thu, 1 Jun 2023 18:23:27 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1685636607; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=bB0SC+LeBuQJvTvZmEqKQ4kgIBllLee4tHh+m7FuKd4=; b=QETewgHZ4/Ft5djjQXSEgn3IT4gJLyJToWkZglKN1jh8EGi2RxEbbBhTzuz18jmYbKik0N noXalrRHoaEA8zxzNIWBsvcxQFsqypA/4Yt3WKmb/yoXQloteFJLXSowI0ObjaXpAlyQ5a WGPd4uUxTgx6yi0l0T9wCJl1hsHTsiI= Received: from mail-wr1-f71.google.com (mail-wr1-f71.google.com [209.85.221.71]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-605-1RQNO7s3NNGBuGfMk_arDA-1; Thu, 01 Jun 2023 12:23:25 -0400 X-MC-Unique: 1RQNO7s3NNGBuGfMk_arDA-1 Received: by mail-wr1-f71.google.com with SMTP id ffacd0b85a97d-30ab548ba06so1272334f8f.1 for ; Thu, 01 Jun 2023 09:23:25 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1685636604; x=1688228604; h=content-transfer-encoding:in-reply-to:subject:from:references:cc:to :content-language:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=bB0SC+LeBuQJvTvZmEqKQ4kgIBllLee4tHh+m7FuKd4=; b=L2UsUT3JaCRrbJ7SYD8MFKc7sy7bQYTeaH8UX0lZuPZGFSUolQDbLM1Cw5/VAIdXj7 OM1zCqPkJMPCgQ2R48HLKYRJzQCzL15qcI9jfKMfeuNP5g4GFCGIzeV3y9YvFB1cogHU FbQOxOwEfQBycgFZ0ZKegxf52wgjZSZ3iriV2zrhjqnrukK9jFjvKVL6b8Q0W335V15J By703vJaVLV2iw/i5uuoDtkHR3pCR0uZN3f2mwbHPcD2IaAigS7w1hCcmApZ5ByrHFwp VDSNAxr3Ayi3ZkvtTh8A+8RTTgl2eQUC985KQ5904K+mHqZpdn5I8xi0dNPoJ6TOEj7P JwdA== X-Gm-Message-State: AC+VfDwzKRa2opoyOm0gMfwkVt1JScO8mLzKEuXeHB/d8xgdcVGEHO9o GLzSeucnSNfYjUcQA34nbEuTtoZu9cEQKLnaUH2ZzlZv0fwLWF+Ks3owXtIBs0hsE8hopnxRRwj FFbY= X-Received: by 2002:adf:f292:0:b0:306:3945:65e9 with SMTP id k18-20020adff292000000b00306394565e9mr2606911wro.3.1685636604073; Thu, 01 Jun 2023 09:23:24 -0700 (PDT) X-Google-Smtp-Source: ACHHUZ4gFS1D++xo1OZIq7kA4amj3yPzDbiCXEF3+ADEodHAp8jxN7spU4RnkXB1k5z5U7USAVE16w== X-Received: by 2002:adf:f292:0:b0:306:3945:65e9 with SMTP id k18-20020adff292000000b00306394565e9mr2606888wro.3.1685636603548; Thu, 01 Jun 2023 09:23:23 -0700 (PDT) Received: from [192.168.0.36] ([78.18.21.203]) by smtp.gmail.com with ESMTPSA id c10-20020a5d63ca000000b002c70ce264bfsm10787628wrw.76.2023.06.01.09.23.22 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Thu, 01 Jun 2023 09:23:22 -0700 (PDT) Message-ID: <2c6ba959-acd8-b496-8d2c-2689878ccab3@redhat.com> Date: Thu, 1 Jun 2023 17:23:22 +0100 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.11.0 To: Mike Pattrick Cc: Aman Singh , Yuying Zhang , dev@dpdk.org, Ferruh Yigit , David Marchand References: <20230417185536.1817578-1-mkp@redhat.com> <303c7549-6aa5-18d5-fdb4-42b8df1d0dc5@redhat.com> From: Kevin Traynor Subject: Re: [PATCH v3] app/testpmd: expand noisy neighbour forward mode support In-Reply-To: X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Language: en-US Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org On 26/05/2023 18:32, Mike Pattrick wrote: > On Fri, May 26, 2023 at 11:34 AM Kevin Traynor wrote: >> >> On 17/04/2023 19:55, Mike Pattrick wrote: >>> Previously the noisy neighbour vnf simulation would only operate in io >>> mode, forwarding packets as is. However, this limited the usefulness of >>> noisy neighbour simulation. >>> >>> This feature has now been expanded to supporting mac, macswap, and >>> 5tswap modes. To facilitate adding this support, some new header files >>> were added. >>> >> >> Hi Mike, >> >> I think it makes sense to allow these functionalities to be combined. It >> does seem like that noisy neighbour shouldn't have been added as a >> standalone fowarding option in the first place, as it's not mutually >> exclusive with the other fwding modes. Now it's forcing a user to set a >> fwding mode within a fwding mode to combine the functionality :/ >> >> The approach is to allow the fwding modes to expose their forwarding >> method so noisy can call them. Did you consider to expose the noisy >> functionality and allow the other forwarding modes to call that when the >> --noisy-* flags are set? > > Personally I like this approach more, and that was the strategy I took > with the previous version. However, that change didn't seem popular at > the time. > > http://patches.dpdk.org/project/dpdk/patch/20230126045516.176917-1-mkp@redhat.com/#157095 > Ok, interesting, I hadn't realised that. It doesn't apply anymore since David has restructured the fowarding engines, but i applied to older code and took a look. I had been thinking to an expose a function to do the noisy processing (if any) like a do_noise() and then to return the packets to the original fowarding engine for tx, similar to what is done the opposite way around in v3. I can understand Aman's concerns about tx of pkts moved from the fwding engines to noisy fwd engine. I think we can summarize like this (will use macswp as example) v3 - fwding engines expose their processing (e.g. do_macswp) - user enables by setting --fwd=noisy and --noisy-fwd-mode=macswp - noisy vnf does rx and tx - noisy calls other fwding engine for other mode processing - backwards compatible as existing noisy params have no effect when fwd!=noisy v2 - noisy engine exposes a function for noise+tx (e.g. noisy_eth_tx_burst()) - other fwd engines call this to add noise and tx - user enables by setting --fwd=macswp and setting existing noisy params - breaks backwards compatibility as previously noisy params would have no effect in macswp mode It seems the ideal would be to have taken noisy as a feature, not a fwding engine but it's easy in hindsight. I'm not sure it's worth another big rework to try and add a do_noise() which may be a nice design but will break backwards compatability. So I'd probably vote for v3 design over v2. What do others think ? >> For example, a user sets fwding mode as normal 'fwd=macfwd' but then if >> the user adds the --noisy-* params, the macfwd'ing mode calls some >> functions exposed to add the noise, which seems a bit more intuitive. >> >> Also, if someone adds a new fwding mode, they can just call the noisy >> functions and don't have update noisy fwding mode to add the new mode. >> >> I'm not sure if it's better, just throwing it out there as an >> alternative idea. >> >> On the downside, it would break backwards compatibility because >> previously those --noisy-* params would have had no effect with say >> macfwd mode, but now they will. So maybe that's enough to prohibit it. >> >> In the past, I would have had all the params set and just changed >> fwdmode to enable/disable noisy vnf. That would behaviour would now be >> changed with this approach. >> >> What do you think? >> >> Few comments on the code below. >> >> thanks, >> Kevin. >> >>> Signed-off-by: Mike Pattrick >>> --- >>> v2: Reverted changes to random memory lookup >>> v3: Refactored entire patch >>> --- >>> app/test-pmd/5tswap.c | 118 +---------------------- >>> app/test-pmd/5tswap.h | 130 ++++++++++++++++++++++++++ >>> app/test-pmd/macfwd.c | 33 +------ >>> app/test-pmd/macfwd.h | 46 +++++++++ >>> app/test-pmd/noisy_vnf.c | 92 +++++++++++++++--- >>> app/test-pmd/parameters.c | 17 ++++ >>> app/test-pmd/testpmd.c | 5 + >>> app/test-pmd/testpmd.h | 8 ++ >>> doc/guides/testpmd_app_ug/run_app.rst | 9 ++ >>> 9 files changed, 299 insertions(+), 159 deletions(-) >>> create mode 100644 app/test-pmd/5tswap.h >>> create mode 100644 app/test-pmd/macfwd.h >>> >>> diff --git a/app/test-pmd/5tswap.c b/app/test-pmd/5tswap.c >>> index ff8c2dcde5..8e8de2557a 100644 >>> --- a/app/test-pmd/5tswap.c >>> +++ b/app/test-pmd/5tswap.c >>> @@ -17,64 +17,8 @@ >>> #include >>> #include >>> >>> -#include "macswap_common.h" >>> #include "testpmd.h" >>> - >>> - >>> -static inline void >>> -swap_mac(struct rte_ether_hdr *eth_hdr) >>> -{ >>> - struct rte_ether_addr addr; >>> - >>> - /* Swap dest and src mac addresses. */ >>> - rte_ether_addr_copy(ð_hdr->dst_addr, &addr); >>> - rte_ether_addr_copy(ð_hdr->src_addr, ð_hdr->dst_addr); >>> - rte_ether_addr_copy(&addr, ð_hdr->src_addr); >>> -} >>> - >>> -static inline void >>> -swap_ipv4(struct rte_ipv4_hdr *ipv4_hdr) >>> -{ >>> - rte_be32_t addr; >>> - >>> - /* Swap dest and src ipv4 addresses. */ >>> - addr = ipv4_hdr->src_addr; >>> - ipv4_hdr->src_addr = ipv4_hdr->dst_addr; >>> - ipv4_hdr->dst_addr = addr; >>> -} >>> - >>> -static inline void >>> -swap_ipv6(struct rte_ipv6_hdr *ipv6_hdr) >>> -{ >>> - uint8_t addr[16]; >>> - >>> - /* Swap dest and src ipv6 addresses. */ >>> - memcpy(&addr, &ipv6_hdr->src_addr, 16); >>> - memcpy(&ipv6_hdr->src_addr, &ipv6_hdr->dst_addr, 16); >>> - memcpy(&ipv6_hdr->dst_addr, &addr, 16); >>> -} >>> - >>> -static inline void >>> -swap_tcp(struct rte_tcp_hdr *tcp_hdr) >>> -{ >>> - rte_be16_t port; >>> - >>> - /* Swap dest and src tcp port. */ >>> - port = tcp_hdr->src_port; >>> - tcp_hdr->src_port = tcp_hdr->dst_port; >>> - tcp_hdr->dst_port = port; >>> -} >>> - >>> -static inline void >>> -swap_udp(struct rte_udp_hdr *udp_hdr) >>> -{ >>> - rte_be16_t port; >>> - >>> - /* Swap dest and src udp port */ >>> - port = udp_hdr->src_port; >>> - udp_hdr->src_port = udp_hdr->dst_port; >>> - udp_hdr->dst_port = port; >>> -} >>> +#include "5tswap.h" >>> >>> /* >>> * 5 tuple swap forwarding mode: Swap the source and the destination of layers >>> @@ -85,22 +29,7 @@ static bool >>> pkt_burst_5tuple_swap(struct fwd_stream *fs) >>> { >>> struct rte_mbuf *pkts_burst[MAX_PKT_BURST]; >>> - struct rte_port *txp; >>> - struct rte_mbuf *mb; >>> - uint16_t next_proto; >>> - uint64_t ol_flags; >>> - uint16_t proto; >>> uint16_t nb_rx; >>> - int i; >>> - union { >>> - struct rte_ether_hdr *eth; >>> - struct rte_vlan_hdr *vlan; >>> - struct rte_ipv4_hdr *ipv4; >>> - struct rte_ipv6_hdr *ipv6; >>> - struct rte_tcp_hdr *tcp; >>> - struct rte_udp_hdr *udp; >>> - uint8_t *byte; >>> - } h; >>> >>> /* >>> * Receive a burst of packets and forward them. >>> @@ -109,49 +38,8 @@ pkt_burst_5tuple_swap(struct fwd_stream *fs) >>> if (unlikely(nb_rx == 0)) >>> return false; >>> >>> - txp = &ports[fs->tx_port]; >>> - ol_flags = ol_flags_init(txp->dev_conf.txmode.offloads); >>> - vlan_qinq_set(pkts_burst, nb_rx, ol_flags, >>> - txp->tx_vlan_id, txp->tx_vlan_id_outer); >>> - for (i = 0; i < nb_rx; i++) { >>> - if (likely(i < nb_rx - 1)) >>> - rte_prefetch0(rte_pktmbuf_mtod(pkts_burst[i+1], >>> - void *)); >>> - mb = pkts_burst[i]; >>> - h.eth = rte_pktmbuf_mtod(mb, struct rte_ether_hdr *); >>> - proto = h.eth->ether_type; >>> - swap_mac(h.eth); >>> - mb->l2_len = sizeof(struct rte_ether_hdr); >>> - h.eth++; >>> - while (proto == RTE_BE16(RTE_ETHER_TYPE_VLAN) || >>> - proto == RTE_BE16(RTE_ETHER_TYPE_QINQ)) { >>> - proto = h.vlan->eth_proto; >>> - h.vlan++; >>> - mb->l2_len += sizeof(struct rte_vlan_hdr); >>> - } >>> - if (proto == RTE_BE16(RTE_ETHER_TYPE_IPV4)) { >>> - swap_ipv4(h.ipv4); >>> - next_proto = h.ipv4->next_proto_id; >>> - mb->l3_len = rte_ipv4_hdr_len(h.ipv4); >>> - h.byte += mb->l3_len; >>> - } else if (proto == RTE_BE16(RTE_ETHER_TYPE_IPV6)) { >>> - swap_ipv6(h.ipv6); >>> - next_proto = h.ipv6->proto; >>> - h.ipv6++; >>> - mb->l3_len = sizeof(struct rte_ipv6_hdr); >>> - } else { >>> - mbuf_field_set(mb, ol_flags); >>> - continue; >>> - } >>> - if (next_proto == IPPROTO_UDP) { >>> - swap_udp(h.udp); >>> - mb->l4_len = sizeof(struct rte_udp_hdr); >>> - } else if (next_proto == IPPROTO_TCP) { >>> - swap_tcp(h.tcp); >>> - mb->l4_len = (h.tcp->data_off & 0xf0) >> 2; >>> - } >>> - mbuf_field_set(mb, ol_flags); >>> - } >>> + do_5tswap(pkts_burst, nb_rx, fs); >>> + >> >> this simplification is nice. >> >>> common_fwd_stream_transmit(fs, pkts_burst, nb_rx); >>> >>> return true; >>> diff --git a/app/test-pmd/5tswap.h b/app/test-pmd/5tswap.h >>> new file mode 100644 >>> index 0000000000..48da9236dc >>> --- /dev/null >>> +++ b/app/test-pmd/5tswap.h >>> @@ -0,0 +1,130 @@ >>> +/* SPDX-License-Identifier: BSD-3-Clause >>> + * Copyright(c) 2018 Intel Corporation >>> + */ >>> + >>> +#ifndef _5TSWAP_H_ >>> +#define _5TSWAP_H_ >>> + >>> +#include "macswap_common.h" >>> + >>> +static inline void >>> +swap_mac(struct rte_ether_hdr *eth_hdr) >>> +{ >>> + struct rte_ether_addr addr; >>> + >>> + /* Swap dest and src mac addresses. */ >>> + rte_ether_addr_copy(ð_hdr->dst_addr, &addr); >>> + rte_ether_addr_copy(ð_hdr->src_addr, ð_hdr->dst_addr); >>> + rte_ether_addr_copy(&addr, ð_hdr->src_addr); >>> +} >>> + >>> +static inline void >>> +swap_ipv4(struct rte_ipv4_hdr *ipv4_hdr) >>> +{ >>> + rte_be32_t addr; >>> + >>> + /* Swap dest and src ipv4 addresses. */ >>> + addr = ipv4_hdr->src_addr; >>> + ipv4_hdr->src_addr = ipv4_hdr->dst_addr; >>> + ipv4_hdr->dst_addr = addr; >>> +} >>> + >>> +static inline void >>> +swap_ipv6(struct rte_ipv6_hdr *ipv6_hdr) >>> +{ >>> + uint8_t addr[16]; >>> + >>> + /* Swap dest and src ipv6 addresses. */ >>> + memcpy(&addr, &ipv6_hdr->src_addr, 16); >>> + memcpy(&ipv6_hdr->src_addr, &ipv6_hdr->dst_addr, 16); >>> + memcpy(&ipv6_hdr->dst_addr, &addr, 16); >>> +} >>> + >>> +static inline void >>> +swap_tcp(struct rte_tcp_hdr *tcp_hdr) >>> +{ >>> + rte_be16_t port; >>> + >>> + /* Swap dest and src tcp port. */ >>> + port = tcp_hdr->src_port; >>> + tcp_hdr->src_port = tcp_hdr->dst_port; >>> + tcp_hdr->dst_port = port; >>> +} >>> + >>> +static inline void >>> +swap_udp(struct rte_udp_hdr *udp_hdr) >>> +{ >>> + rte_be16_t port; >>> + >>> + /* Swap dest and src udp port */ >>> + port = udp_hdr->src_port; >>> + udp_hdr->src_port = udp_hdr->dst_port; >>> + udp_hdr->dst_port = port; >>> +} >>> + >>> +static inline void >>> +do_5tswap(struct rte_mbuf *pkts_burst[], uint16_t nb_rx, >>> + struct fwd_stream *fs) >>> +{ >>> + struct rte_port *txp; >>> + struct rte_mbuf *mb; >>> + uint16_t next_proto; >>> + uint64_t ol_flags; >>> + uint16_t proto; >>> + int i; >>> + union { >>> + struct rte_ether_hdr *eth; >>> + struct rte_vlan_hdr *vlan; >>> + struct rte_ipv4_hdr *ipv4; >>> + struct rte_ipv6_hdr *ipv6; >>> + struct rte_tcp_hdr *tcp; >>> + struct rte_udp_hdr *udp; >>> + uint8_t *byte; >>> + } h; >>> + >>> + txp = &ports[fs->tx_port]; >>> + ol_flags = ol_flags_init(txp->dev_conf.txmode.offloads); >>> + vlan_qinq_set(pkts_burst, nb_rx, ol_flags, >>> + txp->tx_vlan_id, txp->tx_vlan_id_outer); >>> + for (i = 0; i < nb_rx; i++) { >>> + if (likely(i < nb_rx - 1)) >>> + rte_prefetch0(rte_pktmbuf_mtod(pkts_burst[i+1], >>> + void *)); >>> + mb = pkts_burst[i]; >>> + h.eth = rte_pktmbuf_mtod(mb, struct rte_ether_hdr *); >>> + proto = h.eth->ether_type; >>> + swap_mac(h.eth); >>> + mb->l2_len = sizeof(struct rte_ether_hdr); >>> + h.eth++; >>> + while (proto == RTE_BE16(RTE_ETHER_TYPE_VLAN) || >>> + proto == RTE_BE16(RTE_ETHER_TYPE_QINQ)) { >>> + proto = h.vlan->eth_proto; >>> + h.vlan++; >>> + mb->l2_len += sizeof(struct rte_vlan_hdr); >>> + } >>> + if (proto == RTE_BE16(RTE_ETHER_TYPE_IPV4)) { >>> + swap_ipv4(h.ipv4); >>> + next_proto = h.ipv4->next_proto_id; >>> + mb->l3_len = rte_ipv4_hdr_len(h.ipv4); >>> + h.byte += mb->l3_len; >>> + } else if (proto == RTE_BE16(RTE_ETHER_TYPE_IPV6)) { >>> + swap_ipv6(h.ipv6); >>> + next_proto = h.ipv6->proto; >>> + h.ipv6++; >>> + mb->l3_len = sizeof(struct rte_ipv6_hdr); >>> + } else { >>> + mbuf_field_set(mb, ol_flags); >>> + continue; >>> + } >>> + if (next_proto == IPPROTO_UDP) { >>> + swap_udp(h.udp); >>> + mb->l4_len = sizeof(struct rte_udp_hdr); >>> + } else if (next_proto == IPPROTO_TCP) { >>> + swap_tcp(h.tcp); >>> + mb->l4_len = (h.tcp->data_off & 0xf0) >> 2; >>> + } >>> + mbuf_field_set(mb, ol_flags); >>> + } >>> +} >>> + >>> +#endif /* _5TSWAP_H_ */ >>> diff --git a/app/test-pmd/macfwd.c b/app/test-pmd/macfwd.c >>> index 7316d73315..d19ace7395 100644 >>> --- a/app/test-pmd/macfwd.c >>> +++ b/app/test-pmd/macfwd.c >>> @@ -35,6 +35,7 @@ >>> #include >>> >>> #include "testpmd.h" >>> +#include "macfwd.h" >>> >>> /* >>> * Forwarding of packets in MAC mode. >>> @@ -45,13 +46,7 @@ static bool >>> pkt_burst_mac_forward(struct fwd_stream *fs) >>> { >>> struct rte_mbuf *pkts_burst[MAX_PKT_BURST]; >>> - struct rte_port *txp; >>> - struct rte_mbuf *mb; >>> - struct rte_ether_hdr *eth_hdr; >>> uint16_t nb_rx; >>> - uint16_t i; >>> - uint64_t ol_flags = 0; >>> - uint64_t tx_offloads; >>> >>> /* >>> * Receive a burst of packets and forward them. >>> @@ -60,31 +55,7 @@ pkt_burst_mac_forward(struct fwd_stream *fs) >>> if (unlikely(nb_rx == 0)) >>> return false; >>> >>> - txp = &ports[fs->tx_port]; >>> - tx_offloads = txp->dev_conf.txmode.offloads; >>> - if (tx_offloads & RTE_ETH_TX_OFFLOAD_VLAN_INSERT) >>> - ol_flags = RTE_MBUF_F_TX_VLAN; >>> - if (tx_offloads & RTE_ETH_TX_OFFLOAD_QINQ_INSERT) >>> - ol_flags |= RTE_MBUF_F_TX_QINQ; >>> - if (tx_offloads & RTE_ETH_TX_OFFLOAD_MACSEC_INSERT) >>> - ol_flags |= RTE_MBUF_F_TX_MACSEC; >>> - for (i = 0; i < nb_rx; i++) { >>> - if (likely(i < nb_rx - 1)) >>> - rte_prefetch0(rte_pktmbuf_mtod(pkts_burst[i + 1], >>> - void *)); >>> - mb = pkts_burst[i]; >>> - eth_hdr = rte_pktmbuf_mtod(mb, struct rte_ether_hdr *); >>> - rte_ether_addr_copy(&peer_eth_addrs[fs->peer_addr], >>> - ð_hdr->dst_addr); >>> - rte_ether_addr_copy(&ports[fs->tx_port].eth_addr, >>> - ð_hdr->src_addr); >>> - mb->ol_flags &= RTE_MBUF_F_INDIRECT | RTE_MBUF_F_EXTERNAL; >>> - mb->ol_flags |= ol_flags; >>> - mb->l2_len = sizeof(struct rte_ether_hdr); >>> - mb->l3_len = sizeof(struct rte_ipv4_hdr); >>> - mb->vlan_tci = txp->tx_vlan_id; >>> - mb->vlan_tci_outer = txp->tx_vlan_id_outer; >>> - } >>> + do_macfwd(pkts_burst, nb_rx, fs); >>> >>> common_fwd_stream_transmit(fs, pkts_burst, nb_rx); >>> >>> diff --git a/app/test-pmd/macfwd.h b/app/test-pmd/macfwd.h >>> new file mode 100644 >>> index 0000000000..3f3e7189e1 >>> --- /dev/null >>> +++ b/app/test-pmd/macfwd.h >>> @@ -0,0 +1,46 @@ >>> +/* SPDX-License-Identifier: BSD-3-Clause >>> + * Copyright(c) 2018 Intel Corporation >>> + */ >>> + >>> +#ifndef _MACFWD_H_ >>> +#define _MACFWD_H_ >>> + >>> +static inline void >>> +do_macfwd(struct rte_mbuf *pkts_burst[], uint16_t nb_rx, >>> + struct fwd_stream *fs) >> >> nit: indent/alignment is a little off here. There is some extra spaces. >> Same with other do_*() >> >>> +{ >>> + struct rte_ether_hdr *eth_hdr; >>> + uint64_t ol_flags = 0; >>> + uint64_t tx_offloads; >>> + struct rte_mbuf *mb; >>> + struct rte_port *txp = &ports[fs->tx_port]; >>> + uint16_t i; >>> + >>> + >> >> can remove addition blank line >> >>> + tx_offloads = txp->dev_conf.txmode.offloads; >>> + if (tx_offloads & RTE_ETH_TX_OFFLOAD_VLAN_INSERT) >>> + ol_flags = RTE_MBUF_F_TX_VLAN; >>> + if (tx_offloads & RTE_ETH_TX_OFFLOAD_QINQ_INSERT) >>> + ol_flags |= RTE_MBUF_F_TX_QINQ; >>> + if (tx_offloads & RTE_ETH_TX_OFFLOAD_MACSEC_INSERT) >>> + ol_flags |= RTE_MBUF_F_TX_MACSEC; >>> + for (i = 0; i < nb_rx; i++) { >>> + if (likely(i < nb_rx - 1)) >>> + rte_prefetch0(rte_pktmbuf_mtod(pkts_burst[i + 1], >>> + void *)); >>> + mb = pkts_burst[i]; >>> + eth_hdr = rte_pktmbuf_mtod(mb, struct rte_ether_hdr *); >>> + rte_ether_addr_copy(&peer_eth_addrs[fs->peer_addr], >>> + ð_hdr->dst_addr); >>> + rte_ether_addr_copy(&ports[fs->tx_port].eth_addr, >>> + ð_hdr->src_addr); >>> + mb->ol_flags &= RTE_MBUF_F_INDIRECT | RTE_MBUF_F_EXTERNAL; >>> + mb->ol_flags |= ol_flags; >>> + mb->l2_len = sizeof(struct rte_ether_hdr); >>> + mb->l3_len = sizeof(struct rte_ipv4_hdr); >>> + mb->vlan_tci = txp->tx_vlan_id; >>> + mb->vlan_tci_outer = txp->tx_vlan_id_outer; >>> + } >>> +} >>> + >>> +#endif /* _MACFWD_H_ */ >>> diff --git a/app/test-pmd/noisy_vnf.c b/app/test-pmd/noisy_vnf.c >>> index 2bf90a983c..1d5a2e470a 100644 >>> --- a/app/test-pmd/noisy_vnf.c >>> +++ b/app/test-pmd/noisy_vnf.c >>> @@ -32,6 +32,18 @@ >>> #include >>> >>> #include "testpmd.h" >>> +#include "5tswap.h" >>> +#include "macfwd.h" >>> +#if defined(RTE_ARCH_X86) >>> +#include "macswap_sse.h" >>> +#elif defined(__ARM_NEON) >>> +#include "macswap_neon.h" >>> +#else >>> +#include "macswap.h" >>> +#endif >>> + >>> +#define NOISY_STRSIZE 256 >>> +#define NOISY_RING "noisy_ring_%d\n" >>> >>> struct noisy_config { >>> struct rte_ring *f; >>> @@ -80,9 +92,6 @@ sim_memory_lookups(struct noisy_config *ncf, uint16_t nb_pkts) >>> { >>> uint16_t i, j; >>> >>> - if (!ncf->do_sim) >>> - return; >>> - >> >> Why is this removed? It's not checked before all calls to this function >> >>> for (i = 0; i < nb_pkts; i++) { >>> for (j = 0; j < noisy_lkup_num_writes; j++) >>> do_write(ncf->vnf_mem); >>> @@ -110,15 +119,13 @@ sim_memory_lookups(struct noisy_config *ncf, uint16_t nb_pkts) >>> * out of the FIFO >>> * 4. Cases 2 and 3 combined >>> */ >>> -static bool >>> -pkt_burst_noisy_vnf(struct fwd_stream *fs) >>> +static uint16_t >>> +noisy_eth_tx_burst(struct fwd_stream *fs, uint16_t nb_rx, struct rte_mbuf **pkts_burst) >>> { >>> const uint64_t freq_khz = rte_get_timer_hz() / 1000; >>> struct noisy_config *ncf = noisy_cfg[fs->rx_port]; >>> - struct rte_mbuf *pkts_burst[MAX_PKT_BURST]; >>> struct rte_mbuf *tmp_pkts[MAX_PKT_BURST]; >>> uint16_t nb_deqd = 0; >>> - uint16_t nb_rx = 0; >>> uint16_t nb_tx = 0; >>> uint16_t nb_enqd; >>> unsigned int fifo_free; >>> @@ -126,12 +133,12 @@ pkt_burst_noisy_vnf(struct fwd_stream *fs) >>> bool needs_flush = false; >>> uint64_t now; >>> >>> - nb_rx = common_fwd_stream_receive(fs, pkts_burst, nb_pkt_per_burst); >>> if (unlikely(nb_rx == 0)) >>> goto flush; >>> >>> if (!ncf->do_buffering) { >>> - sim_memory_lookups(ncf, nb_rx); >>> + if (ncf->do_sim) >>> + sim_memory_lookups(ncf, nb_rx); >>> nb_tx = common_fwd_stream_transmit(fs, pkts_burst, nb_rx); >>> goto end; >>> } >>> @@ -169,11 +176,61 @@ pkt_burst_noisy_vnf(struct fwd_stream *fs) >>> ncf->prev_time = rte_get_timer_cycles(); >>> } >>> end: >>> + return nb_tx; >>> +} >>> + >>> +static bool >>> +pkt_burst_io(struct fwd_stream *fs) >>> +{ >>> + struct rte_mbuf *pkts_burst[MAX_PKT_BURST]; >>> + uint16_t nb_rx; >>> + uint16_t nb_tx; >>> + >>> + nb_rx = common_fwd_stream_receive(fs, pkts_burst, nb_pkt_per_burst); >>> + nb_tx = noisy_eth_tx_burst(fs, nb_rx, pkts_burst); >>> + >>> return nb_rx > 0 || nb_tx > 0; >>> } >>> >>> -#define NOISY_STRSIZE 256 >>> -#define NOISY_RING "noisy_ring_%d\n" >>> +static bool >>> +pkt_burst_mac(struct fwd_stream *fs) >>> +{ >>> + struct rte_mbuf *pkts_burst[MAX_PKT_BURST]; >>> + uint16_t nb_rx; >>> + uint16_t nb_tx; >>> + >>> + nb_rx = common_fwd_stream_receive(fs, pkts_burst, nb_pkt_per_burst); >> >> Similar to macfwd.c, you can add check for unlikely(nb_rx == 0) at this >> point in these fns. > > That's true. I removed it because I'll have to call > noisy_eth_tx_burst() regardless, but we could avoid the middle call in > these locations. > > I agree with the other suggestions. > > Thanks, > M > >> >>> + do_macfwd(pkts_burst, nb_rx, fs); >>> + nb_tx = noisy_eth_tx_burst(fs, nb_rx, pkts_burst); >>> + >>> + return nb_rx > 0 || nb_tx > 0; >>> +} >>> +static bool >>> +pkt_burst_macswap(struct fwd_stream *fs) >>> +{ >>> + struct rte_mbuf *pkts_burst[MAX_PKT_BURST]; >>> + uint16_t nb_rx = 0; >>> + uint16_t nb_tx = 0; >> >> nit: these are not assigned in the other functions >> >>> + >>> + nb_rx = common_fwd_stream_receive(fs, pkts_burst, nb_pkt_per_burst); >>> + do_macswap(pkts_burst, nb_rx, &ports[fs->tx_port]); >>> + nb_tx = noisy_eth_tx_burst(fs, nb_rx, pkts_burst); >>> + >>> + return nb_rx > 0 || nb_tx > 0; >>> +} >>> +static bool >>> +pkt_brust_5tswap(struct fwd_stream *fs) >>> +{ >>> + struct rte_mbuf *pkts_burst[MAX_PKT_BURST]; >>> + uint16_t nb_rx = 0; >>> + uint16_t nb_tx = 0; >>> + >>> + nb_rx = common_fwd_stream_receive(fs, pkts_burst, nb_pkt_per_burst); >>> + do_5tswap(pkts_burst, nb_rx, fs); >>> + nb_tx = noisy_eth_tx_burst(fs, nb_rx, pkts_burst); >>> + >>> + return nb_rx > 0 || nb_tx > 0; >>> +} >>> >>> static void >>> noisy_fwd_end(portid_t pi) >>> @@ -226,6 +283,15 @@ noisy_fwd_begin(portid_t pi) >>> "--noisy-lkup-memory-size must be > 0\n"); >>> } >>> >>> + if (noisy_fwd_mode == NOISY_FWD_MODE_IO) >>> + noisy_vnf_engine.packet_fwd = pkt_burst_io; >>> + else if (noisy_fwd_mode == NOISY_FWD_MODE_MAC) >>> + noisy_vnf_engine.packet_fwd = pkt_burst_mac; >>> + else if (noisy_fwd_mode == NOISY_FWD_MODE_MACSWAP) >>> + noisy_vnf_engine.packet_fwd = pkt_burst_macswap; >>> + else if (noisy_fwd_mode == NOISY_FWD_MODE_5TSWAP) >>> + noisy_vnf_engine.packet_fwd = pkt_brust_5tswap; >>> + >>> return 0; >>> } >>> >>> @@ -233,6 +299,6 @@ struct fwd_engine noisy_vnf_engine = { >>> .fwd_mode_name = "noisy", >>> .port_fwd_begin = noisy_fwd_begin, >>> .port_fwd_end = noisy_fwd_end, >>> - .stream_init = common_fwd_stream_init, >>> - .packet_fwd = pkt_burst_noisy_vnf, >>> + .stream_init = common_fwd_stream_init, >>> + .packet_fwd = pkt_burst_io, >>> }; >>> diff --git a/app/test-pmd/parameters.c b/app/test-pmd/parameters.c >>> index 3b37809baf..129c55c0ad 100644 >>> --- a/app/test-pmd/parameters.c >>> +++ b/app/test-pmd/parameters.c >>> @@ -196,6 +196,7 @@ usage(char* progname) >>> printf(" --noisy-lkup-num-writes=N: do N random writes per packet\n"); >>> printf(" --noisy-lkup-num-reads=N: do N random reads per packet\n"); >>> printf(" --noisy-lkup-num-reads-writes=N: do N random reads and writes per packet\n"); >>> + printf(" --noisy-fwd-mode=mode: set the fwd mode\n"); >>> printf(" --no-iova-contig: mempool memory can be IOVA non contiguous. " >>> "valid only with --mp-alloc=anon\n"); >>> printf(" --rx-mq-mode=0xX: hexadecimal bitmask of RX mq mode can be " >>> @@ -704,6 +705,7 @@ launch_args_parse(int argc, char** argv) >>> { "noisy-lkup-num-writes", 1, 0, 0 }, >>> { "noisy-lkup-num-reads", 1, 0, 0 }, >>> { "noisy-lkup-num-reads-writes", 1, 0, 0 }, >>> + { "noisy-fwd-mode", 1, 0, 0 }, >>> { "no-iova-contig", 0, 0, 0 }, >>> { "rx-mq-mode", 1, 0, 0 }, >>> { "record-core-cycles", 0, 0, 0 }, >>> @@ -1444,6 +1446,21 @@ launch_args_parse(int argc, char** argv) >>> rte_exit(EXIT_FAILURE, >>> "noisy-lkup-num-reads-writes must be >= 0\n"); >>> } >>> + if (!strcmp(lgopts[opt_idx].name, >>> + "noisy-fwd-mode")) { >>> + if (!strcmp(optarg, "io")) >>> + noisy_fwd_mode = NOISY_FWD_MODE_IO; >>> + else if (!strcmp(optarg, "mac")) >>> + noisy_fwd_mode = NOISY_FWD_MODE_MAC; >>> + else if (!strcmp(optarg, "macswap")) >>> + noisy_fwd_mode = NOISY_FWD_MODE_MACSWAP; >>> + else if (!strcmp(optarg, "5tswap")) >>> + noisy_fwd_mode = NOISY_FWD_MODE_5TSWAP; >>> + else >>> + rte_exit(EXIT_FAILURE, "noisy-fwd-mode %s invalid -" >>> + " must b a valid fwd mode\n", >> >> typo "be" >> >> I would specify "..must be a valid noisy-fwd-mode value" to avoid >> confusion with the full set of general fwd modes and easier for user to >> search them in docs. >> >>> + optarg); >>> + } >>> if (!strcmp(lgopts[opt_idx].name, "no-iova-contig")) >>> mempool_flags = RTE_MEMPOOL_F_NO_IOVA_CONTIG; >>> >>> diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c >>> index 5cb6f92523..92784873ff 100644 >>> --- a/app/test-pmd/testpmd.c >>> +++ b/app/test-pmd/testpmd.c >>> @@ -364,6 +364,11 @@ uint64_t noisy_lkup_num_reads; >>> */ >>> uint64_t noisy_lkup_num_reads_writes; >>> >>> +/* >>> + * Configurable forwarding mode in VNF simulation. >>> + */ >>> +int noisy_fwd_mode; >>> + >>> /* >>> * Receive Side Scaling (RSS) configuration. >>> */ >>> diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h >>> index bdfbfd36d3..f70397ad26 100644 >>> --- a/app/test-pmd/testpmd.h >>> +++ b/app/test-pmd/testpmd.h >>> @@ -116,6 +116,13 @@ enum { >>> QUEUE_JOB_TYPE_ACTION_QUERY, >>> }; >>> >>> +enum { >>> + NOISY_FWD_MODE_IO, >>> + NOISY_FWD_MODE_MAC, >>> + NOISY_FWD_MODE_MACSWAP, >>> + NOISY_FWD_MODE_5TSWAP >>> +}; >>> + >>> /** >>> * The data structure associated with RX and TX packet burst statistics >>> * that are recorded for each forwarding stream. >>> @@ -561,6 +568,7 @@ extern uint64_t noisy_lkup_mem_sz; >>> extern uint64_t noisy_lkup_num_writes; >>> extern uint64_t noisy_lkup_num_reads; >>> extern uint64_t noisy_lkup_num_reads_writes; >>> +extern int noisy_fwd_mode; >>> >>> extern uint8_t dcb_config; >>> >>> diff --git a/doc/guides/testpmd_app_ug/run_app.rst b/doc/guides/testpmd_app_ug/run_app.rst >>> index 57b23241cf..fcca3e8921 100644 >>> --- a/doc/guides/testpmd_app_ug/run_app.rst >>> +++ b/doc/guides/testpmd_app_ug/run_app.rst >>> @@ -519,6 +519,15 @@ The command line options are: >>> Set the number of r/w accesses to be done in noisy neighbor simulation memory buffer to N. >>> Only available with the noisy forwarding mode. The default value is 0. >>> >>> +* ``--noisy-fwd-mode=mode`` >>> + >>> + Set the noisy vnf forwarding mode where ``mode`` is one of the following:: >>> + >>> + io (the default) >>> + mac >>> + macswap >>> + 5tswap >>> + >>> * ``--no-iova-contig`` >>> >>> Enable to create mempool which is not IOVA contiguous. Valid only with --mp-alloc=anon. >> >