Hi,

Thanks for following up with this patch.
See my comments below.


On Thu, Jul 08, 2021 at 09:41:54PM +0530, kumar Amber wrote:
> From: Harry van Haaren <harry.van.haa...@intel.com>
> 
> This commit avoids many instances of "using subtable X for miniflow (x,y)"
> in the ovs-vswitchd log when using the DPCLS Autovalidator. This occurs
> when no specialized subtable is found, and the generic "_any" version of
> the avx512 subtable search implementation was used. This change logs the
> subtable usage once, avoiding duplicates.
> 
> Signed-off-by: Harry van Haaren <harry.van.haa...@intel.com>
> 
> ---
> v3:
> - add comments from Flavio
> - add documentation update
> ---
>  Documentation/topics/dpdk/bridge.rst   | 31 ++++++++++++++++++++++++++
>  lib/dpif-netdev-lookup-avx512-gather.c |  4 ++--
>  2 files changed, 33 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/topics/dpdk/bridge.rst 
> b/Documentation/topics/dpdk/bridge.rst
> index 0f70a0cad..e74c839b5 100644
> --- a/Documentation/topics/dpdk/bridge.rst
> +++ b/Documentation/topics/dpdk/bridge.rst
> @@ -182,6 +182,37 @@ chosen, and the 2nd occurance of that priority is not 
> used. Put in logical
>  terms, a subtable is chosen if its priority is greater than the previous
>  best candidate.
>  
> +Optimizing Specific Subtable Search
> +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> +
> +The DPCLS wildcarding engine can be "specialized" to handle specific subtable
> +searching even faster than the generic subtable search implementation.Not all
> +subtable searches are specialized, if you see a message like the following in
> +your OVS output, please run the below commands to inform OVS community of the

The documentation is supposed to help users to understand what is
happening and perhaps consider the next steps. The text above
requires deeper knowledge of how OVS internally works and provides
no indication of what is causing it or consequences like if it is
harmful or not. Also, it commits to add new specialized lookups
in the next release, which might not be realistic.

What do you think about the suggestion below based on your proposal?

----8<---
During the packet classification, the datapath can use specialized
lookup tables to optimize the search. However, not all situations
are optimized. If you see a message like the following one in the OVS
logs, it means that there is no specialized implementation available
for the current networking traffic. In this case, OVS will continue
to process the traffic normally using a more generic lookup table."

"Using non-specialized AVX512 lookup for subtable (4,1) and possibly others."

(Note that the numbers 4 and 1 will likely be different in your logs)

Additional specialized lookups can be added to OVS if the user
provides that log message along with the command output as show
below to the OVS mailing list. Note that the numbers in the log
message ("subtable (X,Y)") need to match with the numbers in
the provided command output ("dp-extra-info:miniflow_bits(X,Y)").

"ovs-appctl dpctl/dump-flows -m", which results in output like this:

    ufid:82770b5d-ca38-44ff-8283-74ba36bd1ca5, skb_priority(0/0),skb_mark(0/0)
    ,ct_state(0/0),ct_zone(0/0),ct_mark(0/0),ct_label(0/0),recirc_id(0),
    dp_hash(0/0),in_port(pcap0),packet_type(ns=0,id=0),eth(src=00:00:00:00:00:
    00/00:00:00:00:00:00,dst=ff:ff:ff:ff:ff:ff/00:00:00:00:00:00),eth_type(
    0x8100),vlan(vid=1,pcp=0),encap(eth_type(0x0800),ipv4(src=127.0.0.1/0.0.0.0
    ,dst=127.0.0.1/0.0.0.0,proto=17/0,tos=0/0,ttl=64/0,frag=no),udp(src=53/0,
    dst=53/0)), packets:77072681, bytes:3545343326, used:0.000s, dp:ovs,
    actions:vhostuserclient0, dp-extra-info:miniflow_bits(4,1)

----8<---

Thanks,
fbl

> +subtables in use in your deployment, and we can optimize these subtables for
> +you in the next release.
> +
> +"Using non-specialized AVX512 lookup for subtable (7,2) and possibly others."
> +
> +(Note that the numbers 7 and 2 will likely be different in your logs, these
> +are the numbers OVS community requires to specialize the subtable in your
> +deployment!)
> +
> +If you see this log message, please run the command
> +"ovs-appctl dpctl/dump-flows -m", which results in output like this:
> +
> +    ufid:82770b5d-ca38-44ff-8283-74ba36bd1ca5, 
> skb_priority(0/0),skb_mark(0/0)
> +    ,ct_state(0/0),ct_zone(0/0),ct_mark(0/0),ct_label(0/0),recirc_id(0),
> +    
> dp_hash(0/0),in_port(pcap0),packet_type(ns=0,id=0),eth(src=00:00:00:00:00:
> +    00/00:00:00:00:00:00,dst=ff:ff:ff:ff:ff:ff/00:00:00:00:00:00),eth_type(
> +    
> 0x8100),vlan(vid=1,pcp=0),encap(eth_type(0x0800),ipv4(src=127.0.0.1/0.0.0.0
> +    ,dst=127.0.0.1/0.0.0.0,proto=17/0,tos=0/0,ttl=64/0,frag=no),udp(src=53/0,
> +    dst=53/0)), packets:77072681, bytes:3545343326, used:0.000s, dp:ovs,
> +    actions:vhostuserclient0, dp-extra-info:miniflow_bits(4,1)
> +
> +Please send an email to the OVS mailing list ovs-dev@openvswitch.org with
> +the output of the "dp-extra-info:miniflow_bits(7,2)" values.
> +
>  CPU ISA Testing and Validation
>  ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>  
> diff --git a/lib/dpif-netdev-lookup-avx512-gather.c 
> b/lib/dpif-netdev-lookup-avx512-gather.c
> index bc359dc4a..ced846aa7 100644
> --- a/lib/dpif-netdev-lookup-avx512-gather.c
> +++ b/lib/dpif-netdev-lookup-avx512-gather.c
> @@ -411,8 +411,8 @@ dpcls_subtable_avx512_gather_probe(uint32_t u0_bits, 
> uint32_t u1_bits)
>       */
>      if (!f && (u0_bits + u1_bits) < (NUM_U64_IN_ZMM_REG * 2)) {
>          f = dpcls_avx512_gather_mf_any;
> -        VLOG_INFO("Using avx512_gather_mf_any for subtable (%d,%d)\n",
> -                  u0_bits, u1_bits);
> +        VLOG_INFO_ONCE("Using non-specialized AVX512 lookup for subtable"
> +                       " (%d,%d) and possibly others.", u0_bits, u1_bits);
>      }
>  
>      return f;
> -- 
> 2.25.1
> 

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

Reply via email to