* [PATCH net-next] act_connmark: Add missing dependency on NF_CONNTRACK_MARK
From: Thomas Graf @ 2015-01-20 12:44 UTC (permalink / raw)
To: davem; +Cc: netdev, Felix Fietkau, Jamal Hadi Salim
Depending on NETFILTER is not sufficient to ensure the presence of the
'mark' field in nf_conn, also needs to depend on NF_CONNTRACK_MARK.
Fixes: 22a5dc ("net: sched: Introduce connmark action")
Cc: Felix Fietkau <nbd@openwrt.org>
Cc: Jamal Hadi Salim <jhs@mojatatu.com>
Signed-off-by: Thomas Graf <tgraf@suug.ch>
---
net/sched/Kconfig | 1 +
1 file changed, 1 insertion(+)
diff --git a/net/sched/Kconfig b/net/sched/Kconfig
index 475e35e..7a57f66 100644
--- a/net/sched/Kconfig
+++ b/net/sched/Kconfig
@@ -713,6 +713,7 @@ config NET_ACT_BPF
config NET_ACT_CONNMARK
tristate "Netfilter Connection Mark Retriever"
depends on NET_CLS_ACT && NETFILTER && IP_NF_IPTABLES
+ depends on NF_CONNTRACK_MARK
---help---
Say Y here to allow retrieving of conn mark
--
1.9.3
^ permalink raw reply related
* [PATCH 05/11] hso: fix small indentation error
From: Olivier Sobrie @ 2015-01-20 12:29 UTC (permalink / raw)
To: Jan Dumon, Greg Kroah-Hartman
Cc: linux-kernel, linux-usb, netdev, Olivier Sobrie
In-Reply-To: <1421756978-4093-5-git-send-email-olivier@sobrie.be>
Simply remove the useless extra tab.
Signed-off-by: Olivier Sobrie <olivier@sobrie.be>
---
drivers/net/usb/hso.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/usb/hso.c b/drivers/net/usb/hso.c
index a49ac2e..2f2343d 100644
--- a/drivers/net/usb/hso.c
+++ b/drivers/net/usb/hso.c
@@ -2206,8 +2206,8 @@ static int hso_stop_serial_device(struct hso_device *hso_dev)
for (i = 0; i < serial->num_rx_urbs; i++) {
if (serial->rx_urb[i]) {
- usb_kill_urb(serial->rx_urb[i]);
- serial->rx_urb_filled[i] = 0;
+ usb_kill_urb(serial->rx_urb[i]);
+ serial->rx_urb_filled[i] = 0;
}
}
serial->curr_rx_urb_idx = 0;
--
2.2.0
^ permalink raw reply related
* [PATCH 11/11] usb: core: fix a race with usb_queue_reset_device()
From: Olivier Sobrie @ 2015-01-20 12:29 UTC (permalink / raw)
To: Jan Dumon, Greg Kroah-Hartman
Cc: linux-kernel, linux-usb, netdev, Olivier Sobrie
In-Reply-To: <1421756978-4093-11-git-send-email-olivier@sobrie.be>
When usb_queue_reset() is called it schedules a work in view of
resetting the usb interface. When the reset work is running, it
can be scheduled again (e.g. by the usb disconnect method of
the driver).
Consider that the reset work is queued again while the reset work
is running and that this work leads to a forced unbinding of the
usb interface (e.g. because a driver is bound to the interface
and has no pre/post_reset methods - see usb_reset_device()).
In such condition, usb_unbind_interface() gets called and this
function calls usb_cancel_queued_reset() which does nothing
because the flag "reset_running" is set to 1. The second reset
work that has been scheduled is therefore not cancelled.
Later, the usb_reset_device() tries to rebind the interface.
If it fails, then the usb interface context which contain the
reset work struct is freed and it most likely crash when the
second reset work tries to be run.
The following flow shows the problem:
* usb_queue_reset_device()
* __usb_queue_reset_device() <- If the reset work is queued after here, then
reset_running = 1 it will never be cancelled.
usb_reset_device()
usb_forced_unbind_intf()
usb_driver_release_interface()
usb_unbind_interface()
driver->disconnect()
usb_queue_reset_device() <- second reset
usb_cancel_queued_reset() <- does nothing because
the flag reset_running
is set
usb_unbind_and_rebind_marked_interfaces()
usb_rebind_intf()
device_attach()
driver->probe() <- fails (no more drivers hold a reference to
the usb interface)
reset_running = 0
* hub_event()
usb_disconnect()
usb_disable_device()
kobject_release()
device_release()
usb_release_interface()
kfree(intf) <- usb interface context is released
while we still have a pending reset
work that should be run
To avoid this problem, we use a delayed work so that if the reset
work is currently run, we can avoid further call to
__usb_queue_reset_device() work by using cancel_delayed_work().
Unfortunately it increases the size of the usb_interface structure...
Signed-off-by: Olivier Sobrie <olivier@sobrie.be>
---
drivers/usb/core/driver.c | 4 +++-
drivers/usb/core/hub.c | 4 ++--
drivers/usb/core/message.c | 4 ++--
include/linux/usb.h | 2 +-
4 files changed, 8 insertions(+), 6 deletions(-)
diff --git a/drivers/usb/core/driver.c b/drivers/usb/core/driver.c
index 9bffd26..c93fbbbb 100644
--- a/drivers/usb/core/driver.c
+++ b/drivers/usb/core/driver.c
@@ -287,7 +287,9 @@ static int usb_unbind_device(struct device *dev)
static void usb_cancel_queued_reset(struct usb_interface *iface)
{
if (iface->reset_running == 0)
- cancel_work_sync(&iface->reset_ws);
+ cancel_delayed_work_sync(&iface->reset_ws);
+ else
+ cancel_delayed_work(&iface->reset_ws);
}
/* called from driver core with dev locked */
diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index b649fef..52fba97 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -5604,13 +5604,13 @@ EXPORT_SYMBOL_GPL(usb_reset_device);
* NOTE: We don't do any reference count tracking because it is not
* needed. The lifecycle of the work_struct is tied to the
* usb_interface. Before destroying the interface we cancel the
- * work_struct, so the fact that work_struct is queued and or
+ * delayed_work, so the fact that delayed_work is queued and or
* running means the interface (and thus, the device) exist and
* are referenced.
*/
void usb_queue_reset_device(struct usb_interface *iface)
{
- schedule_work(&iface->reset_ws);
+ schedule_delayed_work(&iface->reset_ws, 0);
}
EXPORT_SYMBOL_GPL(usb_queue_reset_device);
diff --git a/drivers/usb/core/message.c b/drivers/usb/core/message.c
index f7b7713..d9f3f68 100644
--- a/drivers/usb/core/message.c
+++ b/drivers/usb/core/message.c
@@ -1650,7 +1650,7 @@ static void __usb_queue_reset_device(struct work_struct *ws)
{
int rc;
struct usb_interface *iface =
- container_of(ws, struct usb_interface, reset_ws);
+ container_of(ws, struct usb_interface, reset_ws.work);
struct usb_device *udev = interface_to_usbdev(iface);
rc = usb_lock_device_for_reset(udev, iface);
@@ -1847,7 +1847,7 @@ free_interfaces:
intf->dev.type = &usb_if_device_type;
intf->dev.groups = usb_interface_groups;
intf->dev.dma_mask = dev->dev.dma_mask;
- INIT_WORK(&intf->reset_ws, __usb_queue_reset_device);
+ INIT_DELAYED_WORK(&intf->reset_ws, __usb_queue_reset_device);
intf->minor = -1;
device_initialize(&intf->dev);
pm_runtime_no_callbacks(&intf->dev);
diff --git a/include/linux/usb.h b/include/linux/usb.h
index 447a7e2..ffb3da1 100644
--- a/include/linux/usb.h
+++ b/include/linux/usb.h
@@ -187,7 +187,7 @@ struct usb_interface {
struct device dev; /* interface specific device info */
struct device *usb_dev;
atomic_t pm_usage_cnt; /* usage counter for autosuspend */
- struct work_struct reset_ws; /* for resets in atomic context */
+ struct delayed_work reset_ws; /* for resets in atomic context */
};
#define to_usb_interface(d) container_of(d, struct usb_interface, dev)
--
2.2.0
^ permalink raw reply related
* [PATCH 10/11] hso: add missing cancel_work_sync in disconnect()
From: Olivier Sobrie @ 2015-01-20 12:29 UTC (permalink / raw)
To: Jan Dumon, Greg Kroah-Hartman
Cc: linux-kernel, linux-usb, netdev, Olivier Sobrie
In-Reply-To: <1421756978-4093-10-git-send-email-olivier@sobrie.be>
For hso serial devices, two cancel_work_sync were missing in the
disconnect method.
Signed-off-by: Olivier Sobrie <olivier@sobrie.be>
---
drivers/net/usb/hso.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/net/usb/hso.c b/drivers/net/usb/hso.c
index c916ab5..c14fc80 100644
--- a/drivers/net/usb/hso.c
+++ b/drivers/net/usb/hso.c
@@ -3107,6 +3107,8 @@ static void hso_free_interface(struct usb_interface *interface)
mutex_lock(&serial->parent->mutex);
serial->parent->usb_gone = 1;
mutex_unlock(&serial->parent->mutex);
+ cancel_work_sync(&serial_table[i]->async_put_intf);
+ cancel_work_sync(&serial_table[i]->async_get_intf);
hso_serial_tty_unregister(serial);
kref_put(&serial_table[i]->ref, hso_serial_ref_free);
set_serial_by_index(i, NULL);
--
2.2.0
^ permalink raw reply related
* [PATCH 09/11] hso: update serial_table in usb disconnect method
From: Olivier Sobrie @ 2015-01-20 12:29 UTC (permalink / raw)
To: Jan Dumon, Greg Kroah-Hartman
Cc: linux-kernel, linux-usb, netdev, Olivier Sobrie
In-Reply-To: <1421756978-4093-9-git-send-email-olivier@sobrie.be>
The serial_table is used to map the minor number of the usb serial device
to its associated context. The table is updated in the probe method and
in hso_serial_ref_free() which is called either from the tty cleanup
method or from the usb disconnect method.
This patch ensures that the serial_table is updated in the disconnect
method and no more from the cleanup method to avoid the following
potential race condition.
- hso_disconnect() is called for usb interface "x". Because the serial
port was open and because the cleanup method of the tty_port hasn't
been called yet, hso_serial_ref_free() is not run.
- hso_probe() is called and fails for a new hso serial usb interface
"y". The function hso_free_interface() is called and iterates
over the element of serial_table to find the device associated to
the usb interface context.
If the usb interface context of usb interface "y" has been created
at the same place as for usb interface "x", then the cleanup
functions are called for usb interfaces "x" and "y" and
hso_serial_ref_free() is called for both interfaces.
- release_tty() is called for serial port linked to usb interface "x"
and possibly crash because the tty_port structure contained in the
hso_device structure has been freed.
Signed-off-by: Olivier Sobrie <olivier@sobrie.be>
---
drivers/net/usb/hso.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/usb/hso.c b/drivers/net/usb/hso.c
index 5b157ad..c916ab5 100644
--- a/drivers/net/usb/hso.c
+++ b/drivers/net/usb/hso.c
@@ -2597,7 +2597,6 @@ static void hso_free_serial_device(struct hso_device *hso_dev)
if (!serial)
return;
- set_serial_by_index(serial->minor, NULL);
hso_serial_common_free(serial);
@@ -3110,6 +3109,7 @@ static void hso_free_interface(struct usb_interface *interface)
mutex_unlock(&serial->parent->mutex);
hso_serial_tty_unregister(serial);
kref_put(&serial_table[i]->ref, hso_serial_ref_free);
+ set_serial_by_index(i, NULL);
}
}
--
2.2.0
^ permalink raw reply related
* [PATCH 08/11] hso: move tty_unregister outside hso_serial_common_free()
From: Olivier Sobrie @ 2015-01-20 12:29 UTC (permalink / raw)
To: Jan Dumon, Greg Kroah-Hartman
Cc: linux-kernel, linux-usb, netdev, Olivier Sobrie
In-Reply-To: <1421756978-4093-8-git-send-email-olivier@sobrie.be>
The function hso_serial_common_free() is called either by the cleanup
method of the tty or by the usb disconnect method.
In the former case, the usb_disconnect() has been already called
and the sysfs group associated to the device has been removed.
By calling tty_unregister directly from the usb_disconnect() method,
we avoid a warning due to the removal of the sysfs group of the usb
device.
Example of warning:
------------[ cut here ]------------
WARNING: CPU: 0 PID: 778 at fs/sysfs/group.c:225 sysfs_remove_group+0x50/0x94()
sysfs group c0645a88 not found for kobject 'ttyHS5'
Modules linked in:
CPU: 0 PID: 778 Comm: kworker/0:3 Tainted: G W 3.18.0+ #105
Workqueue: events release_one_tty
[<c000dfe4>] (unwind_backtrace) from [<c000c014>] (show_stack+0x14/0x1c)
[<c000c014>] (show_stack) from [<c0016bac>] (warn_slowpath_common+0x5c/0x7c)
[<c0016bac>] (warn_slowpath_common) from [<c0016c60>] (warn_slowpath_fmt+0x30/0x40)
[<c0016c60>] (warn_slowpath_fmt) from [<c00ddd14>] (sysfs_remove_group+0x50/0x94)
[<c00ddd14>] (sysfs_remove_group) from [<c0221e44>] (device_del+0x30/0x190)
[<c0221e44>] (device_del) from [<c0221fb0>] (device_unregister+0xc/0x18)
[<c0221fb0>] (device_unregister) from [<c0221fec>] (device_destroy+0x30/0x3c)
[<c0221fec>] (device_destroy) from [<c01fe1dc>] (tty_unregister_device+0x2c/0x5c)
[<c01fe1dc>] (tty_unregister_device) from [<c029a428>] (hso_serial_common_free+0x2c/0x88)
[<c029a428>] (hso_serial_common_free) from [<c029a4c0>] (hso_serial_ref_free+0x3c/0xb8)
[<c029a4c0>] (hso_serial_ref_free) from [<c01ff430>] (release_one_tty+0x30/0x84)
[<c01ff430>] (release_one_tty) from [<c00271d4>] (process_one_work+0x21c/0x3c8)
[<c00271d4>] (process_one_work) from [<c0027758>] (worker_thread+0x3d8/0x560)
[<c0027758>] (worker_thread) from [<c002be4c>] (kthread+0xc0/0xcc)
[<c002be4c>] (kthread) from [<c0009630>] (ret_from_fork+0x14/0x24)
---[ end trace cb88537fdc8fa208 ]---
Signed-off-by: Olivier Sobrie <olivier@sobrie.be>
---
drivers/net/usb/hso.c | 12 +++++++++---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/drivers/net/usb/hso.c b/drivers/net/usb/hso.c
index 55074da..5b157ad 100644
--- a/drivers/net/usb/hso.c
+++ b/drivers/net/usb/hso.c
@@ -2234,14 +2234,17 @@ static int hso_stop_serial_device(struct hso_device *hso_dev)
return 0;
}
-static void hso_serial_common_free(struct hso_serial *serial)
+static void hso_serial_tty_unregister(struct hso_serial *serial)
{
- int i;
-
if (serial->parent->dev)
device_remove_file(serial->parent->dev, &dev_attr_hsotype);
tty_unregister_device(tty_drv, serial->minor);
+}
+
+static void hso_serial_common_free(struct hso_serial *serial)
+{
+ int i;
for (i = 0; i < serial->num_rx_urbs; i++) {
/* unlink and free RX URB */
@@ -2323,6 +2326,7 @@ static int hso_serial_common_create(struct hso_serial *serial, int num_urbs,
return 0;
exit:
+ hso_serial_tty_unregister(serial);
hso_serial_common_free(serial);
return -1;
}
@@ -2683,6 +2687,7 @@ static struct hso_device *hso_create_bulk_serial_device(
return hso_dev;
exit2:
+ hso_serial_tty_unregister(serial);
hso_serial_common_free(serial);
exit:
hso_free_tiomget(serial);
@@ -3103,6 +3108,7 @@ static void hso_free_interface(struct usb_interface *interface)
mutex_lock(&serial->parent->mutex);
serial->parent->usb_gone = 1;
mutex_unlock(&serial->parent->mutex);
+ hso_serial_tty_unregister(serial);
kref_put(&serial_table[i]->ref, hso_serial_ref_free);
}
}
--
2.2.0
^ permalink raw reply related
* [PATCH 07/11] hso: replace reset_device work by usb_queue_reset_device()
From: Olivier Sobrie @ 2015-01-20 12:29 UTC (permalink / raw)
To: Jan Dumon, Greg Kroah-Hartman
Cc: linux-kernel, linux-usb, netdev, Olivier Sobrie
In-Reply-To: <1421756978-4093-7-git-send-email-olivier@sobrie.be>
There is no need for a dedicated reset work in the hso driver since
there is already a reset work foreseen in usb_interface that does
the same.
Signed-off-by: Olivier Sobrie <olivier@sobrie.be>
---
drivers/net/usb/hso.c | 25 +------------------------
1 file changed, 1 insertion(+), 24 deletions(-)
diff --git a/drivers/net/usb/hso.c b/drivers/net/usb/hso.c
index 484e423..55074da 100644
--- a/drivers/net/usb/hso.c
+++ b/drivers/net/usb/hso.c
@@ -274,7 +274,6 @@ struct hso_device {
u8 usb_gone;
struct work_struct async_get_intf;
struct work_struct async_put_intf;
- struct work_struct reset_device;
struct usb_device *usb;
struct usb_interface *interface;
@@ -340,7 +339,6 @@ static void async_put_intf(struct work_struct *data);
static int hso_put_activity(struct hso_device *hso_dev);
static int hso_get_activity(struct hso_device *hso_dev);
static void tiocmget_intr_callback(struct urb *urb);
-static void reset_device(struct work_struct *data);
/*****************************************************************************/
/* Helping functions */
/*****************************************************************************/
@@ -696,7 +694,7 @@ static void handle_usb_error(int status, const char *function,
case -ETIMEDOUT:
explanation = "protocol error";
if (hso_dev)
- schedule_work(&hso_dev->reset_device);
+ usb_queue_reset_device(hso_dev->interface);
break;
default:
explanation = "unknown status";
@@ -2347,7 +2345,6 @@ static struct hso_device *hso_create_device(struct usb_interface *intf,
INIT_WORK(&hso_dev->async_get_intf, async_get_intf);
INIT_WORK(&hso_dev->async_put_intf, async_put_intf);
- INIT_WORK(&hso_dev->reset_device, reset_device);
return hso_dev;
}
@@ -3086,26 +3083,6 @@ out:
return result;
}
-static void reset_device(struct work_struct *data)
-{
- struct hso_device *hso_dev =
- container_of(data, struct hso_device, reset_device);
- struct usb_device *usb = hso_dev->usb;
- int result;
-
- if (hso_dev->usb_gone) {
- D1("No reset during disconnect\n");
- } else {
- result = usb_lock_device_for_reset(usb, hso_dev->interface);
- if (result < 0)
- D1("unable to lock device for reset: %d\n", result);
- else {
- usb_reset_device(usb);
- usb_unlock_device(usb);
- }
- }
-}
-
static void hso_serial_ref_free(struct kref *ref)
{
struct hso_device *hso_dev = container_of(ref, struct hso_device, ref);
--
2.2.0
^ permalink raw reply related
* [PATCH 06/11] hso: rename hso_dev into serial in hso_free_interface()
From: Olivier Sobrie @ 2015-01-20 12:29 UTC (permalink / raw)
To: Jan Dumon, Greg Kroah-Hartman
Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-usb-u79uwXL29TY76Z2rM5mHXA, netdev-u79uwXL29TY76Z2rM5mHXA,
Olivier Sobrie
In-Reply-To: <1421756978-4093-6-git-send-email-olivier-Ui3EtX6WB9GzQB+pC5nmwQ@public.gmane.org>
In other functions of the driver, variables of type "struct hso_serial"
are denoted by "serial" and variables of type "struct hso_device" are
denoted by "hso_dev". This patch makes the hso_free_interface()
consistent with these notations.
Signed-off-by: Olivier Sobrie <olivier-Ui3EtX6WB9GzQB+pC5nmwQ@public.gmane.org>
---
drivers/net/usb/hso.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/net/usb/hso.c b/drivers/net/usb/hso.c
index 2f2343d..484e423 100644
--- a/drivers/net/usb/hso.c
+++ b/drivers/net/usb/hso.c
@@ -3115,17 +3115,17 @@ static void hso_serial_ref_free(struct kref *ref)
static void hso_free_interface(struct usb_interface *interface)
{
- struct hso_serial *hso_dev;
+ struct hso_serial *serial;
int i;
for (i = 0; i < HSO_SERIAL_TTY_MINORS; i++) {
if (serial_table[i] &&
(serial_table[i]->interface == interface)) {
- hso_dev = dev2ser(serial_table[i]);
- tty_port_tty_hangup(&hso_dev->port, false);
- mutex_lock(&hso_dev->parent->mutex);
- hso_dev->parent->usb_gone = 1;
- mutex_unlock(&hso_dev->parent->mutex);
+ serial = dev2ser(serial_table[i]);
+ tty_port_tty_hangup(&serial->port, false);
+ mutex_lock(&serial->parent->mutex);
+ serial->parent->usb_gone = 1;
+ mutex_unlock(&serial->parent->mutex);
kref_put(&serial_table[i]->ref, hso_serial_ref_free);
}
}
--
2.2.0
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [PATCH 04/11] hso: fix memory leak in hso_create_rfkill()
From: Olivier Sobrie @ 2015-01-20 12:29 UTC (permalink / raw)
To: Jan Dumon, Greg Kroah-Hartman
Cc: linux-kernel, linux-usb, netdev, Olivier Sobrie
In-Reply-To: <1421756978-4093-4-git-send-email-olivier@sobrie.be>
When the rfkill interface was created, a buffer containing the name
of the rfkill node was allocated. This buffer was never freed when the
device disappears.
To fix the problem, we put the name given to rfkill_alloc() in
the hso_net structure.
Signed-off-by: Olivier Sobrie <olivier@sobrie.be>
---
drivers/net/usb/hso.c | 12 +++---------
1 file changed, 3 insertions(+), 9 deletions(-)
diff --git a/drivers/net/usb/hso.c b/drivers/net/usb/hso.c
index 470ef9e..a49ac2e 100644
--- a/drivers/net/usb/hso.c
+++ b/drivers/net/usb/hso.c
@@ -153,6 +153,7 @@ struct hso_net {
struct hso_device *parent;
struct net_device *net;
struct rfkill *rfkill;
+ char name[8];
struct usb_endpoint_descriptor *in_endp;
struct usb_endpoint_descriptor *out_endp;
@@ -2467,27 +2468,20 @@ static void hso_create_rfkill(struct hso_device *hso_dev,
{
struct hso_net *hso_net = dev2net(hso_dev);
struct device *dev = &hso_net->net->dev;
- char *rfkn;
- rfkn = kzalloc(20, GFP_KERNEL);
- if (!rfkn)
- dev_err(dev, "%s - Out of memory\n", __func__);
-
- snprintf(rfkn, 20, "hso-%d",
+ snprintf(hso_net->name, sizeof(hso_net->name), "hso-%d",
interface->altsetting->desc.bInterfaceNumber);
- hso_net->rfkill = rfkill_alloc(rfkn,
+ hso_net->rfkill = rfkill_alloc(hso_net->name,
&interface_to_usbdev(interface)->dev,
RFKILL_TYPE_WWAN,
&hso_rfkill_ops, hso_dev);
if (!hso_net->rfkill) {
dev_err(dev, "%s - Out of memory\n", __func__);
- kfree(rfkn);
return;
}
if (rfkill_register(hso_net->rfkill) < 0) {
rfkill_destroy(hso_net->rfkill);
- kfree(rfkn);
hso_net->rfkill = NULL;
dev_err(dev, "%s - Failed to register rfkill\n", __func__);
return;
--
2.2.0
^ permalink raw reply related
* [PATCH 03/11] hso: fix memory leak when device disconnects
From: Olivier Sobrie @ 2015-01-20 12:29 UTC (permalink / raw)
To: Jan Dumon, Greg Kroah-Hartman
Cc: linux-kernel, linux-usb, netdev, Olivier Sobrie
In-Reply-To: <1421756978-4093-3-git-send-email-olivier@sobrie.be>
In the disconnect path, tx_buffer should freed like tx_data to avoid
a memory leak when the device disconnects.
Signed-off-by: Olivier Sobrie <olivier@sobrie.be>
---
drivers/net/usb/hso.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/net/usb/hso.c b/drivers/net/usb/hso.c
index 3a6c630..470ef9e 100644
--- a/drivers/net/usb/hso.c
+++ b/drivers/net/usb/hso.c
@@ -2253,6 +2253,7 @@ static void hso_serial_common_free(struct hso_serial *serial)
/* unlink and free TX URB */
usb_free_urb(serial->tx_urb);
+ kfree(serial->tx_buffer);
kfree(serial->tx_data);
tty_port_destroy(&serial->port);
}
--
2.2.0
^ permalink raw reply related
* [PATCH 02/11] hso: fix crash when device disappears while serial port is open
From: Olivier Sobrie @ 2015-01-20 12:29 UTC (permalink / raw)
To: Jan Dumon, Greg Kroah-Hartman
Cc: linux-kernel, linux-usb, netdev, Olivier Sobrie
In-Reply-To: <1421756978-4093-2-git-send-email-olivier@sobrie.be>
When the device disappear, the function hso_disconnect() is called to
perform cleanup. In the cleanup function, hso_free_interface() calls
tty_port_tty_hangup() in view of scheduling a work to hang up the tty if
needed. If the port was not open then hso_serial_ref_free() is called
directly to cleanup everything. Otherwise, hso_serial_ref_free() is called
when the last fd associated to the port is closed.
For each open port, tty_release() will call the close method,
hso_serial_close(), which drops the last kref and call
hso_serial_ref_free() which unregisters, destroys the tty port
and finally frees the structure in which the tty_port structure
is included. Later, in tty_release(), more precisely when release_tty()
is called, the tty_port previously freed is accessed to cancel
the tty buf workqueue and it leads to a crash.
In view of avoiding this crash, we add a cleanup method that is called
at the end of the hangup process and we drop the last kref in this
function when all the ports have been closed, when tty_port is no
more needed and when it is safe to free the structure containing the
tty_port structure.
Signed-off-by: Olivier Sobrie <olivier@sobrie.be>
---
drivers/net/usb/hso.c | 17 +++++++++++++----
1 file changed, 13 insertions(+), 4 deletions(-)
diff --git a/drivers/net/usb/hso.c b/drivers/net/usb/hso.c
index cb0bcc1..3a6c630 100644
--- a/drivers/net/usb/hso.c
+++ b/drivers/net/usb/hso.c
@@ -1270,7 +1270,6 @@ static int hso_serial_open(struct tty_struct *tty, struct file *filp)
goto err_out;
D1("Opening %d", serial->minor);
- kref_get(&serial->parent->ref);
/* setup */
tty->driver_data = serial;
@@ -1289,7 +1288,8 @@ static int hso_serial_open(struct tty_struct *tty, struct file *filp)
if (result) {
hso_stop_serial_device(serial->parent);
serial->port.count--;
- kref_put(&serial->parent->ref, hso_serial_ref_free);
+ } else {
+ kref_get(&serial->parent->ref);
}
} else {
D1("Port was already open");
@@ -1339,8 +1339,6 @@ static void hso_serial_close(struct tty_struct *tty, struct file *filp)
usb_autopm_put_interface(serial->parent->interface);
mutex_unlock(&serial->parent->mutex);
-
- kref_put(&serial->parent->ref, hso_serial_ref_free);
}
/* close the requested serial port */
@@ -1391,6 +1389,16 @@ static int hso_serial_write_room(struct tty_struct *tty)
return room;
}
+static void hso_serial_cleanup(struct tty_struct *tty)
+{
+ struct hso_serial *serial = tty->driver_data;
+
+ if (!serial)
+ return;
+
+ kref_put(&serial->parent->ref, hso_serial_ref_free);
+}
+
/* setup the term */
static void hso_serial_set_termios(struct tty_struct *tty, struct ktermios *old)
{
@@ -3215,6 +3223,7 @@ static const struct tty_operations hso_serial_ops = {
.close = hso_serial_close,
.write = hso_serial_write,
.write_room = hso_serial_write_room,
+ .cleanup = hso_serial_cleanup,
.ioctl = hso_serial_ioctl,
.set_termios = hso_serial_set_termios,
.chars_in_buffer = hso_serial_chars_in_buffer,
--
2.2.0
^ permalink raw reply related
* [PATCH 01/11] hso: remove useless header file timer.h
From: Olivier Sobrie @ 2015-01-20 12:29 UTC (permalink / raw)
To: Jan Dumon, Greg Kroah-Hartman
Cc: linux-kernel, linux-usb, netdev, Olivier Sobrie
In-Reply-To: <1421756978-4093-1-git-send-email-olivier@sobrie.be>
No timer related function is used in this driver.
Signed-off-by: Olivier Sobrie <olivier@sobrie.be>
---
drivers/net/usb/hso.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/net/usb/hso.c b/drivers/net/usb/hso.c
index babda7d..cb0bcc1 100644
--- a/drivers/net/usb/hso.c
+++ b/drivers/net/usb/hso.c
@@ -58,7 +58,6 @@
#include <linux/module.h>
#include <linux/ethtool.h>
#include <linux/usb.h>
-#include <linux/timer.h>
#include <linux/tty.h>
#include <linux/tty_driver.h>
#include <linux/tty_flip.h>
--
2.2.0
^ permalink raw reply related
* [PATCH 00/11] hso: fix some problems with reset/disconnect path
From: Olivier Sobrie @ 2015-01-20 12:29 UTC (permalink / raw)
To: Jan Dumon, Greg Kroah-Hartman
Cc: linux-kernel, linux-usb, netdev, Olivier Sobrie
These patches attempt to fix some problems I observed when the hso
device is disconnected or when a usb reset is queued by the hso driver
Several patches of this serie are fixing crashes or memleaks in hso.
The last patch of this serie fix a race condition occurring when multiple
usb resets are queued on an usb interface.
This serie of patches is based on v3.18.
Olivier Sobrie (11):
hso: remove useless header file timer.h
hso: fix crash when device disappears while serial port is open
hso: fix memory leak when device disconnects
hso: fix memory leak in hso_create_rfkill()
hso: fix small indentation error
hso: rename hso_dev into serial in hso_free_interface()
hso: replace reset_device work by usb_queue_reset_device()
hso: move tty_unregister outside hso_serial_common_free()
hso: update serial_table in usb disconnect method
hso: add missing cancel_work_sync in disconnect()
usb: core: fix a race with usb_queue_reset_device()
drivers/net/usb/hso.c | 88 ++++++++++++++++++++--------------------------
drivers/usb/core/driver.c | 4 ++-
drivers/usb/core/hub.c | 4 +--
drivers/usb/core/message.c | 4 +--
include/linux/usb.h | 2 +-
5 files changed, 46 insertions(+), 56 deletions(-)
--
2.2.0
^ permalink raw reply
* [net PATCH 1/1] drivers: net: cpsw: discard dual emac default vlan configuration
From: Mugunthan V N @ 2015-01-20 12:25 UTC (permalink / raw)
To: netdev; +Cc: davem, Mugunthan V N, stable
In Dual EMAC, the default vlans are used to segregate rx packets between
the ports, so adding the same default vlan to the switch will affect the
normal packet transfers. So returning error on addition of dual emac
default vlans.
Even if emac 0 default port vlan is added to emac 1, it will lead to
break dual EMAC port seperations.
Fixes: d9ba8f9 (driver: net: ethernet: cpsw: dual emac interface implementation)
Cc: <stable@vger.kernel.org> # v3.9+
Reported-by: Felipe Balbi <balbi@ti.com>
Signed-off-by: Mugunthan V N <mugunthanvnm@ti.com>
---
drivers/net/ethernet/ti/cpsw.c | 22 ++++++++++++++++++++++
1 file changed, 22 insertions(+)
diff --git a/drivers/net/ethernet/ti/cpsw.c b/drivers/net/ethernet/ti/cpsw.c
index e068d48..7c32815 100644
--- a/drivers/net/ethernet/ti/cpsw.c
+++ b/drivers/net/ethernet/ti/cpsw.c
@@ -1683,6 +1683,19 @@ static int cpsw_ndo_vlan_rx_add_vid(struct net_device *ndev,
if (vid == priv->data.default_vlan)
return 0;
+ if (priv->data.dual_emac) {
+ /* In dual EMAC, reserved VLAN id should not be used of
+ * creating vlan interfaces as this can break the dual
+ * EMAC port seperation
+ */
+ int i;
+
+ for (i = 0; i < priv->data.slaves; i++) {
+ if (vid == priv->slaves[i].port_vlan)
+ return -EINVAL;
+ }
+ }
+
dev_info(priv->dev, "Adding vlanid %d to vlan filter\n", vid);
return cpsw_add_vlan_ale_entry(priv, vid);
}
@@ -1696,6 +1709,15 @@ static int cpsw_ndo_vlan_rx_kill_vid(struct net_device *ndev,
if (vid == priv->data.default_vlan)
return 0;
+ if (priv->data.dual_emac) {
+ int i;
+
+ for (i = 0; i < priv->data.slaves; i++) {
+ if (vid == priv->slaves[i].port_vlan)
+ return -EINVAL;
+ }
+ }
+
dev_info(priv->dev, "removing vlanid %d from vlan filter\n", vid);
ret = cpsw_ale_del_vlan(priv->ale, vid, 0);
if (ret != 0)
--
2.2.1.62.g3f15098
^ permalink raw reply related
* Re: [PATCH net-next v13 3/3] net: hisilicon: new hip04 ethernet driver
From: Arnd Bergmann @ 2015-01-20 12:01 UTC (permalink / raw)
To: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
Cc: Ding Tianhong, Alexander Graf, devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-lFZ/pmaqli7XmaaqVzeoHQ,
sergei.shtylyov-M4DtvfQ/ZS1MRgGoP+s0PdBPR1lH4CV8,
eric.dumazet-Re5JQEeQqe8AvxtiuMwx3w,
netdev-u79uwXL29TY76Z2rM5mHXA, xuwei5-C8/M+/jPZTeaMJb+Lgu22Q,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
grant.likely-QSEj5FYQhm4dnm+yROfE0A,
zhangfei.gao-QSEj5FYQhm4dnm+yROfE0A, davem-fT/PcQaiUtIeIZ0/mPfg9Q
In-Reply-To: <54BDBA29.10703-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
On Tuesday 20 January 2015 10:15:05 Ding Tianhong wrote:
> On 2015/1/20 4:34, Arnd Bergmann wrote:
> > On Monday 19 January 2015 19:11:11 Alexander Graf wrote:
> >>
> >> After hammering on the box a bit again, I'm in a situation where I get
> >> lots of
> >>
> >> [302398.232603] hip04-ether e28b0000.ethernet eth0: rx drop
> >> [302398.377309] hip04-ether e28b0000.ethernet eth0: rx drop
> >> [302398.395198] hip04-ether e28b0000.ethernet eth0: rx drop
> >> [302398.466118] hip04-ether e28b0000.ethernet eth0: rx drop
> >> [302398.659009] hip04-ether e28b0000.ethernet eth0: rx drop
> >> [302399.053389] hip04-ether e28b0000.ethernet eth0: rx drop
> >> [302399.122067] hip04-ether e28b0000.ethernet eth0: rx drop
> >> [302399.268192] hip04-ether e28b0000.ethernet eth0: rx drop
> >> [302399.286081] hip04-ether e28b0000.ethernet eth0: rx drop
> >> [302399.594201] hip04-ether e28b0000.ethernet eth0: rx drop
> >> [302399.683416] hip04-ether e28b0000.ethernet eth0: rx drop
> >> [302399.701307] hip04-ether e28b0000.ethernet eth0: rx drop
> >>
> >> and I really am getting a lot of drops - I can't even ping the machine
> >> anymore.
> >>
> >> However, as it is there's a good chance the machine is simply
> >> unreachable because it's busy writing to the UART, and even if not all
> >> useful messages indicating anything have scrolled out. I really don't
> >> think you should emit any message over and over again to the user. Once
> >> or twice is enough.
> >>
> >> Please make sure to rate limit it.
> >
> > I would argue that packet loss is not an error condition at all
> > and you should not print this at netdev_err() level. You could make
> > this a netdev_dbg(), or just make it silent because it's already
> > counted in the statistics.
> >
>
> I think something wrong with Graf's board, I will try to make it happen on my board, and
> in any case I will add rate limit to xx_drop and change to dbg log level.
I've looked at the interrupt handling in more detail and came up
with this patch. Please review, and forward if you are happy with the
changes.
Alex, could you try if this solves your problem?
8<----
Subject: [PATCH] net/hip04: refactor interrupt masking
The hip04 ethernet driver currently acknowledges all interrupts directly
in the interrupt handler, and leaves all interrupts except the RX data
enabled the whole time. This causes multiple problems:
- When more packets come in between the original interrupt and the
NAPI poll function, we will get an extraneous RX interrupt as soon
as interrupts are enabled again.
- The two error interrupts are by definition combining all errors that
may have happened since the last time they were handled, but just
acknowledging the irq without dealing with the cause of the condition
makes it come back immediately. In particular, when NAPI is intentionally
stalling the rx queue, this causes a storm of "rx dropped" messages.
- The access to the 'reg_inten' field in hip_priv is used for serializing
access, but is in fact racy itself.
To deal with these issues, the driver is changed to only acknowledge
the IRQ at the point when it is being handled. The RX interrupts now get
acked right before reading the number of received frames to keep spurious
interrupts to the absolute minimum without losing interrupts.
As there is no tx-complete interrupt, only an informational tx-dropped
one, we now ack that in the tx reclaim handler, hoping that clearing
the descriptors has also eliminated the error condition.
The only place that reads the reg_inten now just relies on
napi_schedule_prep() to return whether NAPI is active or not, and
the poll() function is then going to ack and reenabled the IRQ if
necessary.
Finally, when we disable interrupts, they are now all disabled
together, in order to simplify the logic and to avoid getting
rx-dropped interrupts when NAPI is in control of the rx queue.
Signed-off-by: Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>
diff --git a/drivers/net/ethernet/hisilicon/hip04_eth.c b/drivers/net/ethernet/hisilicon/hip04_eth.c
index 525214ef5984..83773247a368 100644
--- a/drivers/net/ethernet/hisilicon/hip04_eth.c
+++ b/drivers/net/ethernet/hisilicon/hip04_eth.c
@@ -56,6 +56,8 @@
#define RCV_DROP BIT(7)
#define TX_DROP BIT(6)
#define DEF_INT_ERR (RCV_NOBUF | RCV_DROP | TX_DROP)
+#define DEF_INT_RX (RCV_INT | RCV_NOBUF | RCV_DROP)
+#define DEF_INT_TX (TX_DROP)
#define DEF_INT_MASK (RCV_INT | DEF_INT_ERR)
/* TX descriptor config */
@@ -154,7 +156,6 @@ struct hip04_priv {
unsigned int port;
unsigned int speed;
unsigned int duplex;
- unsigned int reg_inten;
struct napi_struct napi;
struct net_device *ndev;
@@ -303,17 +304,15 @@ static void hip04_mac_enable(struct net_device *ndev)
val |= GE_RX_PORT_EN | GE_TX_PORT_EN;
writel_relaxed(val, priv->base + GE_PORT_EN);
- /* clear rx int */
- val = RCV_INT;
- writel_relaxed(val, priv->base + PPE_RINT);
+ /* clear prior interrupts */
+ writel_relaxed(DEF_INT_MASK, priv->base + PPE_RINT);
/* config recv int */
val = GE_RX_INT_THRESHOLD | GE_RX_TIMEOUT;
writel_relaxed(val, priv->base + PPE_CFG_RX_PKT_INT);
/* enable interrupt */
- priv->reg_inten = DEF_INT_MASK;
- writel_relaxed(priv->reg_inten, priv->base + PPE_INTEN);
+ writel_relaxed(DEF_INT_MASK, priv->base + PPE_INTEN);
}
static void hip04_mac_disable(struct net_device *ndev)
@@ -322,8 +321,7 @@ static void hip04_mac_disable(struct net_device *ndev)
u32 val;
/* disable int */
- priv->reg_inten &= ~(DEF_INT_MASK);
- writel_relaxed(priv->reg_inten, priv->base + PPE_INTEN);
+ writel_relaxed(0, priv->base + PPE_INTEN);
/* disable tx & rx */
val = readl_relaxed(priv->base + GE_PORT_EN);
@@ -403,6 +401,8 @@ static int hip04_tx_reclaim(struct net_device *ndev, bool force)
priv->tx_tail = tx_tail;
smp_wmb(); /* Ensure tx_tail visible to xmit */
+ writel_relaxed(DEF_INT_TX, priv->base + PPE_RINT);
+
out:
if (pkts_compl || bytes_compl)
netdev_completed_queue(ndev, pkts_compl, bytes_compl);
@@ -458,9 +458,7 @@ static int hip04_mac_start_xmit(struct sk_buff *skb, struct net_device *ndev)
if (count >= priv->tx_coalesce_frames) {
if (napi_schedule_prep(&priv->napi)) {
/* disable rx interrupt and timer */
- priv->reg_inten &= ~(RCV_INT);
- writel_relaxed(DEF_INT_MASK & ~RCV_INT,
- priv->base + PPE_INTEN);
+ writel_relaxed(0, priv->base + PPE_INTEN);
hrtimer_cancel(&priv->tx_coalesce_timer);
__napi_schedule(&priv->napi);
}
@@ -478,7 +476,7 @@ static int hip04_rx_poll(struct napi_struct *napi, int budget)
struct hip04_priv *priv = container_of(napi, struct hip04_priv, napi);
struct net_device *ndev = priv->ndev;
struct net_device_stats *stats = &ndev->stats;
- unsigned int cnt = hip04_recv_cnt(priv);
+ unsigned int cnt;
struct rx_desc *desc;
struct sk_buff *skb;
unsigned char *buf;
@@ -489,6 +487,10 @@ static int hip04_rx_poll(struct napi_struct *napi, int budget)
u16 len;
u32 err;
+ /* acknowledge interrupts and read status */
+ writel_relaxed(DEF_INT_RX, priv->base + PPE_RINT);
+ cnt = hip04_recv_cnt(priv);
+
while (cnt && !last) {
buf = priv->rx_buf[priv->rx_head];
skb = build_skb(buf, priv->rx_buf_size);
@@ -539,11 +541,8 @@ static int hip04_rx_poll(struct napi_struct *napi, int budget)
cnt = hip04_recv_cnt(priv);
}
- if (!(priv->reg_inten & RCV_INT)) {
- /* enable rx interrupt */
- priv->reg_inten |= RCV_INT;
- writel_relaxed(priv->reg_inten, priv->base + PPE_INTEN);
- }
+ /* enable rx interrupt */
+ writel_relaxed(DEF_INT_MASK, priv->base + PPE_INTEN);
napi_complete(napi);
done:
/* clean up tx descriptors and start a new timer if necessary */
@@ -564,23 +563,21 @@ static irqreturn_t hip04_mac_interrupt(int irq, void *dev_id)
if (!ists)
return IRQ_NONE;
- writel_relaxed(DEF_INT_MASK, priv->base + PPE_RINT);
-
if (unlikely(ists & DEF_INT_ERR)) {
- if (ists & (RCV_NOBUF | RCV_DROP))
+ if (ists & (RCV_NOBUF | RCV_DROP)) {
stats->rx_errors++;
stats->rx_dropped++;
- netdev_err(ndev, "rx drop\n");
+ netdev_dbg(ndev, "rx drop\n");
+ }
if (ists & TX_DROP) {
stats->tx_dropped++;
- netdev_err(ndev, "tx drop\n");
+ netdev_dbg(ndev, "tx drop\n");
}
}
- if (ists & RCV_INT && napi_schedule_prep(&priv->napi)) {
- /* disable rx interrupt */
- priv->reg_inten &= ~(RCV_INT);
- writel_relaxed(DEF_INT_MASK & ~RCV_INT, priv->base + PPE_INTEN);
+ if (napi_schedule_prep(&priv->napi)) {
+ /* disable interrupt */
+ writel_relaxed(0, priv->base + PPE_INTEN);
hrtimer_cancel(&priv->tx_coalesce_timer);
__napi_schedule(&priv->napi);
}
@@ -596,8 +593,7 @@ enum hrtimer_restart tx_done(struct hrtimer *hrtimer)
if (napi_schedule_prep(&priv->napi)) {
/* disable rx interrupt */
- priv->reg_inten &= ~(RCV_INT);
- writel_relaxed(DEF_INT_MASK & ~RCV_INT, priv->base + PPE_INTEN);
+ writel_relaxed(0, priv->base + PPE_INTEN);
__napi_schedule(&priv->napi);
}
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* Re: [PATCH tip 4/9] samples: bpf: simple tracing example in eBPF assembler
From: Masami Hiramatsu @ 2015-01-20 11:57 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: Ingo Molnar, Steven Rostedt, Namhyung Kim,
Arnaldo Carvalho de Melo, Jiri Olsa, David S. Miller,
Daniel Borkmann, Hannes Frederic Sowa, Brendan Gregg, linux-api,
netdev, linux-kernel, yrl.pp-manager.tt@hitachi.com
In-Reply-To: <1421381770-4866-5-git-send-email-ast@plumgrid.com>
(2015/01/16 13:16), Alexei Starovoitov wrote:
> simple packet drop monitor:
> - in-kernel eBPF program attaches to kfree_skb() event and records number
> of packet drops at given location
> - userspace iterates over the map every second and prints stats
Hmm, this eBPF assembly macros are very interesting. Maybe I can
replace current kprobe's argument "fetching methods" with this.
Thank you,
>
> Usage:
> $ sudo dropmon
> location 0xffffffff81695995 count 1
> location 0xffffffff816d0da9 count 2
>
> location 0xffffffff81695995 count 2
> location 0xffffffff816d0da9 count 2
>
> location 0xffffffff81695995 count 3
> location 0xffffffff816d0da9 count 2
>
> $ addr2line -ape ./bld_x64/vmlinux 0xffffffff81695995 0xffffffff816d0da9
> 0xffffffff81695995: ./bld_x64/../net/ipv4/icmp.c:1038
> 0xffffffff816d0da9: ./bld_x64/../net/unix/af_unix.c:1231
>
> Signed-off-by: Alexei Starovoitov <ast@plumgrid.com>
> ---
> samples/bpf/Makefile | 2 +
> samples/bpf/dropmon.c | 129 +++++++++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 131 insertions(+)
> create mode 100644 samples/bpf/dropmon.c
>
> diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile
> index b5b3600dcdf5..789691374562 100644
> --- a/samples/bpf/Makefile
> +++ b/samples/bpf/Makefile
> @@ -6,7 +6,9 @@ hostprogs-y := test_verifier test_maps
> hostprogs-y += sock_example
> hostprogs-y += sockex1
> hostprogs-y += sockex2
> +hostprogs-y += dropmon
>
> +dropmon-objs := dropmon.o libbpf.o
> test_verifier-objs := test_verifier.o libbpf.o
> test_maps-objs := test_maps.o libbpf.o
> sock_example-objs := sock_example.o libbpf.o
> diff --git a/samples/bpf/dropmon.c b/samples/bpf/dropmon.c
> new file mode 100644
> index 000000000000..9a2cd3344d69
> --- /dev/null
> +++ b/samples/bpf/dropmon.c
> @@ -0,0 +1,129 @@
> +/* simple packet drop monitor:
> + * - in-kernel eBPF program attaches to kfree_skb() event and records number
> + * of packet drops at given location
> + * - userspace iterates over the map every second and prints stats
> + */
> +#include <stdio.h>
> +#include <unistd.h>
> +#include <linux/bpf.h>
> +#include <errno.h>
> +#include <linux/unistd.h>
> +#include <string.h>
> +#include <linux/filter.h>
> +#include <stdlib.h>
> +#include <sys/types.h>
> +#include <sys/stat.h>
> +#include <fcntl.h>
> +#include <stdbool.h>
> +#include "libbpf.h"
> +
> +#define TRACEPOINT "/sys/kernel/debug/tracing/events/skb/kfree_skb/"
> +
> +static int write_to_file(const char *file, const char *str, bool keep_open)
> +{
> + int fd, err;
> +
> + fd = open(file, O_WRONLY);
> + err = write(fd, str, strlen(str));
> + (void) err;
> +
> + if (keep_open) {
> + return fd;
> + } else {
> + close(fd);
> + return -1;
> + }
> +}
> +
> +static int dropmon(void)
> +{
> + long long key, next_key, value = 0;
> + int prog_fd, map_fd, i;
> + char fmt[32];
> +
> + map_fd = bpf_create_map(BPF_MAP_TYPE_HASH, sizeof(key), sizeof(value), 1024);
> + if (map_fd < 0) {
> + printf("failed to create map '%s'\n", strerror(errno));
> + goto cleanup;
> + }
> +
> + /* the following eBPF program is equivalent to C:
> + * int filter(struct bpf_context *ctx)
> + * {
> + * long loc = ctx->arg2;
> + * long init_val = 1;
> + * long *value;
> + *
> + * value = bpf_map_lookup_elem(MAP_ID, &loc);
> + * if (value) {
> + * __sync_fetch_and_add(value, 1);
> + * } else {
> + * bpf_map_update_elem(MAP_ID, &loc, &init_val, BPF_ANY);
> + * }
> + * return 0;
> + * }
> + */
> + struct bpf_insn prog[] = {
> + BPF_LDX_MEM(BPF_DW, BPF_REG_2, BPF_REG_1, 8), /* r2 = *(u64 *)(r1 + 8) */
> + BPF_STX_MEM(BPF_DW, BPF_REG_10, BPF_REG_2, -8), /* *(u64 *)(fp - 8) = r2 */
> + BPF_MOV64_REG(BPF_REG_2, BPF_REG_10),
> + BPF_ALU64_IMM(BPF_ADD, BPF_REG_2, -8), /* r2 = fp - 8 */
> + BPF_LD_MAP_FD(BPF_REG_1, map_fd),
> + BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, 0, 0, BPF_FUNC_map_lookup_elem),
> + BPF_JMP_IMM(BPF_JEQ, BPF_REG_0, 0, 4),
> + BPF_MOV64_IMM(BPF_REG_1, 1), /* r1 = 1 */
> + BPF_RAW_INSN(BPF_STX | BPF_XADD | BPF_DW, BPF_REG_0, BPF_REG_1, 0, 0), /* xadd r0 += r1 */
> + BPF_MOV64_IMM(BPF_REG_0, 0), /* r0 = 0 */
> + BPF_EXIT_INSN(),
> + BPF_ST_MEM(BPF_DW, BPF_REG_10, -16, 1), /* *(u64 *)(fp - 16) = 1 */
> + BPF_MOV64_IMM(BPF_REG_4, BPF_ANY),
> + BPF_MOV64_REG(BPF_REG_3, BPF_REG_10),
> + BPF_ALU64_IMM(BPF_ADD, BPF_REG_3, -16), /* r3 = fp - 16 */
> + BPF_MOV64_REG(BPF_REG_2, BPF_REG_10),
> + BPF_ALU64_IMM(BPF_ADD, BPF_REG_2, -8), /* r2 = fp - 8 */
> + BPF_LD_MAP_FD(BPF_REG_1, map_fd),
> + BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, 0, 0, BPF_FUNC_map_update_elem),
> + BPF_MOV64_IMM(BPF_REG_0, 0), /* r0 = 0 */
> + BPF_EXIT_INSN(),
> + };
> +
> + prog_fd = bpf_prog_load(BPF_PROG_TYPE_TRACING_FILTER, prog,
> + sizeof(prog), "GPL");
> + if (prog_fd < 0) {
> + printf("failed to load prog '%s'\n%s", strerror(errno), bpf_log_buf);
> + return -1;
> + }
> +
> + sprintf(fmt, "bpf_%d", prog_fd);
> +
> + write_to_file(TRACEPOINT "filter", fmt, true);
> +
> + for (i = 0; i < 10; i++) {
> + key = 0;
> + while (bpf_get_next_key(map_fd, &key, &next_key) == 0) {
> + bpf_lookup_elem(map_fd, &next_key, &value);
> + printf("location 0x%llx count %lld\n", next_key, value);
> + key = next_key;
> + }
> + if (key)
> + printf("\n");
> + sleep(1);
> + }
> +
> +cleanup:
> + /* maps, programs, tracepoint filters will auto cleanup on process exit */
> +
> + return 0;
> +}
> +
> +int main(void)
> +{
> + FILE *f;
> +
> + /* start ping in the background to get some kfree_skb events */
> + f = popen("ping -c5 localhost", "r");
> + (void) f;
> +
> + dropmon();
> + return 0;
> +}
>
--
Masami HIRAMATSU
Software Platform Research Dept. Linux Technology Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: masami.hiramatsu.pt@hitachi.com
^ permalink raw reply
* Re: Re: Re: [PATCH tip 0/9] tracing: attach eBPF programs to tracepoints/syscalls/kprobe
From: Masami Hiramatsu @ 2015-01-20 11:57 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: Ingo Molnar, Steven Rostedt, Namhyung Kim,
Arnaldo Carvalho de Melo, Jiri Olsa, David S. Miller,
Daniel Borkmann, Hannes Frederic Sowa, Brendan Gregg, Linux API,
Network Development, LKML, zhangwei(Jovi),
yrl.pp-manager.tt@hitachi.com
In-Reply-To: <CAMEtUuwrpqRG4a=Hqnj3JBKuLbC4yV+trVAZhevKLbCsm_6U4Q@mail.gmail.com>
(2015/01/20 12:55), Alexei Starovoitov wrote:
> On Mon, Jan 19, 2015 at 6:58 PM, Masami Hiramatsu
> <masami.hiramatsu.pt@hitachi.com> wrote:
>>>
>>> it's done already... one can do the same skb->dev->name logic
>>> in kprobe attached program... so from bpf program point of view,
>>> tracepoints and kprobes feature-wise are exactly the same.
>>> Only input is different.
>>
>> No, I meant that the input should also be same, at least for the first step.
>> I guess it is easy to hook the ring buffer committing and fetch arguments
>> from the event entry.
>
> No. That would be very slow. See my comment to Steven
> and more detailed numbers below.
Thank you for measuring the performance differences.
Indeed, the ring buffer looks slow.
> Allocating ring buffer takes too much time.
>
>> And what I expected scenario was
>>
>> 1. setup kprobe traceevent with fd, buf, count by using perf-probe.
>> 2. load bpf module
>> 3. the module processes given event arguments.
>
> from ring buffer? that's too slow.
Ok, BTW, would you think is it possible to use a reusable small scratchpad
memory for passing arguments? (just a thought)
> It's not usable for high frequency events which
> need this in-kernel aggregation.
> If events are rare, then just dumping everything
> into trace buffer is just fine. No in-kernel program is needed.
Hmm, let me ensure your point, the performance number is the reason why
we need to do it in the kernel, right? Not mainly for the flexibility but speed.
>> Hmm, it sounds making another systemtap on top of tracepoint and kprobes.
>> Why don't you just reuse the existing facilities (perftools and ftrace)
>> instead of co-exist?
>
> hmm. I don't think we're on the same page yet...
> ring buffer and tracing interface is fully reused.
> programs are run as soon as event triggers.
> They can return non-zero and kernel will allocate ring
> buffer which user space will consume.
> Please take a look at tracex1
I see, this code itself is not a destructive change.
>>> Just look how ktap scripts look alike for kprobes and tracepoints.
>>
>> Ktap is a good example, it provides only a language parser and a runtime engine.
>> Actually, currently it lacks a feature to execute "perf-probe" helper from
>> script, but it is easy to add such feature.
> ...
>> For this usecase, I've made --output option for perf probe
>> https://lkml.org/lkml/2014/10/31/210
>
> you're proposing to call perf binary from ktap binary?
Yes, that's right :)
> I think packaging headaches and error conditions
> will make such approach very hard to use.
No, I don't think so. perf can be a "buffer" from the kernel API
and command-line API. If you need to get clearer error, you also
can join the upstream development.
> it would be much cleaner to have ktap as part of perf
> generating bpf on the fly and feeding into kernel.
> 'perf probe' parsing and functions don't belong in kernel
> when userspace can generate them in more efficient way.
No, perf probe still be needed to users who don't choose "injecting
binary blob" tracing. Efficiency is NOT only one index.
- perf probe and kprobe-event gives us a complete understandable
interface for what will be recorded at where.
(we can see the event definitions via kprobe_events interface,
without any tools)
- kprobe-event gives a completely same interface as other tracepoint
events.
- it also doesn't require any build-binary parts :) nor special tools.
We can play with ftrace on just a small busybox.
However, this does NOT interfere your patch upstreaming. I just said current
ftrace method is also meaningful for some reasons :)
By the way, I concern about that bpf compiler can become another systemtap,
especially if you build it on llvm. Would you plan to develop it on kernel
tree? or apart from the kernel-side development?
I think it is hard to sync the development if you do it out-of-tree.
Thank you,
--
Masami HIRAMATSU
Software Platform Research Dept. Linux Technology Research Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: masami.hiramatsu.pt@hitachi.com
^ permalink raw reply
* RE: [PATCH 1/2] if_link: Add VF multicast promiscuous mode control
From: David Laight @ 2015-01-20 11:53 UTC (permalink / raw)
To: 'Hiroshi Shimamoto', Alexander Duyck,
e1000-devel@lists.sourceforge.net
Cc: netdev@vger.kernel.org, Choi, Sy Jong, Hayato Momma,
linux-kernel@vger.kernel.org
In-Reply-To: <7F861DC0615E0C47A872E6F3C5FCDDBD05E0734E@BPXM14GP.gisp.nec.co.jp>
From: Hiroshi Shimamoto
> From: Hiroshi Shimamoto <h-shimamoto@ct.jp.nec.com>
>
> Add netlink directives and ndo entry to control VF multicast promiscuous mode.
>
> Intel ixgbe and ixgbevf driver can handle only 30 multicast MAC addresses
> per VF. It means that we cannot assign over 30 IPv6 addresses to a single
> VF interface on VM. We want thousands IPv6 addresses in VM.
>
> There is capability of multicast promiscuous mode in Intel 82599 chip.
> It enables all multicast packets are delivered to the target VF.
A lot of ethernet chips (certainly older ones) used a hash for the
multicast filter. So the hardware filter was never perfect.
I even know of one that needed to be told the addresses and used
those to set the hash bits.
For that device I generated a canonical set of MAC addresses (one
for each multicast hash) and did a software count of how many times
each one was needed for each of the enabled multicasts.
My guess is that 'multicast promiscuous mode' is always valid
and should be enabled by the relevant low level driver when
any table setup fails because there are too many entries.
I actually suspect that most systems want all the multicasts
that occur at any moderate rate on the local network segment.
David
^ permalink raw reply
* Re: [PATCH iproute2 3/3] ss: Unify tcp stats output
From: Hagen Paul Pfeifer @ 2015-01-20 11:43 UTC (permalink / raw)
To: Daniel Borkmann; +Cc: Vadim Kochan, netdev, Stephen Hemminger, Eric Dumazet
In-Reply-To: <54BE392F.2070804@redhat.com>
On 20 January 2015 at 12:17, Daniel Borkmann <dborkman@redhat.com> wrote:
> I have no strong opinion on this, but I also have a limited view on
> what applications try to parse ss output in general.
>
> As mentioned, for human readability, we should implement a top-like
> display option which is allowed to have a rather 'instable' output
> by nature and levels of detail can be folded/unfolded on demand.
+1 on that.
> Given the recent discussion on web10g, json output option might
> be very useful to provide i.e. when stats are being further extended.
Ok, there seems conses on JSON output.
Stephan, what do you think?
^ permalink raw reply
* Re: [PATCH 1/2] if_link: Add VF multicast promiscuous mode control
From: Bjørn Mork @ 2015-01-20 11:42 UTC (permalink / raw)
To: Hiroshi Shimamoto
Cc: Alexander Duyck, e1000-devel@lists.sourceforge.net,
netdev@vger.kernel.org, Choi, Sy Jong, Hayato Momma,
linux-kernel@vger.kernel.org
In-Reply-To: <7F861DC0615E0C47A872E6F3C5FCDDBD05E0734E@BPXM14GP.gisp.nec.co.jp>
Hiroshi Shimamoto <h-shimamoto@ct.jp.nec.com> writes:
> From: Hiroshi Shimamoto <h-shimamoto@ct.jp.nec.com>
>
> Add netlink directives and ndo entry to control VF multicast promiscuous mode.
>
> Intel ixgbe and ixgbevf driver can handle only 30 multicast MAC addresses
> per VF. It means that we cannot assign over 30 IPv6 addresses to a single
> VF interface on VM. We want thousands IPv6 addresses in VM.
>
> There is capability of multicast promiscuous mode in Intel 82599 chip.
> It enables all multicast packets are delivered to the target VF.
>
> This patch prepares to control that VF multicast promiscuous functionality.
Adding a new hook for this seems over-complicated to me. And it still
doesn't solve the real problems that
a) the user has to know about this limit, and
b) manually configure the feature
Most of us, lacking the ability to imagine such arbitrary hardware
limitations, will go through a few hours of frustrating debugging before
we figure this one out...
Why can't the ixgbevf driver just automatically signal the ixgbe driver
to enable multicast promiscuous mode whenever the list grows past the
limit?
I'd also like to note that this comment in
drivers/net/ethernet/intel/ixgbevf/vf.c
indicates that the author had some ideas about how more than 30
addresses could/should be handled:
static s32 ixgbevf_update_mc_addr_list_vf(struct ixgbe_hw *hw,
struct net_device *netdev)
{
struct netdev_hw_addr *ha;
u32 msgbuf[IXGBE_VFMAILBOX_SIZE];
u16 *vector_list = (u16 *)&msgbuf[1];
u32 cnt, i;
/* Each entry in the list uses 1 16 bit word. We have 30
* 16 bit words available in our HW msg buffer (minus 1 for the
* msg type). That's 30 hash values if we pack 'em right. If
* there are more than 30 MC addresses to add then punt the
* extras for now and then add code to handle more than 30 later.
* It would be unusual for a server to request that many multi-cast
* addresses except for in large enterprise network environments.
*/
The last 2 lines of that comment are of course totally bogus and
pointless and should be deleted in any case... It's obvious that 30
multicast addresses is ridiculously low for lots of normal use cases.
Bjørn
^ permalink raw reply
* Re: [RFC PATCHv1 net-next] xen-netback: always fully coalesce guest Rx packets
From: David Vrabel @ 2015-01-20 11:38 UTC (permalink / raw)
To: Ian Campbell; +Cc: netdev, xen-devel, Wei Liu
In-Reply-To: <1421753646.10440.228.camel@citrix.com>
On 20/01/15 11:34, Ian Campbell wrote:
> On Tue, 2015-01-13 at 14:05 +0000, David Vrabel wrote:
>> Always fully coalesce guest Rx packets into the minimum number of ring
>> slots. Reducing the number of slots per packet has significant
>> performance benefits (e.g., 7.2 Gbit/s to 11 Gbit/s in an off-host
>> receive test).
>>
>> However, this does increase the number of grant ops per packet which
>> decreases performance with some workloads (intrahost VM to VM)
>> /unless/ grant copy has been optimized for adjacent ops with the same
>> source or destination (see "grant-table: defer releasing pages
>> acquired in a grant copy"[1]).
>>
>> Do we need to retain the existing path and make the always coalesce
>> path conditional on a suitable version of Xen?
>>
>> [1] http://lists.xen.org/archives/html/xen-devel/2015-01/msg01118.html
>>
>> Signed-off-by: David Vrabel <david.vrabel@citrix.com>
>
> Looks good to me, thanks. Based on that plus the conversation in the
> other subthread:
>
> Acked-by: Ian Campbell <ian.campbell@citrix.com>
Thanks. I'll resend with a better commit message.
David
^ permalink raw reply
* Re: [RFC PATCHv1 net-next] xen-netback: always fully coalesce guest Rx packets
From: Ian Campbell @ 2015-01-20 11:34 UTC (permalink / raw)
To: David Vrabel; +Cc: netdev, xen-devel, Wei Liu
In-Reply-To: <1421157917-31333-1-git-send-email-david.vrabel@citrix.com>
On Tue, 2015-01-13 at 14:05 +0000, David Vrabel wrote:
> Always fully coalesce guest Rx packets into the minimum number of ring
> slots. Reducing the number of slots per packet has significant
> performance benefits (e.g., 7.2 Gbit/s to 11 Gbit/s in an off-host
> receive test).
>
> However, this does increase the number of grant ops per packet which
> decreases performance with some workloads (intrahost VM to VM)
> /unless/ grant copy has been optimized for adjacent ops with the same
> source or destination (see "grant-table: defer releasing pages
> acquired in a grant copy"[1]).
>
> Do we need to retain the existing path and make the always coalesce
> path conditional on a suitable version of Xen?
>
> [1] http://lists.xen.org/archives/html/xen-devel/2015-01/msg01118.html
>
> Signed-off-by: David Vrabel <david.vrabel@citrix.com>
Looks good to me, thanks. Based on that plus the conversation in the
other subthread:
Acked-by: Ian Campbell <ian.campbell@citrix.com>
^ permalink raw reply
* Re: [Xen-devel] [RFC PATCHv1 net-next] xen-netback: always fully coalesce guest Rx packets
From: Ian Campbell @ 2015-01-20 11:23 UTC (permalink / raw)
To: David Vrabel; +Cc: Wei Liu, netdev, xen-devel, Jonathan Davies
In-Reply-To: <54BD408A.6080405@citrix.com>
On Mon, 2015-01-19 at 17:36 +0000, David Vrabel wrote:
> On 13/01/15 14:30, Wei Liu wrote:
> > On Tue, Jan 13, 2015 at 02:05:17PM +0000, David Vrabel wrote:
> >> Always fully coalesce guest Rx packets into the minimum number of ring
> >> slots. Reducing the number of slots per packet has significant
> >> performance benefits (e.g., 7.2 Gbit/s to 11 Gbit/s in an off-host
> >> receive test).
> >>
> >
> > Good number.
> >
> >> However, this does increase the number of grant ops per packet which
> >> decreases performance with some workloads (intrahost VM to VM)
> >
> > Do you have figures before and after this change?
>
> Some better (more rigorous) results done by Jonathan Davies shows no
> regressions with full coalescing even without the grant copy
> optimization, and a big improvement to single stream receive.
>
> baseline Full coalesce
> Interhost aggregate 24 Gb/s 24 Gb/s
> Interhost VM receive 7.2 Gb/s 11 Gb/s
> Intrahost single stream 14 Gb/s 14 Gb/s
> Intrahost aggregate 34 Gb/s 34 Gb/s
Do you have a third column to hand, for Full-coalesce w/ grant copy
optimisation case? (Don't worry if not)
Ian.
^ permalink raw reply
* Re: [Xen-devel] [RFC PATCHv1 net-next] xen-netback: always fully coalesce guest Rx packets
From: Ian Campbell @ 2015-01-20 11:21 UTC (permalink / raw)
To: David Vrabel; +Cc: Wei Liu, netdev, xen-devel, Jonathan Davies
In-Reply-To: <54BD408A.6080405@citrix.com>
On Mon, 2015-01-19 at 17:36 +0000, David Vrabel wrote:
> On 13/01/15 14:30, Wei Liu wrote:
> > On Tue, Jan 13, 2015 at 02:05:17PM +0000, David Vrabel wrote:
> >> Always fully coalesce guest Rx packets into the minimum number of ring
> >> slots. Reducing the number of slots per packet has significant
> >> performance benefits (e.g., 7.2 Gbit/s to 11 Gbit/s in an off-host
> >> receive test).
> >>
> >
> > Good number.
> >
> >> However, this does increase the number of grant ops per packet which
> >> decreases performance with some workloads (intrahost VM to VM)
> >
> > Do you have figures before and after this change?
>
> Some better (more rigorous) results done by Jonathan Davies shows no
> regressions with full coalescing even without the grant copy
> optimization, and a big improvement to single stream receive.
>
> baseline Full coalesce
> Interhost aggregate 24 Gb/s 24 Gb/s
> Interhost VM receive 7.2 Gb/s 11 Gb/s
> Intrahost single stream 14 Gb/s 14 Gb/s
> Intrahost aggregate 34 Gb/s 34 Gb/s
>
> We do not measure the performance of dom0 to guest traffic but my ad-hoc
> measurements suggest this may be 5-10% slower. I don't think this is a
> very important use case though.
If you are updating your dom0 kernel to take advantage of this
improvement and you care about dom0->domU performance too then also
updating your Xen at the same is not a huge deal, I think. Or at least I
don't consider it a blocker for making progress (certainly not progress
of the order of 50% improvements!).
> So...
>
> >> /unless/ grant copy has been optimized for adjacent ops with the same
> >> source or destination (see "grant-table: defer releasing pages
> >> acquired in a grant copy"[1]).
> >>
> >> Do we need to retain the existing path and make the always coalesce
> >> path conditional on a suitable version of Xen?
>
> ...I think the answer to this is no.
Agreed.
> >> ---
> >> drivers/net/xen-netback/common.h | 1 -
> >> drivers/net/xen-netback/netback.c | 106 ++-----------------------------------
> >> 2 files changed, 3 insertions(+), 104 deletions(-)
> >
> > Love the diffstat!
>
> Yes, it's always nice when you delete code and it goes faster... :)
Full-Ack to that ;-)
Ian.
^ permalink raw reply
* Re: [PATCH iproute2 3/3] ss: Unify tcp stats output
From: Vadim Kochan @ 2015-01-20 11:09 UTC (permalink / raw)
To: Daniel Borkmann
Cc: Hagen Paul Pfeifer, Vadim Kochan, netdev, Stephen Hemminger,
Eric Dumazet
In-Reply-To: <54BE392F.2070804@redhat.com>
On Tue, Jan 20, 2015 at 12:17:03PM +0100, Daniel Borkmann wrote:
> Hi Hagen,
>
> On 01/20/2015 11:29 AM, Hagen Paul Pfeifer wrote:
> >On 18 January 2015 at 21:43, Vadim Kochan <vadim4j@gmail.com> wrote:
> ...
> >My proposal:
> >
> >1) support grouping. E.g. DCTCP could be separated in the following
> >manner. I.e. "dctcp:[ ce_state: %d, alpha: %d, ... ]" or
> >"dctcp:ce_state.%d;alpha,%d" like the socket memory output.
> > This makes it easier for humans and scripts to parse the output.
> >Since some time ss output is extended really extensive (especially
> >Eric make use of this). This is not the end and additional values are
> >added - make the current babylon even worse.
>
> I have no strong opinion on this, but I also have a limited view on
> what applications try to parse ss output in general.
>
> As mentioned, for human readability, we should implement a top-like
> display option which is allowed to have a rather 'instable' output
> by nature and levels of detail can be folded/unfolded on demand.
I think it would be good to have as option and output some default basic
info ?
>
> >2) add a JSON formater as soon as possible to make the output
> >parseable. I would do this - it is required anyway.
>
> Given the recent discussion on web10g, json output option might
> be very useful to provide i.e. when stats are being further extended.
>
> Cheers,
> Daniel
^ 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