* 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
* Re: [PATCH 3/3] powerpc: bpf: implement in-register swap for 64-bit endian operations
From: Benjamin Herrenschmidt @ 2017-01-15 15:00 UTC (permalink / raw)
To: 'Naveen N. Rao', David Laight
Cc: daniel@iogearbox.net, ast@fb.com, netdev@vger.kernel.org,
linuxppc-dev@lists.ozlabs.org, davem@davemloft.net
In-Reply-To: <20170113175201.GD3470@naverao1-tp.localdomain>
On Fri, 2017-01-13 at 23:22 +0530, 'Naveen N. Rao' wrote:
> > That rather depends on whether the processor has a store to load forwarder
> > that will satisfy the read from the store buffer.
> > I don't know about ppc, but at least some x86 will do that.
>
> Interesting - good to know that.
>
> However, I don't think powerpc does that and in-register swap is likely
> faster regardless. Note also that gcc prefers this form at higher
> optimization levels.
Of course powerpc has a load-store forwarder these days, however, I
wouldn't be surprised if the in-register form was still faster on some
implementations, but this needs to be tested.
Ideally, you'd want to try to "optimize" load+swap or swap+store
though.
Cheers,
Ben.
^ permalink raw reply
* [PATCH net-next v2] ipv6: sr: add missing Kbuild export for header files
From: David Lebrun @ 2017-01-15 14:26 UTC (permalink / raw)
To: netdev; +Cc: David Lebrun
Add missing IPv6-SR header files in include/uapi/linux/Kbuild.
Also, prevent seg6_lwt_headroom() from being exported and add
missing linux/types.h include.
Signed-off-by: David Lebrun <david.lebrun@uclouvain.be>
---
include/uapi/linux/Kbuild | 4 ++++
include/uapi/linux/seg6.h | 2 ++
include/uapi/linux/seg6_hmac.h | 1 +
include/uapi/linux/seg6_iptunnel.h | 4 ++++
4 files changed, 11 insertions(+)
diff --git a/include/uapi/linux/Kbuild b/include/uapi/linux/Kbuild
index f330ba4..e600b50 100644
--- a/include/uapi/linux/Kbuild
+++ b/include/uapi/linux/Kbuild
@@ -379,6 +379,10 @@ header-y += sctp.h
header-y += sdla.h
header-y += seccomp.h
header-y += securebits.h
+header-y += seg6_genl.h
+header-y += seg6.h
+header-y += seg6_hmac.h
+header-y += seg6_iptunnel.h
header-y += selinux_netlink.h
header-y += sem.h
header-y += serial_core.h
diff --git a/include/uapi/linux/seg6.h b/include/uapi/linux/seg6.h
index c396a80..3349659 100644
--- a/include/uapi/linux/seg6.h
+++ b/include/uapi/linux/seg6.h
@@ -14,6 +14,8 @@
#ifndef _UAPI_LINUX_SEG6_H
#define _UAPI_LINUX_SEG6_H
+#include <linux/types.h>
+
/*
* SRH
*/
diff --git a/include/uapi/linux/seg6_hmac.h b/include/uapi/linux/seg6_hmac.h
index b652dfd..e691c75 100644
--- a/include/uapi/linux/seg6_hmac.h
+++ b/include/uapi/linux/seg6_hmac.h
@@ -1,6 +1,7 @@
#ifndef _UAPI_LINUX_SEG6_HMAC_H
#define _UAPI_LINUX_SEG6_HMAC_H
+#include <linux/types.h>
#include <linux/seg6.h>
#define SEG6_HMAC_SECRET_LEN 64
diff --git a/include/uapi/linux/seg6_iptunnel.h b/include/uapi/linux/seg6_iptunnel.h
index 0f7dbd2..7a7183d 100644
--- a/include/uapi/linux/seg6_iptunnel.h
+++ b/include/uapi/linux/seg6_iptunnel.h
@@ -33,6 +33,8 @@ enum {
SEG6_IPTUN_MODE_ENCAP,
};
+#ifdef __KERNEL__
+
static inline size_t seg6_lwt_headroom(struct seg6_iptunnel_encap *tuninfo)
{
int encap = (tuninfo->mode == SEG6_IPTUN_MODE_ENCAP);
@@ -42,3 +44,5 @@ static inline size_t seg6_lwt_headroom(struct seg6_iptunnel_encap *tuninfo)
}
#endif
+
+#endif
--
2.7.3
^ permalink raw reply related
* [PATCH iproute2] tc: flower: Fix flower output for src and dst ports
From: Roi Dayan @ 2017-01-15 14:23 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev, Simon Horman, Roi Dayan, Paul Blakey
This fix a missing use case after the introduction of enum flower_endpoint.
Fixes: 6910d65661a3 ("tc: flower: introduce enum flower_endpoint")
Signed-off-by: Roi Dayan <roid@mellanox.com>
Signed-off-by: Paul Blakey <paulb@mellanox.com>
---
tc/f_flower.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/tc/f_flower.c b/tc/f_flower.c
index 71e9515..1dbc532 100644
--- a/tc/f_flower.c
+++ b/tc/f_flower.c
@@ -744,10 +744,10 @@ static int flower_print_opt(struct filter_util *qu, FILE *f,
tb[TCA_FLOWER_KEY_IPV6_SRC],
tb[TCA_FLOWER_KEY_IPV6_SRC_MASK]);
- nl_type = flower_port_attr_type(ip_proto, false);
+ nl_type = flower_port_attr_type(ip_proto, FLOWER_ENDPOINT_DST);
if (nl_type >= 0)
flower_print_port(f, "dst_port", tb[nl_type]);
- nl_type = flower_port_attr_type(ip_proto, true);
+ nl_type = flower_port_attr_type(ip_proto, FLOWER_ENDPOINT_SRC);
if (nl_type >= 0)
flower_print_port(f, "src_port", tb[nl_type]);
--
1.7.1
^ permalink raw reply related
* [PATCH] secure_seq: initialize secret at boot instead of at runtime
From: Jason A. Donenfeld @ 2017-01-15 14:01 UTC (permalink / raw)
To: netdev, davem; +Cc: Jason A. Donenfeld
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?)
b) __initcall is too early for get_random_bytes to return good
randomness. (Is this even true?)
c) late_initcall is too late since packets have already started
to flow. (Is this even true?)
So if one of these holds, feel free to ignore this patch. Otherwise,
it seems like it could be worthwhile.
net/core/secure_seq.c | 13 +++++--------
1 file changed, 5 insertions(+), 8 deletions(-)
diff --git a/net/core/secure_seq.c b/net/core/secure_seq.c
index 758f140b6bed..fd123f3b76c1 100644
--- a/net/core/secure_seq.c
+++ b/net/core/secure_seq.c
@@ -16,15 +16,18 @@
#include <net/secure_seq.h>
#if IS_ENABLED(CONFIG_IPV6) || IS_ENABLED(CONFIG_INET)
+#include <linux/init.h>
#include <linux/in6.h>
#include <net/tcp.h>
static siphash_key_t net_secret __read_mostly;
-static __always_inline void net_secret_init(void)
+static int net_secret_init(void)
{
- net_get_random_once(&net_secret, sizeof(net_secret));
+ get_random_bytes(&net_secret, sizeof(net_secret));
+ return 0;
}
+__initcall(net_secret_init);
#endif
#ifdef CONFIG_INET
@@ -60,7 +63,6 @@ u32 secure_tcpv6_sequence_number(const __be32 *saddr, const __be32 *daddr,
.dport = dport
};
u64 hash;
- net_secret_init();
hash = siphash(&combined, offsetofend(typeof(combined), dport),
&net_secret);
*tsoff = sysctl_tcp_timestamps == 1 ? (hash >> 32) : 0;
@@ -80,7 +82,6 @@ u32 secure_ipv6_port_ephemeral(const __be32 *saddr, const __be32 *daddr,
.daddr = *(struct in6_addr *)daddr,
.dport = dport
};
- net_secret_init();
return siphash(&combined, offsetofend(typeof(combined), dport),
&net_secret);
}
@@ -99,7 +100,6 @@ u32 secure_tcp_sequence_number(__be32 saddr, __be32 daddr,
__be16 sport, __be16 dport, u32 *tsoff)
{
u64 hash;
- net_secret_init();
hash = siphash_3u32((__force u32)saddr, (__force u32)daddr,
(__force u32)sport << 16 | (__force u32)dport,
&net_secret);
@@ -109,7 +109,6 @@ u32 secure_tcp_sequence_number(__be32 saddr, __be32 daddr,
u32 secure_ipv4_port_ephemeral(__be32 saddr, __be32 daddr, __be16 dport)
{
- net_secret_init();
return siphash_3u32((__force u32)saddr, (__force u32)daddr,
(__force u16)dport, &net_secret);
}
@@ -121,7 +120,6 @@ u64 secure_dccp_sequence_number(__be32 saddr, __be32 daddr,
__be16 sport, __be16 dport)
{
u64 seq;
- net_secret_init();
seq = siphash_3u32((__force u32)saddr, (__force u32)daddr,
(__force u32)sport << 16 | (__force u32)dport,
&net_secret);
@@ -147,7 +145,6 @@ u64 secure_dccpv6_sequence_number(__be32 *saddr, __be32 *daddr,
.dport = dport
};
u64 seq;
- net_secret_init();
seq = siphash(&combined, offsetofend(typeof(combined), dport),
&net_secret);
seq += ktime_get_real_ns();
--
2.11.0
^ permalink raw reply related
* [PATCH net-next v3 1/1] net sched actions: Add support for user cookies
From: Jamal Hadi Salim @ 2017-01-15 14:01 UTC (permalink / raw)
To: davem
Cc: netdev, jiri, paulb, john.fastabend, simon.horman, mrv, hadarh,
ogerlitz, roid, xiyou.wangcong, daniel, Jamal Hadi Salim
In-Reply-To: <1484488879-29560-1-git-send-email-jhs@emojatatu.com>
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
+ */
+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)
{
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 related
* [PATCH net-next v3 0/1] Add support for tc cookies
From: Jamal Hadi Salim @ 2017-01-15 14:01 UTC (permalink / raw)
To: davem
Cc: netdev, jiri, paulb, john.fastabend, simon.horman, mrv, hadarh,
ogerlitz, roid, xiyou.wangcong, daniel, Jamal Hadi Salim
From: Jamal Hadi Salim <jhs@mojatatu.com>
Changes in v3:
- use TC_ prefix for the max size
- move the cookie struct so visible only to kernel
- remove unneeded void * cast
Changes in V2:
-move from a union to a length-value representation
Jamal Hadi Salim (1):
net sched actions: Add support for user cookies
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(-)
--
1.9.1
^ permalink raw reply
* RE: [PATCH iproute2 v4 3/4] ifstat: Add 64 bits based stats to extended statistics
From: Nogah Frankel @ 2017-01-15 13:54 UTC (permalink / raw)
To: Stephen Hemminger
Cc: netdev@vger.kernel.org, roszenrami@gmail.com,
roopa@cumulusnetworks.com, Jiri Pirko, Ido Schimmel, Elad Raz,
Yotam Gigi, Or Gerlitz
In-Reply-To: <20170112174335.1bd391c2@xeon-e3>
> -----Original Message-----
> From: Stephen Hemminger [mailto:stephen@networkplumber.org]
> Sent: Friday, January 13, 2017 3:44 AM
> To: Nogah Frankel <nogahf@mellanox.com>
> Cc: netdev@vger.kernel.org; roszenrami@gmail.com; roopa@cumulusnetworks.com; Jiri
> Pirko <jiri@mellanox.com>; Ido Schimmel <idosch@mellanox.com>; Elad Raz
> <eladr@mellanox.com>; Yotam Gigi <yotamg@mellanox.com>; Or Gerlitz
> <ogerlitz@mellanox.com>
> Subject: Re: [PATCH iproute2 v4 3/4] ifstat: Add 64 bits based stats to extended statistics
>
> On Thu, 12 Jan 2017 15:49:50 +0200
> Nogah Frankel <nogahf@mellanox.com> wrote:
>
> > The default stats for ifstat are 32 bits based.
> > The kernel supports 64 bits based stats. (They are returned in struct
> > rtnl_link_stats64 which is an exact copy of struct rtnl_link_stats, in
> > which the "normal" stats are returned, but with fields of u64 instead of
> > u32). This patch adds them as an extended stats.
> >
> > It is read with filter type IFLA_STATS_LINK_64 and no sub type.
> >
> > It is under the name 64bits
> > (or any shorten of it as "64")
> >
> > For example:
> > ifstat -x 64bit
> >
> > Signed-off-by: Nogah Frankel <nogahf@mellanox.com>
> > Reviewed-by: Jiri Pirko <jiri@mellanox.com>
>
> Other commands (like ip link) always use the 64 bit statistics if available
> from the device. I see no reason that ifstat needs to be different.
>
Do you mean to change the default ifstat results to be 64 bits based?
I tried it in the first version, but Roopa commented that it was not a good idea.
She said they tried it in the past and it caused backward compatibilities problems.
(Or maybe I didn't understand correctly)
^ permalink raw reply
* RE: [PATCH iproute2 v4 1/4] ifstat: Includes reorder
From: Nogah Frankel @ 2017-01-15 13:49 UTC (permalink / raw)
To: Sergei Shtylyov, netdev@vger.kernel.org
Cc: stephen@networkplumber.org, roszenrami@gmail.com,
roopa@cumulusnetworks.com, Jiri Pirko, Ido Schimmel, Elad Raz,
Yotam Gigi, Or Gerlitz
In-Reply-To: <a7377c99-c323-b9d8-9c73-dbb7871fd14e@cogentembedded.com>
> -----Original Message-----
> From: Sergei Shtylyov [mailto:sergei.shtylyov@cogentembedded.com]
> Sent: Friday, January 13, 2017 12:11 PM
> To: Nogah Frankel <nogahf@mellanox.com>; netdev@vger.kernel.org
> Cc: stephen@networkplumber.org; roszenrami@gmail.com;
> roopa@cumulusnetworks.com; Jiri Pirko <jiri@mellanox.com>; Ido Schimmel
> <idosch@mellanox.com>; Elad Raz <eladr@mellanox.com>; Yotam Gigi
> <yotamg@mellanox.com>; Or Gerlitz <ogerlitz@mellanox.com>
> Subject: Re: [PATCH iproute2 v4 1/4] ifstat: Includes reorder
>
> On 1/12/2017 4:49 PM, Nogah Frankel wrote:
>
> > Reorder the includes order in misc/ifstat.c to match convention.
>
> "Reorder the ... order" sounds a bit tautological. :-)
>
> MBR, Sergei
I'll find a better description.
Thanks :)
^ permalink raw reply
* Re: [PATCH net-next 1/1] net sched actions: Add support for user cookies
From: Jamal Hadi Salim @ 2017-01-15 12:59 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: <20170115091127.GA1795@nanopsycho.orion>
On 17-01-15 04:11 AM, Jiri Pirko wrote:
> Subject should contain "V2"
>
> Sat, Jan 14, 2017 at 10:52:44PM 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(using two 64bit values to represent the 128 bits):
>
> Looks like you did not update the description.
>
Yikes. Yes - will send v3.
>>
>> Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
>
> V1->V2 changelog would ne nice.
>
Will do.
>
>> ---
>> include/net/act_api.h | 1 +
>> include/uapi/linux/pkt_cls.h | 11 +++++++++++
>> net/sched/act_api.c | 28 ++++++++++++++++++++++++++--
>> 3 files changed, 38 insertions(+), 2 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;
>
> I wonder if we just can't do:
> struct tc_cookie act_ck;
> You would safe kzalloc. I don't have strong opinion though..
>
Lets spare some RAM. I'll keep it the way it is.
>
>> };
>> #define tcf_head common.tcfa_head
>> #define tcf_index common.tcfa_index
>> diff --git a/include/uapi/linux/pkt_cls.h b/include/uapi/linux/pkt_cls.h
>> index 1e5e1dd..063bc89 100644
>> --- a/include/uapi/linux/pkt_cls.h
>> +++ b/include/uapi/linux/pkt_cls.h
>> @@ -4,6 +4,16 @@
>> #include <linux/types.h>
>> #include <linux/pkt_sched.h>
>>
>> +#define MAX_TC_COOKIE_SZ 16
>
> I like to have some "namespace" prefix for user api.
> "TC_COOKIE_MAX_SIZE" perhaps?
>
I just cutnpasted from something familiar: MAX_PHYS_ITEM_ID_LEN ;->
Will make the change.
>> +
>> +/* This structure holds cookie structure that is passed from user
>> + * to the kernel for actions and classifiers
>> + */
>> +struct tc_cookie {
>> + unsigned char ck[MAX_TC_COOKIE_SZ];
>> + unsigned char ck_len;
>
> This struct should certainly not be in UAPI header.
>
I will move it.
>
>> +};
>> +
>> /* Action attributes */
>> enum {
>> TCA_ACT_UNSPEC,
>> @@ -12,6 +22,7 @@ enum {
>> TCA_ACT_INDEX,
>> TCA_ACT_STATS,
>> TCA_ACT_PAD,
>> + TCA_ACT_COOKIE,
>> __TCA_ACT_MAX
>> };
>> + memcpy((void *)a->act_ck->ck, nla_data(tb[TCA_ACT_COOKIE]),
>
> Unneeded (void *) cast.
>
ok. I think gcc whined for some reason.
Will post after some test.
cheers,
jamal
^ permalink raw reply
* qla3xxx: u32 constant overflow in fm93c56a_select()
From: Nicolas Iooss @ 2017-01-15 12:03 UTC (permalink / raw)
To: Dept-GELinuxNICDev, netdev; +Cc: linux-kernel
Hello,
In drivers/net/ethernet/qlogic/qla3xxx.c, fm93c56a_select() currently calls:
ql_write_nvram_reg(qdev, spir,
((ISP_NVRAM_MASK << 16) | qdev->eeprom_cmd_data));
and ISP_NVRAM_MASK is defined as (0x000F << 16).
ql_write_nvram_reg() writes a 32-bit value but (ISP_NVRAM_MASK << 16)
expands to ((0x000F << 16) << 16) = 0xF << 32, which overflows the u32
type. This means the above call is equivalent to:
ql_write_nvram_reg(qdev, spir, qdev->eeprom_cmd_data);
Is this something normal, in which case (ISP_NVRAM_MASK << 16) would be
removed, or a bug?
Thanks,
Nicolas
^ permalink raw reply
* Re: [PATCH net-next v3 00/10] net: dsa: Support for pdata in dsa2
From: Greg KH @ 2017-01-15 11:08 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: <20170114214713.28109-1-f.fainelli@gmail.com>
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.
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH net-next v3 07/10] net: Relocate dev_to_net_device() into core
From: Greg KH @ 2017-01-15 11:07 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: <20170114214713.28109-8-f.fainelli@gmail.com>
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...
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH net-next v3 06/10] net: dsa: Migrate to device_find_class()
From: Greg KH @ 2017-01-15 11:06 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: <20170114214713.28109-7-f.fainelli@gmail.com>
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?
> @@ -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?
totally confused,
greg k-h
^ permalink raw reply
* Re: [PATCH net-next v3 05/10] drivers: base: Add device_find_class()
From: Greg KH @ 2017-01-15 11:04 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: <20170114214713.28109-6-f.fainelli@gmail.com>
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.
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...
> +{
> + 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?
Again, what is this being used for?
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.
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH] net: add regs attribute to phy device for user diagnose
From: yuan linyu @ 2017-01-15 10:40 UTC (permalink / raw)
To: Florian Fainelli, Andrew Lunn; +Cc: David S . Miller, netdev, yuan linyu
In-Reply-To: <580590a9-d03b-2fab-efe7-173427b7c331@gmail.com>
On 六, 2017-01-14 at 17:57 -0800, Florian Fainelli wrote:
> Le 01/14/17 à 17:51, yuan linyu a écrit :
> >
> > I think mii-tool or ethtool can't do it currently.
> Maybe they cannot right now but they can certainly be patched to support
> that. sysfs is not an appropriate interface for what you are proposing
> here. We already have a set/get register via ethtool (-d/-D) it would
> seem natural to use this.
I think most ethernet driver implement ethtool -d to dump mac register.
>
> Besides that, are not the current ioctl() good enough for that?
I think user/developer diagnose through simple attribute file will be easy.
^ permalink raw reply
* Re: [PATCH net-next v3 04/10] net: dsa: Move ports assignment closer to error checking
From: Sergei Shtylyov @ 2017-01-15 10:17 UTC (permalink / raw)
To: Florian Fainelli, netdev
Cc: Jason Cooper, Andrew Lunn, Sebastian Hesselbarth, Gregory Clement,
Russell King, Vivien Didelot, David S. Miller,
moderated list:ARM SUB-ARCHITECTURES, open list, gregkh
In-Reply-To: <20170114214713.28109-5-f.fainelli@gmail.com>
Hello!
On 1/15/2017 12:47 AM, Florian Fainelli wrote:
> Move the assignment of ports in _dsa_register_switch() closer to where
> it is checked, no functional change. Re-order declarations to be
"Be" not needed.
> preserve the inverted christmas tree style.
>
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
[...]
MBR, Sergei
^ permalink raw reply
* (unknown)
From: bigbiglotto @ 2017-01-15 9:49 UTC (permalink / raw)
In-Reply-To: <864765730.5250882.1484473748647@mail.yahoo.com>
[-- Attachment #1: Type: text/plain, Size: 1 bytes --]
[-- Attachment #2: ATM CARD READY.jpg --]
[-- Type: image/jpeg, Size: 532619 bytes --]
^ permalink raw reply
* Re: [PATCH net-next 1/1] net sched actions: Add support for user cookies
From: Jiri Pirko @ 2017-01-15 9:11 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: <1484430764-30788-1-git-send-email-jhs@emojatatu.com>
Subject should contain "V2"
Sat, Jan 14, 2017 at 10:52:44PM 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(using two 64bit values to represent the 128 bits):
Looks like you did not update the description.
>
>.. 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>
V1->V2 changelog would ne nice.
>---
> include/net/act_api.h | 1 +
> include/uapi/linux/pkt_cls.h | 11 +++++++++++
> net/sched/act_api.c | 28 ++++++++++++++++++++++++++--
> 3 files changed, 38 insertions(+), 2 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;
I wonder if we just can't do:
struct tc_cookie act_ck;
You would safe kzalloc. I don't have strong opinion though..
> };
> #define tcf_head common.tcfa_head
> #define tcf_index common.tcfa_index
>diff --git a/include/uapi/linux/pkt_cls.h b/include/uapi/linux/pkt_cls.h
>index 1e5e1dd..063bc89 100644
>--- a/include/uapi/linux/pkt_cls.h
>+++ b/include/uapi/linux/pkt_cls.h
>@@ -4,6 +4,16 @@
> #include <linux/types.h>
> #include <linux/pkt_sched.h>
>
>+#define MAX_TC_COOKIE_SZ 16
I like to have some "namespace" prefix for user api.
"TC_COOKIE_MAX_SIZE" perhaps?
>+
>+/* This structure holds cookie structure that is passed from user
>+ * to the kernel for actions and classifiers
>+ */
>+struct tc_cookie {
>+ unsigned char ck[MAX_TC_COOKIE_SZ];
>+ unsigned char ck_len;
This struct should certainly not be in UAPI header.
>+};
>+
> /* Action attributes */
> enum {
> TCA_ACT_UNSPEC,
>@@ -12,6 +22,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..b82908a 100644
>--- a/net/sched/act_api.c
>+++ b/net/sched/act_api.c
>@@ -33,6 +33,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 +465,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)
> {
> int err = -EINVAL;
> unsigned char *b = skb_tail_pointer(skb);
>@@ -475,6 +476,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 +582,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]) > MAX_TC_COOKIE_SZ) {
>+ 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((void *)a->act_ck->ck, nla_data(tb[TCA_ACT_COOKIE]),
Unneeded (void *) cast.
>+ 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
* Re: [pull request][for-next] Mellanox mlx5 Reorganize core driver directory layout
From: Leon Romanovsky @ 2017-01-15 7:20 UTC (permalink / raw)
To: Tom Herbert
Cc: Saeed Mahameed, David Miller, Saeed Mahameed, Doug Ledford,
Linux Kernel Network Developers,
linux-rdma-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <CALx6S37FHv3d0_hsXd63m37CEgT4N8myjUey29iDo1DRR=T9tQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 7042 bytes --]
On Sat, Jan 14, 2017 at 09:37:20AM -0800, Tom Herbert wrote:
> On Fri, Jan 13, 2017 at 2:45 PM, Saeed Mahameed
> <saeedm-LDSdmyG8hGV8YrgS2mwiifqBs+8SCbDb@public.gmane.org> wrote:
> > On Sat, Jan 14, 2017 at 12:06 AM, Tom Herbert <tom-BjP2VixgY4xUbtYUoyoikg@public.gmane.org> wrote:
> >> On Fri, Jan 13, 2017 at 12:29 PM, Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> >>> On Fri, Jan 13, 2017 at 12:14:07PM -0500, David Miller wrote:
> >>>> From: Saeed Mahameed <saeedm-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> >>>> Date: Thu, 12 Jan 2017 19:22:34 +0200
> >>>>
> >>>> > This pull request includes one patch from Leon, this patch as described
> >>>> > below will change the driver directory structure and layout for better,
> >>>> > logical and modular driver files separation.
> >>>> >
> >>>> > This change is important to both rdma and net maintainers in order to
> >>>> > have smoother management of driver patches for different mlx5 sub modules
> >>>> > and smoother rdma-next vs. net-next features submissions.
> >>>> >
> >>>> > Please find more info below -in the tag commit message-,
> >>>> > review and let us know if there's any problem.
> >>>> >
> >>>> > This change doesn't introduce any conflicts with the current mlx5
> >>>> > fixes and cleanups posted on 2017-01-10 to net branch, and merge tests
> >>>> > worked flawlessly with no issues.
> >>>> >
> >>>> > This is the last pull request meant for both rdma-next and net-next.
> >>>> > Once pulled, this will be the base shared code for both trees.
> >>>>
> >>>> This is pretty crazy, it will make all bug fix backporting to -stable
> >>>> a complete nightmare for myself, Doug, various distribution maintainers
> >>>> and many other people who quietly have to maintain their own trees and
> >>>> do backporting.
> >>>
> >>> Hi Dave,
> >>>
> >>> I understand your worries, but our case is similar to various other
> >>> drivers, for example hfi1 which was in staging for years while
> >>> supported in RedHat and moved from there to IB. The Chelsio drivers did
> >>> similar reorg in 2016 (drivers/net/ethernet/chelsio/libcxgb) while their
> >>> drivers were in the tree for long time before.
> >>>
> >>> Additionally, Doug doesn't need to maintain -stable queue and it is done
> >>> by relevant submaintainers who are adding stable tags by themselves. In
> >>> the IB case, the burden will continue to be on me and not on Doug.
> >>>
> >> Recently I had to backport the mlx5 driver from 4.9 to 4.6 in order to
> >> get support for XDP. The biggest issue I faced was the lack of
> >> modularity in the many driver features that are now supported. The
> >> problem with backporting these new features is the spider web of
> >> dependencies that they bring in from the rest of the kernel. I ended
> >> up taking out en_rep, vxlan, en_tc, eswitch, and dcbnl. The result was
> >> ~340 patches which is still a lot but at least this was constrained to
> >> patches in the mlx5 directories and are relevant to what we want to
> >> do.
We had that complexity in mind. This pull request is our initial step
to simplify backporters' work (e.g. rename of en_XXX.c to be XXX.c is
the same as en/XXX.c for the backports).
We are aware of IB backports which required EN part of driver which
had all net dependencies mentioned above and IMHO it is wrong.
Our plan is:
1. Reorg/rename files.
2. Separate en and IB from shared code, to allow parallel work.
3. Reduce en and IB dependencies, to simplify backporting.
It is really awkward to move to item 2 and 3 before we cleaned the desk.
That rename patch provided to all backporters an easy way to bring new
features into the driver.
> >>
> >> In lieu of restructuring the directories, I would much rather see more
> >> config options so that we can build drivers that don't unnecessarily
> >> complicate our lives with features we don't use. This is not just true
> >> for Mellanox, but I would say it would be true of any driver that
> >> someone is trying to deploy and maintain at large scale.
Different maintainers have a different view on the subject and not everyone
is ready to accept new drivers config options.
> >>
> >
> > I think we should have both, if the restructuring made right,
> > new whole features (e.g eswitch and eswitch offlaods or any independent module),
> > can sit in their own directory and keep their own logic concentrated
> > in one place, and only touch the
> > main driver code with simple entry points in the main flow, this way
> > you can simply compile their whole directories
> > out with a config flag directly from the Makefile.
> >
> That would be nice, but that is not what your new layout gives us yet.
> The starting point for this structure should be to discern what the
> minimum set of files is to build a functional and good performance
> driver with the basic functionality including only the most common
> offloads. These I would think constitute the core files for the driver
> and hence make sense to be in the "core" directory. Personally I would
> drop the en_ file prefix and not make an en directory, we're already
> in the ethernet driver part of the tree so I don't know what this name
> is supposed to convey. For reference I gave you this list of
> components I disabled to do the backport, some we're not split out in
> your directory layout. A good example is the VXLAN offload support,
> the changes in that area we're mostly due to Alexander's patch set to
> redo the interface for these accelerations-- so to back port driver
> from 4.9 to 4.6 I would need to pull those in and whatever else those
> depend on. It was way easier to just ifdef out the relevant calls and
> not compile the vxlan files to do the backport.
We would love to move into that direction, but right now that core
directory serves our IB driver too. Once we separate them, your proposal
will be possible to implement.
>
> Tom
>
> >> Btw, we did hit one issue in the backport. We started to get rx csum
> >> faults (checksum complete value indicates TCP checksum is bad, but
> >> host computation says checksum is good). I ran against 4.9 upstream
> >> kernel and do see these, however don't see them in 4.10. I haven't
> >> bisected yet. Is this a known issue?
> >>
> >
> > Not to me, I don't recall any csum related fixes or feature submitted
> > lately to mlx5,
> > Maybe something changed in the stack ?
> >
> > what configuration are you running ? what traffic ?
> >
> >> Thanks,
> >> Tom
> >>
> >>>>
> >>>> I really don't think you can justify this rearrangement based upon the
> >>>> consequences and how much activity happens in this driver.
> >>>>
> >>>> You should have thought long and hard about the layout a long time ago
> >>>> rather than after the driver has been in the tree for many years.
> >>>>
> >>>> Sorry.
> >> --
> >> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> >> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> >> More majordomo info at http://vger.kernel.org/majordomo-info.html
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* [PATCH v2 net-next] IPsec: do not ignore crypto err in ah input
From: Gilad Ben-Yossef @ 2017-01-15 6:09 UTC (permalink / raw)
To: steffen.klassert, herbert, davem, netdev
Cc: ofir.drang, gilad.benyossef, Gilad Ben-Yossef, Alexander Alemayhu
ah input processing uses the asynchronous hash crypto API which supplies
an error code as part of the operation completion but the error code was
being ignored.
Treat a crypto API error indication as a verification failure.
While a crypto API reported error would almost certainly result in a
memcmp of the digest failing anyway and thus the security risk seems
minor, performing a memory compare on what might be uninitialized memory
is wrong.
Signed-off-by: Gilad Ben-Yossef <gilad@benyossef.com>
CC: Alexander Alemayhu <alexander@alemayhu.com>
---
The change was boot tested on Arm64 but I did not exercise
the specific error code path in question.
Changes from v1:
- Fixed typo in patch description pointed out by Alexander
net/ipv4/ah4.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/net/ipv4/ah4.c b/net/ipv4/ah4.c
index f2a7102..22377c8 100644
--- a/net/ipv4/ah4.c
+++ b/net/ipv4/ah4.c
@@ -270,6 +270,9 @@ static void ah_input_done(struct crypto_async_request *base, int err)
int ihl = ip_hdrlen(skb);
int ah_hlen = (ah->hdrlen + 2) << 2;
+ if (err)
+ goto out;
+
work_iph = AH_SKB_CB(skb)->tmp;
auth_data = ah_tmp_auth(work_iph, ihl);
icv = ah_tmp_icv(ahp->ahash, auth_data, ahp->icv_trunc_len);
--
2.1.4
^ permalink raw reply related
* Re: [PATCH net-next] IPsec: do not ignore crypto err in ah input
From: Gilad Ben-Yossef @ 2017-01-15 6:06 UTC (permalink / raw)
To: Alexander Alemayhu
Cc: steffen.klassert, Herbert Xu, David Miller, netdev, Ofir Drang,
gilad.benyossef
In-Reply-To: <20170113074551.GA24287@gmail.com>
On Fri, Jan 13, 2017 at 9:45 AM, Alexander Alemayhu
<alexander@alemayhu.com> wrote:
> On Thu, Jan 12, 2017 at 03:33:22PM +0200, Gilad Ben-Yossef wrote:
>> ah input processing uses the asynchrnous hash crypto API which
>> supplies an error code as part of the operation completion but
>> the error code was being ignored.
>>
> s/asynchrnous/asynchronous
Yes, I have missed that.
Thank you for your review. I will send a fixed version.
Thanks,
Gilad
--
Gilad Ben-Yossef
Chief Coffee Drinker
"If you take a class in large-scale robotics, can you end up in a
situation where the homework eats your dog?"
-- Jean-Baptiste Queru
^ permalink raw reply
* Re: [PATCH] net: add regs attribute to phy device for user diagnose
From: Florian Fainelli @ 2017-01-15 1:57 UTC (permalink / raw)
To: cugyly, Andrew Lunn; +Cc: David S . Miller, netdev, yuan linyu
In-Reply-To: <1484445063.4555.8.camel@163.com>
Le 01/14/17 à 17:51, yuan linyu a écrit :
> 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.
You need to submit a PHY driver that implements the API you are
proposing here. Right now no PHY driver implements these {set,get}_regs
function pointers, so this is essentially dead code.
>
> I think mii-tool or ethtool can't do it currently.
Maybe they cannot right now but they can certainly be patched to support
that. sysfs is not an appropriate interface for what you are proposing
here. We already have a set/get register via ethtool (-d/-D) it would
seem natural to use this.
Besides that, are not the current ioctl() good enough for that?
--
Florian
^ permalink raw reply
* Re: [PATCH] net: add regs attribute to phy device for user diagnose
From: yuan linyu @ 2017-01-15 1:51 UTC (permalink / raw)
To: Florian Fainelli, Andrew Lunn; +Cc: David S . Miller, netdev, yuan linyu
In-Reply-To: <da255b0c-dba6-ef35-d436-3110d8492f95@gmail.com>
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.
I think mii-tool or ethtool can't do it currently.
>
> Thank you
^ permalink raw reply
* Re: [PATCH v5 05/13] net: ethernet: aquantia: Support for NIC-specific code
From: Andrew Lunn @ 2017-01-15 1:15 UTC (permalink / raw)
To: Rami Rosen
Cc: Florian Fainelli, Alexander Loktionov, Netdev, David VomLehn,
David S . Miller, Simon Edelhaus, Dmitrii Tarakanov, Pavel Belous,
Dmitry Bezrukov
In-Reply-To: <CAKoUArkh3gyVSrX1GWRQsrFFBg8B59LjtXK=yG6JWMHan7fW7A@mail.gmail.com>
On Sun, Jan 15, 2017 at 12:55:02AM +0200, Rami Rosen wrote:
> Hi, Florian,
>
> > +}
> > +
> > +static int aq_ndev_change_mtu(struct net_device *ndev, int new_mtu)
> > +{
> > + struct aq_nic_s *aq_nic = (struct aq_nic_s *)netdev_priv(ndev);
> > + int err = 0;
> > +
> > + if (new_mtu == ndev->mtu) {
> > + err = 0;
> > + goto err_exit;
> > + }
> > + if (new_mtu < 68) {
> > + err = -EINVAL;
> > + goto err_exit;
> > + }
Actually, the core will already impose min/max of ETH_MIN_MTU and
ETH_DATA_LEN. See
commit a52ad514fdf3b8a57ca4322c92d2d8d5c6182485
Author: Jarod Wilson <jarod@redhat.com>
Date: Fri Oct 7 22:04:34 2016 -0400
net: deprecate eth_change_mtu, remove usage
With centralized MTU checking, there's nothing productive done by
eth_change_mtu that isn't already done in dev_set_mtu, so mark it as
deprecated and remove all usage of it in the kernel. All callers have been
audited for calls to alloc_etherdev* or ether_setup directly, which means
they all have a valid dev->min_mtu and dev->max_mtu. Now eth_change_mtu
prints out a netdev_warn about being deprecated, for the benefit of
out-of-tree drivers that might be utilizing it.
Of note, dvb_net.c actually had dev->mtu = 4096, while using
eth_change_mtu, meaning that if you ever tried changing it's mtu, you
couldn't set it above 1500 anymore. It's now getting dev->max_mtu also set
to 4096 to remedy that.
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