netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Stefano Brivio <sbrivio@redhat.com>
To: Colin King <colin.king@canonical.com>
Cc: Pablo Neira Ayuso <pablo@netfilter.org>,
	Jozsef Kadlecsik <kadlec@netfilter.org>,
	Florian Westphal <fw@strlen.de>,
	"David S . Miller" <davem@davemloft.net>,
	Jakub Kicinski <kuba@kernel.org>,
	netfilter-devel@vger.kernel.org, coreteam@netfilter.org,
	netdev@vger.kernel.org, kernel-janitors@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH] nft_set_pipapo: remove unused pointer lt
Date: Tue, 7 Apr 2020 01:39:51 +0200	[thread overview]
Message-ID: <20200407013951.77a6409f@redhat.com> (raw)
In-Reply-To: <20200406232031.657615-1-colin.king@canonical.com>

Hi Colin,

On Tue,  7 Apr 2020 00:20:31 +0100
Colin King <colin.king@canonical.com> wrote:

> From: Colin Ian King <colin.king@canonical.com>
> 
> Pointer lt being assigned with a value that is never read and
> the pointer is redundant and can be removed.
> 
> Addresses-Coverity: ("Unused value")
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> ---
>  net/netfilter/nft_set_pipapo_avx2.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/net/netfilter/nft_set_pipapo_avx2.c b/net/netfilter/nft_set_pipapo_avx2.c
> index d65ae0e23028..9458c6b6ea04 100644
> --- a/net/netfilter/nft_set_pipapo_avx2.c
> +++ b/net/netfilter/nft_set_pipapo_avx2.c
> @@ -1049,11 +1049,9 @@ static int nft_pipapo_avx2_lookup_slow(unsigned long *map, unsigned long *fill,
>  					struct nft_pipapo_field *f, int offset,
>  					const u8 *pkt, bool first, bool last)
>  {
> -	unsigned long *lt = f->lt, bsize = f->bsize;
> +	unsigned long bsize = f->bsize;
>  	int i, ret = -1, b;
>  
> -	lt += offset * NFT_PIPAPO_LONGS_PER_M256;
> -
>  	if (first)
>  		memset(map, 0xff, bsize * sizeof(*map));
>  
        for (i = offset; i < bsize; i++) {
                if (f->bb == 8)
                        pipapo_and_field_buckets_8bit(f, map, pkt);
                else
                        pipapo_and_field_buckets_4bit(f, map, pkt);

Now, this function should never be called, it's provided as a safety net
in case this algorithm is ever run with some strange packet field size,
still, your clean-up shows another "issue" here: as
pipapo_and_field_buckets_*() functions use the full buckets in lookup
tables, not just starting from an offset, there's no need to repeat
those operations starting from offset up to bsize.

It's fine to ignore the offset (which is just a "hint" here for faster
lookups) -- this function isn't supposed to be optimised in any way.

That is, this for loop should go away altogether, and the 'offset'
argument should be dropped as well. Let me know if you're comfortable
taking care of that as well, or if you prefer that I send a patch.

-- 
Stefano


  reply	other threads:[~2020-04-06 23:40 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-04-06 23:20 [PATCH] nft_set_pipapo: remove unused pointer lt Colin King
2020-04-06 23:39 ` Stefano Brivio [this message]
2020-04-06 23:51   ` Colin Ian King

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200407013951.77a6409f@redhat.com \
    --to=sbrivio@redhat.com \
    --cc=colin.king@canonical.com \
    --cc=coreteam@netfilter.org \
    --cc=davem@davemloft.net \
    --cc=fw@strlen.de \
    --cc=kadlec@netfilter.org \
    --cc=kernel-janitors@vger.kernel.org \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=netfilter-devel@vger.kernel.org \
    --cc=pablo@netfilter.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).