* RE: [Intel-wired-lan] [PATCH net-next V2 2/2] intel: ixgbevf: Support Windows hosts (Hyper-V)
From: KY Srinivasan @ 2016-04-18 19:42 UTC (permalink / raw)
To: Joe Perches, Alexander Duyck
Cc: olaf@aepfle.de, Netdev, Jason Wang, linux-kernel@vger.kernel.org,
jackm@mellanox.com, yevgenyp@mellanox.com, John Ronciak,
intel-wired-lan, eli@mellanox.com, Robo Bot,
devel@linuxdriverproject.org, David Miller
In-Reply-To: <1460998819.1917.10.camel@perches.com>
> -----Original Message-----
> From: Joe Perches [mailto:joe@perches.com]
> Sent: Monday, April 18, 2016 10:00 AM
> To: KY Srinivasan <kys@microsoft.com>; Alexander Duyck
> <alexander.duyck@gmail.com>
> Cc: David Miller <davem@davemloft.net>; Netdev
> <netdev@vger.kernel.org>; linux-kernel@vger.kernel.org;
> devel@linuxdriverproject.org; olaf@aepfle.de; Robo Bot
> <apw@canonical.com>; Jason Wang <jasowang@redhat.com>;
> eli@mellanox.com; jackm@mellanox.com; yevgenyp@mellanox.com; John
> Ronciak <john.ronciak@intel.com>; intel-wired-lan <intel-wired-
> lan@lists.osuosl.org>
> Subject: Re: [Intel-wired-lan] [PATCH net-next V2 2/2] intel: ixgbevf: Support
> Windows hosts (Hyper-V)
>
> On Mon, 2016-04-18 at 16:52 +0000, KY Srinivasan wrote:
> []
> > > > +bool ixgbevf_on_hyperv(struct ixgbe_hw *hw)
> > > > +{
> > > > + if (hw->mbx.ops.check_for_msg == NULL)
> > > > + return true;
> > > > + else
> > > > + return false;
> > > > +}
>
> trivia:
>
> bool func(...)
> {
> if (<bar>)
> return true;
> else
> return false;
> }
>
> can generally be written as:
>
> bool func(...)
> {
> return <bar>;
> }
Thanks Joe; will update.
K. Y
^ permalink raw reply
* [PATCH net-next 7/7] geneve: break dependency with netdev drivers
From: Hannes Frederic Sowa @ 2016-04-18 19:19 UTC (permalink / raw)
To: netdev; +Cc: jesse
In-Reply-To: <1461007188-1603-1-git-send-email-hannes@stressinduktion.org>
Equivalent to "vxlan: break dependency with netdev drivers", don't
autoload geneve module in case the driver is loaded. Instead make the
coupling weaker by using netdevice notifiers as proxy.
Cc: Jesse Gross <jesse@kernel.org>
Signed-off-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
---
drivers/net/geneve.c | 31 ++++++++++++++++++++++++++++---
include/linux/netdevice.h | 1 +
include/net/geneve.h | 6 ++----
3 files changed, 31 insertions(+), 7 deletions(-)
diff --git a/drivers/net/geneve.c b/drivers/net/geneve.c
index efbc7ceedc3a13..59314cf47584e4 100644
--- a/drivers/net/geneve.c
+++ b/drivers/net/geneve.c
@@ -1172,7 +1172,7 @@ static struct device_type geneve_type = {
* supply the listening GENEVE udp ports. Callers are expected
* to implement the ndo_add_geneve_port.
*/
-void geneve_get_rx_port(struct net_device *dev)
+static void geneve_push_rx_ports(struct net_device *dev)
{
struct net *net = dev_net(dev);
struct geneve_net *gn = net_generic(net, geneve_net_id);
@@ -1181,6 +1181,9 @@ void geneve_get_rx_port(struct net_device *dev)
struct sock *sk;
__be16 port;
+ if (!dev->netdev_ops->ndo_add_geneve_port)
+ return;
+
rcu_read_lock();
list_for_each_entry_rcu(gs, &gn->sock_list, list) {
sk = gs->sock->sk;
@@ -1190,7 +1193,6 @@ void geneve_get_rx_port(struct net_device *dev)
}
rcu_read_unlock();
}
-EXPORT_SYMBOL_GPL(geneve_get_rx_port);
/* Initialize the device structure. */
static void geneve_setup(struct net_device *dev)
@@ -1538,6 +1540,21 @@ struct net_device *geneve_dev_create_fb(struct net *net, const char *name,
}
EXPORT_SYMBOL_GPL(geneve_dev_create_fb);
+static int geneve_netdevice_event(struct notifier_block *unused,
+ unsigned long event, void *ptr)
+{
+ struct net_device *dev = netdev_notifier_info_to_dev(ptr);
+
+ if (event == NETDEV_OFFLOAD_PUSH_GENEVE)
+ geneve_push_rx_ports(dev);
+
+ return NOTIFY_DONE;
+}
+
+static struct notifier_block geneve_notifier_block __read_mostly = {
+ .notifier_call = geneve_netdevice_event,
+};
+
static __net_init int geneve_init_net(struct net *net)
{
struct geneve_net *gn = net_generic(net, geneve_net_id);
@@ -1590,11 +1607,18 @@ static int __init geneve_init_module(void)
if (rc)
goto out1;
- rc = rtnl_link_register(&geneve_link_ops);
+ rc = register_netdevice_notifier(&geneve_notifier_block);
if (rc)
goto out2;
+ rc = rtnl_link_register(&geneve_link_ops);
+ if (rc)
+ goto out3;
+
return 0;
+
+out3:
+ unregister_netdevice_notifier(&geneve_notifier_block);
out2:
unregister_pernet_subsys(&geneve_net_ops);
out1:
@@ -1605,6 +1629,7 @@ late_initcall(geneve_init_module);
static void __exit geneve_cleanup_module(void)
{
rtnl_link_unregister(&geneve_link_ops);
+ unregister_netdevice_notifier(&geneve_notifier_block);
unregister_pernet_subsys(&geneve_net_ops);
}
module_exit(geneve_cleanup_module);
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index d4c8cd424f8dbc..1f6d5db471a24f 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -2245,6 +2245,7 @@ struct netdev_lag_lower_state_info {
#define NETDEV_PRECHANGEUPPER 0x001A
#define NETDEV_CHANGELOWERSTATE 0x001B
#define NETDEV_OFFLOAD_PUSH_VXLAN 0x001C
+#define NETDEV_OFFLOAD_PUSH_GENEVE 0x001D
int register_netdevice_notifier(struct notifier_block *nb);
int unregister_netdevice_notifier(struct notifier_block *nb);
diff --git a/include/net/geneve.h b/include/net/geneve.h
index e6c23dc765f7ec..cb544a530146c2 100644
--- a/include/net/geneve.h
+++ b/include/net/geneve.h
@@ -62,13 +62,11 @@ struct genevehdr {
struct geneve_opt options[];
};
-#if IS_ENABLED(CONFIG_GENEVE)
-void geneve_get_rx_port(struct net_device *netdev);
-#else
static inline void geneve_get_rx_port(struct net_device *netdev)
{
+ ASSERT_RTNL();
+ call_netdevice_notifiers(NETDEV_OFFLOAD_PUSH_GENEVE, netdev);
}
-#endif
#ifdef CONFIG_INET
struct net_device *geneve_dev_create_fb(struct net *net, const char *name,
--
2.5.5
^ permalink raw reply related
* [PATCH net-next 6/7] vxlan: break dependency with netdev drivers
From: Hannes Frederic Sowa @ 2016-04-18 19:19 UTC (permalink / raw)
To: netdev; +Cc: jesse
In-Reply-To: <1461007188-1603-1-git-send-email-hannes@stressinduktion.org>
Currently all drivers depend and autoload the vxlan module because how
vxlan_get_rx_port is linked into them. Remove this dependency:
By using a new event type in the netdevice notifier call chain we proxy
the request from the drivers to flush and resetup the vxlan ports not
directly via function call but by the already existing netdevice
notifier call chain.
I added a separate new event type, NETDEV_OFFLOAD_PUSH_VXLAN, to do so.
We don't need to save those ids, as the event type field is an unsigned
long and using specialized event types for this purpose seemed to be a
more elegant way. This also comes in beneficial if in future we want to
add offloading knobs for vxlan.
Cc: Jesse Gross <jesse@kernel.org>
Signed-off-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
---
drivers/net/vxlan.c | 14 +++++++++-----
include/linux/netdevice.h | 1 +
include/net/vxlan.h | 6 ++----
3 files changed, 12 insertions(+), 9 deletions(-)
diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
index c2e22c2532a1a8..6fb93b57a72489 100644
--- a/drivers/net/vxlan.c
+++ b/drivers/net/vxlan.c
@@ -2527,7 +2527,7 @@ static struct device_type vxlan_type = {
* supply the listening VXLAN udp ports. Callers are expected
* to implement the ndo_add_vxlan_port.
*/
-void vxlan_get_rx_port(struct net_device *dev)
+static void vxlan_push_rx_ports(struct net_device *dev)
{
struct vxlan_sock *vs;
struct net *net = dev_net(dev);
@@ -2536,6 +2536,9 @@ void vxlan_get_rx_port(struct net_device *dev)
__be16 port;
unsigned int i;
+ if (!dev->netdev_ops->ndo_add_vxlan_port)
+ return;
+
spin_lock(&vn->sock_lock);
for (i = 0; i < PORT_HASH_SIZE; ++i) {
hlist_for_each_entry_rcu(vs, &vn->sock_list[i], hlist) {
@@ -2547,7 +2550,6 @@ void vxlan_get_rx_port(struct net_device *dev)
}
spin_unlock(&vn->sock_lock);
}
-EXPORT_SYMBOL_GPL(vxlan_get_rx_port);
/* Initialize the device structure. */
static void vxlan_setup(struct net_device *dev)
@@ -3283,20 +3285,22 @@ static void vxlan_handle_lowerdev_unregister(struct vxlan_net *vn,
unregister_netdevice_many(&list_kill);
}
-static int vxlan_lowerdev_event(struct notifier_block *unused,
- unsigned long event, void *ptr)
+static int vxlan_netdevice_event(struct notifier_block *unused,
+ unsigned long event, void *ptr)
{
struct net_device *dev = netdev_notifier_info_to_dev(ptr);
struct vxlan_net *vn = net_generic(dev_net(dev), vxlan_net_id);
if (event == NETDEV_UNREGISTER)
vxlan_handle_lowerdev_unregister(vn, dev);
+ else if (event == NETDEV_OFFLOAD_PUSH_VXLAN)
+ vxlan_push_rx_ports(dev);
return NOTIFY_DONE;
}
static struct notifier_block vxlan_notifier_block __read_mostly = {
- .notifier_call = vxlan_lowerdev_event,
+ .notifier_call = vxlan_netdevice_event,
};
static __net_init int vxlan_init_net(struct net *net)
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index a3bb534576a383..d4c8cd424f8dbc 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -2244,6 +2244,7 @@ struct netdev_lag_lower_state_info {
#define NETDEV_BONDING_INFO 0x0019
#define NETDEV_PRECHANGEUPPER 0x001A
#define NETDEV_CHANGELOWERSTATE 0x001B
+#define NETDEV_OFFLOAD_PUSH_VXLAN 0x001C
int register_netdevice_notifier(struct notifier_block *nb);
int unregister_netdevice_notifier(struct notifier_block *nb);
diff --git a/include/net/vxlan.h b/include/net/vxlan.h
index d442eb3129cde4..673e9f9e6da768 100644
--- a/include/net/vxlan.h
+++ b/include/net/vxlan.h
@@ -390,13 +390,11 @@ static inline __be32 vxlan_compute_rco(unsigned int start, unsigned int offset)
return vni_field;
}
-#if IS_ENABLED(CONFIG_VXLAN)
-void vxlan_get_rx_port(struct net_device *netdev);
-#else
static inline void vxlan_get_rx_port(struct net_device *netdev)
{
+ ASSERT_RTNL();
+ call_netdevice_notifiers(NETDEV_OFFLOAD_PUSH_VXLAN, netdev);
}
-#endif
static inline unsigned short vxlan_get_sk_family(struct vxlan_sock *vs)
{
--
2.5.5
^ permalink raw reply related
* [PATCH net-next 5/7] qlcnic: protect qlicnic_attach_func with rtnl_lock
From: Hannes Frederic Sowa @ 2016-04-18 19:19 UTC (permalink / raw)
To: netdev; +Cc: jesse, Dept-GELinuxNICDev
In-Reply-To: <1461007188-1603-1-git-send-email-hannes@stressinduktion.org>
qlcnic_attach_func requires rtnl_lock to be held.
Cc: Dept-GELinuxNICDev@qlogic.com
Signed-off-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
---
drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c
index 1205f6f9c94173..1c29105b6c364f 100644
--- a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c
+++ b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c
@@ -3952,8 +3952,14 @@ static pci_ers_result_t qlcnic_82xx_io_error_detected(struct pci_dev *pdev,
static pci_ers_result_t qlcnic_82xx_io_slot_reset(struct pci_dev *pdev)
{
- return qlcnic_attach_func(pdev) ? PCI_ERS_RESULT_DISCONNECT :
- PCI_ERS_RESULT_RECOVERED;
+ pci_ers_result_t res;
+
+ rtnl_lock();
+ res = qlcnic_attach_func(pdev) ? PCI_ERS_RESULT_DISCONNECT :
+ PCI_ERS_RESULT_RECOVERED;
+ rtnl_unlock();
+
+ return res;
}
static void qlcnic_82xx_io_resume(struct pci_dev *pdev)
--
2.5.5
^ permalink raw reply related
* [PATCH net-next 4/7] ixgbe: protect vxlan_get_rx_port in ixgbe_service_task with rtnl_lock
From: Hannes Frederic Sowa @ 2016-04-18 19:19 UTC (permalink / raw)
To: netdev
Cc: jesse, Jeff Kirsher, Jesse Brandeburg, Shannon Nelson,
Carolyn Wyborny, Don Skidmore, Bruce Allan, John Ronciak,
Mitch Williams
In-Reply-To: <1461007188-1603-1-git-send-email-hannes@stressinduktion.org>
vxlan_get_rx_port requires rtnl_lock to be held.
Cc: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Cc: Jesse Brandeburg <jesse.brandeburg@intel.com>
Cc: Shannon Nelson <shannon.nelson@intel.com>
Cc: Carolyn Wyborny <carolyn.wyborny@intel.com>
Cc: Don Skidmore <donald.c.skidmore@intel.com>
Cc: Bruce Allan <bruce.w.allan@intel.com>
Cc: John Ronciak <john.ronciak@intel.com>
Cc: Mitch Williams <mitch.a.williams@intel.com>
Signed-off-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
---
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index 2976df77bf14f5..b2f2cf40f06a87 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -7192,10 +7192,12 @@ static void ixgbe_service_task(struct work_struct *work)
return;
}
#ifdef CONFIG_IXGBE_VXLAN
+ rtnl_lock();
if (adapter->flags2 & IXGBE_FLAG2_VXLAN_REREG_NEEDED) {
adapter->flags2 &= ~IXGBE_FLAG2_VXLAN_REREG_NEEDED;
vxlan_get_rx_port(adapter->netdev);
}
+ rtnl_unlock();
#endif /* CONFIG_IXGBE_VXLAN */
ixgbe_reset_subtask(adapter);
ixgbe_phy_interrupt_subtask(adapter);
--
2.5.5
^ permalink raw reply related
* [PATCH net-next 3/7] mlx4: protect mlx4_en_start_port in mlx4_en_restart with rtnl_lock
From: Hannes Frederic Sowa @ 2016-04-18 19:19 UTC (permalink / raw)
To: netdev; +Cc: jesse, Eugenia Emantayev, Yishai Hadas
In-Reply-To: <1461007188-1603-1-git-send-email-hannes@stressinduktion.org>
mlx4_en_start_port requires rtnl_lock to be held.
Cc: Eugenia Emantayev <eugenia@mellanox.com>
Cc: Yishai Hadas <yishaih@mellanox.com>
Signed-off-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
---
drivers/net/ethernet/mellanox/mlx4/en_netdev.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
index b4b258c8ca47d4..8bd143dda95d11 100644
--- a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
+++ b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
@@ -1856,6 +1856,7 @@ static void mlx4_en_restart(struct work_struct *work)
en_dbg(DRV, priv, "Watchdog task called for port %d\n", priv->port);
+ rtnl_lock();
mutex_lock(&mdev->state_lock);
if (priv->port_up) {
mlx4_en_stop_port(dev, 1);
@@ -1863,6 +1864,7 @@ static void mlx4_en_restart(struct work_struct *work)
en_err(priv, "Failed restarting port %d\n", priv->port);
}
mutex_unlock(&mdev->state_lock);
+ rtnl_unlock();
}
static void mlx4_en_clear_stats(struct net_device *dev)
--
2.5.5
^ permalink raw reply related
* [PATCH net-next 2/7] fm10k: protect fm10k_open in fm10k_io_resume with rtnl_lock
From: Hannes Frederic Sowa @ 2016-04-18 19:19 UTC (permalink / raw)
To: netdev
Cc: jesse, Jeff Kirsher, Jesse Brandeburg, Shannon Nelson,
Carolyn Wyborny, Don Skidmore, Bruce Allan, John Ronciak,
Mitch Williams
In-Reply-To: <1461007188-1603-1-git-send-email-hannes@stressinduktion.org>
fm10k_open requires rtnl_lock to be held.
Cc: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Cc: Jesse Brandeburg <jesse.brandeburg@intel.com>
Cc: Shannon Nelson <shannon.nelson@intel.com>
Cc: Carolyn Wyborny <carolyn.wyborny@intel.com>
Cc: Don Skidmore <donald.c.skidmore@intel.com>
Cc: Bruce Allan <bruce.w.allan@intel.com>
Cc: John Ronciak <john.ronciak@intel.com>
Cc: Mitch Williams <mitch.a.williams@intel.com>
Signed-off-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
---
drivers/net/ethernet/intel/fm10k/fm10k_pci.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/net/ethernet/intel/fm10k/fm10k_pci.c b/drivers/net/ethernet/intel/fm10k/fm10k_pci.c
index f0992950e228eb..04304d9a633935 100644
--- a/drivers/net/ethernet/intel/fm10k/fm10k_pci.c
+++ b/drivers/net/ethernet/intel/fm10k/fm10k_pci.c
@@ -2362,8 +2362,10 @@ static void fm10k_io_resume(struct pci_dev *pdev)
/* reset clock */
fm10k_ts_reset(interface);
+ rtnl_lock();
if (netif_running(netdev))
err = fm10k_open(netdev);
+ rtnl_unlock();
/* final check of hardware state before registering the interface */
err = err ? : fm10k_hw_ready(interface);
--
2.5.5
^ permalink raw reply related
* [PATCH net-next 1/7] benet: be_resume needs to protect be_open with rtnl_lock
From: Hannes Frederic Sowa @ 2016-04-18 19:19 UTC (permalink / raw)
To: netdev
Cc: jesse, Sathya Perla, Ajit Khaparde, Padmanabh Ratnakar,
Sriharsha Basavapatna, Somnath Kotur
In-Reply-To: <1461007188-1603-1-git-send-email-hannes@stressinduktion.org>
be_open calls down to functions which expects rtnl lock to be held.
Cc: Sathya Perla <sathya.perla@broadcom.com>
Cc: Ajit Khaparde <ajit.khaparde@broadcom.com>
Cc: Padmanabh Ratnakar <padmanabh.ratnakar@broadcom.com>
Cc: Sriharsha Basavapatna <sriharsha.basavapatna@broadcom.com>
Cc: Somnath Kotur <somnath.kotur@broadcom.com>
Signed-off-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
---
drivers/net/ethernet/emulex/benet/be_main.c | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/emulex/benet/be_main.c b/drivers/net/ethernet/emulex/benet/be_main.c
index 536686476369bf..ed98ef1ecac38d 100644
--- a/drivers/net/ethernet/emulex/benet/be_main.c
+++ b/drivers/net/ethernet/emulex/benet/be_main.c
@@ -4890,11 +4890,13 @@ static int be_resume(struct be_adapter *adapter)
if (status)
return status;
- if (netif_running(netdev)) {
+ rtnl_lock();
+ if (netif_running(netdev))
status = be_open(netdev);
- if (status)
- return status;
- }
+ rtnl_unlock();
+
+ if (status)
+ return status;
netif_device_attach(netdev);
--
2.5.5
^ permalink raw reply related
* [PATCH net-next 0/7] net: network drivers should not depend on geneve/vxlan
From: Hannes Frederic Sowa @ 2016-04-18 19:19 UTC (permalink / raw)
To: netdev; +Cc: jesse
This patchset removes the dependency of network drivers on vxlan or
geneve, so those don't get autoloaded when the nic driver is loaded.
Also audited the code such that vxlan_get_rx_port and geneve_get_rx_port
are not called without rtnl lock.
Hannes Frederic Sowa (7):
benet: be_resume needs to protect be_open with rtnl_lock
fm10k: protect fm10k_open in fm10k_io_resume with rtnl_lock
mlx4: protect mlx4_en_start_port in mlx4_en_restart with rtnl_lock
ixgbe: protect vxlan_get_rx_port in ixgbe_service_task with rtnl_lock
qlcnic: protect qlicnic_attach_func with rtnl_lock
vxlan: break dependency with netdev drivers
geneve: break dependency with netdev drivers
drivers/net/ethernet/emulex/benet/be_main.c | 10 +++++---
drivers/net/ethernet/intel/fm10k/fm10k_pci.c | 2 ++
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 2 ++
drivers/net/ethernet/mellanox/mlx4/en_netdev.c | 2 ++
drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c | 10 ++++++--
drivers/net/geneve.c | 31 +++++++++++++++++++++---
drivers/net/vxlan.c | 14 +++++++----
include/linux/netdevice.h | 2 ++
include/net/geneve.h | 6 ++---
include/net/vxlan.h | 6 ++---
10 files changed, 63 insertions(+), 22 deletions(-)
--
2.5.5
^ permalink raw reply
* [PATCH net-next v2 2/2] bpf: add event output helper for notifications/sampling/logging
From: Daniel Borkmann @ 2016-04-18 19:01 UTC (permalink / raw)
To: davem
Cc: alexei.starovoitov, tgraf, netdev, linux-kernel, Daniel Borkmann,
Alexei Starovoitov
In-Reply-To: <cover.1461005139.git.daniel@iogearbox.net>
This patch adds a new helper for cls/act programs that can push events
to user space applications. For networking, this can be f.e. for sampling,
debugging, logging purposes or pushing of arbitrary wake-up events. The
idea is similar to a43eec304259 ("bpf: introduce bpf_perf_event_output()
helper") and 39111695b1b8 ("samples: bpf: add bpf_perf_event_output example").
The eBPF program utilizes a perf event array map that user space populates
with fds from perf_event_open(), the eBPF program calls into the helper
f.e. as skb_event_output(skb, &my_map, BPF_F_CURRENT_CPU, raw, sizeof(raw))
so that the raw data is pushed into the fd f.e. at the map index of the
current CPU.
User space can poll/mmap/etc on this and has a data channel for receiving
events that can be post-processed. The nice thing is that since the eBPF
program and user space application making use of it are tightly coupled,
they can define their own arbitrary raw data format and what/when they
want to push.
While f.e. packet headers could be one part of the meta data that is being
pushed, this is not a substitute for things like packet sockets as whole
packet is not being pushed and push is only done in a single direction.
Intention is more of a generically usable, efficient event pipe to applications.
Workflow is that tc can pin the map and applications can attach themselves
e.g. after cls/act setup to one or multiple map slots, demuxing is done by
the eBPF program.
Adding this facility is with minimal effort, it reuses the helper
introduced in a43eec304259 ("bpf: introduce bpf_perf_event_output() helper")
and we get its functionality for free by overloading its BPF_FUNC_ identifier
for cls/act programs, ctx is currently unused, but will be made use of in
future. Example will be added to iproute2's BPF example files.
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
---
include/linux/bpf.h | 2 ++
kernel/bpf/core.c | 7 +++++++
kernel/trace/bpf_trace.c | 27 +++++++++++++++++++++++++++
net/core/filter.c | 2 ++
4 files changed, 38 insertions(+)
diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index 5fb3c61..f63afdc 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -169,7 +169,9 @@ u64 bpf_tail_call(u64 ctx, u64 r2, u64 index, u64 r4, u64 r5);
u64 bpf_get_stackid(u64 r1, u64 r2, u64 r3, u64 r4, u64 r5);
void bpf_fd_array_map_clear(struct bpf_map *map);
bool bpf_prog_array_compatible(struct bpf_array *array, const struct bpf_prog *fp);
+
const struct bpf_func_proto *bpf_get_trace_printk_proto(void);
+const struct bpf_func_proto *bpf_get_event_output_proto(void);
#ifdef CONFIG_BPF_SYSCALL
DECLARE_PER_CPU(int, bpf_prog_active);
diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
index be0abf6..e4248fe 100644
--- a/kernel/bpf/core.c
+++ b/kernel/bpf/core.c
@@ -764,14 +764,21 @@ const struct bpf_func_proto bpf_map_delete_elem_proto __weak;
const struct bpf_func_proto bpf_get_prandom_u32_proto __weak;
const struct bpf_func_proto bpf_get_smp_processor_id_proto __weak;
const struct bpf_func_proto bpf_ktime_get_ns_proto __weak;
+
const struct bpf_func_proto bpf_get_current_pid_tgid_proto __weak;
const struct bpf_func_proto bpf_get_current_uid_gid_proto __weak;
const struct bpf_func_proto bpf_get_current_comm_proto __weak;
+
const struct bpf_func_proto * __weak bpf_get_trace_printk_proto(void)
{
return NULL;
}
+const struct bpf_func_proto * __weak bpf_get_event_output_proto(void)
+{
+ return NULL;
+}
+
/* Always built-in helper functions. */
const struct bpf_func_proto bpf_tail_call_proto = {
.func = NULL,
diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
index 6bfe55c..75cb154 100644
--- a/kernel/trace/bpf_trace.c
+++ b/kernel/trace/bpf_trace.c
@@ -277,6 +277,33 @@ static const struct bpf_func_proto bpf_perf_event_output_proto = {
.arg5_type = ARG_CONST_STACK_SIZE,
};
+static DEFINE_PER_CPU(struct pt_regs, bpf_pt_regs);
+
+static u64 bpf_event_output(u64 r1, u64 r2, u64 flags, u64 r4, u64 size)
+{
+ struct pt_regs *regs = this_cpu_ptr(&bpf_pt_regs);
+
+ perf_fetch_caller_regs(regs);
+
+ return bpf_perf_event_output((long)regs, r2, flags, r4, size);
+}
+
+static const struct bpf_func_proto bpf_event_output_proto = {
+ .func = bpf_event_output,
+ .gpl_only = true,
+ .ret_type = RET_INTEGER,
+ .arg1_type = ARG_PTR_TO_CTX,
+ .arg2_type = ARG_CONST_MAP_PTR,
+ .arg3_type = ARG_ANYTHING,
+ .arg4_type = ARG_PTR_TO_STACK,
+ .arg5_type = ARG_CONST_STACK_SIZE,
+};
+
+const struct bpf_func_proto *bpf_get_event_output_proto(void)
+{
+ return &bpf_event_output_proto;
+}
+
static const struct bpf_func_proto *tracing_func_proto(enum bpf_func_id func_id)
{
switch (func_id) {
diff --git a/net/core/filter.c b/net/core/filter.c
index 5d2ac2b..218e5de 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -2039,6 +2039,8 @@ tc_cls_act_func_proto(enum bpf_func_id func_id)
return &bpf_redirect_proto;
case BPF_FUNC_get_route_realm:
return &bpf_get_route_realm_proto;
+ case BPF_FUNC_perf_event_output:
+ return bpf_get_event_output_proto();
default:
return sk_filter_func_proto(func_id);
}
--
1.9.3
^ permalink raw reply related
* [PATCH net-next v2 1/2] bpf, trace: add BPF_F_CURRENT_CPU flag for bpf_perf_event_output
From: Daniel Borkmann @ 2016-04-18 19:01 UTC (permalink / raw)
To: davem
Cc: alexei.starovoitov, tgraf, netdev, linux-kernel, Daniel Borkmann,
Alexei Starovoitov
In-Reply-To: <cover.1461005139.git.daniel@iogearbox.net>
Add a BPF_F_CURRENT_CPU flag to optimize the use-case where user space has
per-CPU ring buffers and the eBPF program pushes the data into the current
CPU's ring buffer which saves us an extra helper function call in eBPF.
Also, make sure to properly reserve the remaining flags which are not used.
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
---
include/uapi/linux/bpf.h | 4 ++++
kernel/trace/bpf_trace.c | 7 ++++++-
2 files changed, 10 insertions(+), 1 deletion(-)
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index 70eda5a..b7b0fb1 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -347,6 +347,10 @@ enum bpf_func_id {
#define BPF_F_ZERO_CSUM_TX (1ULL << 1)
#define BPF_F_DONT_FRAGMENT (1ULL << 2)
+/* BPF_FUNC_perf_event_output flags. */
+#define BPF_F_INDEX_MASK 0xffffffffULL
+#define BPF_F_CURRENT_CPU BPF_F_INDEX_MASK
+
/* user accessible mirror of in-kernel sk_buff.
* new fields can only be added to the end of this structure
*/
diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
index 6855878..6bfe55c 100644
--- a/kernel/trace/bpf_trace.c
+++ b/kernel/trace/bpf_trace.c
@@ -225,11 +225,12 @@ static const struct bpf_func_proto bpf_perf_event_read_proto = {
.arg2_type = ARG_ANYTHING,
};
-static u64 bpf_perf_event_output(u64 r1, u64 r2, u64 index, u64 r4, u64 size)
+static u64 bpf_perf_event_output(u64 r1, u64 r2, u64 flags, u64 r4, u64 size)
{
struct pt_regs *regs = (struct pt_regs *) (long) r1;
struct bpf_map *map = (struct bpf_map *) (long) r2;
struct bpf_array *array = container_of(map, struct bpf_array, map);
+ u64 index = flags & BPF_F_INDEX_MASK;
void *data = (void *) (long) r4;
struct perf_sample_data sample_data;
struct perf_event *event;
@@ -239,6 +240,10 @@ static u64 bpf_perf_event_output(u64 r1, u64 r2, u64 index, u64 r4, u64 size)
.data = data,
};
+ if (unlikely(flags & ~(BPF_F_INDEX_MASK)))
+ return -EINVAL;
+ if (index == BPF_F_CURRENT_CPU)
+ index = raw_smp_processor_id();
if (unlikely(index >= array->map.max_entries))
return -E2BIG;
--
1.9.3
^ permalink raw reply related
* [PATCH net-next v2 0/2] BPF updates
From: Daniel Borkmann @ 2016-04-18 19:01 UTC (permalink / raw)
To: davem; +Cc: alexei.starovoitov, tgraf, netdev, linux-kernel, Daniel Borkmann
This minor set adds a new helper bpf_event_output() for eBPF cls/act
program types which allows to pass events to user space applications.
For details, please see individual patches.
Thanks!
v1 -> v2:
- Address kbuild bot found compile issue in patch 2
- Rest as is
Daniel Borkmann (2):
bpf, trace: add BPF_F_CURRENT_CPU flag for bpf_perf_event_output
bpf: add event output helper for notifications/sampling/logging
include/linux/bpf.h | 2 ++
include/uapi/linux/bpf.h | 4 ++++
kernel/bpf/core.c | 7 +++++++
kernel/trace/bpf_trace.c | 34 +++++++++++++++++++++++++++++++++-
net/core/filter.c | 2 ++
5 files changed, 48 insertions(+), 1 deletion(-)
--
1.9.3
^ permalink raw reply
* Re: [PATCH net-next] macvlan: fix failure during registration
From: Eric W. Biederman @ 2016-04-18 18:48 UTC (permalink / raw)
To: Francesco Ruggeri; +Cc: netdev, David S. Miller, Mahesh Bandewar
In-Reply-To: <1460992811-46992-1-git-send-email-fruggeri@arista.com>
Francesco Ruggeri <fruggeri@arista.com> writes:
> Resending, did not include netdev the first time ...
>
> If a macvlan/macvtap creation fails in register_netdevice in
> call_netdevice_notifiers(NETDEV_REGISTER) then while cleaning things up in
> rollback_registered_many it invokes macvlan_uninit. This results in
> port->count being decremented twice (in macvlan_uninit and in
> macvlan_common_newlink).
> A similar problem may exist in the ipvlan driver.
> This patch adds priv_flags to struct macvlan_dev and a flag that
> macvlan_uninit can use to determine if it is invoked in the context of a
> failed newlink.
> In macvtap_device_event(NETDEV_UNREGISTER) it also avoids cleaning up
> /dev/tapNN in case creation of the char device had previously failed.
> Tested in 3.18.
These interactions all seem a little bit funny. At a quick skim it
would make more sense to increment the port count in macvlan_init,
and completely remove the need to mess with port counts anywhere except
macvlan_init and macvlan_uninit.
If for some reason that can't be done the code can easily look at
dev->reg_state. If dev->reg_state == NETREG_UNITIALIZED it should
be exactly the same as your new flag being set.
> diff --git a/include/linux/if_macvlan.h b/include/linux/if_macvlan.h
> index a4ccc31..7cf82d8 100644
> --- a/include/linux/if_macvlan.h
> +++ b/include/linux/if_macvlan.h
> @@ -48,6 +48,7 @@ struct macvlan_dev {
> netdev_features_t set_features;
> enum macvlan_mode mode;
> u16 flags;
> + u16 priv_flags;
> /* This array tracks active taps. */
> struct macvtap_queue __rcu *taps[MAX_MACVTAP_QUEUES];
> /* This list tracks all taps (both enabled and disabled) */
> @@ -63,6 +64,8 @@ struct macvlan_dev {
> unsigned int macaddr_count;
> };
>
> +#define MACVLAN_PRIV_FLAG_REGISTERING 1
> +
> static inline void macvlan_count_rx(const struct macvlan_dev *vlan,
> unsigned int len, bool success,
> bool multicast)
^ permalink raw reply
* Re: [PATCH net-next] enic: set netdev->vlan_features
From: David Miller @ 2016-04-18 18:53 UTC (permalink / raw)
To: gvaradar; +Cc: netdev, benve, mkubecek, _govind
In-Reply-To: <1460747443-14993-1-git-send-email-gvaradar@cisco.com>
From: Govindarajulu Varadarajan <gvaradar@cisco.com>
Date: Sat, 16 Apr 2016 00:40:43 +0530
> From: Govindarajulu Varadarajan <_govind@gmx.com>
>
> Driver sets vlan_feature to netdev->features as hardware supports all of
> them on vlan interface.
>
> Signed-off-by: Govindarajulu Varadarajan <_govind@gmx.com>
Applied.
^ permalink raw reply
* Re: [PATCH 0/3] fec: ethtool: move to new api {get|set}_link_ksettings
From: David Miller @ 2016-04-18 18:45 UTC (permalink / raw)
To: tremyfr; +Cc: decot, f.fainelli, fugang.duan, linux-kernel, netdev
In-Reply-To: <1460673301-10599-1-git-send-email-tremyfr@gmail.com>
From: Philippe Reynes <tremyfr@gmail.com>
Date: Fri, 15 Apr 2016 00:34:58 +0200
> Ethtool has a new api {get|set}_link_ksettings that deprecate
> the old api {get|set}_settings. We update the fec driver to use
> this new ethtool api.
>
> For this first version, I've converted old u32 value in phy structure
> to link_modes structure. Another way would be to replace u32 in
> phy structure to use DECLARE_LINK_MODE_MASK for advertising, ....
Series applied, thanks.
^ permalink raw reply
* Re: [PATCH] netfilter: ctnetlink: add more #ifdef around unused code
From: Pablo Neira Ayuso @ 2016-04-18 18:43 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Patrick McHardy, Jozsef Kadlecsik, David S. Miller,
Daniel Borkmann, Ken-ichirou MATSUZAWA, netfilter-devel, coreteam,
netdev, linux-kernel
In-Reply-To: <4187960.DvZepuNvUx@wuerfel>
On Mon, Apr 18, 2016 at 08:33:15PM +0200, Arnd Bergmann wrote:
> On Monday 18 April 2016 20:16:59 Pablo Neira Ayuso wrote:
> > On Sat, Apr 16, 2016 at 10:17:43PM +0200, Arnd Bergmann wrote:
> > > A recent patch removed many 'inline' annotations for static
> > > functions in this file, which has caused warnings for functions
> > > that are not used in a given configuration, in particular when
> > > CONFIG_NF_CONNTRACK_EVENTS is disabled:
> > >
> > > nf_conntrack_netlink.c:572:15: 'ctnetlink_timestamp_size' defined but not used
> > > nf_conntrack_netlink.c:546:15: 'ctnetlink_acct_size' defined but not used
> > > nf_conntrack_netlink.c:339:12: 'ctnetlink_label_size' defined but not used
> >
> > Arnd, thanks for the fix.
> >
> > I'm planning to push this though:
> >
> > http://patchwork.ozlabs.org/patch/610820/
> >
> > This is restoring the inlines for the size calculation functions, but
> > I think that's ok. They are rather small and they're called from the
> > event notification path (ie. packet path), so the compiler just place
> > them out of the way when not needed and we calm down the gcc warning.
>
> Looks good. I'll put this in my randconfig builder to replace my own
> patch and will let you know if you missed something.
Thanks, will wait for your ack then.
^ permalink raw reply
* Re: [PATCH net-next] tun: don't require serialization lock on tx
From: David Miller @ 2016-04-18 18:36 UTC (permalink / raw)
To: pabeni; +Cc: netdev, mst, hannes, ebiederm, gkurz, jasowang, edumazet
In-Reply-To: <cc7ac4631f0a32c4d61b73f4f28b52a05ab8651d.1460651429.git.pabeni@redhat.com>
From: Paolo Abeni <pabeni@redhat.com>
Date: Thu, 14 Apr 2016 18:39:39 +0200
> The current tun_net_xmit() implementation don't need any external
> lock since it relies on rcu protection for the tun data structure
> and on socket queue lock for skb queuing.
>
> This patch set the NETIF_F_LLTX feature bit in the tun device, so
> that on xmit, in absence of qdisc, no serialization lock is acquired
> by the caller.
>
> The user space can remove the default tun qdisc with:
>
> tc qdisc replace dev <tun device name> root noqueue
>
> Signed-off-by: Paolo Abeni <pabeni@redhat.com>
> Acked-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
> Acked-by: Eric Dumazet <edumazet@google.com>
Applied, thanks.
^ permalink raw reply
* Re: [PATCH nf] netfilter: ipv6: Orphan skbs in nf_ct_frag6_gather()
From: Pablo Neira Ayuso @ 2016-04-18 18:35 UTC (permalink / raw)
To: Joe Stringer
Cc: Florian Westphal, David Laight, netfilter-devel@vger.kernel.org,
diproiettod@vmware.com, netdev@vger.kernel.org
In-Reply-To: <CAPWQB7EV-boR1_yaBeY8tQtHgc09+14jQ_q7yk=bpwrtqL+20Q@mail.gmail.com>
On Thu, Apr 14, 2016 at 05:35:39PM -0700, Joe Stringer wrote:
> On 14 April 2016 at 03:35, Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> > On Thu, Apr 14, 2016 at 10:40:15AM +0200, Florian Westphal wrote:
> >> David Laight <David.Laight@ACULAB.COM> wrote:
> >> > From: Joe Stringer
> >> > > Sent: 13 April 2016 19:10
> >> > > This is the IPv6 equivalent of commit 8282f27449bf ("inet: frag: Always
> >> > > orphan skbs inside ip_defrag()").
> >> > >
> >> > > Prior to commit 029f7f3b8701 ("netfilter: ipv6: nf_defrag: avoid/free
> >> > > clone operations"), ipv6 fragments sent to nf_ct_frag6_gather() would be
> >> > > cloned (implicitly orphaning) prior to queueing for reassembly. As such,
> >> > > when the IPv6 message is eventually reassembled, the skb->sk for all
> >> > > fragments would be NULL. After that commit was introduced, rather than
> >> > > cloning, the original skbs were queued directly without orphaning. The
> >> > > end result is that all frags except for the first and last may have a
> >> > > socket attached.
> >> >
> >> > I'd have thought that the queued fragments would still want to be
> >> > resource-counted against the socket (I think that is what skb->sk is for).
> >>
> >> No, ipv4/ipv6 reasm has its own accouting.
> >>
> >> > Although I can't imagine why IPv6 reassembly is happening on skb
> >> > associated with a socket.
> >>
> >> Right, thats a much more interesting question -- both ipv4 and
> >> ipv6 orphan skbs before NF_HOOK prerouting trip.
> >>
> >> (That being said, I don't mind the patch, I'm just be curious how this
> >> can happen).
> >
> > If this change is specific to get this working in ovs and its
> > conntrack support, then I don't think this belong to core
> > infrastructure. This should be fixed in ovs instead.
>
> I admit I've only been able to reproduce it with OVS. My main reason
> for proposing the fix this way was just because this is what the IPv4
> code does, so I figured IPv6 should be consistent with that.
You mean that this is what you did in 029f7f3b8701 to fix this, right?
But we shouldn't add code to the core that is OVS specific for no
reason. We don't need this orphan from ipv4 and ipv6 as Florian
indicated.
Is there any chance you can fix this from OVS and its conntrack glue
code? Thanks.
^ permalink raw reply
* Re: [PATCH] netfilter: ctnetlink: add more #ifdef around unused code
From: Arnd Bergmann @ 2016-04-18 18:33 UTC (permalink / raw)
To: Pablo Neira Ayuso
Cc: Patrick McHardy, Jozsef Kadlecsik, David S. Miller,
Daniel Borkmann, Ken-ichirou MATSUZAWA, netfilter-devel, coreteam,
netdev, linux-kernel
In-Reply-To: <20160418181659.GA2427@salvia>
On Monday 18 April 2016 20:16:59 Pablo Neira Ayuso wrote:
> On Sat, Apr 16, 2016 at 10:17:43PM +0200, Arnd Bergmann wrote:
> > A recent patch removed many 'inline' annotations for static
> > functions in this file, which has caused warnings for functions
> > that are not used in a given configuration, in particular when
> > CONFIG_NF_CONNTRACK_EVENTS is disabled:
> >
> > nf_conntrack_netlink.c:572:15: 'ctnetlink_timestamp_size' defined but not used
> > nf_conntrack_netlink.c:546:15: 'ctnetlink_acct_size' defined but not used
> > nf_conntrack_netlink.c:339:12: 'ctnetlink_label_size' defined but not used
>
> Arnd, thanks for the fix.
>
> I'm planning to push this though:
>
> http://patchwork.ozlabs.org/patch/610820/
>
> This is restoring the inlines for the size calculation functions, but
> I think that's ok. They are rather small and they're called from the
> event notification path (ie. packet path), so the compiler just place
> them out of the way when not needed and we calm down the gcc warning.
Looks good. I'll put this in my randconfig builder to replace my own
patch and will let you know if you missed something.
Arnd
^ permalink raw reply
* Re: Poorer networking performance in later kernels?
From: Rick Jones @ 2016-04-18 18:22 UTC (permalink / raw)
To: Butler, Peter, netdev@vger.kernel.org
In-Reply-To: <BY2PR0301MB1990F3095CFA07EBBE600F9DD66B0@BY2PR0301MB1990.namprd03.prod.outlook.com>
On 04/18/2016 04:27 AM, Butler, Peter wrote:
> Hi Rick
>
> Thanks for the reply.
>
> Here is some hardware information, as requested (the two systems are
> identical, and are communicating with one another over a 10GB
> full-duplex Ethernet backplane):
>
> - processor type: Intel(R) Xeon(R) CPU C5528 @ 2.13GHz
> - NIC: Intel 82599EB 10GB XAUI/BX4
> - NIC driver: ixgbe version 4.2.1-k (part of 4.4.0 kernel)
>
> As for the buffer sizes, those rather large ones work fine for us
> with the 3.4.2 kernel. However, for the sake of being complete, I
> have re-tried the tests with the 'standard' 4.4.0 kernel parameters
> for all /proc/sys/net/* values, and the results still were extremely
> poor in comparison to the 3.4.2 kernel.
>
> Our MTU is actually just the standard 1500 bytes, however the message
> size was chosen to mimic actual traffic which will be segmented.
>
> I ran ethtool -k (indeed I checked all ethtool parameters, not just
> those via -k) and the only real difference I could find was in
> "large-receive-offload" which was ON in 3.4.2 but OFF in 4.4.0 - so I
> used ethtool to change this to match the 3.4.2 settings and re-ran
> the tests. Didn't help :-( It's possible of course that I have
> missed a parameter here or there in comparing the 3.4.2 setup to the
> 4.4.0 setup. I also tried running the ethtool config with the latest
> and greatest ethtool version (4.5) on the 4.4.0 kernel, as compared
> to the old 3.1 version on our 3.4.2 kernel.
So it would seem the stateless offloads are still enabled. My next
question would be to wonder if they are still "effective." To that end,
you could run a netperf test specifying a particular port number in the
test-specific portion:
netperf ... -- -P ,12345
and while that is running something like
tcpdump -s 96 -c 200000 -w /tmp/foo.pcap -i <interface> port 12345
then post-processed with the likes of:
tcpdump -n -r /tmp/foo.pcap | grep -v "length 0" | awk '{sum +=
$NF}END{print "average",sum/NR}'
the intent behind that is to see what the average post-GRO segment size
happens to be on the receiver and then to compare it between the two
kernels. Grepping-away the "length 0" is to avoid counting ACKs and
look only at data segments. The specific port number is to avoid
including any other connections which might happen to have traffic
passing through at the time.
You could I suspect do the same comparison on the sending side.
There might I suppose be an easier way to get the average segment size -
perhaps something from looking at ethtool stats - but the stone knives
and bear skins of tcpdump above would have the added benefit of having a
packet trace or three for someone to look at if they felt the need. And
for that, I would actually suggest starting the capture *before* the
netperf test so the connection establishment is included.
> I performed the TCP_RR test as requested and in that case, the
> results are much more comparable. The old kernel is still better,
> but now only around 10% better as opposed to 2-3x better.
Did the service demand change by 10% or just the transaction rate?
> However I still contend that the *_STREAM tests are giving us more
> pertinent data, since our product application is only getting 1/3 to
> 1/2 half of the performance on the 4.4.0 kernel, and this is the same
> thing I see when I use netperf to test.
>
> One other note: I tried running our 3.4.2 and 4.4.0 kernels in a VM
> environment on my workstation, so as to take the 'real' production
> hardware out of the equation. When I perform the tests in this setup
> the 3.4.2 and 4.4.0 kernels perform identically - just as you would
> expect.
Running in a VM will likely change things massively and could I suppose
mask other behaviour changes.
happy benchmarking,
rick jones
raj@tardy:~$ cat signatures/toppost
A: Because it fouls the order in which people normally read text.
Q: Why is top-posting such a bad thing?
A: Top-posting.
Q: What is the most annoying thing on usenet and in e-mail?
:)
>
> Any other ideas? What can I be missing here?
>
> Peter
>
>
>
>
> -----Original Message-----
> From: Rick Jones [mailto:rick.jones2@hpe.com]
> Sent: April-15-16 6:37 PM
> To: Butler, Peter <pbutler@sonusnet.com>; netdev@vger.kernel.org
> Subject: Re: Poorer networking performance in later kernels?
>
> On 04/15/2016 02:02 PM, Butler, Peter wrote:
>> (Please keep me CC'd to all comments/responses)
>>
>> I've tried a kernel upgrade from 3.4.2 to 4.4.0 and see a marked drop
>> in networking performance. Nothing was changed on the test systems,
>> other than the kernel itself (and kernel modules). The identical
>> .config used to build the 3.4.2 kernel was brought over into the
>> 4.4.0 kernel source tree, and any configuration differences (e.g. new
>> parameters, etc.) were taken as default values.
>>
>> The testing was performed on the same actual hardware for both kernel
>> versions (i.e. take the existing 3.4.2 physical setup, simply boot
>> into the (new) kernel and run the same test). The netperf utility was
>> used for benchmarking and the testing was always performed on idle
>> systems.
>>
>> TCP testing yielded the following results, where the 4.4.0 kernel only
>> got about 1/2 of the throughput:
>>
>
>> Recv Send Send Utilization Service Demand
>> Socket Socket Message Elapsed Send Recv Send Recv
>> Size Size Size Time Throughput local remote local remote
>> bytes bytes bytes secs. 10^6bits/s % S % S us/KB us/KB
>>
>> 3.4.2 13631488 13631488 8952 30.01 9370.29 10.14 6.50 0.709 0.454
>> 4.4.0 13631488 13631488 8952 30.02 5314.03 9.14 14.31 1.127 1.765
>>
>> SCTP testing yielded the following results, where the 4.4.0 kernel only got about 1/3 of the throughput:
>>
>> Recv Send Send Utilization Service Demand
>> Socket Socket Message Elapsed Send Recv Send Recv
>> Size Size Size Time Throughput local remote local remote
>> bytes bytes bytes secs. 10^6bits/s % S % S us/KB us/KB
>>
>> 3.4.2 13631488 13631488 8952 30.00 2306.22 13.87 13.19 3.941 3.747
>> 4.4.0 13631488 13631488 8952 30.01 882.74 16.86 19.14 12.516 14.210
>>
>> The same tests were performed a multitude of time, and are always
>> consistent (within a few percent). I've also tried playing with
>> various run-time kernel parameters (/proc/sys/kernel/net/...) on the
>> 4.4.0 kernel to alleviate the issue but have had no success at all.
>>
>> I'm at a loss as to what could possibly account for such a discrepancy...
>>
>
> I suspect I am not alone in being curious about the CPU(s) present in the systems and the model/whatnot of the NIC being used. I'm also curious as to why you have what at first glance seem like absurdly large socket buffer sizes.
>
> That said, it looks like you have some Really Big (tm) increases in service demand. Many more CPU cycles being consumed per KB of data transferred.
>
> Your message size makes me wonder if you were using a 9000 byte MTU.
>
> Perhaps in the move from 3.4.2 to 4.4.0 you lost some or all of the stateless offloads for your NIC(s)? Running ethtool -k <interface> on both ends under both kernels might be good.
>
> Also, if you did have a 9000 byte MTU under 3.4.2 are you certain you still had it under 4.4.0?
>
> It would (at least to me) also be interesting to run a TCP_RR test comparing the two kernels. TCP_RR (at least with the default request/response size of one byte) doesn't really care about stateless offloads or MTUs and could show how much difference there is in basic path length (or I suppose in interrupt coalescing behaviour if the NIC in question has a mildly dodgy heuristic for such things).
>
> happy benchmarking,
>
> rick jones
>
^ permalink raw reply
* Re: [PATCH] netfilter: ctnetlink: add more #ifdef around unused code
From: Pablo Neira Ayuso @ 2016-04-18 18:16 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Patrick McHardy, Jozsef Kadlecsik, David S. Miller,
Daniel Borkmann, Ken-ichirou MATSUZAWA, netfilter-devel, coreteam,
netdev, linux-kernel
In-Reply-To: <1460837916-1241019-1-git-send-email-arnd@arndb.de>
On Sat, Apr 16, 2016 at 10:17:43PM +0200, Arnd Bergmann wrote:
> A recent patch removed many 'inline' annotations for static
> functions in this file, which has caused warnings for functions
> that are not used in a given configuration, in particular when
> CONFIG_NF_CONNTRACK_EVENTS is disabled:
>
> nf_conntrack_netlink.c:572:15: 'ctnetlink_timestamp_size' defined but not used
> nf_conntrack_netlink.c:546:15: 'ctnetlink_acct_size' defined but not used
> nf_conntrack_netlink.c:339:12: 'ctnetlink_label_size' defined but not used
Arnd, thanks for the fix.
I'm planning to push this though:
http://patchwork.ozlabs.org/patch/610820/
This is restoring the inlines for the size calculation functions, but
I think that's ok. They are rather small and they're called from the
event notification path (ie. packet path), so the compiler just place
them out of the way when not needed and we calm down the gcc warning.
Thanks!
^ permalink raw reply
* Re: Poorer networking performance in later kernels?
From: Eric Dumazet @ 2016-04-18 18:05 UTC (permalink / raw)
To: Butler, Peter; +Cc: netdev@vger.kernel.org
In-Reply-To: <BY2PR0301MB1990B79988EF1A3FB461882FD66B0@BY2PR0301MB1990.namprd03.prod.outlook.com>
On Mon, 2016-04-18 at 16:27 +0000, Butler, Peter wrote:
> Hi Eric
>
> Thanks for your response. My apologies for being late in getting back
> to you - I wasn't able to have access to the lab hardware on the
> weekend.
>
> I performed your test as suggested - I've provided a side-by-side diff
> of the nstat output below for the SCTP test only (not the TCP test).
> Note that the fields that are output are somewhat different for the
> two kernels - i.e. some fields exist in one but not the other
> (presumably this comes from the kernel internals?).
>
> Other than seeing 'larger' throughput numbers in this output I'm not
> sure what to take from it - I'm certainly not a networking
> expert :-( Let me know if there's anything that speaks to you.
>
> Note that this test was again done on a clean, freshly rebooted and
> idle system. Let me know if there's any issues with the output format
> of this data in the email.
>
> Thanks,
>
> Peter
OK, please do not top-post on netdev.
I can not really comment on SCTP, could you please post numbers with
TCP ?
Thanks !
^ permalink raw reply
* Greetings!!!
From: andreas111 @ 2016-04-18 17:49 UTC (permalink / raw)
To: Mr. Eric
Hi, how are you? My name is J Eric Denials, External Financial Auditor at Lloyds Banking Group plc., London. It is a pleasure to contact you at this time through this medium. I have a cool and legitimate deal to do with you as you're a foreigner, it will be mutually beneficial to both. If you’re interested, urgently, get back to me for more explanation about the deal.
Regards
Eric
^ permalink raw reply
* Re: switchdev fib offload issues
From: David Miller @ 2016-04-18 17:52 UTC (permalink / raw)
To: jiri
Cc: netdev, idosch, eladr, yotamg, ogerlitz, roopa, nikolay, jhs,
john.fastabend, rami.rosen, gospo, stephen, sfeldma, dsa,
f.fainelli, andrew, vivien.didelot, tgraf, aduyck
In-Reply-To: <20160418154757.GA2059@nanopsycho.amit.cz>
From: Jiri Pirko <jiri@resnulli.us>
Date: Mon, 18 Apr 2016 17:47:57 +0200
> However, if for any reason the switchdev add operation fails, there is an
> abort function called (switchdev_fib_ipv4_abort). This function does two
> things which are both unfortunate in many usecases:
> 1) evicts all fib entries from HW leaving all processing done in kernel
> - For Spectrum ASIC this means that all traffic running at 100G between
> all ports is immediately downgraded to ~1-3Gbits
> - Also this happens silently, user knows nothing about anything went wrong,
> only forwarding performance suddenly sucks.
>
> 2) sets net->ipv4.fib_offload_disabled = true
> - That results in no other fib entry being offloaded, forever,
> until net is removed and added again, machine reboot is required
> in case if init_ns
>
> These 2 issues makes fib offload completely unusable. So I propose
> to start thinking about fixing this.
>
> I believe that although the current behaviour might be good for default,
> user should be able to change it by setting a different policy. This
> policy will allow to propagate offload error to user.
There were many length discussions about this.
It is extremely hard to load a partial table into the chip and
have it work correctly. This is because with longest matching
prefix you have to pull out the least specific routing entires
and process them in software.
Also, there is no communication about what makes an entry not be
insertable or not. So it may be the case that the shorter prefixes
all fit into the table, because those can be compressed and take
up less table space in the chip.
So it's extremely hard to know when "room" is available again. Room
for what? One 16-bit prefixed route? Or room for one arbitrarily
prefixed route? Which is it?
The user shouldn't need to know anything about this, and I will
be strongly against any design which puts the onus on the user
to configure a table that will fit into the chip.
^ permalink raw reply
* Re: [PATCH V2] net: ethernet: mellanox: correct page conversion
From: Sinan Kaya @ 2016-04-18 17:47 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Eli Cohen, linux-rdma@vger.kernel.org, timur@codeaurora.org,
cov@codeaurora.org, Yishai Hadas, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org
In-Reply-To: <20160418154058.GA4228@infradead.org>
On 4/18/2016 11:40 AM, Christoph Hellwig wrote:
> On Mon, Apr 18, 2016 at 11:21:12AM -0400, Sinan Kaya wrote:
>> I was looking at the code. I don't see how removing virt_to_page + vmap
>> would solve the issue.
>>
>> The code is trying to access the buffer space with direct.buf member
>> from the CPU side. This member would become NULL, when this code is
>> removed and also in mlx4_en_map_buffer.
>>
>> ...
>>
>> What am I missing?
>
> As mentioned before you'll also need to enforce you hit the nbufs = 1
> case for these. In fact most callers should simply switch to a plain
> dma_zalloc_coherent call without all these wrappers. If we have a case
> where we really want multiple buffers that don't have to be contiguous
> (maybe the MTT case) I'd rather opencode that instead of building this
> confusing interface on top of it.
>
I hit the first problem with CQE. The alloc routine is allocating pages
but CQE code is trying to do linear access with direct buf member.
I see that this code implements page_list support. I'd like to do the same
thing for CQE. Let me know if I'm in the right path.
static struct mlx4_eqe *get_eqe(struct mlx4_eq *eq, u32 entry, u8 eqe_factor,
u8 eqe_size)
--
Sinan Kaya
Qualcomm Technologies, Inc. on behalf of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox