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 v3:
- Patch 2/4: Correct division by 1024.
It was applied to the wrong variable in v2.
- Add Horatiu's Reviewed-by tag.
Changes in v2:
- Patch 1/4: Correct spelling of 'conn' in subject.
- Patch 2/4: Restore division by 1024. It was lost on v1.
---
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: 99676a5766412f3936c55b9d18565d248e5463ee
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 <[email protected]>
Reviewed-by: Horatiu Vultur <[email protected]>
---
v3
- Add Horatiu's Reviewed-by tag
v2
- Correct spelling of 'conn' in subject
- No change
---
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
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 <[email protected]>
Reviewed-by: Horatiu Vultur <[email protected]>
---
v3
- Correct division by 1024. It was applied to the wrong variable in v2.
- Add Horatiu's Reviewed-by tag
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..928e64653837 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, tab_array_size / 1024);
IP_VS_DBG(0, "Each connection entry needs %zd bytes at least\n",
sizeof(struct ip_vs_conn));
--
2.30.2
Correct some spelling errors flagged by codespell and found by inspection.
Signed-off-by: Simon Horman <[email protected]>
Reviewed-by: Horatiu Vultur <[email protected]>
---
v3
- Add Horatiu's Reviewed-by tag
v2
- No change
---
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
Hello,
On Mon, 17 Apr 2023, Simon Horman wrote:
> 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 v3:
> - Patch 2/4: Correct division by 1024.
> It was applied to the wrong variable in v2.
> - Add Horatiu's Reviewed-by tag.
>
> Changes in v2:
> - Patch 1/4: Correct spelling of 'conn' in subject.
> - Patch 2/4: Restore division by 1024. It was lost on v1.
>
> ---
> 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
The patchset looks good to me, thanks!
Acked-by: Julian Anastasov <[email protected]>
> 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: 99676a5766412f3936c55b9d18565d248e5463ee
Regards
--
Julian Anastasov <[email protected]>
On Mon, Apr 17, 2023 at 07:59:35PM +0300, Julian Anastasov wrote:
>
> Hello,
>
> On Mon, 17 Apr 2023, Simon Horman wrote:
>
> > 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 v3:
> > - Patch 2/4: Correct division by 1024.
> > It was applied to the wrong variable in v2.
> > - Add Horatiu's Reviewed-by tag.
> >
> > Changes in v2:
> > - Patch 1/4: Correct spelling of 'conn' in subject.
> > - Patch 2/4: Restore division by 1024. It was lost on v1.
> >
> > ---
> > 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
>
> The patchset looks good to me, thanks!
>
> Acked-by: Julian Anastasov <[email protected]>
Applied, sorry Julian, I missed your Acked-by: tag.
Hello,
On Sat, 22 Apr 2023, Pablo Neira Ayuso wrote:
> On Mon, Apr 17, 2023 at 07:59:35PM +0300, Julian Anastasov wrote:
> >
> > On Mon, 17 Apr 2023, Simon Horman wrote:
> >
> > > 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 v3:
> > > - Patch 2/4: Correct division by 1024.
> > > It was applied to the wrong variable in v2.
> > > - Add Horatiu's Reviewed-by tag.
> > >
> > > Changes in v2:
> > > - Patch 1/4: Correct spelling of 'conn' in subject.
> > > - Patch 2/4: Restore division by 1024. It was lost on v1.
> > >
> > > ---
> > > 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
> >
> > The patchset looks good to me, thanks!
> >
> > Acked-by: Julian Anastasov <[email protected]>
>
> Applied, sorry Julian, I missed your Acked-by: tag.
No problem :)
Regards
--
Julian Anastasov <[email protected]>