IOV iterator bug in net tree (was Re: Deadlock when running DevStack on latest pull of net tree)

[Alexander Duyck]

On Fri, May 27, 2016 at 11:21 AM, Alexander Duyck
 wrote:
> I started out this morning by trying to run DevStack on the latest
> "net' kernel and it looks like I am hanging on some sort of locking
> problem with RabbitMQ.  Specifically I am seeing one CPU jump to 100%
> with perf showing that I am spinning on a lock.
>
> I'm working to bisect it now, but just thought I would put it out
> there if anybody had already root caused this issue.  Below is a few
> traces to the spin lock call on the spinning CPU:

I misread the perf trace.  I wasn't spinning on a lock I was spinning
on the IOV iterator.  It just happened to push the lock to the top of
the perf trace.  I bisected it down to the recent change in
iterate_and_advance which was already fixed in Linus's tree in commit
19f18459330f "do 'fold checks into iterate_and_advance()" right'.

- Alex

[PATCH iproute2 net-next 0/5] bridge: json support for fdb and vlan show

[Roopa Prabhu]

From: Roopa Prabhu 

This patch series adds json support for a few bridge show commands.
We plan to follow up with json support for additional commands soon.

Anuradha Karuppiah (3):
  json_writer: allow base json data type to be array or object
  bridge: add json support for bridge fdb show
  bridge: add json schema for bridge fdb show

Roopa Prabhu (2):
  bridge: add json support for bridge vlan show
  bridge: update man page

 bridge/br_common.h|   1 +
 bridge/bridge.c   |   3 +
 bridge/fdb.c  | 201 +-
 bridge/vlan.c |  93 ++-
 include/json_writer.h |   5 +-
 lib/json_writer.c |  39 +++-
 man/man8/bridge.8 |   9 +-
 misc/ifstat.c |   6 +-
 misc/lnstat.c |   2 +-
 misc/nstat.c  |   4 +-
 schema/bridge_fdb_schema.json |  62 +
 11 files changed, 365 insertions(+), 60 deletions(-)
 create mode 100644 schema/bridge_fdb_schema.json

-- 
1.9.1

[PATCH iproute2 net-next 2/5] bridge: add json support for bridge vlan show

[Roopa Prabhu]

From: Roopa Prabhu 

$bridge -c vlan show
portvlan ids
swp1 1 PVID Egress Untagged
 10-13

swp2 1 PVID Egress Untagged
 10-13

br0  1 PVID Egress Untagged

$bridge -j vlan show
{
"swp1": [{
"vlan": 1,
"flags": "PVID Egress Untagged"
},{
"vlan": 10
},{
"vlan": 11
},{
"vlan": 12
},{
"vlan": 13
}
],
"swp2": [{
"vlan": 1,
"flags": "PVID Egress Untagged"
},{
"vlan": 10
},{
"vlan": 11
},{
"vlan": 12
},{
"vlan": 13
}
],
"br0": [{
"vlan": 1,
"flags": "PVID Egress Untagged"
}
]
}

$bridge -c -j vlan show
{
"swp1": [{
"vlan": 1,
"flags": "PVID Egress Untagged"
},{
"vlan": 10,
"vlanEnd": 13
}
],
"swp2": [{
"vlan": 1,
"flags": "PVID Egress Untagged"
},{
"vlan": 10,
"vlanEnd": 13
}
],
"br0": [{
"vlan": 1,
"flags": "PVID Egress Untagged"
}
]
}

Signed-off-by: Roopa Prabhu 
---
 bridge/br_common.h |  1 +
 bridge/bridge.c|  5 +++-
 bridge/vlan.c  | 83 ++
 3 files changed, 76 insertions(+), 13 deletions(-)

diff --git a/bridge/br_common.h b/bridge/br_common.h
index 5ea45c9..c649e7d 100644
--- a/bridge/br_common.h
+++ b/bridge/br_common.h
@@ -23,4 +23,5 @@ extern int show_stats;
 extern int show_details;
 extern int timestamp;
 extern int compress_vlans;
+extern int json_output;
 extern struct rtnl_handle rth;
diff --git a/bridge/bridge.c b/bridge/bridge.c
index 72f153f..5ff038d 100644
--- a/bridge/bridge.c
+++ b/bridge/bridge.c
@@ -23,6 +23,7 @@ int oneline;
 int show_stats;
 int show_details;
 int compress_vlans;
+int json_output;
 int timestamp;
 char *batch_file;
 int force;
@@ -38,7 +39,7 @@ static void usage(void)
 "where OBJECT := { link | fdb | mdb | vlan | monitor }\n"
 "  OPTIONS := { -V[ersion] | -s[tatistics] | -d[etails] |\n"
 "   -o[neline] | -t[imestamp] | -n[etns] name |\n"
-"   -c[ompressvlans] }\n");
+"   -c[ompressvlans] -j{son} }\n");
exit(-1);
 }
 
@@ -173,6 +174,8 @@ main(int argc, char **argv)
++compress_vlans;
} else if (matches(opt, "-force") == 0) {
++force;
+   } else if (matches(opt, "-json") == 0) {
+   ++json_output;
} else if (matches(opt, "-batch") == 0) {
argc--;
argv++;
diff --git a/bridge/vlan.c b/bridge/vlan.c
index 717025a..722d0af 100644
--- a/bridge/vlan.c
+++ b/bridge/vlan.c
@@ -7,6 +7,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 
 #include "libnetlink.h"
@@ -15,6 +16,8 @@
 
 static unsigned int filter_index, filter_vlan;
 
+json_writer_t *jw_global = NULL;
+
 static void usage(void)
 {
fprintf(stderr, "Usage: bridge vlan { add | del } vid VLAN_ID dev DEV [ 
pvid] [ untagged ]\n");
@@ -158,6 +161,19 @@ static int filter_vlan_check(struct bridge_vlan_info 
*vinfo)
return 1;
 }
 
+static int print_vlan_port(FILE *fp, int ifi_index)
+{
+   if (jw_global) {
+   jsonw_pretty(jw_global, 1);
+   jsonw_name(jw_global,
+  ll_index_to_name(ifi_index));
+   jsonw_start_array(jw_global);
+   } else {
+   fprintf(fp, "%s",
+   ll_index_to_name(ifi_index));
+   }
+}
+
 static int print_vlan(const struct sockaddr_nl *who,
  struct nlmsghdr *n,
  void *arg)
@@ -166,6 +182,7 @@ static int print_vlan(const struct sockaddr_nl *who,
struct ifinfomsg *ifm = NLMSG_DATA(n);
int len = n->nlmsg_len;
struct rtattr *tb[IFLA_MAX+1];
+   char flags[80];
 
if (n->nlmsg_type != RTM_NEWLINK) {
fprintf(stderr, "Not RTM_NEWLINK: %08x %08x %08x\n",
@@ -199,7 +216,8 @@ static int print_vlan(const struct sockaddr_nl *who,
__u16 last_vid_start = 0;
 
if (!filter_vlan)
-   fprintf(fp, "%s", ll_index_to_name(ifm->ifi_index));
+   print_vlan_port(fp, ifm->ifi_index);
+
for (i = RTA_DATA(list); RTA_OK(i, rem); i = RTA_NEXT(i, rem)) {
struct bridge_vlan_info *vinfo;
int vcheck_ret;
@@ -218,20 +236,49 @@ static int print_vlan(const struct sockaddr_nl *who,
continue;
 
if (filter_vlan)
-   fprintf(fp, "%s",
-   

[PATCH iproute2 net-next 5/5] bridge: update man page

[Roopa Prabhu]

From: Roopa Prabhu 

Signed-off-by: Roopa Prabhu 
---
 man/man8/bridge.8 | 9 +++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/man/man8/bridge.8 b/man/man8/bridge.8
index 08e8a5b..abaee63 100644
--- a/man/man8/bridge.8
+++ b/man/man8/bridge.8
@@ -20,8 +20,9 @@ bridge \- show / manipulate bridge addresses and devices
 .IR OPTIONS " := { "
 \fB\-V\fR[\fIersion\fR] |
 \fB\-s\fR[\fItatistics\fR] |
-\fB\-n\fR[\fIetns\fR] name }
-\fB\-b\fR[\fIatch\fR] filename }
+\fB\-n\fR[\fIetns\fR] name |
+\fB\-b\fR[\fIatch\fR] filename |
+\fB\-j\fR[\fIson\fR] }
 
 .ti -8
 .BR "bridge link set"
@@ -153,6 +154,10 @@ Don't terminate bridge command on errors in batch mode.
 If there were any errors during execution of the commands, the application
 return code will be non zero.
 
+.TP
+.BR "\-json"
+Display results in JSON format. Currently available for vlan and fdb.
+
 .SH BRIDGE - COMMAND SYNTAX
 
 .SS
-- 
1.9.1

[PATCH iproute2 net-next 3/5] bridge: add json support for bridge fdb show

[Roopa Prabhu]

From: Anuradha Karuppiah 

Sample output:
$bridge -j fdb show
[{
"mac": "44:38:39:00:69:88",
"dev": "swp2s0",
"vlan": 2,
"master": "br0",
"state": "permanent"
},{
"mac": "00:02:00:00:00:01",
"dev": "swp2s0",
"vlan": 2,
"master": "br0"
},{
"mac": "00:02:00:00:00:02",
"dev": "swp2s1",
"vlan": 2,
"master": "br0"
},{
"mac": "44:38:39:00:69:89",
"dev": "swp2s1",
"master": "br0",
"state": "permanent"
},{
"mac": "44:38:39:00:69:89",
"dev": "swp2s1",
"vlan": 2,
"master": "br0",
"state": "permanent"
},{
"mac": "44:38:39:00:69:88",
"dev": "br0",
"master": "br0",
"state": "permanent"
}
]

Signed-off-by: Anuradha Karuppiah 
Signed-off-by: Roopa Prabhu 
---
 bridge/fdb.c | 204 +++
 1 file changed, 162 insertions(+), 42 deletions(-)

diff --git a/bridge/fdb.c b/bridge/fdb.c
index be849f9..79e9cbb 100644
--- a/bridge/fdb.c
+++ b/bridge/fdb.c
@@ -21,6 +21,8 @@
 #include 
 #include 
 #include 
+#include 
+#include 
 
 #include "libnetlink.h"
 #include "br_common.h"
@@ -29,6 +31,8 @@
 
 static unsigned int filter_index, filter_vlan;
 
+json_writer_t *jw_global;
+
 static void usage(void)
 {
fprintf(stderr, "Usage: bridge fdb { add | append | del | replace } 
ADDR dev DEV\n"
@@ -59,6 +63,15 @@ static const char *state_n2a(unsigned int s)
return buf;
 }
 
+static void start_json_fdb_flags_array(bool *fdb_flags)
+{
+   if (*fdb_flags)
+   return;
+   jsonw_name(jw_global, "flags");
+   jsonw_start_array(jw_global);
+   *fdb_flags = true;
+}
+
 int print_fdb(const struct sockaddr_nl *who, struct nlmsghdr *n, void *arg)
 {
FILE *fp = arg;
@@ -66,11 +79,12 @@ int print_fdb(const struct sockaddr_nl *who, struct 
nlmsghdr *n, void *arg)
int len = n->nlmsg_len;
struct rtattr *tb[NDA_MAX+1];
__u16 vid = 0;
+   bool fdb_flags = false;
+   const char *state_s;
 
if (n->nlmsg_type != RTM_NEWNEIGH && n->nlmsg_type != RTM_DELNEIGH) {
fprintf(stderr, "Not RTM_NEWNEIGH: %08x %08x %08x\n",
n->nlmsg_len, n->nlmsg_type, n->nlmsg_flags);
-
return 0;
}
 
@@ -86,6 +100,11 @@ int print_fdb(const struct sockaddr_nl *who, struct 
nlmsghdr *n, void *arg)
if (filter_index && filter_index != r->ndm_ifindex)
return 0;
 
+   if (jw_global) {
+   jsonw_pretty(jw_global, 1);
+   jsonw_start_object(jw_global);
+   }
+
parse_rtattr(tb, NDA_MAX, NDA_RTA(r),
 n->nlmsg_len - NLMSG_LENGTH(sizeof(*r)));
 
@@ -96,39 +115,73 @@ int print_fdb(const struct sockaddr_nl *who, struct 
nlmsghdr *n, void *arg)
return 0;
 
if (n->nlmsg_type == RTM_DELNEIGH)
-   fprintf(fp, "Deleted ");
+   if (jw_global)
+   jsonw_string_field(jw_global, "opCode", "deleted");
+   else
+   fprintf(fp, "Deleted ");
 
if (tb[NDA_LLADDR]) {
SPRINT_BUF(b1);
-   fprintf(fp, "%s ",
-   ll_addr_n2a(RTA_DATA(tb[NDA_LLADDR]),
-   RTA_PAYLOAD(tb[NDA_LLADDR]),
-   ll_index_to_type(r->ndm_ifindex),
-   b1, sizeof(b1)));
+   ll_addr_n2a(RTA_DATA(tb[NDA_LLADDR]),
+   RTA_PAYLOAD(tb[NDA_LLADDR]),
+   ll_index_to_type(r->ndm_ifindex),
+   b1, sizeof(b1));
+   if (jw_global)
+   jsonw_string_field(jw_global, "mac", b1);
+   else
+   fprintf(fp, "%s ", b1);
}
 
-   if (!filter_index && r->ndm_ifindex)
-   fprintf(fp, "dev %s ", ll_index_to_name(r->ndm_ifindex));
+   if (!filter_index && r->ndm_ifindex) {
+   if (jw_global)
+   jsonw_string_field(jw_global, "dev",
+  ll_index_to_name(r->ndm_ifindex));
+   else
+   fprintf(fp, "dev %s ",
+   ll_index_to_name(r->ndm_ifindex));
+   }
 
if (tb[NDA_DST]) {
int family = AF_INET;
+   const char *abuf_s;
 
if (RTA_PAYLOAD(tb[NDA_DST]) == sizeof(struct in6_addr))
family = AF_INET6;
 
-   fprintf(fp, "dst %s ",
-   format_host(family,
-   RTA_PAYLOAD(tb[NDA_DST]),
-   RTA_DATA(tb[NDA_DST])));
+   abuf_s = format_host(family,

[PATCH iproute2 net-next 4/5] bridge: add json schema for bridge fdb show

[Roopa Prabhu]

From: Anuradha Karuppiah 

we think storing the schema file for the json
format will be useful.

Signed-off-by: Anuradha Karuppiah 
---
 schema/bridge_fdb_schema.json | 62 +++
 1 file changed, 62 insertions(+)
 create mode 100644 schema/bridge_fdb_schema.json

diff --git a/schema/bridge_fdb_schema.json b/schema/bridge_fdb_schema.json
new file mode 100644
index 000..3e5be8d
--- /dev/null
+++ b/schema/bridge_fdb_schema.json
@@ -0,0 +1,62 @@
+{
+"$schema": "http://json-schema.org/draft-04/schema#;,
+"description": "bridge fdb show",
+"type": "array",
+"items": {
+"type": "object",
+"properties": {
+"dev": {
+"type": "string"
+},
+"dst": {
+"description" : "host name or ip address",
+"type": "string"
+},
+"flags": {
+"type": "array",
+"items": {
+"enum": ["self", "master", "router", "offload"]
+},
+"uniqueItems": true
+},
+"linkNetNsId": {
+"type": "integer"
+},
+"mac": {
+"type": "string"
+},
+"master": {
+"type": "string"
+},
+"opCode": {
+"description" : "used to indicate fdb entry del",
+"enum": ["deleted"]
+},
+"port": {
+"type": "integer"
+},
+"state": {
+"description" : "permanent, static, stale, state=#x",
+"type": "string"
+},
+"updated": {
+"type": "integer"
+},
+"used": {
+"type": "integer"
+},
+"viaIf": {
+"type": "string"
+},
+"viaIfIndex": {
+"type": "integer"
+},
+"vlan": {
+"type": "integer"
+},
+"vni": {
+"type": "integer"
+}
+}
+}
+}
-- 
1.9.1

[PATCH iproute2 net-next 1/5] json_writer: allow base json data type to be array or object

[Roopa Prabhu]

From: Anuradha Karuppiah 

This patch adds a type qualifier to json_writer. Type can be a
json object or array. This can be extended to other types like
json-string, json-number etc in the future.

Signed-off-by: Anuradha Karuppiah 
---
 include/json_writer.h |  5 +++--
 lib/json_writer.c | 39 +++
 misc/ifstat.c |  6 +++---
 misc/lnstat.c |  2 +-
 misc/nstat.c  |  4 ++--
 5 files changed, 44 insertions(+), 12 deletions(-)

diff --git a/include/json_writer.h b/include/json_writer.h
index ab9a008..e04a40a 100644
--- a/include/json_writer.h
+++ b/include/json_writer.h
@@ -21,8 +21,9 @@
 /* Opaque class structure */
 typedef struct json_writer json_writer_t;
 
-/* Create a new JSON stream */
-json_writer_t *jsonw_new(FILE *f);
+/* Create a new JSON stream with data type */
+json_writer_t *jsonw_new_object(FILE *f);
+json_writer_t *jsonw_new_array(FILE *f);
 /* End output to JSON stream */
 void jsonw_destroy(json_writer_t **self_p);
 
diff --git a/lib/json_writer.c b/lib/json_writer.c
index 2af16e1..420cd87 100644
--- a/lib/json_writer.c
+++ b/lib/json_writer.c
@@ -22,11 +22,17 @@
 
 #include "json_writer.h"
 
+enum jsonw_data_type {
+   JSONW_TYPE_OBJECT,
+   JSONW_TYPE_ARRAY
+};
+
 struct json_writer {
FILE*out;   /* output file */
unsigneddepth;  /* nesting */
boolpretty; /* optional whitepace */
charsep;/* either nul or comma */
+   int type;   /* currently either object or array */
 };
 
 /* indentation for pretty print */
@@ -94,7 +100,7 @@ static void jsonw_puts(json_writer_t *self, const char *str)
 }
 
 /* Create a new JSON stream */
-json_writer_t *jsonw_new(FILE *f)
+static json_writer_t *jsonw_new(FILE *f, int type)
 {
json_writer_t *self = malloc(sizeof(*self));
if (self) {
@@ -102,11 +108,29 @@ json_writer_t *jsonw_new(FILE *f)
self->depth = 0;
self->pretty = false;
self->sep = '\0';
-   putc('{', self->out);
+   self->type = type;
+   switch (self->type) {
+   case JSONW_TYPE_OBJECT:
+   putc('{', self->out);
+   break;
+   case JSONW_TYPE_ARRAY:
+   putc('[', self->out);
+   break;
+   }
}
return self;
 }
 
+json_writer_t *jsonw_new_object(FILE *f)
+{
+   return jsonw_new(f, JSONW_TYPE_OBJECT);
+}
+
+json_writer_t *jsonw_new_array(FILE *f)
+{
+   return jsonw_new(f, JSONW_TYPE_ARRAY);
+}
+
 /* End output to JSON stream */
 void jsonw_destroy(json_writer_t **self_p)
 {
@@ -114,7 +138,14 @@ void jsonw_destroy(json_writer_t **self_p)
 
assert(self->depth == 0);
jsonw_eol(self);
-   fputs("}\n", self->out);
+   switch (self->type) {
+   case JSONW_TYPE_OBJECT:
+   fputs("}\n", self->out);
+   break;
+   case JSONW_TYPE_ARRAY:
+   fputs("]\n", self->out);
+   break;
+   }
fflush(self->out);
free(self);
*self_p = NULL;
@@ -267,7 +298,7 @@ void jsonw_null_field(json_writer_t *self, const char *prop)
 #ifdef TEST
 int main(int argc, char **argv)
 {
-   json_writer_t *wr = jsonw_new(stdout);
+   json_writer_t *wr = jsonw_new_object(stdout);
 
jsonw_pretty(wr, true);
jsonw_name(wr, "Vyatta");
diff --git a/misc/ifstat.c b/misc/ifstat.c
index abbb4e7..29aa63c 100644
--- a/misc/ifstat.c
+++ b/misc/ifstat.c
@@ -240,7 +240,7 @@ static void load_raw_table(FILE *fp)
 
 static void dump_raw_db(FILE *fp, int to_hist)
 {
-   json_writer_t *jw = json_output ? jsonw_new(fp) : NULL;
+   json_writer_t *jw = json_output ? jsonw_new_object(fp) : NULL;
struct ifstat_ent *n, *h;
 
h = hist_db;
@@ -447,7 +447,7 @@ static void print_one_if(FILE *fp, const struct ifstat_ent 
*n,
 
 static void dump_kern_db(FILE *fp)
 {
-   json_writer_t *jw = json_output ? jsonw_new(fp) : NULL;
+   json_writer_t *jw = json_output ? jsonw_new_object(fp) : NULL;
struct ifstat_ent *n;
 
if (jw) {
@@ -473,7 +473,7 @@ static void dump_kern_db(FILE *fp)
 static void dump_incr_db(FILE *fp)
 {
struct ifstat_ent *n, *h;
-   json_writer_t *jw = json_output ? jsonw_new(fp) : NULL;
+   json_writer_t *jw = json_output ? jsonw_new_object(fp) : NULL;
 
h = hist_db;
if (jw) {
diff --git a/misc/lnstat.c b/misc/lnstat.c
index 659a01b..2988e9e 100644
--- a/misc/lnstat.c
+++ b/misc/lnstat.c
@@ -110,7 +110,7 @@ static void print_line(FILE *of, const struct lnstat_file 
*lnstat_files,
 static void print_json(FILE *of, const struct lnstat_file *lnstat_files,
   const struct field_params *fp)
 {
-   json_writer_t *jw = jsonw_new(of);
+   json_writer_t *jw = 

Confidential And Very Urgent Attention.

[Mr Kamal Ali Mohamed]

Dear friend,

My name is Mr Kamal Ali Mohamed . I am working with one of the prime banks here 
in Burkina Faso. Here in this bank existed a dormant account for many years, 
which belong to one of our late foreign customer.

When I discovered that there had been neither deposits nor withdrawals from 
this account for this long period, I decided to carry out a system 
investigation and discovered that none of the family member or relations of the 
late person are aware of this account, which means nobody will come to take it. 
The amount in this account stands at $8, 500, 000.00 (Eight Million Five 
Hundred Thousand USA Dollars).

I want a foreign account where the bank will transfer this fund. I will front 
you in the bank as the Next of Kin to the late customer and back you up with 
relevant information. What the bank need is proof and information about the 
late customer which I will assist you on. This is a genuine, risk free and 
legal business transaction.

All details shall be sent to you once I hear from you. The information as 
contained herein be accorded the necessary attention, urgency as well as the 
secrecy it deserves. If you are really sure of your integrity, trustworthy and 
confidentiality, Sorry if you received this letter in your spam, Due to recent 
connection error here in my country. Looking forward for your immediate 
response to me.Through my private e-mail id: (kamalalimoha...@terra.com)

Best Regards,
Mr. Kamal Ali Mohamed.

Re: [PATCH] remove lots of IS_ERR_VALUE abuses

[Linus Torvalds]

On Fri, May 27, 2016 at 2:23 PM, Arnd Bergmann  wrote:
>
> This patch changes all users of IS_ERR_VALUE() that I could find
> on 32-bit ARM randconfig builds and x86 allmodconfig. For the
> moment, this doesn't change the definition of IS_ERR_VALUE()
> because there are probably still architecture specific users
> elsewhere.

Patch applied with the fixups from Al Viro edited in.

I also ended up removing a few other users (due to the vm_brk()
interface), and then made IS_ERR_VALUE() do the "void *" cast so that
integer use of a non-pointer size should now complain.

It works for me and has no new warnings in my allmodconfig build, and
with your ARM work that is presumably clean too. But other
architectures may see new warnings.

People who got affected by this should check their subsystem code for
the changes.

  Linus

Re: BUG: net/netlink: KASAN: use-after-free in netlink_sock_destruct

[Herbert Xu]

On Fri, May 27, 2016 at 09:19:48AM -0700, Cong Wang wrote:
>
> This one looks different though, this time the bug is
> triggered in netlink_sock_destruct(), where all the sock
> ref should be gone, which means it is impossible to refer
> nlk->cb anywhere else. Hmm... I have no idea how
> could this happen.

netlink_sock_destruct is one of the two exit paths for cb->skb
so this is consistent with the previous trace.

Cheers,
-- 
Email: Herbert Xu 
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [PATCH] remove lots of IS_ERR_VALUE abuses

[Al Viro]

On Fri, May 27, 2016 at 11:23:25PM +0200, Arnd Bergmann wrote:

> @@ -837,7 +837,7 @@ static int load_flat_shared_library(int id, struct 
> lib_info *libs)
>  
>   res = prepare_binprm();
>  
> - if (!IS_ERR_VALUE(res))
> + if (res >= 0)

if (res == 0), please - prepare_binprm() returns 0 or -E...

> --- a/net/9p/client.c
> +++ b/net/9p/client.c
> @@ -521,7 +521,7 @@ static int p9_check_errors(struct p9_client *c, struct 
> p9_req_t *req)
>   if (p9_is_proto_dotu(c))
>   err = -ecode;
>  
> - if (!err || !IS_ERR_VALUE(err)) {
> + if (!err || !IS_ERR_VALUE((unsigned long)err)) {

Not really - it's actually
if (p9_is_proto_dotu(c) && ecode < 512)
err = -ecode;

if (!err) {
...
> @@ -608,7 +608,7 @@ static int p9_check_zc_errors(struct p9_client *c, struct 
> p9_req_t *req,
>   if (p9_is_proto_dotu(c))
>   err = -ecode;
>  
> - if (!err || !IS_ERR_VALUE(err)) {
> + if (!err || !IS_ERR_VALUE((unsigned long)err)) {

Ditto.

Re: [PATCH] remove lots of IS_ERR_VALUE abuses

[Srinivas Kandagatla]

On 27/05/16 22:23, Arnd Bergmann wrote:

  drivers/acpi/acpi_dbg.c  | 22 +++---
  drivers/ata/sata_highbank.c  |  2 +-
  drivers/clk/tegra/clk-tegra210.c |  2 +-
  drivers/cpufreq/omap-cpufreq.c   |  2 +-
  drivers/crypto/caam/ctrl.c   |  2 +-
  drivers/dma/sun4i-dma.c  | 16 
  drivers/gpio/gpio-xlp.c  |  2 +-
  drivers/gpu/drm/sti/sti_vtg.c|  4 ++--
  drivers/gpu/drm/tilcdc/tilcdc_tfp410.c   |  2 +-
  drivers/gpu/host1x/hw/intr_hw.c  |  2 +-
  drivers/iommu/arm-smmu-v3.c  | 18 +-

...

  drivers/mmc/host/sdhci-of-at91.c |  2 +-
  drivers/mmc/host/sdhci.c |  4 ++--
  drivers/net/ethernet/freescale/fman/fman.c   |  2 +-
  drivers/net/ethernet/freescale/fman/fman_muram.c |  4 ++--
  drivers/net/ethernet/freescale/fman/fman_muram.h |  4 ++--
  drivers/net/wireless/ti/wlcore/spi.c |  4 ++--
  drivers/nvmem/core.c | 22 +++---


For nvmem part,

Acked-by: Srinivas Kandagatla 


  drivers/tty/serial/amba-pl011.c  |  2 +-
  drivers/tty/serial/sprd_serial.c |  2 +-
  drivers/video/fbdev/da8xx-fb.c   |  4 ++--
  fs/afs/write.c   |  4 
  fs/binfmt_flat.c |  6 +++---
  fs/gfs2/dir.c| 15 +--
  kernel/pid.c |  2 +-
  net/9p/client.c  |  4 ++--
  sound/soc/qcom/lpass-platform.c  |  4 ++--

There is already a patch for this in Mark Brown's sound tree,

https://git.kernel.org/cgit/linux/kernel/git/broonie/sound.git/commit/?h=for-linus=cef794f76485f4a4e6affd851ae9f9d0eb4287a5

Thanks,
srini

  38 files changed, 100 insertions(+), 101 deletions(-)

Re: [PATCH] remove lots of IS_ERR_VALUE abuses

[Linus Torvalds]

On Fri, May 27, 2016 at 2:46 PM, Andrew Morton
 wrote:
>
> So you do plan to add some sort of typechecking into IS_ERR_VALUE()?

The easiest way to do it is to just turn the (x) into (unsigned
long)(void *)(x), which then complains about casting an integer to a
pointer if the integer has the wrong size.

But if we get rid of the bogus cases, there's just a few left, and we
should probably just rename the whole thing (the initial double
underscore). It really isn't something normal people should use.

  Linus

Re: [PATCH] remove lots of IS_ERR_VALUE abuses

[Andrew Morton]

On Fri, 27 May 2016 23:23:25 +0200 Arnd Bergmann  wrote:

> Most users of IS_ERR_VALUE() in the kernel are wrong, as they
> pass an 'int' into a function that takes an 'unsigned long'
> argument. This happens to work because the type is sign-extended
> on 64-bit architectures before it gets converted into an
> unsigned type.
> 
> However, anything that passes an 'unsigned short' or 'unsigned int'
> argument into IS_ERR_VALUE() is guaranteed to be broken, as are
> 8-bit integers and types that are wider than 'unsigned long'.
> 
> Andrzej Hajda has already fixed a lot of the worst abusers that
> were causing actual bugs, but it would be nice to prevent any
> users that are not passing 'unsigned long' arguments.
> 
> This patch changes all users of IS_ERR_VALUE() that I could find
> on 32-bit ARM randconfig builds and x86 allmodconfig. For the
> moment, this doesn't change the definition of IS_ERR_VALUE()
> because there are probably still architecture specific users
> elsewhere.

So you do plan to add some sort of typechecking into IS_ERR_VALUE()?

> Almost all the warnings I got are for files that are better off
> using 'if (err)' or 'if (err < 0)'.
> The only legitimate user I could find that we get a warning for
> is the (32-bit only) freescale fman driver, so I did not remove
> the IS_ERR_VALUE() there but changed the type to 'unsigned long'.
> For 9pfs, I just worked around one user whose calling conventions
> are so obscure that I did not dare change the behavior.
> 
> I was using this definition for testing:
> 
>  #define IS_ERR_VALUE(x) ((unsigned long*)NULL == (typeof (x)*)NULL && \
>unlikely((unsigned long long)(x) >= (unsigned long 
> long)(typeof(x))-MAX_ERRNO))
> 
> which ends up making all 16-bit or wider types work correctly with
> the most plausible interpretation of what IS_ERR_VALUE() was supposed
> to return according to its users, but also causes a compile-time
> warning for any users that do not pass an 'unsigned long' argument.
> 
> I suggested this approach earlier this year, but back then we ended
> up deciding to just fix the users that are obviously broken. After
> the initial warning that caused me to get involved in the discussion
> (fs/gfs2/dir.c) showed up again in the mainline kernel, Linus
> asked me to send the whole thing again.

[PATCH] remove lots of IS_ERR_VALUE abuses

[Arnd Bergmann]

Most users of IS_ERR_VALUE() in the kernel are wrong, as they
pass an 'int' into a function that takes an 'unsigned long'
argument. This happens to work because the type is sign-extended
on 64-bit architectures before it gets converted into an
unsigned type.

However, anything that passes an 'unsigned short' or 'unsigned int'
argument into IS_ERR_VALUE() is guaranteed to be broken, as are
8-bit integers and types that are wider than 'unsigned long'.

Andrzej Hajda has already fixed a lot of the worst abusers that
were causing actual bugs, but it would be nice to prevent any
users that are not passing 'unsigned long' arguments.

This patch changes all users of IS_ERR_VALUE() that I could find
on 32-bit ARM randconfig builds and x86 allmodconfig. For the
moment, this doesn't change the definition of IS_ERR_VALUE()
because there are probably still architecture specific users
elsewhere.

Almost all the warnings I got are for files that are better off
using 'if (err)' or 'if (err < 0)'.
The only legitimate user I could find that we get a warning for
is the (32-bit only) freescale fman driver, so I did not remove
the IS_ERR_VALUE() there but changed the type to 'unsigned long'.
For 9pfs, I just worked around one user whose calling conventions
are so obscure that I did not dare change the behavior.

I was using this definition for testing:

 #define IS_ERR_VALUE(x) ((unsigned long*)NULL == (typeof (x)*)NULL && \
   unlikely((unsigned long long)(x) >= (unsigned long 
long)(typeof(x))-MAX_ERRNO))

which ends up making all 16-bit or wider types work correctly with
the most plausible interpretation of what IS_ERR_VALUE() was supposed
to return according to its users, but also causes a compile-time
warning for any users that do not pass an 'unsigned long' argument.

I suggested this approach earlier this year, but back then we ended
up deciding to just fix the users that are obviously broken. After
the initial warning that caused me to get involved in the discussion
(fs/gfs2/dir.c) showed up again in the mainline kernel, Linus
asked me to send the whole thing again.

Signed-off-by: Arnd Bergmann 
Cc: Andrzej Hajda 
Cc: Andrew Morton 
Link: https://lkml.org/lkml/2016/1/7/363
Link: https://lkml.org/lkml/2016/5/27/486
---
 drivers/acpi/acpi_dbg.c  | 22 +++---
 drivers/ata/sata_highbank.c  |  2 +-
 drivers/clk/tegra/clk-tegra210.c |  2 +-
 drivers/cpufreq/omap-cpufreq.c   |  2 +-
 drivers/crypto/caam/ctrl.c   |  2 +-
 drivers/dma/sun4i-dma.c  | 16 
 drivers/gpio/gpio-xlp.c  |  2 +-
 drivers/gpu/drm/sti/sti_vtg.c|  4 ++--
 drivers/gpu/drm/tilcdc/tilcdc_tfp410.c   |  2 +-
 drivers/gpu/host1x/hw/intr_hw.c  |  2 +-
 drivers/iommu/arm-smmu-v3.c  | 18 +-
 drivers/iommu/arm-smmu.c |  8 
 drivers/irqchip/irq-clps711x.c   |  2 +-
 drivers/irqchip/irq-gic.c|  2 +-
 drivers/irqchip/irq-hip04.c  |  2 +-
 drivers/irqchip/spear-shirq.c|  2 +-
 drivers/media/i2c/adp1653.c  | 10 +-
 drivers/media/platform/s5p-tv/mixer_drv.c|  2 +-
 drivers/mfd/twl4030-irq.c|  2 +-
 drivers/mmc/core/mmc.c   |  4 ++--
 drivers/mmc/host/dw_mmc.c|  6 +++---
 drivers/mmc/host/sdhci-esdhc-imx.c   |  2 +-
 drivers/mmc/host/sdhci-of-at91.c |  2 +-
 drivers/mmc/host/sdhci.c |  4 ++--
 drivers/net/ethernet/freescale/fman/fman.c   |  2 +-
 drivers/net/ethernet/freescale/fman/fman_muram.c |  4 ++--
 drivers/net/ethernet/freescale/fman/fman_muram.h |  4 ++--
 drivers/net/wireless/ti/wlcore/spi.c |  4 ++--
 drivers/nvmem/core.c | 22 +++---
 drivers/tty/serial/amba-pl011.c  |  2 +-
 drivers/tty/serial/sprd_serial.c |  2 +-
 drivers/video/fbdev/da8xx-fb.c   |  4 ++--
 fs/afs/write.c   |  4 
 fs/binfmt_flat.c |  6 +++---
 fs/gfs2/dir.c| 15 +--
 kernel/pid.c |  2 +-
 net/9p/client.c  |  4 ++--
 sound/soc/qcom/lpass-platform.c  |  4 ++--
 38 files changed, 100 insertions(+), 101 deletions(-)

diff --git a/drivers/acpi/acpi_dbg.c b/drivers/acpi/acpi_dbg.c
index 15e4604efba7..1f4128487dd4 100644
--- a/drivers/acpi/acpi_dbg.c
+++ b/drivers/acpi/acpi_dbg.c
@@ -265,7 +265,7 @@ static int acpi_aml_write_kern(const char *buf, int len)
char *p;
 
ret = acpi_aml_lock_write(crc, 

Re: [RFC PATCH 14/16] net: dsa: Add new binding implementation

[Florian Fainelli]

On 05/27/2016 01:57 PM, Andrew Lunn wrote:
> On Fri, May 27, 2016 at 04:39:05PM -0400, Vivien Didelot wrote:
>>
>> Hi Andrew, Florian,
>>
>> Here again, I'd suggested an implicit namespace for functions taking a
>> dsa_switch_tree structure as first argument, i.e. dsa_tree_do_foo().
> 
> Using tree actually makes things worse, since tree is never used
> anywhere in the current code. It is always called dst. If you do this,
> you also need to replace every instance of dst with tree.
> 
> We mostly have a good convention
> 
> struct dsa_switch *ds;
> dsa_switch_tree *dst;
> 
> What is not quite consistent is
> 
> struct dsa_chip_data *cd
> 
> which should really be
> 
> struct dsa_chip_data *dcd
> 
> but we are consistent with using cd everywhere.
>  
>> Since we are likely to spend some time in net/dsa, it'd be great to
>> introduce the new bindings and an intuitive API at the same time ;-)
> 
> They are two separate things. And the binding will be set in stone,
> never to be changed again in incompatible ways, where as the API we
> can change as much as we like. We should be concentrate on the
> binding.

I completely agree, Andrew has been working long enough on this it needs
to be posted and merged, anything that becomes a clean up on top of that
can be done at any random time, by you, me, or any kernel janitor who
feels like it.

We need to set priorities here, and the highest priority is to get these
patches accepted to enable more people to utilize DSA, so once we have
more devices we can get to a longer term plan to get a better
abstraction model for the switch devices that are in the source tree.

Considering the fast pace nature of the net-next tree, I am sure that
any kind of cleanup on this code after the patches get merged would be
merged in a matter of weeks, but it does not strike me as being a
blocker here.

Thanks
-- 
Florian

Re: [RFC PATCH 01/16] dsa: slave: chip data is optional, don't dereference NULL

[Florian Fainelli]

On 05/27/2016 11:45 AM, Vivien Didelot wrote:
> Hi Andrew, Florian,
> 
> We are inconsistent on commit titles and messages. Some of them use
> "net: " prefix, some others don't, sometimes lower or upper case titles.
> 
> I'd suggest we stick with the "net: dsa: " prefix and lowercase titles.
> 
> When possible, let's try to respect the 50/72 Git rule.
> 
> So we'd have e.g.:
> 
> net: dsa: add new bindings
> net: dsa: mv88e6xxx: add support for foo
> net: dsa: sf2: remove bar
> ...
> 
> The networking documentation doesn't seem to have opinion on the "net: "
> prefix. We might drop it and keep only "dsa: " for core and drivers.
> 
> What do you think?

My preference goes for "net: dsa: ", but at the same time, I don't
really care as long as I can git log the file.
-- 
Florian

Re: [RFC PATCH 14/16] net: dsa: Add new binding implementation

[Andrew Lunn]

On Fri, May 27, 2016 at 04:39:05PM -0400, Vivien Didelot wrote:
> 
> Hi Andrew, Florian,
> 
> Here again, I'd suggested an implicit namespace for functions taking a
> dsa_switch_tree structure as first argument, i.e. dsa_tree_do_foo().

Using tree actually makes things worse, since tree is never used
anywhere in the current code. It is always called dst. If you do this,
you also need to replace every instance of dst with tree.

We mostly have a good convention

struct dsa_switch *ds;
dsa_switch_tree *dst;

What is not quite consistent is

struct dsa_chip_data *cd

which should really be

struct dsa_chip_data *dcd

but we are consistent with using cd everywhere.
 
> Since we are likely to spend some time in net/dsa, it'd be great to
> introduce the new bindings and an intuitive API at the same time ;-)

They are two separate things. And the binding will be set in stone,
never to be changed again in incompatible ways, where as the API we
can change as much as we like. We should be concentrate on the
binding.

Andrew

Re: [RFC PATCH 14/16] net: dsa: Add new binding implementation

[Vivien Didelot]

Hi Andrew, Florian,

Here again, I'd suggested an implicit namespace for functions taking a
dsa_switch_tree structure as first argument, i.e. dsa_tree_do_foo().

Since we are likely to spend some time in net/dsa, it'd be great to
introduce the new bindings and an intuitive API at the same time ;-)

Examples below.

> +void dsa_unregister_switch(struct dsa_switch *ds);
> +int dsa_register_switch(struct dsa_switch *ds, struct device_node *np);

void dsa_switch_unregister(struct dsa_switch *ds);
int dsa_switch_register(struct dsa_switch *ds, struct device_node *np);

> +static struct dsa_switch_tree *dsa_get_dst(u32 tree)
> +static void dsa_free_dst(struct kref *ref)

static struct dsa_switch_tree *dsa_get_tree(u32 tree)
dsa_free_tree()

> +static void dsa_put_dst(struct dsa_switch_tree *dst)

static void dsa_tree_put(struct dsa_switch_tree *dst)

> +static struct dsa_switch_tree *dsa_add_dst(u32 tree)

dsa_add_tree(u32 tree)

> +static void dsa_dst_add_ds(struct dsa_switch_tree *dst,
> +struct dsa_switch *ds, u32 index)

static void dsa_tree_add_switch(struct dsa_switch_tree *dst, u32 index,
struct dsa_switch *ds)

> +static void dsa_dst_del_ds(struct dsa_switch_tree *dst,
> +struct dsa_switch *ds, u32 index)

static void dsa_tree_del_switch(struct dsa_switch_tree *dst, u32 index)

> +static bool dsa_port_is_dsa(struct device_node *port)
> +static bool dsa_port_is_cpu(struct device_node *port)

It would be consistent to have public helpers like:

bool dsa_port_is_cpu(struct dsa_port *dp);
bool dsa_port_is_dsa(struct dsa_port *dp);

> +static bool dsa_ds_find_port(struct dsa_switch *ds,
> +  struct device_node *port)

dsa_switch_find_port_node()

> +static struct dsa_switch *dsa_dst_find_port(struct dsa_switch_tree *dst,
> + struct device_node *port)

dsa_tree_find_port_node()

> +static int dsa_port_complete(struct dsa_switch_tree *dst,
> +  struct dsa_switch *src_ds,
> +  struct device_node *port,
> +  u32 src_port)

> +static int dsa_ds_complete(struct dsa_switch_tree *dst, struct dsa_switch 
> *ds)

> +static int dsa_dst_complete(struct dsa_switch_tree *dst)

static int dsa_tree_complete_port(struct dsa_switch_tree *dst,
  struct dsa_port *dp)

static int dsa_tree_complete_switch(struct dsa_switch_tree *dst,
struct dsa_switch *ds)

static int dsa_tree_complete(struct dsa_switch_tree *dst)

> +static int dsa_dsa_port_apply(struct device_node *port, u32 index,
> +   struct dsa_switch *ds)
> +static void dsa_dsa_port_unapply(struct device_node *port, u32 index,
> +  struct dsa_switch *ds)
> +static int dsa_cpu_port_apply(struct device_node *port, u32 index,
> +   struct dsa_switch *ds)
> +static void dsa_cpu_port_unapply(struct device_node *port, u32 index,
> +  struct dsa_switch *ds)
> +static int dsa_user_port_apply(struct device_node *port, u32 index,
> +struct dsa_switch *ds)
> +static void dsa_user_port_unapply(struct device_node *port, u32 index,
> +   struct dsa_switch *ds)

dsa_switch_{un,}apply_{cpu,dsa,user}_port(struct dsa_switch *ds,
  struct dsa_port *dp)

> +static int dsa_ds_apply(struct dsa_switch_tree *dst, struct dsa_switch *ds)
> +static void dsa_ds_unapply(struct dsa_switch_tree *dst, struct dsa_switch 
> *ds)

dsa_tree_{un,}apply_switch(dst, ds)

> +static int dsa_dst_apply(struct dsa_switch_tree *dst)
> +static void dsa_dst_unapply(struct dsa_switch_tree *dst)

dsa_tree_{un,}apply(dst)

> +static int dsa_cpu_parse(struct device_node *port, u32 index,
> +  struct dsa_switch_tree *dst,
> +  struct dsa_switch *ds)
> +static int dsa_ds_parse(struct dsa_switch_tree *dst, struct dsa_switch *ds)
> +static int dsa_dst_parse(struct dsa_switch_tree *dst)

dsa_tree_parse(dst)
dsa_tree_parse_switch(dst, ds)
dsa_tree_parse_cpu_port(dst, dp)

> +static int dsa_parse_ports_dn(struct device_node *ports, struct dsa_switch 
> *ds)

dsa_switch_parse_port_nodes(struct dsa_switch *ds, struct device_node *ports)

> +static struct device_node *dsa_get_ports(struct dsa_switch *ds,
> +  struct device_node *np)

dsa_switch_get_port_nodes(struct dsa_switch *ds, struct device_node *np)

> +static int _dsa_register_switch(struct dsa_switch *ds, struct device_node 
> *np)


 
I think it would be great to introduce the following in
include/net/dsa.h:

enum dsa_port_type {
DSA_PORT_TYPE_CPU,
DSA_PORT_TYPE_DSA,
DSA_PORT_TYPE_USER,
};

struct dsa_port {

[PATCH net v2 1/1] net: fec: update dirty_tx even if no skb

[Troy Kisky]

If dirty_tx isn't updated, then dma_unmap_single
can be called twice.

This fixes a
[   58.420980] [ cut here ]
[   58.425667] WARNING: CPU: 0 PID: 377 at 
/home/schurig/d/mkarm/linux-4.5/lib/dma-debug.c:1096 check_unmap+0x9d0/0xab8()
[   58.436405] fec 2188000.ethernet: DMA-API: device driver tries to free DMA 
memory it has not allocated [device address=0x] [size=66 bytes]

encountered by Holger

Signed-off-by: Troy Kisky 
Tested-by: 
Acked-by: Fugang Duan 

---
v2: add ack
---
 drivers/net/ethernet/freescale/fec_main.c | 8 +++-
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/freescale/fec_main.c 
b/drivers/net/ethernet/freescale/fec_main.c
index ca2..3c0255e 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -1197,10 +1197,8 @@ fec_enet_tx_queue(struct net_device *ndev, u16 queue_id)
 fec16_to_cpu(bdp->cbd_datlen),
 DMA_TO_DEVICE);
bdp->cbd_bufaddr = cpu_to_fec32(0);
-   if (!skb) {
-   bdp = fec_enet_get_nextdesc(bdp, >bd);
-   continue;
-   }
+   if (!skb)
+   goto skb_done;
 
/* Check for errors. */
if (status & (BD_ENET_TX_HB | BD_ENET_TX_LC |
@@ -1239,7 +1237,7 @@ fec_enet_tx_queue(struct net_device *ndev, u16 queue_id)
 
/* Free the sk buffer associated with this last transmit */
dev_kfree_skb_any(skb);
-
+skb_done:
/* Make sure the update to bdp and tx_skbuff are performed
 * before dirty_tx
 */
-- 
2.5.0

Re: [RFC PATCH 09/16] dsa: dsa: Split up creating/destroying of DSA and CPU ports

[Andrew Lunn]

> dsa_cpu_dsa_setups is not really meaningful and doesn't add much
> value.

I would disagree.  Quoting Documentation/CodingStyle

Chapter 6: Functions

Functions should be short and sweet, and do just one thing.  They should
fit on one or two screenfuls of text (the ISO/ANSI screen size is 80x24,
as we all know), and do one thing and do that well.

dsa_cpu_dsa_setups iterates the ports and find the dsa and cpu ports
that need setting up.

dsa_cpu_dsa_setup() does the actual setup.

These are completely different things, and so should be in different
functions.

Andrew

Re: [RFC PATCH 11/16] net: dsa: Refactor selection of tag ops into a function

[Andrew Lunn]

On Fri, May 27, 2016 at 03:35:57PM -0400, Vivien Didelot wrote:
> Hi Andrew,
> 
> Andrew Lunn  writes:
> 
> > @@ -26,6 +26,7 @@ enum dsa_tag_protocol {
> > DSA_TAG_PROTO_TRAILER,
> > DSA_TAG_PROTO_EDSA,
> > DSA_TAG_PROTO_BRCM,
> > +   _DSA_TAG_LAST,
> >  };
> 
> I would avoid _ prefixed functions or symbols, we've seen in mv88e6xxx
> that it doesn't make the code really readable.

The point is to make is special, so that people look at it, wonder why
it is special, and don't make the stupid mistakes of adding a new
protocol after it.

However, if you don't like the _, we can just add a comment
/* MUST BE LAST */

> I don't see the need to add a dsa_device_ops array. I'd keep the switch
> case on tag_protocol within this new dsa_resolve_tag_protocol function,
> to make it more readable (no value checking necessary).

I don't see one way being better than the other. I see it as a
personal preference. If you don't like it, i won't NACK a patch
submitted after it has been accepted which changes it to your personal
preference.

  Andrew

Re: [RFC PATCH 09/16] dsa: dsa: Split up creating/destroying of DSA and CPU ports

[Andrew Lunn]

On Fri, May 27, 2016 at 03:25:47PM -0400, Vivien Didelot wrote:
> Hi Andrew, Florian,
> 
> I suggest to use this RFC to agree on a consistent and robust API for
> the DSA layer. Some functions have non-intuitive names or signatures.

Nope.

What is important is getting this patchset accepted and merged, so
people can write drivers using it.

Changes like this can come later, since hopefully all changes like
this are internal. They don't affect the drivers, or the API to the
core.

Andrew

[PATCH 4.7 FIX] brcmfmac: fix lockup when removing P2P interface after event timeout

[Rafał Miłecki]

Removing P2P interface is handled by sending a proper request to the
firmware. On success firmware triggers an event and driver's handler
removes a matching interface.

However on event timeout we remove interface directly from the cfg80211
callback. Current code doesn't handle this case correctly as it always
assumes rtnl to be unlocked.

Fix it by adding an extra rtnl_locked parameter to functions and calling
unregister_netdevice when needed.

Signed-off-by: Rafał Miłecki 
---
 .../broadcom/brcm80211/brcmfmac/cfg80211.c |  2 +-
 .../wireless/broadcom/brcm80211/brcmfmac/core.c| 29 +-
 .../wireless/broadcom/brcm80211/brcmfmac/core.h|  2 +-
 .../wireless/broadcom/brcm80211/brcmfmac/fweh.c|  2 +-
 .../net/wireless/broadcom/brcm80211/brcmfmac/p2p.c |  4 +--
 5 files changed, 22 insertions(+), 17 deletions(-)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c 
b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
index 62f475e..ef09382 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
@@ -5364,7 +5364,7 @@ brcmf_notify_connect_status_ap(struct brcmf_cfg80211_info 
*cfg,
brcmf_dbg(CONN, "AP mode link down\n");
complete(>vif_disabled);
if (ifp->vif->mbss)
-   brcmf_remove_interface(ifp);
+   brcmf_remove_interface(ifp, false);
return 0;
}
 
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c 
b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
index b590499..7f8ba48c 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
@@ -548,12 +548,16 @@ fail:
return -EBADE;
 }
 
-static void brcmf_net_detach(struct net_device *ndev)
+static void brcmf_net_detach(struct net_device *ndev, bool rtnl_locked)
 {
-   if (ndev->reg_state == NETREG_REGISTERED)
-   unregister_netdev(ndev);
-   else
+   if (ndev->reg_state == NETREG_REGISTERED) {
+   if (rtnl_locked)
+   unregister_netdevice(ndev);
+   else
+   unregister_netdev(ndev);
+   } else {
brcmf_cfg80211_free_netdev(ndev);
+   }
 }
 
 void brcmf_net_setcarrier(struct brcmf_if *ifp, bool on)
@@ -651,7 +655,7 @@ struct brcmf_if *brcmf_add_if(struct brcmf_pub *drvr, s32 
bsscfgidx, s32 ifidx,
brcmf_err("ERROR: netdev:%s already exists\n",
  ifp->ndev->name);
netif_stop_queue(ifp->ndev);
-   brcmf_net_detach(ifp->ndev);
+   brcmf_net_detach(ifp->ndev, false);
drvr->iflist[bsscfgidx] = NULL;
} else {
brcmf_dbg(INFO, "netdev:%s ignore IF event\n",
@@ -699,7 +703,8 @@ struct brcmf_if *brcmf_add_if(struct brcmf_pub *drvr, s32 
bsscfgidx, s32 ifidx,
return ifp;
 }
 
-static void brcmf_del_if(struct brcmf_pub *drvr, s32 bsscfgidx)
+static void brcmf_del_if(struct brcmf_pub *drvr, s32 bsscfgidx,
+bool rtnl_locked)
 {
struct brcmf_if *ifp;
 
@@ -729,7 +734,7 @@ static void brcmf_del_if(struct brcmf_pub *drvr, s32 
bsscfgidx)
cancel_work_sync(>multicast_work);
cancel_work_sync(>ndoffload_work);
}
-   brcmf_net_detach(ifp->ndev);
+   brcmf_net_detach(ifp->ndev, rtnl_locked);
} else {
/* Only p2p device interfaces which get dynamically created
 * end up here. In this case the p2p module should be informed
@@ -743,14 +748,14 @@ static void brcmf_del_if(struct brcmf_pub *drvr, s32 
bsscfgidx)
}
 }
 
-void brcmf_remove_interface(struct brcmf_if *ifp)
+void brcmf_remove_interface(struct brcmf_if *ifp, bool rtnl_locked)
 {
if (!ifp || WARN_ON(ifp->drvr->iflist[ifp->bsscfgidx] != ifp))
return;
brcmf_dbg(TRACE, "Enter, bsscfgidx=%d, ifidx=%d\n", ifp->bsscfgidx,
  ifp->ifidx);
brcmf_fws_del_interface(ifp);
-   brcmf_del_if(ifp->drvr, ifp->bsscfgidx);
+   brcmf_del_if(ifp->drvr, ifp->bsscfgidx, rtnl_locked);
 }
 
 int brcmf_get_next_free_bsscfgidx(struct brcmf_pub *drvr)
@@ -1081,9 +1086,9 @@ fail:
brcmf_fws_deinit(drvr);
}
if (ifp)
-   brcmf_net_detach(ifp->ndev);
+   brcmf_net_detach(ifp->ndev, false);
if (p2p_ifp)
-   brcmf_net_detach(p2p_ifp->ndev);
+   brcmf_net_detach(p2p_ifp->ndev, false);
drvr->iflist[0] = NULL;
drvr->iflist[1] = NULL;
if (drvr->settings->ignore_probe_fail)
@@ -1152,7 +1157,7 @@ void brcmf_detach(struct device *dev)
 
/* make sure primary 

Re: [RFC PATCH 13/16] net: dsa: mv88e6xxx: Refactor MDIO so driver registers mdio bus

[Vivien Didelot]

Hi Andrew,

I like the s/phy/mdio/ renaming, but please move it in its own
non-functional patch for easier reviewing.

Also, we may want to use this opportunity to get rid of some _ prefixed
functions. It is hard to following the distinction between these two
following signatures:

int _mv88e6xxx_mdio_read(struct mv88e6xxx_priv_state *ps, int addr, int 
reg_num)

and

int mv88e6xxx_mdio_read(struct mii_bus *bus, int port, int regnum)

(which take different first argument.)

It would be great to introduce here the two low-level, bus-agnostic,
SMI unlocked functions: mv88e6xxx_read() and mv88e6xxx_write().

Thanks,

Vivien

Re: [RFC PATCH 11/16] net: dsa: Refactor selection of tag ops into a function

[Vivien Didelot]

Hi Andrew,

Andrew Lunn  writes:

> @@ -26,6 +26,7 @@ enum dsa_tag_protocol {
>   DSA_TAG_PROTO_TRAILER,
>   DSA_TAG_PROTO_EDSA,
>   DSA_TAG_PROTO_BRCM,
> + _DSA_TAG_LAST,
>  };

I would avoid _ prefixed functions or symbols, we've seen in mv88e6xxx
that it doesn't make the code really readable.

There's already an implicit "DSA_TAG_PROTO" namespace, so I suggest
"DSA_MAX_TAGS" to keep it consistent with DSA_MAX_{PORTS,SWITCHES}.

[...]

> + /*
> +  * Tagging protocol operations for adding and removing an
> +  * encapsulation tag.
> +  */
> + const struct dsa_device_ops *tag_ops;

dsa_device_ops seems too generic for the xmit/rcv tag-related pair. That
being said, I don't really have better suggestions. Any idea?

[...]

> +const struct dsa_device_ops *dsa_device_ops[_DSA_TAG_LAST] = {
> +#ifdef CONFIG_NET_DSA_TAG_DSA
> + [DSA_TAG_PROTO_DSA] = _netdev_ops,
> +#endif
> +#ifdef CONFIG_NET_DSA_TAG_EDSA
> + [DSA_TAG_PROTO_EDSA] = _netdev_ops,
> +#endif
> +#ifdef CONFIG_NET_DSA_TAG_TRAILER
> + [DSA_TAG_PROTO_TRAILER] = _netdev_ops,
> +#endif
> +#ifdef CONFIG_NET_DSA_TAG_BRCM
> + [DSA_TAG_PROTO_BRCM] = _netdev_ops,
> +#endif
> + [DSA_TAG_PROTO_NONE] = _ops,
> +};
>  
>  /* switch driver registration 
> ***/
>  static DEFINE_MUTEX(dsa_switch_drivers_mutex);
> @@ -225,6 +252,20 @@ static int dsa_cpu_dsa_setups(struct dsa_switch *ds, 
> struct device *dev)
>   return 0;
>  }
>  
> +const struct dsa_device_ops *dsa_resolve_tag_protocol(int tag_protocol)
> +{
> + const struct dsa_device_ops *ops;
> +
> + if (tag_protocol >= _DSA_TAG_LAST)
> + return ERR_PTR(-EINVAL);
> + ops = dsa_device_ops[tag_protocol];
> +
> + if (!ops)
> + return ERR_PTR(-ENOPROTOOPT);
> +
> + return ops;
> +}

I don't see the need to add a dsa_device_ops array. I'd keep the switch
case on tag_protocol within this new dsa_resolve_tag_protocol function,
to make it more readable (no value checking necessary).

Thanks,

Vivien

Re: [RFC PATCH 09/16] dsa: dsa: Split up creating/destroying of DSA and CPU ports

[Vivien Didelot]

Hi Andrew, Florian,

I suggest to use this RFC to agree on a consistent and robust API for
the DSA layer. Some functions have non-intuitive names or signatures.

What about:

_[_]

where  matches the first argument. So instead of
dsa_cpu_dsa_setup, we would have:

dsa_switch_setup_cpu_dsa_port(struct dsa_switch *ds, ...)

[...]

Andrew Lunn  writes:

>  /* basic switch operations 
> **/
> -static int dsa_cpu_dsa_setup(struct dsa_switch *ds, struct net_device 
> *master)
> +int dsa_cpu_dsa_setup(struct dsa_switch *ds, struct device *dev,
> +   struct device_node *port_dn, int port)
>  {

We will need the port number in the struct dsa_port at some point. The
device_node is already a member of this structure.

I suggest to add the port index before this patch and change this
function for a more logical:

dsa_switch_setup_cpu_dsa_port(struct dsa_switch *ds, struct dsa_port *dp)

[...]

>   /* Perform configuration of the CPU and DSA ports */
> - ret = dsa_cpu_dsa_setup(ds, dst->master_netdev);
> + ret = dsa_cpu_dsa_setups(ds, parent);
>   if (ret < 0) {
>   netdev_err(dst->master_netdev, "[%d] : can't configure CPU and 
> DSA ports\n",
>  index);

dsa_cpu_dsa_setups is not really meaningful and doesn't add much
value. At least I'd call it dsa_switch_setup_cpu_dsa_ports(), or better,
move the loop directly where it is needed so we know which port sucked:

dsa_switch_setup_one:

/* Perform configuration of the CPU and DSA ports */
for (port = 0; port < DSA_MAX_PORTS; port++) {
if (!(dsa_is_cpu_port(ds, port) || dsa_is_dsa_port(ds, port)))
continue;

ret = dsa_switch_setup_cpu_dsa_port(ds, dev, ds->ports[port]);
if (ret) {
netdev_err(dst->master_netdev,
   "[%d] : can't configure port %d\n",
   index, ds->ports[port]->index);
break;
}
}

Thanks,

Vivien

[PATCH V2] brcmfmac: fix setting AP channel with new firmwares

[Rafał Miłecki]

Firmware for new chipsets is based on a new major version of code
internally maintained at Broadcom. E.g. brcmfmac4366b-pcie.bin (used for
BCM4366B1) is based on 10.10.69.3309 while brcmfmac43602-pcie.ap.bin was
based on 7.35.177.56.

Currently setting AP 5 GHz channel doesn't work reliably with BCM4366B1.
When setting e.g. 36 control channel with VHT80 (center channel 42)
firmware may randomly pick one of:
1) 52 control channel with 58 as center one
2) 100 control channel with 106 as center one
3) 116 control channel with 122 as center one
4) 149 control channel with 155 as center one

It seems new firmwares require setting AP mode (BRCMF_C_SET_AP) before
specifying a channel. Changing an order of firmware calls fixes the
problem. This requirement resulted in two separated "chanspec" calls,
one in AP code path and one in P2P path.

This fix was verified with BCM4366B1 and tested for regressions on
BCM43602.

Signed-off-by: Rafał Miłecki 
---
V2: Avoid checking AP vs. P2P at two different places, just add code setting
"chanspec" to existing paths.
---
 .../broadcom/brcm80211/brcmfmac/cfg80211.c | 36 +++---
 1 file changed, 25 insertions(+), 11 deletions(-)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c 
b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
index 1e2527f..38df1be 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
@@ -4438,7 +4438,7 @@ brcmf_cfg80211_start_ap(struct wiphy *wiphy, struct 
net_device *ndev,
struct brcmf_join_params join_params;
enum nl80211_iftype dev_role;
struct brcmf_fil_bss_enable_le bss_enable;
-   u16 chanspec;
+   u16 chanspec = chandef_to_chanspec(>d11inf, >chandef);
bool mbss;
int is_11d;
 
@@ -4514,16 +4514,8 @@ brcmf_cfg80211_start_ap(struct wiphy *wiphy, struct 
net_device *ndev,
 
brcmf_config_ap_mgmt_ie(ifp->vif, >beacon);
 
+   /* Parameters shared by all radio interfaces */
if (!mbss) {
-   chanspec = chandef_to_chanspec(>d11inf,
-  >chandef);
-   err = brcmf_fil_iovar_int_set(ifp, "chanspec", chanspec);
-   if (err < 0) {
-   brcmf_err("Set Channel failed: chspec=%d, %d\n",
- chanspec, err);
-   goto exit;
-   }
-
if (is_11d != ifp->vif->is_11d) {
err = brcmf_fil_cmd_int_set(ifp, BRCMF_C_SET_REGULATORY,
is_11d);
@@ -4571,6 +4563,8 @@ brcmf_cfg80211_start_ap(struct wiphy *wiphy, struct 
net_device *ndev,
err = -EINVAL;
goto exit;
}
+
+   /* Interface specific setup */
if (dev_role == NL80211_IFTYPE_AP) {
if ((brcmf_feat_is_enabled(ifp, BRCMF_FEAT_MBSS)) && (!mbss))
brcmf_fil_iovar_int_set(ifp, "mbss", 1);
@@ -4580,6 +4574,17 @@ brcmf_cfg80211_start_ap(struct wiphy *wiphy, struct 
net_device *ndev,
brcmf_err("setting AP mode failed %d\n", err);
goto exit;
}
+   if (!mbss) {
+   /* Firmware 10.x requires setting channel after enabling
+* AP and before bringing interface up.
+*/
+   err = brcmf_fil_iovar_int_set(ifp, "chanspec", 
chanspec);
+   if (err < 0) {
+   brcmf_err("Set Channel failed: chspec=%d, %d\n",
+ chanspec, err);
+   goto exit;
+   }
+   }
err = brcmf_fil_cmd_int_set(ifp, BRCMF_C_UP, 1);
if (err < 0) {
brcmf_err("BRCMF_C_UP error (%d)\n", err);
@@ -4601,7 +4606,13 @@ brcmf_cfg80211_start_ap(struct wiphy *wiphy, struct 
net_device *ndev,
goto exit;
}
brcmf_dbg(TRACE, "AP mode configuration complete\n");
-   } else {
+   } else if (dev_role == NL80211_IFTYPE_P2P_GO) {
+   err = brcmf_fil_iovar_int_set(ifp, "chanspec", chanspec);
+   if (err < 0) {
+   brcmf_err("Set Channel failed: chspec=%d, %d\n",
+ chanspec, err);
+   goto exit;
+   }
err = brcmf_fil_bsscfg_data_set(ifp, "ssid", _le,
sizeof(ssid_le));
if (err < 0) {
@@ -4618,7 +4629,10 @@ brcmf_cfg80211_start_ap(struct wiphy *wiphy, struct 
net_device *ndev,
}
 
brcmf_dbg(TRACE, "GO mode configuration complete\n");
+   } else {
+   WARN_ON(1);
}
+

Re: [RFC PATCH 07/16] dsa: Remove dynamic allocate of routing table

[Vivien Didelot]

Hi Andrew,

Andrew Lunn  writes:

>   /*
> -  * An array (with nr_chips elements) of which element [a]
> -  * indicates which port on this switch should be used to
> -  * send packets to that are destined for switch a.  Can be
> -  * NULL if there is only one switch chip.
> +  * An array of which element [a] indicates which port on this
> +  * switch should be used to send packets to that are destined
> +  * for switch a.  Can be NULL if there is only one switch
> +  * chip.
>*/

Please drop the double-space and reformat on 3 lines in the meantime.

Thanks,

Vivien

Re: [RFC 04/12] ndisc: get rid off dev parameter in ndisc_opt_addr_space

[Alexander Aring]

Hi Hannes,

On 05/27/2016 06:56 PM, Hannes Frederic Sowa wrote:
> On 25.05.2016 07:15, YOSHIFUJI Hideaki wrote:
>>
>>
>> Alexander Aring wrote:
>>> This patch removes the net_device parameter from ndisc_opt_addr_space
>>> function. This can be useful for calling such functionality which
>>> doesn't depends on dev parameter. For current existing functionality
>>> which depends on dev parameter, we introduce ndisc_dev_opt_addr_space to 
>>> have
>>> an easy replacement for the ndisc_opt_addr_space function.
>>>
>>> Cc: David S. Miller 
>>> Cc: Alexey Kuznetsov 
>>> Cc: James Morris 
>>> Cc: Hideaki YOSHIFUJI 
>>> Cc: Patrick McHardy 
>>> Signed-off-by: Alexander Aring 
>>> ---
>>>  include/net/ndisc.h | 13 +
>>>  net/ipv6/ndisc.c| 12 ++--
>>>  2 files changed, 15 insertions(+), 10 deletions(-)
>>>
>>> diff --git a/include/net/ndisc.h b/include/net/ndisc.h
>>> index 2d8edaa..dbc8d01 100644
>>> --- a/include/net/ndisc.h
>>> +++ b/include/net/ndisc.h
>>> @@ -127,10 +127,15 @@ static inline int ndisc_addr_option_pad(unsigned 
>>> short type)
>>> }
>>>  }
>>>  
>>> -static inline int ndisc_opt_addr_space(struct net_device *dev)
>>> +static inline int ndisc_opt_addr_space(unsigned char addr_len, int pad)
>>>  {
>>> -   return NDISC_OPT_SPACE(dev->addr_len +
>>> -  ndisc_addr_option_pad(dev->type));
>>> +   return NDISC_OPT_SPACE(addr_len + pad);
>>> +}
>>> +
>>> +static inline int ndisc_dev_opt_addr_space(const struct net_device *dev)
>>> +{
>>> +   return ndisc_opt_addr_space(dev->addr_len,
>>> +   ndisc_addr_option_pad(dev->type));
>>>  }
>>>  
>>
>> I prefer not to change existing functions such as ndisc_opt_addr_space(),
>> and name new function __ndisc_opt_addr_space() etc.
>>
>> Plus, my original thought (when I implement these functions) was to
>> have per-net_device ndisc_opt_addr_spece(), ndisc_opt_adr_data() etc.
>>
>> What do you think of that?
> 
> As I understood it 6lowpan devices need to handle both, non-compressed
> and compressed options/addresses. Probably one can make them
> per-interface, but a change to the arguments has still to happen.
> 

Yes, we need to handle both addresses at the same time.

I think you mixed some keywords here: "non-compressed/compressed", it
should be extended(sometimes also named long) and short address.

But the short address is an optional address, the extended address is
always available (that's why we handle them as dev->addr).

I suppose Hideaki suggest here to don't rename the function, what I have
now is:

diff --git a/include/net/ndisc.h b/include/net/ndisc.h
index 2d8edaa..4cee826 100644
--- a/include/net/ndisc.h
+++ b/include/net/ndisc.h
@@ -127,10 +127,15 @@ static inline int ndisc_addr_option_pad(unsigned short 
type)
}
 }
 
+static inline int __ndisc_opt_addr_space(unsigned char addr_len, int pad)
+{
+   return NDISC_OPT_SPACE(addr_len + pad);
+}
+
 static inline int ndisc_opt_addr_space(struct net_device *dev)
 {
-   return NDISC_OPT_SPACE(dev->addr_len +
-  ndisc_addr_option_pad(dev->type));
+   return __ndisc_opt_addr_space(dev->addr_len,
+ ndisc_addr_option_pad(dev->type));
 }
 
 static inline u8 *ndisc_opt_addr_data(struct nd_opt_hdr *p,
--

which removes a lot the renaming stuff in "ndisc.c". I can still use
"__ndisc_opt_addr_space" as a function which doesn't require net_device
argument. Example:

addr_space = __ndisc_opt_addr_space(IEEE802154_SHORT_ADDR_LEN, 0);

Looks better now.

- Alex

Re: [RFC PATCH 01/16] dsa: slave: chip data is optional, don't dereference NULL

[Vivien Didelot]

Hi Andrew, Florian,

We are inconsistent on commit titles and messages. Some of them use
"net: " prefix, some others don't, sometimes lower or upper case titles.

I'd suggest we stick with the "net: dsa: " prefix and lowercase titles.

When possible, let's try to respect the 50/72 Git rule.

So we'd have e.g.:

net: dsa: add new bindings
net: dsa: mv88e6xxx: add support for foo
net: dsa: sf2: remove bar
...

The networking documentation doesn't seem to have opinion on the "net: "
prefix. We might drop it and keep only "dsa: " for core and drivers.

What do you think?

Thanks,

Vivien

Re: [RFC PATCH 00/16] New DSA bind, switches as devices

[Vivien Didelot]

Hi Andrew,

I tested this RFC and it works fine for me, looks good overall.

I'll do a second pass for nit-picking.

Thanks for this work,

   Vivien

Deadlock when running DevStack on latest pull of net tree

[Alexander Duyck]

I started out this morning by trying to run DevStack on the latest
"net' kernel and it looks like I am hanging on some sort of locking
problem with RabbitMQ.  Specifically I am seeing one CPU jump to 100%
with perf showing that I am spinning on a lock.

I'm working to bisect it now, but just thought I would put it out
there if anybody had already root caused this issue.  Below is a few
traces to the spin lock call on the spinning CPU:
-   18.81%18.81%  beam.smp  [kernel.vmlinux]  [k] _raw_spin_lock_irqsave
   - _raw_spin_lock_irqsave
  - 32.86% add_wait_queue
   n_tty_write
   tty_write
   do_loop_readv_writev
   do_readv_writev
   vfs_writev
   do_writev
   sys_writev
   do_syscall_64
   return_from_SYSCALL_64
   __libc_writev
  - 32.49% remove_wait_queue
   n_tty_write
   tty_write
   do_loop_readv_writev
   do_readv_writev
   vfs_writev
   do_writev
   sys_writev
   do_syscall_64
   return_from_SYSCALL_64
   __libc_writev
  - 31.71% __wake_up
   tty_write_unlock
   tty_write
   do_loop_readv_writev
   do_readv_writev
   vfs_writev
   do_writev
   sys_writev
   do_syscall_64
   return_from_SYSCALL_64
   __libc_writev
  - 1.96% n_tty_write
   tty_write
   do_loop_readv_writev
   do_readv_writev
   vfs_writev
   do_writev
   sys_writev
   do_syscall_64
   return_from_SYSCALL_64
   __libc_writev
  - 0.87% tty_write_unlock
   tty_write
   do_loop_readv_writev
   do_readv_writev
   vfs_writev
   do_writev
   sys_writev
   do_syscall_64
   return_from_SYSCALL_64
   __libc_writev

Re: [PATCH,RFC] macvlan: Handle broadcasts inline if we have only a few macvlans.

[Lennert Buytenhek]

On Fri, May 27, 2016 at 10:56:44AM -0700, Cong Wang wrote:

> > Commit 412ca1550cbecb2c ("macvlan: Move broadcasts into a work queue")
> > moved processing of all macvlan multicasts into a work queue.  This
> > causes a noticable performance regression when there is heavy multicast
> > traffic on the underlying interface for multicast groups that the
> > macvlan subinterfaces are not members of, in which case we end up
> > cloning all those packets and then freeing them again from a work queue
> > without really doing any useful work with them in between.
> 
> But we only queue up to 1000 packets in our backlog.
> 
> How about adding a quick check before cloning it?
> 
> diff --git a/drivers/net/macvlan.c b/drivers/net/macvlan.c
> index cb01023..1c73d0f 100644
> --- a/drivers/net/macvlan.c
> +++ b/drivers/net/macvlan.c
> @@ -315,6 +315,9 @@ static void macvlan_broadcast_enqueue(struct
> macvlan_port *port,
> struct sk_buff *nskb;
> int err = -ENOMEM;
> 
> +   if (skb_queue_len(>bc_queue) >= MACVLAN_BC_QUEUE_LEN)
> +   return;
> +
> nskb = skb_clone(skb, GFP_ATOMIC);
> if (!nskb)
> goto err;

We're not hitting the bc_queue skb limit in our environment, as the
machine can keep up with the traffic -- it's just that taking an
extra clone of the skb and queueing and running the work queue item
to free it again is eating up a lot of cycles.

But doing the queue length check before the clone might not be a bad
idea?  (You'd probably want to atomic_long_inc(>dev->rx_dropped)
before returning, though?)

Re: [PATCH,RFC] macvlan: Handle broadcasts inline if we have only a few macvlans.

[Cong Wang]

On Thu, May 26, 2016 at 4:44 PM, Lennert Buytenhek
 wrote:
> Commit 412ca1550cbecb2c ("macvlan: Move broadcasts into a work queue")
> moved processing of all macvlan multicasts into a work queue.  This
> causes a noticable performance regression when there is heavy multicast
> traffic on the underlying interface for multicast groups that the
> macvlan subinterfaces are not members of, in which case we end up
> cloning all those packets and then freeing them again from a work queue
> without really doing any useful work with them in between.

But we only queue up to 1000 packets in our backlog.

How about adding a quick check before cloning it?

diff --git a/drivers/net/macvlan.c b/drivers/net/macvlan.c
index cb01023..1c73d0f 100644
--- a/drivers/net/macvlan.c
+++ b/drivers/net/macvlan.c
@@ -315,6 +315,9 @@ static void macvlan_broadcast_enqueue(struct
macvlan_port *port,
struct sk_buff *nskb;
int err = -ENOMEM;

+   if (skb_queue_len(>bc_queue) >= MACVLAN_BC_QUEUE_LEN)
+   return;
+
nskb = skb_clone(skb, GFP_ATOMIC);
if (!nskb)
goto err;

BUG: netfilter hooks: problems porting to 4.4.6

[Sam Patton]

Did something change in Netfilter on the 4.4.6 kernel that would affect my

Netfilter hook driver?  I have a driver that works in 2.6.38, 3.10.49 and

3.15.3.   I started porting the driver to the 4.4.6 kernel and

can't get it to work.  Specifically, apply_eip_snat() in the driver never gets

called and it should get called as packets leave the NAT.  The

print statement below  where I print out maniptype is always returning 1.

e.g. NF_NAT_MANIP_DST.


Here's my iptables command:

iptables -t nat -A POSTROUTING -o eth0 -j MASQUERADE


Here's relevant code snippets.

static struct nf_hook_ops nf_nat_ops[] __read_mostly = {

  // Before packet filtering, change destination

  {

  .hook   = nf_eipnat_in,

  .owner  = THIS_MODULE,

  .pf = PF_INET,

  .hooknum= NF_INET_PRE_ROUTING,

  .priority   = NF_IP_PRI_NAT_DST,

  },

  // After packet filtering, change source

  {

  .hook   = nf_eipnat_out,

  .owner  = THIS_MODULE,

  .pf = PF_INET,

  .hooknum= NF_INET_POST_ROUTING,

  .priority   = NF_IP_PRI_NAT_SRC,

  },

};



nf_eipnat_in(unsigned int hooknum,

struct sk_buff *skb,

const struct net_device *in,

const struct net_device *out,

int (*okfn)(struct sk_buff *))

{

  printk("nf_eipnat_in\n");

  return nf_nat_fn("dnat in prerouting", hooknum, skb, in, out, okfn);

}



static unsigned int

nf_eipnat_out(unsigned int hooknum,

 struct sk_buff *skb,

 const struct net_device *in,

 const struct net_device *out,

 int (*okfn)(struct sk_buff *))

{

  printk("nf_eipnat_out\n");

  return nf_nat_fn("snat out postrouting", hooknum, skb, in, out, okfn);

}


static unsigned int

nf_nat_fn(char *instring,

unsigned int hooknum,

struct sk_buff *skb,

const struct net_device *in,

const struct net_device *out,

int (*okfn)(struct sk_buff *))

{

  unsigned int retval = 0;


  /* maniptype == SRC for postrouting. */

  enum nf_nat_manip_type maniptype = HOOK2MANIP(hooknum);

  printk("maniptype = %d\n", maniptype);


  /* We never see fragments: conntrack defrags on pre-routing

 and local-out, and nf_nat_out protects post-routing. */

  NF_CT_ASSERT(!(ip_hdr(skb)->frag_off & htons(IP_MF | IP_OFFSET)));


  //printskb(instring, skb, in, out);

  if(maniptype == NF_NAT_MANIP_DST){

  retval = apply_eip_dnat(in, skb);

  }

  else{

  retval = apply_eip_snat(out, skb);

  }


  return retval;

}

Thanks!

-Sam

Re: IPv6 extension header privileges

[Tom Herbert]

On Fri, May 27, 2016 at 10:14 AM, Hannes Frederic Sowa
 wrote:
> Hi,
>
> On 27.05.2016 18:59, Tom Herbert wrote:
>> On Fri, May 27, 2016 at 8:03 AM, Sowmini Varadhan
>>  wrote:
>>> On (05/27/16 11:53), Hannes Frederic Sowa wrote:
> Thinking about this some more, the per option white list is a better
> approach. If we allow an open ended mechanism for applications to
> signal the network with arbitrary data (like user specified hbp
> options would be), then use of that mechanism will inevitably
> exploited by some authorities to force user to hand over private data
> about their communications. It's better to not build in back doors to
> security...

 Sorry, Tom, can you try to explain again, I think I might not have
 understood you correctly.
>>>
>> Option whitelists are the right approach, applications should not be
>> able to set random options in IP extension headers.
>>
>>> yes, me too. Might help to discuss this by looking at an instance
>>> of the type of option we are talking about.
>>>
>>> Usually hbh options are handled in the forwarding path (and thus
>>> as unpopular as ip options, since they derail the packet to the
>>> slow path). Are you suggesting some type of AAA to vet the hbh
>>> option itself?
>>>
>> The issue that came up in IETF is that network operators (particularly
>> radio networks) are concerned about the loss of visibility into the
>> content since TLS became widely deployed. Unfortunately from the
>> operators point of view at least, we are likely to see transport layer
>> headers also being encrypted in the Internet (like Transports over
>> UDP) which means they will have even less information. There is
>> discussion now about rather applications can "give back" information
>> that network operators previously deduced from inspecting clear text
>> transport headers and payload. This would be accomplished with some
>> sort of signaling to the network from applications. IP ext. headers
>> are the standard mechanism for such signaling, although are a lot of
>> people don't want to use them because they need to go through the
>> kernel to set them-- because kernel takes too long deploy, bad
>> interfaces, has too much control, etc.
>
> I also wonder how helpful TCP middleboxes are nowadays. I haven't done
> any kind of tests for example over satellite connections with TCP. A TCP
> frame cache probably helps a lot getting a faster connection but at the
> same time messes with the RTT. I could imagine network operators wanting
> control over the data stream.
>
No doubt. Things like in-network TCP optimizers have existed for a long time.

> Your proposal with DTLS also leaks a lot of information. E.g. I remember
> that certificates are always send in clear text and I currently don't
> know if TLS 1.3 solves this (DTLS is basically TLS split on record
> boundaries to UDP frames). Sequence numbers ensure that e.g. no reply
> attack can happen. The DTLS "connection establishment" and "teardown"
> also provides the necessary information to handle the UDP flow stateful,
> thus connection tracking is not completely circumvented. (D)TLS also
> provides extensions which could be used for signaling and keeping state
> in the network.
>
Yes, in a perfect world the network would only look at IP headers and
have no incentive or even ability to look beyond that. AFAIK, we
always need some cleartext to provide the security context, but at
least that would be authenticated.

> Otherwise my problem with DTLS is that it is much less deployed. E.g. I
> remember from the mosh paper [1] that no current implementation of DTLS
> actually supports roaming of "DTLS connections".
>
It's an end to end issue. To deploy DTLS I should only need to update
my client and server application, not the kernel are any intermediate
device. This is essential to getting us back to the E2E principle.
Probably the more operative question is rather UDP can be widely
deployed in the Internet.

> [1] https://mosh.mit.edu/mosh-paper-draft.pdf
>
>> Hop by hop are open ended options defined as "The Hop-by-Hop Options
>> header is used to carry optional information that must be examined by
>> every node along a packet's delivery path.". If we allow applications
>> to set arbitrary hbh options then the danger is that their network
>> operator or local authority may require their own defined options.
>> There's no reason to believe that this would be benevolent, such a
>> mechanism could be used to force applications to leak private
>> information about encrypted content which would diminish security or
>> net neutrality. For instance a network provider could require its
>> users to mark packets that are for cat videos, or want the URLs being
>> accessed in http requests (described in a accord BOF @IETF), and there
>> are even going to be authorities that demand they have access to
>> crypto keys. Obviously there are always going to be 

Re: [PATCH v1 1/6] drivers: net: xgene: MAC and PHY configuration changes

[Matthias Brugger]

On 27/05/16 09:22, Iyappan Subramanian wrote:

This patch fixes MAC configuration to support 10/100GbE for SGMII and
link_state call back. It also sets pdata->mdio_driver flag based on
ethernet mdio subnode and prepare for MDIO driver support.

In summary, following are the changes,

- Added set_speed function pointer in mac_ops
- Changed link_state to call the set_speed
- Add 10/100 support for SGMII based 1G
- Fixed mac_init for 1G

- Call mac_ops rx_enable/disable and tx_enable/disable function pointers


This is just a code clean-up, right? If so, this should be a patch apart.


- Add acpi_phy_find_device to find PHY using phy-handle reference object
- Changing phy_start and phy_stop calls based on phy_dev object existence
- Calling phy_connect based on pdata->mdio_driver flag

Signed-off-by: Iyappan Subramanian 
Tested-by: Fushen Chen 
Tested-by: Toan Le 
---
  drivers/net/ethernet/apm/xgene/xgene_enet_hw.c| 189 +-
  drivers/net/ethernet/apm/xgene/xgene_enet_hw.h|   4 +
  drivers/net/ethernet/apm/xgene/xgene_enet_main.c  |  40 +++--
  drivers/net/ethernet/apm/xgene/xgene_enet_main.h  |   2 +
  drivers/net/ethernet/apm/xgene/xgene_enet_sgmac.c | 106 +++-
  drivers/net/ethernet/apm/xgene/xgene_enet_sgmac.h |   8 +
  drivers/net/ethernet/apm/xgene/xgene_enet_xgmac.h |   4 +
  7 files changed, 256 insertions(+), 97 deletions(-)

diff --git a/drivers/net/ethernet/apm/xgene/xgene_enet_hw.c 
b/drivers/net/ethernet/apm/xgene/xgene_enet_hw.c
index 2f5638f..6bc8360 100644
--- a/drivers/net/ethernet/apm/xgene/xgene_enet_hw.c
+++ b/drivers/net/ethernet/apm/xgene/xgene_enet_hw.c
@@ -512,14 +512,11 @@ static void xgene_enet_configure_clock(struct 
xgene_enet_pdata *pdata)


[...]


  static void xgene_sgmac_init(struct xgene_enet_pdata *p)
  {
u32 data, loop = 10;
-   u32 offset = p->port_id * 4;
+   u32 offset = 0;
u32 enet_spare_cfg_reg, rsif_config_reg;
u32 cfg_bypass_reg, rx_dv_gate_reg;

xgene_sgmac_reset(p);

/* Enable auto-negotiation */
-   xgene_mii_phy_write(p, INT_PHY_ADDR, SGMII_CONTROL_ADDR >> 2, 0x1000);
+   xgene_mii_phy_write(p, INT_PHY_ADDR, SGMII_TBI_CONTROL_ADDR >> 2,
+   0x8000);
+   xgene_mii_phy_write(p, INT_PHY_ADDR, SGMII_CONTROL_ADDR >> 2, 0x9000);
xgene_mii_phy_write(p, INT_PHY_ADDR, SGMII_TBI_CONTROL_ADDR >> 2, 0);

while (loop--) {
@@ -256,16 +343,14 @@ static void xgene_sgmac_init(struct xgene_enet_pdata *p)
if (!(data & AUTO_NEG_COMPLETE) || !(data & LINK_STATUS))
netdev_err(p->ndev, "Auto-negotiation failed\n");

-   data = xgene_enet_rd_mac(p, MAC_CONFIG_2_ADDR);
-   ENET_INTERFACE_MODE2_SET(, 2);
-   xgene_enet_wr_mac(p, MAC_CONFIG_2_ADDR, data | FULL_DUPLEX2);
-   xgene_enet_wr_mac(p, INTERFACE_CONTROL_ADDR, ENET_GHD_MODE);
+   xgene_sgmac_set_speed(p);

if (p->enet_id == XGENE_ENET1) {
enet_spare_cfg_reg = ENET_SPARE_CFG_REG_ADDR;
rsif_config_reg = RSIF_CONFIG_REG_ADDR;
cfg_bypass_reg = CFG_BYPASS_ADDR;
rx_dv_gate_reg = SG_RX_DV_GATE_REG_0_ADDR;
+   offset = p->port_id * 4;


At least for me it is not clear where the "4" comes from. Maybe you can 
add a comment or declare a constant for that.
Anyway this looks like it fixes the driver for xgene2-sgnet and therefor 
should go into a separate patch, or am I wrong?


Regards,
Matthias

Re: IPv6 extension header privileges

[Hannes Frederic Sowa]

Hi,

On 27.05.2016 18:59, Tom Herbert wrote:
> On Fri, May 27, 2016 at 8:03 AM, Sowmini Varadhan
>  wrote:
>> On (05/27/16 11:53), Hannes Frederic Sowa wrote:
 Thinking about this some more, the per option white list is a better
 approach. If we allow an open ended mechanism for applications to
 signal the network with arbitrary data (like user specified hbp
 options would be), then use of that mechanism will inevitably
 exploited by some authorities to force user to hand over private data
 about their communications. It's better to not build in back doors to
 security...
>>>
>>> Sorry, Tom, can you try to explain again, I think I might not have
>>> understood you correctly.
>>
> Option whitelists are the right approach, applications should not be
> able to set random options in IP extension headers.
> 
>> yes, me too. Might help to discuss this by looking at an instance
>> of the type of option we are talking about.
>>
>> Usually hbh options are handled in the forwarding path (and thus
>> as unpopular as ip options, since they derail the packet to the
>> slow path). Are you suggesting some type of AAA to vet the hbh
>> option itself?
>>
> The issue that came up in IETF is that network operators (particularly
> radio networks) are concerned about the loss of visibility into the
> content since TLS became widely deployed. Unfortunately from the
> operators point of view at least, we are likely to see transport layer
> headers also being encrypted in the Internet (like Transports over
> UDP) which means they will have even less information. There is
> discussion now about rather applications can "give back" information
> that network operators previously deduced from inspecting clear text
> transport headers and payload. This would be accomplished with some
> sort of signaling to the network from applications. IP ext. headers
> are the standard mechanism for such signaling, although are a lot of
> people don't want to use them because they need to go through the
> kernel to set them-- because kernel takes too long deploy, bad
> interfaces, has too much control, etc.

I also wonder how helpful TCP middleboxes are nowadays. I haven't done
any kind of tests for example over satellite connections with TCP. A TCP
frame cache probably helps a lot getting a faster connection but at the
same time messes with the RTT. I could imagine network operators wanting
control over the data stream.

Your proposal with DTLS also leaks a lot of information. E.g. I remember
that certificates are always send in clear text and I currently don't
know if TLS 1.3 solves this (DTLS is basically TLS split on record
boundaries to UDP frames). Sequence numbers ensure that e.g. no reply
attack can happen. The DTLS "connection establishment" and "teardown"
also provides the necessary information to handle the UDP flow stateful,
thus connection tracking is not completely circumvented. (D)TLS also
provides extensions which could be used for signaling and keeping state
in the network.

Otherwise my problem with DTLS is that it is much less deployed. E.g. I
remember from the mosh paper [1] that no current implementation of DTLS
actually supports roaming of "DTLS connections".

[1] https://mosh.mit.edu/mosh-paper-draft.pdf

> Hop by hop are open ended options defined as "The Hop-by-Hop Options
> header is used to carry optional information that must be examined by
> every node along a packet's delivery path.". If we allow applications
> to set arbitrary hbh options then the danger is that their network
> operator or local authority may require their own defined options.
> There's no reason to believe that this would be benevolent, such a
> mechanism could be used to force applications to leak private
> information about encrypted content which would diminish security or
> net neutrality. For instance a network provider could require its
> users to mark packets that are for cat videos, or want the URLs being
> accessed in http requests (described in a accord BOF @IETF), and there
> are even going to be authorities that demand they have access to
> crypto keys. Obviously there are always going to be attempts to force
> users to give up private information, but I don't think we (neither
> Linux nor IETF) should be building mechanisms that make it easy to do
> that.

Sounds like solving social problems with technology. :)

In general I agree, but it seems to me that the provider edges are
already capable of doing so, adding headers at the edge and transporting
them over the backbone and removing them add the edge again. They could
as well also be tunneled in geneve completely to provide arbitrary TLV
options.

Another questions about your approach:

Do you expect e.g. Facebook deploying TLS inside DTLS or is the DTLS
connection in the long run a way to defeat end-to-end crypto, as only
the tunnel themselves will be encrypted? Adding encryption in encryption
might be a huge performance loss?

Re: [PATCH v1 0/6] drivers: net: xgene: Fix 1G hot-plug and module support

[Matthias Brugger]

On 27/05/16 09:22, Iyappan Subramanian wrote:

This patchset addresses the following issues,

1. hot-plug issue on the SGMII 1G interface
- by adding a driver for MDIO management
2. fixes the kernel crash when the driver loaded as an kernel module
- by fixing hardware cleanups and rearrange kernel API calls

Signed-off-by: Iyappan Subramanian 
---


Without this patches we observed problems on some hardware when the 
driver was loaded as modules, as well as the kernel crash when 
unloading/loading the module.


I tested this with an older FW and both issues are fixed, so you can add

Tested-by: Matthias Brugger 

But I think the way the series is split in different patches is not 
correct. Patch 1 on its own won't compile and get fixed in patch 2.
Although I think backward compatibility to older firmware should be part 
of the changes which provide support for the new firmware and should not 
be a patch apart. Said this, I'm not sure if just squashing patch 1 and 
patch 2 is the right to do, as to me it seems as if some other fixes 
slipped into the series.


From my point of view, patch 4 and 5 should be squashed, as xge0clk and 
sge1clk nodes are useless when the mdio nodes are present.


Regards,
Matthias



Iyappan Subramanian (6):
   drivers: net: xgene: MAC and PHY configuration changes
   drivers: net: xgene: Backward compatibility with older firmware
   drivers: net: phy: Add MDIO driver
   dtb: xgene: Add MDIO node
   dtb: xgene: Remove clock nodes
   drivers: net: xgene: Fix module load/unload crash

  arch/arm64/boot/dts/apm/apm-merlin.dts|  10 +
  arch/arm64/boot/dts/apm/apm-mustang.dts   |  12 +
  arch/arm64/boot/dts/apm/apm-shadowcat.dtsi|  23 +-
  arch/arm64/boot/dts/apm/apm-storm.dtsi|  38 +-
  drivers/net/ethernet/apm/xgene/Kconfig|   1 +
  drivers/net/ethernet/apm/xgene/xgene_enet_hw.c| 241 ++
  drivers/net/ethernet/apm/xgene/xgene_enet_hw.h|  18 +-
  drivers/net/ethernet/apm/xgene/xgene_enet_main.c  | 325 -
  drivers/net/ethernet/apm/xgene/xgene_enet_main.h  |  36 +-
  drivers/net/ethernet/apm/xgene/xgene_enet_sgmac.c | 191 +++-
  drivers/net/ethernet/apm/xgene/xgene_enet_sgmac.h |   8 +
  drivers/net/ethernet/apm/xgene/xgene_enet_xgmac.c |  66 ++-
  drivers/net/ethernet/apm/xgene/xgene_enet_xgmac.h |   4 +
  drivers/net/phy/Kconfig   |   7 +
  drivers/net/phy/Makefile  |   1 +
  drivers/net/phy/mdio-xgene.c  | 531 ++
  drivers/net/phy/mdio-xgene.h  | 140 ++
  17 files changed, 1373 insertions(+), 279 deletions(-)
  create mode 100644 drivers/net/phy/mdio-xgene.c
  create mode 100644 drivers/net/phy/mdio-xgene.h

Re: IPv6 extension header privileges

[Tom Herbert]

On Fri, May 27, 2016 at 9:46 AM, Hannes Frederic Sowa
 wrote:
> On Thu, May 26, 2016, at 20:42, Tom Herbert wrote:
>> Thinking about this some more, the per option white list is a better
>> approach. If we allow an open ended mechanism for applications to
>> signal the network with arbitrary data (like user specified hbp
>> options would be), then use of that mechanism will inevitably
>> exploited by some authorities to force user to hand over private data
>> about their communications. It's better to not build in back doors to
>> security...
>
> Also I don't think that HbH options form some kind of hidden covert
> channel. They mostly appear by unused fields which cannot be verified by
> the other (receiving) side in any way.

It would be pretty easy to make it that. All a network operator would
need to do is strip their proprietary options on egress from their
network. So a receiver, say our servers at FB, would have no way to
determine that our clients are being manipulated.

Tom

Re: IPv6 extension header privileges

[Tom Herbert]

On Fri, May 27, 2016 at 8:03 AM, Sowmini Varadhan
 wrote:
> On (05/27/16 11:53), Hannes Frederic Sowa wrote:
>> > Thinking about this some more, the per option white list is a better
>> > approach. If we allow an open ended mechanism for applications to
>> > signal the network with arbitrary data (like user specified hbp
>> > options would be), then use of that mechanism will inevitably
>> > exploited by some authorities to force user to hand over private data
>> > about their communications. It's better to not build in back doors to
>> > security...
>>
>> Sorry, Tom, can you try to explain again, I think I might not have
>> understood you correctly.
>
Option whitelists are the right approach, applications should not be
able to set random options in IP extension headers.

> yes, me too. Might help to discuss this by looking at an instance
> of the type of option we are talking about.
>
> Usually hbh options are handled in the forwarding path (and thus
> as unpopular as ip options, since they derail the packet to the
> slow path). Are you suggesting some type of AAA to vet the hbh
> option itself?
>
The issue that came up in IETF is that network operators (particularly
radio networks) are concerned about the loss of visibility into the
content since TLS became widely deployed. Unfortunately from the
operators point of view at least, we are likely to see transport layer
headers also being encrypted in the Internet (like Transports over
UDP) which means they will have even less information. There is
discussion now about rather applications can "give back" information
that network operators previously deduced from inspecting clear text
transport headers and payload. This would be accomplished with some
sort of signaling to the network from applications. IP ext. headers
are the standard mechanism for such signaling, although are a lot of
people don't want to use them because they need to go through the
kernel to set them-- because kernel takes too long deploy, bad
interfaces, has too much control, etc.

Hop by hop are open ended options defined as "The Hop-by-Hop Options
header is used to carry optional information that must be examined by
every node along a packet's delivery path.". If we allow applications
to set arbitrary hbh options then the danger is that their network
operator or local authority may require their own defined options.
There's no reason to believe that this would be benevolent, such a
mechanism could be used to force applications to leak private
information about encrypted content which would diminish security or
net neutrality. For instance a network provider could require its
users to mark packets that are for cat videos, or want the URLs being
accessed in http requests (described in a accord BOF @IETF), and there
are even going to be authorities that demand they have access to
crypto keys. Obviously there are always going to be attempts to force
users to give up private information, but I don't think we (neither
Linux nor IETF) should be building mechanisms that make it easy to do
that.

Tom

> --Sowmini

Re: [RFC 04/12] ndisc: get rid off dev parameter in ndisc_opt_addr_space

[Hannes Frederic Sowa]

On 25.05.2016 07:15, YOSHIFUJI Hideaki wrote:
> 
> 
> Alexander Aring wrote:
>> This patch removes the net_device parameter from ndisc_opt_addr_space
>> function. This can be useful for calling such functionality which
>> doesn't depends on dev parameter. For current existing functionality
>> which depends on dev parameter, we introduce ndisc_dev_opt_addr_space to have
>> an easy replacement for the ndisc_opt_addr_space function.
>>
>> Cc: David S. Miller 
>> Cc: Alexey Kuznetsov 
>> Cc: James Morris 
>> Cc: Hideaki YOSHIFUJI 
>> Cc: Patrick McHardy 
>> Signed-off-by: Alexander Aring 
>> ---
>>  include/net/ndisc.h | 13 +
>>  net/ipv6/ndisc.c| 12 ++--
>>  2 files changed, 15 insertions(+), 10 deletions(-)
>>
>> diff --git a/include/net/ndisc.h b/include/net/ndisc.h
>> index 2d8edaa..dbc8d01 100644
>> --- a/include/net/ndisc.h
>> +++ b/include/net/ndisc.h
>> @@ -127,10 +127,15 @@ static inline int ndisc_addr_option_pad(unsigned short 
>> type)
>>  }
>>  }
>>  
>> -static inline int ndisc_opt_addr_space(struct net_device *dev)
>> +static inline int ndisc_opt_addr_space(unsigned char addr_len, int pad)
>>  {
>> -return NDISC_OPT_SPACE(dev->addr_len +
>> -   ndisc_addr_option_pad(dev->type));
>> +return NDISC_OPT_SPACE(addr_len + pad);
>> +}
>> +
>> +static inline int ndisc_dev_opt_addr_space(const struct net_device *dev)
>> +{
>> +return ndisc_opt_addr_space(dev->addr_len,
>> +ndisc_addr_option_pad(dev->type));
>>  }
>>  
> 
> I prefer not to change existing functions such as ndisc_opt_addr_space(),
> and name new function __ndisc_opt_addr_space() etc.
> 
> Plus, my original thought (when I implement these functions) was to
> have per-net_device ndisc_opt_addr_spece(), ndisc_opt_adr_data() etc.
> 
> What do you think of that?

As I understood it 6lowpan devices need to handle both, non-compressed
and compressed options/addresses. Probably one can make them
per-interface, but a change to the arguments has still to happen.

Alex?

Thanks,
Hannes

Re: IPv6 extension header privileges

[Hannes Frederic Sowa]

On Thu, May 26, 2016, at 20:42, Tom Herbert wrote:
> Thinking about this some more, the per option white list is a better
> approach. If we allow an open ended mechanism for applications to
> signal the network with arbitrary data (like user specified hbp
> options would be), then use of that mechanism will inevitably
> exploited by some authorities to force user to hand over private data
> about their communications. It's better to not build in back doors to
> security...

Also I don't think that HbH options form some kind of hidden covert
channel. They mostly appear by unused fields which cannot be verified by
the other (receiving) side in any way.

[PATCH net v5] vlan: Propagate MAC address to VLANs

[Mike Manning]

The MAC address of the physical interface is only copied to the VLAN
when it is first created, resulting in an inconsistency after MAC
address changes of only newly created VLANs having an up-to-date MAC.

The VLANs should continue inheriting the MAC address of the physical
interface until the VLAN MAC address is explicitly set to any value. 
This allows IPv6 EUI64 addresses for the VLAN to reflect any changes
to the MAC of the physical interface and thus for DAD to behave as
expected.

Signed-off-by: Mike Manning 
---
 net/8021q/vlan.c |5 +
 net/8021q/vlan.h |2 ++
 net/8021q/vlan_dev.c |   20 +---
 3 files changed, 24 insertions(+), 3 deletions(-)

--- a/net/8021q/vlan.c
+++ b/net/8021q/vlan.c
@@ -291,6 +291,10 @@ static void vlan_sync_address(struct net
if (ether_addr_equal(vlan->real_dev_addr, dev->dev_addr))
return;
 
+   /* vlan continues to inherit address of lower device */
+   if (vlan_dev_inherit_address(vlandev, dev))
+   goto out;
+
/* vlan address was different from the old address and is equal to
 * the new address */
if (!ether_addr_equal(vlandev->dev_addr, vlan->real_dev_addr) &&
@@ -303,6 +307,7 @@ static void vlan_sync_address(struct net
!ether_addr_equal(vlandev->dev_addr, dev->dev_addr))
dev_uc_add(dev, vlandev->dev_addr);
 
+out:
ether_addr_copy(vlan->real_dev_addr, dev->dev_addr);
 }
 
--- a/net/8021q/vlan.h
+++ b/net/8021q/vlan.h
@@ -109,6 +109,8 @@ int vlan_check_real_dev(struct net_devic
 void vlan_setup(struct net_device *dev);
 int register_vlan_dev(struct net_device *dev);
 void unregister_vlan_dev(struct net_device *dev, struct list_head *head);
+bool vlan_dev_inherit_address(struct net_device *dev,
+ struct net_device *real_dev);
 
 static inline u32 vlan_get_ingress_priority(struct net_device *dev,
u16 vlan_tci)
--- a/net/8021q/vlan_dev.c
+++ b/net/8021q/vlan_dev.c
@@ -244,6 +244,17 @@ void vlan_dev_get_realdev_name(const str
strncpy(result, vlan_dev_priv(dev)->real_dev->name, 23);
 }
 
+bool vlan_dev_inherit_address(struct net_device *dev,
+ struct net_device *real_dev)
+{
+   if (dev->addr_assign_type != NET_ADDR_STOLEN)
+   return false;
+
+   ether_addr_copy(dev->dev_addr, real_dev->dev_addr);
+   call_netdevice_notifiers(NETDEV_CHANGEADDR, dev);
+   return true;
+}
+
 static int vlan_dev_open(struct net_device *dev)
 {
struct vlan_dev_priv *vlan = vlan_dev_priv(dev);
@@ -254,7 +265,8 @@ static int vlan_dev_open(struct net_devi
!(vlan->flags & VLAN_FLAG_LOOSE_BINDING))
return -ENETDOWN;
 
-   if (!ether_addr_equal(dev->dev_addr, real_dev->dev_addr)) {
+   if (!ether_addr_equal(dev->dev_addr, real_dev->dev_addr) &&
+   !vlan_dev_inherit_address(dev, real_dev)) {
err = dev_uc_add(real_dev, dev->dev_addr);
if (err < 0)
goto out;
@@ -558,8 +570,10 @@ static int vlan_dev_init(struct net_devi
/* ipv6 shared card related stuff */
dev->dev_id = real_dev->dev_id;
 
-   if (is_zero_ether_addr(dev->dev_addr))
-   eth_hw_addr_inherit(dev, real_dev);
+   if (is_zero_ether_addr(dev->dev_addr)) {
+   ether_addr_copy(dev->dev_addr, real_dev->dev_addr);
+   dev->addr_assign_type = NET_ADDR_STOLEN;
+   }
if (is_zero_ether_addr(dev->broadcast))
memcpy(dev->broadcast, real_dev->broadcast, dev->addr_len);
 
-- 
1.7.10.4

Re: [RFC PATCH 09/16] dsa: dsa: Split up creating/destroying of DSA and CPU ports

[Vivien Didelot]

Hi Andrew,

Andrew Lunn  writes:

> On Fri, May 27, 2016 at 10:33:49AM -0400, Vivien Didelot wrote:
>> Hi Andrew,
>> 
>> Andrew Lunn  writes:
>> 
>> > -static void dsa_switch_destroy(struct dsa_switch *ds)
>> > +void dsa_cpu_dsa_destroy(struct device_node *port_dn)
>> >  {
>> > -  struct device_node *port_dn;
>> >struct phy_device *phydev;
>> > +
>> > +  if (of_phy_is_fixed_link(port_dn)) {
>> > +  phydev = of_phy_find_device(port_dn);
>> > +  if (phydev) {
>> > +  phy_device_free(phydev);
>> > +  fixed_phy_unregister(phydev);
>> > +  }
>> > +  }
>> > +}
>> > +
>> > +static void dsa_switch_destroy(struct dsa_switch *ds)
>> > +{
>> >int port;
>> >  
>> >  #ifdef CONFIG_NET_DSA_HWMON
>> > @@ -445,17 +467,11 @@ static void dsa_switch_destroy(struct dsa_switch *ds)
>> >dsa_slave_destroy(ds->ports[port].netdev);
>> >}
>> >  
>> > -  /* Remove any fixed link PHYs */
>> > +  /* Disable configuration of the CPU and DSA ports */
>> >for (port = 0; port < DSA_MAX_PORTS; port++) {
>> > -  port_dn = ds->ports[port].dn;
>> > -  if (of_phy_is_fixed_link(port_dn)) {
>> > -  phydev = of_phy_find_device(port_dn);
>> > -  if (phydev) {
>> > -  phy_device_free(phydev);
>> > -  of_node_put(port_dn);
>> 
>> Why does dsa_cpu_dsa_destroy drop that of_node_put call?
>
> The of node reference counting is broken. The DT maintainers actually
> say not to care, the whole reference counting scheme is broken. Which
> is a bit sad really. There was a discussion about this a couple of
> months ago.
>
> Anyway, the reference is taken in dsa_of_probe() as part of the
> or_each_available_child_of_node(child, port). This reference has
> nothing to do with the port being a fixed link or not. So freeing it
> here is inappropriate. The correct place to free it would probably be
> in dsa_of_remove.

OK, good to know. Can you split that in its own patch (prefered), or at
least document that in the commit message?

>> > -  fixed_phy_unregister(phydev);
>> > -  }
>> > -  }
>> > +  if ((dsa_is_cpu_port(ds, port) || dsa_is_dsa_port(ds, port)))
>> > +  continue;
>> 
>> Why do we skip DSA and CPU ports here? The previous code didn't.
>> 
>> > +  dsa_cpu_dsa_destroy(ds->ports[port].dn);
>
> They are now destroyed by the newly added dsa_cpu_dsa_destroy().  I'm
> making the code more symmetrical and easier to re-use. The inverse of
> this function is dsa_switch_setup_one() and it also uses a helper
> function to setup the dsa and cpu ports, dsa_cpu_dsa_setups().

But dsa_cpu_dsa_destroy() is not called here. Shouldn't we drop (like
before) or at least invert the condition above?

Thanks,

Vivien

Re: [RFC PATCH 12/16] dsa: Make mdio bus optional

[Vivien Didelot]

Hi Andrew,

Andrew Lunn  writes:

> On Fri, May 27, 2016 at 10:55:45AM -0400, Vivien Didelot wrote:
>> Hi Andrew,
>> 
>> Andrew Lunn  writes:
>> 
>> > -  mdiobus_unregister(ds->slave_mii_bus);
>> > +  if (ds->slave_mii_bus && ds->drv->phy_read)
>> > +  mdiobus_unregister(ds->slave_mii_bus);
>> 
>> So if a driver registered the slave MII bus itself, it may have
>> unregistered it itself as well, so checking ds->slave_mii_bus is OK
>> (assuming the driver correctly zero'ed it).
>> 
>> But is it necessary to check ds->drv->phy_read?
>
> It makes the code symmetrical to the register code, which makes the
> same check. My experience is that keeping the code symmetrical results
> in less surprises late on.
>
> One such surprise could be a driver that does not zero
> ds->slave_mii_bus. In fact, mv88e6xxx does not zero it. So we would
> get a double unregister happening. We also don't want the core
> unregistering it, since we have other cleanup work to do, we have an
> of_node_put() to call.

OK good for me, as long as it is intuitive for the driver
implementation.

Thanks,

Vivien

Re: BUG: net/netlink: KASAN: use-after-free in netlink_sock_destruct

[Cong Wang]

On Thu, May 26, 2016 at 8:06 AM, Eric Dumazet  wrote:
> On Thu, 2016-05-26 at 22:48 +0800, Baozeng Ding wrote:
>> Hi all,
>> I've got the following report use-after-free in netlink_sock_destruct while 
>> running syzkaller.
>> Unfortunately no reproducer.The kernel version is 4.6 (May 15, on commit 
>> 2dcd0af568b0cf583645c8a317dd12e344b1c72a). Thanks.
>>
>> ==
>> BUG: KASAN: use-after-free in kfree_skb+0x28c/0x310 at addr 880036c1179c
>> Read of size 4 by task syz-executor/21618
>> =
>> BUG skbuff_head_cache (Tainted: GW  ): kasan: bad access detected
>> -
>>
>> Disabling lock debugging due to kernel taint
>> INFO: Slab 0xeadb0400 objects=25 used=3 fp=0x880036c116c0 
>> flags=0x1fffc004080
>> INFO: Object 0x880036c11680 @offset=5760 fp=0x
>> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 
>> rel-1.8.2-0-g33fbe13 by qemu-project.org 04/01/2014
>>  0002 88006da07c40 8295f5f1 88003e0fc5c0
>>  880036c11680 eadb0400 880036c1 88006da07c70
>>  8171144d 88003e0fc5c0 eadb0400 880036c11680
>> Call Trace:
>>  [< inline >] __dump_stack /lib/dump_stack.c:15
>>  [] dump_stack+0xb3/0x112 /lib/dump_stack.c:51
>>  [] print_trailer+0x10d/0x190 /mm/slub.c:667
>>  [] object_err+0x2f/0x40 /mm/slub.c:674
>>  [< inline >] print_address_description /mm/kasan/report.c:179
>>  [] kasan_report_error+0x218/0x530 /mm/kasan/report.c:275
>>  [] ? debug_check_no_locks_freed+0x290/0x290 
>> /kernel/locking/lockdep.c:4212
>>  [< inline >] kasan_report /mm/kasan/report.c:297
>>  [] __asan_report_load4_noabort+0x3e/0x40 
>> /mm/kasan/report.c:317
>>  [< inline >] ? atomic_read /include/linux/compiler.h:222
>>  [] ? kfree_skb+0x28c/0x310 /net/core/skbuff.c:699
>>  [< inline >] atomic_read /include/linux/compiler.h:222
>>  [] kfree_skb+0x28c/0x310 /net/core/skbuff.c:699
>>  [] netlink_sock_destruct+0xeb/0x2b0 
>> /net/netlink/af_netlink.c:334
>>  [] ? __netlink_create+0x1d0/0x1d0 
>> /net/netlink/af_netlink.c:577
>>  [] sk_destruct+0x4a/0x4f0 /net/core/sock.c:1429
>>  [] __sk_free+0x57/0x200 /net/core/sock.c:1459
>>  [] sk_free+0x30/0x40 /net/core/sock.c:1470
>>  [< inline >] sock_put /include/net/sock.h:1506
>>  [] deferred_put_nlk_sk+0x34/0x40 
>> /net/netlink/af_netlink.c:652
>>  [< inline >] __rcu_reclaim /kernel/rcu/rcu.h:118
>>  [< inline >] rcu_do_batch /kernel/rcu/tree.c:2681
>>  [< inline >] invoke_rcu_callbacks /kernel/rcu/tree.c:2947
>>  [< inline >] __rcu_process_callbacks /kernel/rcu/tree.c:2914
>>  [] rcu_process_callbacks+0xa71/0x11d0 
>> /kernel/rcu/tree.c:2931
>>  [< inline >] ? __rcu_reclaim /kernel/rcu/rcu.h:108
>>  [< inline >] ? rcu_do_batch /kernel/rcu/tree.c:2681
>>  [< inline >] ? invoke_rcu_callbacks /kernel/rcu/tree.c:2947
>>  [< inline >] ? __rcu_process_callbacks /kernel/rcu/tree.c:2914
>>  [] ? rcu_process_callbacks+0xa1c/0x11d0 
>> /kernel/rcu/tree.c:2931
>>  [] ? __netlink_deliver_tap+0x7c0/0x7c0 
>> /net/netlink/af_netlink.c:204
>>  [] __do_softirq+0x22b/0x8da /kernel/softirq.c:273
>>  [< inline >] invoke_softirq /kernel/softirq.c:350
>>  [] irq_exit+0x15d/0x190 /kernel/softirq.c:391
>>  [< inline >] exiting_irq /./arch/x86/include/asm/apic.h:658
>>  [] smp_apic_timer_interrupt+0x7b/0xa0 
>> /arch/x86/kernel/apic/apic.c:932
>>  [] apic_timer_interrupt+0x8c/0xa0 
>> /arch/x86/entry/entry_64.S:454
>>  [< inline >] ? atomic_add_return 
>> /./arch/x86/include/asm/atomic.h:156
>>  [< inline >] ? kref_get /include/linux/kref.h:46
>>  [] ? klist_next+0x177/0x400 /lib/klist.c:393
>>  [< inline >] ? kref_get /include/linux/kref.h:46
>>  [] ? klist_next+0x168/0x400 /lib/klist.c:393
>>  [] class_dev_iter_next+0x8b/0xd0 /drivers/base/class.c:324
>>  [] ? tty_get_pgrp+0x80/0x80 /drivers/tty/tty_io.c:2525
>>  [] class_find_device+0x101/0x1c0 /drivers/base/class.c:428
>>  [] ? class_for_each_device+0x1d0/0x1d0 
>> /drivers/base/class.c:375
>>  [< inline >] tty_get_device /drivers/tty/tty_io.c:3139
>>  [] alloc_tty_struct+0x5fb/0x840 /drivers/tty/tty_io.c:3183
>>  [] ? do_SAK_work+0x20/0x20 /drivers/tty/tty_io.c:3112
>>  [] ? mutex_lock_interruptible_nested+0x980/0x980 ??:?
>>  [] tty_init_dev+0x78/0x4b0 /drivers/tty/tty_io.c:1532
>>  [< inline >] tty_open_by_driver /drivers/tty/tty_io.c:2065
>>  [] tty_open+0xd31/0x1050 /drivers/tty/tty_io.c:2113
>>  [] ? tty_init_dev+0x4b0/0x4b0 /drivers/tty/tty_io.c:1543
>>  [< inline >] ? spin_unlock /include/linux/spinlock.h:347
>>  [] ? chrdev_open+0xbf/0x4c0 /fs/char_dev.c:376
>>  [] ? tty_init_dev+0x4b0/0x4b0 /drivers/tty/tty_io.c:1543
>> 

Re: [PATCH] vxlan: Accept user specified MTU value when create new vxlan link

[Cong Wang]

On Thu, May 26, 2016 at 7:49 PM, Chen Haiquan  wrote:
> When create a new vxlan link, example:
>   ip link add vtap mtu 1440 type vxlan vni 1 dev eth0
>
> The argument "mtu" has no effect, because it is not set to conf->mtu. The
> default value is used in vxlan_dev_configure function.
>
> This problem was introduced by commit 0dfbdf4102b9 (vxlan: Factor out device
> configuration).
>
> Fixes: 0dfbdf4102b9 (vxlan: Factor out device configuration)
>
> Signed-off-by:  Chen Haiquan 
> ---
>  drivers/net/vxlan.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
> index 8ff30c3..f999db2 100644
> --- a/drivers/net/vxlan.c
> +++ b/drivers/net/vxlan.c
> @@ -3086,6 +3086,9 @@ static int vxlan_newlink(struct net *src_net, struct 
> net_device *dev,
> if (data[IFLA_VXLAN_REMCSUM_NOPARTIAL])
> conf.flags |= VXLAN_F_REMCSUM_NOPARTIAL;
>
> +   if (tb[IFLA_MTU])
> +   conf.mtu = nla_get_u32(tb[IFLA_MTU]);
> +

IFLA_MTU is already parsed in rtnl_create_link(),
so just use dev->mtu here? conf.mtu = dev->mtu?

Re: [PATCH net] sctp: sctp_diag should dump sctp socket type

[Xin Long]

On Thu, May 26, 2016 at 4:21 AM, Eric Dumazet  wrote:
> On Thu, 2016-05-26 at 03:14 +0800, Xin Long wrote:
>> Now we cannot distinguish that one sk is a udp or sctp style when
>> we use ss to dump sctp_info. it's necessary to dump it as well.
>>
>> For sctp_diag, ss support is not officially available, thus there
>> are no official users of this yet, so we can add this field in the
>> middle of sctp_info without breaking user API.
>>
>> Signed-off-by: Xin Long 
>> ---
>>  include/linux/sctp.h | 1 +
>>  net/sctp/socket.c| 1 +
>>  2 files changed, 2 insertions(+)
>>
>> diff --git a/include/linux/sctp.h b/include/linux/sctp.h
>> index dacb5e7..3a406af 100644
>> --- a/include/linux/sctp.h
>> +++ b/include/linux/sctp.h
>> @@ -761,6 +761,7 @@ struct sctp_info {
>>   __u32   sctpi_s_autoclose;
>>   __u32   sctpi_s_adaptation_ind;
>>   __u32   sctpi_s_pd_point;
>> + __u32   sctpi_s_type;
>
> Well, this is also adding a 4-byte padding at the end of the structure.
>
>
> Basically, because of the 8-byte alignment cause by the __u64 fields,
> adding a single __u32 adds a padding.
>
> Don't you have another u32 info you'd like to publish ?
>
these days, I was trying to find an right u32 one for here, but no good
one, so I will put a __u32   __reserved3; there.

sctp still have some feature haven't yet been implemented, maybe
after that, we will find a good one. :)


>
>

Re: [PATCH net] sctp: sctp_diag should dump sctp socket type

[Xin Long]

On Thu, May 26, 2016 at 3:24 AM, David Miller  wrote:
> From: Xin Long 
> Date: Thu, 26 May 2016 03:14:28 +0800
>
>> For sctp_diag, ss support is not officially available, thus there
>> are no official users of this yet, so we can add this field in the
>> middle of sctp_info without breaking user API.
>
> This is not what matters.
>
> What matters is if a released kernel went out with the existing API.
got you, I will move this field to the end of struct sctp_info.

Re: [RFC PATCH 12/16] dsa: Make mdio bus optional

[Andrew Lunn]

On Fri, May 27, 2016 at 10:55:45AM -0400, Vivien Didelot wrote:
> Hi Andrew,
> 
> Andrew Lunn  writes:
> 
> > -   mdiobus_unregister(ds->slave_mii_bus);
> > +   if (ds->slave_mii_bus && ds->drv->phy_read)
> > +   mdiobus_unregister(ds->slave_mii_bus);
> 
> So if a driver registered the slave MII bus itself, it may have
> unregistered it itself as well, so checking ds->slave_mii_bus is OK
> (assuming the driver correctly zero'ed it).
> 
> But is it necessary to check ds->drv->phy_read?

It makes the code symmetrical to the register code, which makes the
same check. My experience is that keeping the code symmetrical results
in less surprises late on.

One such surprise could be a driver that does not zero
ds->slave_mii_bus. In fact, mv88e6xxx does not zero it. So we would
get a double unregister happening. We also don't want the core
unregistering it, since we have other cleanup work to do, we have an
of_node_put() to call.

Andrew

[PATCH] wl3501_cs: avoid bogus gcc-6 warning

[Arnd Bergmann]

gcc-6 on x86 started warning about wl3501_get_encode when building
with -O2:

drivers/net/wireless/wl3501_cs.c: In function ‘wl3501_get_encode’:
drivers/net/wireless/wl3501_cs.c:1769:5: warning: ‘implemented’ may be used 
uninitialized in this function
drivers/net/wireless/wl3501_cs.c:1686:19: warning: ‘threshold’ may be used 
uninitialized in this function
drivers/net/wireless/wl3501_cs.c:1702:20: warning: ‘threshold’ may be used 
uninitialized in this function
drivers/net/wireless/wl3501_cs.c:1719:23: warning: ‘txpow’ may be used 
uninitialized in this function
drivers/net/wireless/wl3501_cs.c:1752:20: warning: ‘retry’ may be used 
uninitialized in this function
drivers/net/wireless/wl3501_cs.c:1806:25: warning: ‘pwr_state’ may be used 
uninitialized in this function
drivers/net/wireless/wl3501_cs.c:1383:24: warning: ‘value’ may be used 
uninitialized in this function

I could not figure out what exactly confuses gcc here, but splitting the
wl3501_get_mib_value function into two helps the compiler to figure out
that the variables are not actually used uninitialized, and makes it
slightly clearer to a human reader what the function actually does and
which parts of it are under the spinlock.

Signed-off-by: Arnd Bergmann 

diff --git a/drivers/net/wireless/wl3501_cs.c b/drivers/net/wireless/wl3501_cs.c
index 13fd734b61ec..82d94f83b6b4 100644
--- a/drivers/net/wireless/wl3501_cs.c
+++ b/drivers/net/wireless/wl3501_cs.c
@@ -378,8 +378,7 @@ static int wl3501_esbq_exec(struct wl3501_card *this, void 
*sig, int sig_size)
return rc;
 }
 
-static int wl3501_get_mib_value(struct wl3501_card *this, u8 index,
-   void *bf, int size)
+static int wl3501_request_mib(struct wl3501_card *this, u8 index, void *bf)
 {
struct wl3501_get_req sig = {
.sig_id = WL3501_SIG_GET_REQ,
@@ -395,20 +394,32 @@ static int wl3501_get_mib_value(struct wl3501_card *this, 
u8 index,
wl3501_set_to_wla(this, ptr, , sizeof(sig));
wl3501_esbq_req(this, );
this->sig_get_confirm.mib_status = 255;
-   spin_unlock_irqrestore(>lock, flags);
-   rc = wait_event_interruptible(this->wait,
-   this->sig_get_confirm.mib_status != 255);
-   if (!rc)
-   memcpy(bf, this->sig_get_confirm.mib_value,
-  size);
-   goto out;
+   rc = 0;
}
}
spin_unlock_irqrestore(>lock, flags);
-out:
+
return rc;
 }
 
+static int wl3501_get_mib_value(struct wl3501_card *this, u8 index,
+   void *bf, int size)
+{
+   int rc;
+
+   rc = wl3501_request_mib(this, index, bf);
+   if (rc)
+   return rc;
+
+   rc = wait_event_interruptible(this->wait,
+   this->sig_get_confirm.mib_status != 255);
+   if (rc)
+   return rc;
+
+   memcpy(bf, this->sig_get_confirm.mib_value, size);
+   return 0;
+}
+
 static int wl3501_pwr_mgmt(struct wl3501_card *this, int suspend)
 {
struct wl3501_pwr_mgmt_req sig = {

Re: [RFC PATCH 09/16] dsa: dsa: Split up creating/destroying of DSA and CPU ports

[Andrew Lunn]

On Fri, May 27, 2016 at 10:33:49AM -0400, Vivien Didelot wrote:
> Hi Andrew,
> 
> Andrew Lunn  writes:
> 
> > -static void dsa_switch_destroy(struct dsa_switch *ds)
> > +void dsa_cpu_dsa_destroy(struct device_node *port_dn)
> >  {
> > -   struct device_node *port_dn;
> > struct phy_device *phydev;
> > +
> > +   if (of_phy_is_fixed_link(port_dn)) {
> > +   phydev = of_phy_find_device(port_dn);
> > +   if (phydev) {
> > +   phy_device_free(phydev);
> > +   fixed_phy_unregister(phydev);
> > +   }
> > +   }
> > +}
> > +
> > +static void dsa_switch_destroy(struct dsa_switch *ds)
> > +{
> > int port;
> >  
> >  #ifdef CONFIG_NET_DSA_HWMON
> > @@ -445,17 +467,11 @@ static void dsa_switch_destroy(struct dsa_switch *ds)
> > dsa_slave_destroy(ds->ports[port].netdev);
> > }
> >  
> > -   /* Remove any fixed link PHYs */
> > +   /* Disable configuration of the CPU and DSA ports */
> > for (port = 0; port < DSA_MAX_PORTS; port++) {
> > -   port_dn = ds->ports[port].dn;
> > -   if (of_phy_is_fixed_link(port_dn)) {
> > -   phydev = of_phy_find_device(port_dn);
> > -   if (phydev) {
> > -   phy_device_free(phydev);
> > -   of_node_put(port_dn);
> 
> Why does dsa_cpu_dsa_destroy drop that of_node_put call?

The of node reference counting is broken. The DT maintainers actually
say not to care, the whole reference counting scheme is broken. Which
is a bit sad really. There was a discussion about this a couple of
months ago.

Anyway, the reference is taken in dsa_of_probe() as part of the
or_each_available_child_of_node(child, port). This reference has
nothing to do with the port being a fixed link or not. So freeing it
here is inappropriate. The correct place to free it would probably be
in dsa_of_remove.
 
> > -   fixed_phy_unregister(phydev);
> > -   }
> > -   }
> > +   if ((dsa_is_cpu_port(ds, port) || dsa_is_dsa_port(ds, port)))
> > +   continue;
> 
> Why do we skip DSA and CPU ports here? The previous code didn't.
> 
> > +   dsa_cpu_dsa_destroy(ds->ports[port].dn);

They are now destroyed by the newly added dsa_cpu_dsa_destroy().  I'm
making the code more symmetrical and easier to re-use. The inverse of
this function is dsa_switch_setup_one() and it also uses a helper
function to setup the dsa and cpu ports, dsa_cpu_dsa_setups().

 Andrew

Re: IPv6 extension header privileges

[Sowmini Varadhan]

On (05/27/16 11:53), Hannes Frederic Sowa wrote:
> > Thinking about this some more, the per option white list is a better
> > approach. If we allow an open ended mechanism for applications to
> > signal the network with arbitrary data (like user specified hbp
> > options would be), then use of that mechanism will inevitably
> > exploited by some authorities to force user to hand over private data
> > about their communications. It's better to not build in back doors to
> > security...
> 
> Sorry, Tom, can you try to explain again, I think I might not have
> understood you correctly.

yes, me too. Might help to discuss this by looking at an instance
of the type of option we are talking about. 

Usually hbh options are handled in the forwarding path (and thus
as unpopular as ip options, since they derail the packet to the 
slow path). Are you suggesting some type of AAA to vet the hbh 
option itself?

--Sowmini

Re: [PATCH RESEND 7/8] pipe: account to kmemcg

[Vladimir Davydov]

On Thu, May 26, 2016 at 07:15:49AM -0700, Eric Dumazet wrote:
> On Thu, 2016-05-26 at 16:59 +0300, Vladimir Davydov wrote:
> > On Thu, May 26, 2016 at 04:04:55PM +0900, Minchan Kim wrote:
> > > On Wed, May 25, 2016 at 01:30:11PM +0300, Vladimir Davydov wrote:
> > > > On Tue, May 24, 2016 at 01:04:33PM -0700, Eric Dumazet wrote:
> > > > > On Tue, 2016-05-24 at 19:13 +0300, Vladimir Davydov wrote:
> > > > > > On Tue, May 24, 2016 at 05:59:02AM -0700, Eric Dumazet wrote:
> > > > > > ...
> > > > > > > > +static int anon_pipe_buf_steal(struct pipe_inode_info *pipe,
> > > > > > > > +  struct pipe_buffer *buf)
> > > > > > > > +{
> > > > > > > > +   struct page *page = buf->page;
> > > > > > > > +
> > > > > > > > +   if (page_count(page) == 1) {
> > > > > > > 
> > > > > > > This looks racy : some cpu could have temporarily elevated page 
> > > > > > > count.
> > > > > > 
> > > > > > All pipe operations (pipe_buf_operations->get, ->release, ->steal) 
> > > > > > are
> > > > > > supposed to be called under pipe_lock. So, if we see a 
> > > > > > pipe_buffer->page
> > > > > > with refcount of 1 in ->steal, that means that we are the only its 
> > > > > > user
> > > > > > and it can't be spliced to another pipe.
> > > > > > 
> > > > > > In fact, I just copied the code from generic_pipe_buf_steal, adding
> > > > > > kmemcg related checks along the way, so it should be fine.
> > > > > 
> > > > > So you guarantee that no other cpu might have done
> > > > > get_page_unless_zero() right before this test ?
> > > > 
> > > > Each pipe_buffer holds a reference to its page. If we find page's
> > > > refcount to be 1 here, then it can be referenced only by our
> > > > pipe_buffer. And the refcount cannot be increased by a parallel thread,
> > > > because we hold pipe_lock, which rules out splice, and otherwise it's
> > > > impossible to reach the page as it is not on lru. That said, I think I
> > > > guarantee that this should be safe.
> > > 
> > > I don't know kmemcg internal and pipe stuff so my comment might be
> > > totally crap.
> > > 
> > > No one cannot guarantee any CPU cannot held a reference of a page.
> > > Look at get_page_unless_zero usecases.
> > > 
> > > 1. balloon_page_isolate
> > > 
> > > It can hold a reference in random page and then verify the page
> > > is balloon page. Otherwise, just put.
> > > 
> > > 2. page_idle_get_page
> > > 
> > > It has PageLRU check but it's racy so it can hold a reference
> > > of randome page and then verify within zone->lru_lock. If it's
> > > not LRU page, just put.
> > 
> > Well, I see your concern now - even if a page is not on lru and we
> > locked all structs pointing to it, it can always get accessed by pfn in
> > a completely unrelated thread, like in examples you gave above. That's a
> > fair point.
> > 
> > However, I still think that it's OK in case of pipe buffers. What can
> > happen if somebody takes a transient reference to a pipe buffer page? At
> > worst, we'll see page_count > 1 due to temporary ref and abort stealing,
> > falling back on copying instead. That's OK, because stealing is not
> > guaranteed. Can a function that takes a transient ref to page by pfn
> > mistakenly assume that this is a page it's interested in? I don't think
> > so, because this page has no marks on it except special _mapcount value,
> > which should only be set on kmemcg pages.
> 
> Well, all this information deserve to be in the changelog.
> 
> Maybe in 6 months, this will be incredibly useful for bug hunting.
> 
> pipes can be used to exchange data (or pages) between processes in
> different domains.
> 
> If kmemcg is not precise, this could be used by some attackers to force
> some processes to consume all their budget and eventually not be able to
> allocate new pages.
> 

Here goes the patch with updated change log.
---
From: Vladimir Davydov 
Subject: [PATCH] pipe: account to kmemcg

Pipes can consume a significant amount of system memory, hence they
should be accounted to kmemcg.

This patch marks pipe_inode_info and anonymous pipe buffer page
allocations as __GFP_ACCOUNT so that they would be charged to kmemcg.
Note, since a pipe buffer page can be "stolen" and get reused for other
purposes, including mapping to userspace, we clear PageKmemcg thus
resetting page->_mapcount and uncharge it in anon_pipe_buf_steal, which
is introduced by this patch.

A note regarding anon_pipe_buf_steal implementation. We allow to steal
the page if its ref count equals 1. It looks racy, but it is correct for
anonymous pipe buffer pages, because:

 - We lock out all other pipe users, because ->steal is called with
   pipe_lock held, so the page can't be spliced to another pipe from
   under us.

 - The page is not on LRU and it never was.

 - Thus a parallel thread can access it only by PFN. Although this is
   quite possible (e.g. see page_idle_get_page and balloon_page_isolate)
   this is not dangerous, because all such functions do is 

Re: Fwd: [PATCH 1/1] team: remove unnecessary lock

[Jiri Pirko]

Fri, May 27, 2016 at 04:25:29AM CEST, zyjzyj2...@gmail.com wrote:
>Hi, Jiri
>
>I delved into the source code of team. And I found that the function
>team_add_slave actually works in the protection of rtnl lock. So in my
>humble opinion, it is not necessary to use the mutex lock to protect
>team_port_add again.
>
>And I made tests in ubuntu 16.04 server, without the mutex lock, the
>team_add_slave can also work well.
>The steps:
>
>1.
>nmcli con add type team con-name team0 ifname team0 config
>'{"runner":{"name":"activebackup"}}'
>2.
>nmcli con add type team-slave con-name team0-port1 ifname eno1636
>master team0
>nmcli con add type team-slave con-name team0-port2 ifname eno33554960
>master team0
>
>The above test can work well without the mutex lock. So I made the
>following patch, please comment on this patch.

Well, team lock could be replaced by rtnl and removed. Removing just
this tiny usage solves nothing. I decided to use team-specific lock to
do not overload rtnl at the beginning.


>
>Thanks a lot.
>Zhu Yanjun
>
>From: Zhu Yanjun
>
>The function team_add_slave works in the context of the rtnl lock.
>It is not necessary to use the mutex lock since the rtnl lock is
>enough.
>
>Signed-off-by: Zhu Yanjun 
>---
> drivers/net/team/team.c | 2 --
> 1 file changed, 2 deletions(-)
>
>diff --git a/drivers/net/team/team.c b/drivers/net/team/team.c
>index a0f64cb..ad84069 100644
>--- a/drivers/net/team/team.c
>+++ b/drivers/net/team/team.c
>@@ -1936,9 +1936,7 @@ static int team_add_slave(struct net_device *dev,
>struct net_device *port_dev)
>struct team *team = netdev_priv(dev);
>int err;
>
>-   mutex_lock(>lock);
>err = team_port_add(team, port_dev);
>-   mutex_unlock(>lock);
>return err;
> }
>
>--
>1.9.1

Re: [RFC PATCH 12/16] dsa: Make mdio bus optional

[Vivien Didelot]

Hi Andrew,

Andrew Lunn  writes:

> - mdiobus_unregister(ds->slave_mii_bus);
> + if (ds->slave_mii_bus && ds->drv->phy_read)
> + mdiobus_unregister(ds->slave_mii_bus);

So if a driver registered the slave MII bus itself, it may have
unregistered it itself as well, so checking ds->slave_mii_bus is OK
(assuming the driver correctly zero'ed it).

But is it necessary to check ds->drv->phy_read?

Thanks,

Vivien

Re: [RFC PATCH 09/16] dsa: dsa: Split up creating/destroying of DSA and CPU ports

[Vivien Didelot]

Hi Andrew,

Andrew Lunn  writes:

> -static void dsa_switch_destroy(struct dsa_switch *ds)
> +void dsa_cpu_dsa_destroy(struct device_node *port_dn)
>  {
> - struct device_node *port_dn;
>   struct phy_device *phydev;
> +
> + if (of_phy_is_fixed_link(port_dn)) {
> + phydev = of_phy_find_device(port_dn);
> + if (phydev) {
> + phy_device_free(phydev);
> + fixed_phy_unregister(phydev);
> + }
> + }
> +}
> +
> +static void dsa_switch_destroy(struct dsa_switch *ds)
> +{
>   int port;
>  
>  #ifdef CONFIG_NET_DSA_HWMON
> @@ -445,17 +467,11 @@ static void dsa_switch_destroy(struct dsa_switch *ds)
>   dsa_slave_destroy(ds->ports[port].netdev);
>   }
>  
> - /* Remove any fixed link PHYs */
> + /* Disable configuration of the CPU and DSA ports */
>   for (port = 0; port < DSA_MAX_PORTS; port++) {
> - port_dn = ds->ports[port].dn;
> - if (of_phy_is_fixed_link(port_dn)) {
> - phydev = of_phy_find_device(port_dn);
> - if (phydev) {
> - phy_device_free(phydev);
> - of_node_put(port_dn);

Why does dsa_cpu_dsa_destroy drop that of_node_put call?

> - fixed_phy_unregister(phydev);
> - }
> - }
> + if ((dsa_is_cpu_port(ds, port) || dsa_is_dsa_port(ds, port)))
> + continue;

Why do we skip DSA and CPU ports here? The previous code didn't.

> + dsa_cpu_dsa_destroy(ds->ports[port].dn);

Thanks,

Vivien

[PATCH 2/8] mwifiex: propagate sdio_enable_func() errno code in mwifiex_sdio_probe()

[Javier Martinez Canillas]

If the sdio_enable_func() function fails on .probe, the -EIO errno code
is always returned but that could make more difficult to debug and find
the cause of why the function actually failed.

Since the driver/device core prints the value returned by .probe in its
error message propagate what was returned by sdio_enable_func() at fail.

Signed-off-by: Javier Martinez Canillas 
---

 drivers/net/wireless/marvell/mwifiex/sdio.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/marvell/mwifiex/sdio.c 
b/drivers/net/wireless/marvell/mwifiex/sdio.c
index 285b1b68f7e9..ab64507c84e1 100644
--- a/drivers/net/wireless/marvell/mwifiex/sdio.c
+++ b/drivers/net/wireless/marvell/mwifiex/sdio.c
@@ -184,7 +184,7 @@ mwifiex_sdio_probe(struct sdio_func *func, const struct 
sdio_device_id *id)
if (ret) {
pr_err("%s: failed to enable function\n", __func__);
kfree(card);
-   return -EIO;
+   return ret;
}
 
/* device tree node parsing and platform specific configuration*/
-- 
2.5.5

[PATCH 3/8] mwifiex: propagate mwifiex_add_card() errno code in mwifiex_sdio_probe()

[Javier Martinez Canillas]

There's only a check if mwifiex_add_card() returned a nonzero value, but
the actual error code is neither stored nor propagated to the caller. So
instead of always returning -1 (which is -EPERM and not a suitable errno
code in this case), propagate the value returned by mwifiex_add_card().

Patch also removes the assignment of sdio_disable_func() returned value
since it was overwritten anyways and what matters is to know the error
value returned by the first function that failed.

Signed-off-by: Javier Martinez Canillas 
---

 drivers/net/wireless/marvell/mwifiex/sdio.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/net/wireless/marvell/mwifiex/sdio.c 
b/drivers/net/wireless/marvell/mwifiex/sdio.c
index ab64507c84e1..81003fbe5025 100644
--- a/drivers/net/wireless/marvell/mwifiex/sdio.c
+++ b/drivers/net/wireless/marvell/mwifiex/sdio.c
@@ -191,14 +191,14 @@ mwifiex_sdio_probe(struct sdio_func *func, const struct 
sdio_device_id *id)
if (func->dev.of_node)
mwifiex_sdio_probe_of(>dev, card);
 
-   if (mwifiex_add_card(card, _remove_card_sem, _ops,
-MWIFIEX_SDIO)) {
+   ret = mwifiex_add_card(card, _remove_card_sem, _ops,
+  MWIFIEX_SDIO);
+   if (ret) {
pr_err("%s: add card failed\n", __func__);
kfree(card);
sdio_claim_host(func);
-   ret = sdio_disable_func(func);
+   sdio_disable_func(func);
sdio_release_host(func);
-   ret = -1;
}
 
return ret;
-- 
2.5.5

[PATCH 1/8] mwifiex: only call mwifiex_sdio_probe_of() if dev has an OF node

[Javier Martinez Canillas]

SDIO is an auto enumerable bus so the SDIO devices are matched using the
sdio_device_id table and not using compatible strings from a OF id table.

However, commit ce4f6f0c353b ("mwifiex: add platform specific wakeup
interrupt support") allowed to match nodes defined as child of the SDIO
host controller in the probe function using a compatible string to setup
platform specific parameters in the DT.

The problem is that the OF parse function is always called regardless if
the SDIO dev has an OF node associated or not, and prints an error if it
is not found. So, on a platform that doesn't have a node for a SDIO dev,
the following misleading error message will be printed:

[  12.480042] mwifiex_sdio mmc2:0001:1: sdio platform data not available

Signed-off-by: Javier Martinez Canillas 
---

 drivers/net/wireless/marvell/mwifiex/sdio.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/net/wireless/marvell/mwifiex/sdio.c 
b/drivers/net/wireless/marvell/mwifiex/sdio.c
index bdc51ffd43ec..285b1b68f7e9 100644
--- a/drivers/net/wireless/marvell/mwifiex/sdio.c
+++ b/drivers/net/wireless/marvell/mwifiex/sdio.c
@@ -102,8 +102,7 @@ static int mwifiex_sdio_probe_of(struct device *dev, struct 
sdio_mmc_card *card)
struct mwifiex_plt_wake_cfg *cfg;
int ret;
 
-   if (!dev->of_node ||
-   !of_match_node(mwifiex_sdio_of_match_table, dev->of_node)) {
+   if (!of_match_node(mwifiex_sdio_of_match_table, dev->of_node)) {
dev_err(dev, "sdio platform data not available\n");
return -1;
}
@@ -189,7 +188,8 @@ mwifiex_sdio_probe(struct sdio_func *func, const struct 
sdio_device_id *id)
}
 
/* device tree node parsing and platform specific configuration*/
-   mwifiex_sdio_probe_of(>dev, card);
+   if (func->dev.of_node)
+   mwifiex_sdio_probe_of(>dev, card);
 
if (mwifiex_add_card(card, _remove_card_sem, _ops,
 MWIFIEX_SDIO)) {
-- 
2.5.5

[PATCH 0/8] mwifiex: Fix some error handling issues in mwifiex_sdio_probe() function

[Javier Martinez Canillas]

Hello,

While booting a system with a mwifiex WiFi card, I noticed the following
missleading error message:

[  12.480042] mwifiex_sdio mmc2:0001:1: sdio platform data not available

This error only applies to platforms that define a child node for the SDIO
device, but it's currently shown even in platforms that don't have a child
node defined.

So this series fixes this issue and others I found in the .probe function
(mostly related to error handling and the error path) while looking at it.

Best regards,
Javier


Javier Martinez Canillas (8):
  mwifiex: only call mwifiex_sdio_probe_of() if dev has an OF node
  mwifiex: propagate sdio_enable_func() errno code in
mwifiex_sdio_probe()
  mwifiex: propagate mwifiex_add_card() errno code in
mwifiex_sdio_probe()
  mwifiex: consolidate mwifiex_sdio_probe() error paths
  mwifiex: use dev_err() instead of pr_err() in mwifiex_sdio_probe()
  mwifiex: check if mwifiex_sdio_probe_of() fails and return error
  mwifiex: don't print an error if an optional DT property is missing
  mwifiex: use better message and error code when OF node doesn't match

 drivers/net/wireless/marvell/mwifiex/sdio.c | 46 ++---
 1 file changed, 28 insertions(+), 18 deletions(-)

-- 
2.5.5

[PATCH 4/8] mwifiex: consolidate mwifiex_sdio_probe() error paths

[Javier Martinez Canillas]

Instead of duplicating part of the cleanups needed in case of an error
in .probe callback, have a single error path and use goto labels as is
common practice in the kernel.

This also has the nice side effect that the cleanup operations are made
in the inverse order of their counterparts, which was not the case for
the mwifiex_add_card() error path.

Signed-off-by: Javier Martinez Canillas 
---

 drivers/net/wireless/marvell/mwifiex/sdio.c | 17 +++--
 1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/drivers/net/wireless/marvell/mwifiex/sdio.c 
b/drivers/net/wireless/marvell/mwifiex/sdio.c
index 81003fbe5025..7aeee88b858f 100644
--- a/drivers/net/wireless/marvell/mwifiex/sdio.c
+++ b/drivers/net/wireless/marvell/mwifiex/sdio.c
@@ -183,8 +183,7 @@ mwifiex_sdio_probe(struct sdio_func *func, const struct 
sdio_device_id *id)
 
if (ret) {
pr_err("%s: failed to enable function\n", __func__);
-   kfree(card);
-   return ret;
+   goto err_free;
}
 
/* device tree node parsing and platform specific configuration*/
@@ -195,12 +194,18 @@ mwifiex_sdio_probe(struct sdio_func *func, const struct 
sdio_device_id *id)
   MWIFIEX_SDIO);
if (ret) {
pr_err("%s: add card failed\n", __func__);
-   kfree(card);
-   sdio_claim_host(func);
-   sdio_disable_func(func);
-   sdio_release_host(func);
+   goto err_disable;
}
 
+   return 0;
+
+err_disable:
+   sdio_claim_host(func);
+   sdio_disable_func(func);
+   sdio_release_host(func);
+err_free:
+   kfree(card);
+
return ret;
 }
 
-- 
2.5.5

[PATCH 8/8] mwifiex: use better message and error code when OF node doesn't match

[Javier Martinez Canillas]

The Documentation/devicetree/bindings/net/wireless/marvell-sd8xxx.txt DT
binding document lists the possible compatible strings that a SDIO child
node can have, so the driver checks if the defined in the node matches.

But the error message when that's not the case is misleading, so change
for one that makes clear what the error really is. Also, returning a -1
as errno code is not correct since that's -EPERM. A -EINVAL seems to be
a more appropriate one.

Signed-off-by: Javier Martinez Canillas 

---

 drivers/net/wireless/marvell/mwifiex/sdio.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/marvell/mwifiex/sdio.c 
b/drivers/net/wireless/marvell/mwifiex/sdio.c
index 8b3292eaecb2..e6d56be04e08 100644
--- a/drivers/net/wireless/marvell/mwifiex/sdio.c
+++ b/drivers/net/wireless/marvell/mwifiex/sdio.c
@@ -103,8 +103,8 @@ static int mwifiex_sdio_probe_of(struct device *dev, struct 
sdio_mmc_card *card)
int ret;
 
if (!of_match_node(mwifiex_sdio_of_match_table, dev->of_node)) {
-   dev_err(dev, "sdio platform data not available\n");
-   return -1;
+   dev_err(dev, "required compatible string missing\n");
+   return -EINVAL;
}
 
card->plt_of_node = dev->of_node;
-- 
2.5.5

[PATCH 5/8] mwifiex: use dev_err() instead of pr_err() in mwifiex_sdio_probe()

[Javier Martinez Canillas]

It's better to have the device name prefixed in the error message.

Signed-off-by: Javier Martinez Canillas 
---

 drivers/net/wireless/marvell/mwifiex/sdio.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/marvell/mwifiex/sdio.c 
b/drivers/net/wireless/marvell/mwifiex/sdio.c
index 7aeee88b858f..1ffbb972318f 100644
--- a/drivers/net/wireless/marvell/mwifiex/sdio.c
+++ b/drivers/net/wireless/marvell/mwifiex/sdio.c
@@ -182,7 +182,7 @@ mwifiex_sdio_probe(struct sdio_func *func, const struct 
sdio_device_id *id)
sdio_release_host(func);
 
if (ret) {
-   pr_err("%s: failed to enable function\n", __func__);
+   dev_err(>dev, "failed to enable function\n");
goto err_free;
}
 
@@ -193,7 +193,7 @@ mwifiex_sdio_probe(struct sdio_func *func, const struct 
sdio_device_id *id)
ret = mwifiex_add_card(card, _remove_card_sem, _ops,
   MWIFIEX_SDIO);
if (ret) {
-   pr_err("%s: add card failed\n", __func__);
+   dev_err(>dev, "add card failed\n");
goto err_disable;
}
 
-- 
2.5.5

[PATCH 7/8] mwifiex: don't print an error if an optional DT property is missing

[Javier Martinez Canillas]

The Documentation/devicetree/bindings/net/wireless/marvell-sd8xxx.txt DT
binding document say that the "interrupts" property in the child node is
optional. So the property being missed shouldn't be treated as an error.

Signed-off-by: Javier Martinez Canillas 
---

 drivers/net/wireless/marvell/mwifiex/sdio.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/marvell/mwifiex/sdio.c 
b/drivers/net/wireless/marvell/mwifiex/sdio.c
index 1c17e624547a..8b3292eaecb2 100644
--- a/drivers/net/wireless/marvell/mwifiex/sdio.c
+++ b/drivers/net/wireless/marvell/mwifiex/sdio.c
@@ -114,7 +114,7 @@ static int mwifiex_sdio_probe_of(struct device *dev, struct 
sdio_mmc_card *card)
if (cfg && card->plt_of_node) {
cfg->irq_wifi = irq_of_parse_and_map(card->plt_of_node, 0);
if (!cfg->irq_wifi) {
-   dev_err(dev,
+   dev_dbg(dev,
"fail to parse irq_wifi from device tree\n");
} else {
ret = devm_request_irq(dev, cfg->irq_wifi,
-- 
2.5.5

[PATCH 6/8] mwifiex: check if mwifiex_sdio_probe_of() fails and return error

[Javier Martinez Canillas]

The function can fail so the returned value should be checked
and the error propagated to the caller in case of a failure.

Signed-off-by: Javier Martinez Canillas 
---

 drivers/net/wireless/marvell/mwifiex/sdio.c | 9 +++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/marvell/mwifiex/sdio.c 
b/drivers/net/wireless/marvell/mwifiex/sdio.c
index 1ffbb972318f..1c17e624547a 100644
--- a/drivers/net/wireless/marvell/mwifiex/sdio.c
+++ b/drivers/net/wireless/marvell/mwifiex/sdio.c
@@ -187,8 +187,13 @@ mwifiex_sdio_probe(struct sdio_func *func, const struct 
sdio_device_id *id)
}
 
/* device tree node parsing and platform specific configuration*/
-   if (func->dev.of_node)
-   mwifiex_sdio_probe_of(>dev, card);
+   if (func->dev.of_node) {
+   ret = mwifiex_sdio_probe_of(>dev, card);
+   if (ret) {
+   dev_err(>dev, "SDIO dt node parse failed\n");
+   goto err_disable;
+   }
+   }
 
ret = mwifiex_add_card(card, _remove_card_sem, _ops,
   MWIFIEX_SDIO);
-- 
2.5.5

Re: [RFC 11/12] 6lowpan: add support for getting short address

[Stefan Schmidt]

Hello.

On 27/05/16 13:03, Alexander Aring wrote:

Hi,

On 05/27/2016 12:05 PM, Stefan Schmidt wrote:

Hello.

On 23/05/16 21:22, Alexander Aring wrote:

In case of sending RA messages we need some way to get the short address
from an 802.15.4 6LoWPAN interface. This patch will add a temporary
debugfs entry for experimental userspace api.

Signed-off-by: Alexander Aring
---
   net/6lowpan/debugfs.c | 35 +++
   1 file changed, 35 insertions(+)

diff --git a/net/6lowpan/debugfs.c b/net/6lowpan/debugfs.c
index acbaa3d..638ae59 100644
--- a/net/6lowpan/debugfs.c
+++ b/net/6lowpan/debugfs.c
@@ -245,6 +245,37 @@ static const struct file_operations lowpan_context_fops = {
   .release= single_release,
   };
   +static int lowpan_short_addr_get(void *data, u64 *val)
+{
+struct wpan_dev *wdev = data;
+
+rtnl_lock();
+*val = le16_to_cpu(wdev->short_addr);
+rtnl_unlock();
+
+return 0;
+}
+
+DEFINE_SIMPLE_ATTRIBUTE(lowpan_short_addr_fops, lowpan_short_addr_get,
+NULL, "0x%04llx\n");
+
+static int lowpan_dev_debugfs_802154_init(const struct net_device *dev,
+  struct lowpan_dev *ldev)
+{
+struct dentry *dentry;
+
+if (!lowpan_is_ll(dev, LOWPAN_LLTYPE_IEEE802154))
+return 0;
+
+dentry = debugfs_create_file("short_addr", 0444, ldev->iface_debugfs,
+ lowpan_802154_dev(dev)->wdev->ieee802154_ptr,
+ _short_addr_fops);
+if (!dentry)
+return -EINVAL;
+
+return 0;
+}
+
   int lowpan_dev_debugfs_init(struct net_device *dev)
   {
   struct lowpan_dev *ldev = lowpan_dev(dev);
@@ -272,6 +303,10 @@ int lowpan_dev_debugfs_init(struct net_device *dev)
   goto remove_root;
   }
   +ret = lowpan_dev_debugfs_802154_init(dev, ldev);
+if (ret < 0)
+goto remove_root;
+
   return 0;
 remove_root:

Reviewed-by: Stefan Schmidt


grml, I changed this patch now to have an 802154/short_addr directory
inside the root of 6lowpan debugfs entry.

This is because Michael Richardson wrote that other L2 has also
short_addr (any kind of second L2 address type), see [0].


The question is if we ever will have support for them in Linux. But I 
agree that it might be better to have an architecture that would fit for 
those cases as well.



Each of them has their own "constraints" e.g. 802.15.4 2-byte length, or
sometimes 1-byte. This constraint depends on L2, so I added 802154
subdir.


OK

regards
Stefan Schmidt

Re: [RFC 07/12] addrconf: put prefix address add in an own function

[Stefan Schmidt]

Hello.

On 27/05/16 13:41, Alexander Aring wrote:

Hi,

On 05/27/2016 11:45 AM, Stefan Schmidt wrote:

Hello.


sorry, again I will change something on this patch which is buggy.


On 23/05/16 21:22, Alexander Aring wrote:

This patch moves the functionality to add a RA PIO prefix generated
address in an own function. This move prepares to add a hook for
adding a second address for a second link-layer address. E.g. short
address for 802.15.4 6LoWPAN.

Cc: David S. Miller
Cc: Alexey Kuznetsov
Cc: James Morris
Cc: Hideaki YOSHIFUJI
Cc: Patrick McHardy
Signed-off-by: Alexander Aring
---
   net/ipv6/addrconf.c | 191 

   1 file changed, 102 insertions(+), 89 deletions(-)

diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index beaad49..393cdbf 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -2333,6 +2333,104 @@ static bool is_addr_mode_generate_stable(struct 
inet6_dev *idev)
  idev->addr_gen_mode == IN6_ADDR_GEN_MODE_RANDOM;
   }
   +static void addrconf_prefix_rcv_add_addr(struct net *net,

will change to return int here to indicates errors.


+ struct net_device *dev,
+ const struct prefix_info *pinfo,
+ struct inet6_dev *in6_dev,
+ const struct in6_addr *addr,
+ int addr_type, u32 addr_flags,
+ bool sllao, bool tokenized,
+ __u32 valid_lft, u32 prefered_lft)
+{
+struct inet6_ifaddr *ifp = ipv6_get_ifaddr(net, addr, dev, 1);
+int create = 0, update_lft = 0;
+
+if (!ifp && valid_lft) {
+int max_addresses = in6_dev->cnf.max_addresses;
+
+#ifdef CONFIG_IPV6_OPTIMISTIC_DAD
+if (in6_dev->cnf.optimistic_dad &&
+!net->ipv6.devconf_all->forwarding && sllao)
+addr_flags |= IFA_F_OPTIMISTIC;
+#endif
+
+/* Do not allow to create too much of autoconfigured
+ * addresses; this would be too easy way to crash kernel.
+ */
+if (!max_addresses ||
+ipv6_count_addresses(in6_dev) < max_addresses)
+ifp = ipv6_add_addr(in6_dev, addr, NULL,
+pinfo->prefix_len,
+addr_type_ADDR_SCOPE_MASK,
+addr_flags, valid_lft,
+prefered_lft);
+
+if (IS_ERR_OR_NULL(ifp)) {
+in6_dev_put(in6_dev);
+return;

return -1; and remove in6_dev_put stuff.


+}
+
+update_lft = 0;
+create = 1;
+spin_lock_bh(>lock);
+ifp->flags |= IFA_F_MANAGETEMPADDR;
+ifp->cstamp = jiffies;
+ifp->tokenized = tokenized;
+spin_unlock_bh(>lock);
+addrconf_dad_start(ifp);
+}
+
+if (ifp) {
+u32 flags;
+unsigned long now;
+u32 stored_lft;
+
+/* update lifetime (RFC2462 5.5.3 e) */
+spin_lock_bh(>lock);
+now = jiffies;
+if (ifp->valid_lft > (now - ifp->tstamp) / HZ)
+stored_lft = ifp->valid_lft - (now - ifp->tstamp) / HZ;
+else
+stored_lft = 0;
+if (!update_lft && !create && stored_lft) {
+const u32 minimum_lft = min_t(u32,
+stored_lft, MIN_VALID_LIFETIME);
+valid_lft = max(valid_lft, minimum_lft);
+
+/* RFC4862 Section 5.5.3e:
+ * "Note that the preferred lifetime of the
+ *  corresponding address is always reset to
+ *  the Preferred Lifetime in the received
+ *  Prefix Information option, regardless of
+ *  whether the valid lifetime is also reset or
+ *  ignored."
+ *
+ * So we should always update prefered_lft here.
+ */
+update_lft = 1;
+}
+
+if (update_lft) {
+ifp->valid_lft = valid_lft;
+ifp->prefered_lft = prefered_lft;
+ifp->tstamp = now;
+flags = ifp->flags;
+ifp->flags &= ~IFA_F_DEPRECATED;
+spin_unlock_bh(>lock);
+
+if (!(flags_F_TENTATIVE))
+ipv6_ifa_notify(0, ifp);
+} else
+spin_unlock_bh(>lock);
+
+manage_tempaddrs(in6_dev, ifp, valid_lft, prefered_lft,
+ create, now);
+
+in6_ifa_put(ifp);
+addrconf_verify();
+}
+}
+
   void addrconf_prefix_rcv(struct net_device *dev, u8 *opt, int len, bool 
sllao)
   {
   struct prefix_info *pinfo;
@@ -2432,9 +2530,7 @@ void addrconf_prefix_rcv(struct net_device *dev, u8 *opt, 
int len, bool sllao)
   /* Try to figure out our local address for this prefix */
 if (pinfo->autoconf && in6_dev->cnf.autoconf) {
-struct inet6_ifaddr *ifp;
   struct in6_addr addr;
-int create = 0, update_lft = 0;
  

[PATCH] iwlwifi: mvm: avoid harmless -Wmaybe-uninialized warning

[Arnd Bergmann]

gcc is apparently unablel to track the state of the local 'resp_v2'
variable across the kzalloc() function, and warns about the response
variable being used without an initialization:

drivers/net/wireless/intel/iwlwifi/mvm/nvm.c: In function ‘iwl_mvm_update_mcc’:
drivers/net/wireless/intel/iwlwifi/mvm/nvm.c:727:36: warning: ‘mcc_resp_v1’ may 
be used uninitialized in this function [-Wmaybe-uninitialized]
   resp_cp->n_channels = mcc_resp_v1->n_channels;
drivers/net/wireless/intel/iwlwifi/mvm/nvm.c:721:3: warning: ‘mcc_resp’ may be 
used uninitialized in this function [-Wmaybe-uninitialized]
   memcpy(resp_cp, mcc_resp, resp_len);

The warning showed up in x86 allmodconfig after my patch to
unhide -Wmaybe-uninitialized warnings by default was merged,
though it always existed in randconfig builds. I did not
catch the warning earlier because I was testing on ARM, which
never produced the warning.

This rearranges the code in a way that improves readability for
both humans and the compiler, and that avoids the warning.

Signed-off-by: Arnd Bergmann 
Fixes: 6fa52430f0b3 ("iwlwifi: mvm: change mcc update API")
---
 drivers/net/wireless/intel/iwlwifi/mvm/nvm.c | 41 ++--
 1 file changed, 21 insertions(+), 20 deletions(-)

diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/nvm.c 
b/drivers/net/wireless/intel/iwlwifi/mvm/nvm.c
index 25a98401a64f..0551a4bb163c 100644
--- a/drivers/net/wireless/intel/iwlwifi/mvm/nvm.c
+++ b/drivers/net/wireless/intel/iwlwifi/mvm/nvm.c
@@ -667,8 +667,7 @@ iwl_mvm_update_mcc(struct iwl_mvm *mvm, const char *alpha2,
.mcc = cpu_to_le16(alpha2[0] << 8 | alpha2[1]),
.source_id = (u8)src_id,
};
-   struct iwl_mcc_update_resp *mcc_resp, *resp_cp = NULL;
-   struct iwl_mcc_update_resp_v1 *mcc_resp_v1 = NULL;
+   struct iwl_mcc_update_resp *resp_cp;
struct iwl_rx_packet *pkt;
struct iwl_host_cmd cmd = {
.id = MCC_UPDATE_CMD,
@@ -701,34 +700,36 @@ iwl_mvm_update_mcc(struct iwl_mvm *mvm, const char 
*alpha2,
 
/* Extract MCC response */
if (resp_v2) {
-   mcc_resp = (void *)pkt->data;
+   struct iwl_mcc_update_resp *mcc_resp = (void *)pkt->data;
+
n_channels =  __le32_to_cpu(mcc_resp->n_channels);
+   resp_len = sizeof(struct iwl_mcc_update_resp) +
+  n_channels * sizeof(__le32);
+   resp_cp = kmemdup(mcc_resp, resp_len, GFP_KERNEL);
} else {
-   mcc_resp_v1 = (void *)pkt->data;
+   struct iwl_mcc_update_resp_v1 *mcc_resp_v1 = (void *)pkt->data;
+
n_channels =  __le32_to_cpu(mcc_resp_v1->n_channels);
+   resp_len = sizeof(struct iwl_mcc_update_resp) +
+  n_channels * sizeof(__le32);
+   resp_cp = kzalloc(resp_len, GFP_KERNEL);
+
+   if (resp_cp) {
+   resp_cp->status = mcc_resp_v1->status;
+   resp_cp->mcc = mcc_resp_v1->mcc;
+   resp_cp->cap = mcc_resp_v1->cap;
+   resp_cp->source_id = mcc_resp_v1->source_id;
+   resp_cp->n_channels = mcc_resp_v1->n_channels;
+   memcpy(resp_cp->channels, mcc_resp_v1->channels,
+  n_channels * sizeof(__le32));
+   }
}
 
-   resp_len = sizeof(struct iwl_mcc_update_resp) + n_channels *
-   sizeof(__le32);
-
-   resp_cp = kzalloc(resp_len, GFP_KERNEL);
if (!resp_cp) {
ret = -ENOMEM;
goto exit;
}
 
-   if (resp_v2) {
-   memcpy(resp_cp, mcc_resp, resp_len);
-   } else {
-   resp_cp->status = mcc_resp_v1->status;
-   resp_cp->mcc = mcc_resp_v1->mcc;
-   resp_cp->cap = mcc_resp_v1->cap;
-   resp_cp->source_id = mcc_resp_v1->source_id;
-   resp_cp->n_channels = mcc_resp_v1->n_channels;
-   memcpy(resp_cp->channels, mcc_resp_v1->channels,
-  n_channels * sizeof(__le32));
-   }
-
status = le32_to_cpu(resp_cp->status);
 
mcc = le16_to_cpu(resp_cp->mcc);
-- 
2.7.0

Re: [RFC 07/12] addrconf: put prefix address add in an own function

[Alexander Aring]

Hi,

On 05/27/2016 11:45 AM, Stefan Schmidt wrote:
> Hello.
> 

sorry, again I will change something on this patch which is buggy.

> On 23/05/16 21:22, Alexander Aring wrote:
>> This patch moves the functionality to add a RA PIO prefix generated
>> address in an own function. This move prepares to add a hook for
>> adding a second address for a second link-layer address. E.g. short
>> address for 802.15.4 6LoWPAN.
>>
>> Cc: David S. Miller
>> Cc: Alexey Kuznetsov
>> Cc: James Morris
>> Cc: Hideaki YOSHIFUJI
>> Cc: Patrick McHardy
>> Signed-off-by: Alexander Aring
>> ---
>>   net/ipv6/addrconf.c | 191 
>> 
>>   1 file changed, 102 insertions(+), 89 deletions(-)
>>
>> diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
>> index beaad49..393cdbf 100644
>> --- a/net/ipv6/addrconf.c
>> +++ b/net/ipv6/addrconf.c
>> @@ -2333,6 +2333,104 @@ static bool is_addr_mode_generate_stable(struct 
>> inet6_dev *idev)
>>  idev->addr_gen_mode == IN6_ADDR_GEN_MODE_RANDOM;
>>   }
>>   +static void addrconf_prefix_rcv_add_addr(struct net *net,

will change to return int here to indicates errors.

>> + struct net_device *dev,
>> + const struct prefix_info *pinfo,
>> + struct inet6_dev *in6_dev,
>> + const struct in6_addr *addr,
>> + int addr_type, u32 addr_flags,
>> + bool sllao, bool tokenized,
>> + __u32 valid_lft, u32 prefered_lft)
>> +{
>> +struct inet6_ifaddr *ifp = ipv6_get_ifaddr(net, addr, dev, 1);
>> +int create = 0, update_lft = 0;
>> +
>> +if (!ifp && valid_lft) {
>> +int max_addresses = in6_dev->cnf.max_addresses;
>> +
>> +#ifdef CONFIG_IPV6_OPTIMISTIC_DAD
>> +if (in6_dev->cnf.optimistic_dad &&
>> +!net->ipv6.devconf_all->forwarding && sllao)
>> +addr_flags |= IFA_F_OPTIMISTIC;
>> +#endif
>> +
>> +/* Do not allow to create too much of autoconfigured
>> + * addresses; this would be too easy way to crash kernel.
>> + */
>> +if (!max_addresses ||
>> +ipv6_count_addresses(in6_dev) < max_addresses)
>> +ifp = ipv6_add_addr(in6_dev, addr, NULL,
>> +pinfo->prefix_len,
>> +addr_type_ADDR_SCOPE_MASK,
>> +addr_flags, valid_lft,
>> +prefered_lft);
>> +
>> +if (IS_ERR_OR_NULL(ifp)) {
>> +in6_dev_put(in6_dev);
>> +return;

return -1; and remove in6_dev_put stuff.

>> +}
>> +
>> +update_lft = 0;
>> +create = 1;
>> +spin_lock_bh(>lock);
>> +ifp->flags |= IFA_F_MANAGETEMPADDR;
>> +ifp->cstamp = jiffies;
>> +ifp->tokenized = tokenized;
>> +spin_unlock_bh(>lock);
>> +addrconf_dad_start(ifp);
>> +}
>> +
>> +if (ifp) {
>> +u32 flags;
>> +unsigned long now;
>> +u32 stored_lft;
>> +
>> +/* update lifetime (RFC2462 5.5.3 e) */
>> +spin_lock_bh(>lock);
>> +now = jiffies;
>> +if (ifp->valid_lft > (now - ifp->tstamp) / HZ)
>> +stored_lft = ifp->valid_lft - (now - ifp->tstamp) / HZ;
>> +else
>> +stored_lft = 0;
>> +if (!update_lft && !create && stored_lft) {
>> +const u32 minimum_lft = min_t(u32,
>> +stored_lft, MIN_VALID_LIFETIME);
>> +valid_lft = max(valid_lft, minimum_lft);
>> +
>> +/* RFC4862 Section 5.5.3e:
>> + * "Note that the preferred lifetime of the
>> + *  corresponding address is always reset to
>> + *  the Preferred Lifetime in the received
>> + *  Prefix Information option, regardless of
>> + *  whether the valid lifetime is also reset or
>> + *  ignored."
>> + *
>> + * So we should always update prefered_lft here.
>> + */
>> +update_lft = 1;
>> +}
>> +
>> +if (update_lft) {
>> +ifp->valid_lft = valid_lft;
>> +ifp->prefered_lft = prefered_lft;
>> +ifp->tstamp = now;
>> +flags = ifp->flags;
>> +ifp->flags &= ~IFA_F_DEPRECATED;
>> +spin_unlock_bh(>lock);
>> +
>> +if (!(flags_F_TENTATIVE))
>> +ipv6_ifa_notify(0, ifp);
>> +} else
>> +spin_unlock_bh(>lock);
>> +
>> +manage_tempaddrs(in6_dev, ifp, valid_lft, prefered_lft,
>> + create, now);
>> +
>> +in6_ifa_put(ifp);
>> +addrconf_verify();
>> +}
>> +}
>> +
>>   void addrconf_prefix_rcv(struct net_device *dev, u8 *opt, int len, bool 
>> sllao)
>>   {
>>   struct prefix_info *pinfo;
>> @@ -2432,9 +2530,7 @@ 

Re: [RFC 11/12] 6lowpan: add support for getting short address

[Alexander Aring]

Hi,

On 05/27/2016 12:05 PM, Stefan Schmidt wrote:
> Hello.
> 
> On 23/05/16 21:22, Alexander Aring wrote:
>> In case of sending RA messages we need some way to get the short address
>> from an 802.15.4 6LoWPAN interface. This patch will add a temporary
>> debugfs entry for experimental userspace api.
>>
>> Signed-off-by: Alexander Aring
>> ---
>>   net/6lowpan/debugfs.c | 35 +++
>>   1 file changed, 35 insertions(+)
>>
>> diff --git a/net/6lowpan/debugfs.c b/net/6lowpan/debugfs.c
>> index acbaa3d..638ae59 100644
>> --- a/net/6lowpan/debugfs.c
>> +++ b/net/6lowpan/debugfs.c
>> @@ -245,6 +245,37 @@ static const struct file_operations lowpan_context_fops 
>> = {
>>   .release= single_release,
>>   };
>>   +static int lowpan_short_addr_get(void *data, u64 *val)
>> +{
>> +struct wpan_dev *wdev = data;
>> +
>> +rtnl_lock();
>> +*val = le16_to_cpu(wdev->short_addr);
>> +rtnl_unlock();
>> +
>> +return 0;
>> +}
>> +
>> +DEFINE_SIMPLE_ATTRIBUTE(lowpan_short_addr_fops, lowpan_short_addr_get,
>> +NULL, "0x%04llx\n");
>> +
>> +static int lowpan_dev_debugfs_802154_init(const struct net_device *dev,
>> +  struct lowpan_dev *ldev)
>> +{
>> +struct dentry *dentry;
>> +
>> +if (!lowpan_is_ll(dev, LOWPAN_LLTYPE_IEEE802154))
>> +return 0;
>> +
>> +dentry = debugfs_create_file("short_addr", 0444, ldev->iface_debugfs,
>> + lowpan_802154_dev(dev)->wdev->ieee802154_ptr,
>> + _short_addr_fops);
>> +if (!dentry)
>> +return -EINVAL;
>> +
>> +return 0;
>> +}
>> +
>>   int lowpan_dev_debugfs_init(struct net_device *dev)
>>   {
>>   struct lowpan_dev *ldev = lowpan_dev(dev);
>> @@ -272,6 +303,10 @@ int lowpan_dev_debugfs_init(struct net_device *dev)
>>   goto remove_root;
>>   }
>>   +ret = lowpan_dev_debugfs_802154_init(dev, ldev);
>> +if (ret < 0)
>> +goto remove_root;
>> +
>>   return 0;
>> remove_root:
> 
> Reviewed-by: Stefan Schmidt
> 

grml, I changed this patch now to have an 802154/short_addr directory
inside the root of 6lowpan debugfs entry.

This is because Michael Richardson wrote that other L2 has also
short_addr (any kind of second L2 address type), see [0].

Each of them has their own "constraints" e.g. 802.15.4 2-byte length, or
sometimes 1-byte. This constraint depends on L2, so I added 802154
subdir.

Nevertheless it's debugfs and we can still change it when introduce real
UAPI, but it will remind me to introduce it as setting which has some
kind of L2 dependency naming.

- Alex

[0] http://marc.info/?l=linux-netdev=146360758922131=4

Re: FWD: [PATCH v2] Marvell phy: add fiber status check for some components

[Charles-Antoine Couret]

Hello,
I'm sorry to repost that, but after one month, I need a answer to continue to 
imrpove my patch in the right direction. :)

Thank you in advance.
Regards.
Charles-Antoine Couret

Le 29/04/2016 à 10:28, Charles-Antoine Couret a écrit :
> Le 11/04/2016 à 21:47, Florian Fainelli a écrit :
>>> Do we actually need to stay on page 1 if fibre is in use? How do we
>>> initially change to page 1 when the fibre link is still down?
>>
>> I also do not feel very comfortable with reading the fiber status first,
>> and then copper and then combine these two. At the very best, could we
>> do something like:
>>
>> - identify if the PHY is configured for fiber in drv->probe or
>> drv->config_init, retain that information
> 
> But, how configure in runtime the user choice? Add a driver option?
> Else, a default choice should be added in init/probe function and the user 
> should change by ethtool or driver recompilation.
> 
>> - have two paths in drv->read_status which take care of reading one
>> status or the other?
> 
> I worked for a solution around that.
> But the Marvell's datasheet seems to agree with my previous method:
> 
> Extract:
> "Notes on Determining which Media Linked Up
> 
> Since there are two sets of IEEE registers (one for copper and the other for 
> fiber) the software needs
> to be aware of register 22.7:0 so that the correct set of registers are 
> selected. In general the
> sequence is as follows.
> 
> 1-Set the Auto-Negotiation registers of the copper medium. (This step may not 
> be necessary if the
> hardware configuration defaults are acceptable).
> 
> 2-Set the Auto-Negotiation registers of the fiber medium. (This step may not 
> be necessary if the
> hardware configuration defaults are acceptable).
> 
> 3-Poll for link status.Go to step 4 if there is link.
> 
> 4-Once there is link determine whether the link is copper or fiber medium.
> 
> 5-Look at the Auto-Negotiation results for the medium that established link.
> 
> 6-Poll for link status. If link status goes down then go back to step 3."
> 
> By default, the phy is configured to be connected on both interfaces with any 
> preference. The first connected was selected.
> A preference could be added, but it is not a force mode.
> 
> Extract:
> "Preferred Media
> 
> The device can be programmed to give one media priority over the other. In 
> other words if the
> non-preferred media establishes link first and subsequently energy is 
> detected on the preferred
> media, the PHY will drop link on the non-preferred media for 4 seconds and 
> give the preferred media
> a chance to establish link."
> 
> We don't have registers to know really if Copper or Fiber is selected. We can 
> know only by checking status registers as in my commit.
> 
> 
>>> Should we be using the old mechanism to swap between TP, BNC and AUI
>>> to swap between copper and fibre?
>>
>> Did you mean using ethtool -s  port fibre for instance?
> 
> So, to implement that I have to renable the port change in phy_ethtool_sset 
> function.
> But, the datasheet seems to disagree with this method.
> 
> Finally, what do I have to do? I continue my previous way or your suggestion?
> I prefer to respect the datasheet, but if it's better for you to follow the 
> other way, I will implement that.
> 
> Thank you in advance and have a nice day.
> Regards,
> Charles-Antoine Couret
> 

[patch] atm: iphase: off by one in rx_pkt()

[Dan Carpenter]

The iadev->rx_open[] array holds "iadev->num_vc" pointers (this code
assumes that pointers are 32 bits).  So the > here should be >= or else
we could end up reading a garbage pointer from one element beyond the
end of the array.

Signed-off-by: Dan Carpenter 

diff --git a/drivers/atm/iphase.c b/drivers/atm/iphase.c
index 7d00f29..f86e318 100644
--- a/drivers/atm/iphase.c
+++ b/drivers/atm/iphase.c
@@ -1128,7 +1128,7 @@ static int rx_pkt(struct atm_dev *dev)
/* make the ptr point to the corresponding buffer desc entry */  
buf_desc_ptr += desc; 
 if (!desc || (desc > iadev->num_rx_desc) || 
-  ((buf_desc_ptr->vc_index & 0x) > iadev->num_vc)) { 
+  ((buf_desc_ptr->vc_index & 0x) >= iadev->num_vc)) { 
 free_desc(dev, desc);
 IF_ERR(printk("IA: bad descriptor desc = %d \n", desc);)
 return -1;

[patch] atm: firestream: add more reserved strings

[Dan Carpenter]

This bug was there when the driver was first added in back in year 2000.
It causes a Smatch warning:

drivers/atm/firestream.c:849 process_incoming()
error: buffer overflow 'res_strings' 60 <= 63

There are supposed to be 64 entries in this array and the missing
strings are clearly in the 30 40 range.  I added them as reserved 37 to
reserved 40.  It's possible that strings are really supposed to be added
in the middle instead of at the end, but this approach is safe, in that
it fixes the bug and doesn't break anything that wasn't already broken.

Signed-off-by: Dan Carpenter 

diff --git a/drivers/atm/firestream.c b/drivers/atm/firestream.c
index a969a7e..4777064 100644
--- a/drivers/atm/firestream.c
+++ b/drivers/atm/firestream.c
@@ -181,13 +181,17 @@ static char *res_strings[] = {
"reserved 27", 
"reserved 28", 
"reserved 29", 
-   "reserved 30", 
+   "reserved 30", /* FIXME: The strings between 30-40 might be wrong. */
"reassembly abort: no buffers", 
"receive buffer overflow", 
"change in GFC", 
"receive buffer full", 
"low priority discard - no receive descriptor", 
"low priority discard - missing end of packet", 
+   "reserved 37",
+   "reserved 38",
+   "reserved 39",
+   "reseverd 40",
"reserved 41", 
"reserved 42", 
"reserved 43", 

Re: [PATCH net] ieee802154: fix logic error in ieee802154_llsec_parse_dev_addr

[Stefan Schmidt]

Hello.

On 26/05/16 15:07, Baozeng Ding wrote:

Fix a logic error to avoid potential null pointer dereference.

Signed-off-by: Baozeng Ding
---
  net/ieee802154/nl802154.c | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/ieee802154/nl802154.c b/net/ieee802154/nl802154.c
index ca207db..116187b 100644
--- a/net/ieee802154/nl802154.c
+++ b/net/ieee802154/nl802154.c
@@ -1289,8 +1289,8 @@ ieee802154_llsec_parse_dev_addr(struct nlattr *nla,
 nl802154_dev_addr_policy))
return -EINVAL;
  
-	if (!attrs[NL802154_DEV_ADDR_ATTR_PAN_ID] &&

-   !attrs[NL802154_DEV_ADDR_ATTR_MODE] &&
+   if (!attrs[NL802154_DEV_ADDR_ATTR_PAN_ID] ||
+   !attrs[NL802154_DEV_ADDR_ATTR_MODE] ||
!(attrs[NL802154_DEV_ADDR_ATTR_SHORT] ||
  attrs[NL802154_DEV_ADDR_ATTR_EXTENDED]))
return -EINVAL;


Good catch!

Reviewed-by: Stefan Schmidt

regards
Stefan Schmidt

Re: [RFC 10/12] 6lowpan: introduce 6lowpan-nd

[Stefan Schmidt]

Hello.

On 23/05/16 21:22, Alexander Aring wrote:

This patch introduce different 6lowpan handling for receive and transmit
NS/NA messages for the ipv6 neighbour discovery. The first use-case is
for supporting 802.15.4 short addresses inside the option fields and
handling for RFC6775 6CO option field as userspace option.

Cc: David S. Miller
Cc: Alexey Kuznetsov
Cc: James Morris
Cc: Hideaki YOSHIFUJI
Cc: Patrick McHardy
Signed-off-by: Alexander Aring
---
  include/net/ndisc.h |  18 ++--
  net/6lowpan/6lowpan_i.h |   4 +
  net/6lowpan/Makefile|   2 +-
  net/6lowpan/core.c  |   4 +-
  net/6lowpan/ndisc.c | 223 
  5 files changed, 243 insertions(+), 8 deletions(-)
  create mode 100644 net/6lowpan/ndisc.c

diff --git a/include/net/ndisc.h b/include/net/ndisc.h
index c92ebdb..5299b52 100644
--- a/include/net/ndisc.h
+++ b/include/net/ndisc.h
@@ -35,6 +35,7 @@ enum {
ND_OPT_ROUTE_INFO = 24, /* RFC4191 */
ND_OPT_RDNSS = 25,  /* RFC5006 */
ND_OPT_DNSSL = 31,  /* RFC6106 */
+   ND_OPT_6CO = 34,/* RFC6775 */
__ND_OPT_MAX
  };
  
@@ -109,14 +110,19 @@ struct ndisc_options {

  #endif
struct nd_opt_hdr *nd_useropts;
struct nd_opt_hdr *nd_useropts_end;
+#if IS_ENABLED(CONFIG_IEEE802154_6LOWPAN)
+   struct nd_opt_hdr *nd_802154_opt_array[ND_OPT_TARGET_LL_ADDR + 1];
+#endif
  };
  
-#define nd_opts_src_lladdr	nd_opt_array[ND_OPT_SOURCE_LL_ADDR]

-#define nd_opts_tgt_lladdr nd_opt_array[ND_OPT_TARGET_LL_ADDR]
-#define nd_opts_pi nd_opt_array[ND_OPT_PREFIX_INFO]
-#define nd_opts_pi_end nd_opt_array[__ND_OPT_PREFIX_INFO_END]
-#define nd_opts_rh nd_opt_array[ND_OPT_REDIRECT_HDR]
-#define nd_opts_mtund_opt_array[ND_OPT_MTU]
+#define nd_opts_src_lladdr nd_opt_array[ND_OPT_SOURCE_LL_ADDR]
+#define nd_opts_tgt_lladdr nd_opt_array[ND_OPT_TARGET_LL_ADDR]
+#define nd_opts_pi nd_opt_array[ND_OPT_PREFIX_INFO]
+#define nd_opts_pi_end nd_opt_array[__ND_OPT_PREFIX_INFO_END]
+#define nd_opts_rh nd_opt_array[ND_OPT_REDIRECT_HDR]
+#define nd_opts_mtund_opt_array[ND_OPT_MTU]
+#define nd_802154_opts_src_lladdr  
nd_802154_opt_array[ND_OPT_SOURCE_LL_ADDR]
+#define nd_802154_opts_tgt_lladdr  
nd_802154_opt_array[ND_OPT_TARGET_LL_ADDR]
  
  #define NDISC_OPT_SPACE(len) (((len)+2+7)&~7)
  
diff --git a/net/6lowpan/6lowpan_i.h b/net/6lowpan/6lowpan_i.h

index 97ecc27..a67caee 100644
--- a/net/6lowpan/6lowpan_i.h
+++ b/net/6lowpan/6lowpan_i.h
@@ -12,6 +12,10 @@ static inline bool lowpan_is_ll(const struct net_device *dev,
return lowpan_dev(dev)->lltype == lltype;
  }
  
+extern const struct ndisc_ops lowpan_ndisc_ops;

+
+int addrconf_ifid_802154_6lowpan(u8 *eui, struct net_device *dev);
+
  #ifdef CONFIG_6LOWPAN_DEBUGFS
  int lowpan_dev_debugfs_init(struct net_device *dev);
  void lowpan_dev_debugfs_exit(struct net_device *dev);
diff --git a/net/6lowpan/Makefile b/net/6lowpan/Makefile
index e44f3bf..12d131a 100644
--- a/net/6lowpan/Makefile
+++ b/net/6lowpan/Makefile
@@ -1,6 +1,6 @@
  obj-$(CONFIG_6LOWPAN) += 6lowpan.o
  
-6lowpan-y := core.o iphc.o nhc.o

+6lowpan-y := core.o iphc.o nhc.o ndisc.o


Later on we want to make this a option that can get selected because not 
all networks using 6lowpan are also using the ndisc optimizations for it.

For now this is fine as is though.


  6lowpan-$(CONFIG_6LOWPAN_DEBUGFS) += debugfs.o
  
  #rfc6282 nhcs

diff --git a/net/6lowpan/core.c b/net/6lowpan/core.c
index 824d1bc..3de7fd9 100644
--- a/net/6lowpan/core.c
+++ b/net/6lowpan/core.c
@@ -34,6 +34,8 @@ int lowpan_register_netdevice(struct net_device *dev,
for (i = 0; i < LOWPAN_IPHC_CTX_TABLE_SIZE; i++)
lowpan_dev(dev)->ctx.table[i].id = i;
  
+	dev->ndisc_ops = _ndisc_ops;

+
ret = register_netdevice(dev);
if (ret < 0)
return ret;
@@ -73,7 +75,7 @@ void lowpan_unregister_netdev(struct net_device *dev)
  }
  EXPORT_SYMBOL(lowpan_unregister_netdev);
  
-static int addrconf_ifid_802154_6lowpan(u8 *eui, struct net_device *dev)

+int addrconf_ifid_802154_6lowpan(u8 *eui, struct net_device *dev)
  {
struct wpan_dev *wpan_dev = 
lowpan_802154_dev(dev)->wdev->ieee802154_ptr;
  
diff --git a/net/6lowpan/ndisc.c b/net/6lowpan/ndisc.c

new file mode 100644
index 000..f67967b
--- /dev/null
+++ b/net/6lowpan/ndisc.c
@@ -0,0 +1,223 @@
+/* This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2
+ * as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied 

Re: [RFC 11/12] 6lowpan: add support for getting short address

[Stefan Schmidt]

Hello.

On 23/05/16 21:22, Alexander Aring wrote:

In case of sending RA messages we need some way to get the short address
from an 802.15.4 6LoWPAN interface. This patch will add a temporary
debugfs entry for experimental userspace api.

Signed-off-by: Alexander Aring
---
  net/6lowpan/debugfs.c | 35 +++
  1 file changed, 35 insertions(+)

diff --git a/net/6lowpan/debugfs.c b/net/6lowpan/debugfs.c
index acbaa3d..638ae59 100644
--- a/net/6lowpan/debugfs.c
+++ b/net/6lowpan/debugfs.c
@@ -245,6 +245,37 @@ static const struct file_operations lowpan_context_fops = {
.release= single_release,
  };
  
+static int lowpan_short_addr_get(void *data, u64 *val)

+{
+   struct wpan_dev *wdev = data;
+
+   rtnl_lock();
+   *val = le16_to_cpu(wdev->short_addr);
+   rtnl_unlock();
+
+   return 0;
+}
+
+DEFINE_SIMPLE_ATTRIBUTE(lowpan_short_addr_fops, lowpan_short_addr_get,
+   NULL, "0x%04llx\n");
+
+static int lowpan_dev_debugfs_802154_init(const struct net_device *dev,
+ struct lowpan_dev *ldev)
+{
+   struct dentry *dentry;
+
+   if (!lowpan_is_ll(dev, LOWPAN_LLTYPE_IEEE802154))
+   return 0;
+
+   dentry = debugfs_create_file("short_addr", 0444, ldev->iface_debugfs,
+
lowpan_802154_dev(dev)->wdev->ieee802154_ptr,
+_short_addr_fops);
+   if (!dentry)
+   return -EINVAL;
+
+   return 0;
+}
+
  int lowpan_dev_debugfs_init(struct net_device *dev)
  {
struct lowpan_dev *ldev = lowpan_dev(dev);
@@ -272,6 +303,10 @@ int lowpan_dev_debugfs_init(struct net_device *dev)
goto remove_root;
}
  
+	ret = lowpan_dev_debugfs_802154_init(dev, ldev);

+   if (ret < 0)
+   goto remove_root;
+
return 0;
  
  remove_root:


Reviewed-by: Stefan Schmidt

regards
Stefan Schmidt

Re: [RFC 09/12] ipv6: export several functions

[Stefan Schmidt]

Hello.

On 23/05/16 21:22, Alexander Aring wrote:

This patch exports some neighbour discovery functions which can be used
by 6lowpan neighbour discovery ops functionality then.

Cc: David S. Miller
Cc: Alexey Kuznetsov
Cc: James Morris
Cc: Hideaki YOSHIFUJI
Cc: Patrick McHardy
Signed-off-by: Alexander Aring
---
  include/net/addrconf.h |  7 +++
  include/net/ndisc.h| 12 
  net/ipv6/addrconf.c| 15 +++
  net/ipv6/ndisc.c   | 14 +++---
  4 files changed, 29 insertions(+), 19 deletions(-)

diff --git a/include/net/addrconf.h b/include/net/addrconf.h
index b1774eb..f5d2230 100644
--- a/include/net/addrconf.h
+++ b/include/net/addrconf.h
@@ -97,6 +97,13 @@ void addrconf_leave_solict(struct inet6_dev *idev, const 
struct in6_addr *addr);
  void addrconf_add_linklocal(struct inet6_dev *idev,
const struct in6_addr *addr, u32 flags);
  
+void addrconf_prefix_rcv_add_addr(struct net *net, struct net_device *dev,

+ const struct prefix_info *pinfo,
+ struct inet6_dev *in6_dev,
+ const struct in6_addr *addr, int addr_type,
+ u32 addr_flags, bool sllao, bool tokenized,
+ __u32 valid_lft, u32 prefered_lft);
+
  static inline int addrconf_ifid_eui48(u8 *eui, struct net_device *dev)
  {
if (dev->addr_len != ETH_ALEN)
diff --git a/include/net/ndisc.h b/include/net/ndisc.h
index e4a711e..c92ebdb 100644
--- a/include/net/ndisc.h
+++ b/include/net/ndisc.h
@@ -53,6 +53,15 @@ enum {
  
  #include 
  
+/* Set to 3 to get tracing... */

+#define ND_DEBUG 1
+
+#define ND_PRINTK(val, level, fmt, ...)\
+do {   \
+   if (val <= ND_DEBUG) \
+   net_##level##_ratelimited(fmt, ##__VA_ARGS__);  \
+} while (0)
+
  struct ctl_table;
  struct inet6_dev;
  struct net_device;
@@ -115,6 +124,9 @@ struct ndisc_options *ndisc_parse_options(const struct 
net_device *dev,
  u8 *opt, int opt_len,
  struct ndisc_options *ndopts);
  
+void ndisc_fill_addr_option(struct sk_buff *skb, int type, void *data,

+   int data_len, int pad);
+
  /*
   * Return the padding between the option length and the start of the
   * link addr.  Currently only IP-over-InfiniBand needs this, although
diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index 4506cac..aeea54e 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -2333,14 +2333,12 @@ static bool is_addr_mode_generate_stable(struct 
inet6_dev *idev)
   idev->addr_gen_mode == IN6_ADDR_GEN_MODE_RANDOM;
  }
  
-static void addrconf_prefix_rcv_add_addr(struct net *net,

-struct net_device *dev,
-const struct prefix_info *pinfo,
-struct inet6_dev *in6_dev,
-const struct in6_addr *addr,
-int addr_type, u32 addr_flags,
-bool sllao, bool tokenized,
-__u32 valid_lft, u32 prefered_lft)
+void addrconf_prefix_rcv_add_addr(struct net *net, struct net_device *dev,
+ const struct prefix_info *pinfo,
+ struct inet6_dev *in6_dev,
+ const struct in6_addr *addr, int addr_type,
+ u32 addr_flags, bool sllao, bool tokenized,
+ __u32 valid_lft, u32 prefered_lft)
  {
struct inet6_ifaddr *ifp = ipv6_get_ifaddr(net, addr, dev, 1);
int create = 0, update_lft = 0;
@@ -2430,6 +2428,7 @@ static void addrconf_prefix_rcv_add_addr(struct net *net,
addrconf_verify();
}
  }
+EXPORT_SYMBOL_GPL(addrconf_prefix_rcv_add_addr);
  
  void addrconf_prefix_rcv(struct net_device *dev, u8 *opt, int len, bool sllao)

  {
diff --git a/net/ipv6/ndisc.c b/net/ipv6/ndisc.c
index 99fd53c..d30f241 100644
--- a/net/ipv6/ndisc.c
+++ b/net/ipv6/ndisc.c
@@ -73,15 +73,6 @@
  #include 
  #include 
  
-/* Set to 3 to get tracing... */

-#define ND_DEBUG 1
-
-#define ND_PRINTK(val, level, fmt, ...)\
-do {   \
-   if (val <= ND_DEBUG) \
-   net_##level##_ratelimited(fmt, ##__VA_ARGS__);  \
-} while (0)
-
  static u32 ndisc_hash(const void *pkey,
  const struct net_device *dev,
  __u32 *hash_rnd);
@@ 

Re: IPv6 extension header privileges

[Hannes Frederic Sowa]

On 26.05.2016 20:42, Tom Herbert wrote:
> On Mon, May 23, 2016 at 11:11 AM, Tom Herbert  wrote:
>> On Sun, May 22, 2016 at 4:56 AM, Sowmini Varadhan
>>  wrote:
>>>
> Tom Herbert wrote:
> If you don't mind I'll change this to make specific options are
> privileged and not all hbh and destopt. There is talk in IETF about
> reinventing IP extensibility within UDP since the kernel APIs don't
> allow setting EH. I would like to avoid that :-)
>>>
 On 21.05.2016 19:46, Sowmini Varadhan wrote:
> Do you mean this
>   http://www.ietf.org/mail-archive/web/spud/current/msg00365.html
>>>
>>> On (05/22/16 03:08), Hannes Frederic Sowa wrote:
 Hmm, haven't read carefully but isn't that just plain TCP in UDP? I saw
 extension headers mentioned but haven't grasped why they deem necessary.
>>>
>>> Tom should clarify what he meant, but perhaps he was referring to other
>>> threads discussing v6 EH. In any case, I dont think the way least-privileges
>>> for EH are implemented in an OS is directly relevant or causational for
>>> whether or not the kernel should be bypassed - looks like there are a lot
>>> of other drafts floating around, arguing for implementing various tcp/ip
>>> protocols in uspace and beyond, motivated by various reasons.
>>>
>> It's a deployment conundrum. Suppose tomorrow that IANA registers some
>> new hpb option that would be useful to the network, but is of no
>> interest to the kernel other than it needs to be set in packets when
>> the user requests it. In the white list model, there is no problem
>> getting support for such a thing into the upstream kernel, the time
>> frame for that is one release cycle. Neither is there any problem
>> updating the apps to set the option, for instance we can update FB app
>> to do this within a week. The problem is that getting something into
>> the kernel does not make it useful, the kernel needs to actually be
>> deployed which is mostly out of our control (for those of us who don't
>> own the client platform). So get the options deployed on clients
>> (particularly Android), this takes much, much longer. And if the
>> feature requires explicit action do be enabled, like turning a sysctl,
>> it is going to take even longer possibly an indeterminate amount of
>> time to ever get enabled.
>>
> Thinking about this some more, the per option white list is a better
> approach. If we allow an open ended mechanism for applications to
> signal the network with arbitrary data (like user specified hbp
> options would be), then use of that mechanism will inevitably
> exploited by some authorities to force user to hand over private data
> about their communications. It's better to not build in back doors to
> security...

Sorry, Tom, can you try to explain again, I think I might not have
understood you correctly.

Thanks,
Hannes

Re: [RFC 07/12] addrconf: put prefix address add in an own function

[Stefan Schmidt]

Hello.

On 23/05/16 21:22, Alexander Aring wrote:

This patch moves the functionality to add a RA PIO prefix generated
address in an own function. This move prepares to add a hook for
adding a second address for a second link-layer address. E.g. short
address for 802.15.4 6LoWPAN.

Cc: David S. Miller
Cc: Alexey Kuznetsov
Cc: James Morris
Cc: Hideaki YOSHIFUJI
Cc: Patrick McHardy
Signed-off-by: Alexander Aring
---
  net/ipv6/addrconf.c | 191 
  1 file changed, 102 insertions(+), 89 deletions(-)

diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index beaad49..393cdbf 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -2333,6 +2333,104 @@ static bool is_addr_mode_generate_stable(struct 
inet6_dev *idev)
   idev->addr_gen_mode == IN6_ADDR_GEN_MODE_RANDOM;
  }
  
+static void addrconf_prefix_rcv_add_addr(struct net *net,

+struct net_device *dev,
+const struct prefix_info *pinfo,
+struct inet6_dev *in6_dev,
+const struct in6_addr *addr,
+int addr_type, u32 addr_flags,
+bool sllao, bool tokenized,
+__u32 valid_lft, u32 prefered_lft)
+{
+   struct inet6_ifaddr *ifp = ipv6_get_ifaddr(net, addr, dev, 1);
+   int create = 0, update_lft = 0;
+
+   if (!ifp && valid_lft) {
+   int max_addresses = in6_dev->cnf.max_addresses;
+
+#ifdef CONFIG_IPV6_OPTIMISTIC_DAD
+   if (in6_dev->cnf.optimistic_dad &&
+   !net->ipv6.devconf_all->forwarding && sllao)
+   addr_flags |= IFA_F_OPTIMISTIC;
+#endif
+
+   /* Do not allow to create too much of autoconfigured
+* addresses; this would be too easy way to crash kernel.
+*/
+   if (!max_addresses ||
+   ipv6_count_addresses(in6_dev) < max_addresses)
+   ifp = ipv6_add_addr(in6_dev, addr, NULL,
+   pinfo->prefix_len,
+   addr_type_ADDR_SCOPE_MASK,
+   addr_flags, valid_lft,
+   prefered_lft);
+
+   if (IS_ERR_OR_NULL(ifp)) {
+   in6_dev_put(in6_dev);
+   return;
+   }
+
+   update_lft = 0;
+   create = 1;
+   spin_lock_bh(>lock);
+   ifp->flags |= IFA_F_MANAGETEMPADDR;
+   ifp->cstamp = jiffies;
+   ifp->tokenized = tokenized;
+   spin_unlock_bh(>lock);
+   addrconf_dad_start(ifp);
+   }
+
+   if (ifp) {
+   u32 flags;
+   unsigned long now;
+   u32 stored_lft;
+
+   /* update lifetime (RFC2462 5.5.3 e) */
+   spin_lock_bh(>lock);
+   now = jiffies;
+   if (ifp->valid_lft > (now - ifp->tstamp) / HZ)
+   stored_lft = ifp->valid_lft - (now - ifp->tstamp) / HZ;
+   else
+   stored_lft = 0;
+   if (!update_lft && !create && stored_lft) {
+   const u32 minimum_lft = min_t(u32,
+   stored_lft, MIN_VALID_LIFETIME);
+   valid_lft = max(valid_lft, minimum_lft);
+
+   /* RFC4862 Section 5.5.3e:
+* "Note that the preferred lifetime of the
+*  corresponding address is always reset to
+*  the Preferred Lifetime in the received
+*  Prefix Information option, regardless of
+*  whether the valid lifetime is also reset or
+*  ignored."
+*
+* So we should always update prefered_lft here.
+*/
+   update_lft = 1;
+   }
+
+   if (update_lft) {
+   ifp->valid_lft = valid_lft;
+   ifp->prefered_lft = prefered_lft;
+   ifp->tstamp = now;
+   flags = ifp->flags;
+   ifp->flags &= ~IFA_F_DEPRECATED;
+   spin_unlock_bh(>lock);
+
+   if (!(flags_F_TENTATIVE))
+   ipv6_ifa_notify(0, ifp);
+   } else
+   spin_unlock_bh(>lock);
+
+   manage_tempaddrs(in6_dev, ifp, valid_lft, prefered_lft,
+create, now);
+
+   

[PATCH v1 5/6] dtb: xgene: Remove clock nodes

[Iyappan Subramanian]

Since the MDIO will be responsible for clock reset, removing the clock
nodes from shadowcat xge0 and storm sgenet1.

Signed-off-by: Iyappan Subramanian 
Tested-by: Fushen Chen 
Tested-by: Toan Le 
---
 arch/arm64/boot/dts/apm/apm-shadowcat.dtsi | 12 
 arch/arm64/boot/dts/apm/apm-storm.dtsi | 18 --
 2 files changed, 4 insertions(+), 26 deletions(-)

diff --git a/arch/arm64/boot/dts/apm/apm-shadowcat.dtsi 
b/arch/arm64/boot/dts/apm/apm-shadowcat.dtsi
index 8106957..f27563d 100644
--- a/arch/arm64/boot/dts/apm/apm-shadowcat.dtsi
+++ b/arch/arm64/boot/dts/apm/apm-shadowcat.dtsi
@@ -299,17 +299,6 @@
clock-output-names = "pcie1clk";
};
 
-   xge0clk: xge0clk@1f61c000 {
-   compatible = "apm,xgene-device-clock";
-   #clock-cells = <1>;
-   clocks = < 0>;
-   reg = <0x0 0x1f61c000 0x0 0x1000>;
-   reg-names = "csr-reg";
-   enable-mask = <0x3>;
-   csr-mask = <0x3>;
-   clock-output-names = "xge0clk";
-   };
-
xge1clk: xge1clk@1f62c000 {
compatible = "apm,xgene-device-clock";
#clock-cells = <1>;
@@ -643,7 +632,6 @@
interrupts = <0 96 4>,
 <0 97 4>;
dma-coherent;
-   clocks = < 0>;
local-mac-address = [00 01 73 00 00 01];
phy-connection-type = "sgmii";
phy-handle = <>;
diff --git a/arch/arm64/boot/dts/apm/apm-storm.dtsi 
b/arch/arm64/boot/dts/apm/apm-storm.dtsi
index 18f694ea..f631fe4 100644
--- a/arch/arm64/boot/dts/apm/apm-storm.dtsi
+++ b/arch/arm64/boot/dts/apm/apm-storm.dtsi
@@ -237,20 +237,11 @@
clocks = < 0>;
reg = <0x0 0x1f21c000 0x0 0x1000>;
reg-names = "csr-reg";
-   csr-mask = <0x3>;
+   csr-mask = <0xa>;
+   enable-mask = <0xf>;
clock-output-names = "sge0clk";
};
 
-   sge1clk: sge1clk@1f21c000 {
-   compatible = "apm,xgene-device-clock";
-   #clock-cells = <1>;
-   clocks = < 0>;
-   reg = <0x0 0x1f21c000 0x0 0x1000>;
-   reg-names = "csr-reg";
-   csr-mask = <0xc>;
-   clock-output-names = "sge1clk";
-   };
-
xge0clk: xge0clk@1f61c000 {
compatible = "apm,xgene-device-clock";
#clock-cells = <1>;
@@ -938,9 +929,9 @@
reg-names = "enet_csr", "ring_csr", "ring_cmd";
interrupts = <0x0 0x3c 0x4>;
dma-coherent;
-   clocks = < 0>;
/* mac address will be overwritten by the bootloader */
local-mac-address = [00 00 00 00 00 00];
+   clocks = < 0>;
phy-connection-type = "rgmii";
phy-handle = <>;
};
@@ -955,8 +946,8 @@
interrupts = <0x0 0xA0 0x4>,
 <0x0 0xA1 0x4>;
dma-coherent;
-   clocks = < 0>;
local-mac-address = [00 00 00 00 00 00];
+   clocks = < 0>;
phy-connection-type = "sgmii";
phy-handle = <>;
};
@@ -972,7 +963,6 @@
 <0x0 0xAD 0x4>;
port-id = <1>;
dma-coherent;
-   clocks = < 0>;
local-mac-address = [00 00 00 00 00 00];
phy-connection-type = "sgmii";
phy-handle = <>;
-- 
1.9.1

[PATCH v1 1/6] drivers: net: xgene: MAC and PHY configuration changes

[Iyappan Subramanian]

This patch fixes MAC configuration to support 10/100GbE for SGMII and
link_state call back. It also sets pdata->mdio_driver flag based on
ethernet mdio subnode and prepare for MDIO driver support.

In summary, following are the changes,

- Added set_speed function pointer in mac_ops
- Changed link_state to call the set_speed
- Add 10/100 support for SGMII based 1G
- Fixed mac_init for 1G

- Call mac_ops rx_enable/disable and tx_enable/disable function pointers
- Add acpi_phy_find_device to find PHY using phy-handle reference object
- Changing phy_start and phy_stop calls based on phy_dev object existence
- Calling phy_connect based on pdata->mdio_driver flag

Signed-off-by: Iyappan Subramanian 
Tested-by: Fushen Chen 
Tested-by: Toan Le 
---
 drivers/net/ethernet/apm/xgene/xgene_enet_hw.c| 189 +-
 drivers/net/ethernet/apm/xgene/xgene_enet_hw.h|   4 +
 drivers/net/ethernet/apm/xgene/xgene_enet_main.c  |  40 +++--
 drivers/net/ethernet/apm/xgene/xgene_enet_main.h  |   2 +
 drivers/net/ethernet/apm/xgene/xgene_enet_sgmac.c | 106 +++-
 drivers/net/ethernet/apm/xgene/xgene_enet_sgmac.h |   8 +
 drivers/net/ethernet/apm/xgene/xgene_enet_xgmac.h |   4 +
 7 files changed, 256 insertions(+), 97 deletions(-)

diff --git a/drivers/net/ethernet/apm/xgene/xgene_enet_hw.c 
b/drivers/net/ethernet/apm/xgene/xgene_enet_hw.c
index 2f5638f..6bc8360 100644
--- a/drivers/net/ethernet/apm/xgene/xgene_enet_hw.c
+++ b/drivers/net/ethernet/apm/xgene/xgene_enet_hw.c
@@ -512,14 +512,11 @@ static void xgene_enet_configure_clock(struct 
xgene_enet_pdata *pdata)
 #endif
 }
 
-static void xgene_gmac_init(struct xgene_enet_pdata *pdata)
+static void xgene_gmac_set_speed(struct xgene_enet_pdata *pdata)
 {
struct device *dev = >pdev->dev;
-   u32 value, mc2;
-   u32 intf_ctl, rgmii;
-   u32 icm0, icm2;
-
-   xgene_gmac_reset(pdata);
+   u32 icm0, icm2, mc2;
+   u32 intf_ctl, rgmii, value;
 
xgene_enet_rd_mcx_csr(pdata, ICM_CONFIG0_REG_0_ADDR, );
xgene_enet_rd_mcx_csr(pdata, ICM_CONFIG2_REG_0_ADDR, );
@@ -564,7 +561,18 @@ static void xgene_gmac_init(struct xgene_enet_pdata *pdata)
mc2 |= FULL_DUPLEX2 | PAD_CRC;
xgene_enet_wr_mcx_mac(pdata, MAC_CONFIG_2_ADDR, mc2);
xgene_enet_wr_mcx_mac(pdata, INTERFACE_CONTROL_ADDR, intf_ctl);
+   xgene_enet_wr_csr(pdata, RGMII_REG_0_ADDR, rgmii);
+   xgene_enet_configure_clock(pdata);
+
+   xgene_enet_wr_mcx_csr(pdata, ICM_CONFIG0_REG_0_ADDR, icm0);
+   xgene_enet_wr_mcx_csr(pdata, ICM_CONFIG2_REG_0_ADDR, icm2);
+}
 
+static void xgene_gmac_init(struct xgene_enet_pdata *pdata)
+{
+   u32 value;
+
+   xgene_gmac_set_speed(pdata);
xgene_gmac_set_mac_addr(pdata);
 
/* Adjust MDC clock frequency */
@@ -579,15 +587,10 @@ static void xgene_gmac_init(struct xgene_enet_pdata 
*pdata)
 
/* Rtype should be copied from FP */
xgene_enet_wr_csr(pdata, RSIF_RAM_DBG_REG0_ADDR, 0);
-   xgene_enet_wr_csr(pdata, RGMII_REG_0_ADDR, rgmii);
-   xgene_enet_configure_clock(pdata);
 
/* Rx-Tx traffic resume */
xgene_enet_wr_csr(pdata, CFG_LINK_AGGR_RESUME_0_ADDR, TX_PORT0);
 
-   xgene_enet_wr_mcx_csr(pdata, ICM_CONFIG0_REG_0_ADDR, icm0);
-   xgene_enet_wr_mcx_csr(pdata, ICM_CONFIG2_REG_0_ADDR, icm2);
-
xgene_enet_rd_mcx_csr(pdata, RX_DV_GATE_REG_0_ADDR, );
value &= ~TX_DV_GATE_EN0;
value &= ~RX_DV_GATE_EN0;
@@ -671,25 +674,11 @@ bool xgene_ring_mgr_init(struct xgene_enet_pdata *p)
 
 static int xgene_enet_reset(struct xgene_enet_pdata *pdata)
 {
-   u32 val;
-
if (!xgene_ring_mgr_init(pdata))
return -ENODEV;
 
-   if (!IS_ERR(pdata->clk)) {
-   clk_prepare_enable(pdata->clk);
-   clk_disable_unprepare(pdata->clk);
-   clk_prepare_enable(pdata->clk);
-   xgene_enet_ecc_init(pdata);
-   }
xgene_enet_config_ring_if_assoc(pdata);
 
-   /* Enable auto-incr for scanning */
-   xgene_enet_rd_mcx_mac(pdata, MII_MGMT_CONFIG_ADDR, );
-   val |= SCAN_AUTO_INCR;
-   MGMT_CLOCK_SEL_SET(, 1);
-   xgene_enet_wr_mcx_mac(pdata, MII_MGMT_CONFIG_ADDR, val);
-
return 0;
 }
 
@@ -724,29 +713,49 @@ static int xgene_enet_mdio_write(struct mii_bus *bus, int 
mii_id, int regnum,
 static void xgene_enet_adjust_link(struct net_device *ndev)
 {
struct xgene_enet_pdata *pdata = netdev_priv(ndev);
+   const struct xgene_mac_ops *mac_ops = pdata->mac_ops;
struct phy_device *phydev = pdata->phy_dev;
 
if (phydev->link) {
if (pdata->phy_speed != phydev->speed) {
pdata->phy_speed = phydev->speed;
-   xgene_gmac_init(pdata);
-   xgene_gmac_rx_enable(pdata);
-   xgene_gmac_tx_enable(pdata);
+   mac_ops->set_speed(pdata);
+   

[PATCH v1 4/6] dtb: xgene: Add MDIO node

[Iyappan Subramanian]

Added mdio node for mdio driver.  Also added phy-handle
reference to the ethernet nodes.  Removed unused mdio
subnode within ethernet node.

Signed-off-by: Iyappan Subramanian 
Tested-by: Fushen Chen 
Tested-by: Toan Le 
---
 arch/arm64/boot/dts/apm/apm-merlin.dts | 10 ++
 arch/arm64/boot/dts/apm/apm-mustang.dts| 12 
 arch/arm64/boot/dts/apm/apm-shadowcat.dtsi | 11 +++
 arch/arm64/boot/dts/apm/apm-storm.dtsi | 20 ++--
 4 files changed, 43 insertions(+), 10 deletions(-)

diff --git a/arch/arm64/boot/dts/apm/apm-merlin.dts 
b/arch/arm64/boot/dts/apm/apm-merlin.dts
index 387c6a8..7d47a0f 100644
--- a/arch/arm64/boot/dts/apm/apm-merlin.dts
+++ b/arch/arm64/boot/dts/apm/apm-merlin.dts
@@ -83,3 +83,13 @@
status = "ok";
};
 };
+
+ {
+   sgenet0phy: phy@3 {
+   reg = <0x0>;
+   };
+   sgenet1phy: phy@2 {
+   reg = <0x2>;
+   };
+};
+
diff --git a/arch/arm64/boot/dts/apm/apm-mustang.dts 
b/arch/arm64/boot/dts/apm/apm-mustang.dts
index 44db32e..c4e2bc4 100644
--- a/arch/arm64/boot/dts/apm/apm-mustang.dts
+++ b/arch/arm64/boot/dts/apm/apm-mustang.dts
@@ -79,3 +79,15 @@
  {
status = "ok";
 };
+
+ {
+   menetphy: phy@3 {
+   reg = <0x3>;
+   };
+   sgenet0phy: phy@4 {
+   reg = <0x4>;
+   };
+   sgenet1phy: phy@5 {
+   reg = <0x5>;
+   };
+};
diff --git a/arch/arm64/boot/dts/apm/apm-shadowcat.dtsi 
b/arch/arm64/boot/dts/apm/apm-shadowcat.dtsi
index ba04877..8106957 100644
--- a/arch/arm64/boot/dts/apm/apm-shadowcat.dtsi
+++ b/arch/arm64/boot/dts/apm/apm-shadowcat.dtsi
@@ -625,6 +625,15 @@
apm,irq-start = <8>;
};
 
+   mdio: mdio@0x1f61 {
+   compatible = "apm,xgene-mdio-sgmii";
+   #address-cells = <1>;
+   #size-cells = <0>;
+   reg = <0x0 0x1f61 0x0 0x100>,
+ <0x0 0X1f61d000 0x0 0X100>;
+   clocks = < 0>;
+   };
+
sgenet0: ethernet@1f61 {
compatible = "apm,xgene2-sgenet";
status = "disabled";
@@ -637,6 +646,7 @@
clocks = < 0>;
local-mac-address = [00 01 73 00 00 01];
phy-connection-type = "sgmii";
+   phy-handle = <>;
};
 
xgenet1: ethernet@1f62 {
@@ -659,6 +669,7 @@
clocks = < 0>;
local-mac-address = [00 01 73 00 00 02];
phy-connection-type = "xgmii";
+   phy-handle = <>;
};
 
rng: rng@1052 {
diff --git a/arch/arm64/boot/dts/apm/apm-storm.dtsi 
b/arch/arm64/boot/dts/apm/apm-storm.dtsi
index 5147d76..18f694ea 100644
--- a/arch/arm64/boot/dts/apm/apm-storm.dtsi
+++ b/arch/arm64/boot/dts/apm/apm-storm.dtsi
@@ -921,6 +921,14 @@
clocks = < 0>;
};
 
+   mdio: mdio@0x1702 {
+   compatible = "apm,xgene-mdio-rgmii";
+   #address-cells = <1>;
+   #size-cells = <0>;
+   reg = <0x0 0x1702 0x0 0xd100>;
+   clocks = < 0>;
+   };
+
menet: ethernet@1702 {
compatible = "apm,xgene-enet";
status = "disabled";
@@ -935,16 +943,6 @@
local-mac-address = [00 00 00 00 00 00];
phy-connection-type = "rgmii";
phy-handle = <>;
-   mdio {
-   compatible = "apm,xgene-mdio";
-   #address-cells = <1>;
-   #size-cells = <0>;
-   menetphy: menetphy@3 {
-   compatible = "ethernet-phy-id001c.c915";
-   reg = <0x3>;
-   };
-
-   };
};
 
sgenet0: ethernet@1f21 {
@@ -960,6 +958,7 @@
clocks = < 0>;
local-mac-address = [00 00 00 00 00 00];
phy-connection-type = "sgmii";
+   phy-handle = <>;
};
 
sgenet1: ethernet@1f210030 {
@@ -976,6 +975,7 @@
clocks = < 0>;
local-mac-address = [00 00 00 00 00 00];
phy-connection-type = "sgmii";
+   phy-handle = <>;
};
 
xgenet: ethernet@1f61 {
-- 
1.9.1

[PATCH v1 6/6] drivers: net: xgene: Fix module load/unload crash

[Iyappan Subramanian]

When the driver is configured as kernel module and when it gets unloaded
and reloaded, kernel crash was observed, due to incomplete hardware
cleanup.  This patch addresses this issue with the following changes,

- Reordered mac enable and disable
- Added hardware prefetch buffer cleanup
- Added Tx completion ring free
- Fixed delete bufpool
- Moved down delete_desc_rings after ring cleanup

- Moved regsiter_netdev call after hardware is ready
- deleted napi_del function call, since it's called by free_netdev
- Calling netif_tx_start_queues and netif_tx_stop_queues
- correcting setting irq_name before calling request_irq.
- Calling dev_close() within remove
- Added shutdown callback
- Changed to use dmam_ APIs

Signed-off-by: Iyappan Subramanian 
Tested-by: Fushen Chen 
Tested-by: Toan Le 
---
 drivers/net/ethernet/apm/xgene/xgene_enet_hw.c|  71 +++-
 drivers/net/ethernet/apm/xgene/xgene_enet_hw.h|  14 +-
 drivers/net/ethernet/apm/xgene/xgene_enet_main.c  | 199 +-
 drivers/net/ethernet/apm/xgene/xgene_enet_main.h  |  32 +---
 drivers/net/ethernet/apm/xgene/xgene_enet_sgmac.c |  90 --
 drivers/net/ethernet/apm/xgene/xgene_enet_xgmac.c |  66 ++-
 6 files changed, 329 insertions(+), 143 deletions(-)

diff --git a/drivers/net/ethernet/apm/xgene/xgene_enet_hw.c 
b/drivers/net/ethernet/apm/xgene/xgene_enet_hw.c
index 38d6ee4..8e3827f 100644
--- a/drivers/net/ethernet/apm/xgene/xgene_enet_hw.c
+++ b/drivers/net/ethernet/apm/xgene/xgene_enet_hw.c
@@ -684,21 +684,75 @@ static int xgene_enet_reset(struct xgene_enet_pdata 
*pdata)
if (!xgene_ring_mgr_init(pdata))
return -ENODEV;
 
-   if (!pdata->mdio_driver) {
-   if (!IS_ERR(pdata->clk)) {
-   clk_prepare_enable(pdata->clk);
-   clk_disable_unprepare(pdata->clk);
-   clk_prepare_enable(pdata->clk);
-   xgene_enet_ecc_init(pdata);
+   if (pdata->mdio_driver) {
+   xgene_enet_config_ring_if_assoc(pdata);
+   return 0;
+   }
+   if (!IS_ERR(pdata->clk)) {
+   clk_prepare_enable(pdata->clk);
+   clk_disable_unprepare(pdata->clk);
+   clk_prepare_enable(pdata->clk);
+   } else {
+#ifdef CONFIG_ACPI
+   if (acpi_has_method(ACPI_HANDLE(>pdev->dev), "_RST")) {
+   acpi_evaluate_object(ACPI_HANDLE(>pdev->dev),
+"_RST", NULL, NULL);
+   } else if (acpi_has_method(ACPI_HANDLE(>pdev->dev),
+"_INI")) {
+   acpi_evaluate_object(ACPI_HANDLE(>pdev->dev),
+"_INI", NULL, NULL);
}
+#endif
}
+
+   xgene_enet_ecc_init(pdata);
xgene_enet_config_ring_if_assoc(pdata);
 
return 0;
 }
 
+static void xgene_enet_clear(struct xgene_enet_pdata *pdata,
+struct xgene_enet_desc_ring *ring)
+{
+   u32 addr, val, data;
+
+   val = xgene_enet_ring_bufnum(ring->id);
+
+   if (xgene_enet_is_bufpool(ring->id)) {
+   addr = ENET_CFGSSQMIFPRESET_ADDR;
+   data = BIT(val - 0x20);
+   } else {
+   addr = ENET_CFGSSQMIWQRESET_ADDR;
+   data = BIT(val);
+   }
+
+   xgene_enet_wr_ring_if(pdata, addr, data);
+}
+
 static void xgene_gport_shutdown(struct xgene_enet_pdata *pdata)
 {
+   struct xgene_enet_desc_ring *ring;
+   u32 pb, val;
+   int i;
+
+   pb = 0;
+   for (i = 0; i < pdata->rxq_cnt; i++) {
+   ring = pdata->rx_ring[i]->buf_pool;
+
+   val = xgene_enet_ring_bufnum(ring->id);
+   pb |= BIT(val - 0x20);
+   }
+   xgene_enet_wr_ring_if(pdata, ENET_CFGSSQMIFPRESET_ADDR, pb);
+
+   pb = 0;
+   for (i = 0; i < pdata->txq_cnt; i++) {
+   ring = pdata->tx_ring[i];
+
+   val = xgene_enet_ring_bufnum(ring->id);
+   pb |= BIT(val);
+   }
+   xgene_enet_wr_ring_if(pdata, ENET_CFGSSQMIWQRESET_ADDR, pb);
+
if (!IS_ERR(pdata->clk))
clk_disable_unprepare(pdata->clk);
 }
@@ -748,7 +802,7 @@ static void xgene_enet_adjust_link(struct net_device *ndev)
 }
 
 #ifdef CONFIG_ACPI
-static struct acpi_device *acpi_phy_find_device(struct device *dev)
+struct acpi_device *acpi_phy_find_device(struct device *dev)
 {
struct acpi_reference_args args;
struct fwnode_handle *fw_node;
@@ -869,7 +923,7 @@ int xgene_enet_mdio_config(struct xgene_enet_pdata *pdata)
 ndev->name);
 
mdio_bus->priv = pdata;
-   mdio_bus->parent = >dev;
+   mdio_bus->parent = >pdev->dev;
 
ret = xgene_mdiobus_register(pdata, mdio_bus);
if (ret) {
@@ -917,6 +971,7 @@ const struct xgene_mac_ops xgene_gmac_ops = {
 
 const struct 

[PATCH v1 3/6] drivers: net: phy: Add MDIO driver

[Iyappan Subramanian]

Currently, SGMII based 1G rely on the hardware registers for link state
and sometimes it's not reliable.  To get most accurate link state, this
interface has to use the MDIO bus to poll the PHY.

In X-Gene SoC, MDIO bus is shared across RGMII and SGMII based 1G
interfaces, so adding this driver to manage MDIO bus.  This driver
registers the mdio bus and registers the PHYs connected to it.

Signed-off-by: Iyappan Subramanian 
Tested-by: Fushen Chen 
Tested-by: Toan Le 
---
 drivers/net/ethernet/apm/xgene/Kconfig |   1 +
 drivers/net/phy/Kconfig|   7 +
 drivers/net/phy/Makefile   |   1 +
 drivers/net/phy/mdio-xgene.c   | 531 +
 drivers/net/phy/mdio-xgene.h   | 140 +
 5 files changed, 680 insertions(+)
 create mode 100644 drivers/net/phy/mdio-xgene.c
 create mode 100644 drivers/net/phy/mdio-xgene.h

diff --git a/drivers/net/ethernet/apm/xgene/Kconfig 
b/drivers/net/ethernet/apm/xgene/Kconfig
index 19e38af..300e3b5 100644
--- a/drivers/net/ethernet/apm/xgene/Kconfig
+++ b/drivers/net/ethernet/apm/xgene/Kconfig
@@ -3,6 +3,7 @@ config NET_XGENE
depends on HAS_DMA
depends on ARCH_XGENE || COMPILE_TEST
select PHYLIB
+   select MDIO_XGENE
help
  This is the Ethernet driver for the on-chip ethernet interface on the
  APM X-Gene SoC.
diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
index 6dad9a9..193f418 100644
--- a/drivers/net/phy/Kconfig
+++ b/drivers/net/phy/Kconfig
@@ -271,6 +271,13 @@ config MDIO_BCM_IPROC
  This module provides a driver for the MDIO busses found in the
  Broadcom iProc SoC's.
 
+config MDIO_XGENE
+   tristate "APM X-Gene SoC MDIO bus controller"
+   help
+ This module provides a driver for the MDIO busses found in the
+ APM X-Gene SoC's.
+
+
 endif # PHYLIB
 
 config MICREL_KS8995MA
diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile
index fcdbb92..9cbd2af 100644
--- a/drivers/net/phy/Makefile
+++ b/drivers/net/phy/Makefile
@@ -44,3 +44,4 @@ obj-$(CONFIG_MDIO_MOXART) += mdio-moxart.o
 obj-$(CONFIG_MDIO_BCM_UNIMAC)  += mdio-bcm-unimac.o
 obj-$(CONFIG_MICROCHIP_PHY)+= microchip.o
 obj-$(CONFIG_MDIO_BCM_IPROC)   += mdio-bcm-iproc.o
+obj-$(CONFIG_MDIO_XGENE)   += mdio-xgene.o
diff --git a/drivers/net/phy/mdio-xgene.c b/drivers/net/phy/mdio-xgene.c
new file mode 100644
index 000..bd07fd9
--- /dev/null
+++ b/drivers/net/phy/mdio-xgene.c
@@ -0,0 +1,531 @@
+/* Applied Micro X-Gene SoC MDIO Driver
+ *
+ * Copyright (c) 2016, Applied Micro Circuits Corporation
+ * Author: Iyappan Subramanian 
+ *
+ * This program is free software; you can redistribute  it and/or modify it
+ * under  the terms of  the GNU General  Public License as published by the
+ * Free Software Foundation;  either version 2 of the  License, or (at your
+ * option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see .
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include "mdio-xgene.h"
+
+static bool xgene_mdio_status;
+
+static bool xgene_enet_rd_indirect(void __iomem *addr, void __iomem *rd,
+  void __iomem *cmd, void __iomem *cmd_done,
+  u32 rd_addr, u32 *rd_data)
+{
+   u32 done;
+   u8 wait = 10;
+
+   iowrite32(rd_addr, addr);
+   iowrite32(XGENE_ENET_RD_CMD, cmd);
+
+   /* wait for read command to complete */
+   while (!(done = ioread32(cmd_done)) && wait--)
+   udelay(1);
+
+   if (!done)
+   return false;
+
+   *rd_data = ioread32(rd);
+   iowrite32(0, cmd);
+
+   return true;
+}
+
+static void xgene_enet_rd_mcx_mac(struct xgene_mdio_pdata *pdata,
+ u32 rd_addr, u32 *rd_data)
+{
+   void __iomem *addr, *rd, *cmd, *cmd_done;
+
+   addr = pdata->mac_csr_addr + MAC_ADDR_REG_OFFSET;
+   rd = pdata->mac_csr_addr + MAC_READ_REG_OFFSET;
+   cmd = pdata->mac_csr_addr + MAC_COMMAND_REG_OFFSET;
+   cmd_done = pdata->mac_csr_addr + MAC_COMMAND_DONE_REG_OFFSET;
+
+   if (!xgene_enet_rd_indirect(addr, rd, cmd, cmd_done, rd_addr, rd_data))
+   dev_err(pdata->dev, "MCX mac read failed, addr: 0x%04x\n",
+   rd_addr);
+}
+
+static bool xgene_enet_wr_indirect(void __iomem *addr, void __iomem *wr,
+  void __iomem *cmd, void __iomem *cmd_done,
+   

[PATCH v1 2/6] drivers: net: xgene: Backward compatibility with older firmware

[Iyappan Subramanian]

This patch looks for CONFIG_MDIO_XGENE and based on phy-handle DT/ACPI
fields, sets the mdio_driver flag.  The rest of the driver uses the
this flag for any MDIO management, in the case of backward compatibility.
Also, some code clean up done around mdio configuration/remove.

Signed-off-by: Iyappan Subramanian 
Tested-by: Fushen Chen 
Tested-by: Toan Le 
---
 drivers/net/ethernet/apm/xgene/xgene_enet_hw.c|  59 +++-
 drivers/net/ethernet/apm/xgene/xgene_enet_main.c  | 166 +++---
 drivers/net/ethernet/apm/xgene/xgene_enet_main.h  |   2 +
 drivers/net/ethernet/apm/xgene/xgene_enet_sgmac.c |  13 +-
 4 files changed, 149 insertions(+), 91 deletions(-)

diff --git a/drivers/net/ethernet/apm/xgene/xgene_enet_hw.c 
b/drivers/net/ethernet/apm/xgene/xgene_enet_hw.c
index 6bc8360..38d6ee4 100644
--- a/drivers/net/ethernet/apm/xgene/xgene_enet_hw.c
+++ b/drivers/net/ethernet/apm/xgene/xgene_enet_hw.c
@@ -476,9 +476,13 @@ static void xgene_gmac_reset(struct xgene_enet_pdata 
*pdata)
 static void xgene_enet_configure_clock(struct xgene_enet_pdata *pdata)
 {
struct device *dev = >pdev->dev;
+   struct clk *parent;
 
if (dev->of_node) {
-   struct clk *parent = clk_get_parent(pdata->clk);
+   if (IS_ERR(pdata->clk))
+   return;
+
+   parent = clk_get_parent(pdata->clk);
 
switch (pdata->phy_speed) {
case SPEED_10:
@@ -572,6 +576,9 @@ static void xgene_gmac_init(struct xgene_enet_pdata *pdata)
 {
u32 value;
 
+   if (!pdata->mdio_driver)
+   xgene_gmac_reset(pdata);
+
xgene_gmac_set_speed(pdata);
xgene_gmac_set_mac_addr(pdata);
 
@@ -677,6 +684,14 @@ static int xgene_enet_reset(struct xgene_enet_pdata *pdata)
if (!xgene_ring_mgr_init(pdata))
return -ENODEV;
 
+   if (!pdata->mdio_driver) {
+   if (!IS_ERR(pdata->clk)) {
+   clk_prepare_enable(pdata->clk);
+   clk_disable_unprepare(pdata->clk);
+   clk_prepare_enable(pdata->clk);
+   xgene_enet_ecc_init(pdata);
+   }
+   }
xgene_enet_config_ring_if_assoc(pdata);
 
return 0;
@@ -799,27 +814,9 @@ static int xgene_mdiobus_register(struct xgene_enet_pdata 
*pdata,
  struct mii_bus *mdio)
 {
struct device *dev = >pdev->dev;
-   struct device_node *mdio_np = NULL;
-   struct device_node *child_np;
-   u32 phyid;
 
if (dev->of_node) {
-   for_each_child_of_node(dev->of_node, child_np) {
-   if (of_device_is_compatible(child_np,
-   "apm,xgene-mdio")) {
-   mdio_np = child_np;
-   break;
-   }
-   }
-
-   if (!mdio_np) {
-   mdiobus_free(mdio);
-   return 0;
-   }
-
-   pdata->mdio_driver = false;
-
-   return of_mdiobus_register(mdio, mdio_np);
+   return of_mdiobus_register(mdio, pdata->mdio_np);
} else {
 #ifdef CONFIG_ACPI
struct phy_device *phy;
@@ -838,13 +835,7 @@ static int xgene_mdiobus_register(struct xgene_enet_pdata 
*pdata,
if (ret)
return ret;
 
-   ret = device_property_read_u32(dev, "phy-channel", );
-   if (ret)
-   ret = device_property_read_u32(dev, "phy-addr", );
-   if (ret)
-   return -EINVAL;
-
-   phy = get_phy_device(mdio, phy_id, false);
+   phy = get_phy_device(mdio, pdata->phy_id, false);
if (IS_ERR(phy))
return -EIO;
 
@@ -857,6 +848,8 @@ static int xgene_mdiobus_register(struct xgene_enet_pdata 
*pdata,
return ret;
 #endif
}
+
+   return 0;
 }
 
 int xgene_enet_mdio_config(struct xgene_enet_pdata *pdata)
@@ -884,10 +877,6 @@ int xgene_enet_mdio_config(struct xgene_enet_pdata *pdata)
if (mdio_bus->state == MDIOBUS_REGISTERED)
mdiobus_unregister(pdata->mdio_bus);
mdiobus_free(mdio_bus);
-   if (pdata->mdio_driver) {
-   ret = xgene_enet_phy_connect(ndev);
-   return 0;
-   }
return ret;
}
pdata->mdio_bus = mdio_bus;
@@ -910,11 +899,9 @@ void xgene_enet_mdio_remove(struct xgene_enet_pdata *pdata)
if (pdata->phy_dev)
phy_disconnect(pdata->phy_dev);
 
-   if (!pdata->mdio_driver) {
-   mdiobus_unregister(pdata->mdio_bus);
-   mdiobus_free(pdata->mdio_bus);
-   pdata->mdio_bus = NULL;
-   }
+   

[PATCH v1 0/6] drivers: net: xgene: Fix 1G hot-plug and module support

[Iyappan Subramanian]

This patchset addresses the following issues,

1. hot-plug issue on the SGMII 1G interface
- by adding a driver for MDIO management
2. fixes the kernel crash when the driver loaded as an kernel module
- by fixing hardware cleanups and rearrange kernel API calls

Signed-off-by: Iyappan Subramanian 
---

Iyappan Subramanian (6):
  drivers: net: xgene: MAC and PHY configuration changes
  drivers: net: xgene: Backward compatibility with older firmware
  drivers: net: phy: Add MDIO driver
  dtb: xgene: Add MDIO node
  dtb: xgene: Remove clock nodes
  drivers: net: xgene: Fix module load/unload crash

 arch/arm64/boot/dts/apm/apm-merlin.dts|  10 +
 arch/arm64/boot/dts/apm/apm-mustang.dts   |  12 +
 arch/arm64/boot/dts/apm/apm-shadowcat.dtsi|  23 +-
 arch/arm64/boot/dts/apm/apm-storm.dtsi|  38 +-
 drivers/net/ethernet/apm/xgene/Kconfig|   1 +
 drivers/net/ethernet/apm/xgene/xgene_enet_hw.c| 241 ++
 drivers/net/ethernet/apm/xgene/xgene_enet_hw.h|  18 +-
 drivers/net/ethernet/apm/xgene/xgene_enet_main.c  | 325 -
 drivers/net/ethernet/apm/xgene/xgene_enet_main.h  |  36 +-
 drivers/net/ethernet/apm/xgene/xgene_enet_sgmac.c | 191 +++-
 drivers/net/ethernet/apm/xgene/xgene_enet_sgmac.h |   8 +
 drivers/net/ethernet/apm/xgene/xgene_enet_xgmac.c |  66 ++-
 drivers/net/ethernet/apm/xgene/xgene_enet_xgmac.h |   4 +
 drivers/net/phy/Kconfig   |   7 +
 drivers/net/phy/Makefile  |   1 +
 drivers/net/phy/mdio-xgene.c  | 531 ++
 drivers/net/phy/mdio-xgene.h  | 140 ++
 17 files changed, 1373 insertions(+), 279 deletions(-)
 create mode 100644 drivers/net/phy/mdio-xgene.c
 create mode 100644 drivers/net/phy/mdio-xgene.h

-- 
1.9.1

[PATCH net] openvswitch: update checksum in {push,pop}_mpls

[Simon Horman]

In the case of CHECKSUM_COMPLETE the skb checksum should be updated in
{push,pop}_mpls() as they the type in the ethernet header.

As suggested by Pravin Shelar.

Cc: Pravin Shelar 
Fixes: 25cd9ba0abc0 ("openvswitch: Add basic MPLS support to kernel")
Signed-off-by: Simon Horman 
---
 net/openvswitch/actions.c | 12 
 1 file changed, 12 insertions(+)

diff --git a/net/openvswitch/actions.c b/net/openvswitch/actions.c
index 879185fe183f..d6010b44d423 100644
--- a/net/openvswitch/actions.c
+++ b/net/openvswitch/actions.c
@@ -161,6 +161,12 @@ static int push_mpls(struct sk_buff *skb, struct 
sw_flow_key *key,
skb_postpush_rcsum(skb, new_mpls_lse, MPLS_HLEN);
 
hdr = eth_hdr(skb);
+   if (skb->ip_summed == CHECKSUM_COMPLETE) {
+   __be16 diff[] = { ~(hdr->h_proto), mpls->mpls_ethertype };
+
+   skb->csum = ~csum_partial((char *)diff, sizeof(diff),
+   ~skb->csum);
+   }
hdr->h_proto = mpls->mpls_ethertype;
 
if (!skb->inner_protocol)
@@ -193,6 +199,12 @@ static int pop_mpls(struct sk_buff *skb, struct 
sw_flow_key *key,
 * field correctly in the presence of VLAN tags.
 */
hdr = (struct ethhdr *)(skb_mpls_header(skb) - ETH_HLEN);
+   if (skb->ip_summed == CHECKSUM_COMPLETE) {
+   __be16 diff[] = { ~(hdr->h_proto), ethertype };
+
+   skb->csum = ~csum_partial((char *)diff, sizeof(diff),
+   ~skb->csum);
+   }
hdr->h_proto = ethertype;
if (eth_p_mpls(skb->protocol))
skb->protocol = ethertype;
-- 
2.1.4