* Re: [PATCH RFC net-next 1/7] net: Fix fib notifer to return errno
From: David Ahern @ 2018-03-25 14:00 UTC (permalink / raw)
To: Ido Schimmel; +Cc: netdev, davem, roopa, shm, jiri, idosch, jakub.kicinski
In-Reply-To: <20180325081632.GA11327@splinter>
On 3/25/18 2:16 AM, Ido Schimmel wrote:
> On Thu, Mar 22, 2018 at 03:57:51PM -0700, David Ahern wrote:
>> Notifier handlers use notifier_from_errno to convert any potential error
>> to an encoded format. As a consequence the other side, call_fib_notifiers
>> in this case, needs to use notifier_to_errno to return the error from
>> the handler back to its caller.
>>
>> Signed-off-by: David Ahern <dsa@cumulusnetworks.com>
>> ---
>> net/core/fib_notifier.c | 5 ++++-
>> 1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/net/core/fib_notifier.c b/net/core/fib_notifier.c
>> index 5ace0705a3f9..14ba52ebe8c9 100644
>> --- a/net/core/fib_notifier.c
>> +++ b/net/core/fib_notifier.c
>> @@ -21,8 +21,11 @@ EXPORT_SYMBOL(call_fib_notifier);
>> int call_fib_notifiers(struct net *net, enum fib_event_type event_type,
>> struct fib_notifier_info *info)
>
> There's another (less interesting case) - call_fib_notifier(), which is
> used to dump the FIB tables for the caller registering to the
> notification chain.
>
> For example, if you have a non-default FIB rule in the system and you
> modprobe mlxsw, you'll get a silent failure and routes will not be
> offloaded. On the other hand, I'm not sure we want to fail the module
> loading in such cases.
right. In normal cases the driver is loaded to create the netdevices
long before any networking config is done. So it seems to me the use
case you refer to, some user would have go out of there way to create a
situation where they install config that is not supported by the driver.
>
> A possible solution is to have the driver emit a warning via extack for
> each route/rule being notified after the abort mechanism was triggered.
extack is not available on module load.
Per past discussions, something you suggested, we need a message for
"out-of-line" cases like this where a driver notifies userspace of a
problem.
^ permalink raw reply
* RE: [PATCH iproute2-next] rdma: Move RDMA UAPI header file to be under RDMA responsibility
From: Steve Wise @ 2018-03-25 13:52 UTC (permalink / raw)
To: 'Leon Romanovsky', 'David Ahern'
Cc: 'Leon Romanovsky', 'netdev',
'RDMA mailing list', 'Stephen Hemminger'
In-Reply-To: <20180325063856.31709-1-leon@kernel.org>
> Subject: [PATCH iproute2-next] rdma: Move RDMA UAPI header file to be
> under RDMA responsibility
>
> From: Leon Romanovsky <leonro@mellanox.com>
>
> In iproute2 package, the updates of UAPIs files are performed
> after the needed feature lands in kernel's net-next tree.
>
> Such development flow created delays to the rdma tool developers,
> who uses rdma-next tree as a basis for their work.
>
> Move RDMA UAPI file to be under rdma/ folder, so whole responsibility
> of syncing this file will be on them.
>
> Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
Reviewed-by: Steve Wise <swise@opengridcomputing.com>
^ permalink raw reply
* Re: [PATCH v4 17/17] net: ena: Eliminate duplicate barriers on weakly-ordered archs
From: okaya @ 2018-03-25 13:33 UTC (permalink / raw)
To: Belgazal, Netanel
Cc: netdev, timur, sulrich, Kiyanovski, Arthur, linux-arm-msm,
linux-arm-kernel, Bshara, Saeed, Machulsky, Zorik,
David S. Miller, Tobias Klauser, linux-kernel
In-Reply-To: <7BF039D2-9E0F-45BE-9F40-8E785814C17D@amazon.com>
On 2018-03-25 08:06, Belgazal, Netanel wrote:
> I think you should either add a parameter to
> ena_com_write_sq_doorbell() or add ena_com_write_sq_doorbell_rel().
> Right now, you have unused function.
That is true. I got rid of ena_com_write_sq_doorbell_rel.
>
> On 3/20/18, 4:43 AM, "Sinan Kaya" <okaya@codeaurora.org> wrote:
>
> Code includes barrier() followed by writel(). writel() already has
> a
> barrier
> on some architectures like arm64.
>
> This ends up CPU observing two barriers back to back before
> executing the
> register write.
>
> Create a new wrapper function with relaxed write operator. Use the
> new
> wrapper when a write is following a barrier().
>
> Since code already has an explicit barrier call, changing writel()
> to
> writel_relaxed().
>
> Signed-off-by: Sinan Kaya <okaya@codeaurora.org>
> ---
> drivers/net/ethernet/amazon/ena/ena_com.c | 6 ++++--
> drivers/net/ethernet/amazon/ena/ena_eth_com.h | 22
> ++++++++++++++++++++--
> drivers/net/ethernet/amazon/ena/ena_netdev.c | 4 ++--
> 3 files changed, 26 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/net/ethernet/amazon/ena/ena_com.c
> b/drivers/net/ethernet/amazon/ena/ena_com.c
> index bf2de52..b6e628f 100644
> --- a/drivers/net/ethernet/amazon/ena/ena_com.c
> +++ b/drivers/net/ethernet/amazon/ena/ena_com.c
> @@ -631,7 +631,8 @@ static u32 ena_com_reg_bar_read32(struct
> ena_com_dev *ena_dev, u16 offset)
> */
> wmb();
>
> - writel(mmio_read_reg, ena_dev->reg_bar +
> ENA_REGS_MMIO_REG_READ_OFF);
> + writel_relaxed(mmio_read_reg,
> + ena_dev->reg_bar + ENA_REGS_MMIO_REG_READ_OFF);
>
> for (i = 0; i < timeout; i++) {
> if (read_resp->req_id == mmio_read->seq_num)
> @@ -1826,7 +1827,8 @@ void ena_com_aenq_intr_handler(struct
> ena_com_dev *dev, void *data)
>
> /* write the aenq doorbell after all AENQ descriptors were read
> */
> mb();
> - writel((u32)aenq->head, dev->reg_bar +
> ENA_REGS_AENQ_HEAD_DB_OFF);
> + writel_relaxed((u32)aenq->head,
> + dev->reg_bar + ENA_REGS_AENQ_HEAD_DB_OFF);
> }
>
> int ena_com_dev_reset(struct ena_com_dev *ena_dev,
> diff --git a/drivers/net/ethernet/amazon/ena/ena_eth_com.h
> b/drivers/net/ethernet/amazon/ena/ena_eth_com.h
> index 2f76572..09ef7cd 100644
> --- a/drivers/net/ethernet/amazon/ena/ena_eth_com.h
> +++ b/drivers/net/ethernet/amazon/ena/ena_eth_com.h
> @@ -107,7 +107,8 @@ static inline int
> ena_com_sq_empty_space(struct ena_com_io_sq *io_sq)
> return io_sq->q_depth - 1 - cnt;
> }
>
> -static inline int ena_com_write_sq_doorbell(struct ena_com_io_sq
> *io_sq)
> +static inline int ena_com_write_sq_doorbell(struct ena_com_io_sq
> *io_sq,
> + bool relaxed)
> {
> u16 tail;
>
> @@ -116,7 +117,24 @@ static inline int
> ena_com_write_sq_doorbell(struct ena_com_io_sq *io_sq)
> pr_debug("write submission queue doorbell for queue: %d tail:
> %d\n",
> io_sq->qid, tail);
>
> - writel(tail, io_sq->db_addr);
> + if (relaxed)
> + writel_relaxed(tail, io_sq->db_addr);
> + else
> + writel(tail, io_sq->db_addr);
> +
> + return 0;
> +}
> +
> +static inline int ena_com_write_sq_doorbell_rel(struct
> ena_com_io_sq *io_sq)
> +{
> + u16 tail;
> +
> + tail = io_sq->tail;
> +
> + pr_debug("write submission queue doorbell for queue: %d tail:
> %d\n",
> + io_sq->qid, tail);
> +
> + writel_relaxed(tail, io_sq->db_addr);
>
> return 0;
> }
> diff --git a/drivers/net/ethernet/amazon/ena/ena_netdev.c
> b/drivers/net/ethernet/amazon/ena/ena_netdev.c
> index 6975150..0530201 100644
> --- a/drivers/net/ethernet/amazon/ena/ena_netdev.c
> +++ b/drivers/net/ethernet/amazon/ena/ena_netdev.c
> @@ -556,7 +556,7 @@ static int ena_refill_rx_bufs(struct ena_ring
> *rx_ring, u32 num)
> * issue a doorbell
> */
> wmb();
> - ena_com_write_sq_doorbell(rx_ring->ena_com_io_sq);
> + ena_com_write_sq_doorbell(rx_ring->ena_com_io_sq, true);
> }
>
> rx_ring->next_to_use = next_to_use;
> @@ -2151,7 +2151,7 @@ static netdev_tx_t ena_start_xmit(struct
> sk_buff *skb, struct net_device *dev)
>
> if (netif_xmit_stopped(txq) || !skb->xmit_more) {
> /* trigger the dma engine */
> - ena_com_write_sq_doorbell(tx_ring->ena_com_io_sq);
> + ena_com_write_sq_doorbell(tx_ring->ena_com_io_sq, false);
> u64_stats_update_begin(&tx_ring->syncp);
> tx_ring->tx_stats.doorbells++;
> u64_stats_update_end(&tx_ring->syncp);
> --
> 2.7.4
^ permalink raw reply
* Re: [PATCH net-next v2 1/2] fs/crashdd: add API to collect hardware dump in second kernel
From: kbuild test robot @ 2018-03-25 12:43 UTC (permalink / raw)
To: Rahul Lakkireddy
Cc: kbuild-all, netdev, linux-fsdevel, kexec, linux-kernel, davem,
viro, ebiederm, stephen, akpm, torvalds, ganeshgr, nirranjan,
indranil, Rahul Lakkireddy
In-Reply-To: <296ffbd47fd4f30238689e636bd2480683224227.1521888444.git.rahul.lakkireddy@chelsio.com>
[-- Attachment #1: Type: text/plain, Size: 7182 bytes --]
Hi Rahul,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on net-next/master]
url: https://github.com/0day-ci/linux/commits/Rahul-Lakkireddy/fs-crashdd-add-API-to-collect-hardware-dump-in-second-kernel/20180325-191308
config: i386-randconfig-s0-03251817 (attached as .config)
compiler: gcc-6 (Debian 6.4.0-9) 6.4.0 20171026
reproduce:
# save the attached .config to linux build tree
make ARCH=i386
All warnings (new ones prefixed by >>):
In file included from fs//crashdd/crashdd.c:8:0:
fs//crashdd/crashdd_internal.h:13:23: error: field 'bin_attr' has incomplete type
struct bin_attribute bin_attr; /* Binary dump file's attributes */
^~~~~~~~
fs//crashdd/crashdd.c: In function 'crashdd_read':
fs//crashdd/crashdd.c:19:43: error: dereferencing pointer to incomplete type 'struct bin_attribute'
struct crashdd_dump_node *dump = bin_attr->private;
^~
fs//crashdd/crashdd.c: In function 'crashdd_mkdir':
fs//crashdd/crashdd.c:27:9: error: implicit declaration of function 'kobject_create_and_add' [-Werror=implicit-function-declaration]
return kobject_create_and_add(name, crashdd_kobj);
^~~~~~~~~~~~~~~~~~~~~~
fs//crashdd/crashdd.c:27:9: warning: return makes pointer from integer without a cast [-Wint-conversion]
return kobject_create_and_add(name, crashdd_kobj);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
fs//crashdd/crashdd.c: In function 'crashdd_add_file':
fs//crashdd/crashdd.c:39:9: error: implicit declaration of function 'sysfs_create_bin_file' [-Werror=implicit-function-declaration]
return sysfs_create_bin_file(kobj, &dump->bin_attr);
^~~~~~~~~~~~~~~~~~~~~
fs//crashdd/crashdd.c: In function 'crashdd_rmdir':
fs//crashdd/crashdd.c:44:2: error: implicit declaration of function 'kobject_put' [-Werror=implicit-function-declaration]
kobject_put(kobj);
^~~~~~~~~~~
In file included from include/linux/kernel.h:10:0,
from include/linux/list.h:9,
from include/linux/preempt.h:11,
from include/linux/spinlock.h:51,
from include/linux/vmalloc.h:5,
from fs//crashdd/crashdd.c:4:
fs//crashdd/crashdd.c: In function 'crashdd_get_driver':
fs//crashdd/crashdd.c:101:25: error: dereferencing pointer to incomplete type 'struct kobject'
if (!strcmp(node->kobj->name, name)) {
^
include/linux/compiler.h:58:30: note: in definition of macro '__trace_if'
if (__builtin_constant_p(!!(cond)) ? !!(cond) : \
^~~~
>> fs//crashdd/crashdd.c:101:3: note: in expansion of macro 'if'
if (!strcmp(node->kobj->name, name)) {
^~
fs//crashdd/crashdd.c: In function 'crashdd_init':
fs//crashdd/crashdd.c:227:51: error: 'kernel_kobj' undeclared (first use in this function)
crashdd_kobj = kobject_create_and_add("crashdd", kernel_kobj);
^~~~~~~~~~~
fs//crashdd/crashdd.c:227:51: note: each undeclared identifier is reported only once for each function it appears in
fs//crashdd/crashdd.c: In function 'crashdd_add_file':
fs//crashdd/crashdd.c:40:1: warning: control reaches end of non-void function [-Wreturn-type]
}
^
cc1: some warnings being treated as errors
vim +/if +101 fs//crashdd/crashdd.c
3
> 4 #include <linux/vmalloc.h>
5 #include <linux/crash_dump.h>
6 #include <linux/crashdd.h>
7
8 #include "crashdd_internal.h"
9
10 static LIST_HEAD(crashdd_list);
11 static DEFINE_MUTEX(crashdd_mutex);
12
13 static struct kobject *crashdd_kobj;
14
15 static ssize_t crashdd_read(struct file *filp, struct kobject *kobj,
16 struct bin_attribute *bin_attr,
17 char *buf, loff_t fpos, size_t count)
18 {
19 struct crashdd_dump_node *dump = bin_attr->private;
20
21 memcpy(buf, dump->buf + fpos, count);
22 return count;
23 }
24
25 static struct kobject *crashdd_mkdir(const char *name)
26 {
27 return kobject_create_and_add(name, crashdd_kobj);
28 }
29
30 static int crashdd_add_file(struct kobject *kobj, const char *name,
31 struct crashdd_dump_node *dump)
32 {
33 dump->bin_attr.attr.name = name;
34 dump->bin_attr.attr.mode = 0444;
35 dump->bin_attr.size = dump->size;
36 dump->bin_attr.read = crashdd_read;
37 dump->bin_attr.private = dump;
38
39 return sysfs_create_bin_file(kobj, &dump->bin_attr);
40 }
41
42 static void crashdd_rmdir(struct kobject *kobj)
43 {
44 kobject_put(kobj);
45 }
46
47 /**
48 * crashdd_init_driver - create a sysfs driver entry.
49 * @name: Name of the directory.
50 *
51 * Creates a directory under /sys/kernel/crashdd/ with @name. Allocates
52 * and saves the sysfs entry. The sysfs entry is added to the global
53 * list and then returned to the caller. On failure, returns NULL.
54 */
55 static struct crashdd_driver_node *crashdd_init_driver(const char *name)
56 {
57 struct crashdd_driver_node *node;
58
59 node = vzalloc(sizeof(*node));
60 if (!node)
61 return NULL;
62
63 /* Create a driver's directory under /sys/kernel/crashdd/ */
64 node->kobj = crashdd_mkdir(name);
65 if (!node->kobj) {
66 vfree(node);
67 return NULL;
68 }
69
70 atomic_set(&node->refcnt, 1);
71
72 /* Initialize the list of dumps that go under this driver's
73 * directory.
74 */
75 INIT_LIST_HEAD(&node->dump_list);
76
77 /* Add the driver's entry to global list */
78 mutex_lock(&crashdd_mutex);
79 list_add_tail(&node->list, &crashdd_list);
80 mutex_unlock(&crashdd_mutex);
81
82 return node;
83 }
84
85 /**
86 * crashdd_get_driver - get an exisiting sysfs driver entry.
87 * @name: Name of the directory.
88 *
89 * Searches and fetches a sysfs entry having @name. If @name is
90 * found, then the reference count is incremented and the entry
91 * is returned. If @name is not found, NULL is returned.
92 */
93 static struct crashdd_driver_node *crashdd_get_driver(const char *name)
94 {
95 struct crashdd_driver_node *node;
96 int found = 0;
97
98 /* Search for an existing driver sysfs entry having @name */
99 mutex_lock(&crashdd_mutex);
100 list_for_each_entry(node, &crashdd_list, list) {
> 101 if (!strcmp(node->kobj->name, name)) {
102 atomic_inc(&node->refcnt);
103 found = 1;
104 break;
105 }
106 }
107 mutex_unlock(&crashdd_mutex);
108
109 if (found)
110 return node;
111
112 /* No driver with @name found */
113 return NULL;
114 }
115
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 32369 bytes --]
^ permalink raw reply
* Re: [PATCH v4 17/17] net: ena: Eliminate duplicate barriers on weakly-ordered archs
From: Belgazal, Netanel @ 2018-03-25 12:06 UTC (permalink / raw)
To: Sinan Kaya, netdev@vger.kernel.org, timur@codeaurora.org,
sulrich@codeaurora.org, Kiyanovski, Arthur
Cc: linux-arm-msm@vger.kernel.org,
linux-arm-kernel@lists.infradead.org, Bshara, Saeed,
Machulsky, Zorik, David S. Miller, Tobias Klauser,
linux-kernel@vger.kernel.org
In-Reply-To: <1521513753-7325-18-git-send-email-okaya@codeaurora.org>
I think you should either add a parameter to ena_com_write_sq_doorbell() or add ena_com_write_sq_doorbell_rel().
Right now, you have unused function.
On 3/20/18, 4:43 AM, "Sinan Kaya" <okaya@codeaurora.org> wrote:
Code includes barrier() followed by writel(). writel() already has a
barrier
on some architectures like arm64.
This ends up CPU observing two barriers back to back before executing the
register write.
Create a new wrapper function with relaxed write operator. Use the new
wrapper when a write is following a barrier().
Since code already has an explicit barrier call, changing writel() to
writel_relaxed().
Signed-off-by: Sinan Kaya <okaya@codeaurora.org>
---
drivers/net/ethernet/amazon/ena/ena_com.c | 6 ++++--
drivers/net/ethernet/amazon/ena/ena_eth_com.h | 22 ++++++++++++++++++++--
drivers/net/ethernet/amazon/ena/ena_netdev.c | 4 ++--
3 files changed, 26 insertions(+), 6 deletions(-)
diff --git a/drivers/net/ethernet/amazon/ena/ena_com.c b/drivers/net/ethernet/amazon/ena/ena_com.c
index bf2de52..b6e628f 100644
--- a/drivers/net/ethernet/amazon/ena/ena_com.c
+++ b/drivers/net/ethernet/amazon/ena/ena_com.c
@@ -631,7 +631,8 @@ static u32 ena_com_reg_bar_read32(struct ena_com_dev *ena_dev, u16 offset)
*/
wmb();
- writel(mmio_read_reg, ena_dev->reg_bar + ENA_REGS_MMIO_REG_READ_OFF);
+ writel_relaxed(mmio_read_reg,
+ ena_dev->reg_bar + ENA_REGS_MMIO_REG_READ_OFF);
for (i = 0; i < timeout; i++) {
if (read_resp->req_id == mmio_read->seq_num)
@@ -1826,7 +1827,8 @@ void ena_com_aenq_intr_handler(struct ena_com_dev *dev, void *data)
/* write the aenq doorbell after all AENQ descriptors were read */
mb();
- writel((u32)aenq->head, dev->reg_bar + ENA_REGS_AENQ_HEAD_DB_OFF);
+ writel_relaxed((u32)aenq->head,
+ dev->reg_bar + ENA_REGS_AENQ_HEAD_DB_OFF);
}
int ena_com_dev_reset(struct ena_com_dev *ena_dev,
diff --git a/drivers/net/ethernet/amazon/ena/ena_eth_com.h b/drivers/net/ethernet/amazon/ena/ena_eth_com.h
index 2f76572..09ef7cd 100644
--- a/drivers/net/ethernet/amazon/ena/ena_eth_com.h
+++ b/drivers/net/ethernet/amazon/ena/ena_eth_com.h
@@ -107,7 +107,8 @@ static inline int ena_com_sq_empty_space(struct ena_com_io_sq *io_sq)
return io_sq->q_depth - 1 - cnt;
}
-static inline int ena_com_write_sq_doorbell(struct ena_com_io_sq *io_sq)
+static inline int ena_com_write_sq_doorbell(struct ena_com_io_sq *io_sq,
+ bool relaxed)
{
u16 tail;
@@ -116,7 +117,24 @@ static inline int ena_com_write_sq_doorbell(struct ena_com_io_sq *io_sq)
pr_debug("write submission queue doorbell for queue: %d tail: %d\n",
io_sq->qid, tail);
- writel(tail, io_sq->db_addr);
+ if (relaxed)
+ writel_relaxed(tail, io_sq->db_addr);
+ else
+ writel(tail, io_sq->db_addr);
+
+ return 0;
+}
+
+static inline int ena_com_write_sq_doorbell_rel(struct ena_com_io_sq *io_sq)
+{
+ u16 tail;
+
+ tail = io_sq->tail;
+
+ pr_debug("write submission queue doorbell for queue: %d tail: %d\n",
+ io_sq->qid, tail);
+
+ writel_relaxed(tail, io_sq->db_addr);
return 0;
}
diff --git a/drivers/net/ethernet/amazon/ena/ena_netdev.c b/drivers/net/ethernet/amazon/ena/ena_netdev.c
index 6975150..0530201 100644
--- a/drivers/net/ethernet/amazon/ena/ena_netdev.c
+++ b/drivers/net/ethernet/amazon/ena/ena_netdev.c
@@ -556,7 +556,7 @@ static int ena_refill_rx_bufs(struct ena_ring *rx_ring, u32 num)
* issue a doorbell
*/
wmb();
- ena_com_write_sq_doorbell(rx_ring->ena_com_io_sq);
+ ena_com_write_sq_doorbell(rx_ring->ena_com_io_sq, true);
}
rx_ring->next_to_use = next_to_use;
@@ -2151,7 +2151,7 @@ static netdev_tx_t ena_start_xmit(struct sk_buff *skb, struct net_device *dev)
if (netif_xmit_stopped(txq) || !skb->xmit_more) {
/* trigger the dma engine */
- ena_com_write_sq_doorbell(tx_ring->ena_com_io_sq);
+ ena_com_write_sq_doorbell(tx_ring->ena_com_io_sq, false);
u64_stats_update_begin(&tx_ring->syncp);
tx_ring->tx_stats.doorbells++;
u64_stats_update_end(&tx_ring->syncp);
--
2.7.4
^ permalink raw reply
* Re: [RFC v3 net-next 13/18] net/sched: Introduce the TBS Qdisc
From: Thomas Gleixner @ 2018-03-25 11:46 UTC (permalink / raw)
To: Jesus Sanchez-Palencia
Cc: netdev, jhs, xiyou.wangcong, jiri, vinicius.gomes, richardcochran,
anna-maria, henrik, John Stultz, levi.pearson, edumazet, willemb,
mlichvar
In-Reply-To: <2897b562-06e0-0fcc-4fb1-e8c4469c0faa@intel.com>
On Fri, 23 Mar 2018, Jesus Sanchez-Palencia wrote:
> On 03/22/2018 03:52 PM, Thomas Gleixner wrote:
> > So what's the plan for this? Having TAS as a separate entity or TAS feeding
> > into the proposed 'basic' time transmission thing?
>
> The second one, I guess.
That's just wrong. It won't work. See below.
> Elaborating, the plan is at some point having TAS as a separate entity,
> but which can use tbs for one of its classes (and cbs for another, and
> strict priority for everything else, etc).
>
> Basically, the design would something along the lines of 'taprio'. A root qdisc
> that is both time and priority aware, and capable of running a schedule for the
> port. That schedule can run inside the kernel with hrtimers, or just be
> offloaded into the controller if Qbv is supported on HW.
>
> Because it would expose the inner traffic classes in a mq / mqprio / prio style,
> then it would allow for other per-queue qdiscs to be attached to it. On a system
> using the i210, for instance, we could then have tbs installed on traffic class
> 0 just dialing hw offload. The Qbv schedule would be running in SW on the TAS
> entity (i.e. 'taprio') which would be setting the packets' txtime before
> dequeueing packets on a fast path -> tbs -> NIC.
>
> Similarly, other qdisc, like cbs, could be installed if all that traffic class
> requires is traffic shaping once its 'gate' is allowed to execute the selected
> tx algorithm attached to it.
>
> > I've not yet seen a convincing argument why this low level stuff with all
> > of its weird flavours is superiour over something which reflects the basic
> > operating principle of TSN.
>
>
> As you know, not all TSN systems are designed the same. Take AVB systems, for
> example. These not always are running on networks that are aware of any time
> schedule, or at least not quite like what is described by Qbv.
>
> On those systems there is usually a certain number of streams with different
> priorities that care mostly about having their bandwidth reserved along the
> network. The applications running on such systems are usually based on AVTP,
> thus they already have to calculate and set the "avtp presentation time"
> per-packet themselves. A Qbv scheduler would probably provide very little
> benefits to this domain, IMHO. For "talkers" of these AVB systems, shaping
> traffic using txtime (i.e. tbs) can provide a low-jitter alternative to cbs, for
> instance.
You're looking at it from particular use cases and try to accomodate for
them in the simplest possible way. I don't think that cuts it.
Let's take a step back and look at it from a more general POV without
trying to make it fit to any of the standards first. I'm deliberately NOT
using any of the standard defined terms.
At the (local) network level you have always an explicit plan. This plan
might range from no plan at all to an very elaborate plan which is strict
about when each node is allowed to TX a particular class of packets.
So lets assume we have the following picture:
[NIC]
|
[ Time slice manager ]
Now in the simplest case, the time slice manager has no constraints and
exposes a single input which allows the application to say: "Send my packet
at time X". There is no restriction on 'time X' except if there is a time
collision with an already queued packet or the requested TX time has
already passed. That's close to what you implemented.
Is the TX timestamp which you defined in the user space ABI a fixed
scheduling point or is it a deadline?
That's an important distinction and for this all to work accross various
use cases you need a way to express that in the ABI. It might be an
implicit property of the socket/channel to which the application connects
to but still you want to express it from the application side to do
proper sanity checking.
Just think about stuff like audio/video streaming. The point of
transmission does not have to be fixed if you have some intelligent
controller at the receiving end which can buffer stuff. The only relevant
information is the deadline, i.e. the latest point in time where the
packet needs to go out on the wire in order to keep the stream steady at
the consumer side. Having the notion of a deadline and that's the only
thing the provider knows about allows you proper utilization by using an
approriate scheduling algorithm like EDF.
Contrary to that you want very explicit TX points for applications like
automation control. For this kind of use case there is no wiggle room, it
has to go out at a fixed time because that's the way control systems
work.
This is missing right now and you want to get that right from the very
beginning. Duct taping it on the interface later on is a bad idea.
Now lets go one step further and create two time slices for whatever
purpose still on the single node (not network wide). You want to do that
because you want temporal separation of services. The reason might be
bandwidth guarantee, collission avoidance or whatever.
How does the application which was written for the simple manager which
had no restrictions learn about this?
Does it learn it the hard way because now the packets which fall into the
reserved timeslice are rejected? The way you created your interface, the
answer is yes. That's patently bad as it requires to change the
application once it runs on a partitioned node.
So you really want a way for the application to query the timing
constraints and perhaps other properties of the channel it connects
to. And you want that now before the first application starts to use the
new ABI. If the application developer does not use it, you still have to
fix the application, but you have to fix it because the developer was a
lazy bastard and not because the design was bad. That's a major
difference.
Now that we have two time slices, I'm coming back to your idea of having
your proposed qdisc as the entity which sits right at the network
interface. Lets assume the following:
[Slice 1: Timed traffic ] [Slice 2: Other Traffic]
Lets assume further that 'Other traffic' has no idea about time slices at
all. It's just stuff like ssh, http, etc. So if you keep that design
[ NIC ]
|
[ Time slice manager ]
| |
[ Timed traffic ] [ Other traffic ]
feeding into your proposed TBS thingy, then in case of underutilization
of the 'Timed traffic' slot you prevent utilization of remaining time by
pulling 'Other traffic' into the empty slots because 'Other traffic' is
restricted to Slice 2 and 'Timed traffic' does not know about 'Other
traffic' at all. And no, you cannot make TBS magically pull packets from
'Other traffic' just because its not designed for it. So your design
becomes strictly partitioned and forces underutilization.
That's becoming even worse, when you switch to the proposed full hardware
offloading scheme. In that case the only way to do admission control is
the TX time of the farthest out packet which is already queued. That
might work for a single application which controls all of the network
traffic, but it wont ever work for something more flexible. The more I
think about it the less interesting full hardware offload becomes. It's
nice if you have a fully strict scheduling plan for everything, but then
your admission control is bogus once you have more than one channel as
input. So yes, it can be used when the card supports it and you have
other ways to enforce admission control w/o hurting utilization or if you
don't care about utilization at all. It's also useful for channels which
are strictly isolated and have a defined TX time. Such traffic can be
directly fed into the hardware.
Coming back to the overall scheme. If you start upfront with a time slice
manager which is designed to:
- Handle multiple channels
- Expose the time constraints, properties per channel
then you can fit all kind of use cases, whether designed by committee or
not. You can configure that thing per node or network wide. It does not
make a difference. The only difference are the resulting constraints.
We really want to accomodate everything between the 'no restrictions' and
the 'full network wide explicit plan' case. And it's not rocket science
once you realize that the 'no restrictions' case is just a subset of the
'full network wide explicit plan' simply because it exposes a single
channel where:
slice period = slice length.
It's that easy, but at the same time you teach the application from the
very beginning to ask for the time constraints so if it runs on a more
sophisticated system/network, then it will see a different slice period and
a different slice length and can accomodate or react in a useful way
instead of just dying on the 17th packet it tries to send because it is
rejected.
We really want to design for this as we want to be able to run the video
stream on the same node and network which does robot control without
changing the video application. That's not a theoretical problem. These use
cases exist today, but they are forced to use different networks for the
two. But if you look at the utilization of both then they very well fit
into one and industry certainly wants to go for that.
That implies that you need constraint aware applications from the very
beginning and that requires a proper ABI in the first place. The proposed
ad hoc mode does not qualify. Please be aware, that you are creating a user
space ABI and not a random in kernel interface which can be changed at any
given time.
So lets look once more at the picture in an abstract way:
[ NIC ]
|
[ Time slice manager ]
| |
[ Ch 0 ] ... [ Ch N ]
So you have a bunch of properties here:
1) Number of Channels ranging from 1 to N
2) Start point, slice period and slice length per channel
3) Queueing modes assigned per channel. Again that might be anything from
'feed through' over FIFO, PRIO to more complex things like EDF.
The queueing mode can also influence properties like the meaning of the
TX time, i.e. strict or deadline.
Please sit back and map your use cases, standards or whatever you care
about into the above and I would be very surprised if they don't fit.
Thanks,
tglx
^ permalink raw reply
* Hello Beautiful
From: Jack @ 2018-03-25 10:50 UTC (permalink / raw)
Hi Dear, my name is Jack and i am seeking for a relationship in which i will feel loved after a series of failed relationships.
I am hoping that you would be interested and we could possibly get to know each other more if you do not mind. I am open to answering questions from you as i think my approach is a little inappropriate. Hope to hear back from you.
Jack.
^ permalink raw reply
* Re: [net-next 02/15] ethtool: Add support for configuring PFC stall prevention in ethtool
From: Gal Pressman @ 2018-03-25 10:30 UTC (permalink / raw)
To: Andrew Lunn, Saeed Mahameed
Cc: David S. Miller, netdev, Inbar Karmy, Michal Kubecek
In-Reply-To: <20180324145703.GC31941@lunn.ch>
On 24-Mar-18 17:57, Andrew Lunn wrote:
> On Fri, Mar 23, 2018 at 03:39:12PM -0700, Saeed Mahameed wrote:
>> From: Inbar Karmy <inbark@mellanox.com>
>>
>> In the event where the device unexpectedly becomes unresponsive
>> for a long period of time, flow control mechanism may propagate
>> pause frames which will cause congestion spreading to the entire
>> network.
>> To prevent this scenario, when the device is stalled for a period
>> longer than a pre-configured timeout, flow control mechanisms are
>> automatically disabled.
>>
>> This patch adds support for the ETHTOOL_PFC_STALL_PREVENTION
>> as a tunable.
>> This API provides support for configuring flow control storm prevention
>> timeout (msec).
>>
>> Signed-off-by: Inbar Karmy <inbark@mellanox.com>
>> Cc: Michal Kubecek <mkubecek@suse.cz>
>> Cc: Andrew Lunn <andrew@lunn.ch>
>> Signed-off-by: Saeed Mahameed <saeedm@mellanox.com>
>> ---
>> include/uapi/linux/ethtool.h | 4 ++++
>> net/core/ethtool.c | 6 ++++++
>> 2 files changed, 10 insertions(+)
>>
>> diff --git a/include/uapi/linux/ethtool.h b/include/uapi/linux/ethtool.h
>> index 20da156aaf64..9dc63a14a747 100644
>> --- a/include/uapi/linux/ethtool.h
>> +++ b/include/uapi/linux/ethtool.h
>> @@ -217,10 +217,14 @@ struct ethtool_value {
>> __u32 data;
>> };
>>
>> +#define PFC_STORM_PREVENTION_AUTO 0xffff
>> +#define PFC_STORM_PREVENTION_DISABLE 0
>> +
>> enum tunable_id {
>> ETHTOOL_ID_UNSPEC,
>> ETHTOOL_RX_COPYBREAK,
>> ETHTOOL_TX_COPYBREAK,
>> + ETHTOOL_PFC_PREVENTION_TOUT,
>
> Hi Inbar
>
> Please could you add a comment here about the units. Ideally we want
> this file to be self documenting.
Thank you for the review, we will fix that.
>
> Andrew
>
^ permalink raw reply
* Re: [net-next 03/15] net/mlx5e: PFC stall prevention support
From: Gal Pressman @ 2018-03-25 10:28 UTC (permalink / raw)
To: Andrew Lunn, Saeed Mahameed; +Cc: David S. Miller, netdev, Inbar Karmy
In-Reply-To: <20180324150709.GD31941@lunn.ch>
On 24-Mar-18 18:07, Andrew Lunn wrote:
> On Fri, Mar 23, 2018 at 03:39:13PM -0700, Saeed Mahameed wrote:
>> From: Inbar Karmy <inbark@mellanox.com>
>>
>> Implement set/get functions to configure PFC stall prevention
>> timeout by tunables api through ethtool.
>> By default the stall prevention timeout is configured to 8 sec.
>> Timeout range is: 80-8000 msec.
>> Enabling stall prevention without a specific timeout will set
>> the timeout to 100 msec.
>
> Hi Inbar
>
> I think this last sentence should be talking about auto, not without a
> specific timeout.
Hi Andrew,
Good catch, we will fix that.
>
>>
>> Signed-off-by: Inbar Karmy <inbark@mellanox.com>
>> Signed-off-by: Saeed Mahameed <saeedm@mellanox.com>
>> ---
>> .../net/ethernet/mellanox/mlx5/core/en_ethtool.c | 57 +++++++++++++++++++
>> drivers/net/ethernet/mellanox/mlx5/core/port.c | 64 +++++++++++++++++++---
>> include/linux/mlx5/mlx5_ifc.h | 17 ++++--
>> include/linux/mlx5/port.h | 6 ++
>> 4 files changed, 132 insertions(+), 12 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c b/drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c
>> index cc8048f68f11..62061fd23143 100644
>> --- a/drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c
>> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c
>> @@ -1066,6 +1066,57 @@ static int mlx5e_get_rxnfc(struct net_device *netdev,
>> return err;
>> }
>>
>> +#define MLX5E_PFC_PREVEN_AUTO_TOUT_MSEC 100
>> +#define MLX5E_PFC_PREVEN_TOUT_MAX_MSEC 8000
>> +#define MLX5E_PFC_PREVEN_MINOR_PRECENT 85
>> +#define MLX5E_PFC_PREVEN_TOUT_MIN_MSEC 80
>> +#define MLX5E_DEVICE_STALL_MINOR_WATERMARK(critical_tout) \
>> + max_t(u16, MLX5E_PFC_PREVEN_TOUT_MIN_MSEC, \
>> + (critical_tout * MLX5E_PFC_PREVEN_MINOR_PRECENT) / 100)
>> +
>> +static int mlx5e_get_pfc_prevention_tout(struct net_device *netdev,
>> + u16 *pfc_prevention_tout)
>> +{
>> + struct mlx5e_priv *priv = netdev_priv(netdev);
>> + struct mlx5_core_dev *mdev = priv->mdev;
>> +
>> + if (!MLX5_CAP_PCAM_FEATURE((priv)->mdev, pfcc_mask) ||
>> + !MLX5_CAP_DEBUG((priv)->mdev, stall_detect))
>> + return -EOPNOTSUPP;
>> +
>> + return mlx5_query_port_stall_watermark(mdev, pfc_prevention_tout, NULL);
>
> Shouldn't you map a value of MLX5E_PFC_PREVEN_AUTO_TOUT_MSEC back to
> PFC_STORM_PREVENTION_AUTO?
We discussed this point internally, mapping MLX5E_PFC_PREVEN_AUTO_TOUT_MSEC (100) to
PFC_STORM_PREVENTION_AUTO might cause confusion when the user explicitly asks for 100msec timeout
and gets auto in his following query.
Also, this way the "auto" timeout is visible to the user, which might help him get an initial
clue of which values are recommended.
>
> Andrew
>
^ permalink raw reply
* Hello Beautiful
From: Jack @ 2018-03-25 8:59 UTC (permalink / raw)
Cześć Drogi, nazywam się Jack i szukam związku, w którym będę czuć się kochany po serii nieudanych związków.
Mam nadzieję, że byłbyś zainteresowany i moglibyśmy się lepiej poznać, jeśli nie masz nic przeciwko. Jestem otwarty na udzielanie odpowiedzi na pytania od ciebie, ponieważ uważam, że moje podejście jest trochę niewłaściwe. Mam nadzieję, że odezwę się od ciebie.
Jacek.
^ permalink raw reply
* KASAN: use-after-free Read in pppol2tp_connect (3)
From: syzbot @ 2018-03-25 9:06 UTC (permalink / raw)
To: davem, jchapman, linux-kernel, netdev, syzkaller-bugs
Hello,
syzbot hit the following crash on upstream commit
bcfc1f4554662d8f2429ac8bd96064a59c149754 (Sat Mar 24 16:50:12 2018 +0000)
Merge tag 'pinctrl-v4.16-3' of
git://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-pinctrl
syzbot dashboard link:
https://syzkaller.appspot.com/bug?extid=fbeeb5c3b538e8545644
So far this crash happened 5 times on net-next, upstream.
C reproducer: https://syzkaller.appspot.com/x/repro.c?id=5150723616538624
syzkaller reproducer:
https://syzkaller.appspot.com/x/repro.syz?id=4732053023096832
Raw console output:
https://syzkaller.appspot.com/x/log.txt?id=5847288391925760
Kernel config:
https://syzkaller.appspot.com/x/.config?id=-5034017172441945317
compiler: gcc (GCC) 7.1.1 20170620
IMPORTANT: if you fix the bug, please add the following tag to the commit:
Reported-by: syzbot+fbeeb5c3b538e8545644@syzkaller.appspotmail.com
It will help syzbot understand when the bug is fixed. See footer for
details.
If you forward the report, please keep this part and the footer.
l2tp_core: tunl 2: sockfd_lookup(fd=9) returned -9
l2tp_core: tunl 2: sockfd_lookup(fd=8) returned -9
l2tp_core: tunl 2: sockfd_lookup(fd=9) returned -9
==================================================================
BUG: KASAN: use-after-free in __read_once_size include/linux/compiler.h:188
[inline]
BUG: KASAN: use-after-free in sk_dst_get include/net/sock.h:1767 [inline]
BUG: KASAN: use-after-free in pppol2tp_session_init net/l2tp/l2tp_ppp.c:587
[inline]
BUG: KASAN: use-after-free in pppol2tp_connect+0x1a98/0x1dd0
net/l2tp/l2tp_ppp.c:747
Read of size 8 at addr ffff8801d18242a8 by task syzkaller117300/8372
CPU: 1 PID: 8372 Comm: syzkaller117300 Not tainted 4.16.0-rc6+ #365
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
Google 01/01/2011
Call Trace:
__dump_stack lib/dump_stack.c:17 [inline]
dump_stack+0x194/0x24d lib/dump_stack.c:53
print_address_description+0x73/0x250 mm/kasan/report.c:256
kasan_report_error mm/kasan/report.c:354 [inline]
kasan_report+0x23c/0x360 mm/kasan/report.c:412
__asan_report_load8_noabort+0x14/0x20 mm/kasan/report.c:433
__read_once_size include/linux/compiler.h:188 [inline]
sk_dst_get include/net/sock.h:1767 [inline]
pppol2tp_session_init net/l2tp/l2tp_ppp.c:587 [inline]
pppol2tp_connect+0x1a98/0x1dd0 net/l2tp/l2tp_ppp.c:747
SYSC_connect+0x213/0x4a0 net/socket.c:1639
SyS_connect+0x24/0x30 net/socket.c:1620
do_syscall_64+0x281/0x940 arch/x86/entry/common.c:287
entry_SYSCALL_64_after_hwframe+0x42/0xb7
RIP: 0033:0x449f39
RSP: 002b:00007fa9d92efce8 EFLAGS: 00000246 ORIG_RAX: 000000000000002a
RAX: ffffffffffffffda RBX: 0000000000700024 RCX: 0000000000449f39
RDX: 000000000000002e RSI: 0000000020e92000 RDI: 0000000000000009
RBP: 0000000000700020 R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000000
R13: 000000000080efcf R14: 00007fa9d92f09c0 R15: 0000000000000009
Allocated by task 8381:
save_stack+0x43/0xd0 mm/kasan/kasan.c:447
set_track mm/kasan/kasan.c:459 [inline]
kasan_kmalloc+0xad/0xe0 mm/kasan/kasan.c:552
kasan_slab_alloc+0x12/0x20 mm/kasan/kasan.c:489
kmem_cache_alloc+0x12e/0x760 mm/slab.c:3541
sk_prot_alloc+0x65/0x2a0 net/core/sock.c:1465
sk_alloc+0x105/0x1440 net/core/sock.c:1525
inet_create+0x47c/0xf50 net/ipv4/af_inet.c:320
__sock_create+0x4d4/0x850 net/socket.c:1285
sock_create net/socket.c:1325 [inline]
SYSC_socket net/socket.c:1355 [inline]
SyS_socket+0xeb/0x1d0 net/socket.c:1335
do_syscall_64+0x281/0x940 arch/x86/entry/common.c:287
entry_SYSCALL_64_after_hwframe+0x42/0xb7
Freed by task 21:
save_stack+0x43/0xd0 mm/kasan/kasan.c:447
set_track mm/kasan/kasan.c:459 [inline]
__kasan_slab_free+0x11a/0x170 mm/kasan/kasan.c:520
kasan_slab_free+0xe/0x10 mm/kasan/kasan.c:527
__cache_free mm/slab.c:3485 [inline]
kmem_cache_free+0x83/0x2a0 mm/slab.c:3743
sk_prot_free net/core/sock.c:1506 [inline]
__sk_destruct+0x628/0x920 net/core/sock.c:1590
sk_destruct+0x47/0x80 net/core/sock.c:1598
__sk_free+0xf1/0x2b0 net/core/sock.c:1609
sk_free+0x2a/0x40 net/core/sock.c:1620
sock_put include/net/sock.h:1659 [inline]
l2tp_tunnel_free net/l2tp/l2tp_core.c:160 [inline]
l2tp_tunnel_dec_refcount net/l2tp/l2tp_core.h:264 [inline]
l2tp_tunnel_del_work+0x474/0x6a0 net/l2tp/l2tp_core.c:1307
process_one_work+0xc47/0x1bb0 kernel/workqueue.c:2113
worker_thread+0x223/0x1990 kernel/workqueue.c:2247
kthread+0x33c/0x400 kernel/kthread.c:238
ret_from_fork+0x3a/0x50 arch/x86/entry/entry_64.S:406
The buggy address belongs to the object at ffff8801d1824080
which belongs to the cache UDP of size 1472
The buggy address is located 552 bytes inside of
1472-byte region [ffff8801d1824080, ffff8801d1824640)
The buggy address belongs to the page:
page:ffffea0007460900 count:1 mapcount:0 mapping:ffff8801d1824080 index:0x0
compound_mapcount: 0
flags: 0x2fffc0000008100(slab|head)
raw: 02fffc0000008100 ffff8801d1824080 0000000000000000 0000000100000005
raw: ffffea0006b0fda0 ffffea0006a53820 ffff8801d6f36640 0000000000000000
page dumped because: kasan: bad access detected
Memory state around the buggy address:
ffff8801d1824180: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
ffff8801d1824200: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
> ffff8801d1824280: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
^
ffff8801d1824300: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
ffff8801d1824380: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
==================================================================
---
This bug is generated by a dumb bot. It may contain errors.
See https://goo.gl/tpsmEJ for details.
Direct all questions to syzkaller@googlegroups.com.
syzbot will keep track of this bug report.
If you forgot to add the Reported-by tag, once the fix for this bug is
merged
into any tree, please reply to this email with:
#syz fix: exact-commit-title
If you want to test a patch for this bug, please reply with:
#syz test: git://repo/address.git branch
and provide the patch inline or as an attachment.
To mark this as a duplicate of another syzbot report, please reply with:
#syz dup: exact-subject-of-another-report
If it's a one-off invalid bug report, please reply with:
#syz invalid
Note: if the crash happens again, it will cause creation of a new bug
report.
Note: all commands must start from beginning of the line in the email body.
^ permalink raw reply
* Re: [PATCH] of_net: Implement of_get_nvmem_mac_address helper
From: Mike Looijmans @ 2018-03-25 8:17 UTC (permalink / raw)
To: Andrew Lunn
Cc: Florian Fainelli, netdev, linux-kernel, devicetree, robh+dt,
frowand.list
In-Reply-To: <20180324185324.GG31941@lunn.ch>
On 24-03-18 19:53, Andrew Lunn wrote:
>> A quick survey for the of_get_mac_address users learns that most of them do
>> a memcpy (or similar) right after it, so for these drivers the
>> "of_get_nvmem_mac_address" style signature that performs the memcpy (or
>> better, ether_addr_copy) is a better fit, e.g.:
>>
>> int of_get_mac_address(struct device_node *np, void *addr)
>
> Hi Mike
>
> This is a nicer solution, but it is quite a lot of work, there are a
> lot of users. Maybe Coccinelle can help?
About 58 of them, yeah. And this looked like such a simple thing when I
started it...
https://elixir.bootlin.com/linux/v4.16-rc6/ident/of_get_mac_address
I have no experience with Coccinelle though.
--
Mike Looijmans
^ permalink raw reply
* Re: [PATCH RFC net-next 1/7] net: Fix fib notifer to return errno
From: Ido Schimmel @ 2018-03-25 8:16 UTC (permalink / raw)
To: David Ahern; +Cc: netdev, davem, roopa, shm, jiri, idosch, jakub.kicinski
In-Reply-To: <20180322225757.10377-2-dsa@cumulusnetworks.com>
On Thu, Mar 22, 2018 at 03:57:51PM -0700, David Ahern wrote:
> Notifier handlers use notifier_from_errno to convert any potential error
> to an encoded format. As a consequence the other side, call_fib_notifiers
> in this case, needs to use notifier_to_errno to return the error from
> the handler back to its caller.
>
> Signed-off-by: David Ahern <dsa@cumulusnetworks.com>
> ---
> net/core/fib_notifier.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/net/core/fib_notifier.c b/net/core/fib_notifier.c
> index 5ace0705a3f9..14ba52ebe8c9 100644
> --- a/net/core/fib_notifier.c
> +++ b/net/core/fib_notifier.c
> @@ -21,8 +21,11 @@ EXPORT_SYMBOL(call_fib_notifier);
> int call_fib_notifiers(struct net *net, enum fib_event_type event_type,
> struct fib_notifier_info *info)
There's another (less interesting case) - call_fib_notifier(), which is
used to dump the FIB tables for the caller registering to the
notification chain.
For example, if you have a non-default FIB rule in the system and you
modprobe mlxsw, you'll get a silent failure and routes will not be
offloaded. On the other hand, I'm not sure we want to fail the module
loading in such cases.
A possible solution is to have the driver emit a warning via extack for
each route/rule being notified after the abort mechanism was triggered.
> {
> + int err;
> +
> info->net = net;
> - return atomic_notifier_call_chain(&fib_chain, event_type, info);
> + err = atomic_notifier_call_chain(&fib_chain, event_type, info);
> + return notifier_to_errno(err);
> }
> EXPORT_SYMBOL(call_fib_notifiers);
>
> --
> 2.11.0
>
^ permalink raw reply
* [PATCH iproute2-next] rdma: Move RDMA UAPI header file to be under RDMA responsibility
From: Leon Romanovsky @ 2018-03-25 6:38 UTC (permalink / raw)
To: David Ahern
Cc: Leon Romanovsky, netdev, RDMA mailing list, Stephen Hemminger,
Steve Wise
From: Leon Romanovsky <leonro@mellanox.com>
In iproute2 package, the updates of UAPIs files are performed
after the needed feature lands in kernel's net-next tree.
Such development flow created delays to the rdma tool developers,
who uses rdma-next tree as a basis for their work.
Move RDMA UAPI file to be under rdma/ folder, so whole responsibility
of syncing this file will be on them.
Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
---
rdma/Makefile | 1 +
{include => rdma/include}/uapi/rdma/rdma_netlink.h | 0
2 files changed, 1 insertion(+)
rename {include => rdma/include}/uapi/rdma/rdma_netlink.h (100%)
diff --git a/rdma/Makefile b/rdma/Makefile
index 360f09b2..819fcbe3 100644
--- a/rdma/Makefile
+++ b/rdma/Makefile
@@ -4,6 +4,7 @@ include ../config.mk
TARGETS :=
ifeq ($(HAVE_MNL),y)
+CFLAGS += -I./include/uapi/
RDMA_OBJ = rdma.o utils.o dev.o link.o res.o
diff --git a/include/uapi/rdma/rdma_netlink.h b/rdma/include/uapi/rdma/rdma_netlink.h
similarity index 100%
rename from include/uapi/rdma/rdma_netlink.h
rename to rdma/include/uapi/rdma/rdma_netlink.h
^ permalink raw reply related
* Re: [PATCH RFC net-next 7/7] netdevsim: Add simple FIB resource controller via devlink
From: Jakub Kicinski @ 2018-03-25 6:35 UTC (permalink / raw)
To: David Ahern; +Cc: netdev, davem, roopa, shm, jiri, idosch, David Ahern
In-Reply-To: <1b5db863-7949-e4a2-7f22-6dd79ecb8089@cumulusnetworks.com>
On Sat, 24 Mar 2018 09:02:45 -0600, David Ahern wrote:
> >> diff --git a/drivers/net/netdevsim/Makefile b/drivers/net/netdevsim/Makefile
> >> index 09388c06171d..449b2a1a1800 100644
> >> --- a/drivers/net/netdevsim/Makefile
> >> +++ b/drivers/net/netdevsim/Makefile
> >> @@ -9,3 +9,7 @@ ifeq ($(CONFIG_BPF_SYSCALL),y)
> >> netdevsim-objs += \
> >> bpf.o
> >> endif
> >> +
> >> +ifneq ($(CONFIG_NET_DEVLINK),)
> >
> > Hm. Don't you need MAY_USE_DEVLINK dependency perhaps?
>
> mlxsw uses CONFIG_NET_DEVLINK in its Makefile.
>
> MAY_USE_DEVLINK seems to only be used in Kconfig files. Not clear to me
> why it is needed at all.
NETDEVSIM=y && DEVLINK=m
^ permalink raw reply
* Re: syzbot rcu/debugobjects warning
From: Joel Fernandes @ 2018-03-25 6:29 UTC (permalink / raw)
To: Thomas Gleixner
Cc: Paul McKenney, LKML, Todd Poynor,
open list:BPF (Safe dynamic programs and tools), Ben Hutchings,
Greg Kroah-Hartman, Guillaume Nault
In-Reply-To: <alpine.DEB.2.21.1803232133450.1481@nanos.tec.linutronix.de>
On Fri, Mar 23, 2018 at 1:41 PM, Thomas Gleixner <tglx@linutronix.de> wrote:
> On Fri, 23 Mar 2018, Joel Fernandes wrote:
>> On Fri, Mar 23, 2018 at 2:11 AM, Thomas Gleixner <tglx@linutronix.de> wrote:
>> > On Thu, 22 Mar 2018, Joel Fernandes wrote:
>> Sorry. Here is the raw crash log: https://pastebin.com/raw/puvh0cXE
>> (The kernel logs are toward the end with the above).
>
> And that is interesting:
>
> [ 150.629667] <IRQ> [ 150.631700] [<ffffffff81d96069>] dump_stack+0xc1/0x128
> [ 150.637051] [<ffffffff81dfefb6>] ? __debug_object_init+0x526/0xc40
> [ 150.643431] [<ffffffff8142fbd1>] panic+0x1bc/0x3a8
> [ 150.648416] [<ffffffff8142fa15>] ? percpu_up_read_preempt_enable.constprop.53+0xd7/0xd7
> [ 150.656611] [<ffffffff81430835>] ? load_image_and_restore+0xf9/0xf9
> [ 150.663070] [<ffffffff81269efd>] ? vprintk_default+0x1d/0x30
> [ 150.668925] [<ffffffff81131879>] ? __warn+0x1a9/0x1e0
> [ 150.674170] [<ffffffff81dfefb6>] ? __debug_object_init+0x526/0xc40
> [ 150.680543] [<ffffffff81131894>] __warn+0x1c4/0x1e0
> [ 150.685614] [<ffffffff81131afc>] warn_slowpath_null+0x2c/0x40
> [ 150.691972] [<ffffffff81dfefb6>] __debug_object_init+0x526/0xc40
> [ 150.698174] [<ffffffff81dfea90>] ? debug_object_fixup+0x30/0x30
> [ 150.704283] [<ffffffff81dff709>] debug_object_init_on_stack+0x19/0x20
> [ 150.710917] [<ffffffff81287a93>] __wait_rcu_gp+0x93/0x1b0
> [ 150.716508] [<ffffffff81290251>] synchronize_rcu.part.65+0x101/0x110
> [ 150.723054] [<ffffffff81290150>] ? rcu_pm_notify+0xc0/0xc0
> [ 150.728735] [<ffffffff81292bc0>] ? __call_rcu.constprop.72+0x910/0x910
> [ 150.735459] [<ffffffff81235221>] ? __lock_is_held+0xa1/0xf0
> [ 150.741223] [<ffffffff81290287>] synchronize_rcu+0x27/0x90
>
> So this calls synchronize_rcu from a rcu callback. That's a nono. This is
> on the back of an interrupt in softirq context and __wait_rcu_gp() can
> sleep, which is obviously a bad idea in softirq context....
>
> Cc'ed netdev ....
>
> And that also explains the debug object splat because this is not running
> on the task stack. It's running on the softirq stack ....
>
> [ 150.746908] [<ffffffff83588b35>] __l2tp_session_unhash+0x3d5/0x550
> [ 150.753281] [<ffffffff8358891f>] ? __l2tp_session_unhash+0x1bf/0x550
> [ 150.759828] [<ffffffff8114596a>] ? __local_bh_enable_ip+0x6a/0xd0
> [ 150.766123] [<ffffffff8358ddb0>] ? l2tp_udp_encap_recv+0xd90/0xd90
> [ 150.772497] [<ffffffff83588e97>] l2tp_tunnel_closeall+0x1e7/0x3a0
> [ 150.778782] [<ffffffff835897be>] l2tp_tunnel_destruct+0x30e/0x5a0
> [ 150.785067] [<ffffffff8358965a>] ? l2tp_tunnel_destruct+0x1aa/0x5a0
> [ 150.791537] [<ffffffff835894b0>] ? l2tp_tunnel_del_work+0x460/0x460
> [ 150.797997] [<ffffffff82ee8053>] __sk_destruct+0x53/0x570
> [ 150.803588] [<ffffffff81293918>] rcu_process_callbacks+0x898/0x1300
> [ 150.810048] [<ffffffff812939f7>] ? rcu_process_callbacks+0x977/0x1300
> [ 150.816684] [<ffffffff82ee8000>] ? __sk_dst_check+0x240/0x240
> [ 150.822625] [<ffffffff838be5d6>] __do_softirq+0x206/0x951
> [ 150.828223] [<ffffffff81147315>] irq_exit+0x165/0x190
> [ 150.833557] [<ffffffff838bd1eb>] smp_apic_timer_interrupt+0x7b/0xa0
> [ 150.840018] [<ffffffff838b9470>] apic_timer_interrupt+0xa0/0xb0
> [ 150.846132] <EOI> [ 150.848166] [<ffffffff838b6756>] ? native_safe_halt+0x6/0x10
> [ 150.854036] [<ffffffff8123bf2d>] ? trace_hardirqs_on+0xd/0x10
> [ 150.859973] [<ffffffff838b5d85>] default_idle+0x55/0x360
> [ 150.865478] [<ffffffff8106be0a>] arch_cpu_idle+0xa/0x10
> [ 150.870896] [<ffffffff838b6b96>] default_idle_call+0x36/0x60
> [ 150.876751] [<ffffffff81226cb0>] cpu_startup_entry+0x2b0/0x380
> [ 150.882787] [<ffffffff81226a00>] ? cpu_in_idle+0x20/0x20
> [ 150.888291] [<ffffffff812d2343>] ? clockevents_register_device+0x123/0x200
> [ 150.895358] [<ffffffff810b0693>] start_secondary+0x303/0x3e0
> [ 150.901209] [<ffffffff810b0390>] ? set_cpu_sibling_map+0x11f0/0x11f0
Thomas, thanks a lot. It appears this issue will not happen on
mainline since from commit 765924e362d1 (subject "l2tp: don't close
sessions in l2tp_tunnel_destruct()"), l2tp_tunnel_closeall is no
longer called from l2tp_tunnel_destruct. From that commit message it
seems one of the motivations is to solve scheduling from atomic issue.
However for this change to be applied to android-4.9 and/or 4.9
stable, it depends on several other l2p patches and they aren't
straight forward cherry-picks from mainline (and I don't have much
background with this driver).
v3.16.56 stable seems to be further along with l2tp than v4.9.89, in
that it atleast has more of the upstream patches adapted for it, that
the above patch depends on. Since this also related to stable, I am
CC'ing Greg kh and Ben.
Here are some of the commits in 3.16 stable that I couldn't find
applied to v4.9 stable. The above fix quotes the below patches as
dependencies so they would need to be stable backported. Also CC'ing
Guillaume since he authored the above mentioned fix.
0c15ddabbcf l2tp: don't register sessions in l2tp_session_create()
a3c5d5b70f4e l2tp: fix race condition in l2tp_tunnel_delete
5b216e8dcda2 l2tp: prevent creation of sessions on terminated tunnels
76ff5e22f1e0 l2tp: hold tunnel while looking up sessions in l2tp_netlink
ceb8f6b23a38 l2tp: define parameters of l2tp_session_get*() as "const"
0295d020b63f l2tp: initialise session's refcount before making it reachable
29a77518927e l2tp: take reference on sessions being dumped
b301c9b7782f l2tp: take a reference on sessions used in genetlink handlers
By the way I think the reason why scheduling while atomic checks
didn't show up is because the debugobjects warning caused a panic
first, before that could happen.
- Joel
PS: There's also 12d656af4e3d2 ("l2tp: Avoid schedule while atomic in
exit_net") which was fixing a call to synchronize_rcu in the same
path/function, but the caller originated from l2tp_exit_net. But this
patch is already in the stable trees. I am just mentioning it here for
completeness.
^ permalink raw reply
* Re: linux-next on x60: network manager often complains "network is disabled" after resume
From: Pavel Machek @ 2018-03-25 6:19 UTC (permalink / raw)
To: Dan Williams
Cc: Woody Suwalski, Rafael J. Wysocki, kernel list,
Linux-pm mailing list, Netdev list
In-Reply-To: <1521551568.16848.5.camel@redhat.com>
[-- Attachment #1: Type: text/plain, Size: 1453 bytes --]
> > > Ok, what does 'nmcli dev' and 'nmcli radio' show?
> >
> > Broken state.
> >
> > pavel@amd:~$ nmcli dev
> > DEVICE TYPE STATE CONNECTION
> > eth1 ethernet unavailable --
> > lo loopback unmanaged --
> > wlan0 wifi unmanaged --
>
> If the state is "unmanaged" on resume, that would indicate a problem
> with sleep/wake and likely not a kernel network device issue.
>
> We should probably move this discussion to the NM lists to debug
> further. Before you suspend, run "nmcli gen log level trace" to turn
> on full debug logging, then reproduce the issue, and send a pointer to
> those logs (scrubbed for anything you consider sensitive) to the NM
> mailing list.
Hmm :-)
root@amd:/data/pavel# nmcli gen log level trace
Error: Unknown log level 'trace'
root@amd:/data/pavel# nmcli gen log level help
Error: Unknown log level 'help'
root@amd:/data/pavel# nmcli gen log level
Error: value for 'level' argument is required.
root@amd:/data/pavel# nmcli gen log level debug
root@amd:/data/pavel# cat /var/log/sys/log
Where do I get the logs? I don't see much in the syslog...
And.. It seems that it is "every other suspend". One resume results in
broken network, one in working one, one in broken one...
Thanks,
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]
^ permalink raw reply
* [net PATCH v2] net: sched, fix OOO packets with pfifo_fast
From: John Fastabend @ 2018-03-25 5:25 UTC (permalink / raw)
To: eric.dumazet; +Cc: xiyou.wangcong, jiri, davem, netdev
After the qdisc lock was dropped in pfifo_fast we allow multiple
enqueue threads and dequeue threads to run in parallel. On the
enqueue side the skb bit ooo_okay is used to ensure all related
skbs are enqueued in-order. On the dequeue side though there is
no similar logic. What we observe is with fewer queues than CPUs
it is possible to re-order packets when two instances of
__qdisc_run() are running in parallel. Each thread will dequeue
a skb and then whichever thread calls the ndo op first will
be sent on the wire. This doesn't typically happen because
qdisc_run() is usually triggered by the same core that did the
enqueue. However, drivers will trigger __netif_schedule()
when queues are transitioning from stopped to awake using the
netif_tx_wake_* APIs. When this happens netif_schedule() calls
qdisc_run() on the same CPU that did the netif_tx_wake_* which
is usually done in the interrupt completion context. This CPU
is selected with the irq affinity which is unrelated to the
enqueue operations.
To resolve this we add a RUNNING bit to the qdisc to ensure
only a single dequeue per qdisc is running. Enqueue and dequeue
operations can still run in parallel and also on multi queue
NICs we can still have a dequeue in-flight per qdisc, which
is typically per CPU.
Fixes: c5ad119fb6c0 ("net: sched: pfifo_fast use skb_array")
Reported-by: Jakob Unterwurzacher <jakob.unterwurzacher@theobroma-systems.com>
Signed-off-by: John Fastabend <john.fastabend@gmail.com>
---
include/net/sch_generic.h | 1 +
net/sched/sch_generic.c | 17 +++++++++++++----
2 files changed, 14 insertions(+), 4 deletions(-)
diff --git a/include/net/sch_generic.h b/include/net/sch_generic.h
index 2092d33..8da3267 100644
--- a/include/net/sch_generic.h
+++ b/include/net/sch_generic.h
@@ -30,6 +30,7 @@ struct qdisc_rate_table {
enum qdisc_state_t {
__QDISC_STATE_SCHED,
__QDISC_STATE_DEACTIVATED,
+ __QDISC_STATE_RUNNING,
};
struct qdisc_size_table {
diff --git a/net/sched/sch_generic.c b/net/sched/sch_generic.c
index 7e3fbe9..39c144b 100644
--- a/net/sched/sch_generic.c
+++ b/net/sched/sch_generic.c
@@ -373,24 +373,33 @@ bool sch_direct_xmit(struct sk_buff *skb, struct Qdisc *q,
*/
static inline bool qdisc_restart(struct Qdisc *q, int *packets)
{
+ bool more, validate, nolock = q->flags & TCQ_F_NOLOCK;
spinlock_t *root_lock = NULL;
struct netdev_queue *txq;
struct net_device *dev;
struct sk_buff *skb;
- bool validate;
/* Dequeue packet */
+ if (nolock && test_and_set_bit(__QDISC_STATE_RUNNING, &q->state))
+ return false;
+
skb = dequeue_skb(q, &validate, packets);
- if (unlikely(!skb))
+ if (unlikely(!skb)) {
+ if (nolock)
+ clear_bit(__QDISC_STATE_RUNNING, &q->state);
return false;
+ }
- if (!(q->flags & TCQ_F_NOLOCK))
+ if (!nolock)
root_lock = qdisc_lock(q);
dev = qdisc_dev(q);
txq = skb_get_tx_queue(dev, skb);
- return sch_direct_xmit(skb, q, dev, txq, root_lock, validate);
+ more = sch_direct_xmit(skb, q, dev, txq, root_lock, validate);
+ if (nolock)
+ clear_bit(__QDISC_STATE_RUNNING, &q->state);
+ return more;
}
void __qdisc_run(struct Qdisc *q)
^ permalink raw reply related
* Re: [PATCH net-next RFC V1 5/5] net: mdio: Add a driver for InES time stamping IP core.
From: Richard Cochran @ 2018-03-25 4:51 UTC (permalink / raw)
To: Andrew Lunn
Cc: netdev, devicetree, David Miller, Florian Fainelli, Mark Rutland,
Miroslav Lichvar, Rob Herring, Willem de Bruijn
In-Reply-To: <20180324184858.GF31941@lunn.ch>
On Sat, Mar 24, 2018 at 07:48:58PM +0100, Andrew Lunn wrote:
> As far as i can see, you have three basic problems:
>
> 1) How do you associate the PTP device to the netdev?
> 2) How do you get the information you need to configure the PTP device
Yes, yes.
> 3) How do you limit the MAC/PHY to what the PTP device can do.
Hm, I don't think this is important.
> phylib does have all this information. It is phylib that calls the MAC
> with link speed information. When the MAC connects to the PHY, it
> passes the MII mode, and when the PHY requests the MII mode changes,
> phylib knows. The MAC has to call phy_init_eee() to see if the PHY is
> EEE capable. phylib also tells the MAC what speeds the PHY is capable
> off, so it is in the position to mask out speeds the PTP device does
> not support, etc.
Right, so phylib can operate on phydev->attached_dev->mdiots;
> So i really think you need to cleanly integrate into phylib and
> phylink.
So I think I've done that, more or less, but I'd like to hear your
ideas on how to make it cleaner...
Thanks,
Richard
^ permalink raw reply
* RE
From: Ms. Ella Golan @ 2018-03-24 17:45 UTC (permalink / raw)
To: Recipients
I am Ms.Ella Golan, I am the Executive Vice President Banking Division with FIRST INTERNATIONAL BANK OF ISRAEL LTD (FIBI). I am getting in touch with you regarding an extremely important and urgent matter. If you would oblige me the opportunity, I shall provide you with details upon your response.
Faithfully,
Ms.Ella Golan
^ permalink raw reply
* RE: [PATCH net-next,1/2] hv_netvsc: Fix the return status in RX path
From: Haiyang Zhang @ 2018-03-25 0:41 UTC (permalink / raw)
To: Michael Kelley (EOSG), davem@davemloft.net,
netdev@vger.kernel.org
Cc: KY Srinivasan, Stephen Hemminger, olaf@aepfle.de,
vkuznets@redhat.com, devel@linuxdriverproject.org,
linux-kernel@vger.kernel.org
In-Reply-To: <DM5PR2101MB103003AA72955A6C6AD44D0CDCAF0@DM5PR2101MB1030.namprd21.prod.outlook.com>
> -----Original Message-----
> From: Michael Kelley (EOSG)
> Sent: Saturday, March 24, 2018 12:48 PM
> To: Haiyang Zhang <haiyangz@microsoft.com>; davem@davemloft.net;
> netdev@vger.kernel.org
> Cc: KY Srinivasan <kys@microsoft.com>; Stephen Hemminger
> <sthemmin@microsoft.com>; olaf@aepfle.de; vkuznets@redhat.com;
> devel@linuxdriverproject.org; linux-kernel@vger.kernel.org
> Subject: RE: [PATCH net-next,1/2] hv_netvsc: Fix the return status in RX path
>
> > -----Original Message-----
> > From: linux-kernel-owner@vger.kernel.org
> > <linux-kernel-owner@vger.kernel.org> On Behalf Of Haiyang Zhang
> > Sent: Thursday, March 22, 2018 12:01 PM
> > To: davem@davemloft.net; netdev@vger.kernel.org
> > Cc: Haiyang Zhang <haiyangz@microsoft.com>; KY Srinivasan
> > <kys@microsoft.com>; Stephen Hemminger <sthemmin@microsoft.com>;
> > olaf@aepfle.de; vkuznets@redhat.com; devel@linuxdriverproject.org;
> > linux-kernel@vger.kernel.org
> > Subject: [PATCH net-next,1/2] hv_netvsc: Fix the return status in RX
> > path
> >
> > From: Haiyang Zhang <haiyangz@microsoft.com>
> >
> > As defined in hyperv_net.h, the NVSP_STAT_SUCCESS is one not zero.
> > Some functions returns 0 when it actually means NVSP_STAT_SUCCESS.
> > This patch fixes them.
> >
> > In netvsc_receive(), it puts the last RNDIS packet's receive status
> > for all packets in a vmxferpage which may contain multiple RNDIS
> > packets.
> > This patch puts NVSP_STAT_FAIL in the receive completion if one of the
> > packets in a vmxferpage fails.
>
> This patch changes the status field that is being reported back to the Hyper-V
> host in the receive completion message in
> enq_receive_complete(). The current code reports 0 on success,
> and with the patch, it will report 1 on success. So does this change affect
> anything on the Hyper-V side? Or is Hyper-V just ignoring
> the value? If this change doesn't have any impact on the
> interactions with Hyper-V, perhaps it would be good to explain why in the
> commit message.
Here is the definition of each status code for NetVSP.
enum {
NVSP_STAT_NONE = 0,
NVSP_STAT_SUCCESS,
NVSP_STAT_FAIL,
NVSP_STAT_PROTOCOL_TOO_NEW,
NVSP_STAT_PROTOCOL_TOO_OLD,
NVSP_STAT_INVALID_RNDIS_PKT,
NVSP_STAT_BUSY,
NVSP_STAT_PROTOCOL_UNSUPPORTED,
NVSP_STAT_MAX,
};
Existing code returns NVSP_STAT_NONE = 0, and with this patch
we return NVSP_STAT_SUCCESS = 1.
Based on testing, either way works for now. But for correctness
and future stability (e.g. host side becomes more stringent), we
should follow the protocol.
Thanks,
- Haiyang
^ permalink raw reply
* Re: [PATCH net-next v2 2/2] cxgb4: collect hardware dump in second kernel
From: Eric W. Biederman @ 2018-03-25 0:17 UTC (permalink / raw)
To: Thadeu Lima de Souza Cascardo
Cc: Rahul Lakkireddy, netdev, linux-fsdevel, kexec, linux-kernel,
indranil, nirranjan, stephen, ganeshgr, akpm, torvalds, davem,
viro
In-Reply-To: <20180324221849.GW14312@siri.cascardo.eti.br>
Thadeu Lima de Souza Cascardo <cascardo@debian.org> writes:
> On Sat, Mar 24, 2018 at 04:26:34PM +0530, Rahul Lakkireddy wrote:
>> Register callback to collect hardware/firmware dumps in second kernel
>> before hardware/firmware is initialized. The dumps for each device
>> will be available under /sys/kernel/crashdd/cxgb4/ directory in second
>> kernel.
>>
>> Signed-off-by: Rahul Lakkireddy <rahul.lakkireddy@chelsio.com>
>> Signed-off-by: Ganesh Goudar <ganeshgr@chelsio.com>
>> ---
>> v2:
>> - No Changes.
>>
>> Changes since rfc v2:
>> - Update comments and commit message for sysfs change.
>>
>> rfc v2:
>> - Updated dump registration to the new API in patch 1.
> [...]
>> diff --git a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c
>> index e880be8e3c45..265cb026f868 100644
>> --- a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c
>> +++ b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c
>> @@ -5527,6 +5527,18 @@ static int init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
>> if (err)
>> goto out_free_adapter;
>>
>> + if (is_kdump_kernel()) {
>> + /* Collect hardware state and append to
>> + * /sys/kernel/crashdd/cxgb4/ directory
>> + */
>> + err = cxgb4_cudbg_crashdd_add_dump(adapter);
>> + if (err) {
>> + dev_warn(adapter->pdev_dev,
>> + "Fail collecting crash device dump, err: %d. Continuing\n",
>> + err);
>> + err = 0;
>> + }
>> + }
>>
>
> The problem I see with this approach is that you require that the driver
> is built into the kdump kernel (or present as a module in the kdump
> initramfs), and that you will probe the device during the collection of
> the dumps.
Compared to doing something in a crashing kernel anything in the kdump
kernel is a walk in the park. Nothing is trustable in a crashing
kernel.
> IMHO, if you are going to require the device to be probed by the same
> driver during kdump, you might just as well use the device object itself
> to present the crash data. I think that's what Stephen Hemminger meant
> when he said to use sysfs. No need at all for any special crashdd. Just
> add an attribute or attribute group to the device object.
Doing something with the device model might make sense. I am not
certain it does. It is quite possible the device is in such a weird
state that the device driver fails to initialize. That doesn't
mean the device driver can't scrape the registers and present
meaningful information to the rest of the system.
Whatever you do with capturing the state needs to happen early before
the driver initializes and stomps on the relevant state.
I don't expect there is much for the driver model to do, unless we wish
to do something explicitly before the normal device probe methods
happen. What we need is the infrastructure for catching what gets
read from the driver and placing it in the core dump.
> Otherwise, as Eric Biederman pointed out, you should just add that data
> into the vmcore before you kexec, so you don't even need to look at a
> different file, and the driver does not even need to be present in the
> kdump kernel.
No. I do mean before a kexec on panic happens. Doing anything with
gathering this kind of information before kexec on panic is a very very
very very bad idea that will almost certainly make crash dumps less
reliable. Don't even think about doing extra work on the crash dump
path. Not ever. No. No. No. No.
The reason we use kexec on panic instead just creating a core dump
in the kernel is that many have tried and no one has gotten the kernel
to create crash dumps when things go wrong and it matters. Meanwhile
kexec on panic works more often than not.
I mean that /proc/vmcore is a device that is used to gather up the bits
of the crashing kernel and to present it in a format that is easy to
read/save. The tools read /proc/vmcore.
The driver or whatever is gathering this information absolutely needs to
be in the kdump kernel.
Eric
^ permalink raw reply
* Re: [PATCH net-next v2 2/2] cxgb4: collect hardware dump in second kernel
From: Thadeu Lima de Souza Cascardo @ 2018-03-24 22:18 UTC (permalink / raw)
To: Rahul Lakkireddy
Cc: indranil-ut6Up61K2wZBDgjK7y7TUQ, netdev-u79uwXL29TY76Z2rM5mHXA,
nirranjan-ut6Up61K2wZBDgjK7y7TUQ,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
viro-RmSDqhL/yNMiFSDQTTA3OLVCufUGDwFn,
davem-fT/PcQaiUtIeIZ0/mPfg9Q,
stephen-OTpzqLSitTUnbdJkjeBofR2eb7JE58TQ,
ganeshgr-ut6Up61K2wZBDgjK7y7TUQ,
linux-fsdevel-u79uwXL29TY76Z2rM5mHXA,
akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b,
torvalds-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b,
kexec-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
ebiederm-aS9lmoZGLiVWk0Htik3J/w
In-Reply-To: <f06cca21650affa10d83a28dbd7fb577792e35a9.1521888444.git.rahul.lakkireddy-ut6Up61K2wZBDgjK7y7TUQ@public.gmane.org>
On Sat, Mar 24, 2018 at 04:26:34PM +0530, Rahul Lakkireddy wrote:
> Register callback to collect hardware/firmware dumps in second kernel
> before hardware/firmware is initialized. The dumps for each device
> will be available under /sys/kernel/crashdd/cxgb4/ directory in second
> kernel.
>
> Signed-off-by: Rahul Lakkireddy <rahul.lakkireddy-ut6Up61K2wZBDgjK7y7TUQ@public.gmane.org>
> Signed-off-by: Ganesh Goudar <ganeshgr-ut6Up61K2wZBDgjK7y7TUQ@public.gmane.org>
> ---
> v2:
> - No Changes.
>
> Changes since rfc v2:
> - Update comments and commit message for sysfs change.
>
> rfc v2:
> - Updated dump registration to the new API in patch 1.
[...]
> diff --git a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c
> index e880be8e3c45..265cb026f868 100644
> --- a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c
> +++ b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c
> @@ -5527,6 +5527,18 @@ static int init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
> if (err)
> goto out_free_adapter;
>
> + if (is_kdump_kernel()) {
> + /* Collect hardware state and append to
> + * /sys/kernel/crashdd/cxgb4/ directory
> + */
> + err = cxgb4_cudbg_crashdd_add_dump(adapter);
> + if (err) {
> + dev_warn(adapter->pdev_dev,
> + "Fail collecting crash device dump, err: %d. Continuing\n",
> + err);
> + err = 0;
> + }
> + }
>
The problem I see with this approach is that you require that the driver
is built into the kdump kernel (or present as a module in the kdump
initramfs), and that you will probe the device during the collection of
the dumps.
IMHO, if you are going to require the device to be probed by the same
driver during kdump, you might just as well use the device object itself
to present the crash data. I think that's what Stephen Hemminger meant
when he said to use sysfs. No need at all for any special crashdd. Just
add an attribute or attribute group to the device object.
Otherwise, as Eric Biederman pointed out, you should just add that data
into the vmcore before you kexec, so you don't even need to look at a
different file, and the driver does not even need to be present in the
kdump kernel.
Cascardo.
> if (!is_t4(adapter->params.chip)) {
> s_qpp = (QUEUESPERPAGEPF0_S +
> --
> 2.14.1
^ permalink raw reply
* [PATCH net-next 6/6] r8169: change argument type of rtl8169_net_suspend and __rtl8169_resume
From: Heiner Kallweit @ 2018-03-24 22:18 UTC (permalink / raw)
To: Realtek linux nic maintainers, David Miller; +Cc: netdev@vger.kernel.org
In-Reply-To: <6160c32d-3905-683a-7117-d2b11d310a34@gmail.com>
Both functions can be simplified by changing the argument type to
struct rtl8169_private *.
Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
drivers/net/ethernet/realtek/r8169.c | 26 +++++++++++---------------
1 file changed, 11 insertions(+), 15 deletions(-)
diff --git a/drivers/net/ethernet/realtek/r8169.c b/drivers/net/ethernet/realtek/r8169.c
index dd84cc3a..58d84e48 100644
--- a/drivers/net/ethernet/realtek/r8169.c
+++ b/drivers/net/ethernet/realtek/r8169.c
@@ -7776,15 +7776,13 @@ rtl8169_get_stats64(struct net_device *dev, struct rtnl_link_stats64 *stats)
pm_runtime_put_noidle(&pdev->dev);
}
-static void rtl8169_net_suspend(struct net_device *dev)
+static void rtl8169_net_suspend(struct rtl8169_private *tp)
{
- struct rtl8169_private *tp = netdev_priv(dev);
-
- if (!netif_running(dev))
+ if (!netif_running(tp->dev))
return;
- netif_device_detach(dev);
- netif_stop_queue(dev);
+ netif_device_detach(tp->dev);
+ netif_stop_queue(tp->dev);
rtl_lock_work(tp);
napi_disable(&tp->napi);
@@ -7800,16 +7798,14 @@ static int rtl8169_suspend(struct device *device)
{
struct rtl8169_private *tp = dev_get_drvdata(device);
- rtl8169_net_suspend(tp->dev);
+ rtl8169_net_suspend(tp);
return 0;
}
-static void __rtl8169_resume(struct net_device *dev)
+static void __rtl8169_resume(struct rtl8169_private *tp)
{
- struct rtl8169_private *tp = netdev_priv(dev);
-
- netif_device_attach(dev);
+ netif_device_attach(tp->dev);
rtl_pll_power_up(tp);
@@ -7828,7 +7824,7 @@ static int rtl8169_resume(struct device *device)
rtl8169_init_phy(tp);
if (netif_running(tp->dev))
- __rtl8169_resume(tp->dev);
+ __rtl8169_resume(tp);
return 0;
}
@@ -7847,7 +7843,7 @@ static int rtl8169_runtime_suspend(struct device *device)
__rtl8169_set_wol(tp, WAKE_ANY);
rtl_unlock_work(tp);
- rtl8169_net_suspend(tp->dev);
+ rtl8169_net_suspend(tp);
/* Update counters before going runtime suspend */
rtl8169_rx_missed(tp->dev);
@@ -7872,7 +7868,7 @@ static int rtl8169_runtime_resume(struct device *device)
rtl8169_init_phy(tp);
- __rtl8169_resume(tp->dev);
+ __rtl8169_resume(tp);
return 0;
}
@@ -7929,7 +7925,7 @@ static void rtl_shutdown(struct pci_dev *pdev)
{
struct rtl8169_private *tp = pci_get_drvdata(pdev);
- rtl8169_net_suspend(tp->dev);
+ rtl8169_net_suspend(tp);
/* Restore original MAC address */
rtl_rar_set(tp, tp->dev->perm_addr);
--
2.16.2
^ permalink raw reply related
* [PATCH net-next 5/6] r8169: change type of driver_data
From: Heiner Kallweit @ 2018-03-24 22:18 UTC (permalink / raw)
To: Realtek linux nic maintainers, David Miller; +Cc: netdev@vger.kernel.org
In-Reply-To: <6160c32d-3905-683a-7117-d2b11d310a34@gmail.com>
Several functions accessing the device driver_data field don't need the
net_device. All needed parameters can be accessed via struct
rtl8169_private, therefore change type of driver_data accordingly.
Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
drivers/net/ethernet/realtek/r8169.c | 51 +++++++++++++++---------------------
1 file changed, 21 insertions(+), 30 deletions(-)
diff --git a/drivers/net/ethernet/realtek/r8169.c b/drivers/net/ethernet/realtek/r8169.c
index 2d68306e..dd84cc3a 100644
--- a/drivers/net/ethernet/realtek/r8169.c
+++ b/drivers/net/ethernet/realtek/r8169.c
@@ -7798,10 +7798,9 @@ static void rtl8169_net_suspend(struct net_device *dev)
static int rtl8169_suspend(struct device *device)
{
- struct pci_dev *pdev = to_pci_dev(device);
- struct net_device *dev = pci_get_drvdata(pdev);
+ struct rtl8169_private *tp = dev_get_drvdata(device);
- rtl8169_net_suspend(dev);
+ rtl8169_net_suspend(tp->dev);
return 0;
}
@@ -7824,23 +7823,19 @@ static void __rtl8169_resume(struct net_device *dev)
static int rtl8169_resume(struct device *device)
{
- struct pci_dev *pdev = to_pci_dev(device);
- struct net_device *dev = pci_get_drvdata(pdev);
- struct rtl8169_private *tp = netdev_priv(dev);
+ struct rtl8169_private *tp = dev_get_drvdata(device);
rtl8169_init_phy(tp);
- if (netif_running(dev))
- __rtl8169_resume(dev);
+ if (netif_running(tp->dev))
+ __rtl8169_resume(tp->dev);
return 0;
}
static int rtl8169_runtime_suspend(struct device *device)
{
- struct pci_dev *pdev = to_pci_dev(device);
- struct net_device *dev = pci_get_drvdata(pdev);
- struct rtl8169_private *tp = netdev_priv(dev);
+ struct rtl8169_private *tp = dev_get_drvdata(device);
if (!tp->TxDescArray) {
rtl_pll_power_down(tp);
@@ -7852,10 +7847,10 @@ static int rtl8169_runtime_suspend(struct device *device)
__rtl8169_set_wol(tp, WAKE_ANY);
rtl_unlock_work(tp);
- rtl8169_net_suspend(dev);
+ rtl8169_net_suspend(tp->dev);
/* Update counters before going runtime suspend */
- rtl8169_rx_missed(dev);
+ rtl8169_rx_missed(tp->dev);
rtl8169_update_counters(tp);
return 0;
@@ -7863,10 +7858,9 @@ static int rtl8169_runtime_suspend(struct device *device)
static int rtl8169_runtime_resume(struct device *device)
{
- struct pci_dev *pdev = to_pci_dev(device);
- struct net_device *dev = pci_get_drvdata(pdev);
- struct rtl8169_private *tp = netdev_priv(dev);
- rtl_rar_set(tp, dev->dev_addr);
+ struct rtl8169_private *tp = dev_get_drvdata(device);
+
+ rtl_rar_set(tp, tp->dev->dev_addr);
if (!tp->TxDescArray)
return 0;
@@ -7878,17 +7872,16 @@ static int rtl8169_runtime_resume(struct device *device)
rtl8169_init_phy(tp);
- __rtl8169_resume(dev);
+ __rtl8169_resume(tp->dev);
return 0;
}
static int rtl8169_runtime_idle(struct device *device)
{
- struct pci_dev *pdev = to_pci_dev(device);
- struct net_device *dev = pci_get_drvdata(pdev);
+ struct rtl8169_private *tp = dev_get_drvdata(device);
- if (!netif_running(dev) || !netif_carrier_ok(dev))
+ if (!netif_running(tp->dev) || !netif_carrier_ok(tp->dev))
pm_schedule_suspend(device, 10000);
return -EBUSY;
@@ -7934,13 +7927,12 @@ static void rtl_wol_shutdown_quirk(struct rtl8169_private *tp)
static void rtl_shutdown(struct pci_dev *pdev)
{
- struct net_device *dev = pci_get_drvdata(pdev);
- struct rtl8169_private *tp = netdev_priv(dev);
+ struct rtl8169_private *tp = pci_get_drvdata(pdev);
- rtl8169_net_suspend(dev);
+ rtl8169_net_suspend(tp->dev);
/* Restore original MAC address */
- rtl_rar_set(tp, dev->perm_addr);
+ rtl_rar_set(tp, tp->dev->perm_addr);
rtl8169_hw_reset(tp);
@@ -7957,15 +7949,14 @@ static void rtl_shutdown(struct pci_dev *pdev)
static void rtl_remove_one(struct pci_dev *pdev)
{
- struct net_device *dev = pci_get_drvdata(pdev);
- struct rtl8169_private *tp = netdev_priv(dev);
+ struct rtl8169_private *tp = pci_get_drvdata(pdev);
if (r8168_check_dash(tp))
rtl8168_driver_stop(tp);
netif_napi_del(&tp->napi);
- unregister_netdev(dev);
+ unregister_netdev(tp->dev);
rtl_release_firmware(tp);
@@ -7973,7 +7964,7 @@ static void rtl_remove_one(struct pci_dev *pdev)
pm_runtime_get_noresume(&pdev->dev);
/* restore original MAC address */
- rtl_rar_set(tp, dev->perm_addr);
+ rtl_rar_set(tp, tp->dev->perm_addr);
}
static const struct net_device_ops rtl_netdev_ops = {
@@ -8361,7 +8352,7 @@ static int rtl_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
if (rc < 0)
return rc;
- pci_set_drvdata(pdev, dev);
+ pci_set_drvdata(pdev, tp);
netif_info(tp, probe, dev, "%s at 0x%p, %pM, XID %08x IRQ %d\n",
rtl_chip_infos[chipset].name, tp->mmio_addr, dev->dev_addr,
--
2.16.2
^ permalink raw reply related
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