| Message ID | 1507235025-21689-1-git-send-email-ptalbert@redhat.com |
|---|---|
| State | Rejected, archived |
| Delegated to: | David Miller |
| Headers | show |
| Series | [net-next,1/1,net] bonding: Add NUMA notice | expand |
On Thu, Oct 05, 2017 at 04:23:45PM -0400, Patrick Talbert wrote: > Network performance can suffer when a load balancing bond uses slave > interfaces which are in different NUMA domains. > > This compares the NUMA domain of a newly enslaved interface against any > existing enslaved interfaces and prints a warning if they do not match. Hi Patrick Is there a bonding mode which might actually want to do this? Send on the local domain, unless it is overloaded, in which case send it to the other domain? There is also this talk for netdev: https://netdevconf.org/2.2/session.html?shochat-devicemgmt-talk Andrew
On Thu, Oct 5, 2017 at 5:46 PM, Andrew Lunn <andrew@lunn.ch> wrote: > On Thu, Oct 05, 2017 at 04:23:45PM -0400, Patrick Talbert wrote: >> Network performance can suffer when a load balancing bond uses slave >> interfaces which are in different NUMA domains. >> >> This compares the NUMA domain of a newly enslaved interface against any >> existing enslaved interfaces and prints a warning if they do not match. > > Hi Patrick > > Is there a bonding mode which might actually want to do this? Send on > the local domain, unless it is overloaded, in which case send it to > the other domain? > I suppose there could theoretically be a bonding mode that could do that, but currently no such mode exists. > There is also this talk for netdev: > > https://netdevconf.org/2.2/session.html?shochat-devicemgmt-talk From reading the abstract there, it sounds like such a device driver would want to abstract away the numa location of the underlying devices from the "unified" net device it presents to the kernel. > > Andrew My goal with the patch is not to prevent some one from bonding whichever interfaces they want, only to notify them that what they are doing is *likely* to be less than ideal from a performance perspective. Even if some theoretical load balancing bonding mode was intelligent enough to consider NUMA when choosing a transmit interface, it never has control over the interface traffic is received on (excluding the strange balance-alb mode). Patrick
On Fri, 2017-10-06 at 13:54 -0400, Patrick Talbert wrote: > > My goal with the patch is not to prevent some one from bonding > whichever interfaces they want, only to notify them that what they are > doing is *likely* to be less than ideal from a performance > perspective. Even if some theoretical load balancing bonding mode was > intelligent enough to consider NUMA when choosing a transmit > interface, it never has control over the interface traffic is received > on (excluding the strange balance-alb mode). Note that following the NUMA node of current process would lead to reordering for TCP flows. XPS makes sure we stick to one TXQ to avoid reorders. I am not sure your patch will really help, since you do not warn if a process from another NUMA node tries to send packet to a bonding driver using slaves on another NUMA node.
From: Patrick Talbert <ptalbert@redhat.com> Date: Thu, 5 Oct 2017 16:23:45 -0400 > Network performance can suffer when a load balancing bond uses slave > interfaces which are in different NUMA domains. > > This compares the NUMA domain of a newly enslaved interface against any > existing enslaved interfaces and prints a warning if they do not match. > > Signed-off-by: Patrick Talbert <ptalbert@redhat.com> This is a bit over the top, and doesn't even handle cases where the device has no specific NUMA node (-1).
On Sat, Oct 7, 2017 at 6:20 PM, David Miller <davem@davemloft.net> wrote: > From: Patrick Talbert <ptalbert@redhat.com> > Date: Thu, 5 Oct 2017 16:23:45 -0400 > >> Network performance can suffer when a load balancing bond uses slave >> interfaces which are in different NUMA domains. >> >> This compares the NUMA domain of a newly enslaved interface against any >> existing enslaved interfaces and prints a warning if they do not match. >> >> Signed-off-by: Patrick Talbert <ptalbert@redhat.com> > > This is a bit over the top, and doesn't even handle cases where > the device has no specific NUMA node (-1). Hello David, The motivation was simply to have a notification in place if the interface to-be-enslaved does not match the existing one(s). We have seen performance issues with bonding which were eventually tracked down to this mismatched NUMA domain issue. I thought it was worth having the *mild* warning because it can have a decent impact yet is probably not an obvious thing to check for most users. Though I now see your point. If the bonded interfaces are VLANs, and their underlying physical interfaces happen to be in different NUMA domains, then my check will not know as the VLAN interface numa_node member will be -1 no matter what. That's probably a pretty common setup but adding the logic to check for it probably isn't worth it. Patrick
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c index b19dc03..250a969 100644 --- a/drivers/net/bonding/bond_main.c +++ b/drivers/net/bonding/bond_main.c @@ -55,6 +55,7 @@ #include <asm/dma.h> #include <linux/uaccess.h> #include <linux/errno.h> +#include <linux/device.h> #include <linux/netdevice.h> #include <linux/inetdevice.h> #include <linux/igmp.h> @@ -1450,6 +1451,21 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev) } } + if (bond_has_slaves(bond)) { + struct list_head *iter; + struct slave *slave; + + bond_for_each_slave(bond, slave, iter) { + if (slave_dev->dev.numa_node != + slave->dev->dev.numa_node) { + netdev_warn(bond_dev, + "%s does not match NUMA domain of existing slaves. This could have a performance impact.", + slave_dev->name); + break; + } + } + } + call_netdevice_notifiers(NETDEV_JOIN, slave_dev); /* If this is the first slave, then we need to set the master's hardware
Network performance can suffer when a load balancing bond uses slave interfaces which are in different NUMA domains. This compares the NUMA domain of a newly enslaved interface against any existing enslaved interfaces and prints a warning if they do not match. Signed-off-by: Patrick Talbert <ptalbert@redhat.com> --- :100644 100644 b19dc03... 250a969... M drivers/net/bonding/bond_main.c drivers/net/bonding/bond_main.c | 16 ++++++++++++++++ 1 file changed, 16 insertions(+)