* Re: [PATCH net-next v3 1/1] net sched actions: Add support for user cookies
From: Jiri Pirko @ 2017-01-15 15:01 UTC (permalink / raw)
To: Jamal Hadi Salim
Cc: davem, netdev, jiri, paulb, john.fastabend, simon.horman, mrv,
hadarh, ogerlitz, roid, xiyou.wangcong, daniel
In-Reply-To: <1484488879-29560-2-git-send-email-jhs@emojatatu.com>
Sun, Jan 15, 2017 at 03:01:19PM CET, jhs@mojatatu.com wrote:
>From: Jamal Hadi Salim <jhs@mojatatu.com>
>
>Introduce optional 128-bit action cookie.
>Like all other cookie schemes in the networking world (eg in protocols
>like http or existing kernel fib protocol field, etc) the idea is to save
>user state that when retrieved serves as a correlator. The kernel
>_should not_ intepret it. The user can store whatever they wish in the
>128 bits.
>
>Sample exercise(showing variable length use of cookie)
>
>.. create an accept action with cookie a1b2c3d4
>sudo $TC actions add action ok index 1 cookie a1b2c3d4
>
>.. dump all gact actions..
>sudo $TC -s actions ls action gact
>
> action order 0: gact action pass
> random type none pass val 0
> index 1 ref 1 bind 0 installed 5 sec used 5 sec
> Action statistics:
> Sent 0 bytes 0 pkt (dropped 0, overlimits 0 requeues 0)
> backlog 0b 0p requeues 0
> cookie a1b2c3d4
>
>.. bind the accept action to a filter..
>sudo $TC filter add dev lo parent ffff: protocol ip prio 1 \
>u32 match ip dst 127.0.0.1/32 flowid 1:1 action gact index 1
>
>... send some traffic..
>$ ping 127.0.0.1 -c 3
>PING 127.0.0.1 (127.0.0.1) 56(84) bytes of data.
>64 bytes from 127.0.0.1: icmp_seq=1 ttl=64 time=0.020 ms
>64 bytes from 127.0.0.1: icmp_seq=2 ttl=64 time=0.027 ms
>64 bytes from 127.0.0.1: icmp_seq=3 ttl=64 time=0.038 ms
>
>--- 127.0.0.1 ping statistics ---
>3 packets transmitted, 3 received, 0% packet loss, time 2109ms
>rtt min/avg/max/mdev = 0.020/0.028/0.038/0.008 ms 1
>
>... show some stats
>$ sudo $TC -s actions get action gact index 1
>
> action order 1: gact action pass
> random type none pass val 0
> index 1 ref 2 bind 1 installed 204 sec used 5 sec
> Action statistics:
> Sent 12168 bytes 164 pkt (dropped 0, overlimits 0 requeues 0)
> backlog 0b 0p requeues 0
> cookie a1b2c3d4
>
>.. try longer cookie...
>$ sudo $TC actions replace action ok index 1 cookie 1234567890abcdef
>.. dump..
>$ sudo $TC -s actions ls action gact
>
> action order 1: gact action pass
> random type none pass val 0
> index 1 ref 2 bind 1 installed 204 sec used 5 sec
> Action statistics:
> Sent 12168 bytes 164 pkt (dropped 0, overlimits 0 requeues 0)
> backlog 0b 0p requeues 0
> cookie 1234567890abcdef
>
>Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
>---
> include/net/act_api.h | 1 +
> include/net/pkt_cls.h | 19 +++++++++++++------
> include/uapi/linux/pkt_cls.h | 3 +++
> net/sched/act_api.c | 29 +++++++++++++++++++++++++++--
> 4 files changed, 44 insertions(+), 8 deletions(-)
>
>diff --git a/include/net/act_api.h b/include/net/act_api.h
>index 1d71644..0692458 100644
>--- a/include/net/act_api.h
>+++ b/include/net/act_api.h
>@@ -41,6 +41,7 @@ struct tc_action {
> struct rcu_head tcfa_rcu;
> struct gnet_stats_basic_cpu __percpu *cpu_bstats;
> struct gnet_stats_queue __percpu *cpu_qstats;
>+ struct tc_cookie *act_ck;
> };
> #define tcf_head common.tcfa_head
> #define tcf_index common.tcfa_index
>diff --git a/include/net/pkt_cls.h b/include/net/pkt_cls.h
>index f0a0514..f2fafeb 100644
>--- a/include/net/pkt_cls.h
>+++ b/include/net/pkt_cls.h
>@@ -24,15 +24,15 @@ struct tcf_walker {
> }
>
> static inline unsigned long
>-cls_set_class(struct tcf_proto *tp, unsigned long *clp,
>+cls_set_class(struct tcf_proto *tp, unsigned long *clp,
??
> unsigned long cl)
> {
> unsigned long old_cl;
>-
>+
??
> tcf_tree_lock(tp);
> old_cl = __cls_set_class(clp, cl);
> tcf_tree_unlock(tp);
>-
>+
??
> return old_cl;
> }
>
>@@ -237,7 +237,7 @@ static inline int tcf_em_early_end(struct tcf_ematch *em, int result)
>
> return 0;
> }
>-
>+
??
> /**
> * struct tcf_ematch_tree - ematch tree handle
> *
>@@ -246,8 +246,7 @@ static inline int tcf_em_early_end(struct tcf_ematch *em, int result)
> */
> struct tcf_ematch_tree {
> struct tcf_ematch_tree_hdr hdr;
>- struct tcf_ematch * matches;
>-
>+ struct tcf_ematch *matches;
??
> };
>
> /**
>@@ -515,4 +514,12 @@ struct tc_cls_bpf_offload {
> u32 gen_flags;
> };
>
>+
>+/* This structure holds cookie structure that is passed from user
>+ * to the kernel for actions and classifiers
You can add "." to the end of the sentence in V4.
>+ */
>+struct tc_cookie {
>+ unsigned char ck[TC_COOKIE_MAX_SIZE];
>+ unsigned char ck_len;
>+};
> #endif
>diff --git a/include/uapi/linux/pkt_cls.h b/include/uapi/linux/pkt_cls.h
>index 1e5e1dd..2d2414e 100644
>--- a/include/uapi/linux/pkt_cls.h
>+++ b/include/uapi/linux/pkt_cls.h
>@@ -4,6 +4,8 @@
> #include <linux/types.h>
> #include <linux/pkt_sched.h>
>
>+#define TC_COOKIE_MAX_SIZE 16
>+
> /* Action attributes */
> enum {
> TCA_ACT_UNSPEC,
>@@ -12,6 +14,7 @@ enum {
> TCA_ACT_INDEX,
> TCA_ACT_STATS,
> TCA_ACT_PAD,
>+ TCA_ACT_COOKIE,
> __TCA_ACT_MAX
> };
>
>diff --git a/net/sched/act_api.c b/net/sched/act_api.c
>index f04715a..9d05b70 100644
>--- a/net/sched/act_api.c
>+++ b/net/sched/act_api.c
>@@ -24,6 +24,7 @@
> #include <net/net_namespace.h>
> #include <net/sock.h>
> #include <net/sch_generic.h>
>+#include <net/pkt_cls.h>
> #include <net/act_api.h>
> #include <net/netlink.h>
>
>@@ -33,6 +34,7 @@ static void free_tcf(struct rcu_head *head)
>
> free_percpu(p->cpu_bstats);
> free_percpu(p->cpu_qstats);
>+ kfree(p->act_ck);
> kfree(p);
> }
>
>@@ -464,8 +466,8 @@ int tcf_action_destroy(struct list_head *actions, int bind)
> return a->ops->dump(skb, a, bind, ref);
> }
>
>-int
>-tcf_action_dump_1(struct sk_buff *skb, struct tc_action *a, int bind, int ref)
>+int tcf_action_dump_1(struct sk_buff *skb, struct tc_action *a, int bind,
>+ int ref)
??
Otherwise, this looks good. Thanks.
> {
> int err = -EINVAL;
> unsigned char *b = skb_tail_pointer(skb);
>@@ -475,6 +477,12 @@ int tcf_action_destroy(struct list_head *actions, int bind)
> goto nla_put_failure;
> if (tcf_action_copy_stats(skb, a, 0))
> goto nla_put_failure;
>+ if (a->act_ck) {
>+ if (nla_put(skb, TCA_ACT_COOKIE, a->act_ck->ck_len,
>+ a->act_ck))
>+ goto nla_put_failure;
>+ }
>+
> nest = nla_nest_start(skb, TCA_OPTIONS);
> if (nest == NULL)
> goto nla_put_failure;
>@@ -575,6 +583,23 @@ struct tc_action *tcf_action_init_1(struct net *net, struct nlattr *nla,
> if (err < 0)
> goto err_mod;
>
>+ if (tb[TCA_ACT_COOKIE]) {
>+ if (nla_len(tb[TCA_ACT_COOKIE]) > TC_COOKIE_MAX_SIZE) {
>+ err = -EINVAL;
>+ goto err_mod;
>+ }
>+
>+ a->act_ck = kzalloc(sizeof(*a->act_ck), GFP_KERNEL);
>+ if (unlikely(!a->act_ck)) {
>+ err = -ENOMEM;
>+ goto err_mod;
>+ }
>+
>+ memcpy(a->act_ck->ck, nla_data(tb[TCA_ACT_COOKIE]),
>+ nla_len(tb[TCA_ACT_COOKIE]));
>+ a->act_ck->ck_len = nla_len(tb[TCA_ACT_COOKIE]);
>+ }
>+
> /* module count goes up only when brand new policy is created
> * if it exists and is only bound to in a_o->init() then
> * ACT_P_CREATED is not returned (a zero is).
>--
>1.9.1
>
^ permalink raw reply
* [PATCH net 1/1] net sched actions: fix refcnt when GETing of action after bind
From: Jamal Hadi Salim @ 2017-01-15 15:14 UTC (permalink / raw)
To: davem; +Cc: netdev, xiyou.wangcong, Jamal Hadi Salim
From: Jamal Hadi Salim <jhs@mojatatu.com>
Demonstrating the issue:
.. add a drop action
$sudo $TC actions add action drop index 10
.. retrieve it
$ sudo $TC -s actions get action gact index 10
action order 1: gact action drop
random type none pass val 0
index 10 ref 2 bind 0 installed 29 sec used 29 sec
Action statistics:
Sent 0 bytes 0 pkt (dropped 0, overlimits 0 requeues 0)
backlog 0b 0p requeues 0
... bug 1 above: reference is two.
Reference is actually 1 but we forget to subtract 1.
... do a GET again and we see the same issue
try a few times and nothing changes
~$ sudo $TC -s actions get action gact index 10
action order 1: gact action drop
random type none pass val 0
index 10 ref 2 bind 0 installed 31 sec used 31 sec
Action statistics:
Sent 0 bytes 0 pkt (dropped 0, overlimits 0 requeues 0)
backlog 0b 0p requeues 0
... lets try to bind the action to a filter..
$ sudo $TC qdisc add dev lo ingress
$ sudo $TC filter add dev lo parent ffff: protocol ip prio 1 \
u32 match ip dst 127.0.0.1/32 flowid 1:1 action gact index 10
... and now a few GETs:
$ sudo $TC -s actions get action gact index 10
action order 1: gact action drop
random type none pass val 0
index 10 ref 3 bind 1 installed 204 sec used 204 sec
Action statistics:
Sent 0 bytes 0 pkt (dropped 0, overlimits 0 requeues 0)
backlog 0b 0p requeues 0
$ sudo $TC -s actions get action gact index 10
action order 1: gact action drop
random type none pass val 0
index 10 ref 4 bind 1 installed 206 sec used 206 sec
Action statistics:
Sent 0 bytes 0 pkt (dropped 0, overlimits 0 requeues 0)
backlog 0b 0p requeues 0
$ sudo $TC -s actions get action gact index 10
action order 1: gact action drop
random type none pass val 0
index 10 ref 5 bind 1 installed 235 sec used 235 sec
Action statistics:
Sent 0 bytes 0 pkt (dropped 0, overlimits 0 requeues 0)
backlog 0b 0p requeues 0
.... as can be observed the reference count keeps going up.
After the fix
$ sudo $TC actions add action drop index 10
$ sudo $TC -s actions get action gact index 10
action order 1: gact action drop
random type none pass val 0
index 10 ref 1 bind 0 installed 4 sec used 4 sec
Action statistics:
Sent 0 bytes 0 pkt (dropped 0, overlimits 0 requeues 0)
backlog 0b 0p requeues 0
$ sudo $TC -s actions get action gact index 10
action order 1: gact action drop
random type none pass val 0
index 10 ref 1 bind 0 installed 6 sec used 6 sec
Action statistics:
Sent 0 bytes 0 pkt (dropped 0, overlimits 0 requeues 0)
backlog 0b 0p requeues 0
$ sudo $TC qdisc add dev lo ingress
$ sudo $TC filter add dev lo parent ffff: protocol ip prio 1 \
u32 match ip dst 127.0.0.1/32 flowid 1:1 action gact index 10
$ sudo $TC -s actions get action gact index 10
action order 1: gact action drop
random type none pass val 0
index 10 ref 2 bind 1 installed 32 sec used 32 sec
Action statistics:
Sent 0 bytes 0 pkt (dropped 0, overlimits 0 requeues 0)
backlog 0b 0p requeues 0
$ sudo $TC -s actions get action gact index 10
action order 1: gact action drop
random type none pass val 0
index 10 ref 2 bind 1 installed 33 sec used 33 sec
Action statistics:
Sent 0 bytes 0 pkt (dropped 0, overlimits 0 requeues 0)
backlog 0b 0p requeues 0
Fixes: aecc5cefc389 ("net sched actions: fix GETing actions")
Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
---
net/sched/act_api.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/net/sched/act_api.c b/net/sched/act_api.c
index 2095c83..e10456ef6f 100644
--- a/net/sched/act_api.c
+++ b/net/sched/act_api.c
@@ -900,8 +900,6 @@ static int tca_action_flush(struct net *net, struct nlattr *nla,
goto err;
}
act->order = i;
- if (event == RTM_GETACTION)
- act->tcfa_refcnt++;
list_add_tail(&act->list, &actions);
}
@@ -914,7 +912,8 @@ static int tca_action_flush(struct net *net, struct nlattr *nla,
return ret;
}
err:
- tcf_action_destroy(&actions, 0);
+ if (event != RTM_GETACTION)
+ tcf_action_destroy(&actions, 0);
return ret;
}
--
1.9.1
^ permalink raw reply related
* Re: [PATCH net-next v3 1/1] net sched actions: Add support for user cookies
From: Jamal Hadi Salim @ 2017-01-15 15:18 UTC (permalink / raw)
To: Jiri Pirko
Cc: davem, netdev, jiri, paulb, john.fastabend, simon.horman, mrv,
hadarh, ogerlitz, roid, xiyou.wangcong, daniel
In-Reply-To: <20170115150133.GA1774@nanopsycho.orion>
On 17-01-15 10:01 AM, Jiri Pirko wrote:
> Sun, Jan 15, 2017 at 03:01:19PM CET, jhs@mojatatu.com wrote:
>> +cls_set_class(struct tcf_proto *tp, unsigned long *clp,
>
> ??
>
>> unsigned long cl)
>> {
>> unsigned long old_cl;
>> -
>> +
>
> ??
>
>> tcf_tree_lock(tp);
>> old_cl = __cls_set_class(clp, cl);
>> tcf_tree_unlock(tp);
>> -
>> +
>
> ??
>
>> return old_cl;
>> }
>>
>> @@ -237,7 +237,7 @@ static inline int tcf_em_early_end(struct tcf_ematch *em, int result)
>>
>> return 0;
>> }
>> -
>> +
>
> ??
>
All these things with "??" were annoying whitespace and stylistic
things my fingers couldnt resist. It is legit to include such changes
in a patch when touching the code (as long as not a bug fix).
cheers,
jamal
^ permalink raw reply
* Re: [PATCH] secure_seq: initialize secret at boot instead of at runtime
From: Daniel Borkmann @ 2017-01-15 15:37 UTC (permalink / raw)
To: Jason A. Donenfeld, netdev, davem
In-Reply-To: <20170115140136.23135-1-Jason@zx2c4.com>
On 01/15/2017 03:01 PM, Jason A. Donenfeld wrote:
> While the static key stuff is fast, it's not as fast as simply not
> having any code to run. So, this patch generates the secret at
> boot, rather than at runtime.
>
> Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
> ---
> I can imagine this patch not being desirable because:
>
> a) It was done this way in the first place for a reason. (Which?)
See git log:
commit aebda156a570782a86fc4426842152237a19427d
Author: Eric Dumazet <edumazet@google.com>
Date: Mon Apr 29 05:58:52 2013 +0000
net: defer net_secret[] initialization
Instead of feeding net_secret[] at boot time, defer the init
at the point first socket is created.
This permits some platforms to use better entropy sources than
the ones available at boot time.
Signed-off-by: Eric Dumazet <edumazet@google.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
^ permalink raw reply
* Re: [PATCH] secure_seq: initialize secret at boot instead of at runtime
From: Jason A. Donenfeld @ 2017-01-15 15:43 UTC (permalink / raw)
To: Daniel Borkmann; +Cc: Netdev, David Miller
In-Reply-To: <587B973B.8040008@iogearbox.net>
On Sun, Jan 15, 2017 at 4:37 PM, Daniel Borkmann <daniel@iogearbox.net> wrote:
> commit aebda156a570782a86fc4426842152237a19427d
> Author: Eric Dumazet <edumazet@google.com>
> Date: Mon Apr 29 05:58:52 2013 +0000
>
> net: defer net_secret[] initialization
Thanks for that. Suspected as much. Should have checked the log first
myself. Makes sense.
^ permalink raw reply
* Re: [PATCHv3 net-next 2/7] sctp: add support for generating stream reconf ssn reset request chunk
From: Marcelo Ricardo Leitner @ 2017-01-15 15:51 UTC (permalink / raw)
To: Xin Long; +Cc: network dev, linux-sctp, Neil Horman, Vlad Yasevich, davem
In-Reply-To: <6c991b81f4a92cec3da773bae9b19e878e810c88.1484334002.git.lucien.xin@gmail.com>
On Sat, Jan 14, 2017 at 03:15:36AM +0800, Xin Long wrote:
> This patch is to add asoc strreset_outseq and strreset_inseq for
> saving the reconf request sequence, initialize them when create
> assoc and process init, and also to define Incoming and Outgoing
> SSN Reset Request Parameter described in rfc6525 section 4.1 and
> 4.2, As they can be in one same chunk as section rfc6525 3.1-3
> describes, it makes them in one function.
>
> Signed-off-by: Xin Long <lucien.xin@gmail.com>
> ---
> include/linux/sctp.h | 26 ++++++++++++++
> include/net/sctp/sm.h | 5 ++-
> include/net/sctp/structs.h | 3 ++
> net/sctp/associola.c | 1 +
> net/sctp/sm_make_chunk.c | 88 ++++++++++++++++++++++++++++++++++++++++++++++
> 5 files changed, 122 insertions(+), 1 deletion(-)
>
> diff --git a/include/linux/sctp.h b/include/linux/sctp.h
> index cdc3b05..d5da19c 100644
> --- a/include/linux/sctp.h
> +++ b/include/linux/sctp.h
> @@ -200,6 +200,13 @@ typedef enum {
> SCTP_PARAM_SUCCESS_REPORT = cpu_to_be16(0xc005),
> SCTP_PARAM_ADAPTATION_LAYER_IND = cpu_to_be16(0xc006),
>
> + /* RE-CONFIG. Section 4 */
> + SCTP_PARAM_RESET_OUT_REQUEST = cpu_to_be16(0x000d),
> + SCTP_PARAM_RESET_IN_REQUEST = cpu_to_be16(0x000e),
> + SCTP_PARAM_RESET_TSN_REQUEST = cpu_to_be16(0x000f),
> + SCTP_PARAM_RESET_RESPONSE = cpu_to_be16(0x0010),
> + SCTP_PARAM_RESET_ADD_OUT_STREAMS = cpu_to_be16(0x0011),
> + SCTP_PARAM_RESET_ADD_IN_STREAMS = cpu_to_be16(0x0012),
> } sctp_param_t; /* enum */
>
>
> @@ -716,4 +723,23 @@ struct sctp_reconf_chunk {
> __u8 params[0];
> } __packed;
>
> +struct sctp_strreset_req {
> + sctp_paramhdr_t param_hdr;
> + __u32 request_seq;
> +} __packed;
> +
> +struct sctp_strreset_outreq {
> + sctp_paramhdr_t param_hdr;
> + __u32 request_seq;
This should be:
+ struct sctp_strreset_req strreset_req;
Use the definition you created above for the encapsulation and make the
embedding evident.
Like it's done for sctp_chunkhdr_t.
> + __u32 response_seq;
> + __u32 send_reset_at_tsn;
> + __u16 list_of_streams[0];
> +} __packed;
> +
> +struct sctp_strreset_inreq {
> + sctp_paramhdr_t param_hdr;
> + __u32 request_seq;
Same here.
> + __u16 list_of_streams[0];
> +} __packed;
> +
> #endif /* __LINUX_SCTP_H__ */
> diff --git a/include/net/sctp/sm.h b/include/net/sctp/sm.h
> index ca6c971..3462cb0 100644
> --- a/include/net/sctp/sm.h
> +++ b/include/net/sctp/sm.h
> @@ -259,7 +259,10 @@ struct sctp_chunk *sctp_make_fwdtsn(const struct sctp_association *asoc,
> __u32 new_cum_tsn, size_t nstreams,
> struct sctp_fwdtsn_skip *skiplist);
> struct sctp_chunk *sctp_make_auth(const struct sctp_association *asoc);
> -
> +struct sctp_chunk *sctp_make_strreset_req(
> + const struct sctp_association *asoc,
> + __u16 stream_num, __u16 *stream_list,
> + bool out, bool in);
> void sctp_chunk_assign_tsn(struct sctp_chunk *);
> void sctp_chunk_assign_ssn(struct sctp_chunk *);
>
> diff --git a/include/net/sctp/structs.h b/include/net/sctp/structs.h
> index 4741ec2..3dc983e 100644
> --- a/include/net/sctp/structs.h
> +++ b/include/net/sctp/structs.h
> @@ -1865,6 +1865,9 @@ struct sctp_association {
> temp:1, /* Is it a temporary association? */
> prsctp_enable:1;
>
> + __u32 strreset_outseq; /* Update after receiving response */
> + __u32 strreset_inseq; /* Update after receiving request */
> +
> struct sctp_priv_assoc_stats stats;
>
> int sent_cnt_removable;
> diff --git a/net/sctp/associola.c b/net/sctp/associola.c
> index 36294f7..42ece6f 100644
> --- a/net/sctp/associola.c
> +++ b/net/sctp/associola.c
> @@ -207,6 +207,7 @@ static struct sctp_association *sctp_association_init(struct sctp_association *a
> * association to the same value as the initial TSN.
> */
> asoc->addip_serial = asoc->c.initial_tsn;
> + asoc->strreset_outseq = asoc->c.initial_tsn;
>
> INIT_LIST_HEAD(&asoc->addip_chunk_list);
> INIT_LIST_HEAD(&asoc->asconf_ack_list);
> diff --git a/net/sctp/sm_make_chunk.c b/net/sctp/sm_make_chunk.c
> index fd58097..172385c 100644
> --- a/net/sctp/sm_make_chunk.c
> +++ b/net/sctp/sm_make_chunk.c
> @@ -1844,6 +1844,7 @@ struct sctp_association *sctp_unpack_cookie(
> retval->next_tsn = retval->c.initial_tsn;
> retval->ctsn_ack_point = retval->next_tsn - 1;
> retval->addip_serial = retval->c.initial_tsn;
> + retval->strreset_outseq = retval->c.initial_tsn;
> retval->adv_peer_ack_point = retval->ctsn_ack_point;
> retval->peer.prsctp_capable = retval->c.prsctp_capable;
> retval->peer.adaptation_ind = retval->c.adaptation_ind;
> @@ -2387,6 +2388,8 @@ int sctp_process_init(struct sctp_association *asoc, struct sctp_chunk *chunk,
> asoc->peer.i.initial_tsn =
> ntohl(peer_init->init_hdr.initial_tsn);
>
> + asoc->strreset_inseq = asoc->peer.i.initial_tsn;
> +
> /* Apply the upper bounds for output streams based on peer's
> * number of inbound streams.
> */
> @@ -3559,3 +3562,88 @@ static struct sctp_chunk *sctp_make_reconf(
>
> return retval;
> }
> +
> +/* RE-CONFIG 4.1 (STREAM OUT RESET)
> + * 0 1 2 3
> + * 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
> + * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> + * | Parameter Type = 13 | Parameter Length = 16 + 2 * N |
> + * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> + * | Re-configuration Request Sequence Number |
> + * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> + * | Re-configuration Response Sequence Number |
> + * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> + * | Sender's Last Assigned TSN |
> + * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> + * | Stream Number 1 (optional) | Stream Number 2 (optional) |
> + * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> + * / ...... /
> + * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> + * | Stream Number N-1 (optional) | Stream Number N (optional) |
> + * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> + *
> + * RE-CONFIG 4.2 (STREAM IN RESET)
> + * 0 1 2 3
> + * 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
> + * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> + * | Parameter Type = 14 | Parameter Length = 8 + 2 * N |
> + * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> + * | Re-configuration Request Sequence Number |
> + * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> + * | Stream Number 1 (optional) | Stream Number 2 (optional) |
> + * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> + * / ...... /
> + * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> + * | Stream Number N-1 (optional) | Stream Number N (optional) |
> + * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> + */
> +struct sctp_chunk *sctp_make_strreset_req(
> + const struct sctp_association *asoc,
> + __u16 stream_num, __u16 *stream_list,
> + bool out, bool in)
> +{
> + struct sctp_strreset_outreq outreq;
> + __u16 stream_len = stream_num * 2;
> + struct sctp_strreset_inreq inreq;
> + struct sctp_chunk *retval;
> + __u16 outlen, inlen, i;
> +
> + outlen = (sizeof(outreq) + stream_len) * out;
> + inlen = (sizeof(inreq) + stream_len) * in;
> +
> + retval = sctp_make_reconf(asoc, outlen + inlen);
> + if (!retval)
> + return NULL;
> +
> + for (i = 0; i < stream_num; i++)
> + stream_list[i] = htons(stream_list[i]);
> +
> + if (outlen) {
> + outreq.param_hdr.type = SCTP_PARAM_RESET_OUT_REQUEST;
> + outreq.param_hdr.length = htons(outlen);
> + outreq.request_seq = htonl(asoc->strreset_outseq);
> + outreq.response_seq = htonl(asoc->strreset_inseq - 1);
> + outreq.send_reset_at_tsn = htonl(asoc->next_tsn - 1);
> +
> + sctp_addto_chunk(retval, sizeof(outreq), &outreq);
> +
> + if (stream_len)
> + sctp_addto_chunk(retval, stream_len, stream_list);
> + }
> +
> + if (inlen) {
> + inreq.param_hdr.type = SCTP_PARAM_RESET_IN_REQUEST;
> + inreq.param_hdr.length = htons(inlen);
> + inreq.request_seq = htonl(asoc->strreset_outseq + out);
> +
> + sctp_addto_chunk(retval, sizeof(inreq), &inreq);
> +
> + if (stream_len)
> + sctp_addto_chunk(retval, stream_len, stream_list);
> + }
> +
> + for (i = 0; i < stream_num; i++)
> + stream_list[i] = ntohs(stream_list[i]);
> +
> + return retval;
> +}
> --
> 2.1.0
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-sctp" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply
* Re: [PATCH net-next v3 07/10] net: Relocate dev_to_net_device() into core
From: Florian Fainelli @ 2017-01-15 17:20 UTC (permalink / raw)
To: Greg KH
Cc: netdev, Jason Cooper, Andrew Lunn, Sebastian Hesselbarth,
Gregory Clement, Russell King, Vivien Didelot, David S. Miller,
moderated list:ARM SUB-ARCHITECTURES, open list
In-Reply-To: <20170115110753.GG26374@kroah.com>
On 01/15/2017 03:07 AM, Greg KH wrote:
> On Sat, Jan 14, 2017 at 01:47:10PM -0800, Florian Fainelli wrote:
>> dev_to_net_device() is moved from net/dsa/dsa.c to net/core/dev.c since
>> it going to be used by net/dsa/dsa2.c and the namespace of the function
>> justifies making it available to other users potentially.
>>
>> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
>> ---
>> include/linux/netdevice.h | 2 ++
>> net/core/dev.c | 19 +++++++++++++++++++
>> net/dsa/dsa.c | 18 ------------------
>> 3 files changed, 21 insertions(+), 18 deletions(-)
>>
>> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
>> index 97ae0ac513ee..6d021c37b774 100644
>> --- a/include/linux/netdevice.h
>> +++ b/include/linux/netdevice.h
>> @@ -4390,4 +4390,6 @@ do { \
>> #define PTYPE_HASH_SIZE (16)
>> #define PTYPE_HASH_MASK (PTYPE_HASH_SIZE - 1)
>>
>> +struct net_device *dev_to_net_device(struct device *dev);
>> +
>> #endif /* _LINUX_NETDEVICE_H */
>> diff --git a/net/core/dev.c b/net/core/dev.c
>> index ad5959e56116..7547e2ccc06b 100644
>> --- a/net/core/dev.c
>> +++ b/net/core/dev.c
>> @@ -8128,6 +8128,25 @@ const char *netdev_drivername(const struct net_device *dev)
>> return empty;
>> }
>>
>> +struct net_device *dev_to_net_device(struct device *dev)
>> +{
>> + struct device *d;
>> +
>> + d = device_find_class(dev, "net");
>> + if (d) {
>> + struct net_device *nd;
>> +
>> + nd = to_net_dev(d);
>> + dev_hold(nd);
>> + put_device(d);
>> +
>> + return nd;
>> + }
>> +
>> + return NULL;
>> +}
>> +EXPORT_SYMBOL_GPL(dev_to_net_device);
>
> This really isn't just a "struct device to net device cast" type
> function, (otherwise a simple container_of() would work). You are
> walking the device tree and assuming it is in a specific order so that
> this function works. You better document the hell out of this,
> otherwise people are going to try to use this and get very confused,
> very quickly...
Fair enough. Does that make it clearer how the device_find_class() is
used though? Maybe device_find_class() should be named
device_find_by_class_name() instead?
--
Florian
^ permalink raw reply
* Re: [PATCH net-next] net: ipv6: Allow shorthand delete of all nexthops in multipath route
From: David Ahern @ 2017-01-15 17:21 UTC (permalink / raw)
To: netdev, David Miller; +Cc: ddutt
In-Reply-To: <1484351668-8962-1-git-send-email-dsa@cumulusnetworks.com>
On 1/13/17 4:54 PM, David Ahern wrote:
> The same notation does not work with IPv6 because of how multipath routes
> are implemented for IPv6. For IPv6 only the first nexthop of a multipath
> route is deleted if the request contains only a prefix and length. This
> leads to unnecessary complexity in userspace dealing with IPv6 multipath
> routes.
>
> This patch allows all nexthops to be deleted without specifying each one
> in the delete request
Dave: please drop this patch. I found a problem with this approach. Will re-send a new version along with some other ipv6 multipath patches.
^ permalink raw reply
* Re: [PATCH] net: add regs attribute to phy device for user diagnose
From: Andrew Lunn @ 2017-01-15 17:21 UTC (permalink / raw)
To: yuan linyu; +Cc: Florian Fainelli, David S . Miller, netdev, yuan linyu
In-Reply-To: <1484445063.4555.8.camel@163.com>
On Sun, Jan 15, 2017 at 09:51:03AM +0800, yuan linyu wrote:
> On ???, 2017-01-14 at 10:35 -0800, Florian Fainelli wrote:
> > On 01/14/2017 08:24 AM, Andrew Lunn wrote:
> > >
> > > On Sat, Jan 14, 2017 at 10:46:31AM +0800, yuan linyu wrote:
> > > >
> > > > From: yuan linyu <Linyu.Yuan@alcatel-sbell.com.cn>
> > > >
> > > > if phy device have register(s) configuration problem,
> > > > user can use this attribute to diagnose.
> > > > this feature need phy driver maintainer implement.
> > > what is wrong with mii-tool -vv ?
> > Agreed, and without an actual user of this API (ethtool?), nor a PHY
> > driver implementing it, we cannot quite see how you want to make use of
> > this.
> I hope user/developer can read this attribute file "regs" to do
> a full check of all registers value, and they can write any register
> inside PHY through this file.
Since this is intended for debug, it should not be sysfs, but debugfs.
However, in general, Linux does not allow user space to peek and poke
device registers. Can you point me at examples where i can do the same
to my GPU? SATA controller? Ethernet controller, I2C temperature
sensor? Any device?
Andrew
^ permalink raw reply
* Re: [PATCH net-next v3 06/10] net: dsa: Migrate to device_find_class()
From: Florian Fainelli @ 2017-01-15 17:27 UTC (permalink / raw)
To: Greg KH
Cc: netdev, Jason Cooper, Andrew Lunn, Sebastian Hesselbarth,
Gregory Clement, Russell King, Vivien Didelot, David S. Miller,
moderated list:ARM SUB-ARCHITECTURES, open list
In-Reply-To: <20170115110628.GF26374@kroah.com>
On 01/15/2017 03:06 AM, Greg KH wrote:
> On Sat, Jan 14, 2017 at 01:47:09PM -0800, Florian Fainelli wrote:
>> Now that the base device driver code provides an identical
>> implementation of dev_find_class() utilize device_find_class() instead
>> of our own version of it.
>>
>> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
>> ---
>> net/dsa/dsa.c | 22 ++--------------------
>> 1 file changed, 2 insertions(+), 20 deletions(-)
>>
>> diff --git a/net/dsa/dsa.c b/net/dsa/dsa.c
>> index 2306d1b87c83..77fa4c4f5828 100644
>> --- a/net/dsa/dsa.c
>> +++ b/net/dsa/dsa.c
>> @@ -455,29 +455,11 @@ EXPORT_SYMBOL_GPL(dsa_switch_resume);
>> #endif
>>
>> /* platform driver init and cleanup *****************************************/
>> -static int dev_is_class(struct device *dev, void *class)
>> -{
>> - if (dev->class != NULL && !strcmp(dev->class->name, class))
>> - return 1;
>> -
>> - return 0;
>> -}
>> -
>> -static struct device *dev_find_class(struct device *parent, char *class)
>> -{
>> - if (dev_is_class(parent, class)) {
>> - get_device(parent);
>> - return parent;
>> - }
>> -
>> - return device_find_child(parent, class, dev_is_class);
>> -}
>> -
>> struct mii_bus *dsa_host_dev_to_mii_bus(struct device *dev)
>> {
>> struct device *d;
>>
>> - d = dev_find_class(dev, "mdio_bus");
>> + d = device_find_class(dev, "mdio_bus");
>> if (d != NULL) {
>> struct mii_bus *bus;
>
> You want a peer of your device on a specific class? What is this for?
It's not a peer of our device, it's a separate device reference from the
one looked up in the "net" class. In the classic, and now deprecated DSA
device driver model, a "dsa" platform device would represent one or more
Ethernet switches, connected via a MDIO bus (this reference above), and
one Ethernet device (the CPU/host/management interface). This was
completely violating the Linux device driver model and imposed
limitations on what bus would be used, and we did not have proper struct
device references (therefore no adequate hierarchy either).
Thanks to the work of Andrew, we now have proper MDIO, SPI, GPIO, I2C,
PCI, platform and drivers that allow us to register with DSA as a
specialized kind of device (so we are now finally using the right Linux
Device Driver model). What we still need though, in order to our switch
to the networking stack is a reference to the master/host network device
since we mangle packets in and out of it.
>
>> @@ -495,7 +477,7 @@ static struct net_device *dev_to_net_device(struct device *dev)
>> {
>> struct device *d;
>>
>> - d = dev_find_class(dev, "net");
>> + d = device_find_class(dev, "net");
>> if (d != NULL) {
>> struct net_device *nd;
>
> Again, huh? What is the device heirachy here that is so odd that this
> type of function is needed?
An Ethernet switch managed by DSA needs to have one ore more references
to a host/CPU/management network interface, this is what this struct
device reference is here for.
^ permalink raw reply
* sctp: kernel memory overwrite attempt detected in sctp_getsockopt_assoc_stats
From: Dmitry Vyukov @ 2017-01-15 17:29 UTC (permalink / raw)
To: Vladislav Yasevich, Neil Horman, David Miller, linux-sctp, netdev,
LKML, Kees Cook
Cc: syzkaller
Hello,
I've enabled CONFIG_HARDENED_USERCOPY_PAGESPAN on syzkaller fuzzer and
now I am seeing lots of:
usercopy: kernel memory overwrite attempt detected to ffff8801a74f6f10
(<spans multiple pages>) (256 bytes)
kernel BUG at mm/usercopy.c:75!
invalid opcode: 0000 [#1] SMP KASAN
Dumping ftrace buffer:
(ftrace buffer empty)
Modules linked in:
CPU: 1 PID: 15686 Comm: syz-executor3 Not tainted 4.9.0 #1
Hardware name: Google Google Compute Engine/Google Compute Engine,
BIOS Google 01/01/2011
task: ffff8801c89b2500 task.stack: ffff8801a74f0000
RIP: 0010:[<ffffffff81a1b041>] [<ffffffff81a1b041>] report_usercopy
mm/usercopy.c:67 [inline]
RIP: 0010:[<ffffffff81a1b041>] [<ffffffff81a1b041>]
__check_object_size+0x2d1/0x9ec mm/usercopy.c:278
RSP: 0018:ffff8801a74f6cd0 EFLAGS: 00010286
RAX: 000000000000006b RBX: ffffffff84500120 RCX: 0000000000000000
RDX: 000000000000006b RSI: ffffffff815a7791 RDI: ffffed0034e9ed8c
RBP: ffff8801a74f6e48 R08: 0000000000000001 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000001 R12: ffff8801a74f6f10
R13: 0000000000000100 R14: ffffffff845000e0 R15: ffff8801a74f700f
FS: 00007f80918de700(0000) GS:ffff8801dc100000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 0000000020058ffc CR3: 00000001cc1cc000 CR4: 00000000001406e0
Stack:
ffffffff8598fcc8 0000000000000000 000077ff80000000 ffffea0005c99608
ffffffff844fff40 ffffffff844fff40 0000000041b58ab3 ffffffff84ae0fa0
ffffffff81a1ad70 ffff8801c89b2500 dead000000000100 ffffffff814d4425
Call Trace:
[<ffffffff83e4ece9>] check_object_size include/linux/thread_info.h:129 [inline]
[<ffffffff83e4ece9>] copy_from_user arch/x86/include/asm/uaccess.h:692 [inline]
[<ffffffff83e4ece9>] sctp_getsockopt_assoc_stats+0x169/0xa10
net/sctp/socket.c:6182
[<ffffffff83e5cc52>] sctp_getsockopt+0x1af2/0x66a0 net/sctp/socket.c:6556
[<ffffffff834f92c5>] sock_common_getsockopt+0x95/0xd0 net/core/sock.c:2649
[<ffffffff834f4910>] SYSC_getsockopt net/socket.c:1788 [inline]
[<ffffffff834f4910>] SyS_getsockopt+0x240/0x380 net/socket.c:1770
[<ffffffff81009798>] do_syscall_64+0x2e8/0x930 arch/x86/entry/common.c:280
[<ffffffff84370a49>] entry_SYSCALL64_slow_path+0x25/0x25
Code: b0 fe ff ff e8 e1 25 ce ff 48 8b 85 b0 fe ff ff 4d 89 e9 4c 89
e1 4c 89 f2 48 89 de 48 c7 c7 a0 01 50 84 49 89 c0 e8 51 d9 e0 ff <0f>
0b e8 b8 25 ce ff 4c 89 f2 4c 89 ee 4c 89 e7 e8 6a 1b fc ff
RIP [<ffffffff81a1b041>] report_usercopy mm/usercopy.c:67 [inline]
RIP [<ffffffff81a1b041>] __check_object_size+0x2d1/0x9ec mm/usercopy.c:278
RSP <ffff8801a74f6cd0>
---[ end trace 5e438996b2c0b35d ]---
I am not sure why check_object_size flags this an a bug,
copy_from_user copies into a stack object:
static int sctp_getsockopt_assoc_stats(struct sock *sk, int len,
char __user *optval,
int __user *optlen)
{
struct sctp_assoc_stats sas;
len = min_t(size_t, len, sizeof(sas));
if (copy_from_user(&sas, optval, len))
return -EFAULT;
Kees, can this be a false positive?
On commit f4d3935e4f4884ba80561db5549394afb8eef8f7.
^ permalink raw reply
* Re: [PATCH net-next] net/sched: cls_flower: Add user specified data
From: Paul Blakey @ 2017-01-15 17:36 UTC (permalink / raw)
To: Jiri Pirko, Jamal Hadi Salim
Cc: paulb, David S. Miller, netdev, Jiri Pirko, Hadar Hen Zion,
Or Gerlitz, Roi Dayan, Roman Mashak
In-Reply-To: <20170108171251.GF1971@nanopsycho>
On 08/01/2017 19:12, Jiri Pirko wrote:
> Mon, Jan 02, 2017 at 03:59:49PM CET, jhs@mojatatu.com wrote:
>>
>> We have been using a cookie as well for actions (which we have been
>> using but have been too lazy to submit so far). I am going to port
>> it over to the newer kernels and post it.
>
> Hard to deal with something we can't look at :)
>
>
>> In our case that is intended to be opaque to the kernel i.e kernel
>> never inteprets it; in that case it is similar to the kernel
>> FIB protocol field.
>
> In case of this patch, kernel also never interprets it. What makes you
> think otherwise. Bot kernel, it is always a binary blob.
>
>
>>
>> In your case - could this cookie have been a class/flowid
>> (a 32 bit)?
>> And would it not make more sense for it the cookie to be
>> generic to all classifiers? i.e why is it specific to flower?
>
> Correct, makes sense to have it generic for all cls and perhaps also
> acts.
>
>
Hi all,
I've started implementing a general cookie for all classifiers,
I added the cookie on tcf_proto struct but then I realized that it won't
work as I want. What I want is cookie per internal element (those that
are identified by handles), which we can have many per one tcf_proto:
tc filter add dev <DEV> parent ffff: prio 1 cookie 0x123 basic action drop
tc filter add dev <DEV> parent ffff: prio 1 cookie 0x456 "port6" basic
action drop
tc filter add dev <DEV> parent ffff: prio 1 cookie 0x777 basic action drop
So there is three options to do that:
1) Implement it in each classifier, using some generic function. (kinda
like stats, where classifiler_dump() calls tcf_exts_dump_stats())
2) Have a global hash table for cookies [prio,handle]->[cookie]
3) Have it on flower only for now :)
With the first one we will have to change each classifier (or leave it
unsupported as default).
Second is less code to change (instead of changing each classifier), but
might be slower. I'm afraid how it will affect dumping several filters.
Third is this patch.
Thanks,
Paul.
>>
>> cheers,
>> jamal
>>
>> On 17-01-02 08:13 AM, Paul Blakey wrote:
>>> This is to support saving extra data that might be helpful on retrieval.
>>> First use case is upcoming openvswitch flow offloads, extra data will
>>> include UFID and port mappings for each added flow.
>>>
>>> Signed-off-by: Paul Blakey <paulb@mellanox.com>
>>> Reviewed-by: Roi Dayan <roid@mellanox.com>
>>> Acked-by: Jiri Pirko <jiri@mellanox.com>
>>> ---
>>> include/uapi/linux/pkt_cls.h | 3 +++
>>> net/sched/cls_flower.c | 22 +++++++++++++++++++++-
>>> 2 files changed, 24 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/include/uapi/linux/pkt_cls.h b/include/uapi/linux/pkt_cls.h
>>> index cb4bcdc..ca9bbe3 100644
>>> --- a/include/uapi/linux/pkt_cls.h
>>> +++ b/include/uapi/linux/pkt_cls.h
>>> @@ -471,10 +471,13 @@ enum {
>>> TCA_FLOWER_KEY_ICMPV6_TYPE, /* u8 */
>>> TCA_FLOWER_KEY_ICMPV6_TYPE_MASK,/* u8 */
>>>
>>> + TCA_FLOWER_COOKIE, /* binary */
>>> +
>>> __TCA_FLOWER_MAX,
>>> };
>>>
>>> #define TCA_FLOWER_MAX (__TCA_FLOWER_MAX - 1)
>>> +#define FLOWER_MAX_COOKIE_SIZE 128
>>>
>>> enum {
>>> TCA_FLOWER_KEY_FLAGS_IS_FRAGMENT = (1 << 0),
>>> diff --git a/net/sched/cls_flower.c b/net/sched/cls_flower.c
>>> index 333f8e2..e2f5b25 100644
>>> --- a/net/sched/cls_flower.c
>>> +++ b/net/sched/cls_flower.c
>>> @@ -85,6 +85,8 @@ struct cls_fl_filter {
>>> struct rcu_head rcu;
>>> struct tc_to_netdev tc;
>>> struct net_device *hw_dev;
>>> + size_t cookie_len;
>>> + long cookie[0];
>>> };
>>>
>>> static unsigned short int fl_mask_range(const struct fl_flow_mask *mask)
>>> @@ -794,6 +796,9 @@ static int fl_change(struct net *net, struct sk_buff *in_skb,
>>> struct cls_fl_filter *fnew;
>>> struct nlattr *tb[TCA_FLOWER_MAX + 1];
>>> struct fl_flow_mask mask = {};
>>> + const struct nlattr *attr;
>>> + size_t cookie_len = 0;
>>> + void *cookie;
>>> int err;
>>>
>>> if (!tca[TCA_OPTIONS])
>>> @@ -806,10 +811,22 @@ static int fl_change(struct net *net, struct sk_buff *in_skb,
>>> if (fold && handle && fold->handle != handle)
>>> return -EINVAL;
>>>
>>> - fnew = kzalloc(sizeof(*fnew), GFP_KERNEL);
>>> + if (tb[TCA_FLOWER_COOKIE]) {
>>> + attr = tb[TCA_FLOWER_COOKIE];
>>> + cookie_len = nla_len(attr);
>>> + cookie = nla_data(attr);
>>> + if (cookie_len > FLOWER_MAX_COOKIE_SIZE)
>>> + return -EINVAL;
>>> + }
>>> +
>>> + fnew = kzalloc(sizeof(*fnew) + cookie_len, GFP_KERNEL);
>>> if (!fnew)
>>> return -ENOBUFS;
>>>
>>> + fnew->cookie_len = cookie_len;
>>> + if (cookie_len)
>>> + memcpy(fnew->cookie, cookie, cookie_len);
>>> +
>>> err = tcf_exts_init(&fnew->exts, TCA_FLOWER_ACT, 0);
>>> if (err < 0)
>>> goto errout;
>>> @@ -1151,6 +1168,9 @@ static int fl_dump(struct net *net, struct tcf_proto *tp, unsigned long fh,
>>>
>>> nla_put_u32(skb, TCA_FLOWER_FLAGS, f->flags);
>>>
>>> + if (f->cookie_len)
>>> + nla_put(skb, TCA_FLOWER_COOKIE, f->cookie_len, f->cookie);
>>> +
>>> if (tcf_exts_dump(skb, &f->exts))
>>> goto nla_put_failure;
>>>
>>>
>>
^ permalink raw reply
* Re: [PATCH net-next v3 05/10] drivers: base: Add device_find_class()
From: Florian Fainelli @ 2017-01-15 17:39 UTC (permalink / raw)
To: Greg KH
Cc: netdev, Jason Cooper, Andrew Lunn, Sebastian Hesselbarth,
Gregory Clement, Russell King, Vivien Didelot, David S. Miller,
moderated list:ARM SUB-ARCHITECTURES, open list
In-Reply-To: <20170115110455.GE26374@kroah.com>
On 01/15/2017 03:04 AM, Greg KH wrote:
> On Sat, Jan 14, 2017 at 01:47:08PM -0800, Florian Fainelli wrote:
>> Add a helper function to lookup a device reference given a class name.
>> This is a preliminary patch to remove adhoc code from net/dsa/dsa.c and
>> make it more generic.
>>
>> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
>> ---
>> drivers/base/core.c | 19 +++++++++++++++++++
>> include/linux/device.h | 1 +
>> 2 files changed, 20 insertions(+)
>>
>> diff --git a/drivers/base/core.c b/drivers/base/core.c
>> index 020ea7f05520..3dd6047c10d8 100644
>> --- a/drivers/base/core.c
>> +++ b/drivers/base/core.c
>> @@ -2065,6 +2065,25 @@ struct device *device_find_child(struct device *parent, void *data,
>> }
>> EXPORT_SYMBOL_GPL(device_find_child);
>>
>> +static int dev_is_class(struct device *dev, void *class)
>> +{
>> + if (dev->class != NULL && !strcmp(dev->class->name, class))
>> + return 1;
>> +
>> + return 0;
>> +}
>> +
>> +struct device *device_find_class(struct device *parent, char *class)
>
> Why are you using the char * for a class, and not just a pointer to
> "struct class"? That seems to be the most logical one, no need to rely
> on string comparisons here.
A more reflective name of what that does would probably be
device_find_by_class_name() or something alike.
>
> Also, what is this being used for? You aren't trying to walk up the
> device heirachy to find a specific "type" of device, are you? If so,
> ugh, I ranted about this in the past when the hyperv driver was trying
> to do such a thing...
What's a better way to do that though?
>
>> +{
>> + if (dev_is_class(parent, class)) {
>> + get_device(parent);
>> + return parent;
>> + }
>> +
>> + return device_find_child(parent, class, dev_is_class);
>
> You are trying to find a peer device with the same parent that belongs
> to a specific class?
Correct, network devices, and MDIO bus devices usually (always?) set
dev.parent.
>
> Again, what is this being used for?
See my other replies in patches 6, 7 and how it is used in patches 8 and
10 for instance.
>
> And all exported driver core functions should have full kerneldoc
> information for them so that people know how to use them, and what the
> constraints are (see device_find_child() as an example.) Please do that
> here as well because you are returning a pointer to a structure with the
> reference count incremented, callers need to know that.
Sure.
--
Florian
^ permalink raw reply
* Re: [PATCH net-next v3 06/10] net: dsa: Migrate to device_find_class()
From: Greg KH @ 2017-01-15 17:39 UTC (permalink / raw)
To: Florian Fainelli
Cc: netdev, Jason Cooper, Andrew Lunn, Sebastian Hesselbarth,
Gregory Clement, Russell King, Vivien Didelot, David S. Miller,
moderated list:ARM SUB-ARCHITECTURES, open list
In-Reply-To: <fc247aeb-a8d6-5ba9-c26d-cb057a5ac34d@gmail.com>
On Sun, Jan 15, 2017 at 09:27:22AM -0800, Florian Fainelli wrote:
>
>
> On 01/15/2017 03:06 AM, Greg KH wrote:
> > On Sat, Jan 14, 2017 at 01:47:09PM -0800, Florian Fainelli wrote:
> >> Now that the base device driver code provides an identical
> >> implementation of dev_find_class() utilize device_find_class() instead
> >> of our own version of it.
> >>
> >> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
> >> ---
> >> net/dsa/dsa.c | 22 ++--------------------
> >> 1 file changed, 2 insertions(+), 20 deletions(-)
> >>
> >> diff --git a/net/dsa/dsa.c b/net/dsa/dsa.c
> >> index 2306d1b87c83..77fa4c4f5828 100644
> >> --- a/net/dsa/dsa.c
> >> +++ b/net/dsa/dsa.c
> >> @@ -455,29 +455,11 @@ EXPORT_SYMBOL_GPL(dsa_switch_resume);
> >> #endif
> >>
> >> /* platform driver init and cleanup *****************************************/
> >> -static int dev_is_class(struct device *dev, void *class)
> >> -{
> >> - if (dev->class != NULL && !strcmp(dev->class->name, class))
> >> - return 1;
> >> -
> >> - return 0;
> >> -}
> >> -
> >> -static struct device *dev_find_class(struct device *parent, char *class)
> >> -{
> >> - if (dev_is_class(parent, class)) {
> >> - get_device(parent);
> >> - return parent;
> >> - }
> >> -
> >> - return device_find_child(parent, class, dev_is_class);
> >> -}
> >> -
> >> struct mii_bus *dsa_host_dev_to_mii_bus(struct device *dev)
> >> {
> >> struct device *d;
> >>
> >> - d = dev_find_class(dev, "mdio_bus");
> >> + d = device_find_class(dev, "mdio_bus");
> >> if (d != NULL) {
> >> struct mii_bus *bus;
> >
> > You want a peer of your device on a specific class? What is this for?
>
> It's not a peer of our device, it's a separate device reference from the
> one looked up in the "net" class. In the classic, and now deprecated DSA
> device driver model, a "dsa" platform device would represent one or more
> Ethernet switches, connected via a MDIO bus (this reference above), and
> one Ethernet device (the CPU/host/management interface). This was
> completely violating the Linux device driver model and imposed
> limitations on what bus would be used, and we did not have proper struct
> device references (therefore no adequate hierarchy either).
>
> Thanks to the work of Andrew, we now have proper MDIO, SPI, GPIO, I2C,
> PCI, platform and drivers that allow us to register with DSA as a
> specialized kind of device (so we are now finally using the right Linux
> Device Driver model). What we still need though, in order to our switch
> to the networking stack is a reference to the master/host network device
> since we mangle packets in and out of it.
Ok, but where in the tree are you trying to find this other device? Are
you just going to randomly find any device that happens to be of the
specific class type? That seems really fragile and broken :(
You should NEVER have to walk the device tree to find stuff. Why can't
you have a pointer to the device you need to talk to some other way?
What exactly is the relationship between these devices (a ascii-art tree
or sysfs tree output might be nice) so I can try to understand what is
going on here.
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH net-next v3 00/10] net: dsa: Support for pdata in dsa2
From: Florian Fainelli @ 2017-01-15 17:40 UTC (permalink / raw)
To: Greg KH
Cc: netdev, Jason Cooper, Andrew Lunn, Sebastian Hesselbarth,
Gregory Clement, Russell King, Vivien Didelot, David S. Miller,
moderated list:ARM SUB-ARCHITECTURES, open list
In-Reply-To: <20170115110831.GH26374@kroah.com>
On 01/15/2017 03:08 AM, Greg KH wrote:
> On Sat, Jan 14, 2017 at 01:47:03PM -0800, Florian Fainelli wrote:
>> Hi all,
>>
>> This is not exactly new, and was sent before, although back then, I did not
>> have an user of the pre-declared MDIO board information, but now we do. Note
>> that I have additional changes queued up to have b53 register platform data for
>> MIPS bcm47xx and bcm63xx.
>>
>> Yes I know that we should have the Orion platforms eventually be converted to
>> Device Tree, but until that happens, I don't want any remaining users of the
>> old "dsa" platform device (hence the previous DTS submissions for ARM/mvebu)
>> and, there will be platforms out there that most likely won't never see DT
>> coming their way (BCM47xx is almost 100% sure, BCM63xx maybe not in a distant
>> future).
>>
>> We would probably want the whole series to be merged via David Miller's tree
>> to simplify things.
>>
>> Greg, can you Ack/Nack patch 5 since it touched the core LDD?
>
> I've NAKed them for now, you need to describe what you are trying to do
> here, as it doesn't make any sense to me at the moment.
For one, this is moving *existing* code from net/dsa/dsa.c part into the
device core for device_find_class() and part into the network device
core for dev_to_net_device(). Patch 8 is where this actually gets used.
See my individual replies for more details.
Even though the existing code is there in net/dsa/dsa.c, at this point,
and for the sake of getting these patches merged via David, I can
probably just keep it where it is (like what patch series v1 did) and
just namespace it with dsa_. Later on, if this is deemed valuable to
other parts of the kernel, I can try to relocate it to the device core,
does that sound acceptable?
--
Florian
^ permalink raw reply
* Re: [PATCH net-next v3 07/10] net: Relocate dev_to_net_device() into core
From: Greg KH @ 2017-01-15 17:40 UTC (permalink / raw)
To: Florian Fainelli
Cc: netdev, Jason Cooper, Andrew Lunn, Sebastian Hesselbarth,
Gregory Clement, Russell King, Vivien Didelot, David S. Miller,
moderated list:ARM SUB-ARCHITECTURES, open list
In-Reply-To: <621be9fe-0869-2d0a-fd49-97468c2d4972@gmail.com>
On Sun, Jan 15, 2017 at 09:20:06AM -0800, Florian Fainelli wrote:
>
>
> On 01/15/2017 03:07 AM, Greg KH wrote:
> > On Sat, Jan 14, 2017 at 01:47:10PM -0800, Florian Fainelli wrote:
> >> dev_to_net_device() is moved from net/dsa/dsa.c to net/core/dev.c since
> >> it going to be used by net/dsa/dsa2.c and the namespace of the function
> >> justifies making it available to other users potentially.
> >>
> >> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
> >> ---
> >> include/linux/netdevice.h | 2 ++
> >> net/core/dev.c | 19 +++++++++++++++++++
> >> net/dsa/dsa.c | 18 ------------------
> >> 3 files changed, 21 insertions(+), 18 deletions(-)
> >>
> >> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> >> index 97ae0ac513ee..6d021c37b774 100644
> >> --- a/include/linux/netdevice.h
> >> +++ b/include/linux/netdevice.h
> >> @@ -4390,4 +4390,6 @@ do { \
> >> #define PTYPE_HASH_SIZE (16)
> >> #define PTYPE_HASH_MASK (PTYPE_HASH_SIZE - 1)
> >>
> >> +struct net_device *dev_to_net_device(struct device *dev);
> >> +
> >> #endif /* _LINUX_NETDEVICE_H */
> >> diff --git a/net/core/dev.c b/net/core/dev.c
> >> index ad5959e56116..7547e2ccc06b 100644
> >> --- a/net/core/dev.c
> >> +++ b/net/core/dev.c
> >> @@ -8128,6 +8128,25 @@ const char *netdev_drivername(const struct net_device *dev)
> >> return empty;
> >> }
> >>
> >> +struct net_device *dev_to_net_device(struct device *dev)
> >> +{
> >> + struct device *d;
> >> +
> >> + d = device_find_class(dev, "net");
> >> + if (d) {
> >> + struct net_device *nd;
> >> +
> >> + nd = to_net_dev(d);
> >> + dev_hold(nd);
> >> + put_device(d);
> >> +
> >> + return nd;
> >> + }
> >> +
> >> + return NULL;
> >> +}
> >> +EXPORT_SYMBOL_GPL(dev_to_net_device);
> >
> > This really isn't just a "struct device to net device cast" type
> > function, (otherwise a simple container_of() would work). You are
> > walking the device tree and assuming it is in a specific order so that
> > this function works. You better document the hell out of this,
> > otherwise people are going to try to use this and get very confused,
> > very quickly...
>
> Fair enough. Does that make it clearer how the device_find_class() is
> used though? Maybe device_find_class() should be named
> device_find_by_class_name() instead?
Better, but you are just poking around randomly in the device tree and
"hoping" you get it right. What happens if devices move around? You
are assuming some sort of heirachy here that I don't understand at
all...
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH net-next v3 00/10] net: dsa: Support for pdata in dsa2
From: Greg KH @ 2017-01-15 17:49 UTC (permalink / raw)
To: Florian Fainelli
Cc: netdev, Jason Cooper, Andrew Lunn, Sebastian Hesselbarth,
Gregory Clement, Russell King, Vivien Didelot, David S. Miller,
moderated list:ARM SUB-ARCHITECTURES, open list
In-Reply-To: <bc0d9ea7-17ee-abc3-b2aa-4d241413b9a3@gmail.com>
On Sun, Jan 15, 2017 at 09:40:24AM -0800, Florian Fainelli wrote:
> On 01/15/2017 03:08 AM, Greg KH wrote:
> > On Sat, Jan 14, 2017 at 01:47:03PM -0800, Florian Fainelli wrote:
> >> Hi all,
> >>
> >> This is not exactly new, and was sent before, although back then, I did not
> >> have an user of the pre-declared MDIO board information, but now we do. Note
> >> that I have additional changes queued up to have b53 register platform data for
> >> MIPS bcm47xx and bcm63xx.
> >>
> >> Yes I know that we should have the Orion platforms eventually be converted to
> >> Device Tree, but until that happens, I don't want any remaining users of the
> >> old "dsa" platform device (hence the previous DTS submissions for ARM/mvebu)
> >> and, there will be platforms out there that most likely won't never see DT
> >> coming their way (BCM47xx is almost 100% sure, BCM63xx maybe not in a distant
> >> future).
> >>
> >> We would probably want the whole series to be merged via David Miller's tree
> >> to simplify things.
> >>
> >> Greg, can you Ack/Nack patch 5 since it touched the core LDD?
> >
> > I've NAKed them for now, you need to describe what you are trying to do
> > here, as it doesn't make any sense to me at the moment.
>
> For one, this is moving *existing* code from net/dsa/dsa.c part into the
> device core for device_find_class() and part into the network device
> core for dev_to_net_device(). Patch 8 is where this actually gets used.
> See my individual replies for more details.
>
> Even though the existing code is there in net/dsa/dsa.c, at this point,
> and for the sake of getting these patches merged via David, I can
> probably just keep it where it is (like what patch series v1 did) and
> just namespace it with dsa_. Later on, if this is deemed valuable to
> other parts of the kernel, I can try to relocate it to the device core,
> does that sound acceptable?
Nope!
I really want to try to understand what you all are doing with the
device tree that you feel that blindly walking it actually comes up with
a valid result.
See my other email about wanting to see a tree, we can take it from that
thread to try to consolidate all of these different ones.
And sorry, I know you are just trying to move code around, but this
isn't the first time this has come up, and I think it needs to be
resolved properly.
thanks,
greg k-h
^ permalink raw reply
* [PATCH net V2] net/mlx5e: Fix a -Wmaybe-uninitialized warning
From: Or Gerlitz @ 2017-01-15 17:50 UTC (permalink / raw)
To: David S. Miller; +Cc: Hadar Har-Zion, netdev, Arnd Bergmann, Or Gerlitz
From: Arnd Bergmann <arnd@arndb.de>
As found by Olof's build bot, we gain a harmless warning about a
potential uninitialized variable reference in mlx5:
drivers/net/ethernet/mellanox/mlx5/core/en_tc.c: In function 'parse_tc_fdb_actions':
drivers/net/ethernet/mellanox/mlx5/core/en_tc.c:769:13: warning: 'out_dev' may be used uninitialized in this function [-Wmaybe-uninitialized]
drivers/net/ethernet/mellanox/mlx5/core/en_tc.c:811:21: note: 'out_dev' was declared here
This was introduced through the addition of an 'IS_ERR/PTR_ERR' pair
that gcc is unfortunately unable to completely figure out.
The problem being gcc cannot tell that if(IS_ERR()) in
mlx5e_route_lookup_ipv4() is equivalent to checking if(err) later,
so it assumes that 'out_dev' is used after the 'return PTR_ERR(rt)'.
The PTR_ERR_OR_ZERO() case by comparison is fairly easy to detect
by gcc, so it can't get that wrong, so it no longer warns.
Hadar Hen Zion already attempted to fix the warning earlier by adding fake
initializations, but that ended up not fully addressing all warnings, so
I'm reverting it now that it is no longer needed.
Link: http://arm-soc.lixom.net/buildlogs/mainline/v4.10-rc3-98-gcff3b2c/
Fixes: a42485eb0ee4 ("net/mlx5e: TC ipv4 tunnel encap offload error flow fixes")
Fixes: a757d108dc1a ("net/mlx5e: Fix kbuild warnings for uninitialized parameters")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Or Gerlitz <ogerlitz@mellanox.com>
---
changes from V1:
- kept calling ip_route_output_key() only when CONFIG_INET is defined
drivers/net/ethernet/mellanox/mlx5/core/en_tc.c | 11 +++++++----
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c b/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
index 118cea5..46bef6a 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
@@ -668,9 +668,12 @@ static int mlx5e_route_lookup_ipv4(struct mlx5e_priv *priv,
int ttl;
#if IS_ENABLED(CONFIG_INET)
+ int ret;
+
rt = ip_route_output_key(dev_net(mirred_dev), fl4);
- if (IS_ERR(rt))
- return PTR_ERR(rt);
+ ret = PTR_ERR_OR_ZERO(rt);
+ if (ret)
+ return ret;
#else
return -EOPNOTSUPP;
#endif
@@ -741,8 +744,8 @@ static int mlx5e_create_encap_header_ipv4(struct mlx5e_priv *priv,
struct flowi4 fl4 = {};
char *encap_header;
int encap_size;
- __be32 saddr = 0;
- int ttl = 0;
+ __be32 saddr;
+ int ttl;
int err;
encap_header = kzalloc(max_encap_size, GFP_KERNEL);
--
2.3.7
^ permalink raw reply related
* Re: [PATCH net-next v3 06/10] net: dsa: Migrate to device_find_class()
From: Florian Fainelli @ 2017-01-15 17:52 UTC (permalink / raw)
To: Greg KH
Cc: Andrew Lunn, Jason Cooper, Vivien Didelot, netdev, Russell King,
open list, Gregory Clement, David S. Miller,
moderated list:ARM SUB-ARCHITECTURES, Sebastian Hesselbarth
In-Reply-To: <20170115173949.GA19268@kroah.com>
On 01/15/2017 09:39 AM, Greg KH wrote:
> On Sun, Jan 15, 2017 at 09:27:22AM -0800, Florian Fainelli wrote:
>>
>>
>> On 01/15/2017 03:06 AM, Greg KH wrote:
>>> On Sat, Jan 14, 2017 at 01:47:09PM -0800, Florian Fainelli wrote:
>>>> Now that the base device driver code provides an identical
>>>> implementation of dev_find_class() utilize device_find_class() instead
>>>> of our own version of it.
>>>>
>>>> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
>>>> ---
>>>> net/dsa/dsa.c | 22 ++--------------------
>>>> 1 file changed, 2 insertions(+), 20 deletions(-)
>>>>
>>>> diff --git a/net/dsa/dsa.c b/net/dsa/dsa.c
>>>> index 2306d1b87c83..77fa4c4f5828 100644
>>>> --- a/net/dsa/dsa.c
>>>> +++ b/net/dsa/dsa.c
>>>> @@ -455,29 +455,11 @@ EXPORT_SYMBOL_GPL(dsa_switch_resume);
>>>> #endif
>>>>
>>>> /* platform driver init and cleanup *****************************************/
>>>> -static int dev_is_class(struct device *dev, void *class)
>>>> -{
>>>> - if (dev->class != NULL && !strcmp(dev->class->name, class))
>>>> - return 1;
>>>> -
>>>> - return 0;
>>>> -}
>>>> -
>>>> -static struct device *dev_find_class(struct device *parent, char *class)
>>>> -{
>>>> - if (dev_is_class(parent, class)) {
>>>> - get_device(parent);
>>>> - return parent;
>>>> - }
>>>> -
>>>> - return device_find_child(parent, class, dev_is_class);
>>>> -}
>>>> -
>>>> struct mii_bus *dsa_host_dev_to_mii_bus(struct device *dev)
>>>> {
>>>> struct device *d;
>>>>
>>>> - d = dev_find_class(dev, "mdio_bus");
>>>> + d = device_find_class(dev, "mdio_bus");
>>>> if (d != NULL) {
>>>> struct mii_bus *bus;
>>>
>>> You want a peer of your device on a specific class? What is this for?
>>
>> It's not a peer of our device, it's a separate device reference from the
>> one looked up in the "net" class. In the classic, and now deprecated DSA
>> device driver model, a "dsa" platform device would represent one or more
>> Ethernet switches, connected via a MDIO bus (this reference above), and
>> one Ethernet device (the CPU/host/management interface). This was
>> completely violating the Linux device driver model and imposed
>> limitations on what bus would be used, and we did not have proper struct
>> device references (therefore no adequate hierarchy either).
>>
>> Thanks to the work of Andrew, we now have proper MDIO, SPI, GPIO, I2C,
>> PCI, platform and drivers that allow us to register with DSA as a
>> specialized kind of device (so we are now finally using the right Linux
>> Device Driver model). What we still need though, in order to our switch
>> to the networking stack is a reference to the master/host network device
>> since we mangle packets in and out of it.
>
> Ok, but where in the tree are you trying to find this other device? Are
> you just going to randomly find any device that happens to be of the
> specific class type? That seems really fragile and broken :(
The search is not really random, since the platform data for the DSA
switch we want to register gives us a reference to a device registered
via any bus type, and we know (by contract it has to) that it has to
provide a network device of some kind, we do a specific search in the
"net" class for that purpose. I agree this is not great.
>
> You should NEVER have to walk the device tree to find stuff. Why can't
> you have a pointer to the device you need to talk to some other way?
We do, I think the existing code just tries to be extra careful here and
make sure that the device reference we got is actually part of the "net"
class, indicating that the reference passed is indeed pointing to a
network device, and not a random device.
>
> What exactly is the relationship between these devices (a ascii-art tree
> or sysfs tree output might be nice) so I can try to understand what is
> going on here.
OK, let me try and let's take the case of only one Ethernet switch in
the system for all simplicity:
- switch device drivers get probed by any kind of bus allocate and
register a dsa_switch with dsa_register_switch()
- this switch device has platform data (struct dsa_chip_data) that
describes its port layout (number of ports mostly) and has one struct
device references to which Ethernet network interface these ports connect to
- a dsa_switch_tree is created, this struct dsa_switch is attached to
the dsa_switch_tree at position 0 in the tree, we invoke a bunch of
switch driver operations
- we conclude with attaching this dsa_switch_tree to the network device
we looked up and assigning the tree to the dsa_ptr member
Documentation/networking/dsa/dsa.txt has some ascii art drawing that
sort of capture what is going on.
--
Florian
^ permalink raw reply
* net: stmmac: don't use netdev_[dbg, info, ..] before net_device is registered
From: Heiner Kallweit @ 2017-01-15 18:17 UTC (permalink / raw)
To: David S. Miller; +Cc: netdev@vger.kernel.org
Don't use netdev_info and friends before the net_device is registered.
This avoids ugly messages like
"meson8b-dwmac c9410000.ethernet (unnamed net_device) (uninitialized):
Enable RX Mitigation via HW Watchdog Timer"
Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 19 ++++++++++---------
1 file changed, 10 insertions(+), 9 deletions(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index d481c5f7..38e1fb75 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -3305,9 +3305,9 @@ int stmmac_dvr_probe(struct device *device,
(priv->plat->maxmtu >= ndev->min_mtu))
ndev->max_mtu = priv->plat->maxmtu;
else if (priv->plat->maxmtu < ndev->min_mtu)
- netdev_warn(priv->dev,
- "%s: warning: maxmtu having invalid value (%d)\n",
- __func__, priv->plat->maxmtu);
+ dev_warn(priv->device,
+ "%s: warning: maxmtu having invalid value (%d)\n",
+ __func__, priv->plat->maxmtu);
if (flow_ctrl)
priv->flow_ctrl = FLOW_AUTO; /* RX/TX pause on */
@@ -3319,7 +3319,8 @@ int stmmac_dvr_probe(struct device *device,
*/
if ((priv->synopsys_id >= DWMAC_CORE_3_50) && (!priv->plat->riwt_off)) {
priv->use_riwt = 1;
- netdev_info(priv->dev, "Enable RX Mitigation via HW Watchdog Timer\n");
+ dev_info(priv->device,
+ "Enable RX Mitigation via HW Watchdog Timer\n");
}
netif_napi_add(ndev, &priv->napi, stmmac_poll, 64);
@@ -3345,17 +3346,17 @@ int stmmac_dvr_probe(struct device *device,
/* MDIO bus Registration */
ret = stmmac_mdio_register(ndev);
if (ret < 0) {
- netdev_err(priv->dev,
- "%s: MDIO bus (id: %d) registration failed",
- __func__, priv->plat->bus_id);
+ dev_err(priv->device,
+ "%s: MDIO bus (id: %d) registration failed",
+ __func__, priv->plat->bus_id);
goto error_mdio_register;
}
}
ret = register_netdev(ndev);
if (ret) {
- netdev_err(priv->dev, "%s: ERROR %i registering the device\n",
- __func__, ret);
+ dev_err(priv->device, "%s: ERROR %i registering the device\n",
+ __func__, ret);
goto error_netdev_register;
}
--
2.11.0
^ permalink raw reply related
* [PATCH] net: stmmac: don't use netdev_[dbg, info, ..] before net_device is registered
From: Heiner Kallweit @ 2017-01-15 18:19 UTC (permalink / raw)
To: David S. Miller; +Cc: netdev@vger.kernel.org
Don't use netdev_info and friends before the net_device is registered.
This avoids ugly messages like
"meson8b-dwmac c9410000.ethernet (unnamed net_device) (uninitialized):
Enable RX Mitigation via HW Watchdog Timer"
Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 19 ++++++++++---------
1 file changed, 10 insertions(+), 9 deletions(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index d481c5f7..38e1fb75 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -3305,9 +3305,9 @@ int stmmac_dvr_probe(struct device *device,
(priv->plat->maxmtu >= ndev->min_mtu))
ndev->max_mtu = priv->plat->maxmtu;
else if (priv->plat->maxmtu < ndev->min_mtu)
- netdev_warn(priv->dev,
- "%s: warning: maxmtu having invalid value (%d)\n",
- __func__, priv->plat->maxmtu);
+ dev_warn(priv->device,
+ "%s: warning: maxmtu having invalid value (%d)\n",
+ __func__, priv->plat->maxmtu);
if (flow_ctrl)
priv->flow_ctrl = FLOW_AUTO; /* RX/TX pause on */
@@ -3319,7 +3319,8 @@ int stmmac_dvr_probe(struct device *device,
*/
if ((priv->synopsys_id >= DWMAC_CORE_3_50) && (!priv->plat->riwt_off)) {
priv->use_riwt = 1;
- netdev_info(priv->dev, "Enable RX Mitigation via HW Watchdog Timer\n");
+ dev_info(priv->device,
+ "Enable RX Mitigation via HW Watchdog Timer\n");
}
netif_napi_add(ndev, &priv->napi, stmmac_poll, 64);
@@ -3345,17 +3346,17 @@ int stmmac_dvr_probe(struct device *device,
/* MDIO bus Registration */
ret = stmmac_mdio_register(ndev);
if (ret < 0) {
- netdev_err(priv->dev,
- "%s: MDIO bus (id: %d) registration failed",
- __func__, priv->plat->bus_id);
+ dev_err(priv->device,
+ "%s: MDIO bus (id: %d) registration failed",
+ __func__, priv->plat->bus_id);
goto error_mdio_register;
}
}
ret = register_netdev(ndev);
if (ret) {
- netdev_err(priv->dev, "%s: ERROR %i registering the device\n",
- __func__, ret);
+ dev_err(priv->device, "%s: ERROR %i registering the device\n",
+ __func__, ret);
goto error_netdev_register;
}
--
2.11.0
^ permalink raw reply related
* Re: net: stmmac: don't use netdev_[dbg, info, ..] before net_device is registered
From: Joe Perches @ 2017-01-15 18:23 UTC (permalink / raw)
To: Heiner Kallweit, David S. Miller; +Cc: netdev@vger.kernel.org
In-Reply-To: <da21972e-87cd-370c-e9ff-219fc82ebd10@gmail.com>
On Sun, 2017-01-15 at 19:17 +0100, Heiner Kallweit wrote:
> Don't use netdev_info and friends before the net_device is registered.
> This avoids ugly messages like
> "meson8b-dwmac c9410000.ethernet (unnamed net_device) (uninitialized):
> Enable RX Mitigation via HW Watchdog Timer"
It'd be nice to mention the new message prefix.
^ permalink raw reply
* Re: net: stmmac: don't use netdev_[dbg, info, ..] before net_device is registered
From: Heiner Kallweit @ 2017-01-15 18:28 UTC (permalink / raw)
To: Joe Perches, David S. Miller; +Cc: netdev@vger.kernel.org
In-Reply-To: <1484504620.14692.8.camel@perches.com>
Am 15.01.2017 um 19:23 schrieb Joe Perches:
> On Sun, 2017-01-15 at 19:17 +0100, Heiner Kallweit wrote:
>> Don't use netdev_info and friends before the net_device is registered.
>> This avoids ugly messages like
>> "meson8b-dwmac c9410000.ethernet (unnamed net_device) (uninitialized):
>> Enable RX Mitigation via HW Watchdog Timer"
>
> It'd be nice to mention the new message prefix.
>
Sorry, I thought it's obvious because here the message trail looks
like this:
meson8b-dwmac c9410000.ethernet: DMA HW capability register supported
meson8b-dwmac c9410000.ethernet: Normal descriptors
meson8b-dwmac c9410000.ethernet: RX Checksum Offload Engine supported
meson8b-dwmac c9410000.ethernet: COE Type 2
meson8b-dwmac c9410000.ethernet: TX Checksum insertion supported
meson8b-dwmac c9410000.ethernet: Wake-Up On Lan supported
meson8b-dwmac c9410000.ethernet (unnamed net_device) (uninitialized): Enable RX Mitigation via HW Watchdog Timer
Better update the commit message and send a v2?
^ permalink raw reply
* Re: [PATCH net-next] net/sched: cls_flower: Add user specified data
From: John Fastabend @ 2017-01-15 19:08 UTC (permalink / raw)
To: Paul Blakey, Jiri Pirko, Jamal Hadi Salim
Cc: David S. Miller, netdev, Jiri Pirko, Hadar Hen Zion, Or Gerlitz,
Roi Dayan, Roman Mashak
In-Reply-To: <040c6876-a323-0a35-229f-46bc33b6de11@mellanox.com>
On 17-01-15 09:36 AM, Paul Blakey wrote:
>
>
> On 08/01/2017 19:12, Jiri Pirko wrote:
>> Mon, Jan 02, 2017 at 03:59:49PM CET, jhs@mojatatu.com wrote:
>>>
>>> We have been using a cookie as well for actions (which we have been
>>> using but have been too lazy to submit so far). I am going to port
>>> it over to the newer kernels and post it.
>>
>> Hard to deal with something we can't look at :)
>>
>>
>>> In our case that is intended to be opaque to the kernel i.e kernel
>>> never inteprets it; in that case it is similar to the kernel
>>> FIB protocol field.
>>
>> In case of this patch, kernel also never interprets it. What makes you
>> think otherwise. Bot kernel, it is always a binary blob.
>>
>>
>>>
>>> In your case - could this cookie have been a class/flowid
>>> (a 32 bit)?
>>> And would it not make more sense for it the cookie to be
>>> generic to all classifiers? i.e why is it specific to flower?
>>
>> Correct, makes sense to have it generic for all cls and perhaps also
>> acts.
>>
>>
>
> Hi all,
> I've started implementing a general cookie for all classifiers,
> I added the cookie on tcf_proto struct but then I realized that it won't work as
> I want. What I want is cookie per internal element (those that are identified by
> handles), which we can have many per one tcf_proto:
>
> tc filter add dev <DEV> parent ffff: prio 1 cookie 0x123 basic action drop
> tc filter add dev <DEV> parent ffff: prio 1 cookie 0x456 "port6" basic action drop
> tc filter add dev <DEV> parent ffff: prio 1 cookie 0x777 basic action drop
>
> So there is three options to do that:
> 1) Implement it in each classifier, using some generic function. (kinda like
> stats, where classifiler_dump() calls tcf_exts_dump_stats())
> 2) Have a global hash table for cookies [prio,handle]->[cookie]
> 3) Have it on flower only for now :)
>
> With the first one we will have to change each classifier (or leave it
> unsupported as default).
> Second is less code to change (instead of changing each classifier), but might
> be slower. I'm afraid how it will affect dumping several filters.
> Third is this patch.
>
> Thanks,
> Paul.
Avoid (2) it creates way more problems than its worth like is it locked/not
locked, how is it synced, collisions, etc. Seems way overkill.
I like the generic functionality of (1) but unless we see this pop up in
different filters I wouldn't require it for now. If you just do it in flower
(option 3) when its added to another classifier we can generalize it. As a
middle ground creating nice helper routines as needed goes a long way.
.John
^ permalink raw reply
* Re: [PATCH net-next v3 06/10] net: dsa: Migrate to device_find_class()
From: Andrew Lunn @ 2017-01-15 19:16 UTC (permalink / raw)
To: Florian Fainelli
Cc: Jason Cooper, Vivien Didelot, Greg KH, Russell King, open list,
netdev, Gregory Clement, David S. Miller,
moderated list:ARM SUB-ARCHITECTURES, Sebastian Hesselbarth
In-Reply-To: <a9ea1f45-b9a5-d840-68e0-9bf9bc7a6598@gmail.com>
> > What exactly is the relationship between these devices (a ascii-art tree
> > or sysfs tree output might be nice) so I can try to understand what is
> > going on here.
Hi Greg, Florian
A few diagrams and trees which might help understand what is going on.
The first diagram comes from the 2008 patch which added all this code:
+-----------+ +-----------+
| | RGMII | |
| +-------+ +------ 1000baseT MDI ("WAN")
| | | 6-port +------ 1000baseT MDI ("LAN1")
| CPU | | ethernet +------ 1000baseT MDI ("LAN2")
| |MIImgmt| switch +------ 1000baseT MDI ("LAN3")
| +-------+ w/5 PHYs +------ 1000baseT MDI ("LAN4")
| | | |
+-----------+ +-----------+
We have an ethernet switch and a host CPU. The switch is connected to
the CPU in two different ways. RGMII allows us to get Ethernet frames
from the CPU into the switch. MIImgmt, is the management bus normally
used for Ethernet PHYs, but Marvell switches also use it for Managing
switches.
The diagram above is the simplest setup. You can have multiple
Ethernet switches, connected together via switch ports. Each switch
has its own MIImgmt connect to the CPU, but there is only one RGMII
link.
When this code was designed back in 2008, it was decided to represent
this is a platform device, and it has a platform_data, which i have
slightly edited to keep it simple:
struct dsa_platform_data {
/*
* Reference to a Linux network interface that connects
* to the root switch chip of the tree.
*/
struct device *netdev;
/*
* Info structs describing each of the switch chips
* connected via this network interface.
*/
int nr_chips;
struct dsa_chip_data *chip;
};
This netdev is the CPU side of the RGMII interface.
Each switch has a dsa_chip_data, again edited:
struct dsa_chip_data {
/*
* How to access the switch configuration registers.
*/
struct device *host_dev;
int sw_addr;
...
}
The host_dev is the CPU side of the MIImgmt, and we have the address
the switch is using on the bus.
During probe of this platform device, we need to get from the
struct device *netdev to a struct net_device *dev.
So the code looks in the device net class to find the device
| | | |-- f1074000.ethernet
| | | | |-- deferred_probe
| | | | |-- driver -> ../../../../../bus/platform/drivers/mvneta
| | | | |-- driver_override
| | | | |-- modalias
| | | | |-- net
| | | | | `-- eth1
| | | | | |-- addr_assign_type
| | | | | |-- address
| | | | | |-- addr_len
| | | | | |-- broadcast
| | | | | |-- carrier
| | | | | |-- carrier_changes
| | | | | |-- deferred_probe
| | | | | |-- device -> ../../../f1074000.ethernet
and then use container_of() to get the net_device.
Similarly, the code needs to get from struct device *host_dev to a struct mii_bus *.
| | | |-- f1072004.mdio
| | | | |-- deferred_probe
| | | | |-- driver -> ../../../../../bus/platform/drivers/orion-mdio
| | | | |-- driver_override
| | | | |-- mdio_bus
| | | | | `-- f1072004.mdio-mi
| | | | | |-- deferred_probe
| | | | | |-- device -> ../../../f1072004.mdio
Andrew
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox