[PATCH nf-next v2 0/4] ipvs: Cleanups for v6.4

[PATCH nf-next v2 0/4] ipvs: Cleanups for v6.4

[Simon Horman]

Hi Julian,

this series aims to clean up IPVS in several ways without
implementing any functional changes, aside from removing
some debugging output.

Patch 1/4: Update width of source for ip_vs_sync_conn_options
           The operation is safe, use an annotation to describe it properly.

Patch 2/4: Consistently use array_size() in ip_vs_conn_init()
           It seems better to use helpers consistently.

Patch 3/4: Remove {Enter,Leave}Function
           These seem to be well past their use-by date.

Patch 4/4: Correct spelling in comments
	   I can't spell. But codespell helps me these days.

All changes: compile tested only!

---
Changes in v2:
- Patch 1/4: Correct spelling of 'conn' in subject

---
Simon Horman (4):
      ipvs: Update width of source for ip_vs_sync_conn_options
      ipvs: Consistently use array_size() in ip_vs_conn_init()
      ipvs: Remove {Enter,Leave}Function
      ipvs: Correct spelling in comments

 include/net/ip_vs.h             | 32 +++++----------------
 net/netfilter/ipvs/ip_vs_conn.c | 12 ++++----
 net/netfilter/ipvs/ip_vs_core.c |  8 ------
 net/netfilter/ipvs/ip_vs_ctl.c  | 26 +----------------
 net/netfilter/ipvs/ip_vs_sync.c |  7 +----
 net/netfilter/ipvs/ip_vs_xmit.c | 62 ++++++-----------------------------------
 6 files changed, 23 insertions(+), 124 deletions(-)

base-commit: 9bc11460bea751b5c805ac793de35a55629adb9b

[PATCH nf-next v2 1/4] ipvs: Update width of source for ip_vs_sync_conn_options

[Simon Horman]

In ip_vs_sync_conn_v0() copy is made to struct ip_vs_sync_conn_options.
That structure looks like this:

struct ip_vs_sync_conn_options {
        struct ip_vs_seq        in_seq;
        struct ip_vs_seq        out_seq;
};

The source of the copy is the in_seq field of struct ip_vs_conn.  Whose
type is struct ip_vs_seq. Thus we can see that the source - is not as
wide as the amount of data copied, which is the width of struct
ip_vs_sync_conn_option.

The copy is safe because the next field in is another struct ip_vs_seq.
Make use of struct_group() to annotate this.

Flagged by gcc-13 as:

 In file included from ./include/linux/string.h:254,
                  from ./include/linux/bitmap.h:11,
                  from ./include/linux/cpumask.h:12,
                  from ./arch/x86/include/asm/paravirt.h:17,
                  from ./arch/x86/include/asm/cpuid.h:62,
                  from ./arch/x86/include/asm/processor.h:19,
                  from ./arch/x86/include/asm/timex.h:5,
                  from ./include/linux/timex.h:67,
                  from ./include/linux/time32.h:13,
                  from ./include/linux/time.h:60,
                  from ./include/linux/stat.h:19,
                  from ./include/linux/module.h:13,
                  from net/netfilter/ipvs/ip_vs_sync.c:38:
 In function 'fortify_memcpy_chk',
     inlined from 'ip_vs_sync_conn_v0' at net/netfilter/ipvs/ip_vs_sync.c:606:3:
 ./include/linux/fortify-string.h:529:25: error: call to '__read_overflow2_field' declared with attribute warning: detected read beyond size of field (2nd parameter); maybe use struct_group()? [-Werror=attribute-warning]
   529 |                         __read_overflow2_field(q_size_field, size);
       |

Compile tested only.

Signed-off-by: Simon Horman <horms@kernel.org>
---
v2
* Correct spelling of 'conn' in subject
---
 include/net/ip_vs.h             | 6 ++++--
 net/netfilter/ipvs/ip_vs_sync.c | 2 +-
 2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/include/net/ip_vs.h b/include/net/ip_vs.h
index 6d71a5ff52df..e20f1f92066d 100644
--- a/include/net/ip_vs.h
+++ b/include/net/ip_vs.h
@@ -630,8 +630,10 @@ struct ip_vs_conn {
 	 */
 	struct ip_vs_app        *app;           /* bound ip_vs_app object */
 	void                    *app_data;      /* Application private data */
-	struct ip_vs_seq        in_seq;         /* incoming seq. struct */
-	struct ip_vs_seq        out_seq;        /* outgoing seq. struct */
+	struct_group(sync_conn_opt,
+		struct ip_vs_seq  in_seq;       /* incoming seq. struct */
+		struct ip_vs_seq  out_seq;      /* outgoing seq. struct */
+	);
 
 	const struct ip_vs_pe	*pe;
 	char			*pe_data;
diff --git a/net/netfilter/ipvs/ip_vs_sync.c b/net/netfilter/ipvs/ip_vs_sync.c
index 4963fec815da..d4fe7bb4f853 100644
--- a/net/netfilter/ipvs/ip_vs_sync.c
+++ b/net/netfilter/ipvs/ip_vs_sync.c
@@ -603,7 +603,7 @@ static void ip_vs_sync_conn_v0(struct netns_ipvs *ipvs, struct ip_vs_conn *cp,
 	if (cp->flags & IP_VS_CONN_F_SEQ_MASK) {
 		struct ip_vs_sync_conn_options *opt =
 			(struct ip_vs_sync_conn_options *)&s[1];
-		memcpy(opt, &cp->in_seq, sizeof(*opt));
+		memcpy(opt, &cp->sync_conn_opt, sizeof(*opt));
 	}
 
 	m->nr_conns++;

-- 
2.30.2

Re: [PATCH nf-next v2 1/4] ipvs: Update width of source for ip_vs_sync_conn_options

[Horatiu Vultur]

The 04/11/2023 09:10, Simon Horman wrote:
> 
> In ip_vs_sync_conn_v0() copy is made to struct ip_vs_sync_conn_options.
> That structure looks like this:
> 
> struct ip_vs_sync_conn_options {
>         struct ip_vs_seq        in_seq;
>         struct ip_vs_seq        out_seq;
> };
> 
> The source of the copy is the in_seq field of struct ip_vs_conn.  Whose
> type is struct ip_vs_seq. Thus we can see that the source - is not as
> wide as the amount of data copied, which is the width of struct
> ip_vs_sync_conn_option.
> 
> The copy is safe because the next field in is another struct ip_vs_seq.
> Make use of struct_group() to annotate this.
> 
> Flagged by gcc-13 as:
> 
>  In file included from ./include/linux/string.h:254,
>                   from ./include/linux/bitmap.h:11,
>                   from ./include/linux/cpumask.h:12,
>                   from ./arch/x86/include/asm/paravirt.h:17,
>                   from ./arch/x86/include/asm/cpuid.h:62,
>                   from ./arch/x86/include/asm/processor.h:19,
>                   from ./arch/x86/include/asm/timex.h:5,
>                   from ./include/linux/timex.h:67,
>                   from ./include/linux/time32.h:13,
>                   from ./include/linux/time.h:60,
>                   from ./include/linux/stat.h:19,
>                   from ./include/linux/module.h:13,
>                   from net/netfilter/ipvs/ip_vs_sync.c:38:
>  In function 'fortify_memcpy_chk',
>      inlined from 'ip_vs_sync_conn_v0' at net/netfilter/ipvs/ip_vs_sync.c:606:3:
>  ./include/linux/fortify-string.h:529:25: error: call to '__read_overflow2_field' declared with attribute warning: detected read beyond size of field (2nd parameter); maybe use struct_group()? [-Werror=attribute-warning]
>    529 |                         __read_overflow2_field(q_size_field, size);
>        |
> 
> Compile tested only.

Reviewed-by: Horatiu Vultur <horatiu.vultur@microchip.com>

> 
> Signed-off-by: Simon Horman <horms@kernel.org>
> ---
> v2
> * Correct spelling of 'conn' in subject
> ---
>  include/net/ip_vs.h             | 6 ++++--
>  net/netfilter/ipvs/ip_vs_sync.c | 2 +-
>  2 files changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/include/net/ip_vs.h b/include/net/ip_vs.h
> index 6d71a5ff52df..e20f1f92066d 100644
> --- a/include/net/ip_vs.h
> +++ b/include/net/ip_vs.h
> @@ -630,8 +630,10 @@ struct ip_vs_conn {
>          */
>         struct ip_vs_app        *app;           /* bound ip_vs_app object */
>         void                    *app_data;      /* Application private data */
> -       struct ip_vs_seq        in_seq;         /* incoming seq. struct */
> -       struct ip_vs_seq        out_seq;        /* outgoing seq. struct */
> +       struct_group(sync_conn_opt,
> +               struct ip_vs_seq  in_seq;       /* incoming seq. struct */
> +               struct ip_vs_seq  out_seq;      /* outgoing seq. struct */
> +       );
> 
>         const struct ip_vs_pe   *pe;
>         char                    *pe_data;
> diff --git a/net/netfilter/ipvs/ip_vs_sync.c b/net/netfilter/ipvs/ip_vs_sync.c
> index 4963fec815da..d4fe7bb4f853 100644
> --- a/net/netfilter/ipvs/ip_vs_sync.c
> +++ b/net/netfilter/ipvs/ip_vs_sync.c
> @@ -603,7 +603,7 @@ static void ip_vs_sync_conn_v0(struct netns_ipvs *ipvs, struct ip_vs_conn *cp,
>         if (cp->flags & IP_VS_CONN_F_SEQ_MASK) {
>                 struct ip_vs_sync_conn_options *opt =
>                         (struct ip_vs_sync_conn_options *)&s[1];
> -               memcpy(opt, &cp->in_seq, sizeof(*opt));
> +               memcpy(opt, &cp->sync_conn_opt, sizeof(*opt));
>         }
> 
>         m->nr_conns++;
> 
> --
> 2.30.2
> 

-- 
/Horatiu

[PATCH nf-next v2 2/4] ipvs: Consistently use array_size() in ip_vs_conn_init()

[Simon Horman]

Consistently use array_size() to calculate the size of ip_vs_conn_tab
in bytes.

Flagged by Coccinelle:
 WARNING: array_size is already used (line 1498) to compute the same size

No functional change intended.
Compile tested only.

Signed-off-by: Simon Horman <horms@kernel.org>
---
v2
* Retain division by 1024, which was lost in v1
---
 net/netfilter/ipvs/ip_vs_conn.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/net/netfilter/ipvs/ip_vs_conn.c b/net/netfilter/ipvs/ip_vs_conn.c
index 13534e02346c..84d273a84dc8 100644
--- a/net/netfilter/ipvs/ip_vs_conn.c
+++ b/net/netfilter/ipvs/ip_vs_conn.c
@@ -1481,6 +1481,7 @@ void __net_exit ip_vs_conn_net_cleanup(struct netns_ipvs *ipvs)
 
 int __init ip_vs_conn_init(void)
 {
+	size_t tab_array_size;
 	int idx;
 
 	/* Compute size and mask */
@@ -1494,8 +1495,9 @@ int __init ip_vs_conn_init(void)
 	/*
 	 * Allocate the connection hash table and initialize its list heads
 	 */
-	ip_vs_conn_tab = vmalloc(array_size(ip_vs_conn_tab_size,
-					    sizeof(*ip_vs_conn_tab)));
+	tab_array_size = array_size(ip_vs_conn_tab_size,
+				    sizeof(*ip_vs_conn_tab));
+	ip_vs_conn_tab = vmalloc(tab_array_size);
 	if (!ip_vs_conn_tab)
 		return -ENOMEM;
 
@@ -1508,10 +1510,8 @@ int __init ip_vs_conn_init(void)
 		return -ENOMEM;
 	}
 
-	pr_info("Connection hash table configured "
-		"(size=%d, memory=%ldKbytes)\n",
-		ip_vs_conn_tab_size,
-		(long)(ip_vs_conn_tab_size*sizeof(*ip_vs_conn_tab))/1024);
+	pr_info("Connection hash table configured (size=%d, memory=%zdKbytes)\n",
+		ip_vs_conn_tab_size / 1024, tab_array_size);
 	IP_VS_DBG(0, "Each connection entry needs %zd bytes at least\n",
 		  sizeof(struct ip_vs_conn));
 

-- 
2.30.2

Re: [PATCH nf-next v2 2/4] ipvs: Consistently use array_size() in ip_vs_conn_init()

[Horatiu Vultur]

The 04/11/2023 09:10, Simon Horman wrote:
> 
> Consistently use array_size() to calculate the size of ip_vs_conn_tab
> in bytes.
> 
> Flagged by Coccinelle:
>  WARNING: array_size is already used (line 1498) to compute the same size
> 
> No functional change intended.
> Compile tested only.

Reviewed-by: Horatiu Vultur <horatiu.vultur@microchip.com>

> 
> Signed-off-by: Simon Horman <horms@kernel.org>
> ---
> v2
> * Retain division by 1024, which was lost in v1
> ---
>  net/netfilter/ipvs/ip_vs_conn.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/net/netfilter/ipvs/ip_vs_conn.c b/net/netfilter/ipvs/ip_vs_conn.c
> index 13534e02346c..84d273a84dc8 100644
> --- a/net/netfilter/ipvs/ip_vs_conn.c
> +++ b/net/netfilter/ipvs/ip_vs_conn.c
> @@ -1481,6 +1481,7 @@ void __net_exit ip_vs_conn_net_cleanup(struct netns_ipvs *ipvs)
> 
>  int __init ip_vs_conn_init(void)
>  {
> +       size_t tab_array_size;
>         int idx;
> 
>         /* Compute size and mask */
> @@ -1494,8 +1495,9 @@ int __init ip_vs_conn_init(void)
>         /*
>          * Allocate the connection hash table and initialize its list heads
>          */
> -       ip_vs_conn_tab = vmalloc(array_size(ip_vs_conn_tab_size,
> -                                           sizeof(*ip_vs_conn_tab)));
> +       tab_array_size = array_size(ip_vs_conn_tab_size,
> +                                   sizeof(*ip_vs_conn_tab));
> +       ip_vs_conn_tab = vmalloc(tab_array_size);
>         if (!ip_vs_conn_tab)
>                 return -ENOMEM;
> 
> @@ -1508,10 +1510,8 @@ int __init ip_vs_conn_init(void)
>                 return -ENOMEM;
>         }
> 
> -       pr_info("Connection hash table configured "
> -               "(size=%d, memory=%ldKbytes)\n",
> -               ip_vs_conn_tab_size,
> -               (long)(ip_vs_conn_tab_size*sizeof(*ip_vs_conn_tab))/1024);
> +       pr_info("Connection hash table configured (size=%d, memory=%zdKbytes)\n",
> +               ip_vs_conn_tab_size / 1024, tab_array_size);
>         IP_VS_DBG(0, "Each connection entry needs %zd bytes at least\n",
>                   sizeof(struct ip_vs_conn));
> 
> 
> --
> 2.30.2
> 

-- 
/Horatiu

[PATCH nf-next v2 3/4] ipvs: Remove {Enter,Leave}Function

[Simon Horman]

Remove EnterFunction and LeaveFunction.

These debugging macros seem well past their use-by date.  And seem to
have little value these days. Removing them allows some trivial cleanup
of some exit paths for some functions. These are also included in this
patch. There is likely scope for further cleanup of both debugging and
unwind paths. But let's leave that for another day.

Only intended to change debug output, and only when CONFIG_IP_VS_DEBUG
is enabled. Compile tested only.

Signed-off-by: Simon Horman <horms@kernel.org>
---
 include/net/ip_vs.h             | 20 -------------
 net/netfilter/ipvs/ip_vs_core.c |  8 ------
 net/netfilter/ipvs/ip_vs_ctl.c  | 26 +----------------
 net/netfilter/ipvs/ip_vs_sync.c |  5 ----
 net/netfilter/ipvs/ip_vs_xmit.c | 62 ++++++-----------------------------------
 5 files changed, 9 insertions(+), 112 deletions(-)

diff --git a/include/net/ip_vs.h b/include/net/ip_vs.h
index e20f1f92066d..a3adc246ee31 100644
--- a/include/net/ip_vs.h
+++ b/include/net/ip_vs.h
@@ -265,26 +265,6 @@ static inline const char *ip_vs_dbg_addr(int af, char *buf, size_t buf_len,
 			pr_err(msg, ##__VA_ARGS__);			\
 	} while (0)
 
-#ifdef CONFIG_IP_VS_DEBUG
-#define EnterFunction(level)						\
-	do {								\
-		if (level <= ip_vs_get_debug_level())			\
-			printk(KERN_DEBUG				\
-			       pr_fmt("Enter: %s, %s line %i\n"),	\
-			       __func__, __FILE__, __LINE__);		\
-	} while (0)
-#define LeaveFunction(level)						\
-	do {								\
-		if (level <= ip_vs_get_debug_level())			\
-			printk(KERN_DEBUG				\
-			       pr_fmt("Leave: %s, %s line %i\n"),	\
-			       __func__, __FILE__, __LINE__);		\
-	} while (0)
-#else
-#define EnterFunction(level)   do {} while (0)
-#define LeaveFunction(level)   do {} while (0)
-#endif
-
 /* The port number of FTP service (in network order). */
 #define FTPPORT  cpu_to_be16(21)
 #define FTPDATA  cpu_to_be16(20)
diff --git a/net/netfilter/ipvs/ip_vs_core.c b/net/netfilter/ipvs/ip_vs_core.c
index 2fcc26507d69..cb83ca506c5c 100644
--- a/net/netfilter/ipvs/ip_vs_core.c
+++ b/net/netfilter/ipvs/ip_vs_core.c
@@ -1140,7 +1140,6 @@ struct ip_vs_conn *ip_vs_new_conn_out(struct ip_vs_service *svc,
 	__be16 vport;
 	unsigned int flags;
 
-	EnterFunction(12);
 	vaddr = &svc->addr;
 	vport = svc->port;
 	daddr = &iph->saddr;
@@ -1208,7 +1207,6 @@ struct ip_vs_conn *ip_vs_new_conn_out(struct ip_vs_service *svc,
 		      IP_VS_DBG_ADDR(cp->af, &cp->vaddr), ntohs(cp->vport),
 		      IP_VS_DBG_ADDR(cp->af, &cp->daddr), ntohs(cp->dport),
 		      cp->flags, refcount_read(&cp->refcnt));
-	LeaveFunction(12);
 	return cp;
 }
 
@@ -1316,13 +1314,11 @@ handle_response(int af, struct sk_buff *skb, struct ip_vs_proto_data *pd,
 		ip_vs_update_conntrack(skb, cp, 0);
 	ip_vs_conn_put(cp);
 
-	LeaveFunction(11);
 	return NF_ACCEPT;
 
 drop:
 	ip_vs_conn_put(cp);
 	kfree_skb(skb);
-	LeaveFunction(11);
 	return NF_STOLEN;
 }
 
@@ -1341,8 +1337,6 @@ ip_vs_out_hook(void *priv, struct sk_buff *skb, const struct nf_hook_state *stat
 	int af = state->pf;
 	struct sock *sk;
 
-	EnterFunction(11);
-
 	/* Already marked as IPVS request or reply? */
 	if (skb->ipvs_property)
 		return NF_ACCEPT;
@@ -2365,7 +2359,6 @@ static void __net_exit __ip_vs_dev_cleanup_batch(struct list_head *net_list)
 	struct netns_ipvs *ipvs;
 	struct net *net;
 
-	EnterFunction(2);
 	list_for_each_entry(net, net_list, exit_list) {
 		ipvs = net_ipvs(net);
 		ip_vs_unregister_hooks(ipvs, AF_INET);
@@ -2374,7 +2367,6 @@ static void __net_exit __ip_vs_dev_cleanup_batch(struct list_head *net_list)
 		smp_wmb();
 		ip_vs_sync_net_cleanup(ipvs);
 	}
-	LeaveFunction(2);
 }
 
 static struct pernet_operations ipvs_core_ops = {
diff --git a/net/netfilter/ipvs/ip_vs_ctl.c b/net/netfilter/ipvs/ip_vs_ctl.c
index 2a5ed71c82c3..62606fb44d02 100644
--- a/net/netfilter/ipvs/ip_vs_ctl.c
+++ b/net/netfilter/ipvs/ip_vs_ctl.c
@@ -1061,8 +1061,6 @@ ip_vs_new_dest(struct ip_vs_service *svc, struct ip_vs_dest_user_kern *udest)
 	unsigned int atype;
 	int ret;
 
-	EnterFunction(2);
-
 #ifdef CONFIG_IP_VS_IPV6
 	if (udest->af == AF_INET6) {
 		atype = ipv6_addr_type(&udest->addr.in6);
@@ -1111,7 +1109,6 @@ ip_vs_new_dest(struct ip_vs_service *svc, struct ip_vs_dest_user_kern *udest)
 	spin_lock_init(&dest->dst_lock);
 	__ip_vs_update_dest(svc, dest, udest, 1);
 
-	LeaveFunction(2);
 	return 0;
 
 err_stats:
@@ -1134,8 +1131,6 @@ ip_vs_add_dest(struct ip_vs_service *svc, struct ip_vs_dest_user_kern *udest)
 	__be16 dport = udest->port;
 	int ret;
 
-	EnterFunction(2);
-
 	if (udest->weight < 0) {
 		pr_err("%s(): server weight less than zero\n", __func__);
 		return -ERANGE;
@@ -1183,7 +1178,7 @@ ip_vs_add_dest(struct ip_vs_service *svc, struct ip_vs_dest_user_kern *udest)
 
 		ret = ip_vs_start_estimator(svc->ipvs, &dest->stats);
 		if (ret < 0)
-			goto err;
+			return ret;
 		__ip_vs_update_dest(svc, dest, udest, 1);
 	} else {
 		/*
@@ -1192,9 +1187,6 @@ ip_vs_add_dest(struct ip_vs_service *svc, struct ip_vs_dest_user_kern *udest)
 		ret = ip_vs_new_dest(svc, udest);
 	}
 
-err:
-	LeaveFunction(2);
-
 	return ret;
 }
 
@@ -1209,8 +1201,6 @@ ip_vs_edit_dest(struct ip_vs_service *svc, struct ip_vs_dest_user_kern *udest)
 	union nf_inet_addr daddr;
 	__be16 dport = udest->port;
 
-	EnterFunction(2);
-
 	if (udest->weight < 0) {
 		pr_err("%s(): server weight less than zero\n", __func__);
 		return -ERANGE;
@@ -1242,7 +1232,6 @@ ip_vs_edit_dest(struct ip_vs_service *svc, struct ip_vs_dest_user_kern *udest)
 	}
 
 	__ip_vs_update_dest(svc, dest, udest, 0);
-	LeaveFunction(2);
 
 	return 0;
 }
@@ -1317,8 +1306,6 @@ ip_vs_del_dest(struct ip_vs_service *svc, struct ip_vs_dest_user_kern *udest)
 	struct ip_vs_dest *dest;
 	__be16 dport = udest->port;
 
-	EnterFunction(2);
-
 	/* We use function that requires RCU lock */
 	rcu_read_lock();
 	dest = ip_vs_lookup_dest(svc, udest->af, &udest->addr, dport);
@@ -1339,8 +1326,6 @@ ip_vs_del_dest(struct ip_vs_service *svc, struct ip_vs_dest_user_kern *udest)
 	 */
 	__ip_vs_del_dest(svc->ipvs, dest, false);
 
-	LeaveFunction(2);
-
 	return 0;
 }
 
@@ -1746,7 +1731,6 @@ void ip_vs_service_nets_cleanup(struct list_head *net_list)
 	struct netns_ipvs *ipvs;
 	struct net *net;
 
-	EnterFunction(2);
 	/* Check for "full" addressed entries */
 	mutex_lock(&__ip_vs_mutex);
 	list_for_each_entry(net, net_list, exit_list) {
@@ -1754,7 +1738,6 @@ void ip_vs_service_nets_cleanup(struct list_head *net_list)
 		ip_vs_flush(ipvs, true);
 	}
 	mutex_unlock(&__ip_vs_mutex);
-	LeaveFunction(2);
 }
 
 /* Put all references for device (dst_cache) */
@@ -1792,7 +1775,6 @@ static int ip_vs_dst_event(struct notifier_block *this, unsigned long event,
 	if (event != NETDEV_DOWN || !ipvs)
 		return NOTIFY_DONE;
 	IP_VS_DBG(3, "%s() dev=%s\n", __func__, dev->name);
-	EnterFunction(2);
 	mutex_lock(&__ip_vs_mutex);
 	for (idx = 0; idx < IP_VS_SVC_TAB_SIZE; idx++) {
 		hlist_for_each_entry(svc, &ip_vs_svc_table[idx], s_list) {
@@ -1821,7 +1803,6 @@ static int ip_vs_dst_event(struct notifier_block *this, unsigned long event,
 	}
 	spin_unlock_bh(&ipvs->dest_trash_lock);
 	mutex_unlock(&__ip_vs_mutex);
-	LeaveFunction(2);
 	return NOTIFY_DONE;
 }
 
@@ -4537,8 +4518,6 @@ int __init ip_vs_control_init(void)
 	int idx;
 	int ret;
 
-	EnterFunction(2);
-
 	/* Initialize svc_table, ip_vs_svc_fwm_table */
 	for (idx = 0; idx < IP_VS_SVC_TAB_SIZE; idx++) {
 		INIT_HLIST_HEAD(&ip_vs_svc_table[idx]);
@@ -4551,15 +4530,12 @@ int __init ip_vs_control_init(void)
 	if (ret < 0)
 		return ret;
 
-	LeaveFunction(2);
 	return 0;
 }
 
 
 void ip_vs_control_cleanup(void)
 {
-	EnterFunction(2);
 	unregister_netdevice_notifier(&ip_vs_dst_notifier);
 	/* relying on common rcu_barrier() in ip_vs_cleanup() */
-	LeaveFunction(2);
 }
diff --git a/net/netfilter/ipvs/ip_vs_sync.c b/net/netfilter/ipvs/ip_vs_sync.c
index d4fe7bb4f853..264f2f87a437 100644
--- a/net/netfilter/ipvs/ip_vs_sync.c
+++ b/net/netfilter/ipvs/ip_vs_sync.c
@@ -1582,13 +1582,11 @@ ip_vs_send_async(struct socket *sock, const char *buffer, const size_t length)
 	struct kvec	iov;
 	int		len;
 
-	EnterFunction(7);
 	iov.iov_base     = (void *)buffer;
 	iov.iov_len      = length;
 
 	len = kernel_sendmsg(sock, &msg, &iov, 1, (size_t)(length));
 
-	LeaveFunction(7);
 	return len;
 }
 
@@ -1614,15 +1612,12 @@ ip_vs_receive(struct socket *sock, char *buffer, const size_t buflen)
 	struct kvec		iov = {buffer, buflen};
 	int			len;
 
-	EnterFunction(7);
-
 	/* Receive a packet */
 	iov_iter_kvec(&msg.msg_iter, ITER_DEST, &iov, 1, buflen);
 	len = sock_recvmsg(sock, &msg, MSG_DONTWAIT);
 	if (len < 0)
 		return len;
 
-	LeaveFunction(7);
 	return len;
 }
 
diff --git a/net/netfilter/ipvs/ip_vs_xmit.c b/net/netfilter/ipvs/ip_vs_xmit.c
index 99c349c0d968..feb1d7fcb09f 100644
--- a/net/netfilter/ipvs/ip_vs_xmit.c
+++ b/net/netfilter/ipvs/ip_vs_xmit.c
@@ -706,8 +706,6 @@ ip_vs_bypass_xmit(struct sk_buff *skb, struct ip_vs_conn *cp,
 {
 	struct iphdr  *iph = ip_hdr(skb);
 
-	EnterFunction(10);
-
 	if (__ip_vs_get_out_rt(cp->ipvs, cp->af, skb, NULL, iph->daddr,
 			       IP_VS_RT_MODE_NON_LOCAL, NULL, ipvsh) < 0)
 		goto tx_error;
@@ -719,12 +717,10 @@ ip_vs_bypass_xmit(struct sk_buff *skb, struct ip_vs_conn *cp,
 
 	ip_vs_send_or_cont(NFPROTO_IPV4, skb, cp, 0);
 
-	LeaveFunction(10);
 	return NF_STOLEN;
 
  tx_error:
 	kfree_skb(skb);
-	LeaveFunction(10);
 	return NF_STOLEN;
 }
 
@@ -735,8 +731,6 @@ ip_vs_bypass_xmit_v6(struct sk_buff *skb, struct ip_vs_conn *cp,
 {
 	struct ipv6hdr *iph = ipv6_hdr(skb);
 
-	EnterFunction(10);
-
 	if (__ip_vs_get_out_rt_v6(cp->ipvs, cp->af, skb, NULL,
 				  &iph->daddr, NULL,
 				  ipvsh, 0, IP_VS_RT_MODE_NON_LOCAL) < 0)
@@ -747,12 +741,10 @@ ip_vs_bypass_xmit_v6(struct sk_buff *skb, struct ip_vs_conn *cp,
 
 	ip_vs_send_or_cont(NFPROTO_IPV6, skb, cp, 0);
 
-	LeaveFunction(10);
 	return NF_STOLEN;
 
  tx_error:
 	kfree_skb(skb);
-	LeaveFunction(10);
 	return NF_STOLEN;
 }
 #endif
@@ -768,8 +760,6 @@ ip_vs_nat_xmit(struct sk_buff *skb, struct ip_vs_conn *cp,
 	struct rtable *rt;		/* Route to the other host */
 	int local, rc, was_input;
 
-	EnterFunction(10);
-
 	/* check if it is a connection of no-client-port */
 	if (unlikely(cp->flags & IP_VS_CONN_F_NO_CPORT)) {
 		__be16 _pt, *p;
@@ -839,12 +829,10 @@ ip_vs_nat_xmit(struct sk_buff *skb, struct ip_vs_conn *cp,
 
 	rc = ip_vs_nat_send_or_cont(NFPROTO_IPV4, skb, cp, local);
 
-	LeaveFunction(10);
 	return rc;
 
   tx_error:
 	kfree_skb(skb);
-	LeaveFunction(10);
 	return NF_STOLEN;
 }
 
@@ -856,8 +844,6 @@ ip_vs_nat_xmit_v6(struct sk_buff *skb, struct ip_vs_conn *cp,
 	struct rt6_info *rt;		/* Route to the other host */
 	int local, rc;
 
-	EnterFunction(10);
-
 	/* check if it is a connection of no-client-port */
 	if (unlikely(cp->flags & IP_VS_CONN_F_NO_CPORT && !ipvsh->fragoffs)) {
 		__be16 _pt, *p;
@@ -927,11 +913,9 @@ ip_vs_nat_xmit_v6(struct sk_buff *skb, struct ip_vs_conn *cp,
 
 	rc = ip_vs_nat_send_or_cont(NFPROTO_IPV6, skb, cp, local);
 
-	LeaveFunction(10);
 	return rc;
 
 tx_error:
-	LeaveFunction(10);
 	kfree_skb(skb);
 	return NF_STOLEN;
 }
@@ -1149,8 +1133,6 @@ ip_vs_tunnel_xmit(struct sk_buff *skb, struct ip_vs_conn *cp,
 	int tun_type, gso_type;
 	int tun_flags;
 
-	EnterFunction(10);
-
 	local = __ip_vs_get_out_rt(ipvs, cp->af, skb, cp->dest, cp->daddr.ip,
 				   IP_VS_RT_MODE_LOCAL |
 				   IP_VS_RT_MODE_NON_LOCAL |
@@ -1199,7 +1181,7 @@ ip_vs_tunnel_xmit(struct sk_buff *skb, struct ip_vs_conn *cp,
 					 &next_protocol, NULL, &dsfield,
 					 &ttl, dfp);
 	if (IS_ERR(skb))
-		goto tx_error;
+		return NF_STOLEN;
 
 	gso_type = __tun_gso_type_mask(AF_INET, cp->af);
 	if (tun_type == IP_VS_CONN_F_TUNNEL_TYPE_GUE) {
@@ -1267,14 +1249,10 @@ ip_vs_tunnel_xmit(struct sk_buff *skb, struct ip_vs_conn *cp,
 	else if (ret == NF_DROP)
 		kfree_skb(skb);
 
-	LeaveFunction(10);
-
 	return NF_STOLEN;
 
   tx_error:
-	if (!IS_ERR(skb))
-		kfree_skb(skb);
-	LeaveFunction(10);
+	kfree_skb(skb);
 	return NF_STOLEN;
 }
 
@@ -1298,8 +1276,6 @@ ip_vs_tunnel_xmit_v6(struct sk_buff *skb, struct ip_vs_conn *cp,
 	int tun_type, gso_type;
 	int tun_flags;
 
-	EnterFunction(10);
-
 	local = __ip_vs_get_out_rt_v6(ipvs, cp->af, skb, cp->dest,
 				      &cp->daddr.in6,
 				      &saddr, ipvsh, 1,
@@ -1347,7 +1323,7 @@ ip_vs_tunnel_xmit_v6(struct sk_buff *skb, struct ip_vs_conn *cp,
 					 &next_protocol, &payload_len,
 					 &dsfield, &ttl, NULL);
 	if (IS_ERR(skb))
-		goto tx_error;
+		return NF_STOLEN;
 
 	gso_type = __tun_gso_type_mask(AF_INET6, cp->af);
 	if (tun_type == IP_VS_CONN_F_TUNNEL_TYPE_GUE) {
@@ -1414,14 +1390,10 @@ ip_vs_tunnel_xmit_v6(struct sk_buff *skb, struct ip_vs_conn *cp,
 	else if (ret == NF_DROP)
 		kfree_skb(skb);
 
-	LeaveFunction(10);
-
 	return NF_STOLEN;
 
 tx_error:
-	if (!IS_ERR(skb))
-		kfree_skb(skb);
-	LeaveFunction(10);
+	kfree_skb(skb);
 	return NF_STOLEN;
 }
 #endif
@@ -1437,8 +1409,6 @@ ip_vs_dr_xmit(struct sk_buff *skb, struct ip_vs_conn *cp,
 {
 	int local;
 
-	EnterFunction(10);
-
 	local = __ip_vs_get_out_rt(cp->ipvs, cp->af, skb, cp->dest, cp->daddr.ip,
 				   IP_VS_RT_MODE_LOCAL |
 				   IP_VS_RT_MODE_NON_LOCAL |
@@ -1455,12 +1425,10 @@ ip_vs_dr_xmit(struct sk_buff *skb, struct ip_vs_conn *cp,
 
 	ip_vs_send_or_cont(NFPROTO_IPV4, skb, cp, 0);
 
-	LeaveFunction(10);
 	return NF_STOLEN;
 
   tx_error:
 	kfree_skb(skb);
-	LeaveFunction(10);
 	return NF_STOLEN;
 }
 
@@ -1471,8 +1439,6 @@ ip_vs_dr_xmit_v6(struct sk_buff *skb, struct ip_vs_conn *cp,
 {
 	int local;
 
-	EnterFunction(10);
-
 	local = __ip_vs_get_out_rt_v6(cp->ipvs, cp->af, skb, cp->dest,
 				      &cp->daddr.in6,
 				      NULL, ipvsh, 0,
@@ -1489,12 +1455,10 @@ ip_vs_dr_xmit_v6(struct sk_buff *skb, struct ip_vs_conn *cp,
 
 	ip_vs_send_or_cont(NFPROTO_IPV6, skb, cp, 0);
 
-	LeaveFunction(10);
 	return NF_STOLEN;
 
 tx_error:
 	kfree_skb(skb);
-	LeaveFunction(10);
 	return NF_STOLEN;
 }
 #endif
@@ -1514,8 +1478,6 @@ ip_vs_icmp_xmit(struct sk_buff *skb, struct ip_vs_conn *cp,
 	int local;
 	int rt_mode, was_input;
 
-	EnterFunction(10);
-
 	/* The ICMP packet for VS/TUN, VS/DR and LOCALNODE will be
 	   forwarded directly here, because there is no need to
 	   translate address/port back */
@@ -1526,7 +1488,7 @@ ip_vs_icmp_xmit(struct sk_buff *skb, struct ip_vs_conn *cp,
 			rc = NF_ACCEPT;
 		/* do not touch skb anymore */
 		atomic_inc(&cp->in_pkts);
-		goto out;
+		return rc;
 	}
 
 	/*
@@ -1582,14 +1544,11 @@ ip_vs_icmp_xmit(struct sk_buff *skb, struct ip_vs_conn *cp,
 	/* Another hack: avoid icmp_send in ip_fragment */
 	skb->ignore_df = 1;
 
-	rc = ip_vs_nat_send_or_cont(NFPROTO_IPV4, skb, cp, local);
-	goto out;
+	return ip_vs_nat_send_or_cont(NFPROTO_IPV4, skb, cp, local);
 
   tx_error:
 	kfree_skb(skb);
 	rc = NF_STOLEN;
-  out:
-	LeaveFunction(10);
 	return rc;
 }
 
@@ -1604,8 +1563,6 @@ ip_vs_icmp_xmit_v6(struct sk_buff *skb, struct ip_vs_conn *cp,
 	int local;
 	int rt_mode;
 
-	EnterFunction(10);
-
 	/* The ICMP packet for VS/TUN, VS/DR and LOCALNODE will be
 	   forwarded directly here, because there is no need to
 	   translate address/port back */
@@ -1616,7 +1573,7 @@ ip_vs_icmp_xmit_v6(struct sk_buff *skb, struct ip_vs_conn *cp,
 			rc = NF_ACCEPT;
 		/* do not touch skb anymore */
 		atomic_inc(&cp->in_pkts);
-		goto out;
+		return rc;
 	}
 
 	/*
@@ -1671,14 +1628,11 @@ ip_vs_icmp_xmit_v6(struct sk_buff *skb, struct ip_vs_conn *cp,
 	/* Another hack: avoid icmp_send in ip_fragment */
 	skb->ignore_df = 1;
 
-	rc = ip_vs_nat_send_or_cont(NFPROTO_IPV6, skb, cp, local);
-	goto out;
+	return ip_vs_nat_send_or_cont(NFPROTO_IPV6, skb, cp, local);
 
 tx_error:
 	kfree_skb(skb);
 	rc = NF_STOLEN;
-out:
-	LeaveFunction(10);
 	return rc;
 }
 #endif

-- 
2.30.2

Re: [PATCH nf-next v2 3/4] ipvs: Remove {Enter,Leave}Function

[Horatiu Vultur]

The 04/11/2023 09:10, Simon Horman wrote:
> 
> Remove EnterFunction and LeaveFunction.
> 
> These debugging macros seem well past their use-by date.  And seem to
> have little value these days. Removing them allows some trivial cleanup
> of some exit paths for some functions. These are also included in this
> patch. There is likely scope for further cleanup of both debugging and
> unwind paths. But let's leave that for another day.
> 
> Only intended to change debug output, and only when CONFIG_IP_VS_DEBUG
> is enabled. Compile tested only.

Reviewed-by: Horatiu Vultur <horatiu.vultur@microchip.com>

> 
> Signed-off-by: Simon Horman <horms@kernel.org>
> ---
>  include/net/ip_vs.h             | 20 -------------
>  net/netfilter/ipvs/ip_vs_core.c |  8 ------
>  net/netfilter/ipvs/ip_vs_ctl.c  | 26 +----------------
>  net/netfilter/ipvs/ip_vs_sync.c |  5 ----
>  net/netfilter/ipvs/ip_vs_xmit.c | 62 ++++++-----------------------------------
>  5 files changed, 9 insertions(+), 112 deletions(-)
> 
> diff --git a/include/net/ip_vs.h b/include/net/ip_vs.h
> index e20f1f92066d..a3adc246ee31 100644
> --- a/include/net/ip_vs.h
> +++ b/include/net/ip_vs.h
> @@ -265,26 +265,6 @@ static inline const char *ip_vs_dbg_addr(int af, char *buf, size_t buf_len,
>                         pr_err(msg, ##__VA_ARGS__);                     \
>         } while (0)
> 
> -#ifdef CONFIG_IP_VS_DEBUG
> -#define EnterFunction(level)                                           \
> -       do {                                                            \
> -               if (level <= ip_vs_get_debug_level())                   \
> -                       printk(KERN_DEBUG                               \
> -                              pr_fmt("Enter: %s, %s line %i\n"),       \
> -                              __func__, __FILE__, __LINE__);           \
> -       } while (0)
> -#define LeaveFunction(level)                                           \
> -       do {                                                            \
> -               if (level <= ip_vs_get_debug_level())                   \
> -                       printk(KERN_DEBUG                               \
> -                              pr_fmt("Leave: %s, %s line %i\n"),       \
> -                              __func__, __FILE__, __LINE__);           \
> -       } while (0)
> -#else
> -#define EnterFunction(level)   do {} while (0)
> -#define LeaveFunction(level)   do {} while (0)
> -#endif
> -
>  /* The port number of FTP service (in network order). */
>  #define FTPPORT  cpu_to_be16(21)
>  #define FTPDATA  cpu_to_be16(20)
> diff --git a/net/netfilter/ipvs/ip_vs_core.c b/net/netfilter/ipvs/ip_vs_core.c
> index 2fcc26507d69..cb83ca506c5c 100644
> --- a/net/netfilter/ipvs/ip_vs_core.c
> +++ b/net/netfilter/ipvs/ip_vs_core.c
> @@ -1140,7 +1140,6 @@ struct ip_vs_conn *ip_vs_new_conn_out(struct ip_vs_service *svc,
>         __be16 vport;
>         unsigned int flags;
> 
> -       EnterFunction(12);
>         vaddr = &svc->addr;
>         vport = svc->port;
>         daddr = &iph->saddr;
> @@ -1208,7 +1207,6 @@ struct ip_vs_conn *ip_vs_new_conn_out(struct ip_vs_service *svc,
>                       IP_VS_DBG_ADDR(cp->af, &cp->vaddr), ntohs(cp->vport),
>                       IP_VS_DBG_ADDR(cp->af, &cp->daddr), ntohs(cp->dport),
>                       cp->flags, refcount_read(&cp->refcnt));
> -       LeaveFunction(12);
>         return cp;
>  }
> 
> @@ -1316,13 +1314,11 @@ handle_response(int af, struct sk_buff *skb, struct ip_vs_proto_data *pd,
>                 ip_vs_update_conntrack(skb, cp, 0);
>         ip_vs_conn_put(cp);
> 
> -       LeaveFunction(11);
>         return NF_ACCEPT;
> 
>  drop:
>         ip_vs_conn_put(cp);
>         kfree_skb(skb);
> -       LeaveFunction(11);
>         return NF_STOLEN;
>  }
> 
> @@ -1341,8 +1337,6 @@ ip_vs_out_hook(void *priv, struct sk_buff *skb, const struct nf_hook_state *stat
>         int af = state->pf;
>         struct sock *sk;
> 
> -       EnterFunction(11);
> -
>         /* Already marked as IPVS request or reply? */
>         if (skb->ipvs_property)
>                 return NF_ACCEPT;
> @@ -2365,7 +2359,6 @@ static void __net_exit __ip_vs_dev_cleanup_batch(struct list_head *net_list)
>         struct netns_ipvs *ipvs;
>         struct net *net;
> 
> -       EnterFunction(2);
>         list_for_each_entry(net, net_list, exit_list) {
>                 ipvs = net_ipvs(net);
>                 ip_vs_unregister_hooks(ipvs, AF_INET);
> @@ -2374,7 +2367,6 @@ static void __net_exit __ip_vs_dev_cleanup_batch(struct list_head *net_list)
>                 smp_wmb();
>                 ip_vs_sync_net_cleanup(ipvs);
>         }
> -       LeaveFunction(2);
>  }
> 
>  static struct pernet_operations ipvs_core_ops = {
> diff --git a/net/netfilter/ipvs/ip_vs_ctl.c b/net/netfilter/ipvs/ip_vs_ctl.c
> index 2a5ed71c82c3..62606fb44d02 100644
> --- a/net/netfilter/ipvs/ip_vs_ctl.c
> +++ b/net/netfilter/ipvs/ip_vs_ctl.c
> @@ -1061,8 +1061,6 @@ ip_vs_new_dest(struct ip_vs_service *svc, struct ip_vs_dest_user_kern *udest)
>         unsigned int atype;
>         int ret;
> 
> -       EnterFunction(2);
> -
>  #ifdef CONFIG_IP_VS_IPV6
>         if (udest->af == AF_INET6) {
>                 atype = ipv6_addr_type(&udest->addr.in6);
> @@ -1111,7 +1109,6 @@ ip_vs_new_dest(struct ip_vs_service *svc, struct ip_vs_dest_user_kern *udest)
>         spin_lock_init(&dest->dst_lock);
>         __ip_vs_update_dest(svc, dest, udest, 1);
> 
> -       LeaveFunction(2);
>         return 0;
> 
>  err_stats:
> @@ -1134,8 +1131,6 @@ ip_vs_add_dest(struct ip_vs_service *svc, struct ip_vs_dest_user_kern *udest)
>         __be16 dport = udest->port;
>         int ret;
> 
> -       EnterFunction(2);
> -
>         if (udest->weight < 0) {
>                 pr_err("%s(): server weight less than zero\n", __func__);
>                 return -ERANGE;
> @@ -1183,7 +1178,7 @@ ip_vs_add_dest(struct ip_vs_service *svc, struct ip_vs_dest_user_kern *udest)
> 
>                 ret = ip_vs_start_estimator(svc->ipvs, &dest->stats);
>                 if (ret < 0)
> -                       goto err;
> +                       return ret;
>                 __ip_vs_update_dest(svc, dest, udest, 1);
>         } else {
>                 /*
> @@ -1192,9 +1187,6 @@ ip_vs_add_dest(struct ip_vs_service *svc, struct ip_vs_dest_user_kern *udest)
>                 ret = ip_vs_new_dest(svc, udest);
>         }
> 
> -err:
> -       LeaveFunction(2);
> -
>         return ret;
>  }
> 
> @@ -1209,8 +1201,6 @@ ip_vs_edit_dest(struct ip_vs_service *svc, struct ip_vs_dest_user_kern *udest)
>         union nf_inet_addr daddr;
>         __be16 dport = udest->port;
> 
> -       EnterFunction(2);
> -
>         if (udest->weight < 0) {
>                 pr_err("%s(): server weight less than zero\n", __func__);
>                 return -ERANGE;
> @@ -1242,7 +1232,6 @@ ip_vs_edit_dest(struct ip_vs_service *svc, struct ip_vs_dest_user_kern *udest)
>         }
> 
>         __ip_vs_update_dest(svc, dest, udest, 0);
> -       LeaveFunction(2);
> 
>         return 0;
>  }
> @@ -1317,8 +1306,6 @@ ip_vs_del_dest(struct ip_vs_service *svc, struct ip_vs_dest_user_kern *udest)
>         struct ip_vs_dest *dest;
>         __be16 dport = udest->port;
> 
> -       EnterFunction(2);
> -
>         /* We use function that requires RCU lock */
>         rcu_read_lock();
>         dest = ip_vs_lookup_dest(svc, udest->af, &udest->addr, dport);
> @@ -1339,8 +1326,6 @@ ip_vs_del_dest(struct ip_vs_service *svc, struct ip_vs_dest_user_kern *udest)
>          */
>         __ip_vs_del_dest(svc->ipvs, dest, false);
> 
> -       LeaveFunction(2);
> -
>         return 0;
>  }
> 
> @@ -1746,7 +1731,6 @@ void ip_vs_service_nets_cleanup(struct list_head *net_list)
>         struct netns_ipvs *ipvs;
>         struct net *net;
> 
> -       EnterFunction(2);
>         /* Check for "full" addressed entries */
>         mutex_lock(&__ip_vs_mutex);
>         list_for_each_entry(net, net_list, exit_list) {
> @@ -1754,7 +1738,6 @@ void ip_vs_service_nets_cleanup(struct list_head *net_list)
>                 ip_vs_flush(ipvs, true);
>         }
>         mutex_unlock(&__ip_vs_mutex);
> -       LeaveFunction(2);
>  }
> 
>  /* Put all references for device (dst_cache) */
> @@ -1792,7 +1775,6 @@ static int ip_vs_dst_event(struct notifier_block *this, unsigned long event,
>         if (event != NETDEV_DOWN || !ipvs)
>                 return NOTIFY_DONE;
>         IP_VS_DBG(3, "%s() dev=%s\n", __func__, dev->name);
> -       EnterFunction(2);
>         mutex_lock(&__ip_vs_mutex);
>         for (idx = 0; idx < IP_VS_SVC_TAB_SIZE; idx++) {
>                 hlist_for_each_entry(svc, &ip_vs_svc_table[idx], s_list) {
> @@ -1821,7 +1803,6 @@ static int ip_vs_dst_event(struct notifier_block *this, unsigned long event,
>         }
>         spin_unlock_bh(&ipvs->dest_trash_lock);
>         mutex_unlock(&__ip_vs_mutex);
> -       LeaveFunction(2);
>         return NOTIFY_DONE;
>  }
> 
> @@ -4537,8 +4518,6 @@ int __init ip_vs_control_init(void)
>         int idx;
>         int ret;
> 
> -       EnterFunction(2);
> -
>         /* Initialize svc_table, ip_vs_svc_fwm_table */
>         for (idx = 0; idx < IP_VS_SVC_TAB_SIZE; idx++) {
>                 INIT_HLIST_HEAD(&ip_vs_svc_table[idx]);
> @@ -4551,15 +4530,12 @@ int __init ip_vs_control_init(void)
>         if (ret < 0)
>                 return ret;
> 
> -       LeaveFunction(2);
>         return 0;
>  }
> 
> 
>  void ip_vs_control_cleanup(void)
>  {
> -       EnterFunction(2);
>         unregister_netdevice_notifier(&ip_vs_dst_notifier);
>         /* relying on common rcu_barrier() in ip_vs_cleanup() */
> -       LeaveFunction(2);
>  }
> diff --git a/net/netfilter/ipvs/ip_vs_sync.c b/net/netfilter/ipvs/ip_vs_sync.c
> index d4fe7bb4f853..264f2f87a437 100644
> --- a/net/netfilter/ipvs/ip_vs_sync.c
> +++ b/net/netfilter/ipvs/ip_vs_sync.c
> @@ -1582,13 +1582,11 @@ ip_vs_send_async(struct socket *sock, const char *buffer, const size_t length)
>         struct kvec     iov;
>         int             len;
> 
> -       EnterFunction(7);
>         iov.iov_base     = (void *)buffer;
>         iov.iov_len      = length;
> 
>         len = kernel_sendmsg(sock, &msg, &iov, 1, (size_t)(length));
> 
> -       LeaveFunction(7);
>         return len;
>  }
> 
> @@ -1614,15 +1612,12 @@ ip_vs_receive(struct socket *sock, char *buffer, const size_t buflen)
>         struct kvec             iov = {buffer, buflen};
>         int                     len;
> 
> -       EnterFunction(7);
> -
>         /* Receive a packet */
>         iov_iter_kvec(&msg.msg_iter, ITER_DEST, &iov, 1, buflen);
>         len = sock_recvmsg(sock, &msg, MSG_DONTWAIT);
>         if (len < 0)
>                 return len;
> 
> -       LeaveFunction(7);
>         return len;
>  }
> 
> diff --git a/net/netfilter/ipvs/ip_vs_xmit.c b/net/netfilter/ipvs/ip_vs_xmit.c
> index 99c349c0d968..feb1d7fcb09f 100644
> --- a/net/netfilter/ipvs/ip_vs_xmit.c
> +++ b/net/netfilter/ipvs/ip_vs_xmit.c
> @@ -706,8 +706,6 @@ ip_vs_bypass_xmit(struct sk_buff *skb, struct ip_vs_conn *cp,
>  {
>         struct iphdr  *iph = ip_hdr(skb);
> 
> -       EnterFunction(10);
> -
>         if (__ip_vs_get_out_rt(cp->ipvs, cp->af, skb, NULL, iph->daddr,
>                                IP_VS_RT_MODE_NON_LOCAL, NULL, ipvsh) < 0)
>                 goto tx_error;
> @@ -719,12 +717,10 @@ ip_vs_bypass_xmit(struct sk_buff *skb, struct ip_vs_conn *cp,
> 
>         ip_vs_send_or_cont(NFPROTO_IPV4, skb, cp, 0);
> 
> -       LeaveFunction(10);
>         return NF_STOLEN;
> 
>   tx_error:
>         kfree_skb(skb);
> -       LeaveFunction(10);
>         return NF_STOLEN;
>  }
> 
> @@ -735,8 +731,6 @@ ip_vs_bypass_xmit_v6(struct sk_buff *skb, struct ip_vs_conn *cp,
>  {
>         struct ipv6hdr *iph = ipv6_hdr(skb);
> 
> -       EnterFunction(10);
> -
>         if (__ip_vs_get_out_rt_v6(cp->ipvs, cp->af, skb, NULL,
>                                   &iph->daddr, NULL,
>                                   ipvsh, 0, IP_VS_RT_MODE_NON_LOCAL) < 0)
> @@ -747,12 +741,10 @@ ip_vs_bypass_xmit_v6(struct sk_buff *skb, struct ip_vs_conn *cp,
> 
>         ip_vs_send_or_cont(NFPROTO_IPV6, skb, cp, 0);
> 
> -       LeaveFunction(10);
>         return NF_STOLEN;
> 
>   tx_error:
>         kfree_skb(skb);
> -       LeaveFunction(10);
>         return NF_STOLEN;
>  }
>  #endif
> @@ -768,8 +760,6 @@ ip_vs_nat_xmit(struct sk_buff *skb, struct ip_vs_conn *cp,
>         struct rtable *rt;              /* Route to the other host */
>         int local, rc, was_input;
> 
> -       EnterFunction(10);
> -
>         /* check if it is a connection of no-client-port */
>         if (unlikely(cp->flags & IP_VS_CONN_F_NO_CPORT)) {
>                 __be16 _pt, *p;
> @@ -839,12 +829,10 @@ ip_vs_nat_xmit(struct sk_buff *skb, struct ip_vs_conn *cp,
> 
>         rc = ip_vs_nat_send_or_cont(NFPROTO_IPV4, skb, cp, local);
> 
> -       LeaveFunction(10);
>         return rc;
> 
>    tx_error:
>         kfree_skb(skb);
> -       LeaveFunction(10);
>         return NF_STOLEN;
>  }
> 
> @@ -856,8 +844,6 @@ ip_vs_nat_xmit_v6(struct sk_buff *skb, struct ip_vs_conn *cp,
>         struct rt6_info *rt;            /* Route to the other host */
>         int local, rc;
> 
> -       EnterFunction(10);
> -
>         /* check if it is a connection of no-client-port */
>         if (unlikely(cp->flags & IP_VS_CONN_F_NO_CPORT && !ipvsh->fragoffs)) {
>                 __be16 _pt, *p;
> @@ -927,11 +913,9 @@ ip_vs_nat_xmit_v6(struct sk_buff *skb, struct ip_vs_conn *cp,
> 
>         rc = ip_vs_nat_send_or_cont(NFPROTO_IPV6, skb, cp, local);
> 
> -       LeaveFunction(10);
>         return rc;
> 
>  tx_error:
> -       LeaveFunction(10);
>         kfree_skb(skb);
>         return NF_STOLEN;
>  }
> @@ -1149,8 +1133,6 @@ ip_vs_tunnel_xmit(struct sk_buff *skb, struct ip_vs_conn *cp,
>         int tun_type, gso_type;
>         int tun_flags;
> 
> -       EnterFunction(10);
> -
>         local = __ip_vs_get_out_rt(ipvs, cp->af, skb, cp->dest, cp->daddr.ip,
>                                    IP_VS_RT_MODE_LOCAL |
>                                    IP_VS_RT_MODE_NON_LOCAL |
> @@ -1199,7 +1181,7 @@ ip_vs_tunnel_xmit(struct sk_buff *skb, struct ip_vs_conn *cp,
>                                          &next_protocol, NULL, &dsfield,
>                                          &ttl, dfp);
>         if (IS_ERR(skb))
> -               goto tx_error;
> +               return NF_STOLEN;
> 
>         gso_type = __tun_gso_type_mask(AF_INET, cp->af);
>         if (tun_type == IP_VS_CONN_F_TUNNEL_TYPE_GUE) {
> @@ -1267,14 +1249,10 @@ ip_vs_tunnel_xmit(struct sk_buff *skb, struct ip_vs_conn *cp,
>         else if (ret == NF_DROP)
>                 kfree_skb(skb);
> 
> -       LeaveFunction(10);
> -
>         return NF_STOLEN;
> 
>    tx_error:
> -       if (!IS_ERR(skb))
> -               kfree_skb(skb);
> -       LeaveFunction(10);
> +       kfree_skb(skb);
>         return NF_STOLEN;
>  }
> 
> @@ -1298,8 +1276,6 @@ ip_vs_tunnel_xmit_v6(struct sk_buff *skb, struct ip_vs_conn *cp,
>         int tun_type, gso_type;
>         int tun_flags;
> 
> -       EnterFunction(10);
> -
>         local = __ip_vs_get_out_rt_v6(ipvs, cp->af, skb, cp->dest,
>                                       &cp->daddr.in6,
>                                       &saddr, ipvsh, 1,
> @@ -1347,7 +1323,7 @@ ip_vs_tunnel_xmit_v6(struct sk_buff *skb, struct ip_vs_conn *cp,
>                                          &next_protocol, &payload_len,
>                                          &dsfield, &ttl, NULL);
>         if (IS_ERR(skb))
> -               goto tx_error;
> +               return NF_STOLEN;
> 
>         gso_type = __tun_gso_type_mask(AF_INET6, cp->af);
>         if (tun_type == IP_VS_CONN_F_TUNNEL_TYPE_GUE) {
> @@ -1414,14 +1390,10 @@ ip_vs_tunnel_xmit_v6(struct sk_buff *skb, struct ip_vs_conn *cp,
>         else if (ret == NF_DROP)
>                 kfree_skb(skb);
> 
> -       LeaveFunction(10);
> -
>         return NF_STOLEN;
> 
>  tx_error:
> -       if (!IS_ERR(skb))
> -               kfree_skb(skb);
> -       LeaveFunction(10);
> +       kfree_skb(skb);
>         return NF_STOLEN;
>  }
>  #endif
> @@ -1437,8 +1409,6 @@ ip_vs_dr_xmit(struct sk_buff *skb, struct ip_vs_conn *cp,
>  {
>         int local;
> 
> -       EnterFunction(10);
> -
>         local = __ip_vs_get_out_rt(cp->ipvs, cp->af, skb, cp->dest, cp->daddr.ip,
>                                    IP_VS_RT_MODE_LOCAL |
>                                    IP_VS_RT_MODE_NON_LOCAL |
> @@ -1455,12 +1425,10 @@ ip_vs_dr_xmit(struct sk_buff *skb, struct ip_vs_conn *cp,
> 
>         ip_vs_send_or_cont(NFPROTO_IPV4, skb, cp, 0);
> 
> -       LeaveFunction(10);
>         return NF_STOLEN;
> 
>    tx_error:
>         kfree_skb(skb);
> -       LeaveFunction(10);
>         return NF_STOLEN;
>  }
> 
> @@ -1471,8 +1439,6 @@ ip_vs_dr_xmit_v6(struct sk_buff *skb, struct ip_vs_conn *cp,
>  {
>         int local;
> 
> -       EnterFunction(10);
> -
>         local = __ip_vs_get_out_rt_v6(cp->ipvs, cp->af, skb, cp->dest,
>                                       &cp->daddr.in6,
>                                       NULL, ipvsh, 0,
> @@ -1489,12 +1455,10 @@ ip_vs_dr_xmit_v6(struct sk_buff *skb, struct ip_vs_conn *cp,
> 
>         ip_vs_send_or_cont(NFPROTO_IPV6, skb, cp, 0);
> 
> -       LeaveFunction(10);
>         return NF_STOLEN;
> 
>  tx_error:
>         kfree_skb(skb);
> -       LeaveFunction(10);
>         return NF_STOLEN;
>  }
>  #endif
> @@ -1514,8 +1478,6 @@ ip_vs_icmp_xmit(struct sk_buff *skb, struct ip_vs_conn *cp,
>         int local;
>         int rt_mode, was_input;
> 
> -       EnterFunction(10);
> -
>         /* The ICMP packet for VS/TUN, VS/DR and LOCALNODE will be
>            forwarded directly here, because there is no need to
>            translate address/port back */
> @@ -1526,7 +1488,7 @@ ip_vs_icmp_xmit(struct sk_buff *skb, struct ip_vs_conn *cp,
>                         rc = NF_ACCEPT;
>                 /* do not touch skb anymore */
>                 atomic_inc(&cp->in_pkts);
> -               goto out;
> +               return rc;
>         }
> 
>         /*
> @@ -1582,14 +1544,11 @@ ip_vs_icmp_xmit(struct sk_buff *skb, struct ip_vs_conn *cp,
>         /* Another hack: avoid icmp_send in ip_fragment */
>         skb->ignore_df = 1;
> 
> -       rc = ip_vs_nat_send_or_cont(NFPROTO_IPV4, skb, cp, local);
> -       goto out;
> +       return ip_vs_nat_send_or_cont(NFPROTO_IPV4, skb, cp, local);
> 
>    tx_error:
>         kfree_skb(skb);
>         rc = NF_STOLEN;
> -  out:
> -       LeaveFunction(10);
>         return rc;
>  }
> 
> @@ -1604,8 +1563,6 @@ ip_vs_icmp_xmit_v6(struct sk_buff *skb, struct ip_vs_conn *cp,
>         int local;
>         int rt_mode;
> 
> -       EnterFunction(10);
> -
>         /* The ICMP packet for VS/TUN, VS/DR and LOCALNODE will be
>            forwarded directly here, because there is no need to
>            translate address/port back */
> @@ -1616,7 +1573,7 @@ ip_vs_icmp_xmit_v6(struct sk_buff *skb, struct ip_vs_conn *cp,
>                         rc = NF_ACCEPT;
>                 /* do not touch skb anymore */
>                 atomic_inc(&cp->in_pkts);
> -               goto out;
> +               return rc;
>         }
> 
>         /*
> @@ -1671,14 +1628,11 @@ ip_vs_icmp_xmit_v6(struct sk_buff *skb, struct ip_vs_conn *cp,
>         /* Another hack: avoid icmp_send in ip_fragment */
>         skb->ignore_df = 1;
> 
> -       rc = ip_vs_nat_send_or_cont(NFPROTO_IPV6, skb, cp, local);
> -       goto out;
> +       return ip_vs_nat_send_or_cont(NFPROTO_IPV6, skb, cp, local);
> 
>  tx_error:
>         kfree_skb(skb);
>         rc = NF_STOLEN;
> -out:
> -       LeaveFunction(10);
>         return rc;
>  }
>  #endif
> 
> --
> 2.30.2
> 

-- 
/Horatiu

[PATCH nf-next v2 4/4] ipvs: Correct spelling in comments

[Simon Horman]

Correct some spelling errors flagged by codespell and found by inspection.

Signed-off-by: Simon Horman <horms@kernel.org>
---
 include/net/ip_vs.h | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/include/net/ip_vs.h b/include/net/ip_vs.h
index a3adc246ee31..ff406ef4fd4a 100644
--- a/include/net/ip_vs.h
+++ b/include/net/ip_vs.h
@@ -584,7 +584,7 @@ struct ip_vs_conn {
 	spinlock_t              lock;           /* lock for state transition */
 	volatile __u16          state;          /* state info */
 	volatile __u16          old_state;      /* old state, to be used for
-						 * state transition triggerd
+						 * state transition triggered
 						 * synchronization
 						 */
 	__u32			fwmark;		/* Fire wall mark from skb */
@@ -635,7 +635,7 @@ struct ip_vs_service_user_kern {
 	u16			protocol;
 	union nf_inet_addr	addr;		/* virtual ip address */
 	__be16			port;
-	u32			fwmark;		/* firwall mark of service */
+	u32			fwmark;		/* firewall mark of service */
 
 	/* virtual service options */
 	char			*sched_name;
@@ -1036,7 +1036,7 @@ struct netns_ipvs {
 	struct ipvs_sync_daemon_cfg	bcfg;	/* Backup Configuration */
 	/* net name space ptr */
 	struct net		*net;            /* Needed by timer routines */
-	/* Number of heterogeneous destinations, needed becaus heterogeneous
+	/* Number of heterogeneous destinations, needed because heterogeneous
 	 * are not supported when synchronization is enabled.
 	 */
 	unsigned int		mixed_address_family_dests;

-- 
2.30.2

Re: [PATCH nf-next v2 4/4] ipvs: Correct spelling in comments

[Horatiu Vultur]

The 04/11/2023 09:10, Simon Horman wrote:
> 
> Correct some spelling errors flagged by codespell and found by inspection.

Reviewed-by: Horatiu Vultur <horatiu.vultur@microchip.com>

> 
> Signed-off-by: Simon Horman <horms@kernel.org>
> ---
>  include/net/ip_vs.h | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/include/net/ip_vs.h b/include/net/ip_vs.h
> index a3adc246ee31..ff406ef4fd4a 100644
> --- a/include/net/ip_vs.h
> +++ b/include/net/ip_vs.h
> @@ -584,7 +584,7 @@ struct ip_vs_conn {
>         spinlock_t              lock;           /* lock for state transition */
>         volatile __u16          state;          /* state info */
>         volatile __u16          old_state;      /* old state, to be used for
> -                                                * state transition triggerd
> +                                                * state transition triggered
>                                                  * synchronization
>                                                  */
>         __u32                   fwmark;         /* Fire wall mark from skb */
> @@ -635,7 +635,7 @@ struct ip_vs_service_user_kern {
>         u16                     protocol;
>         union nf_inet_addr      addr;           /* virtual ip address */
>         __be16                  port;
> -       u32                     fwmark;         /* firwall mark of service */
> +       u32                     fwmark;         /* firewall mark of service */
> 
>         /* virtual service options */
>         char                    *sched_name;
> @@ -1036,7 +1036,7 @@ struct netns_ipvs {
>         struct ipvs_sync_daemon_cfg     bcfg;   /* Backup Configuration */
>         /* net name space ptr */
>         struct net              *net;            /* Needed by timer routines */
> -       /* Number of heterogeneous destinations, needed becaus heterogeneous
> +       /* Number of heterogeneous destinations, needed because heterogeneous
>          * are not supported when synchronization is enabled.
>          */
>         unsigned int            mixed_address_family_dests;
> 
> --
> 2.30.2
> 

-- 
/Horatiu