* [PATCH v2 net-next 0/2] net: default_rps_mask follow-up @ 2023-02-17 12:28 Paolo Abeni 2023-02-17 12:28 ` [PATCH v2 net-next 1/2] net: make default_rps_mask a per netns attribute Paolo Abeni 2023-02-17 12:28 ` [PATCH v2 net-next 2/2] self-tests: more rps self tests Paolo Abeni 0 siblings, 2 replies; 4+ messages in thread From: Paolo Abeni @ 2023-02-17 12:28 UTC (permalink / raw) To: netdev; +Cc: David S. Miller, Jakub Kicinski, Eric Dumazet, Shuah Khan The first patch namespacify the setting. In the common case, once proper isolation is in place in the main namespace, forwarding to/from each child netns will allways happen on the desidered CPUs. Any additional RPS stage inside the child namespace will not provide additional isolation and could hurt performance badly if picking a CPU on a remote node. The 2nd patch adds more self-tests coverage. Paolo Abeni (2): net: make default_rps_mask a per netns attribute self-tests: more rps self tests include/linux/netdevice.h | 1 - include/net/netns/core.h | 5 ++ net/core/net-sysfs.c | 23 ++++++--- net/core/sysctl_net_core.c | 51 ++++++++++++++----- .../testing/selftests/net/rps_default_mask.sh | 41 ++++++++++----- 5 files changed, 88 insertions(+), 33 deletions(-) -- 2.39.1 ^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH v2 net-next 1/2] net: make default_rps_mask a per netns attribute 2023-02-17 12:28 [PATCH v2 net-next 0/2] net: default_rps_mask follow-up Paolo Abeni @ 2023-02-17 12:28 ` Paolo Abeni 2023-02-21 15:45 ` Simon Horman 2023-02-17 12:28 ` [PATCH v2 net-next 2/2] self-tests: more rps self tests Paolo Abeni 1 sibling, 1 reply; 4+ messages in thread From: Paolo Abeni @ 2023-02-17 12:28 UTC (permalink / raw) To: netdev; +Cc: David S. Miller, Jakub Kicinski, Eric Dumazet, Shuah Khan That really was meant to be a per netns attribute from the beginning. The idea is that once proper isolation is in place in the main namespace, additional demux in the child namespaces will be redundant. Let's make child netns default rps mask empty by default. To avoid bloating the netns with a possibly large cpumask, allocate it on-demand during the first write operation. Signed-off-by: Paolo Abeni <pabeni@redhat.com> --- v1 -> v2: - fix build issue for !CONFIG_CPUMASK_OFFSTACK build --- include/linux/netdevice.h | 1 - include/net/netns/core.h | 5 ++++ net/core/net-sysfs.c | 23 +++++++++++------ net/core/sysctl_net_core.c | 51 ++++++++++++++++++++++++++++---------- 4 files changed, 59 insertions(+), 21 deletions(-) diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h index efbee940bb03..6a14b7b11766 100644 --- a/include/linux/netdevice.h +++ b/include/linux/netdevice.h @@ -224,7 +224,6 @@ struct net_device_core_stats { #include <linux/static_key.h> extern struct static_key_false rps_needed; extern struct static_key_false rfs_needed; -extern struct cpumask rps_default_mask; #endif struct neighbour; diff --git a/include/net/netns/core.h b/include/net/netns/core.h index 8249060cf5d0..a91ef9f8de60 100644 --- a/include/net/netns/core.h +++ b/include/net/netns/core.h @@ -6,6 +6,7 @@ struct ctl_table_header; struct prot_inuse; +struct cpumask; struct netns_core { /* core sysctls */ @@ -17,6 +18,10 @@ struct netns_core { #ifdef CONFIG_PROC_FS struct prot_inuse __percpu *prot_inuse; #endif + +#if IS_ENABLED(CONFIG_RPS) && IS_ENABLED(CONFIG_SYSCTL) + struct cpumask *rps_default_mask; +#endif }; #endif diff --git a/net/core/net-sysfs.c b/net/core/net-sysfs.c index e20784b6f873..15e3f4606b5f 100644 --- a/net/core/net-sysfs.c +++ b/net/core/net-sysfs.c @@ -1060,6 +1060,18 @@ static const struct kobj_type rx_queue_ktype = { .get_ownership = rx_queue_get_ownership, }; +static int rx_queue_default_mask(struct net_device *dev, + struct netdev_rx_queue *queue) +{ +#if IS_ENABLED(CONFIG_RPS) && IS_ENABLED(CONFIG_SYSCTL) + struct cpumask *rps_default_mask = READ_ONCE(dev_net(dev)->core.rps_default_mask); + + if (rps_default_mask && !cpumask_empty(rps_default_mask)) + return netdev_rx_queue_set_rps_mask(queue, rps_default_mask); +#endif + return 0; +} + static int rx_queue_add_kobject(struct net_device *dev, int index) { struct netdev_rx_queue *queue = dev->_rx + index; @@ -1083,13 +1095,10 @@ static int rx_queue_add_kobject(struct net_device *dev, int index) goto err; } -#if IS_ENABLED(CONFIG_RPS) && IS_ENABLED(CONFIG_SYSCTL) - if (!cpumask_empty(&rps_default_mask)) { - error = netdev_rx_queue_set_rps_mask(queue, &rps_default_mask); - if (error) - goto err; - } -#endif + error = rx_queue_default_mask(dev, queue); + if (error) + goto err; + kobject_uevent(kobj, KOBJ_ADD); return error; diff --git a/net/core/sysctl_net_core.c b/net/core/sysctl_net_core.c index 7130e6d9e263..74842b453407 100644 --- a/net/core/sysctl_net_core.c +++ b/net/core/sysctl_net_core.c @@ -74,24 +74,47 @@ static void dump_cpumask(void *buffer, size_t *lenp, loff_t *ppos, #endif #ifdef CONFIG_RPS -struct cpumask rps_default_mask; + +static struct cpumask *rps_default_mask_cow_alloc(struct net *net) +{ + struct cpumask *rps_default_mask; + + if (net->core.rps_default_mask) + return net->core.rps_default_mask; + + rps_default_mask = kzalloc(cpumask_size(), GFP_KERNEL); + if (!rps_default_mask) + return NULL; + + /* pairs with READ_ONCE in rx_queue_default_mask() */ + WRITE_ONCE(net->core.rps_default_mask, rps_default_mask); + return rps_default_mask; +} static int rps_default_mask_sysctl(struct ctl_table *table, int write, void *buffer, size_t *lenp, loff_t *ppos) { + struct net *net = (struct net *)table->data; int err = 0; rtnl_lock(); if (write) { - err = cpumask_parse(buffer, &rps_default_mask); + struct cpumask *rps_default_mask = rps_default_mask_cow_alloc(net); + + err = -ENOMEM; + if (!rps_default_mask) + goto done; + + err = cpumask_parse(buffer, rps_default_mask); if (err) goto done; - err = rps_cpumask_housekeeping(&rps_default_mask); + err = rps_cpumask_housekeeping(rps_default_mask); if (err) goto done; } else { - dump_cpumask(buffer, lenp, ppos, &rps_default_mask); + dump_cpumask(buffer, lenp, ppos, + net->core.rps_default_mask ? : cpu_none_mask); } done: @@ -508,11 +531,6 @@ static struct ctl_table net_core_table[] = { .mode = 0644, .proc_handler = rps_sock_flow_sysctl }, - { - .procname = "rps_default_mask", - .mode = 0644, - .proc_handler = rps_default_mask_sysctl - }, #endif #ifdef CONFIG_NET_FLOW_LIMIT { @@ -639,6 +657,14 @@ static struct ctl_table net_core_table[] = { }; static struct ctl_table netns_core_table[] = { +#if IS_ENABLED(CONFIG_RPS) + { + .procname = "rps_default_mask", + .data = &init_net, + .mode = 0644, + .proc_handler = rps_default_mask_sysctl + }, +#endif { .procname = "somaxconn", .data = &init_net.core.sysctl_somaxconn, @@ -706,6 +732,9 @@ static __net_exit void sysctl_core_net_exit(struct net *net) tbl = net->core.sysctl_hdr->ctl_table_arg; unregister_net_sysctl_table(net->core.sysctl_hdr); BUG_ON(tbl == netns_core_table); +#if IS_ENABLED(CONFIG_RPS) + kfree(net->core.rps_default_mask); +#endif kfree(tbl); } @@ -716,10 +745,6 @@ static __net_initdata struct pernet_operations sysctl_core_ops = { static __init int sysctl_core_init(void) { -#if IS_ENABLED(CONFIG_RPS) - cpumask_copy(&rps_default_mask, cpu_none_mask); -#endif - register_net_sysctl(&init_net, "net/core", net_core_table); return register_pernet_subsys(&sysctl_core_ops); } -- 2.39.1 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH v2 net-next 1/2] net: make default_rps_mask a per netns attribute 2023-02-17 12:28 ` [PATCH v2 net-next 1/2] net: make default_rps_mask a per netns attribute Paolo Abeni @ 2023-02-21 15:45 ` Simon Horman 0 siblings, 0 replies; 4+ messages in thread From: Simon Horman @ 2023-02-21 15:45 UTC (permalink / raw) To: Paolo Abeni Cc: netdev, David S. Miller, Jakub Kicinski, Eric Dumazet, Shuah Khan On Fri, Feb 17, 2023 at 01:28:49PM +0100, Paolo Abeni wrote: > That really was meant to be a per netns attribute from the beginning. > > The idea is that once proper isolation is in place in the main > namespace, additional demux in the child namespaces will be redundant. > Let's make child netns default rps mask empty by default. > > To avoid bloating the netns with a possibly large cpumask, allocate > it on-demand during the first write operation. > > Signed-off-by: Paolo Abeni <pabeni@redhat.com> ... > diff --git a/net/core/sysctl_net_core.c b/net/core/sysctl_net_core.c > index 7130e6d9e263..74842b453407 100644 > --- a/net/core/sysctl_net_core.c > +++ b/net/core/sysctl_net_core.c > @@ -74,24 +74,47 @@ static void dump_cpumask(void *buffer, size_t *lenp, loff_t *ppos, > #endif > > #ifdef CONFIG_RPS > -struct cpumask rps_default_mask; > + > +static struct cpumask *rps_default_mask_cow_alloc(struct net *net) > +{ > + struct cpumask *rps_default_mask; > + > + if (net->core.rps_default_mask) > + return net->core.rps_default_mask; > + > + rps_default_mask = kzalloc(cpumask_size(), GFP_KERNEL); > + if (!rps_default_mask) > + return NULL; > + > + /* pairs with READ_ONCE in rx_queue_default_mask() */ > + WRITE_ONCE(net->core.rps_default_mask, rps_default_mask); > + return rps_default_mask; > +} > > static int rps_default_mask_sysctl(struct ctl_table *table, int write, > void *buffer, size_t *lenp, loff_t *ppos) > { > + struct net *net = (struct net *)table->data; > int err = 0; > > rtnl_lock(); > if (write) { > - err = cpumask_parse(buffer, &rps_default_mask); > + struct cpumask *rps_default_mask = rps_default_mask_cow_alloc(net); > + > + err = -ENOMEM; nit: Would it be nicer to set err to -ENOMEM inside the if clause? I think that is the only path where it is used. > + if (!rps_default_mask) > + goto done; > + > + err = cpumask_parse(buffer, rps_default_mask); > if (err) > goto done; > > - err = rps_cpumask_housekeeping(&rps_default_mask); > + err = rps_cpumask_housekeeping(rps_default_mask); > if (err) > goto done; > } else { > - dump_cpumask(buffer, lenp, ppos, &rps_default_mask); > + dump_cpumask(buffer, lenp, ppos, > + net->core.rps_default_mask ? : cpu_none_mask); > } > > done: ... ^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH v2 net-next 2/2] self-tests: more rps self tests 2023-02-17 12:28 [PATCH v2 net-next 0/2] net: default_rps_mask follow-up Paolo Abeni 2023-02-17 12:28 ` [PATCH v2 net-next 1/2] net: make default_rps_mask a per netns attribute Paolo Abeni @ 2023-02-17 12:28 ` Paolo Abeni 1 sibling, 0 replies; 4+ messages in thread From: Paolo Abeni @ 2023-02-17 12:28 UTC (permalink / raw) To: netdev; +Cc: David S. Miller, Jakub Kicinski, Eric Dumazet, Shuah Khan Explicitly check for child netns and main ns independency Signed-off-by: Paolo Abeni <pabeni@redhat.com> --- .../testing/selftests/net/rps_default_mask.sh | 41 +++++++++++++------ 1 file changed, 29 insertions(+), 12 deletions(-) diff --git a/tools/testing/selftests/net/rps_default_mask.sh b/tools/testing/selftests/net/rps_default_mask.sh index c81c0ac7ddfe..0fd0d2db3abc 100755 --- a/tools/testing/selftests/net/rps_default_mask.sh +++ b/tools/testing/selftests/net/rps_default_mask.sh @@ -8,7 +8,9 @@ ret=0 [ $cpus -gt 2 ] || exit $ksft_skip readonly INITIAL_RPS_DEFAULT_MASK=$(cat /proc/sys/net/core/rps_default_mask) -readonly NETNS="ns-$(mktemp -u XXXXXX)" +readonly TAG="$(mktemp -u XXXXXX)" +readonly VETH="veth${TAG}" +readonly NETNS="ns-${TAG}" setup() { ip netns add "${NETNS}" @@ -21,11 +23,15 @@ cleanup() { } chk_rps() { - local rps_mask expected_rps_mask=$3 - local dev_name=$2 + local rps_mask expected_rps_mask=$4 + local dev_name=$3 + local netns=$2 + local cmd="cat" local msg=$1 - rps_mask=$(ip netns exec $NETNS cat /sys/class/net/$dev_name/queues/rx-0/rps_cpus) + [ -n "$netns" ] && cmd="ip netns exec $netns $cmd" + + rps_mask=$($cmd /sys/class/net/$dev_name/queues/rx-0/rps_cpus) printf "%-60s" "$msg" if [ $rps_mask -eq $expected_rps_mask ]; then echo "[ ok ]" @@ -39,19 +45,30 @@ trap cleanup EXIT echo 0 > /proc/sys/net/core/rps_default_mask setup -chk_rps "empty rps_default_mask" lo 0 +chk_rps "empty rps_default_mask" $NETNS lo 0 cleanup echo 1 > /proc/sys/net/core/rps_default_mask setup -chk_rps "non zero rps_default_mask" lo 1 +chk_rps "changing rps_default_mask dont affect existing devices" "" lo $INITIAL_RPS_DEFAULT_MASK echo 3 > /proc/sys/net/core/rps_default_mask -chk_rps "changing rps_default_mask dont affect existing netns" lo 1 +chk_rps "changing rps_default_mask dont affect existing netns" $NETNS lo 0 + +ip link add name $VETH type veth peer netns $NETNS name $VETH +ip link set dev $VETH up +ip -n $NETNS link set dev $VETH up +chk_rps "changing rps_default_mask affect newly created devices" "" $VETH 3 +chk_rps "changing rps_default_mask don't affect newly child netns[II]" $NETNS $VETH 0 +ip netns del $NETNS + +setup +chk_rps "rps_default_mask is 0 by default in child netns" "$NETNS" lo 0 + +ip netns exec $NETNS sysctl -qw net.core.rps_default_mask=1 +ip link add name $VETH type veth peer netns $NETNS name $VETH +chk_rps "changing rps_default_mask in child ns don't affect the main one" "" lo $INITIAL_RPS_DEFAULT_MASK +chk_rps "changing rps_default_mask in child ns affects new childns devices" $NETNS $VETH 1 +chk_rps "changing rps_default_mask in child ns don't affect existing devices" $NETNS lo 0 -ip -n $NETNS link add type veth -ip -n $NETNS link set dev veth0 up -ip -n $NETNS link set dev veth1 up -chk_rps "changing rps_default_mask affect newly created devices" veth0 3 -chk_rps "changing rps_default_mask affect newly created devices[II]" veth1 3 exit $ret -- 2.39.1 ^ permalink raw reply related [flat|nested] 4+ messages in thread
end of thread, other threads:[~2023-02-21 15:45 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2023-02-17 12:28 [PATCH v2 net-next 0/2] net: default_rps_mask follow-up Paolo Abeni 2023-02-17 12:28 ` [PATCH v2 net-next 1/2] net: make default_rps_mask a per netns attribute Paolo Abeni 2023-02-21 15:45 ` Simon Horman 2023-02-17 12:28 ` [PATCH v2 net-next 2/2] self-tests: more rps self tests Paolo Abeni
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).