* [PATCH iproute2-next 00/10] RDMAtool cleanup and refactoring code
From: Leon Romanovsky @ 2017-12-27 7:57 UTC (permalink / raw)
To: David Ahern; +Cc: Leon Romanovsky, netdev, Stephen Hemminger
From: Leon Romanovsky <leonro@mellanox.com>
Hi,
The following patchset comes as a preparation to more complex code,
which will add resource tracking visibility to the rdmatool, where
the kernel part is under review of RDMA community.
Thanks
[1] https://marc.info/?l=linux-rdma&m=151412508816802&w=2
Leon Romanovsky (10):
rdma: Reduce scope of _dev_map_lookup call
rdma: Protect dev_map_lookup from wrong input
rdma: Move per-device handler function to generic code
rdma: Fix misspelled SYS_IMAGE_GUID
rdma: Check that port index exists before operate on link layer
rdma: Print supplied device name in case of wrong name
rdma: Get rid of dev_map_free call
rdma: Rename free function to be rd_cleanup
rdma: Rename rd_free_devmap to be rd_free
rdma: Move link execution logic to common code
rdma/dev.c | 28 +----------------
rdma/link.c | 51 +++---------------------------
rdma/rdma.c | 7 ++---
rdma/rdma.h | 5 +--
rdma/utils.c | 100 ++++++++++++++++++++++++++++++++++++++++++++++++++++-------
5 files changed, 100 insertions(+), 91 deletions(-)
^ permalink raw reply
* [PATCH iproute2-next 02/10] rdma: Protect dev_map_lookup from wrong input
From: Leon Romanovsky @ 2017-12-27 7:57 UTC (permalink / raw)
To: David Ahern; +Cc: Leon Romanovsky, netdev, Stephen Hemminger
In-Reply-To: <20171227075759.15289-1-leon@kernel.org>
From: Leon Romanovsky <leonro@mellanox.com>
Despite the fact that all callers to dev_map_lookup are ensuring that
there is always device name prior to call to that function, it is better
and safer to check that in the dev_map_lookup itself.
Fixes: 40df8263a0f0 ("rdma: Add dev object")
Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
---
rdma/utils.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/rdma/utils.c b/rdma/utils.c
index 6ce1fd70..bb29fa1a 100644
--- a/rdma/utils.c
+++ b/rdma/utils.c
@@ -253,6 +253,9 @@ struct dev_map *dev_map_lookup(struct rd *rd, bool allow_port_index)
char *dev_name;
char *slash;
+ if (rd_no_arg(rd))
+ return NULL;
+
dev_name = strdup(rd_argv(rd));
if (allow_port_index) {
slash = strrchr(dev_name, '/');
--
2.15.1
^ permalink raw reply related
* [PATCH iproute2-next 03/10] rdma: Move per-device handler function to generic code
From: Leon Romanovsky @ 2017-12-27 7:57 UTC (permalink / raw)
To: David Ahern; +Cc: Leon Romanovsky, netdev, Stephen Hemminger
In-Reply-To: <20171227075759.15289-1-leon@kernel.org>
From: Leon Romanovsky <leonro@mellanox.com>
Most of the proposed objects are working in the scope "dev"
and will implement the same logic. Move the code to utils.c,
so other objects will be able to reuse the code.
Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
---
rdma/dev.c | 28 +---------------------------
rdma/rdma.h | 1 +
rdma/utils.c | 31 +++++++++++++++++++++++++++++++
3 files changed, 33 insertions(+), 27 deletions(-)
diff --git a/rdma/dev.c b/rdma/dev.c
index 9fadf3ac..03ab8683 100644
--- a/rdma/dev.c
+++ b/rdma/dev.c
@@ -241,33 +241,7 @@ static int dev_one_show(struct rd *rd)
static int dev_show(struct rd *rd)
{
- struct dev_map *dev_map;
- int ret = 0;
-
- if (rd->json_output)
- jsonw_start_array(rd->jw);
- if (rd_no_arg(rd)) {
- list_for_each_entry(dev_map, &rd->dev_map_list, list) {
- rd->dev_idx = dev_map->idx;
- ret = dev_one_show(rd);
- if (ret)
- goto out;
- }
- } else {
- dev_map = dev_map_lookup(rd, false);
- if (!dev_map) {
- pr_err("Wrong device name\n");
- ret = -ENOENT;
- goto out;
- }
- rd_arg_inc(rd);
- rd->dev_idx = dev_map->idx;
- ret = dev_one_show(rd);
- }
-out:
- if (rd->json_output)
- jsonw_end_array(rd->jw);
- return ret;
+ return rd_exec_dev(rd, dev_one_show);
}
int cmd_dev(struct rd *rd)
diff --git a/rdma/rdma.h b/rdma/rdma.h
index c07493c9..b85e3748 100644
--- a/rdma/rdma.h
+++ b/rdma/rdma.h
@@ -72,6 +72,7 @@ uint32_t get_port_from_argv(struct rd *rd);
int cmd_dev(struct rd *rd);
int cmd_link(struct rd *rd);
int rd_exec_cmd(struct rd *rd, const struct rd_cmd *c, const char *str);
+int rd_exec_dev(struct rd *rd, int (*cb)(struct rd *rd));
/*
* Device manipulation
diff --git a/rdma/utils.c b/rdma/utils.c
index bb29fa1a..5c0f021a 100644
--- a/rdma/utils.c
+++ b/rdma/utils.c
@@ -159,6 +159,37 @@ void rd_free_devmap(struct rd *rd)
dev_map_cleanup(rd);
}
+int rd_exec_dev(struct rd *rd, int (*cb)(struct rd *rd))
+{
+ struct dev_map *dev_map;
+ int ret = 0;
+
+ if (rd->json_output)
+ jsonw_start_array(rd->jw);
+ if (rd_no_arg(rd)) {
+ list_for_each_entry(dev_map, &rd->dev_map_list, list) {
+ rd->dev_idx = dev_map->idx;
+ ret = cb(rd);
+ if (ret)
+ goto out;
+ }
+ } else {
+ dev_map = dev_map_lookup(rd, false);
+ if (!dev_map) {
+ pr_err("Wrong device name\n");
+ ret = -ENOENT;
+ goto out;
+ }
+ rd_arg_inc(rd);
+ rd->dev_idx = dev_map->idx;
+ ret = cb(rd);
+ }
+out:
+ if (rd->json_output)
+ jsonw_end_array(rd->jw);
+ return ret;
+}
+
int rd_exec_cmd(struct rd *rd, const struct rd_cmd *cmds, const char *str)
{
const struct rd_cmd *c;
--
2.15.1
^ permalink raw reply related
* [PATCH iproute2-next 04/10] rdma: Fix misspelled SYS_IMAGE_GUID
From: Leon Romanovsky @ 2017-12-27 7:57 UTC (permalink / raw)
To: David Ahern; +Cc: Leon Romanovsky, netdev, Stephen Hemminger
In-Reply-To: <20171227075759.15289-1-leon@kernel.org>
From: Leon Romanovsky <leonro@mellanox.com>
SYS_IMAGE_GUIG is actually SYS_IMAGE_GUID.
Fixes: da990ab40a92 ("rdma: Add link object")
Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
---
rdma/link.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/rdma/link.c b/rdma/link.c
index 3a4b00bd..f0eaccbb 100644
--- a/rdma/link.c
+++ b/rdma/link.c
@@ -30,7 +30,7 @@ static const char *caps_to_str(uint32_t idx)
x(PKEY_NVRAM, 8) \
x(LED_INFO, 9) \
x(SM_DISABLED, 10) \
- x(SYS_IMAGE_GUIG, 11) \
+ x(SYS_IMAGE_GUID, 11) \
x(PKEY_SW_EXT_PORT_TRAP, 12) \
x(EXTENDED_SPEEDS, 14) \
x(CM, 16) \
--
2.15.1
^ permalink raw reply related
* [PATCH iproute2-next 06/10] rdma: Print supplied device name in case of wrong name
From: Leon Romanovsky @ 2017-12-27 7:57 UTC (permalink / raw)
To: David Ahern; +Cc: Leon Romanovsky, netdev, Stephen Hemminger
In-Reply-To: <20171227075759.15289-1-leon@kernel.org>
From: Leon Romanovsky <leonro@mellanox.com>
Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
---
rdma/utils.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/rdma/utils.c b/rdma/utils.c
index 5c0f021a..998a178d 100644
--- a/rdma/utils.c
+++ b/rdma/utils.c
@@ -176,7 +176,7 @@ int rd_exec_dev(struct rd *rd, int (*cb)(struct rd *rd))
} else {
dev_map = dev_map_lookup(rd, false);
if (!dev_map) {
- pr_err("Wrong device name\n");
+ pr_err("Wrong device name - %s\n", rd_argv(rd));
ret = -ENOENT;
goto out;
}
--
2.15.1
^ permalink raw reply related
* [PATCH iproute2-next 07/10] rdma: Get rid of dev_map_free call
From: Leon Romanovsky @ 2017-12-27 7:57 UTC (permalink / raw)
To: David Ahern; +Cc: Leon Romanovsky, netdev, Stephen Hemminger
In-Reply-To: <20171227075759.15289-1-leon@kernel.org>
From: Leon Romanovsky <leonro@mellanox.com>
The dev_map_free() is called once only and it is short,
so it is better to integrate it into the caller's site.
Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
---
rdma/utils.c | 12 ++----------
1 file changed, 2 insertions(+), 10 deletions(-)
diff --git a/rdma/utils.c b/rdma/utils.c
index 998a178d..c1069967 100644
--- a/rdma/utils.c
+++ b/rdma/utils.c
@@ -71,15 +71,6 @@ static struct dev_map *dev_map_alloc(const char *dev_name)
return dev_map;
}
-static void dev_map_free(struct dev_map *dev_map)
-{
- if (!dev_map)
- return;
-
- free(dev_map->dev_name);
- free(dev_map);
-}
-
static void dev_map_cleanup(struct rd *rd)
{
struct dev_map *dev_map, *tmp;
@@ -87,7 +78,8 @@ static void dev_map_cleanup(struct rd *rd)
list_for_each_entry_safe(dev_map, tmp,
&rd->dev_map_list, list) {
list_del(&dev_map->list);
- dev_map_free(dev_map);
+ free(dev_map->dev_name);
+ free(dev_map);
}
}
--
2.15.1
^ permalink raw reply related
* [PATCH iproute2-next 08/10] rdma: Rename free function to be rd_cleanup
From: Leon Romanovsky @ 2017-12-27 7:57 UTC (permalink / raw)
To: David Ahern; +Cc: Leon Romanovsky, netdev, Stephen Hemminger
In-Reply-To: <20171227075759.15289-1-leon@kernel.org>
From: Leon Romanovsky <leonro@mellanox.com>
Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
---
rdma/rdma.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/rdma/rdma.c b/rdma/rdma.c
index f9f4f2a2..a05a2fb6 100644
--- a/rdma/rdma.c
+++ b/rdma/rdma.c
@@ -70,7 +70,7 @@ static int rd_init(struct rd *rd, int argc, char **argv, char *filename)
return rd_recv_msg(rd, rd_dev_init_cb, rd, seq);
}
-static void rd_free(struct rd *rd)
+static void rd_cleanup(struct rd *rd)
{
if (rd->json_output)
jsonw_destroy(&rd->jw);
@@ -138,6 +138,6 @@ int main(int argc, char **argv)
err = rd_cmd(&rd);
out:
/* Always cleanup */
- rd_free(&rd);
+ rd_cleanup(&rd);
return err ? EXIT_FAILURE : EXIT_SUCCESS;
}
--
2.15.1
^ permalink raw reply related
* [PATCH iproute2-next 09/10] rdma: Rename rd_free_devmap to be rd_free
From: Leon Romanovsky @ 2017-12-27 7:57 UTC (permalink / raw)
To: David Ahern; +Cc: Leon Romanovsky, netdev, Stephen Hemminger
In-Reply-To: <20171227075759.15289-1-leon@kernel.org>
From: Leon Romanovsky <leonro@mellanox.com>
Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
---
rdma/rdma.c | 3 +--
rdma/rdma.h | 2 +-
rdma/utils.c | 3 ++-
3 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/rdma/rdma.c b/rdma/rdma.c
index a05a2fb6..0e8fd688 100644
--- a/rdma/rdma.c
+++ b/rdma/rdma.c
@@ -74,8 +74,7 @@ static void rd_cleanup(struct rd *rd)
{
if (rd->json_output)
jsonw_destroy(&rd->jw);
- free(rd->buff);
- rd_free_devmap(rd);
+ rd_free(rd);
}
int main(int argc, char **argv)
diff --git a/rdma/rdma.h b/rdma/rdma.h
index b85e3748..afc0e413 100644
--- a/rdma/rdma.h
+++ b/rdma/rdma.h
@@ -73,11 +73,11 @@ int cmd_dev(struct rd *rd);
int cmd_link(struct rd *rd);
int rd_exec_cmd(struct rd *rd, const struct rd_cmd *c, const char *str);
int rd_exec_dev(struct rd *rd, int (*cb)(struct rd *rd));
+void rd_free(struct rd *rd);
/*
* Device manipulation
*/
-void rd_free_devmap(struct rd *rd);
struct dev_map *dev_map_lookup(struct rd *rd, bool allow_port_index);
/*
diff --git a/rdma/utils.c b/rdma/utils.c
index c1069967..d7284801 100644
--- a/rdma/utils.c
+++ b/rdma/utils.c
@@ -144,10 +144,11 @@ int rd_dev_init_cb(const struct nlmsghdr *nlh, void *data)
return MNL_CB_OK;
}
-void rd_free_devmap(struct rd *rd)
+void rd_free(struct rd *rd)
{
if (!rd)
return;
+ free(rd->buff);
dev_map_cleanup(rd);
}
--
2.15.1
^ permalink raw reply related
* [PATCH iproute2-next 10/10] rdma: Move link execution logic to common code
From: Leon Romanovsky @ 2017-12-27 7:57 UTC (permalink / raw)
To: David Ahern; +Cc: Leon Romanovsky, netdev, Stephen Hemminger
In-Reply-To: <20171227075759.15289-1-leon@kernel.org>
From: Leon Romanovsky <leonro@mellanox.com>
Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
---
rdma/link.c | 46 +---------------------------------------------
rdma/rdma.h | 1 +
rdma/utils.c | 49 +++++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 51 insertions(+), 45 deletions(-)
diff --git a/rdma/link.c b/rdma/link.c
index d9392289..676cb21d 100644
--- a/rdma/link.c
+++ b/rdma/link.c
@@ -285,51 +285,7 @@ static int link_one_show(struct rd *rd)
static int link_show(struct rd *rd)
{
- struct dev_map *dev_map;
- uint32_t port;
- int ret = 0;
-
- if (rd->json_output)
- jsonw_start_array(rd->jw);
- if (rd_no_arg(rd)) {
- list_for_each_entry(dev_map, &rd->dev_map_list, list) {
- rd->dev_idx = dev_map->idx;
- for (port = 1; port < dev_map->num_ports + 1; port++) {
- rd->port_idx = port;
- ret = link_one_show(rd);
- if (ret)
- goto out;
- }
- }
-
- } else {
- dev_map = dev_map_lookup(rd, true);
- port = get_port_from_argv(rd);
- if (!dev_map || port > dev_map->num_ports) {
- pr_err("Wrong device name\n");
- ret = -ENOENT;
- goto out;
- }
- rd_arg_inc(rd);
- rd->dev_idx = dev_map->idx;
- rd->port_idx = port ? : 1;
- for (; rd->port_idx < dev_map->num_ports + 1; rd->port_idx++) {
- ret = link_one_show(rd);
- if (ret)
- goto out;
- if (port)
- /*
- * We got request to show link for devname
- * with port index.
- */
- break;
- }
- }
-
-out:
- if (rd->json_output)
- jsonw_end_array(rd->jw);
- return ret;
+ return rd_exec_link(rd, link_one_show);
}
int cmd_link(struct rd *rd)
diff --git a/rdma/rdma.h b/rdma/rdma.h
index afc0e413..8d53d3a0 100644
--- a/rdma/rdma.h
+++ b/rdma/rdma.h
@@ -73,6 +73,7 @@ int cmd_dev(struct rd *rd);
int cmd_link(struct rd *rd);
int rd_exec_cmd(struct rd *rd, const struct rd_cmd *c, const char *str);
int rd_exec_dev(struct rd *rd, int (*cb)(struct rd *rd));
+int rd_exec_link(struct rd *rd, int (*cb)(struct rd *rd));
void rd_free(struct rd *rd);
/*
diff --git a/rdma/utils.c b/rdma/utils.c
index d7284801..7b2001e2 100644
--- a/rdma/utils.c
+++ b/rdma/utils.c
@@ -152,6 +152,55 @@ void rd_free(struct rd *rd)
dev_map_cleanup(rd);
}
+int rd_exec_link(struct rd *rd, int (*cb)(struct rd *rd))
+{
+ struct dev_map *dev_map;
+ uint32_t port;
+ int ret = 0;
+
+ if (rd->json_output)
+ jsonw_start_array(rd->jw);
+ if (rd_no_arg(rd)) {
+ list_for_each_entry(dev_map, &rd->dev_map_list, list) {
+ rd->dev_idx = dev_map->idx;
+ for (port = 1; port < dev_map->num_ports + 1; port++) {
+ rd->port_idx = port;
+ ret = cb(rd);
+ if (ret)
+ goto out;
+ }
+ }
+
+ } else {
+ dev_map = dev_map_lookup(rd, true);
+ port = get_port_from_argv(rd);
+ if (!dev_map || port > dev_map->num_ports) {
+ pr_err("Wrong device name\n");
+ ret = -ENOENT;
+ goto out;
+ }
+ rd_arg_inc(rd);
+ rd->dev_idx = dev_map->idx;
+ rd->port_idx = port ? : 1;
+ for (; rd->port_idx < dev_map->num_ports + 1; rd->port_idx++) {
+ ret = cb(rd);
+ if (ret)
+ goto out;
+ if (port)
+ /*
+ * We got request to show link for devname
+ * with port index.
+ */
+ break;
+ }
+ }
+
+out:
+ if (rd->json_output)
+ jsonw_end_array(rd->jw);
+ return ret;
+}
+
int rd_exec_dev(struct rd *rd, int (*cb)(struct rd *rd))
{
struct dev_map *dev_map;
--
2.15.1
^ permalink raw reply related
* [PATCH iproute2-next 05/10] rdma: Check that port index exists before operate on link layer
From: Leon Romanovsky @ 2017-12-27 7:57 UTC (permalink / raw)
To: David Ahern; +Cc: Leon Romanovsky, netdev, Stephen Hemminger
In-Reply-To: <20171227075759.15289-1-leon@kernel.org>
From: Leon Romanovsky <leonro@mellanox.com>
Link layer operates on port layer, hence it should check
it existence before execution commands.
Fixes: da990ab40a92 ("rdma: Add link object")
Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
---
rdma/link.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/rdma/link.c b/rdma/link.c
index f0eaccbb..d9392289 100644
--- a/rdma/link.c
+++ b/rdma/link.c
@@ -277,6 +277,9 @@ static int link_one_show(struct rd *rd)
{ 0 }
};
+ if (!rd->port_idx)
+ return 0;
+
return rd_exec_cmd(rd, cmds, "parameter");
}
--
2.15.1
^ permalink raw reply related
* RE: [PATCH net-next v2] net: sched: fix skb leak in dev_requeue_skb()
From: weiyongjun (A) @ 2017-12-27 8:02 UTC (permalink / raw)
To: John Fastabend, Jamal Hadi Salim, Cong Wang, Jiri Pirko
Cc: netdev@vger.kernel.org
In-Reply-To: <42a543e3-82ff-2421-186d-cc0957ea45ed@gmail.com>
On 12/27/2017 1:24 PM, John Fastabend wrote:
> On 12/24/2017 07:49 PM, Wei Yongjun wrote:
> > When dev_requeue_skb() is called with bluked skb list, only the
> > first skb of the list will be requeued to qdisc layer, and leak
> > the others without free them.
> >
> > TCP is broken due to skb leak since no free skb will be considered
> > as still in the host queue and never be retransmitted. This happend
> > when dev_requeue_skb() called from qdisc_restart().
> > qdisc_restart
> > |-- dequeue_skb
> > |-- sch_direct_xmit()
> > |-- dev_requeue_skb() <-- skb may bluked
> >
> > Fix dev_requeue_skb() to requeue the full bluked list.
> >
> > Fixes: a53851e2c321 ("net: sched: explicit locking in gso_cpu fallback")
> > Signed-off-by: Wei Yongjun <weiyongjun1@huawei.com>
> > ---
> > v1 -> v2: add net-next prefix
> > ---
>
> First, thanks for tracking this down.
>
> > diff --git a/net/sched/sch_generic.c b/net/sched/sch_generic.c
> > index 981c08f..0df2dbf 100644
> > --- a/net/sched/sch_generic.c
> > +++ b/net/sched/sch_generic.c
> > @@ -111,10 +111,16 @@ static inline void
> qdisc_enqueue_skb_bad_txq(struct Qdisc *q,
> >
> > static inline int __dev_requeue_skb(struct sk_buff *skb, struct Qdisc *q)
> > {
> > - __skb_queue_head(&q->gso_skb, skb);
> > - q->qstats.requeues++;
> > - qdisc_qstats_backlog_inc(q, skb);
> > - q->q.qlen++; /* it's still part of the queue */
> > + while (skb) {
> > + struct sk_buff *next = skb->next;
> > +
> > + __skb_queue_tail(&q->gso_skb, skb);
>
> Was the change from __skb_queue_head to __skb_queue_tail here
> intentional? We should re-queue packets to the head of the list.
skb is dequeue from gso_skb in order, so if packets re-queue to the head
of the list, skb will be sent in reverse order, so I think __skb_queue_tail is
better here. Tcpdump show those skbs are out of order packets.
>
> > + q->qstats.requeues++;
> > + qdisc_qstats_backlog_inc(q, skb);
> > + q->q.qlen++; /* it's still part of the queue */
> > +
> > + skb = next;
> > + }
> > __netif_schedule(q);
> >
> > return 0;
> > @@ -124,13 +130,20 @@ static inline int dev_requeue_skb_locked(struct
> sk_buff *skb, struct Qdisc *q)
> > {
> > spinlock_t *lock = qdisc_lock(q);
> >
> > - spin_lock(lock);
> > - __skb_queue_tail(&q->gso_skb, skb);
> > - spin_unlock(lock);
> > + while (skb) {
> > + struct sk_buff *next = skb->next;
> > +
> > + spin_lock(lock);
>
> In this case I suspect its better to move the lock to be around the
> while loop rather than grab and drop it repeatedly. I don't have
> any data at this point so OK either way. Assuming other head/tail
> comment is addressed.
I will fix it in v3.
>
> > + __skb_queue_tail(&q->gso_skb, skb);
>
> Same here *_tail should be *_head?
>
> > + spin_unlock(lock);
> > +
> > + qdisc_qstats_cpu_requeues_inc(q);
> > + qdisc_qstats_cpu_backlog_inc(q, skb);
> > + qdisc_qstats_cpu_qlen_inc(q);
> > +
> > + skb = next;
> > + }
> >
> > - qdisc_qstats_cpu_requeues_inc(q);
> > - qdisc_qstats_cpu_backlog_inc(q, skb);
> > - qdisc_qstats_cpu_qlen_inc(q);
> > __netif_schedule(q);
> >
> > return 0;
> >
^ permalink raw reply
* Re: [patch net-next v2 00/10] Add support for resource abstraction
From: Jiri Pirko @ 2017-12-27 8:09 UTC (permalink / raw)
To: David Ahern
Cc: netdev, davem, arkadis, mlxsw, andrew, vivien.didelot, f.fainelli,
michael.chan, ganeshgr, saeedm, matanb, leonro, idosch,
jakub.kicinski, ast, daniel, simon.horman, pieter.jansenvanvuuren,
john.hurley, alexander.h.duyck, linville, gospo, steven.lin1,
yuvalm, ogerlitz, roopa
In-Reply-To: <c72fd96e-9f46-974c-733f-3c9336285d52@cumulusnetworks.com>
Wed, Dec 27, 2017 at 05:05:09AM CET, dsa@cumulusnetworks.com wrote:
>On 12/26/17 5:23 AM, Jiri Pirko wrote:
>> From: Jiri Pirko <jiri@mellanox.com>
>>
>> Many of the ASIC's internal resources are limited and are shared between
>> several hardware procedures. For example, unified hash-based memory can
>> be used for many lookup purposes, like FDB and LPM. In many cases the user
>> can provide a partitioning scheme for such a resource in order to perform
>> fine tuning for his application. In such cases performing driver reload is
>> needed for the changes to take place, thus this patchset also adds support
>> for hot reload.
>>
>> Such an abstraction can be coupled with devlink's dpipe interface, which
>> models the ASIC's pipeline as a graph of match/action tables. By modeling
>> the hardware resource object, and by coupling it to several dpipe tables,
>> further visibility can be achieved in order to debug ASIC-wide issues.
>>
>> The proposed interface will provide the user the ability to understand the
>> limitations of the hardware, and receive notification regarding its occupancy.
>> Furthermore, monitoring the resource occupancy can be done in real-time and
>> can be useful in many cases.
>
>In the last RFC (not v1, but RFC) I asked for some kind of description
>for each resource, and you and Arkadi have pushed back. Let's walk
>through an example to see what I mean:
>
>$ devlink resource show pci/0000:03:00.0
>pci/0000:03:00.0:
> name kvd size 245760 size_valid true
> resources:
> name linear size 98304 occ 0
> name hash_double size 60416
> name hash_single size 87040
>
>So this 2700 has 3 resources that can be managed -- some table or
>resource or something named 'kvd' with linear, hash_double and
>hash_single sub-resources. What are these names referring too? The above
>output gives no description, and 'kvd' is not an industry term. Further,
This are internal resources specific to the ASIC. Would you like some
description to each or something like that?
>what are these sizes that a user can control? The output contains no
>units, no description, nothing. In short, the above output provides
>random numbers associated with random names.
Units are now exposed from kernel, just this version of iproute2 patch
does not display it.
>
>I can see dpipe tables exported by this device:
>
>$ devlink dpipe header show pci/0000:03:00.0
>
>pci/0000:03:00.0:
> name mlxsw_meta
> field:
> name erif_port bitwidth 32 mapping_type ifindex
> name l3_forward bitwidth 1
> name l3_drop bitwidth 1
> name adj_index bitwidth 32
> name adj_size bitwidth 32
> name adj_hash_index bitwidth 32
>
> name ipv6
> field:
> name destination ip bitwidth 128
>
> name ipv4
> field:
> name destination ip bitwidth 32
>
> name ethernet
> field:
> name destination mac bitwidth 48
>
>but none mention 'kvd' or 'linear' or 'hash" and none of the other
>various devlink options:
>
>$ devlink
>Usage: devlink [ OPTIONS ] OBJECT { COMMAND | help }
>where OBJECT := { dev | port | sb | monitor | dpipe }
>
>seem to related to resources.
>
>So how does a user know what they are controlling by this 'resource'
>option? Is the user expected to have a PRM or user guide on hand for the
>specific device model that is being configured?
The relation of specific dpipe table to specific resource is exposed by
the kernel as well. Probably the iproute2 patch just does not display
it.
>
>Again, I have no objections to kvd, linear, hash, etc terms as they do
>relate to Mellanox products. But kvd/linear, for example, does correlate
>to industry standard concepts in some way. My request is that the
>resource listing guide the user in some way, stating what these
>resources mean.
So the showed relation to dpipe table would be enougn or you would still
like to see some description? I don't like the description concept here
as the relations to dpipe table should tell user exactly what he needs
to know.
>
>IMO the above output is not user friendly and having to keep a PRM on
>hand for each device model is not a realistic solution.
^ permalink raw reply
* Re: [RFC PATCH bpf-next v2 4/4] error-injection: Support fault injection framework
From: Masami Hiramatsu @ 2017-12-27 8:09 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: Alexei Starovoitov, Josef Bacik, rostedt, mingo, davem, netdev,
linux-kernel, ast, kernel-team, daniel, linux-btrfs, darrick.wong,
Josef Bacik, Akinobu Mita
In-Reply-To: <20171227021255.sahkcpgyzohl5brs@ast-mbp>
On Tue, 26 Dec 2017 18:12:56 -0800
Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote:
> On Tue, Dec 26, 2017 at 04:48:25PM +0900, Masami Hiramatsu wrote:
> > Support in-kernel fault-injection framework via debugfs.
> > This allows you to inject a conditional error to specified
> > function using debugfs interfaces.
> >
> > Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
> > ---
> > Documentation/fault-injection/fault-injection.txt | 5 +
> > kernel/Makefile | 1
> > kernel/fail_function.c | 169 +++++++++++++++++++++
> > lib/Kconfig.debug | 10 +
> > 4 files changed, 185 insertions(+)
> > create mode 100644 kernel/fail_function.c
> >
> > diff --git a/Documentation/fault-injection/fault-injection.txt b/Documentation/fault-injection/fault-injection.txt
> > index 918972babcd8..6243a588dd71 100644
> > --- a/Documentation/fault-injection/fault-injection.txt
> > +++ b/Documentation/fault-injection/fault-injection.txt
> > @@ -30,6 +30,11 @@ o fail_mmc_request
> > injects MMC data errors on devices permitted by setting
> > debugfs entries under /sys/kernel/debug/mmc0/fail_mmc_request
> >
> > +o fail_function
> > +
> > + injects error return on specific functions by setting debugfs entries
> > + under /sys/kernel/debug/fail_function. No boot option supported.
>
> I like it.
> Could you document it a bit better?
Yes, I will do in next series.
> In particular retval is configurable, but without an example no one
> will be able to figure out how to use it.
Ah, right. BTW, as I pointed in the covermail, should we store the
expected error value range into the injectable list? e.g.
ALLOW_ERROR_INJECTION(open_ctree, -1, -MAX_ERRNO)
And provide APIs to check/get it.
const struct error_range *ei_get_error_range(unsigned long addr);
>
> I think you can drop RFC tag from the next version of these patches.
> Thanks!
Thank you, I'll fix some errors came from configurations, and resend it.
Thanks!
--
Masami Hiramatsu <mhiramat@kernel.org>
^ permalink raw reply
* Re: [PATCH v2 net-next 3/4] qed*: Utilize FW 8.33.1.0
From: kbuild test robot @ 2017-12-27 8:09 UTC (permalink / raw)
To: Tomer Tayar
Cc: kbuild-all, davem, netdev, linux-rdma, linux-scsi, Ariel Elior,
Michal Kalderon, Yuval Bason, Ram Amrani, Manish Chopra,
Chad Dupuis, Manish Rangankar
In-Reply-To: <1514310988-3030-4-git-send-email-Tomer.Tayar@cavium.com>
[-- Attachment #1: Type: text/plain, Size: 3139 bytes --]
Hi Tomer,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on net-next/master]
url: https://github.com/0day-ci/linux/commits/Tomer-Tayar/qed-Refactoring-and-rearranging-FW-API-with-no-functional-impact/20171227-110607
config: i386-randconfig-h0-12271326 (attached as .config)
compiler: gcc-4.9 (Debian 4.9.4-2) 4.9.4
reproduce:
# save the attached .config to linux build tree
make ARCH=i386
All errors (new ones prefixed by >>):
drivers/net/ethernet/qlogic/qed/qed_init_fw_funcs.o: In function `qed_calc_cdu_validation_byte':
>> drivers/net/ethernet/qlogic/qed/qed_init_fw_funcs.c:1322: undefined reference to `crc8_populate_msb'
>> drivers/net/ethernet/qlogic/qed/qed_init_fw_funcs.c:1343: undefined reference to `crc8'
vim +1322 drivers/net/ethernet/qlogic/qed/qed_init_fw_funcs.c
1311
1312 /* Calculate and return CDU validation byte per connection type/region/cid */
1313 static u8 qed_calc_cdu_validation_byte(u8 conn_type, u8 region, u32 cid)
1314 {
1315 const u8 validation_cfg = CDU_VALIDATION_DEFAULT_CFG;
1316 u8 crc, validation_byte = 0;
1317 static u8 crc8_table_valid; /* automatically initialized to 0 */
1318 u32 validation_string = 0;
1319 u32 data_to_crc;
1320
1321 if (!crc8_table_valid) {
> 1322 crc8_populate_msb(cdu_crc8_table, 0x07);
1323 crc8_table_valid = 1;
1324 }
1325
1326 /* The CRC is calculated on the String-to-compress:
1327 * [31:8] = {CID[31:20],CID[11:0]}
1328 * [7:4] = Region
1329 * [3:0] = Type
1330 */
1331 if ((validation_cfg >> CDU_CONTEXT_VALIDATION_CFG_USE_CID) & 1)
1332 validation_string |= (cid & 0xFFF00000) | ((cid & 0xFFF) << 8);
1333
1334 if ((validation_cfg >> CDU_CONTEXT_VALIDATION_CFG_USE_REGION) & 1)
1335 validation_string |= ((region & 0xF) << 4);
1336
1337 if ((validation_cfg >> CDU_CONTEXT_VALIDATION_CFG_USE_TYPE) & 1)
1338 validation_string |= (conn_type & 0xF);
1339
1340 /* Convert to big-endian and calculate CRC8 */
1341 data_to_crc = be32_to_cpu(validation_string);
1342
> 1343 crc = crc8(cdu_crc8_table,
1344 (u8 *)&data_to_crc, sizeof(data_to_crc), CRC8_INIT_VALUE);
1345
1346 /* The validation byte [7:0] is composed:
1347 * for type A validation
1348 * [7] = active configuration bit
1349 * [6:0] = crc[6:0]
1350 *
1351 * for type B validation
1352 * [7] = active configuration bit
1353 * [6:3] = connection_type[3:0]
1354 * [2:0] = crc[2:0]
1355 */
1356 validation_byte |=
1357 ((validation_cfg >>
1358 CDU_CONTEXT_VALIDATION_CFG_USE_ACTIVE) & 1) << 7;
1359
1360 if ((validation_cfg >>
1361 CDU_CONTEXT_VALIDATION_CFG_VALIDATION_TYPE_SHIFT) & 1)
1362 validation_byte |= ((conn_type & 0xF) << 3) | (crc & 0x7);
1363 else
1364 validation_byte |= crc & 0x7F;
1365
1366 return validation_byte;
1367 }
1368
---
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: 27141 bytes --]
^ permalink raw reply
* Re: [patch net-next v2 00/10] Add support for resource abstraction
From: Andrew Lunn @ 2017-12-27 8:23 UTC (permalink / raw)
To: Jiri Pirko
Cc: David Ahern, netdev, davem, arkadis, mlxsw, vivien.didelot,
f.fainelli, michael.chan, ganeshgr, saeedm, matanb, leonro,
idosch, jakub.kicinski, ast, daniel, simon.horman,
pieter.jansenvanvuuren, john.hurley, alexander.h.duyck, linville,
gospo, steven.lin1, yuvalm, ogerlitz, roopa
In-Reply-To: <20171227080902.GA1997@nanopsycho>
> >$ devlink resource show pci/0000:03:00.0
> >pci/0000:03:00.0:
> > name kvd size 245760 size_valid true
> > resources:
> > name linear size 98304 occ 0
> > name hash_double size 60416
> > name hash_single size 87040
> >
> >So this 2700 has 3 resources that can be managed -- some table or
> >resource or something named 'kvd' with linear, hash_double and
> >hash_single sub-resources. What are these names referring too? The above
> >output gives no description, and 'kvd' is not an industry term. Further,
>
> This are internal resources specific to the ASIC. Would you like some
> description to each or something like that?
The fact you have decided to expose them means you expect people to
change them. So yes, they need to be documented. Maybe add something
to Documentation/ABI/stable/
> So the showed relation to dpipe table would be enougn or you would still
> like to see some description? I don't like the description concept here
> as the relations to dpipe table should tell user exactly what he needs
> to know.
Documenting the ABI is good practice.
Andrew
^ permalink raw reply
* Re: [PATCHv3 1/3] dt-bindings: net: Add DT bindings for Socionext Netsec
From: Andrew Lunn @ 2017-12-27 8:28 UTC (permalink / raw)
To: Florian Fainelli
Cc: jassisinghbrar, netdev, devicetree, davem, arnd.bergmann,
ard.biesheuvel, robh+dt, mark.rutland, masami.hiramatsu,
Jassi Brar
In-Reply-To: <f42d29cc-6b3f-20dc-3f42-bc9a9fc7ce4a@gmail.com>
On Tue, Dec 26, 2017 at 11:34:26AM -0800, Florian Fainelli wrote:
> On 12/21/2017 05:55 AM, Andrew Lunn wrote:
> >> +- mdio device tree subnode: When the Netsec has a phy connected to its local
> >> + mdio, there must be device tree subnode with the following
> >> + required properties:
> >> +
> >> + - compatible: Must be "socionext,snq-mdio".
> >
> > Is there a need for a compatible string? Is there different versions
> > of the MDIO bus hardware? If it was an independent MDIO bus driver,
> > then yes, you need a compatible string. But since it is embedded in
> > the MAC driver, there should not be a need.
>
> I don't see a problem with doing that though, it may be extra
> information, but if we ever have a standalone MDIO bus controller that
> happens to be supported by the same HW/driver, and the Ethernet driver
> delegates the MDIO management to this MDIO bus driver, then having the
> compatible string is kind of mandatory for proper matching/identification.
Hi Florian
I does not create a problem. It does however make it different to
every other embedded MDIO bus.
My preference is not to have the compatible string, but i can live
with it.
Andrew
^ permalink raw reply
* RE: [patch net-next v2 00/10] Add support for resource abstraction
From: Yuval Mintz @ 2017-12-27 8:28 UTC (permalink / raw)
To: Jiri Pirko, David Ahern
Cc: netdev@vger.kernel.org, davem@davemloft.net, Arkadi Sharshevsky,
mlxsw, andrew@lunn.ch, vivien.didelot@savoirfairelinux.com,
f.fainelli@gmail.com, michael.chan@broadcom.com,
ganeshgr@chelsio.com, Saeed Mahameed, Matan Barak,
Leon Romanovsky, Ido Schimmel, jakub.kicinski@netronome.com,
ast@kernel.org, daniel@iogearbox.net,
"simon.horman@netronome.com" <simon.horman
In-Reply-To: <20171227080902.GA1997@nanopsycho>
> >> Many of the ASIC's internal resources are limited and are shared between
> >> several hardware procedures. For example, unified hash-based memory
> can
> >> be used for many lookup purposes, like FDB and LPM. In many cases the
> user
> >> can provide a partitioning scheme for such a resource in order to perform
> >> fine tuning for his application. In such cases performing driver reload is
> >> needed for the changes to take place, thus this patchset also adds support
> >> for hot reload.
> >>
> >> Such an abstraction can be coupled with devlink's dpipe interface, which
> >> models the ASIC's pipeline as a graph of match/action tables. By modeling
> >> the hardware resource object, and by coupling it to several dpipe tables,
> >> further visibility can be achieved in order to debug ASIC-wide issues.
> >>
> >> The proposed interface will provide the user the ability to understand the
> >> limitations of the hardware, and receive notification regarding its
> occupancy.
> >> Furthermore, monitoring the resource occupancy can be done in real-time
> and
> >> can be useful in many cases.
> >
> >In the last RFC (not v1, but RFC) I asked for some kind of description
> >for each resource, and you and Arkadi have pushed back. Let's walk
> >through an example to see what I mean:
> >
> >$ devlink resource show pci/0000:03:00.0
> >pci/0000:03:00.0:
> > name kvd size 245760 size_valid true
> > resources:
> > name linear size 98304 occ 0
> > name hash_double size 60416
> > name hash_single size 87040
> >
> >So this 2700 has 3 resources that can be managed -- some table or
> >resource or something named 'kvd' with linear, hash_double and
> >hash_single sub-resources. What are these names referring too? The above
> >output gives no description, and 'kvd' is not an industry term. Further,
>
> This are internal resources specific to the ASIC. Would you like some
> description to each or something like that?
>
>
> >what are these sizes that a user can control? The output contains no
> >units, no description, nothing. In short, the above output provides
> >random numbers associated with random names.
>
> Units are now exposed from kernel, just this version of iproute2 patch
> does not display it.
>
>
> >
> >I can see dpipe tables exported by this device:
> >
> >$ devlink dpipe header show pci/0000:03:00.0
> >
> >pci/0000:03:00.0:
> > name mlxsw_meta
> > field:
> > name erif_port bitwidth 32 mapping_type ifindex
> > name l3_forward bitwidth 1
> > name l3_drop bitwidth 1
> > name adj_index bitwidth 32
> > name adj_size bitwidth 32
> > name adj_hash_index bitwidth 32
> >
> > name ipv6
> > field:
> > name destination ip bitwidth 128
> >
> > name ipv4
> > field:
> > name destination ip bitwidth 32
> >
> > name ethernet
> > field:
> > name destination mac bitwidth 48
> >
> >but none mention 'kvd' or 'linear' or 'hash" and none of the other
> >various devlink options:
> >
> >$ devlink
> >Usage: devlink [ OPTIONS ] OBJECT { COMMAND | help }
> >where OBJECT := { dev | port | sb | monitor | dpipe }
> >
> >seem to related to resources.
> >
> >So how does a user know what they are controlling by this 'resource'
> >option? Is the user expected to have a PRM or user guide on hand for the
> >specific device model that is being configured?
>
> The relation of specific dpipe table to specific resource is exposed by
> the kernel as well. Probably the iproute2 patch just does not display
> it.
It does, just under 'table' and not under 'header'. E.g.:
# ./devlink/devlink dpipe -j -p table show pci/0000:03:00.0
...
},{
"resource_path": "/kvd/hash_single",
"name": "mlxsw_host4",
"size": 0,
"counters_enabled": "false",
"match": [{
"type": "field_exact",
"header": "mlxsw_meta",
"field": "erif_port",
"mapping": "ifindex"
},{
"type": "field_exact",
"header": "ipv4",
"field": "destination ip"
}
],
"action": [{
"type": "field_modify",
"header": "ethernet",
"field": "destination mac"
}
]
},{
...
>
>
> >
> >Again, I have no objections to kvd, linear, hash, etc terms as they do
> >relate to Mellanox products. But kvd/linear, for example, does correlate
> >to industry standard concepts in some way. My request is that the
> >resource listing guide the user in some way, stating what these
> >resources mean.
>
> So the showed relation to dpipe table would be enougn or you would still
> like to see some description? I don't like the description concept here
> as the relations to dpipe table should tell user exactly what he needs
> to know.
>
>
> >
> >IMO the above output is not user friendly and having to keep a PRM on
> >hand for each device model is not a realistic solution.
^ permalink raw reply
* [PATCH net-next v3] net: sched: fix skb leak in dev_requeue_skb()
From: Wei Yongjun @ 2017-12-27 9:05 UTC (permalink / raw)
To: John Fastabend, Jamal Hadi Salim, Cong Wang, Jiri Pirko
Cc: Wei Yongjun, netdev
When dev_requeue_skb() is called with bluked skb list, only the
first skb of the list will be requeued to qdisc layer, and leak
the others without free them.
TCP is broken due to skb leak since no free skb will be considered
as still in the host queue and never be retransmitted. This happend
when dev_requeue_skb() called from qdisc_restart().
qdisc_restart
|-- dequeue_skb
|-- sch_direct_xmit()
|-- dev_requeue_skb() <-- skb may bluked
Fix dev_requeue_skb() to requeue the full bluked list. Also change
to use __skb_queue_tail() in __dev_requeue_skb() to avoid skb out
of order.
Fixes: a53851e2c321 ("net: sched: explicit locking in gso_cpu fallback")
Signed-off-by: Wei Yongjun <weiyongjun1@huawei.com>
---
v2 -> v3: move lock out of while loop
---
net/sched/sch_generic.c | 29 +++++++++++++++++++++--------
1 file changed, 21 insertions(+), 8 deletions(-)
diff --git a/net/sched/sch_generic.c b/net/sched/sch_generic.c
index 10aaa3b6..1c149ed 100644
--- a/net/sched/sch_generic.c
+++ b/net/sched/sch_generic.c
@@ -111,10 +111,16 @@ static inline void qdisc_enqueue_skb_bad_txq(struct Qdisc *q,
static inline int __dev_requeue_skb(struct sk_buff *skb, struct Qdisc *q)
{
- __skb_queue_head(&q->gso_skb, skb);
- q->qstats.requeues++;
- qdisc_qstats_backlog_inc(q, skb);
- q->q.qlen++; /* it's still part of the queue */
+ while (skb) {
+ struct sk_buff *next = skb->next;
+
+ __skb_queue_tail(&q->gso_skb, skb);
+ q->qstats.requeues++;
+ qdisc_qstats_backlog_inc(q, skb);
+ q->q.qlen++; /* it's still part of the queue */
+
+ skb = next;
+ }
__netif_schedule(q);
return 0;
@@ -125,12 +131,19 @@ static inline int dev_requeue_skb_locked(struct sk_buff *skb, struct Qdisc *q)
spinlock_t *lock = qdisc_lock(q);
spin_lock(lock);
- __skb_queue_tail(&q->gso_skb, skb);
+ while (skb) {
+ struct sk_buff *next = skb->next;
+
+ __skb_queue_tail(&q->gso_skb, skb);
+
+ qdisc_qstats_cpu_requeues_inc(q);
+ qdisc_qstats_cpu_backlog_inc(q, skb);
+ qdisc_qstats_cpu_qlen_inc(q);
+
+ skb = next;
+ }
spin_unlock(lock);
- qdisc_qstats_cpu_requeues_inc(q);
- qdisc_qstats_cpu_backlog_inc(q, skb);
- qdisc_qstats_cpu_qlen_inc(q);
__netif_schedule(q);
return 0;
--
1.8.3.1
^ permalink raw reply related
* Re: [patch net-next v2 00/10] Add support for resource abstraction
From: Jiri Pirko @ 2017-12-27 9:37 UTC (permalink / raw)
To: Andrew Lunn
Cc: David Ahern, netdev, davem, arkadis, mlxsw, vivien.didelot,
f.fainelli, michael.chan, ganeshgr, saeedm, matanb, leonro,
idosch, jakub.kicinski, ast, daniel, simon.horman,
pieter.jansenvanvuuren, john.hurley, alexander.h.duyck, linville,
gospo, steven.lin1, yuvalm, ogerlitz, roopa
In-Reply-To: <20171227082331.GA10517@lunn.ch>
Wed, Dec 27, 2017 at 09:23:31AM CET, andrew@lunn.ch wrote:
>> >$ devlink resource show pci/0000:03:00.0
>> >pci/0000:03:00.0:
>> > name kvd size 245760 size_valid true
>> > resources:
>> > name linear size 98304 occ 0
>> > name hash_double size 60416
>> > name hash_single size 87040
>> >
>> >So this 2700 has 3 resources that can be managed -- some table or
>> >resource or something named 'kvd' with linear, hash_double and
>> >hash_single sub-resources. What are these names referring too? The above
>> >output gives no description, and 'kvd' is not an industry term. Further,
>>
>> This are internal resources specific to the ASIC. Would you like some
>> description to each or something like that?
>
>The fact you have decided to expose them means you expect people to
>change them. So yes, they need to be documented. Maybe add something
>to Documentation/ABI/stable/
>
>> So the showed relation to dpipe table would be enougn or you would still
>> like to see some description? I don't like the description concept here
>> as the relations to dpipe table should tell user exactly what he needs
>> to know.
>
>Documenting the ABI is good practice.
This is misunderstanding I believe. This is not about ABI. That is well
defined by the netlink attributes. This is about meaning of particular
ASIC-specific internal resources.
>
> Andrew
>
^ permalink raw reply
* Re: [next] ath10k: wmi: remove redundant integer fc
From: Kalle Valo @ 2017-12-27 10:03 UTC (permalink / raw)
To: Colin Ian King
Cc: ath10k, linux-wireless, netdev, kernel-janitors, linux-kernel
In-Reply-To: <20171218150245.9073-1-colin.king@canonical.com>
Colin Ian King <colin.king@canonical.com> wrote:
> Variable fc is being assigned but never used, so remove it. Cleans
> up the clang warning:
>
> warning: Value stored to 'fc' is never read
>
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> Signed-off-by: Kalle Valo <kvalo@qca.qualcomm.com>
Patch applied to ath-next branch of ath.git, thanks.
a0709dfd7ff8 ath10k: wmi: remove redundant integer fc
--
https://patchwork.kernel.org/patch/10119831/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
^ permalink raw reply
* Re: wil6210: fix build warnings without CONFIG_PM
From: Kalle Valo @ 2017-12-27 10:04 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Maya Erez, Kalle Valo, Arnd Bergmann, Lazar Alexei, Dedy Lansky,
Hamad Kadmany, Lior David, Gidon Studinski, linux-wireless,
wil6210, netdev, linux-kernel
In-Reply-To: <20171218134604.3087909-1-arnd@arndb.de>
Arnd Bergmann <arnd@arndb.de> wrote:
> The #ifdef checks are hard to get right, in this case some functions
> should have been left inside a CONFIG_PM_SLEEP check as seen by this
> message:
>
> drivers/net/wireless/ath/wil6210/pcie_bus.c:489:12: error: 'wil6210_pm_resume' defined but not used [-Werror=unused-function]
> drivers/net/wireless/ath/wil6210/pcie_bus.c:484:12: error: 'wil6210_pm_suspend' defined but not used [-Werror=unused-function]
>
> Using an __maybe_unused is easier here, so I'm replacing all the
> other #ifdef in this file as well for consistency.
>
> Fixes: 94162666cd51 ("wil6210: run-time PM when interface down")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> Signed-off-by: Kalle Valo <kvalo@qca.qualcomm.com>
Patch applied to ath-next branch of ath.git, thanks.
203dab8395d9 wil6210: fix build warnings without CONFIG_PM
--
https://patchwork.kernel.org/patch/10119565/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
^ permalink raw reply
* Re: You will definetely be interested...
From: Sra. Angel Rania @ 2017-12-27 10:16 UTC (permalink / raw)
Hi Dear,
Reading your profile has given me courage in search of a reasponsable
and trust worthy Fellow. The past has treated me so awfully but now I
am ready to move on despite of my health condition. I will like to
have a sincere and important discussion with you that will be in your
favor likewise to you and your environment especially to your close
family. Endeavor to reply me and I have attached my picture in case
you long to know who emailed you. I will be waiting to hear from you
as soon as possble.
Thanks for paying attention to my mail and will appreciate so much if
I receive a reply from you for understable details.
Thanks,
Mrs. Rania Hassan
^ permalink raw reply
* RE: [bpf-next V2 PATCH 05/14] xdp/qede: setup xdp_rxq_info and intro xdp_rxq_info_is_reg
From: Chopra, Manish @ 2017-12-27 10:37 UTC (permalink / raw)
To: Jesper Dangaard Brouer, Daniel Borkmann, Alexei Starovoitov
Cc: bjorn.topel@intel.com, netdev@vger.kernel.org, Elior, Ariel,
dsahern@gmail.com, gospo@broadcom.com, Dept-Eng Everest Linux L2,
michael.chan@broadcom.com
In-Reply-To: <151396272008.20006.96305338181911363.stgit@firesoul>
> -----Original Message-----
> From: Jesper Dangaard Brouer [mailto:brouer@redhat.com]
> Sent: Friday, December 22, 2017 10:42 PM
> To: Daniel Borkmann <borkmann@iogearbox.net>; Alexei Starovoitov
> <alexei.starovoitov@gmail.com>
> Cc: bjorn.topel@intel.com; netdev@vger.kernel.org; Jesper Dangaard Brouer
> <brouer@redhat.com>; Elior, Ariel <Ariel.Elior@cavium.com>;
> dsahern@gmail.com; gospo@broadcom.com; Dept-Eng Everest Linux L2 <Dept-
> EngEverestLinuxL2@cavium.com>; michael.chan@broadcom.com
> Subject: [bpf-next V2 PATCH 05/14] xdp/qede: setup xdp_rxq_info and intro
> xdp_rxq_info_is_reg
>
> The driver code qede_free_fp_array() depend on kfree() can be called with a
> NULL pointer. This stems from the qede_alloc_fp_array() function which either
> (kz)alloc memory for fp->txq or fp->rxq.
> This also simplifies error handling code in case of memory allocation failures,
> but xdp_rxq_info_unreg need to know the difference.
>
> Introduce xdp_rxq_info_is_reg() to handle if a memory allocation fails and
> detect this is the failure path by seeing that xdp_rxq_info was not registred yet,
> which first happens after successful alloaction in qede_init_fp().
>
> Driver hook points for xdp_rxq_info:
> * reg : qede_init_fp
> * unreg: qede_free_fp_array
>
> Tested on actual hardware with samples/bpf program.
>
> V2: Driver have no proper error path for failed XDP RX-queue info reg, as
> qede_init_fp() is a void function.
>
> Cc: everest-linux-l2@cavium.com
> Cc: Ariel Elior <Ariel.Elior@cavium.com>
> Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>
> ---
> drivers/net/ethernet/qlogic/qede/qede.h | 2 ++
> drivers/net/ethernet/qlogic/qede/qede_fp.c | 1 +
> drivers/net/ethernet/qlogic/qede/qede_main.c | 10 ++++++++++
> include/net/xdp.h | 1 +
> net/core/xdp.c | 6 ++++++
> 5 files changed, 20 insertions(+)
>
> diff --git a/drivers/net/ethernet/qlogic/qede/qede.h
> b/drivers/net/ethernet/qlogic/qede/qede.h
> index a3a70ade411f..62f47736511b 100644
> --- a/drivers/net/ethernet/qlogic/qede/qede.h
> +++ b/drivers/net/ethernet/qlogic/qede/qede.h
> @@ -40,6 +40,7 @@
> #include <linux/kernel.h>
> #include <linux/mutex.h>
> #include <linux/bpf.h>
> +#include <net/xdp.h>
> #include <linux/qed/qede_rdma.h>
> #include <linux/io.h>
> #ifdef CONFIG_RFS_ACCEL
> @@ -345,6 +346,7 @@ struct qede_rx_queue {
> u64 xdp_no_pass;
>
> void *handle;
> + struct xdp_rxq_info xdp_rxq;
> };
>
> union db_prod {
> diff --git a/drivers/net/ethernet/qlogic/qede/qede_fp.c
> b/drivers/net/ethernet/qlogic/qede/qede_fp.c
> index 48ec4c56cddf..dafc079ab6b9 100644
> --- a/drivers/net/ethernet/qlogic/qede/qede_fp.c
> +++ b/drivers/net/ethernet/qlogic/qede/qede_fp.c
> @@ -1006,6 +1006,7 @@ static bool qede_rx_xdp(struct qede_dev *edev,
> xdp.data = xdp.data_hard_start + *data_offset;
> xdp_set_data_meta_invalid(&xdp);
> xdp.data_end = xdp.data + *len;
> + xdp.rxq = &rxq->xdp_rxq;
>
> /* Queues always have a full reset currently, so for the time
> * being until there's atomic program replace just mark read diff --git
> a/drivers/net/ethernet/qlogic/qede/qede_main.c
> b/drivers/net/ethernet/qlogic/qede/qede_main.c
> index 57332b3e5e64..24160f1fd0e5 100644
> --- a/drivers/net/ethernet/qlogic/qede/qede_main.c
> +++ b/drivers/net/ethernet/qlogic/qede/qede_main.c
> @@ -762,6 +762,12 @@ static void qede_free_fp_array(struct qede_dev *edev)
> fp = &edev->fp_array[i];
>
> kfree(fp->sb_info);
> + /* Handle mem alloc failure case where qede_init_fp
> + * didn't register xdp_rxq_info yet.
> + * Implicit only (fp->type & QEDE_FASTPATH_RX)
> + */
> + if (fp->rxq && xdp_rxq_info_is_reg(&fp->rxq->xdp_rxq))
> + xdp_rxq_info_unreg(&fp->rxq->xdp_rxq);
Isn't this sufficient just ?
If (fp->rxq)
xdp_rxq_info_unreg(&fp->rxq->xdp_rxq);
xdp_rxq_info_is_reg() API seems unnecessary to me, xdp_rxq_info_unreg() seems to be taking care of the case
if it's not registered then WARN already, I think instead of adding these checks in individual drivers, better handle it properly in xdp_rxq_info_unreg() ?
> kfree(fp->rxq);
> kfree(fp->xdp_tx);
> kfree(fp->txq);
> @@ -1498,6 +1504,10 @@ static void qede_init_fp(struct qede_dev *edev)
> else
> fp->rxq->data_direction =
> DMA_FROM_DEVICE;
> fp->rxq->dev = &edev->pdev->dev;
> +
> + /* Driver have no error path from here */
> + WARN_ON(xdp_rxq_info_reg(&fp->rxq->xdp_rxq, edev-
> >ndev,
> + fp->rxq->rxq_id) < 0);
> }
We will reach to this point only when fp->rxq and edev are valid
I don't think that xdp_rxq_info_reg() will fail ever in that case.
^ permalink raw reply
* [PATCH iproute2 0/3] ip/tunnel: Fix noencap- and document "external" parameter handling.
From: Serhey Popovych @ 2017-12-27 11:28 UTC (permalink / raw)
To: netdev
In this series I present next set of improvements/fixes:
1) Fix noencap- option handling: we need to clear bit, instead
of seting all, but one we expect to clear.
2) Document "external" parameter both in ip-link(8) and help
output for link_gre.c. Add "noexternal" option variant to
bring inline with GENEVE and VXLAN.
3) Trivial: clear flowlabel/tclass from flowinfo in case of
inherit to stop sending garbage to the kernel. It has no
functional change, but follows similar behaviour in link_ip6tnl.c
See individual patch description message for details.
Thanks,
Serhii
Serhey Popovych (3):
gre,ip6tnl/tunnel: Fix noencap- support
gre6/tunnel: Do not submit garbage in flowinfo
ip/tunnel: Document "external" parameter
ip/link_gre.c | 7 +++++--
ip/link_gre6.c | 4 ++--
ip/link_ip6tnl.c | 6 ++++--
ip/link_iptnl.c | 4 +++-
man/man8/ip-link.8.in | 6 ++++++
5 files changed, 20 insertions(+), 7 deletions(-)
--
1.7.10.4
^ permalink raw reply
* [PATCH iproute2 1/3] gre,ip6tnl/tunnel: Fix noencap- support
From: Serhey Popovych @ 2017-12-27 11:28 UTC (permalink / raw)
To: netdev
In-Reply-To: <1514374096-1473-1-git-send-email-serhe.popovych@gmail.com>
We must clear bit, not set all but given bit.
Fixes: 858dbb208e39 ("ip link: Add support for remote checksum offload to IP tunnels")
Fixes: 73516e128a5a ("ip6tnl: Support for fou encapsulation"
Signed-off-by: Serhey Popovych <serhe.popovych@gmail.com>
---
ip/link_gre.c | 4 ++--
ip/link_ip6tnl.c | 2 +-
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/ip/link_gre.c b/ip/link_gre.c
index f55c40c..896bb19 100644
--- a/ip/link_gre.c
+++ b/ip/link_gre.c
@@ -281,11 +281,11 @@ get_failed:
} else if (strcmp(*argv, "encap-udp6-csum") == 0) {
encapflags |= TUNNEL_ENCAP_FLAG_CSUM6;
} else if (strcmp(*argv, "noencap-udp6-csum") == 0) {
- encapflags |= ~TUNNEL_ENCAP_FLAG_CSUM6;
+ encapflags &= ~TUNNEL_ENCAP_FLAG_CSUM6;
} else if (strcmp(*argv, "encap-remcsum") == 0) {
encapflags |= TUNNEL_ENCAP_FLAG_REMCSUM;
} else if (strcmp(*argv, "noencap-remcsum") == 0) {
- encapflags |= ~TUNNEL_ENCAP_FLAG_REMCSUM;
+ encapflags &= ~TUNNEL_ENCAP_FLAG_REMCSUM;
} else if (strcmp(*argv, "external") == 0) {
metadata = 1;
} else if (strcmp(*argv, "ignore-df") == 0) {
diff --git a/ip/link_ip6tnl.c b/ip/link_ip6tnl.c
index 0a471c2..84205b1 100644
--- a/ip/link_ip6tnl.c
+++ b/ip/link_ip6tnl.c
@@ -301,7 +301,7 @@ get_failed:
} else if (strcmp(*argv, "encap-remcsum") == 0) {
encapflags |= TUNNEL_ENCAP_FLAG_REMCSUM;
} else if (strcmp(*argv, "noencap-remcsum") == 0) {
- encapflags |= ~TUNNEL_ENCAP_FLAG_REMCSUM;
+ encapflags &= ~TUNNEL_ENCAP_FLAG_REMCSUM;
} else if (strcmp(*argv, "external") == 0) {
metadata = 1;
} else
--
1.7.10.4
^ 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