From: Stefano Brivio <sbrivio@redhat.com>
To: Florian Westphal <fw@strlen.de>
Cc: <netfilter-devel@vger.kernel.org>
Subject: Re: [PATCH nf-next 1/4] netfilter: nft_set_pipapo: constify lookup fn args where possible
Date: Tue, 13 Feb 2024 08:19:29 +0100 [thread overview]
Message-ID: <20240213081929.218433a2@elisabeth> (raw)
In-Reply-To: <20240212100202.10116-2-fw@strlen.de>
Nits only:
On Mon, 12 Feb 2024 11:01:50 +0100
Florian Westphal <fw@strlen.de> wrote:
> Those get called from packet path, content must not be modified.
> No functional changes intended.
>
> Signed-off-by: Florian Westphal <fw@strlen.de>
> ---
> net/netfilter/nft_set_pipapo.c | 16 ++++++++--------
> net/netfilter/nft_set_pipapo.h | 6 +++---
> net/netfilter/nft_set_pipapo_avx2.c | 26 +++++++++++++-------------
> 3 files changed, 24 insertions(+), 24 deletions(-)
>
> diff --git a/net/netfilter/nft_set_pipapo.c b/net/netfilter/nft_set_pipapo.c
> index ed243edaa6a0..395420fa71e5 100644
> --- a/net/netfilter/nft_set_pipapo.c
> +++ b/net/netfilter/nft_set_pipapo.c
> @@ -360,7 +360,7 @@
> * Return: -1 on no match, bit position on 'match_only', 0 otherwise.
> */
> int pipapo_refill(unsigned long *map, int len, int rules, unsigned long *dst,
> - union nft_pipapo_map_bucket *mt, bool match_only)
> + const union nft_pipapo_map_bucket *mt, bool match_only)
> {
> unsigned long bitset;
> int k, ret = -1;
> @@ -412,9 +412,9 @@ bool nft_pipapo_lookup(const struct net *net, const struct nft_set *set,
> struct nft_pipapo_scratch *scratch;
> unsigned long *res_map, *fill_map;
> u8 genmask = nft_genmask_cur(net);
> + const struct nft_pipapo_match *m;
> + const struct nft_pipapo_field *f;
> const u8 *rp = (const u8 *)key;
> - struct nft_pipapo_match *m;
> - struct nft_pipapo_field *f;
> bool map_index;
> int i;
>
> @@ -521,9 +521,9 @@ static struct nft_pipapo_elem *pipapo_get(const struct net *net,
> {
> struct nft_pipapo_elem *ret = ERR_PTR(-ENOENT);
> struct nft_pipapo *priv = nft_set_priv(set);
> - struct nft_pipapo_match *m = priv->clone;
> + const struct nft_pipapo_match *m = priv->clone;
This should go one line below now and have a separate initialiser
(reverse Christmas tree).
> unsigned long *res_map, *fill_map = NULL;
> - struct nft_pipapo_field *f;
> + const struct nft_pipapo_field *f;
> int i;
>
> res_map = kmalloc_array(m->bsize_max, sizeof(*res_map), GFP_ATOMIC);
> @@ -1599,7 +1599,7 @@ static void pipapo_gc(struct nft_set *set, struct nft_pipapo_match *m)
>
> while ((rules_f0 = pipapo_rules_same_key(m->f, first_rule))) {
> union nft_pipapo_map_bucket rulemap[NFT_PIPAPO_MAX_FIELDS];
> - struct nft_pipapo_field *f;
> + const struct nft_pipapo_field *f;
> int i, start, rules_fx;
>
> start = first_rule;
> @@ -2041,8 +2041,8 @@ static void nft_pipapo_walk(const struct nft_ctx *ctx, struct nft_set *set,
> {
> struct nft_pipapo *priv = nft_set_priv(set);
> struct net *net = read_pnet(&set->net);
> - struct nft_pipapo_match *m;
> - struct nft_pipapo_field *f;
> + const struct nft_pipapo_match *m;
> + const struct nft_pipapo_field *f;
> int i, r;
>
> rcu_read_lock();
> diff --git a/net/netfilter/nft_set_pipapo.h b/net/netfilter/nft_set_pipapo.h
> index f59a0cd81105..90d22d691afc 100644
> --- a/net/netfilter/nft_set_pipapo.h
> +++ b/net/netfilter/nft_set_pipapo.h
> @@ -187,7 +187,7 @@ struct nft_pipapo_elem {
> };
>
> int pipapo_refill(unsigned long *map, int len, int rules, unsigned long *dst,
> - union nft_pipapo_map_bucket *mt, bool match_only);
> + const union nft_pipapo_map_bucket *mt, bool match_only);
>
> /**
> * pipapo_and_field_buckets_4bit() - Intersect 4-bit buckets
> @@ -195,7 +195,7 @@ int pipapo_refill(unsigned long *map, int len, int rules, unsigned long *dst,
> * @dst: Area to store result
> * @data: Input data selecting table buckets
> */
> -static inline void pipapo_and_field_buckets_4bit(struct nft_pipapo_field *f,
> +static inline void pipapo_and_field_buckets_4bit(const struct nft_pipapo_field *f,
> unsigned long *dst,
> const u8 *data)
> {
> @@ -223,7 +223,7 @@ static inline void pipapo_and_field_buckets_4bit(struct nft_pipapo_field *f,
> * @dst: Area to store result
> * @data: Input data selecting table buckets
> */
> -static inline void pipapo_and_field_buckets_8bit(struct nft_pipapo_field *f,
> +static inline void pipapo_and_field_buckets_8bit(const struct nft_pipapo_field *f,
> unsigned long *dst,
> const u8 *data)
> {
> diff --git a/net/netfilter/nft_set_pipapo_avx2.c b/net/netfilter/nft_set_pipapo_avx2.c
> index 208e9d577347..d7bea311165f 100644
> --- a/net/netfilter/nft_set_pipapo_avx2.c
> +++ b/net/netfilter/nft_set_pipapo_avx2.c
> @@ -212,7 +212,7 @@ static int nft_pipapo_avx2_refill(int offset, unsigned long *map,
> * word index to be checked next (i.e. first filled word).
> */
> static int nft_pipapo_avx2_lookup_4b_2(unsigned long *map, unsigned long *fill,
> - struct nft_pipapo_field *f, int offset,
> + const struct nft_pipapo_field *f, int offset,
This and all the changed declarations below exceed 80 columns but,
unlike the ones above, we could simply turn them into:
static int nft_pipapo_avx2_lookup_4b_2(unsigned long *map, unsigned long *fill,
const struct nft_pipapo_field *f,
int offset, const u8 *pkt,
bool first, bool last)
and so on.
> const u8 *pkt, bool first, bool last)
> {
> int i, ret = -1, m256_size = f->bsize / NFT_PIPAPO_LONGS_PER_M256, b;
> @@ -274,7 +274,7 @@ static int nft_pipapo_avx2_lookup_4b_2(unsigned long *map, unsigned long *fill,
> * word index to be checked next (i.e. first filled word).
> */
> static int nft_pipapo_avx2_lookup_4b_4(unsigned long *map, unsigned long *fill,
> - struct nft_pipapo_field *f, int offset,
> + const struct nft_pipapo_field *f, int offset,
> const u8 *pkt, bool first, bool last)
> {
> int i, ret = -1, m256_size = f->bsize / NFT_PIPAPO_LONGS_PER_M256, b;
> @@ -350,7 +350,7 @@ static int nft_pipapo_avx2_lookup_4b_4(unsigned long *map, unsigned long *fill,
> * word index to be checked next (i.e. first filled word).
> */
> static int nft_pipapo_avx2_lookup_4b_8(unsigned long *map, unsigned long *fill,
> - struct nft_pipapo_field *f, int offset,
> + const struct nft_pipapo_field *f, int offset,
> const u8 *pkt, bool first, bool last)
> {
> u8 pg[8] = { pkt[0] >> 4, pkt[0] & 0xf, pkt[1] >> 4, pkt[1] & 0xf,
> @@ -445,7 +445,7 @@ static int nft_pipapo_avx2_lookup_4b_8(unsigned long *map, unsigned long *fill,
> * word index to be checked next (i.e. first filled word).
> */
> static int nft_pipapo_avx2_lookup_4b_12(unsigned long *map, unsigned long *fill,
> - struct nft_pipapo_field *f, int offset,
> + const struct nft_pipapo_field *f, int offset,
> const u8 *pkt, bool first, bool last)
> {
> u8 pg[12] = { pkt[0] >> 4, pkt[0] & 0xf, pkt[1] >> 4, pkt[1] & 0xf,
> @@ -534,7 +534,7 @@ static int nft_pipapo_avx2_lookup_4b_12(unsigned long *map, unsigned long *fill,
> * word index to be checked next (i.e. first filled word).
> */
> static int nft_pipapo_avx2_lookup_4b_32(unsigned long *map, unsigned long *fill,
> - struct nft_pipapo_field *f, int offset,
> + const struct nft_pipapo_field *f, int offset,
> const u8 *pkt, bool first, bool last)
> {
> u8 pg[32] = { pkt[0] >> 4, pkt[0] & 0xf, pkt[1] >> 4, pkt[1] & 0xf,
> @@ -669,7 +669,7 @@ static int nft_pipapo_avx2_lookup_4b_32(unsigned long *map, unsigned long *fill,
> * word index to be checked next (i.e. first filled word).
> */
> static int nft_pipapo_avx2_lookup_8b_1(unsigned long *map, unsigned long *fill,
> - struct nft_pipapo_field *f, int offset,
> + const struct nft_pipapo_field *f, int offset,
> const u8 *pkt, bool first, bool last)
> {
> int i, ret = -1, m256_size = f->bsize / NFT_PIPAPO_LONGS_PER_M256, b;
> @@ -726,7 +726,7 @@ static int nft_pipapo_avx2_lookup_8b_1(unsigned long *map, unsigned long *fill,
> * word index to be checked next (i.e. first filled word).
> */
> static int nft_pipapo_avx2_lookup_8b_2(unsigned long *map, unsigned long *fill,
> - struct nft_pipapo_field *f, int offset,
> + const struct nft_pipapo_field *f, int offset,
> const u8 *pkt, bool first, bool last)
> {
> int i, ret = -1, m256_size = f->bsize / NFT_PIPAPO_LONGS_PER_M256, b;
> @@ -790,7 +790,7 @@ static int nft_pipapo_avx2_lookup_8b_2(unsigned long *map, unsigned long *fill,
> * word index to be checked next (i.e. first filled word).
> */
> static int nft_pipapo_avx2_lookup_8b_4(unsigned long *map, unsigned long *fill,
> - struct nft_pipapo_field *f, int offset,
> + const struct nft_pipapo_field *f, int offset,
> const u8 *pkt, bool first, bool last)
> {
> int i, ret = -1, m256_size = f->bsize / NFT_PIPAPO_LONGS_PER_M256, b;
> @@ -865,7 +865,7 @@ static int nft_pipapo_avx2_lookup_8b_4(unsigned long *map, unsigned long *fill,
> * word index to be checked next (i.e. first filled word).
> */
> static int nft_pipapo_avx2_lookup_8b_6(unsigned long *map, unsigned long *fill,
> - struct nft_pipapo_field *f, int offset,
> + const struct nft_pipapo_field *f, int offset,
> const u8 *pkt, bool first, bool last)
> {
> int i, ret = -1, m256_size = f->bsize / NFT_PIPAPO_LONGS_PER_M256, b;
> @@ -950,7 +950,7 @@ static int nft_pipapo_avx2_lookup_8b_6(unsigned long *map, unsigned long *fill,
> * word index to be checked next (i.e. first filled word).
> */
> static int nft_pipapo_avx2_lookup_8b_16(unsigned long *map, unsigned long *fill,
> - struct nft_pipapo_field *f, int offset,
> + const struct nft_pipapo_field *f, int offset,
> const u8 *pkt, bool first, bool last)
> {
> int i, ret = -1, m256_size = f->bsize / NFT_PIPAPO_LONGS_PER_M256, b;
> @@ -1042,7 +1042,7 @@ static int nft_pipapo_avx2_lookup_8b_16(unsigned long *map, unsigned long *fill,
> * word index to be checked next (i.e. first filled word).
> */
> static int nft_pipapo_avx2_lookup_slow(unsigned long *map, unsigned long *fill,
> - struct nft_pipapo_field *f, int offset,
> + const struct nft_pipapo_field *f, int offset,
> const u8 *pkt, bool first, bool last)
> {
> unsigned long bsize = f->bsize;
> @@ -1119,9 +1119,9 @@ bool nft_pipapo_avx2_lookup(const struct net *net, const struct nft_set *set,
> struct nft_pipapo *priv = nft_set_priv(set);
> struct nft_pipapo_scratch *scratch;
> u8 genmask = nft_genmask_cur(net);
> + const struct nft_pipapo_match *m;
> + const struct nft_pipapo_field *f;
> const u8 *rp = (const u8 *)key;
> - struct nft_pipapo_match *m;
> - struct nft_pipapo_field *f;
> unsigned long *res, *fill;
> bool map_index;
> int i, ret = 0;
--
Stefano
next prev parent reply other threads:[~2024-02-13 7:20 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-02-12 10:01 [PATCH nf-next 0/4] netfilter: nft_set_pipapo: speed up bulk element insertions Florian Westphal
2024-02-12 10:01 ` [PATCH nf-next 1/4] netfilter: nft_set_pipapo: constify lookup fn args where possible Florian Westphal
2024-02-13 7:19 ` Stefano Brivio [this message]
2024-02-12 10:01 ` [PATCH nf-next 2/4] netfilter: nft_set_pipapo: do not rely on ZERO_SIZE_PTR Florian Westphal
2024-02-13 7:20 ` Stefano Brivio
2024-02-13 11:09 ` Florian Westphal
2024-02-12 10:01 ` [PATCH nf-next 3/4] netfilter: nft_set_pipapo: shrink data structures Florian Westphal
2024-02-13 13:17 ` Stefano Brivio
2024-02-12 10:01 ` [PATCH nf-next 4/4] netfilter: nft_set_pipapo: speed up bulk element insertions Florian Westphal
2024-02-13 13:17 ` Stefano Brivio
2024-02-13 13:29 ` Florian Westphal
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=20240213081929.218433a2@elisabeth \
--to=sbrivio@redhat.com \
--cc=fw@strlen.de \
--cc=netfilter-devel@vger.kernel.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).