* Re: [PATCH v4 binutils] Add BPF support to binutils...
From: Alexei Starovoitov @ 2017-05-02 3:49 UTC (permalink / raw)
To: David Miller; +Cc: daniel, netdev, xdp-newbies
In-Reply-To: <20170430.120750.651845251226226775.davem@davemloft.net>
On 4/30/17 9:07 AM, David Miller wrote:
> This is mainly a synchronization point, I still need to look
> more deeply into Alexei's -g issue.
>
> New in this version from v3:
> - Remove tailcall from opcode table
> - Rearrange relocations so that numbers match with LLVM ones
> - Emit relocs properly so that dwarf2 debug info tests pass
> - Handle negative load/store offsets properly, add tests
>
> Signed-off-by: David S. Miller <davem@davemloft.net>
dwarf on little endian works now :)
$ /w/binutils-gdb/bld/binutils/objdump -S test.o
test.o: file format elf64-bpfle
Disassembly of section .text:
0000000000000000 <bpf_prog1>:
int bpf_prog1(void *ign)
{
volatile unsigned long t = 0x8983984739ull;
0: 18 01 00 00 39 47 98 83 ldimm64 r0, 590618314553
8: 00 00 00 00 89 00 00 00
10: 7b 1a f8 ff 00 00 00 00 stdw [r1+-8], r10
return *(unsigned long *)((0xffffffff8fff0002ull) + t);
18: 79 a1 f8 ff 00 00 00 00 lddw r10, [r1+-8]
This is great milestone.
Also I finally figured out how to enable native+bpf:
../configure --enable-targets=bpf-elf,x86_64-elf
having support for both in one binary is a big deal :)
Only 'gdb' warns with dual arch support:
"
warning: A handler for the OS ABI "GNU/Linux" is not built into this
configuration
of GDB. Attempting to continue with the default bpf settings.
"
(gdb) x/10i bpf_prog1
0x0 <bpf_prog1>: ldimm64 r0, 590618314553
0x10 <bpf_prog1+16>: stdw [r1+-8], r10
0x18 <bpf_prog1+24>: lddw r10, [r1+-8]
0x20 <bpf_prog1+32>: add r0, -1879113726
0x28 <bpf_prog1+40>: lddw r1, [r0+0]
0x30 <bpf_prog1+48>: exit
0x38: Cannot access memory at address 0x38
the last line also seems wrong. Off by 1 error?
^ permalink raw reply
* [PATCH] sparc64: Fix BPF JIT wrt. branches and ldimm64 instructions.
From: David Miller @ 2017-05-02 3:50 UTC (permalink / raw)
To: ast; +Cc: daniel, netdev, xi.wang, catalin.marinas
Like other JITs, sparc64 maintains an array of instruction offsets but
stores the entries off by one. This is done because jumps to the exit
block are indexed to one past the last BPF instruction.
So if we size the array by the program length, we need to record the
previous instruction in order to stay within the array bounds.
This is explained in ARM JIT commit 8eee539ddea0 ("arm64: bpf: fix
out-of-bounds read in bpf2a64_offset()").
But this scheme requires a little bit of careful handling when the
instruction before the branch destination is a 64-bit load immediate.
It takes up 2 BPF instruction slots.
Therefore, we have to fill in the array entry for the second half of
the 64-bit load immediate instruction rather than for the one for the
beginning of that instruction.
Fixes: 7a12b5031c6b ("sparc64: Add eBPF JIT.")
Signed-off-by: David S. Miller <davem@davemloft.net>
---
arch/sparc/net/bpf_jit_comp_64.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/arch/sparc/net/bpf_jit_comp_64.c b/arch/sparc/net/bpf_jit_comp_64.c
index ec7d10d..21de774 100644
--- a/arch/sparc/net/bpf_jit_comp_64.c
+++ b/arch/sparc/net/bpf_jit_comp_64.c
@@ -1446,12 +1446,13 @@ static int build_body(struct jit_ctx *ctx)
int ret;
ret = build_insn(insn, ctx);
- ctx->offset[i] = ctx->idx;
if (ret > 0) {
i++;
+ ctx->offset[i] = ctx->idx;
continue;
}
+ ctx->offset[i] = ctx->idx;
if (ret)
return ret;
}
--
2.1.2.532.g19b5d50
^ permalink raw reply related
* Re: [PATCH v4 binutils] Add BPF support to binutils...
From: David Miller @ 2017-05-02 3:51 UTC (permalink / raw)
To: ast; +Cc: daniel, netdev, xdp-newbies
In-Reply-To: <33505cff-f730-ebac-c2d7-38f1793062b7@fb.com>
From: Alexei Starovoitov <ast@fb.com>
Date: Mon, 1 May 2017 20:49:21 -0700
> (gdb) x/10i bpf_prog1
> 0x0 <bpf_prog1>: ldimm64 r0, 590618314553
> 0x10 <bpf_prog1+16>: stdw [r1+-8], r10
> 0x18 <bpf_prog1+24>: lddw r10, [r1+-8]
> 0x20 <bpf_prog1+32>: add r0, -1879113726
> 0x28 <bpf_prog1+40>: lddw r1, [r0+0]
> 0x30 <bpf_prog1+48>: exit
> 0x38: Cannot access memory at address 0x38
>
> the last line also seems wrong. Off by 1 error?
Maybe, I'll look into it tomorrow. I'll also post my fix for the
branch destination disassembler output tomorrow as well.
^ permalink raw reply
* bpf_test_finish()
From: David Miller @ 2017-05-02 3:56 UTC (permalink / raw)
To: ast; +Cc: daniel, netdev
It dereferences a user pointer:
static int bpf_test_finish(union bpf_attr __user *uattr, const void *data,
u32 size, u32 retval, u32 duration)
{
void __user *data_out = u64_to_user_ptr(uattr->test.data_out);
^^^^^^^^^^^^^^^^^^^^
Which of course doesn't work so well :-)
I really wish that didn't silently work on x86/x86_64.
You're going to have to do a "get_user(&uattr->test.data_out)"
^ permalink raw reply
* Re: bpf_test_finish()
From: Alexei Starovoitov @ 2017-05-02 4:46 UTC (permalink / raw)
To: David Miller; +Cc: daniel, netdev
In-Reply-To: <20170501.235610.564976046138352257.davem@davemloft.net>
On 5/1/17 8:56 PM, David Miller wrote:
>
> It dereferences a user pointer:
>
> static int bpf_test_finish(union bpf_attr __user *uattr, const void *data,
> u32 size, u32 retval, u32 duration)
> {
> void __user *data_out = u64_to_user_ptr(uattr->test.data_out);
> ^^^^^^^^^^^^^^^^^^^^
> Which of course doesn't work so well :-)
>
> I really wish that didn't silently work on x86/x86_64.
argh. my bad.
I'll send a patch first thing tomorrow unless Daniel beats me to it.
We have kattr there as well which has the whole bpf_attr copied into
kernel memory already. Should have taken data_out from there and
passed into bpf_test_finish().
^ permalink raw reply
* Re: [patch net-next 00/10] net: sched: introduce multichain support for filters
From: Cong Wang @ 2017-05-02 5:26 UTC (permalink / raw)
To: Jiri Pirko
Cc: Linux Kernel Network Developers, David Miller, Jamal Hadi Salim,
David Ahern, Eric Dumazet, Stephen Hemminger, Daniel Borkmann,
Alexander Duyck, mlxsw, Simon Horman
In-Reply-To: <20170428223446.GB1905@nanopsycho.orion>
On Fri, Apr 28, 2017 at 3:34 PM, Jiri Pirko <jiri@resnulli.us> wrote:
> Fri, Apr 28, 2017 at 07:40:24PM CEST, xiyou.wangcong@gmail.com wrote:
>>On Thu, Apr 27, 2017 at 11:53 PM, Jiri Pirko <jiri@resnulli.us> wrote:
>>> Thu, Apr 27, 2017 at 07:46:03PM CEST, xiyou.wangcong@gmail.com wrote:
>>>>On Thu, Apr 27, 2017 at 4:12 AM, Jiri Pirko <jiri@resnulli.us> wrote:
>>>>> Simple example:
>>>>> $ tc qdisc add dev eth0 ingress
>>>>> $ tc filter add dev eth0 parent ffff: protocol ip pref 33 flower dst_mac 52:54:00:3d:c7:6d action goto chain 11
>>>>> $ tc filter add dev eth0 parent ffff: protocol ip pref 22 chain 11 flower dst_ip 192.168.40.1 action drop
>>>>> $ tc filter show dev eth0 root
>>>>
>>>>Interesting.
>>>>
>>>>I don't look into the code yet. If I understand the concepts correctly,
>>>>so with your patchset we can mark either filter with a chain No. to
>>>>choose which chain it belongs to _logically_ even though
>>>>_physically_ it is still in the old-fashion chain (prio, proto)?
>>>
>>> You have to see the code :)
>>
>>I don't understand why I have to, these are high-level concepts
>>and should be put in your cover letter (aka. design doc). You miss
>>a lot of information about the ordering here.
>
> Well, the description is one thing, but seeing the actual code should
> put the whole view. But if you are missing something, I can add it. What
> do you mean by "information about the ordering"?
>
By ordering, I mean:
1) before your patch, filters are ordered by prio and categorized by proto
2) after your patch, we can jump from one filter to a specified one, how
does this work or not work with the prio/proto?
^ permalink raw reply
* 10921 netdev
From: gestu @ 2017-05-02 5:38 UTC (permalink / raw)
To: netdev
[-- Attachment #1: 801737.zip --]
[-- Type: application/zip, Size: 4277 bytes --]
^ permalink raw reply
* Re: [patch net-next 00/10] net: sched: introduce multichain support for filters
From: Jiri Pirko @ 2017-05-02 5:50 UTC (permalink / raw)
To: Cong Wang
Cc: Linux Kernel Network Developers, David Miller, Jamal Hadi Salim,
David Ahern, Eric Dumazet, Stephen Hemminger, Daniel Borkmann,
Alexander Duyck, mlxsw, Simon Horman
In-Reply-To: <CAM_iQpX8i5-YZRkKN9DyvjCeE8W4M33wiUUTHizsyGsnsE=0KQ@mail.gmail.com>
Tue, May 02, 2017 at 07:26:07AM CEST, xiyou.wangcong@gmail.com wrote:
>On Fri, Apr 28, 2017 at 3:34 PM, Jiri Pirko <jiri@resnulli.us> wrote:
>> Fri, Apr 28, 2017 at 07:40:24PM CEST, xiyou.wangcong@gmail.com wrote:
>>>On Thu, Apr 27, 2017 at 11:53 PM, Jiri Pirko <jiri@resnulli.us> wrote:
>>>> Thu, Apr 27, 2017 at 07:46:03PM CEST, xiyou.wangcong@gmail.com wrote:
>>>>>On Thu, Apr 27, 2017 at 4:12 AM, Jiri Pirko <jiri@resnulli.us> wrote:
>>>>>> Simple example:
>>>>>> $ tc qdisc add dev eth0 ingress
>>>>>> $ tc filter add dev eth0 parent ffff: protocol ip pref 33 flower dst_mac 52:54:00:3d:c7:6d action goto chain 11
>>>>>> $ tc filter add dev eth0 parent ffff: protocol ip pref 22 chain 11 flower dst_ip 192.168.40.1 action drop
>>>>>> $ tc filter show dev eth0 root
>>>>>
>>>>>Interesting.
>>>>>
>>>>>I don't look into the code yet. If I understand the concepts correctly,
>>>>>so with your patchset we can mark either filter with a chain No. to
>>>>>choose which chain it belongs to _logically_ even though
>>>>>_physically_ it is still in the old-fashion chain (prio, proto)?
>>>>
>>>> You have to see the code :)
>>>
>>>I don't understand why I have to, these are high-level concepts
>>>and should be put in your cover letter (aka. design doc). You miss
>>>a lot of information about the ordering here.
>>
>> Well, the description is one thing, but seeing the actual code should
>> put the whole view. But if you are missing something, I can add it. What
>> do you mean by "information about the ordering"?
>>
>
>By ordering, I mean:
>
>1) before your patch, filters are ordered by prio and categorized by proto
>
>2) after your patch, we can jump from one filter to a specified one, how
>does this work or not work with the prio/proto?
No, you can jump to another chain. And that chain is also ordered by
prio/proto.
Just imagine currently you have only chain 0. This patchset just extends
for other chains.
^ permalink raw reply
* Re: [patch net-next] net: sched: add helpers to handle extended actions
From: Jiri Pirko @ 2017-05-02 5:51 UTC (permalink / raw)
To: Jamal Hadi Salim; +Cc: netdev, davem, xiyou.wangcong, mlxsw
In-Reply-To: <6450af04-a6d4-866e-82ee-424ff54b2ef3@mojatatu.com>
Sun, Apr 30, 2017 at 04:08:15PM CEST, jhs@mojatatu.com wrote:
>Jiri,
>
>With "goto chain X" this will have to be more generalized. Maybe
>we have 0xAXXXXXXX Where "A" recognizes the extension with
>current values ACT_JUMP(0x1) and GOTO_CHAIN(maybe 0x2)
>and the rest "XXXXXXX" is a free floating parameter values
>which carry the goto count for ACT_JUMP and GOTO_CHAIN chain-id.
That is exactly what this patch is doing.
>
>cheers,
>jamal
>
>On 17-04-28 12:13 PM, Jiri Pirko wrote:
>> From: Jiri Pirko <jiri@mellanox.com>
>>
>> Jump is now the only one using value action opcode. This is going to
>> change soon. So introduce helpers to work with this. Convert TC_ACT_JUMP.
>>
>> Signed-off-by: Jiri Pirko <jiri@mellanox.com>
>> ---
>> include/uapi/linux/pkt_cls.h | 15 ++++++++++++++-
>> net/sched/act_api.c | 2 +-
>> 2 files changed, 15 insertions(+), 2 deletions(-)
>>
>> diff --git a/include/uapi/linux/pkt_cls.h b/include/uapi/linux/pkt_cls.h
>> index f1129e3..d613be3 100644
>> --- a/include/uapi/linux/pkt_cls.h
>> +++ b/include/uapi/linux/pkt_cls.h
>> @@ -37,7 +37,20 @@ enum {
>> #define TC_ACT_QUEUED 5
>> #define TC_ACT_REPEAT 6
>> #define TC_ACT_REDIRECT 7
>> -#define TC_ACT_JUMP 0x10000000
>> +
>> +/* There is a special kind of actions called "extended actions",
>> + * which need a value parameter. These have a local opcode located in
>> + * the highest nibble, starting from 1. The rest of the bits
>> + * are used to carry the value. These two parts together make
>> + * a combined opcode.
>> + */
>> +#define __TC_ACT_EXT_SHIFT 28
>> +#define __TC_ACT_EXT(local) ((local) << __TC_ACT_EXT_SHIFT)
>> +#define TC_ACT_EXT_VAL_MASK ((1 << __TC_ACT_EXT_SHIFT) - 1)
>> +#define TC_ACT_EXT_CMP(combined, opcode) \
>> + (((combined) & (~TC_ACT_EXT_VAL_MASK)) == opcode)
>> +
>> +#define TC_ACT_JUMP __TC_ACT_EXT(1)
>>
>> /* Action type identifiers*/
>> enum {
>> diff --git a/net/sched/act_api.c b/net/sched/act_api.c
>> index 7f2cd70..a90e8f3 100644
>> --- a/net/sched/act_api.c
>> +++ b/net/sched/act_api.c
>> @@ -453,7 +453,7 @@ int tcf_action_exec(struct sk_buff *skb, struct tc_action **actions,
>> if (ret == TC_ACT_REPEAT)
>> goto repeat; /* we need a ttl - JHS */
>>
>> - if (ret & TC_ACT_JUMP) {
>> + if (TC_ACT_EXT_CMP(ret, TC_ACT_JUMP)) {
>> jmp_prgcnt = ret & TCA_ACT_MAX_PRIO_MASK;
>> if (!jmp_prgcnt || (jmp_prgcnt > nr_actions)) {
>> /* faulty opcode, stop pipeline */
>>
>
^ permalink raw reply
* [PATCH net v4 00/12] Fix possbile memleaks when fail to register_netdevice
From: gfree.wind @ 2017-05-02 5:58 UTC (permalink / raw)
To: davem, jiri, mareklindner, sw, a, kuznet, jmorris, yoshfuji,
kaber, steffen.klassert, herbert, netdev
Cc: Gao Feng
From: Gao Feng <gfree.wind@foxmail.com>
These following drivers allocate kinds of resources in its ndo_init
func, free some of them or all in the destructor func. Then there is
one memleak that some errors happen after register_netdevice invokes
the ndo_init callback. Because only the ndo_uninit callback is invoked
in the error handler of register_netdevice, but destructor not.
In my original approach, I tried to free the resources in the newlink
func when fail to register_netdevice, like destructor did except not
free the net_dev. This method is not good when destructor is changed,
and the memleak could be not fixed when there is no newlink callback.
Now create one new func used to free the resources in the destructor,
and the ndo_uninit func also could invokes it when fail to register
the net_device by comparing the dev->reg_state with NETREG_UNINITIALIZED.
If there is no existing ndo_uninit, just add one.
This solution doesn't only make sure free all resources in any case,
but also follows the original desgin that some resources could be kept
until the destructor executes normally after register the device
successfully.
Gao Feng (12):
driver: dummy: Fix one possbile memleak when fail to
register_netdevice
driver: ifb: Fix one possbile memleak when fail to register_netdevice
driver: loopback: Fix one possbile memleak when fail to
register_netdevice
driver: team: Fix one possbile memleak when fail to register_netdevice
driver: veth: Fix one possbile memleak when fail to register_netdevice
net: ip6_gre: Fix one possbile memleak when fail to register_netdevice
ip6_tunnel: Fix one possbile memleak when fail to register_netdevice
net: ip6_vti: Fix one possbile memleak when fail to register_netdevice
net: ip_tunnel: Fix one possbile memleak when fail to
register_netdevice
net: sit: Fix one possbile memleak when fail to register_netdevice
net: vlan: Fix one possbile memleak when fail to register_netdevice
net: batman-adv: Fix one possbile memleak when fail to
register_netdevice
drivers/net/dummy.c | 14 +++++++++++---
drivers/net/ifb.c | 33 +++++++++++++++++++++++----------
drivers/net/loopback.c | 15 ++++++++++++++-
drivers/net/team/team.c | 15 ++++++++++++---
drivers/net/veth.c | 15 ++++++++++++++-
net/8021q/vlan_dev.c | 17 +++++++++++++----
net/batman-adv/soft-interface.c | 18 +++++++++++++++---
net/ipv4/ip_tunnel.c | 11 ++++++++++-
net/ipv6/ip6_gre.c | 17 +++++++++++++----
net/ipv6/ip6_tunnel.c | 11 ++++++++++-
net/ipv6/ip6_vti.c | 11 ++++++++++-
net/ipv6/sit.c | 17 +++++++++++++----
12 files changed, 158 insertions(+), 36 deletions(-)
^ permalink raw reply
* [PATCH net v4 01/12] driver: dummy: Fix one possbile memleak when fail to register_netdevice
From: gfree.wind @ 2017-05-02 5:58 UTC (permalink / raw)
To: davem, jiri, mareklindner, sw, a, kuznet, jmorris, yoshfuji,
kaber, steffen.klassert, herbert, netdev
Cc: Gao Feng
In-Reply-To: <cover.1493699451.git.gfree.wind@foxmail.com>
From: Gao Feng <gfree.wind@foxmail.com>
The dummy driver allocates dev->dstats and priv->vfinfo in its
ndo_init func dummy_dev_init, free the dev->dstats in the ndo_uninit
and free the priv->vfinfo in its destructor func. Then there is one
memleak that some errors happen after register_netdevice invokes the
ndo_init callback. Because only the ndo_uninit callback is invoked in
the error handler of register_netdevice, but destructor not.
Now create one new func dummy_destructor_free to free the mem in the
destructor, and the ndo_uninit func also invokes it when fail to
register the dummy device.
It's not only free all resources, but also follow the original desgin
that the priv->vfinfo is freed in the destructor normally after
register the device successfully.
Signed-off-by: Gao Feng <gfree.wind@foxmail.com>
---
drivers/net/dummy.c | 14 +++++++++++---
1 file changed, 11 insertions(+), 3 deletions(-)
diff --git a/drivers/net/dummy.c b/drivers/net/dummy.c
index 2c80611..0b3c1cc 100644
--- a/drivers/net/dummy.c
+++ b/drivers/net/dummy.c
@@ -153,9 +153,19 @@ static int dummy_dev_init(struct net_device *dev)
return 0;
}
+static void dummy_destructor_free(struct net_device *dev)
+{
+ struct dummy_priv *priv = netdev_priv(dev);
+
+ kfree(priv->vfinfo);
+}
+
static void dummy_dev_uninit(struct net_device *dev)
{
free_percpu(dev->dstats);
+ /* dev is not registered, perform the free instead of destructor */
+ if (dev->reg_state == NETREG_UNINITIALIZED)
+ dummy_destructor_free(dev);
}
static int dummy_change_carrier(struct net_device *dev, bool new_carrier)
@@ -310,9 +320,7 @@ static void dummy_get_drvinfo(struct net_device *dev,
static void dummy_free_netdev(struct net_device *dev)
{
- struct dummy_priv *priv = netdev_priv(dev);
-
- kfree(priv->vfinfo);
+ dummy_destructor_free(dev);
free_netdev(dev);
}
--
1.9.1
^ permalink raw reply related
* [PATCH net v4 06/12] net: ip6_gre: Fix one possbile memleak when fail to register_netdevice
From: gfree.wind @ 2017-05-02 5:58 UTC (permalink / raw)
To: davem, jiri, mareklindner, sw, a, kuznet, jmorris, yoshfuji,
kaber, steffen.klassert, herbert, netdev
Cc: Gao Feng
In-Reply-To: <cover.1493699451.git.gfree.wind@foxmail.com>
From: Gao Feng <gfree.wind@foxmail.com>
The ip6_gre allocates some resources in its ndo_init func, and
free some of them in its destructor func. Then there is one memleak
that some errors happen after register_netdevice invokes the ndo_init
callback. Because only the ndo_uninit callback is invoked in the error
handler of register_netdevice, but destructor not.
Now create one new func ip6_gre_destructor_free to free the mem in
the destructor, and ndo_uninit func also invokes it when fail to
register the ip6_gre device.
It's not only free all resources, but also follow the original desgin
that the resources are freed in the destructor normally after
register the device successfully.
Signed-off-by: Gao Feng <gfree.wind@foxmail.com>
---
net/ipv6/ip6_gre.c | 17 +++++++++++++----
1 file changed, 13 insertions(+), 4 deletions(-)
diff --git a/net/ipv6/ip6_gre.c b/net/ipv6/ip6_gre.c
index 6fcb7cb..e53c3ac 100644
--- a/net/ipv6/ip6_gre.c
+++ b/net/ipv6/ip6_gre.c
@@ -355,6 +355,14 @@ static struct ip6_tnl *ip6gre_tunnel_locate(struct net *net,
return NULL;
}
+static void ip6gre_destructor_free(struct net_device *dev)
+{
+ struct ip6_tnl *t = netdev_priv(dev);
+
+ dst_cache_destroy(&t->dst_cache);
+ free_percpu(dev->tstats);
+}
+
static void ip6gre_tunnel_uninit(struct net_device *dev)
{
struct ip6_tnl *t = netdev_priv(dev);
@@ -363,6 +371,10 @@ static void ip6gre_tunnel_uninit(struct net_device *dev)
ip6gre_tunnel_unlink(ign, t);
dst_cache_reset(&t->dst_cache);
dev_put(dev);
+
+ /* dev is not registered, perform the free instead of destructor */
+ if (dev->reg_state == NETREG_UNINITIALIZED)
+ ip6gre_destructor_free(dev);
}
@@ -981,10 +993,7 @@ static int ip6gre_header(struct sk_buff *skb, struct net_device *dev,
static void ip6gre_dev_free(struct net_device *dev)
{
- struct ip6_tnl *t = netdev_priv(dev);
-
- dst_cache_destroy(&t->dst_cache);
- free_percpu(dev->tstats);
+ ip6gre_destructor_free(dev);
free_netdev(dev);
}
--
1.9.1
^ permalink raw reply related
* [PATCH net v4 02/12] driver: ifb: Fix one possbile memleak when fail to register_netdevice
From: gfree.wind @ 2017-05-02 5:58 UTC (permalink / raw)
To: davem, jiri, mareklindner, sw, a, kuznet, jmorris, yoshfuji,
kaber, steffen.klassert, herbert, netdev
Cc: Gao Feng
In-Reply-To: <cover.1493699451.git.gfree.wind@foxmail.com>
From: Gao Feng <gfree.wind@foxmail.com>
The ifb driver allocates some resources in its ndo_init func, and free
them in its destructor func. Then there is one memleak that some errors
happen after register_netdevice invokes the ndo_init callback. Because
the destructor would not be invoked to free the resources.
Now create one new func ifb_destructor_free to free the mem in the
destructor, and add ndo_uninit func also invokes it when fail to register
the ifb device.
It's not only free all resources, but also follow the original desgin
that the resources are freed in the destructor normally after
register the device successfully.
Signed-off-by: Gao Feng <gfree.wind@foxmail.com>
---
drivers/net/ifb.c | 33 +++++++++++++++++++++++----------
1 file changed, 23 insertions(+), 10 deletions(-)
diff --git a/drivers/net/ifb.c b/drivers/net/ifb.c
index 312fce7..b25aea1 100644
--- a/drivers/net/ifb.c
+++ b/drivers/net/ifb.c
@@ -180,6 +180,27 @@ static int ifb_dev_init(struct net_device *dev)
return 0;
}
+static void ifb_destructor_free(struct net_device *dev)
+{
+ struct ifb_dev_private *dp = netdev_priv(dev);
+ struct ifb_q_private *txp = dp->tx_private;
+ int i;
+
+ for (i = 0; i < dev->num_tx_queues; i++, txp++) {
+ tasklet_kill(&txp->ifb_tasklet);
+ __skb_queue_purge(&txp->rq);
+ __skb_queue_purge(&txp->tq);
+ }
+ kfree(dp->tx_private);
+}
+
+static void ifb_dev_uninit(struct net_device *dev)
+{
+ /* dev is not registered, perform the free instead of destructor */
+ if (dev->reg_state == NETREG_UNINITIALIZED)
+ ifb_destructor_free(dev);
+}
+
static const struct net_device_ops ifb_netdev_ops = {
.ndo_open = ifb_open,
.ndo_stop = ifb_close,
@@ -187,6 +208,7 @@ static int ifb_dev_init(struct net_device *dev)
.ndo_start_xmit = ifb_xmit,
.ndo_validate_addr = eth_validate_addr,
.ndo_init = ifb_dev_init,
+ .ndo_uninit = ifb_dev_uninit,
};
#define IFB_FEATURES (NETIF_F_HW_CSUM | NETIF_F_SG | NETIF_F_FRAGLIST | \
@@ -197,16 +219,7 @@ static int ifb_dev_init(struct net_device *dev)
static void ifb_dev_free(struct net_device *dev)
{
- struct ifb_dev_private *dp = netdev_priv(dev);
- struct ifb_q_private *txp = dp->tx_private;
- int i;
-
- for (i = 0; i < dev->num_tx_queues; i++,txp++) {
- tasklet_kill(&txp->ifb_tasklet);
- __skb_queue_purge(&txp->rq);
- __skb_queue_purge(&txp->tq);
- }
- kfree(dp->tx_private);
+ ifb_destructor_free(dev);
free_netdev(dev);
}
--
1.9.1
^ permalink raw reply related
* [PATCH net v4 04/12] driver: team: Fix one possbile memleak when fail to register_netdevice
From: gfree.wind @ 2017-05-02 5:58 UTC (permalink / raw)
To: davem, jiri, mareklindner, sw, a, kuznet, jmorris, yoshfuji,
kaber, steffen.klassert, herbert, netdev
Cc: Gao Feng
In-Reply-To: <cover.1493699451.git.gfree.wind@foxmail.com>
From: Gao Feng <gfree.wind@foxmail.com>
The team driver allocates some resources in its ndo_init func, and
free some of them in its destructor func. Then there is one memleak
that some errors happen after register_netdevice invokes the ndo_init
callback. Because only the ndo_uninit callback is invoked in the error
handler of register_netdevice, but destructor not.
Now create one new func team_destructor_free to free the mem in the
destructor, and ndo_uninit func also invokes it when fail to register
the team device.
It's not only free all resources, but also follow the original desgin
that the resources are freed in the destructor normally after
register the device successfully.
Signed-off-by: Gao Feng <gfree.wind@foxmail.com>
---
drivers/net/team/team.c | 15 ++++++++++++---
1 file changed, 12 insertions(+), 3 deletions(-)
diff --git a/drivers/net/team/team.c b/drivers/net/team/team.c
index 85c0124..d8dd203 100644
--- a/drivers/net/team/team.c
+++ b/drivers/net/team/team.c
@@ -1619,6 +1619,13 @@ static int team_init(struct net_device *dev)
return err;
}
+static void team_destructor_free(struct net_device *dev)
+{
+ struct team *team = netdev_priv(dev);
+
+ free_percpu(team->pcpu_stats);
+}
+
static void team_uninit(struct net_device *dev)
{
struct team *team = netdev_priv(dev);
@@ -1636,13 +1643,15 @@ static void team_uninit(struct net_device *dev)
team_queue_override_fini(team);
mutex_unlock(&team->lock);
netdev_change_features(dev);
+
+ /* dev is not registered, perform the free instead of destructor */
+ if (dev->reg_state == NETREG_UNINITIALIZED)
+ team_destructor_free(dev);
}
static void team_destructor(struct net_device *dev)
{
- struct team *team = netdev_priv(dev);
-
- free_percpu(team->pcpu_stats);
+ team_destructor_free(dev);
free_netdev(dev);
}
--
1.9.1
^ permalink raw reply related
* [PATCH net v4 05/12] driver: veth: Fix one possbile memleak when fail to register_netdevice
From: gfree.wind @ 2017-05-02 5:58 UTC (permalink / raw)
To: davem, jiri, mareklindner, sw, a, kuznet, jmorris, yoshfuji,
kaber, steffen.klassert, herbert, netdev
Cc: Gao Feng
In-Reply-To: <cover.1493699451.git.gfree.wind@foxmail.com>
From: Gao Feng <gfree.wind@foxmail.com>
The veth driver allocates some resources in its ndo_init func, and
free them in its destructor func. Then there is one memleak that some
errors happen after register_netdevice invokes the ndo_init callback.
Because the destructor would not be invoked to free the resources.
Now create one new func veth_destructor_free to free the mem in the
destructor, and add ndo_uninit func also invokes it when fail to register
the veth device.
It's not only free all resources, but also follow the original desgin
that the resources are freed in the destructor normally after
register the device successfully.
Signed-off-by: Gao Feng <gfree.wind@foxmail.com>
---
drivers/net/veth.c | 15 ++++++++++++++-
1 file changed, 14 insertions(+), 1 deletion(-)
diff --git a/drivers/net/veth.c b/drivers/net/veth.c
index 8c39d6d..418376a 100644
--- a/drivers/net/veth.c
+++ b/drivers/net/veth.c
@@ -224,9 +224,21 @@ static int veth_dev_init(struct net_device *dev)
return 0;
}
-static void veth_dev_free(struct net_device *dev)
+static void veth_destructor_free(struct net_device *dev)
{
free_percpu(dev->vstats);
+}
+
+static void veth_dev_uninit(struct net_device *dev)
+{
+ /* dev is not registered, perform the free instead of destructor */
+ if (dev->reg_state == NETREG_UNINITIALIZED)
+ veth_destructor_free(dev);
+}
+
+static void veth_dev_free(struct net_device *dev)
+{
+ veth_destructor_free(dev);
free_netdev(dev);
}
@@ -284,6 +296,7 @@ static void veth_set_rx_headroom(struct net_device *dev, int new_hr)
static const struct net_device_ops veth_netdev_ops = {
.ndo_init = veth_dev_init,
+ .ndo_uninit = veth_dev_uninit,
.ndo_open = veth_open,
.ndo_stop = veth_close,
.ndo_start_xmit = veth_xmit,
--
1.9.1
^ permalink raw reply related
* [PATCH net v4 03/12] driver: loopback: Fix one possbile memleak when fail to register_netdevice
From: gfree.wind @ 2017-05-02 5:58 UTC (permalink / raw)
To: davem, jiri, mareklindner, sw, a, kuznet, jmorris, yoshfuji,
kaber, steffen.klassert, herbert, netdev
Cc: Gao Feng
In-Reply-To: <cover.1493699451.git.gfree.wind@foxmail.com>
From: Gao Feng <gfree.wind@foxmail.com>
The loopback driver allocates some resources in its ndo_init func, and
free them in its destructor func. Then there is one memleak that some
errors happen after register_netdevice invokes the ndo_init callback.
Because the destructor would not be invoked to free the resources.
Now create one new func loopback_destructor_free to free the mem in
the destructor, and add ndo_uninit func also invokes it when fail to
register the loopback device.
It's not only free all resources, but also follow the original desgin
that the resources are freed in the destructor normally after
register the device successfully.
Signed-off-by: Gao Feng <gfree.wind@foxmail.com>
---
drivers/net/loopback.c | 15 ++++++++++++++-
1 file changed, 14 insertions(+), 1 deletion(-)
diff --git a/drivers/net/loopback.c b/drivers/net/loopback.c
index b23b719..d7c1016 100644
--- a/drivers/net/loopback.c
+++ b/drivers/net/loopback.c
@@ -141,15 +141,28 @@ static int loopback_dev_init(struct net_device *dev)
return 0;
}
-static void loopback_dev_free(struct net_device *dev)
+static void loopback_destructor_free(struct net_device *dev)
{
dev_net(dev)->loopback_dev = NULL;
free_percpu(dev->lstats);
+}
+
+static void loopback_dev_uninit(struct net_device *dev)
+{
+ /* dev is not registered, perform the free instead of destructor */
+ if (dev->reg_state == NETREG_UNINITIALIZED)
+ loopback_destructor_free(dev);
+}
+
+static void loopback_dev_free(struct net_device *dev)
+{
+ loopback_destructor_free(dev);
free_netdev(dev);
}
static const struct net_device_ops loopback_ops = {
.ndo_init = loopback_dev_init,
+ .ndo_uninit = loopback_dev_uninit,
.ndo_start_xmit= loopback_xmit,
.ndo_get_stats64 = loopback_get_stats64,
.ndo_set_mac_address = eth_mac_addr,
--
1.9.1
^ permalink raw reply related
* Re: [PATCH 1/2] PCI: Add new PCIe Fabric End Node flag, PCI_DEV_FLAGS_NO_RELAXED_ORDERING
From: Ding Tianhong @ 2017-05-02 6:49 UTC (permalink / raw)
To: Casey Leedom, Bjorn Helgaas, leedom
Cc: Michael Werner, Ganesh Goudar, Arjun V, David Miller,
Asit K Mallick, Patrick J Cramer, Ashok Raj,
Suravee Suthikulpanit, Bob Shaw, h, Alexander Duyck, Mark Rutland,
Amir Ancel, Gabriele Paoloni, Catalin Marinas, Will Deacon,
LinuxArm, David Laight, jeffrey.t.kirsher, netdev
In-Reply-To: <758d0e431c732fe133e7b0e660bde5fc1beccdba.1493678834.git.leedom@chelsio.com>
hi, Casey:
On 2017/5/2 7:13, Casey Leedom wrote:
> The new flag PCI_DEV_FLAGS_NO_RELAXED_ORDERING indicates that the Relaxed
> Ordering Attribute should not be used on Transaction Layer Packets destined
> for the PCIe End Node so flagged. Initially flagged this way are Intel
> E5-26xx Root Complex Ports which suffer from a Flow Control Credit
> Performance Problem and AMD A1100 ARM ("SEATTLE") Root Complex Ports which
> don't obey PCIe 3.0 ordering rules which can lead to Data Corruption.
> ---
> drivers/pci/quirks.c | 38 ++++++++++++++++++++++++++++++++++++++
> include/linux/pci.h | 2 ++
> 2 files changed, 40 insertions(+)
>
> diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
> index f754453..4ae78b3 100644
> --- a/drivers/pci/quirks.c
> +++ b/drivers/pci/quirks.c
> @@ -3979,6 +3979,44 @@ static void quirk_tw686x_class(struct pci_dev *pdev)
> quirk_tw686x_class);
>
> /*
> + * Some devices have problems with Transaction Layer Packets with the Relaxed
> + * Ordering Attribute set. Such devices should mark themselves and other
> + * Device Drivers should check before sending TLPs with RO set.
> + */
> +static void quirk_relaxedordering_disable(struct pci_dev *dev)
> +{
> + dev->dev_flags |= PCI_DEV_FLAGS_NO_RELAXED_ORDERING;
> +}
> +
> +/*
> + * Intel E5-26xx Root Complex has a Flow Control Credit issue which can
> + * cause performance problems with Upstream Transaction Layer Packets with
> + * Relaxed Ordering set.
> + */
> +DECLARE_PCI_FIXUP_CLASS_EARLY(0x8086, 0x6f02, PCI_CLASS_NOT_DEFINED, 8,
> + quirk_relaxedordering_disable);
> +DECLARE_PCI_FIXUP_CLASS_EARLY(0x8086, 0x6f04, PCI_CLASS_NOT_DEFINED, 8,
> + quirk_relaxedordering_disable);
> +DECLARE_PCI_FIXUP_CLASS_EARLY(0x8086, 0x6f08, PCI_CLASS_NOT_DEFINED, 8,
> + quirk_relaxedordering_disable);
> +
> +/*
> + * The AMD ARM A1100 (AKA "SEATTLE") SoC has a bug in its PCIe Root Complex
> + * where Upstream Transaction Layer Packets with the Relaxed Ordering
> + * Attribute clear are allowed to bypass earlier TLPs with Relaxed Ordering
> + * set. This is a violation of the PCIe 3.0 Transaction Ordering Rules
> + * outlined in Section 2.4.1 (PCI Express(r) Base Specification Revision 3.0
> + * November 10, 2010). As a result, on this platform we can't use Relaxed
> + * Ordering for Upstream TLPs.
> + */
> +DECLARE_PCI_FIXUP_CLASS_EARLY(0x1022, 0x1a00, PCI_CLASS_NOT_DEFINED, 8,
> + quirk_relaxedordering_disable);
> +DECLARE_PCI_FIXUP_CLASS_EARLY(0x1022, 0x1a01, PCI_CLASS_NOT_DEFINED, 8,
> + quirk_relaxedordering_disable);
> +DECLARE_PCI_FIXUP_CLASS_EARLY(0x1022, 0x1a02, PCI_CLASS_NOT_DEFINED, 8,
> + quirk_relaxedordering_disable);
> +
> +/*
> * Per PCIe r3.0, sec 2.2.9, "Completion headers must supply the same
> * values for the Attribute as were supplied in the header of the
> * corresponding Request, except as explicitly allowed when IDO is used."
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index eb3da1a..6764f66 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -178,6 +178,8 @@ enum pci_dev_flags {
> PCI_DEV_FLAGS_NO_PM_RESET = (__force pci_dev_flags_t) (1 << 7),
> /* Get VPD from function 0 VPD */
> PCI_DEV_FLAGS_VPD_REF_F0 = (__force pci_dev_flags_t) (1 << 8),
> + /* Don't use Relaxed Ordering for TLPs directed at this device */
> + PCI_DEV_FLAGS_NO_RELAXED_ORDERING = (__force pci_dev_flags_t) (1 << 9),
> };
What about add a new general func to check the RO for several drivers to use them ?
just like:
#define pci_dev_support_relaxed_ordering(struct pci_dev *root) \
(!(root->dev_flags & PCI_DEV_FLAGS_NO_RELAXED_ORDERING))
Thanks
Ding
>
> enum pci_irq_reroute_variant {
>
^ permalink raw reply
* [PATCH net v4 07/12] ip6_tunnel: Fix one possbile memleak when fail to register_netdevice
From: gfree.wind @ 2017-05-02 6:59 UTC (permalink / raw)
To: davem, jiri, mareklindner, sw, a, kuznet, jmorris, yoshfuji,
kaber, steffen.klassert, herbert, netdev
Cc: Gao Feng
In-Reply-To: <cover.1493699451.git.gfree.wind@foxmail.com>
From: Gao Feng <gfree.wind@foxmail.com>
The ip6_tunnel allocates some resources in its ndo_init func, and
free some of them in its destructor func. Then there is one memleak
that some errors happen after register_netdevice invokes the ndo_init
callback. Because only the ndo_uninit callback is invoked in the error
handler of register_netdevice, but destructor not.
Now create one new func ip6_tnl_destructor_free to free the mem in
the destructor, and ndo_uninit func also invokes it when fail to
register the ip6_tunnel device.
It's not only free all resources, but also follow the original desgin
that the resources are freed in the destructor normally after
register the device successfully.
Signed-off-by: Gao Feng <gfree.wind@foxmail.com>
---
net/ipv6/ip6_tunnel.c | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/net/ipv6/ip6_tunnel.c b/net/ipv6/ip6_tunnel.c
index a9692ec..7dcb234 100644
--- a/net/ipv6/ip6_tunnel.c
+++ b/net/ipv6/ip6_tunnel.c
@@ -247,13 +247,18 @@ static struct net_device_stats *ip6_get_stats(struct net_device *dev)
}
}
-static void ip6_dev_free(struct net_device *dev)
+static void ip6_tnl_destructor_free(struct net_device *dev)
{
struct ip6_tnl *t = netdev_priv(dev);
gro_cells_destroy(&t->gro_cells);
dst_cache_destroy(&t->dst_cache);
free_percpu(dev->tstats);
+}
+
+static void ip6_dev_free(struct net_device *dev)
+{
+ ip6_tnl_destructor_free(dev);
free_netdev(dev);
}
@@ -387,6 +392,10 @@ static struct ip6_tnl *ip6_tnl_locate(struct net *net,
ip6_tnl_unlink(ip6n, t);
dst_cache_reset(&t->dst_cache);
dev_put(dev);
+
+ /* dev is not registered, perform the free instead of destructor */
+ if (dev->reg_state == NETREG_UNINITIALIZED)
+ ip6_tnl_destructor_free(dev);
}
/**
--
1.9.1
^ permalink raw reply related
* Re: [PATCH net v3] driver: veth: Fix one possbile memleak when fail to register_netdevice
From: Xin Long @ 2017-05-02 7:55 UTC (permalink / raw)
To: gfree.wind; +Cc: davem, jarod, Stephen Hemminger, dsa, network dev
In-Reply-To: <1493437911-27167-1-git-send-email-gfree.wind@foxmail.com>
On Sat, Apr 29, 2017 at 11:51 AM, <gfree.wind@foxmail.com> wrote:
> From: Gao Feng <gfree.wind@foxmail.com>
>
> The veth driver allocates some resources in its ndo_init func, and
> free them in its destructor func. Then there is one memleak that some
> errors happen after register_netdevice invokes the ndo_init callback.
> Because the destructor would not be invoked to free the resources.
>
> Now create one new func veth_destructor_free to free the mem in the
> destructor, and add ndo_uninit func also invokes it when fail to register
> the veth device.
>
> It's not only free all resources, but also follow the original desgin
> that the resources are freed in the destructor normally after
> register the device successfully.
>
> Signed-off-by: Gao Feng <gfree.wind@foxmail.com>
> ---
> v3: Split one patch to multiple commits, per David Ahern
> v2: Move the free in ndo_uninit when fail to register, per Herbert Xu
> v1: initial version
>
> drivers/net/veth.c | 15 ++++++++++++++-
> 1 file changed, 14 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/veth.c b/drivers/net/veth.c
> index 8c39d6d..418376a 100644
> --- a/drivers/net/veth.c
> +++ b/drivers/net/veth.c
> @@ -224,9 +224,21 @@ static int veth_dev_init(struct net_device *dev)
> return 0;
> }
>
> -static void veth_dev_free(struct net_device *dev)
> +static void veth_destructor_free(struct net_device *dev)
> {
> free_percpu(dev->vstats);
> +}
not sure why you needed to add this function.
to use free_percpu() directly may be clearer.
> +
> +static void veth_dev_uninit(struct net_device *dev)
> +{
call free_percpu() here, no need to check dev->reg_state.
free_percpu will just return if dev->vstats is NULL.
> + /* dev is not registered, perform the free instead of destructor */
> + if (dev->reg_state == NETREG_UNINITIALIZED)
> + veth_destructor_free(dev);
> +}
> +
> +static void veth_dev_free(struct net_device *dev)
> +{
> + veth_destructor_free(dev);
use free_percpu here as well.
> free_netdev(dev);
> }
>
> @@ -284,6 +296,7 @@ static void veth_set_rx_headroom(struct net_device *dev, int new_hr)
>
> static const struct net_device_ops veth_netdev_ops = {
> .ndo_init = veth_dev_init,
> + .ndo_uninit = veth_dev_uninit,
> .ndo_open = veth_open,
> .ndo_stop = veth_close,
> .ndo_start_xmit = veth_xmit,
> --
> 2.7.4
>
^ permalink raw reply
* BENEFIT
From: Mrs Julie Leach @ 2017-05-02 7:37 UTC (permalink / raw)
To: Recipients
You are a recipient to Mrs Julie Leach Donation of $3 million USD. Contact(julieleach93@gmail.com) for claims.
^ permalink raw reply
* [PATCH v2] stmmac: Add support for SIMATIC IOT2000 platform
From: Jan Kiszka @ 2017-05-02 7:58 UTC (permalink / raw)
To: Giuseppe Cavallaro, Alexandre Torgue, David Miller
Cc: netdev, Linux Kernel Mailing List, Sascha Weisenberger,
Andy Shevchenko
The IOT2000 is industrial controller platform, derived from the Intel
Galileo Gen2 board. The variant IOT2020 comes with one LAN port, the
IOT2040 has two of them. They can be told apart based on the board asset
tag in the DMI table.
Based on patch by Sascha Weisenberger.
Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
Signed-off-by: Sascha Weisenberger <sascha.weisenberger@siemens.com>
---
Changes in v2:
- reformatted match conditions [Andy]
drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c | 26 +++++++++++++++++++++++-
1 file changed, 25 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
index 5c9e462276b9..11d2229e536b 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
@@ -32,6 +32,7 @@
*/
struct stmmac_pci_dmi_data {
const char *name;
+ const char *asset_tag;
unsigned int func;
int phy_addr;
};
@@ -46,6 +47,7 @@ struct stmmac_pci_info {
static int stmmac_pci_find_phy_addr(struct stmmac_pci_info *info)
{
const char *name = dmi_get_system_info(DMI_BOARD_NAME);
+ const char *asset_tag = dmi_get_system_info(DMI_BOARD_ASSET_TAG);
unsigned int func = PCI_FUNC(info->pdev->devfn);
struct stmmac_pci_dmi_data *dmi;
@@ -57,8 +59,12 @@ static int stmmac_pci_find_phy_addr(struct stmmac_pci_info *info)
return 1;
for (dmi = info->dmi; dmi->name && *dmi->name; dmi++) {
- if (!strcmp(dmi->name, name) && dmi->func == func)
+ if (!strcmp(dmi->name, name) && dmi->func == func) {
+ /* If asset tag is provided, match on it as well. */
+ if (dmi->asset_tag && strcmp(dmi->asset_tag, asset_tag))
+ continue;
return dmi->phy_addr;
+ }
}
return -ENODEV;
@@ -142,6 +148,24 @@ static struct stmmac_pci_dmi_data quark_pci_dmi_data[] = {
.func = 6,
.phy_addr = 1,
},
+ {
+ .name = "SIMATIC IOT2000",
+ .asset_tag = "6ES7647-0AA00-0YA2",
+ .func = 6,
+ .phy_addr = 1,
+ },
+ {
+ .name = "SIMATIC IOT2000",
+ .asset_tag = "6ES7647-0AA00-1YA2",
+ .func = 6,
+ .phy_addr = 1,
+ },
+ {
+ .name = "SIMATIC IOT2000",
+ .asset_tag = "6ES7647-0AA00-1YA2",
+ .func = 7,
+ .phy_addr = 1,
+ },
{}
};
^ permalink raw reply related
* Re: [PATCH v2] stmmac: Add support for SIMATIC IOT2000 platform
From: Andy Shevchenko @ 2017-05-02 8:02 UTC (permalink / raw)
To: Jan Kiszka
Cc: Giuseppe Cavallaro, Alexandre Torgue, David Miller, netdev,
Linux Kernel Mailing List, Sascha Weisenberger
In-Reply-To: <aa7a7f82-7933-1a97-12c4-b51efab498c9@siemens.com>
On Tue, May 2, 2017 at 10:58 AM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
> The IOT2000 is industrial controller platform, derived from the Intel
> Galileo Gen2 board. The variant IOT2020 comes with one LAN port, the
> IOT2040 has two of them. They can be told apart based on the board asset
> tag in the DMI table.
>
> Based on patch by Sascha Weisenberger.
>
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> Signed-off-by: Sascha Weisenberger <sascha.weisenberger@siemens.com>
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
> ---
>
> Changes in v2:
> - reformatted match conditions [Andy]
>
> drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c | 26 +++++++++++++++++++++++-
> 1 file changed, 25 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
> index 5c9e462276b9..11d2229e536b 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
> @@ -32,6 +32,7 @@
> */
> struct stmmac_pci_dmi_data {
> const char *name;
> + const char *asset_tag;
> unsigned int func;
> int phy_addr;
> };
> @@ -46,6 +47,7 @@ struct stmmac_pci_info {
> static int stmmac_pci_find_phy_addr(struct stmmac_pci_info *info)
> {
> const char *name = dmi_get_system_info(DMI_BOARD_NAME);
> + const char *asset_tag = dmi_get_system_info(DMI_BOARD_ASSET_TAG);
> unsigned int func = PCI_FUNC(info->pdev->devfn);
> struct stmmac_pci_dmi_data *dmi;
>
> @@ -57,8 +59,12 @@ static int stmmac_pci_find_phy_addr(struct stmmac_pci_info *info)
> return 1;
>
> for (dmi = info->dmi; dmi->name && *dmi->name; dmi++) {
> - if (!strcmp(dmi->name, name) && dmi->func == func)
> + if (!strcmp(dmi->name, name) && dmi->func == func) {
> + /* If asset tag is provided, match on it as well. */
> + if (dmi->asset_tag && strcmp(dmi->asset_tag, asset_tag))
> + continue;
> return dmi->phy_addr;
> + }
> }
>
> return -ENODEV;
> @@ -142,6 +148,24 @@ static struct stmmac_pci_dmi_data quark_pci_dmi_data[] = {
> .func = 6,
> .phy_addr = 1,
> },
> + {
> + .name = "SIMATIC IOT2000",
> + .asset_tag = "6ES7647-0AA00-0YA2",
> + .func = 6,
> + .phy_addr = 1,
> + },
> + {
> + .name = "SIMATIC IOT2000",
> + .asset_tag = "6ES7647-0AA00-1YA2",
> + .func = 6,
> + .phy_addr = 1,
> + },
> + {
> + .name = "SIMATIC IOT2000",
> + .asset_tag = "6ES7647-0AA00-1YA2",
> + .func = 7,
> + .phy_addr = 1,
> + },
> {}
> };
>
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* [PATCH net 0/2] qed*: PTP bug fixes.
From: Sudarsana Reddy Kalluru @ 2017-05-02 8:11 UTC (permalink / raw)
To: davem; +Cc: richardcochran, netdev, Yuval.Mintz, Sudarsana Reddy Kalluru
From: Sudarsana Reddy Kalluru <Sudarsana.Kalluru@cavium.com>
The series addresses couple of issues in the PTP implementation.
Please consider applying it to 'net' branch.
Sudarsana Reddy Kalluru (2):
qede: Fix concurrency issue in PTP Tx path processing.
qed*: Fix issues in the ptp filter config implementation.
drivers/net/ethernet/qlogic/qed/qed_ptp.c | 84 ++++++++++++++++++-----------
drivers/net/ethernet/qlogic/qede/qede.h | 3 +-
drivers/net/ethernet/qlogic/qede/qede_ptp.c | 38 ++++++++++---
include/linux/qed/qed_eth_if.h | 23 +++++---
4 files changed, 104 insertions(+), 44 deletions(-)
--
1.8.3.1
^ permalink raw reply
* [PATCH net 1/2] qede: Fix concurrency issue in PTP Tx path processing.
From: Sudarsana Reddy Kalluru @ 2017-05-02 8:11 UTC (permalink / raw)
To: davem; +Cc: richardcochran, netdev, Yuval.Mintz
In-Reply-To: <20170502081103.4252-1-sudarsana.kalluru@cavium.com>
PTP Tx timestamping data structures are not protected against the
concurrent access in the Tx paths. Protecting the same using atomic
bit locks.
Signed-off-by: Sudarsana Reddy Kalluru <Sudarsana.Kalluru@cavium.com>
Signed-off-by: Yuval Mintz <Yuval.Mintz@cavium.com>
---
drivers/net/ethernet/qlogic/qede/qede.h | 5 +++--
drivers/net/ethernet/qlogic/qede/qede_ptp.c | 4 ++++
2 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/qlogic/qede/qede.h b/drivers/net/ethernet/qlogic/qede/qede.h
index f2aaef2..378b654 100644
--- a/drivers/net/ethernet/qlogic/qede/qede.h
+++ b/drivers/net/ethernet/qlogic/qede/qede.h
@@ -147,10 +147,11 @@ struct qede_dev {
u32 dp_module;
u8 dp_level;
- u32 flags;
-#define QEDE_FLAG_IS_VF BIT(0)
+ unsigned long flags;
+#define QEDE_FLAG_IS_VF BIT(0)
#define IS_VF(edev) (!!((edev)->flags & QEDE_FLAG_IS_VF))
#define QEDE_TX_TIMESTAMPING_EN BIT(1)
+#define QEDE_FLAGS_PTP_TX_IN_PRORGESS BIT(2)
const struct qed_eth_ops *ops;
struct qede_ptp *ptp;
diff --git a/drivers/net/ethernet/qlogic/qede/qede_ptp.c b/drivers/net/ethernet/qlogic/qede/qede_ptp.c
index 2e62dec..f5f743f 100644
--- a/drivers/net/ethernet/qlogic/qede/qede_ptp.c
+++ b/drivers/net/ethernet/qlogic/qede/qede_ptp.c
@@ -181,6 +181,7 @@ static void qede_ptp_task(struct work_struct *work)
skb_tstamp_tx(ptp->tx_skb, &shhwtstamps);
dev_kfree_skb_any(ptp->tx_skb);
ptp->tx_skb = NULL;
+ clear_bit_unlock(QEDE_FLAGS_PTP_TX_IN_PRORGESS, &edev->flags);
DP_VERBOSE(edev, QED_MSG_DEBUG,
"Tx timestamp, timestamp cycles = %llu, ns = %llu\n",
@@ -495,6 +496,9 @@ void qede_ptp_tx_ts(struct qede_dev *edev, struct sk_buff *skb)
if (!ptp)
return;
+ if (test_and_set_bit_lock(QEDE_FLAGS_PTP_TX_IN_PRORGESS, &edev->flags))
+ return;
+
if (unlikely(!(edev->flags & QEDE_TX_TIMESTAMPING_EN))) {
DP_NOTICE(edev,
"Tx timestamping was not enabled, this packet will not be timestamped\n");
--
1.8.3.1
^ permalink raw reply related
* [PATCH net 2/2] qed*: Fix issues in the ptp filter config implementation.
From: Sudarsana Reddy Kalluru @ 2017-05-02 8:11 UTC (permalink / raw)
To: davem; +Cc: richardcochran, netdev, Yuval.Mintz
In-Reply-To: <20170502081103.4252-1-sudarsana.kalluru@cavium.com>
PTP hardware filter configuration performed by the driver for a given
user requested config is not correct for some of the PTP modes.
Following changes are needed for PTP config-filter implementation.
1. NIG_REG_TX_PTP_EN register - Bits 0/1/2 respectively enables
TimeSync/"V1 frame format support"/"V2 frame format support" on
the TX side. Set the associated bits based on the user request.
2. ptp4l application fails to operate in Peer Delay mode. Following
changes are needed to fix this,
a. Driver should enable (set to 0) DA #1-related bits for IPv4,
IPv6 and MAC destination addresses in these registers:
NIG_REG_TX_LLH_PTP_RULE_MASK
NIG_REG_LLH_PTP_RULE_MASK
b. NIG_REG_LLH_PTP_PARAM_MASK/NIG_REG_TX_LLH_PTP_PARAM_MASK should
be set to 0x0 in all modes.
Signed-off-by: Sudarsana Reddy Kalluru <Sudarsana.Kalluru@cavium.com>
Signed-off-by: Yuval Mintz <Yuval.Mintz@cavium.com>
---
drivers/net/ethernet/qlogic/qed/qed_ptp.c | 84 ++++++++++++++++++-----------
drivers/net/ethernet/qlogic/qede/qede_ptp.c | 34 +++++++++---
include/linux/qed/qed_eth_if.h | 23 +++++---
3 files changed, 98 insertions(+), 43 deletions(-)
diff --git a/drivers/net/ethernet/qlogic/qed/qed_ptp.c b/drivers/net/ethernet/qlogic/qed/qed_ptp.c
index d27aa85..f4e5c04 100644
--- a/drivers/net/ethernet/qlogic/qed/qed_ptp.c
+++ b/drivers/net/ethernet/qlogic/qed/qed_ptp.c
@@ -112,39 +112,73 @@ static int qed_ptp_hw_read_cc(struct qed_dev *cdev, u64 *phc_cycles)
}
/* Filter PTP protocol packets that need to be timestamped */
-static int qed_ptp_hw_cfg_rx_filters(struct qed_dev *cdev,
- enum qed_ptp_filter_type type)
+static int qed_ptp_hw_cfg_filters(struct qed_dev *cdev,
+ enum qed_ptp_filter_type rx_type,
+ enum qed_ptp_hwtstamp_tx_type tx_type)
{
struct qed_hwfn *p_hwfn = QED_LEADING_HWFN(cdev);
struct qed_ptt *p_ptt = p_hwfn->p_ptp_ptt;
- u32 rule_mask, parm_mask;
+ u32 rule_mask, enable_cfg = 0x0;
- switch (type) {
- case QED_PTP_FILTER_L2_IPV4_IPV6:
- parm_mask = 0x6AA;
- rule_mask = 0x3EEE;
+ switch (rx_type) {
+ case QED_PTP_FILTER_NONE:
+ enable_cfg = 0x0;
+ rule_mask = 0x3FFF;
break;
- case QED_PTP_FILTER_L2:
- parm_mask = 0x6BF;
- rule_mask = 0x3EFF;
+ case QED_PTP_FILTER_ALL:
+ enable_cfg = 0x7;
+ rule_mask = 0x3CAA;
break;
- case QED_PTP_FILTER_IPV4_IPV6:
- parm_mask = 0x7EA;
- rule_mask = 0x3FFE;
+ case QED_PTP_FILTER_V1_L4_EVENT:
+ enable_cfg = 0x3;
+ rule_mask = 0x3FFA;
break;
- case QED_PTP_FILTER_IPV4:
- parm_mask = 0x7EE;
+ case QED_PTP_FILTER_V1_L4_GEN:
+ enable_cfg = 0x3;
rule_mask = 0x3FFE;
break;
+ case QED_PTP_FILTER_V2_L4_EVENT:
+ enable_cfg = 0x5;
+ rule_mask = 0x3FAA;
+ break;
+ case QED_PTP_FILTER_V2_L4_GEN:
+ enable_cfg = 0x5;
+ rule_mask = 0x3FEE;
+ break;
+ case QED_PTP_FILTER_V2_L2_EVENT:
+ enable_cfg = 0x5;
+ rule_mask = 0x3CFF;
+ break;
+ case QED_PTP_FILTER_V2_L2_GEN:
+ enable_cfg = 0x5;
+ rule_mask = 0x3EFF;
+ break;
+ case QED_PTP_FILTER_V2_EVENT:
+ enable_cfg = 0x5;
+ rule_mask = 0x3CAA;
+ break;
+ case QED_PTP_FILTER_V2_GEN:
+ enable_cfg = 0x5;
+ rule_mask = 0x3EEE;
+ break;
default:
- DP_INFO(p_hwfn, "Invalid PTP filter type %d\n", type);
+ DP_INFO(p_hwfn, "Invalid PTP filter type %d\n", rx_type);
return -EINVAL;
}
- qed_wr(p_hwfn, p_ptt, NIG_REG_LLH_PTP_PARAM_MASK, parm_mask);
+ qed_wr(p_hwfn, p_ptt, NIG_REG_LLH_PTP_PARAM_MASK, 0);
qed_wr(p_hwfn, p_ptt, NIG_REG_LLH_PTP_RULE_MASK, rule_mask);
+ qed_wr(p_hwfn, p_ptt, NIG_REG_RX_PTP_EN, enable_cfg);
- qed_wr(p_hwfn, p_ptt, NIG_REG_LLH_PTP_TO_HOST, 0x1);
+ if (tx_type == QED_PTP_HWTSTAMP_TX_OFF) {
+ qed_wr(p_hwfn, p_ptt, NIG_REG_TX_PTP_EN, 0x0);
+ qed_wr(p_hwfn, p_ptt, NIG_REG_TX_LLH_PTP_PARAM_MASK, 0x7FF);
+ qed_wr(p_hwfn, p_ptt, NIG_REG_TX_LLH_PTP_RULE_MASK, 0x3FFF);
+ } else {
+ qed_wr(p_hwfn, p_ptt, NIG_REG_TX_PTP_EN, enable_cfg);
+ qed_wr(p_hwfn, p_ptt, NIG_REG_TX_LLH_PTP_PARAM_MASK, 0);
+ qed_wr(p_hwfn, p_ptt, NIG_REG_TX_LLH_PTP_RULE_MASK, rule_mask);
+ }
/* Reset possibly old timestamps */
qed_wr(p_hwfn, p_ptt, NIG_REG_LLH_PTP_HOST_BUF_SEQID,
@@ -281,17 +315,6 @@ static int qed_ptp_hw_enable(struct qed_dev *cdev)
return 0;
}
-static int qed_ptp_hw_hwtstamp_tx_on(struct qed_dev *cdev)
-{
- struct qed_hwfn *p_hwfn = QED_LEADING_HWFN(cdev);
- struct qed_ptt *p_ptt = p_hwfn->p_ptp_ptt;
-
- qed_wr(p_hwfn, p_ptt, NIG_REG_TX_LLH_PTP_PARAM_MASK, 0x6AA);
- qed_wr(p_hwfn, p_ptt, NIG_REG_TX_LLH_PTP_RULE_MASK, 0x3EEE);
-
- return 0;
-}
-
static int qed_ptp_hw_disable(struct qed_dev *cdev)
{
struct qed_hwfn *p_hwfn = QED_LEADING_HWFN(cdev);
@@ -312,8 +335,7 @@ static int qed_ptp_hw_disable(struct qed_dev *cdev)
}
const struct qed_eth_ptp_ops qed_ptp_ops_pass = {
- .hwtstamp_tx_on = qed_ptp_hw_hwtstamp_tx_on,
- .cfg_rx_filters = qed_ptp_hw_cfg_rx_filters,
+ .cfg_filters = qed_ptp_hw_cfg_filters,
.read_rx_ts = qed_ptp_hw_read_rx_ts,
.read_tx_ts = qed_ptp_hw_read_tx_ts,
.read_cc = qed_ptp_hw_read_cc,
diff --git a/drivers/net/ethernet/qlogic/qede/qede_ptp.c b/drivers/net/ethernet/qlogic/qede/qede_ptp.c
index f5f743f..26aaf63 100644
--- a/drivers/net/ethernet/qlogic/qede/qede_ptp.c
+++ b/drivers/net/ethernet/qlogic/qede/qede_ptp.c
@@ -224,6 +224,8 @@ static void qede_ptp_init_cc(struct qede_dev *edev)
static int qede_ptp_cfg_filters(struct qede_dev *edev)
{
+ enum qed_ptp_hwtstamp_tx_type tx_type = QED_PTP_HWTSTAMP_TX_ON;
+ enum qed_ptp_filter_type rx_filter = QED_PTP_FILTER_NONE;
struct qede_ptp *ptp = edev->ptp;
if (!ptp)
@@ -237,7 +239,12 @@ static int qede_ptp_cfg_filters(struct qede_dev *edev)
switch (ptp->tx_type) {
case HWTSTAMP_TX_ON:
edev->flags |= QEDE_TX_TIMESTAMPING_EN;
- ptp->ops->hwtstamp_tx_on(edev->cdev);
+ tx_type = QED_PTP_HWTSTAMP_TX_ON;
+ break;
+
+ case HWTSTAMP_TX_OFF:
+ edev->flags &= ~QEDE_TX_TIMESTAMPING_EN;
+ tx_type = QED_PTP_HWTSTAMP_TX_OFF;
break;
case HWTSTAMP_TX_ONESTEP_SYNC:
@@ -248,42 +255,57 @@ static int qede_ptp_cfg_filters(struct qede_dev *edev)
spin_lock_bh(&ptp->lock);
switch (ptp->rx_filter) {
case HWTSTAMP_FILTER_NONE:
+ rx_filter = QED_PTP_FILTER_NONE;
break;
case HWTSTAMP_FILTER_ALL:
case HWTSTAMP_FILTER_SOME:
ptp->rx_filter = HWTSTAMP_FILTER_NONE;
+ rx_filter = QED_PTP_FILTER_ALL;
break;
case HWTSTAMP_FILTER_PTP_V1_L4_EVENT:
+ ptp->rx_filter = HWTSTAMP_FILTER_PTP_V1_L4_EVENT;
+ rx_filter = QED_PTP_FILTER_V1_L4_EVENT;
+ break;
case HWTSTAMP_FILTER_PTP_V1_L4_SYNC:
case HWTSTAMP_FILTER_PTP_V1_L4_DELAY_REQ:
ptp->rx_filter = HWTSTAMP_FILTER_PTP_V1_L4_EVENT;
/* Initialize PTP detection for UDP/IPv4 events */
- ptp->ops->cfg_rx_filters(edev->cdev, QED_PTP_FILTER_IPV4);
+ rx_filter = QED_PTP_FILTER_V1_L4_GEN;
break;
case HWTSTAMP_FILTER_PTP_V2_L4_EVENT:
+ ptp->rx_filter = HWTSTAMP_FILTER_PTP_V2_L4_EVENT;
+ rx_filter = QED_PTP_FILTER_V2_L4_EVENT;
+ break;
case HWTSTAMP_FILTER_PTP_V2_L4_SYNC:
case HWTSTAMP_FILTER_PTP_V2_L4_DELAY_REQ:
ptp->rx_filter = HWTSTAMP_FILTER_PTP_V2_L4_EVENT;
/* Initialize PTP detection for UDP/IPv4 or UDP/IPv6 events */
- ptp->ops->cfg_rx_filters(edev->cdev, QED_PTP_FILTER_IPV4_IPV6);
+ rx_filter = QED_PTP_FILTER_V2_L4_GEN;
break;
case HWTSTAMP_FILTER_PTP_V2_L2_EVENT:
+ ptp->rx_filter = HWTSTAMP_FILTER_PTP_V2_L2_EVENT;
+ rx_filter = QED_PTP_FILTER_V2_L2_EVENT;
+ break;
case HWTSTAMP_FILTER_PTP_V2_L2_SYNC:
case HWTSTAMP_FILTER_PTP_V2_L2_DELAY_REQ:
ptp->rx_filter = HWTSTAMP_FILTER_PTP_V2_L2_EVENT;
/* Initialize PTP detection L2 events */
- ptp->ops->cfg_rx_filters(edev->cdev, QED_PTP_FILTER_L2);
+ rx_filter = QED_PTP_FILTER_V2_L2_GEN;
break;
case HWTSTAMP_FILTER_PTP_V2_EVENT:
+ ptp->rx_filter = HWTSTAMP_FILTER_PTP_V2_EVENT;
+ rx_filter = QED_PTP_FILTER_V2_EVENT;
+ break;
case HWTSTAMP_FILTER_PTP_V2_SYNC:
case HWTSTAMP_FILTER_PTP_V2_DELAY_REQ:
ptp->rx_filter = HWTSTAMP_FILTER_PTP_V2_EVENT;
/* Initialize PTP detection L2, UDP/IPv4 or UDP/IPv6 events */
- ptp->ops->cfg_rx_filters(edev->cdev,
- QED_PTP_FILTER_L2_IPV4_IPV6);
+ rx_filter = QED_PTP_FILTER_V2_GEN;
break;
}
+ ptp->ops->cfg_filters(edev->cdev, rx_filter, tx_type);
+
spin_unlock_bh(&ptp->lock);
return 0;
diff --git a/include/linux/qed/qed_eth_if.h b/include/linux/qed/qed_eth_if.h
index 4cd1f0c..65f6271 100644
--- a/include/linux/qed/qed_eth_if.h
+++ b/include/linux/qed/qed_eth_if.h
@@ -163,10 +163,21 @@ struct qed_eth_cb_ops {
#define QED_MAX_PHC_DRIFT_PPB 291666666
enum qed_ptp_filter_type {
- QED_PTP_FILTER_L2,
- QED_PTP_FILTER_IPV4,
- QED_PTP_FILTER_IPV4_IPV6,
- QED_PTP_FILTER_L2_IPV4_IPV6
+ QED_PTP_FILTER_NONE,
+ QED_PTP_FILTER_ALL,
+ QED_PTP_FILTER_V1_L4_EVENT,
+ QED_PTP_FILTER_V1_L4_GEN,
+ QED_PTP_FILTER_V2_L4_EVENT,
+ QED_PTP_FILTER_V2_L4_GEN,
+ QED_PTP_FILTER_V2_L2_EVENT,
+ QED_PTP_FILTER_V2_L2_GEN,
+ QED_PTP_FILTER_V2_EVENT,
+ QED_PTP_FILTER_V2_GEN
+};
+
+enum qed_ptp_hwtstamp_tx_type {
+ QED_PTP_HWTSTAMP_TX_OFF,
+ QED_PTP_HWTSTAMP_TX_ON,
};
#ifdef CONFIG_DCB
@@ -229,8 +240,8 @@ struct qed_eth_dcbnl_ops {
#endif
struct qed_eth_ptp_ops {
- int (*hwtstamp_tx_on)(struct qed_dev *);
- int (*cfg_rx_filters)(struct qed_dev *, enum qed_ptp_filter_type);
+ int (*cfg_filters)(struct qed_dev *, enum qed_ptp_filter_type,
+ enum qed_ptp_hwtstamp_tx_type);
int (*read_rx_ts)(struct qed_dev *, u64 *);
int (*read_tx_ts)(struct qed_dev *, u64 *);
int (*read_cc)(struct qed_dev *, u64 *);
--
1.8.3.1
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox