* Re: [PATCH] net: phy: Fix truncation of large IRQ numbers in phy_attached_print()
From: Andrew Lunn @ 2017-09-21 15:40 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Florian Fainelli, David S . Miller, Romain Perier, netdev,
linux-kernel
In-Reply-To: <1505993222-16721-1-git-send-email-geert+renesas@glider.be>
On Thu, Sep 21, 2017 at 01:27:02PM +0200, Geert Uytterhoeven wrote:
> Given NR_IRQS is 2048 on sparc64, and even 32784 on alpha, 3 digits is
> not enough to represent interrupt numbers on all architectures. Hence
> PHY interrupt numbers may be truncated during printing.
>
> Increase the buffer size from 4 to 8 bytes to fix this.
>
> Fixes: 5e369aefdce4818c ("net: stmmac: Delete dead code for MDIO registration")
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Andrew
^ permalink raw reply
* Re: [net-next v3] bridge: trigger RTM_NEWLINK when interface is modified by bridge ioctl
From: Vincent Bernat @ 2017-09-21 15:45 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: David Ahern, David Miller, bridge, netdev
In-Reply-To: <20170921081542.4c266c83@xeon-e3>
❦ 21 septembre 2017 08:15 -0700, Stephen Hemminger <stephen@networkplumber.org> :
>> Currently, there is a difference in netlink events received when an
>> interface is modified through bridge ioctl() or through netlink. This
>> patch generates additional events when an interface is added to or
>> removed from a bridge via ioctl().
>>
>> When adding then removing an interface from a bridge with netlink, we
>> get:
>>
>> 5: dummy1: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 qdisc noqueue master bridge0 state UNKNOWN group default
>> link/ether 9e:da:60:ee:cf:c8 brd ff:ff:ff:ff:ff:ff
>> 5: dummy1: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 master bridge0 state UNKNOWN
>> link/ether 9e:da:60:ee:cf:c8
>> 5: dummy1: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 master bridge0 state UNKNOWN
>> link/ether 9e:da:60:ee:cf:c8
>> 5: dummy1: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 master bridge0 state UNKNOWN
>> link/ether 9e:da:60:ee:cf:c8
>> 5: dummy1: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 master bridge0 state UNKNOWN
>> link/ether 9e:da:60:ee:cf:c8
>> 5: dummy1: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 qdisc noqueue master bridge0 state UNKNOWN group default
>> link/ether 9e:da:60:ee:cf:c8 brd ff:ff:ff:ff:ff:ff
>>
>> 5: dummy1: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 qdisc noqueue master bridge0 state UNKNOWN group default
>> link/ether 9e:da:60:ee:cf:c8 brd ff:ff:ff:ff:ff:ff
>> 5: dummy1: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 master bridge0 state UNKNOWN
>> link/ether 9e:da:60:ee:cf:c8
>> Deleted 5: dummy1: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 master bridge0 state UNKNOWN
>> link/ether 9e:da:60:ee:cf:c8
>> 5: dummy1: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 qdisc noqueue state UNKNOWN group default
>> link/ether 9e:da:60:ee:cf:c8 brd ff:ff:ff:ff:ff:ff
>>
>> When using ioctl():
>>
>> 5: dummy1: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 qdisc noqueue master bridge0 state UNKNOWN group default
>> link/ether 9e:da:60:ee:cf:c8 brd ff:ff:ff:ff:ff:ff
>> 5: dummy1: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 master bridge0 state UNKNOWN
>> link/ether 9e:da:60:ee:cf:c8
>> 5: dummy1: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 master bridge0 state UNKNOWN
>> link/ether 9e:da:60:ee:cf:c8
>> 5: dummy1: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 master bridge0 state UNKNOWN
>> link/ether 9e:da:60:ee:cf:c8
>> 5: dummy1: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 qdisc noqueue master bridge0 state UNKNOWN group default
>> link/ether 9e:da:60:ee:cf:c8 brd ff:ff:ff:ff:ff:ff
>>
>> 5: dummy1: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 qdisc noqueue master bridge0 state UNKNOWN group default
>> link/ether 9e:da:60:ee:cf:c8 brd ff:ff:ff:ff:ff:ff
>> 5: dummy1: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 master bridge0 state UNKNOWN
>> link/ether 9e:da:60:ee:cf:c8
>> Deleted 5: dummy1: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 master bridge0 state UNKNOWN
>> link/ether 9e:da:60:ee:cf:c8
>> 5: dummy1: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 qdisc noqueue state UNKNOWN group default
>> link/ether 9e:da:60:ee:cf:c8 brd ff:ff:ff:ff:ff:ff
>>
>> Without this patch, the last netlink notification is not sent.
>>
>> Signed-off-by: Vincent Bernat <vincent@bernat.im>
>
> This makes sense, you should probably add a Fixes: tag to help maintainers
> of long term stable kernels.
>
> Reviewed-by: Stephen Hemminger <stephen@networkplumber.org>
I wouldn't know which commit would be fixed since this is not a
regression, just a behavior difference.
--
Make sure special cases are truly special.
- The Elements of Programming Style (Kernighan & Plauger)
^ permalink raw reply
* Re: [PATCH net-next] bpf/verifier: improve disassembly of BPF_END instructions
From: Alexei Starovoitov @ 2017-09-21 15:52 UTC (permalink / raw)
To: Edward Cree; +Cc: David Miller, netdev, Daniel Borkmann
In-Reply-To: <7013ee9d-a8e6-13fd-cc5f-86cf3d8bf4e0@solarflare.com>
On Thu, Sep 21, 2017 at 04:09:34PM +0100, Edward Cree wrote:
> print_bpf_insn() was treating all BPF_ALU[64] the same, but BPF_END has a
> different structure: it has a size in insn->imm (even if it's BPF_X) and
> uses the BPF_SRC (X or K) to indicate which endianness to use. So it
> needs different code to print it.
>
> Signed-off-by: Edward Cree <ecree@solarflare.com>
> ---
> It's not the same format as the new LLVM asm uses, does that matter?
> AFAIK the LLVM format doesn't comprehend BPF_TO_LE, just assumes that all
> endian ops are necessarily swaps (rather than sometimes nops).
that is being fixed and we will fix asm format too.
Let's pick good format first.
> kernel/bpf/verifier.c | 13 +++++++++++--
> 1 file changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index 799b245..e7657a4 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -331,20 +331,29 @@ static void print_bpf_insn(const struct bpf_verifier_env *env,
> u8 class = BPF_CLASS(insn->code);
>
> if (class == BPF_ALU || class == BPF_ALU64) {
> - if (BPF_SRC(insn->code) == BPF_X)
> + if (BPF_OP(insn->code) == BPF_END) {
> + if (class == BPF_ALU64)
> + verbose("BUG_alu64_%02x\n", insn->code);
> + else
> + verbose("(%02x) (u%d) r%d %s %s\n",
> + insn->code, insn->imm, insn->dst_reg,
> + bpf_alu_string[BPF_END >> 4],
> + BPF_SRC(insn->code) == BPF_X ? "be" : "le");
yes the bit the same, but please use BPF_SRC(insn->code) == BPF_TO_BE.
imo
(u16) r4 endian be
isn't intuitive.
Can we come up with some better syntax?
Like
bswap16be r4
bswap32le r4
or
to_be16 r4
to_le32 r4
It will be more obvious what's happening.
^ permalink raw reply
* Re: [kernel-hardening] Re: [PATCH v3 03/31] usercopy: Mark kmalloc caches as usercopy caches
From: Christopher Lameter @ 2017-09-21 16:04 UTC (permalink / raw)
To: Kees Cook
Cc: LKML, David Windsor, Pekka Enberg, David Rientjes, Joonsoo Kim,
Andrew Morton, Linux-MM, linux-xfs, linux-fsdevel@vger.kernel.org,
Network Development, kernel-hardening@lists.openwall.com
In-Reply-To: <CAGXu5j+X6dWCGocG=P7pszTY-5OZ6Jmp-RsnDKox75M5rmVe4g@mail.gmail.com>
On Thu, 21 Sep 2017, Kees Cook wrote:
> > So what is the point of this patch?
>
> The DMA kmalloc caches are not whitelisted:
The DMA kmalloc caches are pretty obsolete and mostly there for obscure
drivers.
??
> >> kmalloc_dma_caches[i] = create_kmalloc_cache(n,
> >> - size, SLAB_CACHE_DMA | flags);
> >> + size, SLAB_CACHE_DMA | flags, 0, 0);
>
> So this is creating the distinction between the kmallocs that go to
> userspace and those that don't. The expectation is that future work
> can start to distinguish between "for userspace" and "only kernel"
> kmalloc allocations, as is already done here for DMA.
The creation of the kmalloc caches in earlier patches already setup the
"whitelisting". Why do it twice?
^ permalink raw reply
* [PATCH net] net: prevent dst uses after free
From: Eric Dumazet @ 2017-09-21 16:15 UTC (permalink / raw)
To: Paweł Staszewski
Cc: Wei Wang, Cong Wang, Linux Kernel Network Developers,
Eric Dumazet
In-Reply-To: <1506005799.29839.130.camel@edumazet-glaptop3.roam.corp.google.com>
From: Eric Dumazet <edumazet@google.com>
In linux-4.13, Wei worked hard to convert dst to a traditional
refcounted model, removing GC.
We now want to make sure a dst refcount can not transition from 0 back
to 1.
The problem here is that input path attached a not refcounted dst to an
skb. Then later, because packet is forwarded and hits skb_dst_force()
before exiting RCU section, we might try to take a refcount on one dst
that is about to be freed, if another cpu saw 1 -> 0 transition in
dst_release() and queued the dst for freeing after one RCU grace period.
Lets unify skb_dst_force() and skb_dst_force_safe(), since we should
always perform the complete check against dst refcount, and not assume
it is not zero.
Bugzilla : https://bugzilla.kernel.org/show_bug.cgi?id=197005
[ 989.919496] skb_dst_force+0x32/0x34
[ 989.919498] __dev_queue_xmit+0x1ad/0x482
[ 989.919501] ? eth_header+0x28/0xc6
[ 989.919502] dev_queue_xmit+0xb/0xd
[ 989.919504] neigh_connected_output+0x9b/0xb4
[ 989.919507] ip_finish_output2+0x234/0x294
[ 989.919509] ? ipt_do_table+0x369/0x388
[ 989.919510] ip_finish_output+0x12c/0x13f
[ 989.919512] ip_output+0x53/0x87
[ 989.919513] ip_forward_finish+0x53/0x5a
[ 989.919515] ip_forward+0x2cb/0x3e6
[ 989.919516] ? pskb_trim_rcsum.part.9+0x4b/0x4b
[ 989.919518] ip_rcv_finish+0x2e2/0x321
[ 989.919519] ip_rcv+0x26f/0x2eb
[ 989.919522] ? vlan_do_receive+0x4f/0x289
[ 989.919523] __netif_receive_skb_core+0x467/0x50b
[ 989.919526] ? tcp_gro_receive+0x239/0x239
[ 989.919529] ? inet_gro_receive+0x226/0x238
[ 989.919530] __netif_receive_skb+0x4d/0x5f
[ 989.919532] netif_receive_skb_internal+0x5c/0xaf
[ 989.919533] napi_gro_receive+0x45/0x81
[ 989.919536] ixgbe_poll+0xc8a/0xf09
[ 989.919539] ? kmem_cache_free_bulk+0x1b6/0x1f7
[ 989.919540] net_rx_action+0xf4/0x266
[ 989.919543] __do_softirq+0xa8/0x19d
[ 989.919545] irq_exit+0x5d/0x6b
[ 989.919546] do_IRQ+0x9c/0xb5
[ 989.919548] common_interrupt+0x93/0x93
[ 989.919548] </IRQ>
Similarly dst_clone() can use dst_hold() helper to have additional
debugging, as a follow up to commit 44ebe79149ff ("net: add debug
atomic_inc_not_zero() in dst_hold()")
In net-next we will convert dst atomic_t to refcount_t for peace of
mind.
Fixes: a4c2fd7f7891 ("net: remove DST_NOCACHE flag")
Signed-off-by: Eric Dumazet <edumazet@google.com>
Cc: Wei Wang <weiwan@google.com>
Reported-by: Paweł Staszewski <pstaszewski@itcare.pl>
Bisected-by: Paweł Staszewski <pstaszewski@itcare.pl>
---
include/net/dst.h | 22 ++++------------------
include/net/route.h | 2 +-
include/net/sock.h | 2 +-
3 files changed, 6 insertions(+), 20 deletions(-)
diff --git a/include/net/dst.h b/include/net/dst.h
index 93568bd0a3520bb7402f04d90cf04ac99c81cfbe..06a6765da074449e6f1fe42ee05e711e898ad372 100644
--- a/include/net/dst.h
+++ b/include/net/dst.h
@@ -271,7 +271,7 @@ static inline void dst_use_noref(struct dst_entry *dst, unsigned long time)
static inline struct dst_entry *dst_clone(struct dst_entry *dst)
{
if (dst)
- atomic_inc(&dst->__refcnt);
+ dst_hold(dst);
return dst;
}
@@ -311,21 +311,6 @@ static inline void skb_dst_copy(struct sk_buff *nskb, const struct sk_buff *oskb
__skb_dst_copy(nskb, oskb->_skb_refdst);
}
-/**
- * skb_dst_force - makes sure skb dst is refcounted
- * @skb: buffer
- *
- * If dst is not yet refcounted, let's do it
- */
-static inline void skb_dst_force(struct sk_buff *skb)
-{
- if (skb_dst_is_noref(skb)) {
- WARN_ON(!rcu_read_lock_held());
- skb->_skb_refdst &= ~SKB_DST_NOREF;
- dst_clone(skb_dst(skb));
- }
-}
-
/**
* dst_hold_safe - Take a reference on a dst if possible
* @dst: pointer to dst entry
@@ -339,16 +324,17 @@ static inline bool dst_hold_safe(struct dst_entry *dst)
}
/**
- * skb_dst_force_safe - makes sure skb dst is refcounted
+ * skb_dst_force - makes sure skb dst is refcounted
* @skb: buffer
*
* If dst is not yet refcounted and not destroyed, grab a ref on it.
*/
-static inline void skb_dst_force_safe(struct sk_buff *skb)
+static inline void skb_dst_force(struct sk_buff *skb)
{
if (skb_dst_is_noref(skb)) {
struct dst_entry *dst = skb_dst(skb);
+ WARN_ON(!rcu_read_lock_held());
if (!dst_hold_safe(dst))
dst = NULL;
diff --git a/include/net/route.h b/include/net/route.h
index 1b09a9368c68d46f0c5ee8ce3cefe566000c1ec1..57dfc6850d378e4b96f13b140eef554d66c24cdf 100644
--- a/include/net/route.h
+++ b/include/net/route.h
@@ -190,7 +190,7 @@ static inline int ip_route_input(struct sk_buff *skb, __be32 dst, __be32 src,
rcu_read_lock();
err = ip_route_input_noref(skb, dst, src, tos, devin);
if (!err) {
- skb_dst_force_safe(skb);
+ skb_dst_force(skb);
if (!skb_dst(skb))
err = -EINVAL;
}
diff --git a/include/net/sock.h b/include/net/sock.h
index 03a362568357acc7278a318423dd3873103f90ca..a6b9a8d1a6df3f72df8f1aac0f577257fa6452d0 100644
--- a/include/net/sock.h
+++ b/include/net/sock.h
@@ -856,7 +856,7 @@ void sk_stream_write_space(struct sock *sk);
static inline void __sk_add_backlog(struct sock *sk, struct sk_buff *skb)
{
/* dont let skb dst not refcounted, we are going to leave rcu lock */
- skb_dst_force_safe(skb);
+ skb_dst_force(skb);
if (!sk->sk_backlog.tail)
sk->sk_backlog.head = skb;
^ permalink raw reply related
* Re: [PATCH net-next] bpf/verifier: improve disassembly of BPF_END instructions
From: Edward Cree @ 2017-09-21 16:24 UTC (permalink / raw)
To: Alexei Starovoitov; +Cc: David Miller, netdev, Daniel Borkmann
In-Reply-To: <20170921155215.jta52sesbiq54vri@ast-mbp>
On 21/09/17 16:52, Alexei Starovoitov wrote:
> On Thu, Sep 21, 2017 at 04:09:34PM +0100, Edward Cree wrote:
>> print_bpf_insn() was treating all BPF_ALU[64] the same, but BPF_END has a
>> different structure: it has a size in insn->imm (even if it's BPF_X) and
>> uses the BPF_SRC (X or K) to indicate which endianness to use. So it
>> needs different code to print it.
>>
>> Signed-off-by: Edward Cree <ecree@solarflare.com>
>> ---
>> It's not the same format as the new LLVM asm uses, does that matter?
>> AFAIK the LLVM format doesn't comprehend BPF_TO_LE, just assumes that all
>> endian ops are necessarily swaps (rather than sometimes nops).
> that is being fixed and we will fix asm format too.
> Let's pick good format first.
Agreed.
>> kernel/bpf/verifier.c | 13 +++++++++++--
>> 1 file changed, 11 insertions(+), 2 deletions(-)
>>
>> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
>> index 799b245..e7657a4 100644
>> --- a/kernel/bpf/verifier.c
>> +++ b/kernel/bpf/verifier.c
>> @@ -331,20 +331,29 @@ static void print_bpf_insn(const struct bpf_verifier_env *env,
>> u8 class = BPF_CLASS(insn->code);
>>
>> if (class == BPF_ALU || class == BPF_ALU64) {
>> - if (BPF_SRC(insn->code) == BPF_X)
>> + if (BPF_OP(insn->code) == BPF_END) {
>> + if (class == BPF_ALU64)
>> + verbose("BUG_alu64_%02x\n", insn->code);
>> + else
>> + verbose("(%02x) (u%d) r%d %s %s\n",
>> + insn->code, insn->imm, insn->dst_reg,
>> + bpf_alu_string[BPF_END >> 4],
>> + BPF_SRC(insn->code) == BPF_X ? "be" : "le");
> yes the bit the same, but please use BPF_SRC(insn->code) == BPF_TO_BE.
Good point.
> imo
> (u16) r4 endian be
> isn't intuitive.
> Can we come up with some better syntax?
> Like
> bswap16be r4
> bswap32le r4
Hmm, I don't like these, since bswapbe is a swap on *le* and a nop on be.
> or
>
> to_be16 r4
> to_le32 r4
And the problem here is that it's not just to_be, it's also from_be.
Otherwise we could write `(be16) r4 = endian (u16) r4` and be much more
explicit about what's happening.
Really the operation is something like `cpu_tofrom_be16 r4`, but that also
seems a bit clumsy and longwinded. Also it's inconsistent with the other
ops that all indicate sizes with these (u16) etc casts.
`endian (be16) r4`, perhaps?
^ permalink raw reply
* Re: [PATCH net-next] bpf/verifier: improve disassembly of BPF_END instructions
From: Y Song @ 2017-09-21 16:30 UTC (permalink / raw)
To: Alexei Starovoitov; +Cc: Edward Cree, David Miller, netdev, Daniel Borkmann
In-Reply-To: <20170921155215.jta52sesbiq54vri@ast-mbp>
On Thu, Sep 21, 2017 at 8:52 AM, Alexei Starovoitov
<alexei.starovoitov@gmail.com> wrote:
> On Thu, Sep 21, 2017 at 04:09:34PM +0100, Edward Cree wrote:
>> print_bpf_insn() was treating all BPF_ALU[64] the same, but BPF_END has a
>> different structure: it has a size in insn->imm (even if it's BPF_X) and
>> uses the BPF_SRC (X or K) to indicate which endianness to use. So it
>> needs different code to print it.
>>
>> Signed-off-by: Edward Cree <ecree@solarflare.com>
>> ---
>> It's not the same format as the new LLVM asm uses, does that matter?
>> AFAIK the LLVM format doesn't comprehend BPF_TO_LE, just assumes that all
>> endian ops are necessarily swaps (rather than sometimes nops).
>
> that is being fixed and we will fix asm format too.
> Let's pick good format first.
>
>> kernel/bpf/verifier.c | 13 +++++++++++--
>> 1 file changed, 11 insertions(+), 2 deletions(-)
>>
>> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
>> index 799b245..e7657a4 100644
>> --- a/kernel/bpf/verifier.c
>> +++ b/kernel/bpf/verifier.c
>> @@ -331,20 +331,29 @@ static void print_bpf_insn(const struct bpf_verifier_env *env,
>> u8 class = BPF_CLASS(insn->code);
>>
>> if (class == BPF_ALU || class == BPF_ALU64) {
>> - if (BPF_SRC(insn->code) == BPF_X)
>> + if (BPF_OP(insn->code) == BPF_END) {
>> + if (class == BPF_ALU64)
>> + verbose("BUG_alu64_%02x\n", insn->code);
>> + else
>> + verbose("(%02x) (u%d) r%d %s %s\n",
>> + insn->code, insn->imm, insn->dst_reg,
>> + bpf_alu_string[BPF_END >> 4],
>> + BPF_SRC(insn->code) == BPF_X ? "be" : "le");
>
> yes the bit the same, but please use BPF_SRC(insn->code) == BPF_TO_BE.
> imo
> (u16) r4 endian be
> isn't intuitive.
> Can we come up with some better syntax?
> Like
> bswap16be r4
> bswap32le r4
>
> or
>
> to_be16 r4
> to_le32 r4
Currently, llvm bpf backend uses "bswap16 r4" "bswap32 r2" "bswap64 r2" syntax.
I prefer something like "to_be16 r4" "to_le32 r4", or "bswap2be16"
"bswap2le32" or something similar.
This captures what the operation really is.
Right the support to bswap2le will be added to LLVM soon.
>
> It will be more obvious what's happening.
>
^ permalink raw reply
* Re: [PATCH net-next] bpf/verifier: improve disassembly of BPF_END instructions
From: Alexei Starovoitov @ 2017-09-21 16:40 UTC (permalink / raw)
To: Edward Cree; +Cc: David Miller, netdev, Daniel Borkmann
In-Reply-To: <4cfac985-4f99-cf85-fc15-c3ad1f8ff123@solarflare.com>
On Thu, Sep 21, 2017 at 05:24:10PM +0100, Edward Cree wrote:
> On 21/09/17 16:52, Alexei Starovoitov wrote:
> > On Thu, Sep 21, 2017 at 04:09:34PM +0100, Edward Cree wrote:
> >> print_bpf_insn() was treating all BPF_ALU[64] the same, but BPF_END has a
> >> different structure: it has a size in insn->imm (even if it's BPF_X) and
> >> uses the BPF_SRC (X or K) to indicate which endianness to use. So it
> >> needs different code to print it.
> >>
> >> Signed-off-by: Edward Cree <ecree@solarflare.com>
> >> ---
> >> It's not the same format as the new LLVM asm uses, does that matter?
> >> AFAIK the LLVM format doesn't comprehend BPF_TO_LE, just assumes that all
> >> endian ops are necessarily swaps (rather than sometimes nops).
> > that is being fixed and we will fix asm format too.
> > Let's pick good format first.
> Agreed.
> >> kernel/bpf/verifier.c | 13 +++++++++++--
> >> 1 file changed, 11 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> >> index 799b245..e7657a4 100644
> >> --- a/kernel/bpf/verifier.c
> >> +++ b/kernel/bpf/verifier.c
> >> @@ -331,20 +331,29 @@ static void print_bpf_insn(const struct bpf_verifier_env *env,
> >> u8 class = BPF_CLASS(insn->code);
> >>
> >> if (class == BPF_ALU || class == BPF_ALU64) {
> >> - if (BPF_SRC(insn->code) == BPF_X)
> >> + if (BPF_OP(insn->code) == BPF_END) {
> >> + if (class == BPF_ALU64)
> >> + verbose("BUG_alu64_%02x\n", insn->code);
> >> + else
> >> + verbose("(%02x) (u%d) r%d %s %s\n",
> >> + insn->code, insn->imm, insn->dst_reg,
> >> + bpf_alu_string[BPF_END >> 4],
> >> + BPF_SRC(insn->code) == BPF_X ? "be" : "le");
> > yes the bit the same, but please use BPF_SRC(insn->code) == BPF_TO_BE.
> Good point.
> > imo
> > (u16) r4 endian be
> > isn't intuitive.
> > Can we come up with some better syntax?
> > Like
> > bswap16be r4
> > bswap32le r4
> Hmm, I don't like these, since bswapbe is a swap on *le* and a nop on be.
> > or
> >
> > to_be16 r4
> > to_le32 r4
> And the problem here is that it's not just to_be, it's also from_be.
> Otherwise we could write `(be16) r4 = endian (u16) r4` and be much more
> explicit about what's happening.
> Really the operation is something like `cpu_tofrom_be16 r4`, but that also
> seems a bit clumsy and longwinded. Also it's inconsistent with the other
> ops that all indicate sizes with these (u16) etc casts.
> `endian (be16) r4`, perhaps?
How about:
r4 = (be16) (u16) r4
r4 = (le64) (u64) r4
most C like and most explicit ?
it should be easy to grasp that (be16) cast on big-endian arch is a nop and
swap on little.
^ permalink raw reply
* Re: [PATCH net-next] bpf/verifier: improve disassembly of BPF_END instructions
From: Y Song @ 2017-09-21 16:40 UTC (permalink / raw)
To: Edward Cree; +Cc: Alexei Starovoitov, David Miller, netdev, Daniel Borkmann
In-Reply-To: <4cfac985-4f99-cf85-fc15-c3ad1f8ff123@solarflare.com>
On Thu, Sep 21, 2017 at 9:24 AM, Edward Cree <ecree@solarflare.com> wrote:
> On 21/09/17 16:52, Alexei Starovoitov wrote:
>> On Thu, Sep 21, 2017 at 04:09:34PM +0100, Edward Cree wrote:
>>> print_bpf_insn() was treating all BPF_ALU[64] the same, but BPF_END has a
>>> different structure: it has a size in insn->imm (even if it's BPF_X) and
>>> uses the BPF_SRC (X or K) to indicate which endianness to use. So it
>>> needs different code to print it.
>>>
>>> Signed-off-by: Edward Cree <ecree@solarflare.com>
>>> ---
>>> It's not the same format as the new LLVM asm uses, does that matter?
>>> AFAIK the LLVM format doesn't comprehend BPF_TO_LE, just assumes that all
>>> endian ops are necessarily swaps (rather than sometimes nops).
>> that is being fixed and we will fix asm format too.
>> Let's pick good format first.
> Agreed.
>>> kernel/bpf/verifier.c | 13 +++++++++++--
>>> 1 file changed, 11 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
>>> index 799b245..e7657a4 100644
>>> --- a/kernel/bpf/verifier.c
>>> +++ b/kernel/bpf/verifier.c
>>> @@ -331,20 +331,29 @@ static void print_bpf_insn(const struct bpf_verifier_env *env,
>>> u8 class = BPF_CLASS(insn->code);
>>>
>>> if (class == BPF_ALU || class == BPF_ALU64) {
>>> - if (BPF_SRC(insn->code) == BPF_X)
>>> + if (BPF_OP(insn->code) == BPF_END) {
>>> + if (class == BPF_ALU64)
>>> + verbose("BUG_alu64_%02x\n", insn->code);
>>> + else
>>> + verbose("(%02x) (u%d) r%d %s %s\n",
>>> + insn->code, insn->imm, insn->dst_reg,
>>> + bpf_alu_string[BPF_END >> 4],
>>> + BPF_SRC(insn->code) == BPF_X ? "be" : "le");
>> yes the bit the same, but please use BPF_SRC(insn->code) == BPF_TO_BE.
> Good point.
>> imo
>> (u16) r4 endian be
>> isn't intuitive.
>> Can we come up with some better syntax?
>> Like
>> bswap16be r4
>> bswap32le r4
> Hmm, I don't like these, since bswapbe is a swap on *le* and a nop on be.
>> or
>>
>> to_be16 r4
>> to_le32 r4
> And the problem here is that it's not just to_be, it's also from_be.
Could you explain what is "from_be" here? Do not quite understand.
> Otherwise we could write `(be16) r4 = endian (u16) r4` and be much more
> explicit about what's happening.
> Really the operation is something like `cpu_tofrom_be16 r4`, but that also
> seems a bit clumsy and longwinded. Also it's inconsistent with the other
> ops that all indicate sizes with these (u16) etc casts.
> `endian (be16) r4`, perhaps?
^ permalink raw reply
* Re: [PATCH net-next 09/14] gtp: Allow configuring GTP interface as standalone
From: Tom Herbert @ 2017-09-21 16:43 UTC (permalink / raw)
To: Harald Welte
Cc: Andreas Schultz, Tom Herbert, David S. Miller,
Linux Kernel Network Developers, Pablo Neira Ayuso, Rohit Seth
In-Reply-To: <20170921151253.4udm54dcq7x3ed4q@nataraja>
On Thu, Sep 21, 2017 at 8:12 AM, Harald Welte <laforge@netfilter.org> wrote:
> Hi Tom,
>
> On Wed, Sep 20, 2017 at 05:55:01PM -0700, Tom Herbert wrote:
>> You have the point of view of someone who has a lot of experience
>> dealing with this protocol. Try to imagine if you were some random
>> kernel network programmer with no experience in the area. If they
>> happen to find a one-off bug and want to do the right thing by running
>> a test, you want to make that as easy as possible.
>
> Agreed. But we're not talking abut fixing a random bug in your patch
> series, but we're talking about adding significant new features - and
> those features need to be tested in real use caes, not just in an
> artificial test setup that holds assumptions that are not true.
>
> To improve performance, or to fix simple bugs that only affect the
> processing of the GTP-U G-PDU, a much more limited and hence
> "unrealistic" test scenario is probably sufficient/acceptable.
>
>> From that perspective, building protocol specific libraries and
>> finding the right cmd line to run is significant hoops (I can attest
>> to this).
>
> I understand your argument. But then, there is actually quite some
> tools to help you (see further below), as well as the wiki page at
> http://osmocom.org/projects/linux-kernel-gtp-u/wiki/Basic_Testing
>
> Of course, existing tools and existing wiki pages also only document
> existing features of the kernel code :)
>
> Yes, the documentation could be better. But then, how much more can you
> expect from somebody who's doing this mostly for fun and who - despite
> working in his dayjob on FOSS cellular projects - has no single
> commercial project/context that uses the kernel GTP code.
>
> In any case, working on a specific protocol or technology will require
> that you understand that technology to some extent, including the
> available tools. There's always a learning curve involved.
>
>> There are other examples in the kernel of systems bigger than GTP that
>> require a whole lot of effort just to run a simple test; you'll notice
>> for those it's rare that best developers ever bother to look at them
>> unless they're making a global change that affects the code. We don't
>> want GTP to take be like that!
>
> I'm all for following your argument. My point is simply: You cannot
> develop code solely based on mock-ups without any 'realistic' test
> scenarios. Otherwise you will end up with something that works only in
> your artificial lab setup, and follows all the best practises of the way
> how the Linux kernel traditionally approaches tunneling implementations,
> but it will never work/interop in the real world.
>
> And I'm very strongly opposed to merging code where we have not been
> able to show that it will inter-operate in at least one realistic
> scenario. This would raise wrong expectations with users and all sorts
> of downstream problems.
>
Harald,
Please see the cover letter for the original GTP kernel patches dated
May 10, 2016. My first question on those was "Is there a timeline for
adding IPv6 support?". To which Pablo replied that there was a
preliminary patch for it that has not been released. That was almost a
year and half ago and we have not heard anything since. If you don't
like my patches or don't think that can be adapted to fully support
the GTP specification, that's fine. But then you need to provide a
viable alternative. We are at the point where a kernel networking
feature that only supports IPv4 when it could support IPv6 must be
considered incomplete.
Thanks,
Tom
^ permalink raw reply
* Re: [net-next v3] bridge: trigger RTM_NEWLINK when interface is modified by bridge ioctl
From: David Ahern @ 2017-09-21 16:43 UTC (permalink / raw)
To: Vincent Bernat, Stephen Hemminger, David Miller, bridge, netdev
In-Reply-To: <20170921100525.20395-1-vincent@bernat.im>
On 9/21/17 4:05 AM, Vincent Bernat wrote:
> Currently, there is a difference in netlink events received when an
> interface is modified through bridge ioctl() or through netlink. This
> patch generates additional events when an interface is added to or
> removed from a bridge via ioctl().
>
> When adding then removing an interface from a bridge with netlink, we
> get:
>
> 5: dummy1: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 qdisc noqueue master bridge0 state UNKNOWN group default
> link/ether 9e:da:60:ee:cf:c8 brd ff:ff:ff:ff:ff:ff
> 5: dummy1: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 master bridge0 state UNKNOWN
> link/ether 9e:da:60:ee:cf:c8
> 5: dummy1: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 master bridge0 state UNKNOWN
> link/ether 9e:da:60:ee:cf:c8
> 5: dummy1: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 master bridge0 state UNKNOWN
> link/ether 9e:da:60:ee:cf:c8
> 5: dummy1: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 master bridge0 state UNKNOWN
> link/ether 9e:da:60:ee:cf:c8
> 5: dummy1: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 qdisc noqueue master bridge0 state UNKNOWN group default
> link/ether 9e:da:60:ee:cf:c8 brd ff:ff:ff:ff:ff:ff
>
> 5: dummy1: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 qdisc noqueue master bridge0 state UNKNOWN group default
> link/ether 9e:da:60:ee:cf:c8 brd ff:ff:ff:ff:ff:ff
> 5: dummy1: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 master bridge0 state UNKNOWN
> link/ether 9e:da:60:ee:cf:c8
> Deleted 5: dummy1: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 master bridge0 state UNKNOWN
> link/ether 9e:da:60:ee:cf:c8
> 5: dummy1: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 qdisc noqueue state UNKNOWN group default
> link/ether 9e:da:60:ee:cf:c8 brd ff:ff:ff:ff:ff:ff
>
> When using ioctl():
>
> 5: dummy1: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 qdisc noqueue master bridge0 state UNKNOWN group default
> link/ether 9e:da:60:ee:cf:c8 brd ff:ff:ff:ff:ff:ff
> 5: dummy1: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 master bridge0 state UNKNOWN
> link/ether 9e:da:60:ee:cf:c8
> 5: dummy1: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 master bridge0 state UNKNOWN
> link/ether 9e:da:60:ee:cf:c8
> 5: dummy1: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 master bridge0 state UNKNOWN
> link/ether 9e:da:60:ee:cf:c8
> 5: dummy1: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 qdisc noqueue master bridge0 state UNKNOWN group default
> link/ether 9e:da:60:ee:cf:c8 brd ff:ff:ff:ff:ff:ff
>
> 5: dummy1: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 qdisc noqueue master bridge0 state UNKNOWN group default
> link/ether 9e:da:60:ee:cf:c8 brd ff:ff:ff:ff:ff:ff
> 5: dummy1: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 master bridge0 state UNKNOWN
> link/ether 9e:da:60:ee:cf:c8
> Deleted 5: dummy1: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 master bridge0 state UNKNOWN
> link/ether 9e:da:60:ee:cf:c8
> 5: dummy1: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 qdisc noqueue state UNKNOWN group default
> link/ether 9e:da:60:ee:cf:c8 brd ff:ff:ff:ff:ff:ff
>
> Without this patch, the last netlink notification is not sent.
>
> Signed-off-by: Vincent Bernat <vincent@bernat.im>
> ---
> net/bridge/br_ioctl.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/net/bridge/br_ioctl.c b/net/bridge/br_ioctl.c
> index 7970f8540cbb..66cd98772051 100644
> --- a/net/bridge/br_ioctl.c
> +++ b/net/bridge/br_ioctl.c
> @@ -102,6 +102,9 @@ static int add_del_if(struct net_bridge *br, int ifindex, int isadd)
> else
> ret = br_del_if(br, dev);
>
> + if (!ret)
> + rtmsg_ifinfo(RTM_NEWLINK, dev, IFF_MASTER, GFP_KERNEL);
> +
> return ret;
> }
>
>
Agreed that this is needed for userspace to know about the master change
when done through ioctl. The bridge code is emitting a lot of what
appears to be redundant messages for both paths (netlink and ioctl).
Reviewed-by: David Ahern <dsahern@gmail.com>
^ permalink raw reply
* Re: [PATCH net] net: prevent dst uses after free
From: Wei Wang @ 2017-09-21 16:49 UTC (permalink / raw)
To: Eric Dumazet
Cc: Paweł Staszewski, Cong Wang, Linux Kernel Network Developers,
Eric Dumazet
In-Reply-To: <1506010546.29839.148.camel@edumazet-glaptop3.roam.corp.google.com>
On Thu, Sep 21, 2017 at 9:15 AM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> From: Eric Dumazet <edumazet@google.com>
>
> In linux-4.13, Wei worked hard to convert dst to a traditional
> refcounted model, removing GC.
>
> We now want to make sure a dst refcount can not transition from 0 back
> to 1.
>
> The problem here is that input path attached a not refcounted dst to an
> skb. Then later, because packet is forwarded and hits skb_dst_force()
> before exiting RCU section, we might try to take a refcount on one dst
> that is about to be freed, if another cpu saw 1 -> 0 transition in
> dst_release() and queued the dst for freeing after one RCU grace period.
>
> Lets unify skb_dst_force() and skb_dst_force_safe(), since we should
> always perform the complete check against dst refcount, and not assume
> it is not zero.
>
> Bugzilla : https://bugzilla.kernel.org/show_bug.cgi?id=197005
>
> [ 989.919496] skb_dst_force+0x32/0x34
> [ 989.919498] __dev_queue_xmit+0x1ad/0x482
> [ 989.919501] ? eth_header+0x28/0xc6
> [ 989.919502] dev_queue_xmit+0xb/0xd
> [ 989.919504] neigh_connected_output+0x9b/0xb4
> [ 989.919507] ip_finish_output2+0x234/0x294
> [ 989.919509] ? ipt_do_table+0x369/0x388
> [ 989.919510] ip_finish_output+0x12c/0x13f
> [ 989.919512] ip_output+0x53/0x87
> [ 989.919513] ip_forward_finish+0x53/0x5a
> [ 989.919515] ip_forward+0x2cb/0x3e6
> [ 989.919516] ? pskb_trim_rcsum.part.9+0x4b/0x4b
> [ 989.919518] ip_rcv_finish+0x2e2/0x321
> [ 989.919519] ip_rcv+0x26f/0x2eb
> [ 989.919522] ? vlan_do_receive+0x4f/0x289
> [ 989.919523] __netif_receive_skb_core+0x467/0x50b
> [ 989.919526] ? tcp_gro_receive+0x239/0x239
> [ 989.919529] ? inet_gro_receive+0x226/0x238
> [ 989.919530] __netif_receive_skb+0x4d/0x5f
> [ 989.919532] netif_receive_skb_internal+0x5c/0xaf
> [ 989.919533] napi_gro_receive+0x45/0x81
> [ 989.919536] ixgbe_poll+0xc8a/0xf09
> [ 989.919539] ? kmem_cache_free_bulk+0x1b6/0x1f7
> [ 989.919540] net_rx_action+0xf4/0x266
> [ 989.919543] __do_softirq+0xa8/0x19d
> [ 989.919545] irq_exit+0x5d/0x6b
> [ 989.919546] do_IRQ+0x9c/0xb5
> [ 989.919548] common_interrupt+0x93/0x93
> [ 989.919548] </IRQ>
>
>
> Similarly dst_clone() can use dst_hold() helper to have additional
> debugging, as a follow up to commit 44ebe79149ff ("net: add debug
> atomic_inc_not_zero() in dst_hold()")
>
> In net-next we will convert dst atomic_t to refcount_t for peace of
> mind.
>
> Fixes: a4c2fd7f7891 ("net: remove DST_NOCACHE flag")
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Cc: Wei Wang <weiwan@google.com>
> Reported-by: Paweł Staszewski <pstaszewski@itcare.pl>
> Bisected-by: Paweł Staszewski <pstaszewski@itcare.pl>
> ---
Thanks a lot for the fix Eric. It makes sense to unify all the usage
of skb_dst_force() to always check on the refcnt not being 0.
And thank you Pawel for reporting and testing on this.
Acked-by: Wei Wang <weiwan@google.com>
> include/net/dst.h | 22 ++++------------------
> include/net/route.h | 2 +-
> include/net/sock.h | 2 +-
> 3 files changed, 6 insertions(+), 20 deletions(-)
>
> diff --git a/include/net/dst.h b/include/net/dst.h
> index 93568bd0a3520bb7402f04d90cf04ac99c81cfbe..06a6765da074449e6f1fe42ee05e711e898ad372 100644
> --- a/include/net/dst.h
> +++ b/include/net/dst.h
> @@ -271,7 +271,7 @@ static inline void dst_use_noref(struct dst_entry *dst, unsigned long time)
> static inline struct dst_entry *dst_clone(struct dst_entry *dst)
> {
> if (dst)
> - atomic_inc(&dst->__refcnt);
> + dst_hold(dst);
> return dst;
> }
>
> @@ -311,21 +311,6 @@ static inline void skb_dst_copy(struct sk_buff *nskb, const struct sk_buff *oskb
> __skb_dst_copy(nskb, oskb->_skb_refdst);
> }
>
> -/**
> - * skb_dst_force - makes sure skb dst is refcounted
> - * @skb: buffer
> - *
> - * If dst is not yet refcounted, let's do it
> - */
> -static inline void skb_dst_force(struct sk_buff *skb)
> -{
> - if (skb_dst_is_noref(skb)) {
> - WARN_ON(!rcu_read_lock_held());
> - skb->_skb_refdst &= ~SKB_DST_NOREF;
> - dst_clone(skb_dst(skb));
> - }
> -}
> -
> /**
> * dst_hold_safe - Take a reference on a dst if possible
> * @dst: pointer to dst entry
> @@ -339,16 +324,17 @@ static inline bool dst_hold_safe(struct dst_entry *dst)
> }
>
> /**
> - * skb_dst_force_safe - makes sure skb dst is refcounted
> + * skb_dst_force - makes sure skb dst is refcounted
> * @skb: buffer
> *
> * If dst is not yet refcounted and not destroyed, grab a ref on it.
> */
> -static inline void skb_dst_force_safe(struct sk_buff *skb)
> +static inline void skb_dst_force(struct sk_buff *skb)
> {
> if (skb_dst_is_noref(skb)) {
> struct dst_entry *dst = skb_dst(skb);
>
> + WARN_ON(!rcu_read_lock_held());
> if (!dst_hold_safe(dst))
> dst = NULL;
>
> diff --git a/include/net/route.h b/include/net/route.h
> index 1b09a9368c68d46f0c5ee8ce3cefe566000c1ec1..57dfc6850d378e4b96f13b140eef554d66c24cdf 100644
> --- a/include/net/route.h
> +++ b/include/net/route.h
> @@ -190,7 +190,7 @@ static inline int ip_route_input(struct sk_buff *skb, __be32 dst, __be32 src,
> rcu_read_lock();
> err = ip_route_input_noref(skb, dst, src, tos, devin);
> if (!err) {
> - skb_dst_force_safe(skb);
> + skb_dst_force(skb);
> if (!skb_dst(skb))
> err = -EINVAL;
> }
> diff --git a/include/net/sock.h b/include/net/sock.h
> index 03a362568357acc7278a318423dd3873103f90ca..a6b9a8d1a6df3f72df8f1aac0f577257fa6452d0 100644
> --- a/include/net/sock.h
> +++ b/include/net/sock.h
> @@ -856,7 +856,7 @@ void sk_stream_write_space(struct sock *sk);
> static inline void __sk_add_backlog(struct sock *sk, struct sk_buff *skb)
> {
> /* dont let skb dst not refcounted, we are going to leave rcu lock */
> - skb_dst_force_safe(skb);
> + skb_dst_force(skb);
>
> if (!sk->sk_backlog.tail)
> sk->sk_backlog.head = skb;
>
>
^ permalink raw reply
* Re: [PATCH net-next] bpf/verifier: improve disassembly of BPF_END instructions
From: Edward Cree @ 2017-09-21 16:58 UTC (permalink / raw)
To: Y Song; +Cc: Alexei Starovoitov, David Miller, netdev, Daniel Borkmann
In-Reply-To: <CAH3MdRWAHJS8Y8ketOAghYyorVUp6JijjZcSVY7gOiGAr7e_pw@mail.gmail.com>
On 21/09/17 17:40, Y Song wrote:
> On Thu, Sep 21, 2017 at 9:24 AM, Edward Cree <ecree@solarflare.com> wrote:
>> On 21/09/17 16:52, Alexei Starovoitov wrote:
>>> imo
>>> (u16) r4 endian be
>>> isn't intuitive.
>>> Can we come up with some better syntax?
>>> Like
>>> bswap16be r4
>>> bswap32le r4
>> Hmm, I don't like these, since bswapbe is a swap on *le* and a nop on be.
>>> or
>>>
>>> to_be16 r4
>>> to_le32 r4
>> And the problem here is that it's not just to_be, it's also from_be.
> Could you explain what is "from_be" here? Do not quite understand.
Taking the example of a little-endian processor:
cpu_to_be16() is a byte-swap, converting a u16 (cpu-endian) to a __be16.
be16_to_cpu(), to convert a __be16 to a u16, is *also* a byte-swap.
Meanwhile, cpu_to_le16() and le16_to_cpu() are both no-ops.
More generally, the conversions between cpu-endian and fixed-endian for
any given size are self-inverses. eBPF takes advantage of this by only
having a single opcode for both the "to" and "from" direction. So to
specify an endianness conversion, you need only the size and the fixed
endianness (le or be), not the to/from direction. Conversely, when
disassembling one of these instructions, you don't know whether it's a
cpu_to_be16() or a be16_to_cpu(), because they both look the same at an
instruction level (they only differ in what types the programmer thought
of the register as holding before and after).
^ permalink raw reply
* [PATCH net-next] rtnetlink: prepare for further rtnl lock pushdown/removal
From: Florian Westphal @ 2017-09-21 16:58 UTC (permalink / raw)
To: netdev
First patch adds a rudimentary vrf test case.
Remaining patches split large rtnl_fill_ifinfo into smaller chunks
to better see which parts
1. require rtnl
2. do not require it at all
3. rely on rtnl locking now but could be converted
add ASSERT_RTNL annotations/checks in a few spots as reminder for future
rtnl removal/pushdown patches.
net/core/rtnetlink.c | 161 ++++++++++++++++++++++---------
tools/testing/selftests/net/rtnetlink.sh | 42 ++++++++
2 files changed, 161 insertions(+), 42 deletions(-)
^ permalink raw reply
* [PATCH net-next 1/7] selftests: rtnetlink.sh: add rudimentary vrf test
From: Florian Westphal @ 2017-09-21 16:58 UTC (permalink / raw)
To: netdev; +Cc: Florian Westphal, David Ahern
In-Reply-To: <20170921165902.10746-1-fw@strlen.de>
Cc: David Ahern <dsahern@gmail.com>
Signed-off-by: Florian Westphal <fw@strlen.de>
---
tools/testing/selftests/net/rtnetlink.sh | 42 ++++++++++++++++++++++++++++++++
1 file changed, 42 insertions(+)
diff --git a/tools/testing/selftests/net/rtnetlink.sh b/tools/testing/selftests/net/rtnetlink.sh
index 4b48de565cae..6ee2bbaebcd4 100755
--- a/tools/testing/selftests/net/rtnetlink.sh
+++ b/tools/testing/selftests/net/rtnetlink.sh
@@ -291,6 +291,47 @@ kci_test_ifalias()
echo "PASS: set ifalias $namewant for $devdummy"
}
+kci_test_vrf()
+{
+ vrfname="test-vrf"
+ ret=0
+
+ ip link show type vrf 2>/dev/null
+ if [ $? -ne 0 ]; then
+ echo "SKIP: vrf: iproute2 too old"
+ return 0
+ fi
+
+ ip link add "$vrfname" type vrf table 10
+ check_err $?
+ if [ $ret -ne 0 ];then
+ echo "FAIL: can't add vrf interface, skipping test"
+ return 0
+ fi
+
+ ip -br link show type vrf | grep -q "$vrfname"
+ check_err $?
+ if [ $ret -ne 0 ];then
+ echo "FAIL: created vrf device not found"
+ return 1
+ fi
+
+ ip link set dev "$vrfname" up
+ check_err $?
+
+ ip link set dev "$devdummy" master "$vrfname"
+ check_err $?
+ ip link del dev "$vrfname"
+ check_err $?
+
+ if [ $ret -ne 0 ];then
+ echo "FAIL: vrf"
+ return 1
+ fi
+
+ echo "PASS: vrf"
+}
+
kci_test_rtnl()
{
kci_add_dummy
@@ -306,6 +347,7 @@ kci_test_rtnl()
kci_test_bridge
kci_test_addrlabel
kci_test_ifalias
+ kci_test_vrf
kci_del_dummy
}
--
2.13.5
^ permalink raw reply related
* [PATCH net-next 2/7] rtnetlink: add helper to put master ifindex
From: Florian Westphal @ 2017-09-21 16:58 UTC (permalink / raw)
To: netdev; +Cc: Florian Westphal
In-Reply-To: <20170921165902.10746-1-fw@strlen.de>
rtnl_fill_ifinfo currently requires caller to hold the rtnl mutex.
Unfortunately the function is quite large which makes it harder to see
which spots need the lock, which spots assume it and which ones could do
without.
Add helpers to factor out the ifindex dumping.
One helper can use rcu to remove rtnl dependency.
Signed-off-by: Florian Westphal <fw@strlen.de>
---
net/core/rtnetlink.c | 32 +++++++++++++++++++++++++++-----
1 file changed, 27 insertions(+), 5 deletions(-)
diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index a78fd61da0ec..c801212ee40e 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -1307,6 +1307,31 @@ static u32 rtnl_get_event(unsigned long event)
return rtnl_event_type;
}
+static int put_master_ifindex(struct sk_buff *skb, struct net_device *dev)
+{
+ const struct net_device *upper_dev;
+ int ret = 0;
+
+ rcu_read_lock();
+
+ upper_dev = netdev_master_upper_dev_get_rcu(dev);
+ if (upper_dev)
+ ret = nla_put_u32(skb, IFLA_MASTER, upper_dev->ifindex);
+
+ rcu_read_unlock();
+ return ret;
+}
+
+static int nla_put_iflink(struct sk_buff *skb, const struct net_device *dev)
+{
+ int ifindex = dev_get_iflink(dev);
+
+ if (dev->ifindex == ifindex)
+ return 0;
+
+ return nla_put_u32(skb, IFLA_LINK, ifindex);
+}
+
static int rtnl_fill_ifinfo(struct sk_buff *skb, struct net_device *dev,
int type, u32 pid, u32 seq, u32 change,
unsigned int flags, u32 ext_filter_mask,
@@ -1316,7 +1341,6 @@ static int rtnl_fill_ifinfo(struct sk_buff *skb, struct net_device *dev,
struct nlmsghdr *nlh;
struct nlattr *af_spec;
struct rtnl_af_ops *af_ops;
- struct net_device *upper_dev = netdev_master_upper_dev_get(dev);
ASSERT_RTNL();
nlh = nlmsg_put(skb, pid, seq, type, sizeof(*ifm), flags);
@@ -1345,10 +1369,8 @@ static int rtnl_fill_ifinfo(struct sk_buff *skb, struct net_device *dev,
#ifdef CONFIG_RPS
nla_put_u32(skb, IFLA_NUM_RX_QUEUES, dev->num_rx_queues) ||
#endif
- (dev->ifindex != dev_get_iflink(dev) &&
- nla_put_u32(skb, IFLA_LINK, dev_get_iflink(dev))) ||
- (upper_dev &&
- nla_put_u32(skb, IFLA_MASTER, upper_dev->ifindex)) ||
+ nla_put_iflink(skb, dev) ||
+ put_master_ifindex(skb, dev) ||
nla_put_u8(skb, IFLA_CARRIER, netif_carrier_ok(dev)) ||
(dev->qdisc &&
nla_put_string(skb, IFLA_QDISC, dev->qdisc->ops->id)) ||
--
2.13.5
^ permalink raw reply related
* [PATCH net-next 3/7] rtnetlink: add helper to dump qdisc name
From: Florian Westphal @ 2017-09-21 16:58 UTC (permalink / raw)
To: netdev; +Cc: Florian Westphal
In-Reply-To: <20170921165902.10746-1-fw@strlen.de>
We can use rcu here to make this safe even if we would not hold rtnl,
qdisc_destroy uses call_rcu to delay free of the qdisc for one grace period.
Signed-off-by: Florian Westphal <fw@strlen.de>
---
net/core/rtnetlink.c | 16 ++++++++++++++--
1 file changed, 14 insertions(+), 2 deletions(-)
diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index c801212ee40e..ad3f27da37a8 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -1332,6 +1332,19 @@ static int nla_put_iflink(struct sk_buff *skb, const struct net_device *dev)
return nla_put_u32(skb, IFLA_LINK, ifindex);
}
+static int nla_put_qdisc(struct sk_buff *skb, struct net_device *dev)
+{
+ struct Qdisc *q;
+ int ret = 0;
+
+ rcu_read_lock();
+ q = READ_ONCE(dev->qdisc);
+ if (q)
+ ret = nla_put_string(skb, IFLA_QDISC, q->ops->id);
+ rcu_read_unlock();
+ return ret;
+}
+
static int rtnl_fill_ifinfo(struct sk_buff *skb, struct net_device *dev,
int type, u32 pid, u32 seq, u32 change,
unsigned int flags, u32 ext_filter_mask,
@@ -1372,8 +1385,7 @@ static int rtnl_fill_ifinfo(struct sk_buff *skb, struct net_device *dev,
nla_put_iflink(skb, dev) ||
put_master_ifindex(skb, dev) ||
nla_put_u8(skb, IFLA_CARRIER, netif_carrier_ok(dev)) ||
- (dev->qdisc &&
- nla_put_string(skb, IFLA_QDISC, dev->qdisc->ops->id)) ||
+ nla_put_qdisc(skb, dev) ||
(dev->ifalias &&
nla_put_string(skb, IFLA_IFALIAS, dev->ifalias)) ||
nla_put_u32(skb, IFLA_CARRIER_CHANGES,
--
2.13.5
^ permalink raw reply related
* [PATCH net-next 4/7] rtnetlink: add helper to dump ifalias
From: Florian Westphal @ 2017-09-21 16:58 UTC (permalink / raw)
To: netdev; +Cc: Florian Westphal
In-Reply-To: <20170921165902.10746-1-fw@strlen.de>
ifalias is currently protected by rtnl mutex, add assertion
as a reminder.
Signed-off-by: Florian Westphal <fw@strlen.de>
---
net/core/rtnetlink.c | 13 +++++++++++--
1 file changed, 11 insertions(+), 2 deletions(-)
diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index ad3f27da37a8..1af3ef7f329d 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -1345,6 +1345,16 @@ static int nla_put_qdisc(struct sk_buff *skb, struct net_device *dev)
return ret;
}
+static int noinline nla_put_ifalias(struct sk_buff *skb, struct net_device *dev)
+{
+ ASSERT_RTNL();
+
+ if (dev->ifalias)
+ return nla_put_string(skb, IFLA_IFALIAS, dev->ifalias);
+
+ return 0;
+}
+
static int rtnl_fill_ifinfo(struct sk_buff *skb, struct net_device *dev,
int type, u32 pid, u32 seq, u32 change,
unsigned int flags, u32 ext_filter_mask,
@@ -1386,8 +1396,7 @@ static int rtnl_fill_ifinfo(struct sk_buff *skb, struct net_device *dev,
put_master_ifindex(skb, dev) ||
nla_put_u8(skb, IFLA_CARRIER, netif_carrier_ok(dev)) ||
nla_put_qdisc(skb, dev) ||
- (dev->ifalias &&
- nla_put_string(skb, IFLA_IFALIAS, dev->ifalias)) ||
+ nla_put_ifalias(skb, dev) ||
nla_put_u32(skb, IFLA_CARRIER_CHANGES,
atomic_read(&dev->carrier_changes)) ||
nla_put_u8(skb, IFLA_PROTO_DOWN, dev->proto_down))
--
2.13.5
^ permalink raw reply related
* [PATCH net-next 5/7] rtnetlink: add helper to dump vf information
From: Florian Westphal @ 2017-09-21 16:59 UTC (permalink / raw)
To: netdev; +Cc: Florian Westphal
In-Reply-To: <20170921165902.10746-1-fw@strlen.de>
similar to earlier patches, split out more parts of this function to
better see what is happening and where we assume rtnl is locked.
Signed-off-by: Florian Westphal <fw@strlen.de>
---
net/core/rtnetlink.c | 80 ++++++++++++++++++++++++++++++++--------------------
1 file changed, 50 insertions(+), 30 deletions(-)
diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index 1af3ef7f329d..7503021fe308 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -1211,6 +1211,36 @@ static noinline_for_stack int rtnl_fill_vfinfo(struct sk_buff *skb,
return -EMSGSIZE;
}
+static noinline_for_stack int rtnl_fill_vf(struct sk_buff *skb,
+ struct net_device *dev,
+ u32 ext_filter_mask)
+{
+ struct nlattr *vfinfo;
+ int i, num_vfs;
+
+ if (!dev->dev.parent || ((ext_filter_mask & RTEXT_FILTER_VF) == 0))
+ return 0;
+
+ num_vfs = dev_num_vf(dev->dev.parent);
+ if (nla_put_u32(skb, IFLA_NUM_VF, num_vfs))
+ return -EMSGSIZE;
+
+ if (!dev->netdev_ops->ndo_get_vf_config)
+ return 0;
+
+ vfinfo = nla_nest_start(skb, IFLA_VFINFO_LIST);
+ if (!vfinfo)
+ return -EMSGSIZE;
+
+ for (i = 0; i < num_vfs; i++) {
+ if (rtnl_fill_vfinfo(skb, dev, i, vfinfo))
+ return -EMSGSIZE;
+ }
+
+ nla_nest_end(skb, vfinfo);
+ return 0;
+}
+
static int rtnl_fill_link_ifmap(struct sk_buff *skb, struct net_device *dev)
{
struct rtnl_link_ifmap map;
@@ -1355,6 +1385,23 @@ static int noinline nla_put_ifalias(struct sk_buff *skb, struct net_device *dev)
return 0;
}
+static int noinline rtnl_fill_link_netnsid(struct sk_buff *skb,
+ const struct net_device *dev)
+{
+ if (dev->rtnl_link_ops && dev->rtnl_link_ops->get_link_net) {
+ struct net *link_net = dev->rtnl_link_ops->get_link_net(dev);
+
+ if (!net_eq(dev_net(dev), link_net)) {
+ int id = peernet2id_alloc(dev_net(dev), link_net);
+
+ if (nla_put_s32(skb, IFLA_LINK_NETNSID, id))
+ return -EMSGSIZE;
+ }
+ }
+
+ return 0;
+}
+
static int rtnl_fill_ifinfo(struct sk_buff *skb, struct net_device *dev,
int type, u32 pid, u32 seq, u32 change,
unsigned int flags, u32 ext_filter_mask,
@@ -1428,27 +1475,9 @@ static int rtnl_fill_ifinfo(struct sk_buff *skb, struct net_device *dev,
if (rtnl_fill_stats(skb, dev))
goto nla_put_failure;
- if (dev->dev.parent && (ext_filter_mask & RTEXT_FILTER_VF) &&
- nla_put_u32(skb, IFLA_NUM_VF, dev_num_vf(dev->dev.parent)))
+ if (rtnl_fill_vf(skb, dev, ext_filter_mask))
goto nla_put_failure;
- if (dev->netdev_ops->ndo_get_vf_config && dev->dev.parent &&
- ext_filter_mask & RTEXT_FILTER_VF) {
- int i;
- struct nlattr *vfinfo;
- int num_vfs = dev_num_vf(dev->dev.parent);
-
- vfinfo = nla_nest_start(skb, IFLA_VFINFO_LIST);
- if (!vfinfo)
- goto nla_put_failure;
- for (i = 0; i < num_vfs; i++) {
- if (rtnl_fill_vfinfo(skb, dev, i, vfinfo))
- goto nla_put_failure;
- }
-
- nla_nest_end(skb, vfinfo);
- }
-
if (rtnl_port_fill(skb, dev, ext_filter_mask))
goto nla_put_failure;
@@ -1460,17 +1489,8 @@ static int rtnl_fill_ifinfo(struct sk_buff *skb, struct net_device *dev,
goto nla_put_failure;
}
- if (dev->rtnl_link_ops &&
- dev->rtnl_link_ops->get_link_net) {
- struct net *link_net = dev->rtnl_link_ops->get_link_net(dev);
-
- if (!net_eq(dev_net(dev), link_net)) {
- int id = peernet2id_alloc(dev_net(dev), link_net);
-
- if (nla_put_s32(skb, IFLA_LINK_NETNSID, id))
- goto nla_put_failure;
- }
- }
+ if (rtnl_fill_link_netnsid(skb, dev))
+ goto nla_put_failure;
if (!(af_spec = nla_nest_start(skb, IFLA_AF_SPEC)))
goto nla_put_failure;
--
2.13.5
^ permalink raw reply related
* [PATCH net-next 6/7] rtnetlink: link ops lookup must occur with rtnl lock held
From: Florian Westphal @ 2017-09-21 16:59 UTC (permalink / raw)
To: netdev; +Cc: Florian Westphal
In-Reply-To: <20170921165902.10746-1-fw@strlen.de>
all callers meet this requirement, this serves as reminder to not
forget about this in future rtnl pushdown work.
Signed-off-by: Florian Westphal <fw@strlen.de>
---
net/core/rtnetlink.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index 7503021fe308..7af9774aec40 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -274,6 +274,8 @@ static const struct rtnl_link_ops *rtnl_link_ops_get(const char *kind)
{
const struct rtnl_link_ops *ops;
+ ASSERT_RTNL();
+
list_for_each_entry(ops, &link_ops, list) {
if (!strcmp(ops->kind, kind))
return ops;
@@ -1618,6 +1620,8 @@ static const struct rtnl_link_ops *linkinfo_to_kind_ops(const struct nlattr *nla
const struct rtnl_link_ops *ops = NULL;
struct nlattr *linfo[IFLA_INFO_MAX + 1];
+ ASSERT_RTNL();
+
if (nla_parse_nested(linfo, IFLA_INFO_MAX, nla,
ifla_info_policy, NULL) < 0)
return NULL;
--
2.13.5
^ permalink raw reply related
* [PATCH net-next 7/7] rtnetlink: rtnl_have_link_slave_info doesn't need rtnl
From: Florian Westphal @ 2017-09-21 16:59 UTC (permalink / raw)
To: netdev; +Cc: Florian Westphal
In-Reply-To: <20170921165902.10746-1-fw@strlen.de>
it can be switched to rcu.
rtnl_link_slave_info_fill on the other hand does need it,
at least for now. Add ASSERT_RTNL annotation as a reminder.
Signed-off-by: Florian Westphal <fw@strlen.de>
---
net/core/rtnetlink.c | 16 +++++++++++++---
1 file changed, 13 insertions(+), 3 deletions(-)
diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index 7af9774aec40..9fd48b437d64 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -524,11 +524,15 @@ static size_t rtnl_link_get_af_size(const struct net_device *dev,
static bool rtnl_have_link_slave_info(const struct net_device *dev)
{
struct net_device *master_dev;
+ bool ret = false;
- master_dev = netdev_master_upper_dev_get((struct net_device *) dev);
+ rcu_read_lock();
+
+ master_dev = netdev_master_upper_dev_get_rcu((struct net_device *) dev);
if (master_dev && master_dev->rtnl_link_ops)
- return true;
- return false;
+ ret = true;
+ rcu_read_unlock();
+ return ret;
}
static int rtnl_link_slave_info_fill(struct sk_buff *skb,
@@ -539,6 +543,8 @@ static int rtnl_link_slave_info_fill(struct sk_buff *skb,
struct nlattr *slave_data;
int err;
+ ASSERT_RTNL();
+
master_dev = netdev_master_upper_dev_get((struct net_device *) dev);
if (!master_dev)
return 0;
@@ -570,6 +576,8 @@ static int rtnl_link_info_fill(struct sk_buff *skb,
struct nlattr *data;
int err;
+ ASSERT_RTNL();
+
if (!ops)
return 0;
if (nla_put_string(skb, IFLA_INFO_KIND, ops->kind) < 0)
@@ -600,6 +608,8 @@ static int rtnl_link_fill(struct sk_buff *skb, const struct net_device *dev)
struct nlattr *linkinfo;
int err = -EMSGSIZE;
+ ASSERT_RTNL();
+
linkinfo = nla_nest_start(skb, IFLA_LINKINFO);
if (linkinfo == NULL)
goto out;
--
2.13.5
^ permalink raw reply related
* [PATCH 00/64] use setup_timer() helper function.
From: Allen Pais @ 2017-09-21 17:04 UTC (permalink / raw)
To: netdev
Cc: linux-kernel, m.grzeschik, dmitry.tarnyagin, wg, mkl, mark.einon,
linux, pcnet32, f.fainelli, bcm-kernel-feedback-list, venza, ajk,
jes, romieu, khc, simon, davem, linux-can, adi-buildroot-devel,
Allen Pais
This series uses setup_timer() helper function. The series
addresses the files under drivers/net/*.
Allen Pais (64):
drivers: net: de4x: use setup_timer() helper.
drivers: net: b44: use setup_timer() helper.
drivers: net: pcnet32: use setup_timer() helper.
drivers: net: brcm80211: use setup_timer() helper.
drivers : net: niu: use setup_timer() helper.
drivers: net: bcm63xx: use setup_timer() helper.
drivers: net: declance: use setup_timer() helper.
drivers: net: am79c961: use setup_timer() helper.
drivers: net: et131x: use setup_timer() helper.
drivers: net: appletalk: cops: use setup_timer() helper.
drivers: net: rsi_91x: use setup_timer() helper.
drivers: net: atp: use setup_timer() helper.
drivers: net: ns83820: use setup_timer() helper.
drivers: net: ixgb: use setup_timer() helper.
drivers: net: sundance: use setup_timer() helper.
drivers: net: tg3: use setup_timer() helper.
drivers: net: sdla: use setup_timer() helper.
drivers: net: cisco_hdlc: use setup_timer() helper.
drivers: net: slip: use setup_timer() helper.
drivers: net: spider_net: use setup_timer() helper.
drivers: net: sun: cassini: use setup_timer() helper.
drivers: net: natsemi: use setup_timer() helper.
drivers: net: winbond-840: use setup_timer() helper.
drivers: net: enic: use setup_timer() helper.
drivers: net: bnx2: use setup_timer() helper.
drivers: net: xen-netback: use setup_timer() helper.
drivers: net: atmel: use setup_timer() helper.
drivers: net: hippi: use setup_timer() helper.
drivers: net: smsc: use setup_timer() helper.
drivers: net: qlogic: use setup_timer() helper.
drivers: net: e1000e: use setup_timer() helper.
drivers: net: amd: use setup_timer() helper.
drivers: net: amd8111e: use setup_timer() helper.
drivers: net: eql: use setup_timer() helper.
drivers: net: can: usb: use setup_timer() helper.
drivers: net: can: use setup_timer() helper.
drivers: net: arcnet: use setup_timer() helper.
drivers: net: ath6kl: use setup_timer() helper.
drivers: net: sun: use setup_timer() helper.
drivers: net: sis900: use setup_timer() helper.
drivers: net: packetengines: use setup_timer() helper.
drivers: net: mlx5: use setup_timer() helper.
drivers: net: mlx4: use setup_timer() helper.
drivers: net: pxa168: use setup_timer() helper.
drivers: net: fealnx: use setup_timer() helper.
drivers: net: dmfe: use setup_timer() helper.
drivers: net: bnxt: use setup_timer() helper.
drivers: net: amd: use setup_timer() helper.
drivers: net: adi: use setup_timer() helper.
drivers: net: can: sja1000: use setup_timer() helper.
drivers: net: caif: use setup_timer() helper.
drivers: net: appletalk: use setup_timer() helper.
drivers: net: dscc: use setup_timer() helper.
drivers: net: hdlc_ppp: use setup_timer() helper.
drivers: net: hamradio: use setup_timer() helper.
drivers: net: cpsw_ale: use setup_timer() helper.
drivers: net: stmmac: use setup_timer() helper.
drivers: net: packetengines: use setup_timer() helper.
drivers: net: i40evf: use setup_timer() helper.
drivers: net: uli526x: use setup_timer() helper.
drivers: net: enic: use setup_timer() helper.
drivers: net: cxgb: use setup_timer() helper.
drivers: net: bnx2x: use setup_timer() helper.
drivers: net: lmc: use setup_timer() helper.
drivers/net/appletalk/cops.c | 4 +---
drivers/net/appletalk/ltpc.c | 4 +---
drivers/net/arcnet/arcnet.c | 4 +---
drivers/net/caif/caif_hsi.c | 15 ++++++---------
drivers/net/can/grcan.c | 10 ++++------
drivers/net/can/sja1000/peak_pcmcia.c | 4 +---
drivers/net/can/usb/peak_usb/pcan_usb.c | 5 ++---
drivers/net/eql.c | 4 +---
drivers/net/ethernet/adi/bfin_mac.c | 5 ++---
drivers/net/ethernet/agere/et131x.c | 5 ++---
drivers/net/ethernet/amd/a2065.c | 7 +++----
drivers/net/ethernet/amd/am79c961a.c | 4 +---
drivers/net/ethernet/amd/amd8111e.c | 5 ++---
drivers/net/ethernet/amd/declance.c | 6 +++---
drivers/net/ethernet/amd/pcnet32.c | 5 ++---
drivers/net/ethernet/amd/sunlance.c | 5 ++---
drivers/net/ethernet/broadcom/b44.c | 4 +---
drivers/net/ethernet/broadcom/bcm63xx_enet.c | 5 ++---
drivers/net/ethernet/broadcom/bnx2.c | 4 +---
drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c | 4 +---
drivers/net/ethernet/broadcom/bnxt/bnxt.c | 4 +---
drivers/net/ethernet/broadcom/tg3.c | 4 +---
drivers/net/ethernet/chelsio/cxgb/sge.c | 5 ++---
drivers/net/ethernet/cisco/enic/enic_clsf.h | 5 ++---
drivers/net/ethernet/cisco/enic/enic_main.c | 5 ++---
drivers/net/ethernet/dec/tulip/de4x5.c | 5 ++---
drivers/net/ethernet/dec/tulip/dmfe.c | 4 +---
drivers/net/ethernet/dec/tulip/uli526x.c | 4 +---
drivers/net/ethernet/dec/tulip/winbond-840.c | 4 +---
drivers/net/ethernet/dlink/sundance.c | 4 +---
drivers/net/ethernet/fealnx.c | 8 ++------
drivers/net/ethernet/intel/e1000e/netdev.c | 11 ++++-------
drivers/net/ethernet/intel/i40evf/i40evf_main.c | 5 ++---
drivers/net/ethernet/intel/ixgb/ixgb_main.c | 5 ++---
drivers/net/ethernet/marvell/pxa168_eth.c | 5 ++---
drivers/net/ethernet/mellanox/mlx4/catas.c | 4 +---
drivers/net/ethernet/mellanox/mlx5/core/health.c | 4 +---
drivers/net/ethernet/natsemi/natsemi.c | 4 +---
drivers/net/ethernet/natsemi/ns83820.c | 4 +---
drivers/net/ethernet/packetengines/hamachi.c | 4 +---
drivers/net/ethernet/packetengines/yellowfin.c | 4 +---
drivers/net/ethernet/qlogic/qla3xxx.c | 4 +---
drivers/net/ethernet/realtek/atp.c | 4 +---
drivers/net/ethernet/sis/sis900.c | 4 +---
drivers/net/ethernet/smsc/epic100.c | 4 +---
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 4 +---
drivers/net/ethernet/sun/cassini.c | 5 +----
drivers/net/ethernet/sun/niu.c | 4 +---
drivers/net/ethernet/sun/sungem.c | 4 +---
drivers/net/ethernet/ti/cpsw_ale.c | 4 +---
drivers/net/ethernet/toshiba/spider_net.c | 12 +++++-------
drivers/net/hamradio/6pack.c | 4 +---
drivers/net/hippi/rrunner.c | 4 +---
drivers/net/slip/slip.c | 8 ++------
drivers/net/wan/dscc4.c | 4 +---
drivers/net/wan/hdlc_cisco.c | 4 +---
drivers/net/wan/hdlc_ppp.c | 4 +---
drivers/net/wan/lmc/lmc_main.c | 4 +---
drivers/net/wan/sdla.c | 4 +---
drivers/net/wireless/ath/ath6kl/txrx.c | 4 +---
drivers/net/wireless/atmel/atmel.c | 5 ++---
.../net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c | 5 ++---
drivers/net/wireless/rsi/rsi_91x_hal.c | 5 ++---
drivers/net/xen-netback/interface.c | 3 +--
64 files changed, 102 insertions(+), 216 deletions(-)
--
2.7.4
^ permalink raw reply
* [PATCH 01/64] drivers: net: de4x: use setup_timer() helper.
From: Allen Pais @ 2017-09-21 17:04 UTC (permalink / raw)
To: netdev
Cc: linux-kernel, m.grzeschik, dmitry.tarnyagin, wg, mkl, mark.einon,
linux, pcnet32, f.fainelli, bcm-kernel-feedback-list, venza, ajk,
jes, romieu, khc, simon, davem, linux-can, adi-buildroot-devel,
Allen Pais
In-Reply-To: <1506013525-29291-1-git-send-email-allen.lkml@gmail.com>
Use setup_timer function instead of initializing timer with the
function and data fields.
Signed-off-by: Allen Pais <allen.lkml@gmail.com>
---
drivers/net/ethernet/dec/tulip/de4x5.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/dec/tulip/de4x5.c b/drivers/net/ethernet/dec/tulip/de4x5.c
index 0affee9..299812e 100644
--- a/drivers/net/ethernet/dec/tulip/de4x5.c
+++ b/drivers/net/ethernet/dec/tulip/de4x5.c
@@ -1147,9 +1147,8 @@ de4x5_hw_init(struct net_device *dev, u_long iobase, struct device *gendev)
lp->timeout = -1;
lp->gendev = gendev;
spin_lock_init(&lp->lock);
- init_timer(&lp->timer);
- lp->timer.function = (void (*)(unsigned long))de4x5_ast;
- lp->timer.data = (unsigned long)dev;
+ setup_timer(&lp->timer, (void (*)(unsigned long))de4x5_ast,
+ (unsigned long)dev);
de4x5_parse_params(dev);
/*
--
2.7.4
^ permalink raw reply related
* [PATCH 02/64] drivers: net: b44: use setup_timer() helper.
From: Allen Pais @ 2017-09-21 17:04 UTC (permalink / raw)
To: netdev
Cc: linux-kernel, m.grzeschik, dmitry.tarnyagin, wg, mkl, mark.einon,
linux, pcnet32, f.fainelli, bcm-kernel-feedback-list, venza, ajk,
jes, romieu, khc, simon, davem, linux-can, adi-buildroot-devel,
Allen Pais
In-Reply-To: <1506013525-29291-1-git-send-email-allen.lkml@gmail.com>
Use setup_timer function instead of initializing timer with the
function and data fields.
Signed-off-by: Allen Pais <allen.lkml@gmail.com>
---
drivers/net/ethernet/broadcom/b44.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/broadcom/b44.c b/drivers/net/ethernet/broadcom/b44.c
index a1125d1..42e44fc 100644
--- a/drivers/net/ethernet/broadcom/b44.c
+++ b/drivers/net/ethernet/broadcom/b44.c
@@ -1474,10 +1474,8 @@ static int b44_open(struct net_device *dev)
goto out;
}
- init_timer(&bp->timer);
+ setup_timer(&bp->timer, b44_timer, (unsigned long)bp);
bp->timer.expires = jiffies + HZ;
- bp->timer.data = (unsigned long) bp;
- bp->timer.function = b44_timer;
add_timer(&bp->timer);
b44_enable_ints(bp);
--
2.7.4
^ permalink raw reply related
* [PATCH 03/64] drivers: net: pcnet32: use setup_timer() helper.
From: Allen Pais @ 2017-09-21 17:04 UTC (permalink / raw)
To: netdev
Cc: linux-kernel, m.grzeschik, dmitry.tarnyagin, wg, mkl, mark.einon,
linux, pcnet32, f.fainelli, bcm-kernel-feedback-list, venza, ajk,
jes, romieu, khc, simon, davem, linux-can, adi-buildroot-devel,
Allen Pais
In-Reply-To: <1506013525-29291-1-git-send-email-allen.lkml@gmail.com>
Use setup_timer function instead of initializing timer with the
function and data fields.
Signed-off-by: Allen Pais <allen.lkml@gmail.com>
---
drivers/net/ethernet/amd/pcnet32.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/amd/pcnet32.c b/drivers/net/ethernet/amd/pcnet32.c
index 7f60d17..e461536 100644
--- a/drivers/net/ethernet/amd/pcnet32.c
+++ b/drivers/net/ethernet/amd/pcnet32.c
@@ -1970,9 +1970,8 @@ pcnet32_probe1(unsigned long ioaddr, int shared, struct pci_dev *pdev)
lp->options |= PCNET32_PORT_MII;
}
- init_timer(&lp->watchdog_timer);
- lp->watchdog_timer.data = (unsigned long)dev;
- lp->watchdog_timer.function = (void *)&pcnet32_watchdog;
+ setup_timer(&lp->watchdog_timer, (void *)&pcnet32_watchdog,
+ (unsigned long)dev);
/* The PCNET32-specific entries in the device structure. */
dev->netdev_ops = &pcnet32_netdev_ops;
--
2.7.4
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox