* [PATCH] nft_set_pipapo: remove unused pointer lt
@ 2020-04-06 23:20 ` Colin King
0 siblings, 0 replies; 6+ messages in thread
From: Colin King @ 2020-04-06 23:20 UTC (permalink / raw)
To: Pablo Neira Ayuso, Jozsef Kadlecsik, Florian Westphal,
David S . Miller, Jakub Kicinski, netfilter-devel, coreteam,
netdev
Cc: kernel-janitors, linux-kernel
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));
--
2.25.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH] nft_set_pipapo: remove unused pointer lt @ 2020-04-06 23:20 ` Colin King 0 siblings, 0 replies; 6+ messages in thread From: Colin King @ 2020-04-06 23:20 UTC (permalink / raw) To: Pablo Neira Ayuso, Jozsef Kadlecsik, Florian Westphal, David S . Miller, Jakub Kicinski, netfilter-devel, coreteam, netdev Cc: kernel-janitors, linux-kernel 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)); -- 2.25.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] nft_set_pipapo: remove unused pointer lt 2020-04-06 23:20 ` Colin King @ 2020-04-06 23:39 ` Stefano Brivio -1 siblings, 0 replies; 6+ messages in thread From: Stefano Brivio @ 2020-04-06 23:39 UTC (permalink / raw) To: Colin King Cc: Pablo Neira Ayuso, Jozsef Kadlecsik, Florian Westphal, David S . Miller, Jakub Kicinski, netfilter-devel, coreteam, netdev, kernel-janitors, linux-kernel 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 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] nft_set_pipapo: remove unused pointer lt @ 2020-04-06 23:39 ` Stefano Brivio 0 siblings, 0 replies; 6+ messages in thread From: Stefano Brivio @ 2020-04-06 23:39 UTC (permalink / raw) To: Colin King Cc: Pablo Neira Ayuso, Jozsef Kadlecsik, Florian Westphal, David S . Miller, Jakub Kicinski, netfilter-devel, coreteam, netdev, kernel-janitors, linux-kernel 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 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] nft_set_pipapo: remove unused pointer lt 2020-04-06 23:39 ` Stefano Brivio @ 2020-04-06 23:51 ` Colin Ian King -1 siblings, 0 replies; 6+ messages in thread From: Colin Ian King @ 2020-04-06 23:51 UTC (permalink / raw) To: Stefano Brivio Cc: Pablo Neira Ayuso, Jozsef Kadlecsik, Florian Westphal, David S . Miller, Jakub Kicinski, netfilter-devel, coreteam, netdev, kernel-janitors, linux-kernel On 07/04/2020 00:39, Stefano Brivio wrote: > 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. Ah, OK, thanks for the detailed explanation. > > 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. Probably at this point I think I'll pass the baton over to you to fix this up as you understand the code more than I do. > Cheers, Colin ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] nft_set_pipapo: remove unused pointer lt @ 2020-04-06 23:51 ` Colin Ian King 0 siblings, 0 replies; 6+ messages in thread From: Colin Ian King @ 2020-04-06 23:51 UTC (permalink / raw) To: Stefano Brivio Cc: Pablo Neira Ayuso, Jozsef Kadlecsik, Florian Westphal, David S . Miller, Jakub Kicinski, netfilter-devel, coreteam, netdev, kernel-janitors, linux-kernel On 07/04/2020 00:39, Stefano Brivio wrote: > 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. Ah, OK, thanks for the detailed explanation. > > 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. Probably at this point I think I'll pass the baton over to you to fix this up as you understand the code more than I do. > Cheers, Colin ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-04-06 23:51 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2020-04-06 23:20 [PATCH] nft_set_pipapo: remove unused pointer lt Colin King 2020-04-06 23:20 ` Colin King 2020-04-06 23:39 ` Stefano Brivio 2020-04-06 23:39 ` Stefano Brivio 2020-04-06 23:51 ` Colin Ian King 2020-04-06 23:51 ` Colin Ian King
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.