Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Subject: Re: [net-next] seg6: add counters support for SRv6 Behaviors
To:     Andrea Mayer <andrea.mayer@uniroma2.it>,
        "David S. Miller" <davem@davemloft.net>,
        Hideaki YOSHIFUJI <yoshfuji@linux-ipv6.org>,
        David Ahern <dsahern@kernel.org>,
        Jakub Kicinski <kuba@kernel.org>, linux-kernel@vger.kernel.org,
        netdev@vger.kernel.org
Cc:     Stefano Salsano <stefano.salsano@uniroma2.it>,
        Paolo Lungaroni <paolo.lungaroni@uniroma2.it>,
        Ahmed Abdelsalam <ahabdels.dev@gmail.com>
References: <20210427154404.20546-1-andrea.mayer@uniroma2.it>
From:   David Ahern <dsahern@gmail.com>
Message-ID: <a49ed340-3f92-8533-3efb-ac7ee2231ca3@gmail.com>
Date:   Wed, 28 Apr 2021 20:03:23 -0600
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:78.0)
 Gecko/20100101 Thunderbird/78.10.0
MIME-Version: 1.0
In-Reply-To: <20210427154404.20546-1-andrea.mayer@uniroma2.it>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 4/27/21 9:44 AM, Andrea Mayer wrote:
> This patch provides counters for SRv6 Behaviors as defined in [1],
> section 6. For each SRv6 Behavior instance, counters defined in [1] are:
> 
>  - the total number of packets that have been correctly processed;
>  - the total amount of traffic in bytes of all packets that have been
>    correctly processed;
> 

...

> 
> Results of tests are shown in the following table:
> 
> Scenario (1): average 1504764,81 pps (~1504,76 kpps); std. dev 3956,82 pps
> Scenario (2): average 1501469,78 pps (~1501,47 kpps); std. dev 2979,85 pps
> Scenario (3): average 1501315,13 pps (~1501,32 kpps); std. dev 2956,00 pps
> 
> As can be observed, throughputs achieved in scenarios (2),(3) did not
> suffer any observable degradation compared to scenario (1).
> 
> Thanks to Jakub Kicinski and David Ahern for their valuable suggestions
> and comments provided during the discussion of the proposed RFCs.
> 
> [2] https://www.cloudlab.us
> 
> Signed-off-by: Andrea Mayer <andrea.mayer@uniroma2.it>
> ---
>  include/uapi/linux/seg6_local.h |  30 +++++
>  net/ipv6/seg6_local.c           | 198 +++++++++++++++++++++++++++++++-
>  2 files changed, 226 insertions(+), 2 deletions(-)

Thanks for the detailed commit message and stats on performance impact.

> @@ -977,7 +1044,14 @@ static int seg6_local_input(struct sk_buff *skb)
>  	slwt = seg6_local_lwtunnel(orig_dst->lwtstate);
>  	desc = slwt->desc;
>  
> -	return desc->input(skb, slwt);
> +	rc = desc->input(skb, slwt);
> +
> +	if (!seg6_lwtunnel_counters_enabled(slwt))
> +		return rc;
> +
> +	seg6_local_update_counters(slwt, len, rc);
> +
> +	return rc;

Nit: This would be simpler as

	if (seg6_lwtunnel_counters_enabled(slwt))
		seg6_local_update_counters(slwt, len, rc);

	return rc;

but not worth a re-do since net-next is about to close, so:

Reviewed-by: David Ahern <dsahern@kernel.org>