* [PATCH net-next 1/4] ipv6: coding style - no assignment in if statements
From: Ian Morris @ 2014-09-02 19:16 UTC (permalink / raw)
To: netdev; +Cc: Ian Morris
In-Reply-To: <1409685364-4327-1-git-send-email-ipm@chirality.org.uk>
This patch makes no changes to the logic of the ipv6 stack however
it addresses some coding style issues by removing assignments made
in if statements.
No change in the object output is detected by the objdiff script.
Signed-off-by: Ian Morris <ipm@chirality.org.uk>
---
net/ipv6/addrconf.c | 12 ++++++++----
net/ipv6/ah6.c | 7 ++++---
net/ipv6/datagram.c | 3 ++-
net/ipv6/esp6.c | 3 ++-
net/ipv6/icmp.c | 3 ++-
net/ipv6/ip6_flowlabel.c | 6 +++++-
net/ipv6/ip6_input.c | 3 ++-
net/ipv6/ip6_output.c | 17 ++++++++++-------
net/ipv6/ip6_tunnel.c | 16 ++++++++--------
net/ipv6/ip6_vti.c | 4 ++--
net/ipv6/ndisc.c | 7 ++++---
net/ipv6/raw.c | 3 ++-
net/ipv6/reassembly.c | 3 ++-
net/ipv6/sit.c | 7 ++++---
net/ipv6/udp.c | 6 ++++--
net/ipv6/xfrm6_protocol.c | 25 ++++++++++++++++---------
16 files changed, 77 insertions(+), 48 deletions(-)
diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index 267ce3c..509c53e 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -2524,7 +2524,8 @@ static int inet6_addr_del(struct net *net, int ifindex, u32 ifa_flags,
if (!dev)
return -ENODEV;
- if ((idev = __in6_dev_get(dev)) == NULL)
+ idev = __in6_dev_get(dev);
+ if (idev == NULL)
return -ENXIO;
read_lock_bh(&idev->lock);
@@ -2671,7 +2672,8 @@ static void init_loopback(struct net_device *dev)
ASSERT_RTNL();
- if ((idev = ipv6_find_idev(dev)) == NULL) {
+ idev = ipv6_find_idev(dev);
+ if (idev == NULL) {
pr_debug("%s: add_dev failed\n", __func__);
return;
}
@@ -2794,7 +2796,8 @@ static void addrconf_sit_config(struct net_device *dev)
* our v4 addrs in the tunnel
*/
- if ((idev = ipv6_find_idev(dev)) == NULL) {
+ idev = ipv6_find_idev(dev);
+ if (idev == NULL) {
pr_debug("%s: add_dev failed\n", __func__);
return;
}
@@ -2818,7 +2821,8 @@ static void addrconf_gre_config(struct net_device *dev)
ASSERT_RTNL();
- if ((idev = ipv6_find_idev(dev)) == NULL) {
+ idev = ipv6_find_idev(dev);
+ if (idev == NULL) {
pr_debug("%s: add_dev failed\n", __func__);
return;
}
diff --git a/net/ipv6/ah6.c b/net/ipv6/ah6.c
index fcffd4e..dc33a77 100644
--- a/net/ipv6/ah6.c
+++ b/net/ipv6/ah6.c
@@ -354,7 +354,8 @@ static int ah6_output(struct xfrm_state *x, struct sk_buff *skb)
ahp = x->data;
ahash = ahp->ahash;
- if ((err = skb_cow_data(skb, 0, &trailer)) < 0)
+ err = skb_cow_data(skb, 0, &trailer);
+ if (err < 0)
goto out;
nfrags = err;
@@ -560,8 +561,8 @@ static int ah6_input(struct xfrm_state *x, struct sk_buff *skb)
if (!pskb_may_pull(skb, ah_hlen))
goto out;
-
- if ((err = skb_cow_data(skb, 0, &trailer)) < 0)
+ err = skb_cow_data(skb, 0, &trailer);
+ if (err < 0)
goto out;
nfrags = err;
diff --git a/net/ipv6/datagram.c b/net/ipv6/datagram.c
index 1844e87..69f3153 100644
--- a/net/ipv6/datagram.c
+++ b/net/ipv6/datagram.c
@@ -418,7 +418,8 @@ int ipv6_recv_error(struct sock *sk, struct msghdr *msg, int len, int *addr_len)
/* Reset and regenerate socket error */
spin_lock_bh(&sk->sk_error_queue.lock);
sk->sk_err = 0;
- if ((skb2 = skb_peek(&sk->sk_error_queue)) != NULL) {
+ skb2 = skb_peek(&sk->sk_error_queue);
+ if (skb2 != NULL) {
sk->sk_err = SKB_EXT_ERR(skb2)->ee.ee_errno;
spin_unlock_bh(&sk->sk_error_queue.lock);
sk->sk_error_report(sk);
diff --git a/net/ipv6/esp6.c b/net/ipv6/esp6.c
index 83fc3a3..3cee92d 100644
--- a/net/ipv6/esp6.c
+++ b/net/ipv6/esp6.c
@@ -345,7 +345,8 @@ static int esp6_input(struct xfrm_state *x, struct sk_buff *skb)
goto out;
}
- if ((nfrags = skb_cow_data(skb, 0, &trailer)) < 0) {
+ nfrags = skb_cow_data(skb, 0, &trailer);
+ if (nfrags < 0) {
ret = -EINVAL;
goto out;
}
diff --git a/net/ipv6/icmp.c b/net/ipv6/icmp.c
index 394bb82..5c79dbb 100644
--- a/net/ipv6/icmp.c
+++ b/net/ipv6/icmp.c
@@ -239,7 +239,8 @@ int icmpv6_push_pending_frames(struct sock *sk, struct flowi6 *fl6,
struct icmp6hdr *icmp6h;
int err = 0;
- if ((skb = skb_peek(&sk->sk_write_queue)) == NULL)
+ skb = skb_peek(&sk->sk_write_queue);
+ if (skb == NULL)
goto out;
icmp6h = icmp6_hdr(skb);
diff --git a/net/ipv6/ip6_flowlabel.c b/net/ipv6/ip6_flowlabel.c
index 3dd7d4e..36da541 100644
--- a/net/ipv6/ip6_flowlabel.c
+++ b/net/ipv6/ip6_flowlabel.c
@@ -654,7 +654,11 @@ release:
goto done;
err = -ENOMEM;
- if (sfl1 == NULL || (err = mem_check(sk)) != 0)
+ if (sfl1 == NULL)
+ goto done;
+
+ err = mem_check(sk);
+ if (err != 0)
goto done;
fl1 = fl_intern(net, fl, freq.flr_label);
diff --git a/net/ipv6/ip6_input.c b/net/ipv6/ip6_input.c
index a3084ab..aacdcb4 100644
--- a/net/ipv6/ip6_input.c
+++ b/net/ipv6/ip6_input.c
@@ -220,7 +220,8 @@ resubmit:
nexthdr = skb_network_header(skb)[nhoff];
raw = raw6_local_deliver(skb, nexthdr);
- if ((ipprot = rcu_dereference(inet6_protos[nexthdr])) != NULL) {
+ ipprot = rcu_dereference(inet6_protos[nexthdr]);
+ if (ipprot != NULL) {
int ret;
if (ipprot->flags & INET6_PROTO_FINAL) {
diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c
index b7a3e7b..86fc687 100644
--- a/net/ipv6/ip6_output.c
+++ b/net/ipv6/ip6_output.c
@@ -754,9 +754,8 @@ slow_path:
/*
* Allocate buffer.
*/
-
- if ((frag = alloc_skb(len + hlen + sizeof(struct frag_hdr) +
- hroom + troom, GFP_ATOMIC)) == NULL) {
+ frag = alloc_skb(len + hlen + sizeof(struct frag_hdr) + hroom + troom, GFP_ATOMIC);
+ if (frag == NULL) {
NETDEBUG(KERN_INFO "IPv6: frag: no memory for new fragment!\n");
IP6_INC_STATS(net, ip6_dst_idev(skb_dst(skb)),
IPSTATS_MIB_FRAGFAILS);
@@ -904,7 +903,8 @@ static int ip6_dst_lookup_tail(struct sock *sk,
if (*dst == NULL)
*dst = ip6_route_output(net, sk, fl6);
- if ((err = (*dst)->error))
+ err = (*dst)->error;
+ if (err)
goto out_err_release;
if (ipv6_addr_any(&fl6->saddr)) {
@@ -952,7 +952,8 @@ static int ip6_dst_lookup_tail(struct sock *sk,
memcpy(&fl_gw6, fl6, sizeof(struct flowi6));
memset(&fl_gw6.daddr, 0, sizeof(struct in6_addr));
*dst = ip6_route_output(net, sk, &fl_gw6);
- if ((err = (*dst)->error))
+ err = (*dst)->error;
+ if (err)
goto out_err_release;
}
}
@@ -1060,7 +1061,8 @@ static inline int ip6_ufo_append_data(struct sock *sk,
* device, so create one single skb packet containing complete
* udp datagram
*/
- if ((skb = skb_peek_tail(&sk->sk_write_queue)) == NULL) {
+ skb = skb_peek_tail(&sk->sk_write_queue);
+ if (skb == NULL) {
skb = sock_alloc_send_skb(sk,
hh_len + fragheaderlen + transhdrlen + 20,
(flags & MSG_DONTWAIT), &err);
@@ -1540,7 +1542,8 @@ int ip6_push_pending_frames(struct sock *sk)
unsigned char proto = fl6->flowi6_proto;
int err = 0;
- if ((skb = __skb_dequeue(&sk->sk_write_queue)) == NULL)
+ skb = __skb_dequeue(&sk->sk_write_queue);
+ if (skb == NULL)
goto out;
tail_skb = &(skb_shinfo(skb)->frag_list);
diff --git a/net/ipv6/ip6_tunnel.c b/net/ipv6/ip6_tunnel.c
index e01bd03..b38cc18 100644
--- a/net/ipv6/ip6_tunnel.c
+++ b/net/ipv6/ip6_tunnel.c
@@ -482,8 +482,8 @@ ip6_tnl_err(struct sk_buff *skb, __u8 ipproto, struct inet6_skb_parm *opt,
processing of the error. */
rcu_read_lock();
- if ((t = ip6_tnl_lookup(dev_net(skb->dev), &ipv6h->daddr,
- &ipv6h->saddr)) == NULL)
+ t = ip6_tnl_lookup(dev_net(skb->dev), &ipv6h->daddr, &ipv6h->saddr);
+ if (t == NULL)
goto out;
if (t->parms.proto != ipproto && t->parms.proto != 0)
@@ -530,7 +530,8 @@ ip6_tnl_err(struct sk_buff *skb, __u8 ipproto, struct inet6_skb_parm *opt,
mtu = IPV6_MIN_MTU;
t->dev->mtu = mtu;
- if ((len = sizeof(*ipv6h) + ntohs(ipv6h->payload_len)) > mtu) {
+ len = sizeof(*ipv6h) + ntohs(ipv6h->payload_len);
+ if (len > mtu) {
rel_type = ICMPV6_PKT_TOOBIG;
rel_code = 0;
rel_info = mtu;
@@ -790,9 +791,8 @@ static int ip6_tnl_rcv(struct sk_buff *skb, __u16 protocol,
int err;
rcu_read_lock();
-
- if ((t = ip6_tnl_lookup(dev_net(skb->dev), &ipv6h->saddr,
- &ipv6h->daddr)) != NULL) {
+ t = ip6_tnl_lookup(dev_net(skb->dev), &ipv6h->saddr, &ipv6h->daddr);
+ if (t != NULL) {
struct pcpu_sw_netstats *tstats;
if (t->parms.proto != ipproto && t->parms.proto != 0) {
@@ -1016,8 +1016,8 @@ static int ip6_tnl_xmit2(struct sk_buff *skb,
if (skb_headroom(skb) < max_headroom || skb_shared(skb) ||
(skb_cloned(skb) && !skb_clone_writable(skb, 0))) {
struct sk_buff *new_skb;
-
- if (!(new_skb = skb_realloc_headroom(skb, max_headroom)))
+ new_skb = skb_realloc_headroom(skb, max_headroom);
+ if (!new_skb)
goto tx_err_dst_release;
if (skb->sk)
diff --git a/net/ipv6/ip6_vti.c b/net/ipv6/ip6_vti.c
index 7f52fd9..77a194f 100644
--- a/net/ipv6/ip6_vti.c
+++ b/net/ipv6/ip6_vti.c
@@ -287,8 +287,8 @@ static int vti6_rcv(struct sk_buff *skb)
const struct ipv6hdr *ipv6h = ipv6_hdr(skb);
rcu_read_lock();
- if ((t = vti6_tnl_lookup(dev_net(skb->dev), &ipv6h->saddr,
- &ipv6h->daddr)) != NULL) {
+ t = vti6_tnl_lookup(dev_net(skb->dev), &ipv6h->saddr, &ipv6h->daddr);
+ if (t != NULL) {
if (t->parms.proto != IPPROTO_IPV6 && t->parms.proto != 0) {
rcu_read_unlock();
goto discard;
diff --git a/net/ipv6/ndisc.c b/net/ipv6/ndisc.c
index 4cb45c1..d0232b2 100644
--- a/net/ipv6/ndisc.c
+++ b/net/ipv6/ndisc.c
@@ -162,7 +162,8 @@ static void ndisc_fill_addr_option(struct sk_buff *skb, int type, void *data)
memcpy(opt+2, data, data_len);
data_len += 2;
opt += data_len;
- if ((space -= data_len) > 0)
+ space -= data_len;
+ if (space > 0)
memset(opt, 0, space);
}
@@ -656,8 +657,8 @@ static void ndisc_solicit(struct neighbour *neigh, struct sk_buff *skb)
if (skb && ipv6_chk_addr(dev_net(dev), &ipv6_hdr(skb)->saddr, dev, 1))
saddr = &ipv6_hdr(skb)->saddr;
-
- if ((probes -= NEIGH_VAR(neigh->parms, UCAST_PROBES)) < 0) {
+ probes -= NEIGH_VAR(neigh->parms, UCAST_PROBES);
+ if (probes < 0) {
if (!(neigh->nud_state & NUD_VALID)) {
ND_PRINTK(1, dbg,
"%s: trying to ucast probe in NUD_INVALID: %pI6\n",
diff --git a/net/ipv6/raw.c b/net/ipv6/raw.c
index 896af88..af3661c 100644
--- a/net/ipv6/raw.c
+++ b/net/ipv6/raw.c
@@ -548,7 +548,8 @@ static int rawv6_push_pending_frames(struct sock *sk, struct flowi6 *fl6,
if (!rp->checksum)
goto send;
- if ((skb = skb_peek(&sk->sk_write_queue)) == NULL)
+ skb = skb_peek(&sk->sk_write_queue);
+ if (skb == NULL)
goto out;
offset = rp->offset;
diff --git a/net/ipv6/reassembly.c b/net/ipv6/reassembly.c
index 1a157ca..9bd8a04 100644
--- a/net/ipv6/reassembly.c
+++ b/net/ipv6/reassembly.c
@@ -429,7 +429,8 @@ static int ip6_frag_reasm(struct frag_queue *fq, struct sk_buff *prev,
struct sk_buff *clone;
int i, plen = 0;
- if ((clone = alloc_skb(0, GFP_ATOMIC)) == NULL)
+ clone = alloc_skb(0, GFP_ATOMIC);
+ if (clone == NULL)
goto out_oom;
clone->next = head->next;
head->next = clone;
diff --git a/net/ipv6/sit.c b/net/ipv6/sit.c
index 86e3fa8..e2b199f 100644
--- a/net/ipv6/sit.c
+++ b/net/ipv6/sit.c
@@ -1229,7 +1229,8 @@ ipip6_tunnel_ioctl(struct net_device *dev, struct ifreq *ifr, int cmd)
if (copy_from_user(&p, ifr->ifr_ifru.ifru_data, sizeof(p)))
goto done;
err = -ENOENT;
- if ((t = ipip6_tunnel_locate(net, &p, 0)) == NULL)
+ t = ipip6_tunnel_locate(net, &p, 0);
+ if (t == NULL)
goto done;
err = -EPERM;
if (t == netdev_priv(sitn->fb_tunnel_dev))
@@ -1748,8 +1749,8 @@ static int __net_init sit_init_net(struct net *net)
goto err_dev_free;
ipip6_tunnel_clone_6rd(sitn->fb_tunnel_dev, sitn);
-
- if ((err = register_netdev(sitn->fb_tunnel_dev)))
+ err = register_netdev(sitn->fb_tunnel_dev);
+ if (err)
goto err_reg_dev;
t = netdev_priv(sitn->fb_tunnel_dev);
diff --git a/net/ipv6/udp.c b/net/ipv6/udp.c
index 12fcce8f..e06d460 100644
--- a/net/ipv6/udp.c
+++ b/net/ipv6/udp.c
@@ -357,7 +357,8 @@ static struct sock *__udp6_lib_lookup_skb(struct sk_buff *skb,
struct sock *sk;
const struct ipv6hdr *iph = ipv6_hdr(skb);
- if (unlikely(sk = skb_steal_sock(skb)))
+ sk = skb_steal_sock(skb);
+ if (unlikely(sk))
return sk;
return __udp6_lib_lookup(dev_net(skb_dst(skb)->dev), &iph->saddr, sport,
&iph->daddr, dport, inet6_iif(skb),
@@ -1021,7 +1022,8 @@ static int udp_v6_push_pending_frames(struct sock *sk)
fl6 = &inet->cork.fl.u.ip6;
/* Grab the skbuff where UDP header space exists. */
- if ((skb = skb_peek(&sk->sk_write_queue)) == NULL)
+ skb = skb_peek(&sk->sk_write_queue);
+ if (skb == NULL)
goto out;
/*
diff --git a/net/ipv6/xfrm6_protocol.c b/net/ipv6/xfrm6_protocol.c
index 54d13f8..f200927 100644
--- a/net/ipv6/xfrm6_protocol.c
+++ b/net/ipv6/xfrm6_protocol.c
@@ -55,9 +55,11 @@ int xfrm6_rcv_cb(struct sk_buff *skb, u8 protocol, int err)
if (!head)
return 0;
- for_each_protocol_rcu(*proto_handlers(protocol), handler)
- if ((ret = handler->cb_handler(skb, err)) <= 0)
+ for_each_protocol_rcu(*proto_handlers(protocol), handler) {
+ ret = handler->cb_handler(skb, err);
+ if (ret <= 0)
return ret;
+ }
return 0;
}
@@ -70,9 +72,11 @@ static int xfrm6_esp_rcv(struct sk_buff *skb)
XFRM_TUNNEL_SKB_CB(skb)->tunnel.ip6 = NULL;
- for_each_protocol_rcu(esp6_handlers, handler)
- if ((ret = handler->handler(skb)) != -EINVAL)
+ for_each_protocol_rcu(esp6_handlers, handler) {
+ ret = handler->handler(skb);
+ if (ret != -EINVAL)
return ret;
+ }
icmpv6_send(skb, ICMPV6_DEST_UNREACH, ICMPV6_PORT_UNREACH, 0);
@@ -97,9 +101,11 @@ static int xfrm6_ah_rcv(struct sk_buff *skb)
XFRM_TUNNEL_SKB_CB(skb)->tunnel.ip6 = NULL;
- for_each_protocol_rcu(ah6_handlers, handler)
- if ((ret = handler->handler(skb)) != -EINVAL)
+ for_each_protocol_rcu(ah6_handlers, handler) {
+ ret = handler->handler(skb);
+ if (ret != -EINVAL)
return ret;
+ }
icmpv6_send(skb, ICMPV6_DEST_UNREACH, ICMPV6_PORT_UNREACH, 0);
@@ -124,10 +130,11 @@ static int xfrm6_ipcomp_rcv(struct sk_buff *skb)
XFRM_TUNNEL_SKB_CB(skb)->tunnel.ip6 = NULL;
- for_each_protocol_rcu(ipcomp6_handlers, handler)
- if ((ret = handler->handler(skb)) != -EINVAL)
+ for_each_protocol_rcu(ipcomp6_handlers, handler) {
+ ret = handler->handler(skb);
+ if (ret != -EINVAL)
return ret;
-
+ }
icmpv6_send(skb, ICMPV6_DEST_UNREACH, ICMPV6_PORT_UNREACH, 0);
kfree_skb(skb);
--
1.7.9.5
^ permalink raw reply related
* [PATCH net-next 0/4] ipv6: coding style improvements
From: Ian Morris @ 2014-09-02 19:16 UTC (permalink / raw)
To: netdev; +Cc: Ian Morris
This sequences of patches makes adjustments to the ipv6 code to try to better
follow the kernel coding style guidelines. No changes to the resultant object
code is detected by objdiff.
Ian Morris (4):
ipv6: coding style - no assignment in if statements
ipv6: coding style - min to min_t conversion
ipv6: coding style - convert printk
ipv6: coding style - cleanse bracing
net/ipv6/addrconf.c | 30 ++++++++++++++++------------
net/ipv6/addrconf_core.c | 3 +--
net/ipv6/ah6.c | 7 ++++---
net/ipv6/datagram.c | 3 ++-
net/ipv6/esp6.c | 3 ++-
net/ipv6/exthdrs_core.c | 6 +++---
net/ipv6/icmp.c | 3 ++-
net/ipv6/ip6_fib.c | 4 ++--
net/ipv6/ip6_flowlabel.c | 9 ++++++---
net/ipv6/ip6_gre.c | 4 ++--
net/ipv6/ip6_input.c | 16 +++++++--------
net/ipv6/ip6_output.c | 32 ++++++++++++++++--------------
net/ipv6/ip6_tunnel.c | 22 ++++++++++-----------
net/ipv6/ip6_vti.c | 4 ++--
net/ipv6/ip6mr.c | 13 ++++++------
net/ipv6/ipv6_sockglue.c | 7 ++++---
net/ipv6/mcast.c | 48 ++++++++++++++++++++++++++-------------------
net/ipv6/ndisc.c | 25 ++++++++++-------------
net/ipv6/ping.c | 8 ++++----
net/ipv6/raw.c | 3 ++-
net/ipv6/reassembly.c | 3 ++-
net/ipv6/route.c | 25 +++++++++++------------
net/ipv6/sit.c | 7 ++++---
net/ipv6/tcp_ipv6.c | 10 ++++++----
net/ipv6/udp.c | 26 ++++++++++++------------
net/ipv6/xfrm6_output.c | 4 ++--
net/ipv6/xfrm6_protocol.c | 25 ++++++++++++++---------
net/ipv6/xfrm6_tunnel.c | 3 ++-
28 files changed, 191 insertions(+), 162 deletions(-)
--
1.7.9.5
^ permalink raw reply
* Re: RTNL: assertion failed at net/ipv6/addrconf.c (1699)
From: Vlad Yasevich @ 2014-09-02 19:08 UTC (permalink / raw)
To: Cong Wang, Eric Dumazet
Cc: Hannes Frederic Sowa, Sabrina Dubroca, Tommi Rantala,
David S. Miller, Alexey Kuznetsov, James Morris,
Hideaki YOSHIFUJI, Patrick McHardy, netdev, LKML, trinity,
Dave Jones
In-Reply-To: <CAHA+R7PY=Pu7VKA3wp1DxeJRckxPNWL-xfv2XXkpaY935kt61A@mail.gmail.com>
On 09/02/2014 02:15 PM, Cong Wang wrote:
> On Tue, Sep 2, 2014 at 11:11 AM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
>> On Tue, 2014-09-02 at 11:04 -0700, Cong Wang wrote:
>>> On Tue, Sep 2, 2014 at 10:58 AM, Hannes Frederic Sowa
>>
>>>> I definitely don't have a problem cleaning this up in net-next. I wanted
>>>> a minimal patch for stable because I didn't check history where and when
>>>> additional users of dev_get_by_flags_rcu were removed.
>>>
>>> `git grep` should show you we only have one caller. Apparently we don't
>>> care about any out-of-tree module.
>>
>> Point is : you did not check if some stable versions had more callers.
>>
>> Its very nice you checked current version, but it is not enough for a
>> stable candidate.
>
> That is what we do when backporting patches, I can do that if David asks
> me to backport it, but you know for netdev that is David's work.
>
> (I am not saying I don't want to help him, I just want to point out the fact.
> I am very pleased to help David for stable backports as long as he asks)
Instead of helping after the fact, why not arrange the patches so that it's
not such a big issue. Leave the _rcu variant alone. Add an _rtnl variant
of the function and use that in the patch. Have a follow-on patch that
removes the _rcu variant all by itself. This way backports become easier,
and if anyone wants the _rcu variant back, all they have to do is revert
a very simple commit.
-vlad
^ permalink raw reply
* Re: RTNL: assertion failed at net/ipv6/addrconf.c (1699)
From: Hannes Frederic Sowa @ 2014-09-02 19:02 UTC (permalink / raw)
To: Cong Wang
Cc: Sabrina Dubroca, Tommi Rantala, David S. Miller, Alexey Kuznetsov,
James Morris, Hideaki YOSHIFUJI, Patrick McHardy, netdev, LKML,
trinity, Dave Jones
In-Reply-To: <CAHA+R7MMe+O9-sNHQvgOO78tBzDy+hO+GtJFsuJPKx0eG6eMaw@mail.gmail.com>
On Di, 2014-09-02 at 11:40 -0700, Cong Wang wrote:
> On Tue, Sep 2, 2014 at 11:18 AM, Hannes Frederic Sowa
> <hannes@stressinduktion.org> wrote:
> > Those ASSERT_RTNLs were misplaced and only caught the callers mostly
> > from addrconf.c. I don't mind getting reports from stable kernel users
> > and fixing those, too (or help fixing those). ASSERT_RTNL is not
> > dangerous.
> >
> > We had a long history in not correctly using rtnl lock in ipv6/multicast
> > code and those wrongfully placed ASSERT_RTNLs were my bad when I fixed
> > the duplicate address detection handling.
> >
> > If enough multicast addresses are subscribed to an interface we might
> > again get those splats because enabling promisc mode on an interface
> > will also check for rtnl lock.
> >
>
> Sure, I never doubt adding ASSERT_RTNL() is helpful, I just still think
> this should be for net-next, or at least a separated patch. I don't want
> my patch to be blamed in others' "Fixes:". :)
Come on, that's why we have community review. Nobody blames anyone
because of added regressions. It's more a fault of the community then,
and it works out fairly good I think! Even others are keen on fixing
your bugs sometimes. ;)
If fixes tag is well researched, it won't point to the addition of
ASSERT_RTNL() but your patch would help to discover a bug somewhere else
in the stack.
I think for this patch a fixes-tag is hard to find because it is hard to
find because it dates back to the beginning of the git history IMHO.
Bye,
Hannes
^ permalink raw reply
* Re: [net PATCH 1/1] drivers: net: cpsw: dual_emac: fix reducing of rx descriptor during ifdown
From: David Miller @ 2014-09-02 18:54 UTC (permalink / raw)
To: mugunthanvnm; +Cc: netdev
In-Reply-To: <54058C01.3010402@ti.com>
From: Mugunthan V N <mugunthanvnm@ti.com>
Date: Tue, 2 Sep 2014 14:51:05 +0530
> When ifup and ifdown is run continuously, for each spilled packet (for
> interface which is down) from DMA, the total number of rx descriptor
> goes down and at one instance all the descriptor is lost and both the
> interface stops working.
>
> To recover from this we need to put down both the interface and open the
> interface which will re-init the DMA which intern queues fresh set of
> skbs for rx.
But you still should not receive packets for a netdev which is down.
As far as I can tell, you're feeding it into the stack still.
Also this doesn't explain why the "status < 0" case applies to this
new logic, you have not explained that at all.
^ permalink raw reply
* Re: [RFC PATCH] netlink: Safer deletion of sk_bind_node
From: David Miller @ 2014-09-02 18:52 UTC (permalink / raw)
To: harish_kandiga
Cc: dborkman, tgraf, ebiederm, darkjames-ws, rgb, eric.dumazet,
stephen, netdev, linux-kernel
In-Reply-To: <54058376.9090700@mentor.com>
From: Harish Jenny Kandiga Nagaraj <harish_kandiga@mentor.com>
Date: Tue, 2 Sep 2014 14:14:38 +0530
> In one of our random test runs we observed the crash mentioned in the previous mail.
>
> After debugging we found out that the call flow of the inline and static functions were
> netlink_release
> -----netlink_remove
> ---------__sk_del_bind_node
> --------------__hlist_del
>
> *pprev was NULL in __hlist_del function while deleting &sk->sk_bind_node hlist_node. Hence the patch was given.
>
> In netlink_remove function , first the sk_del_node_init function will be called. This internally calls __sk_del_node_init function. While deleting &sk->sk_node hlist_node using __sk_del_node function there is a NULL check with sk_hashed function.
>
> Why there is no NULL check for *pprev while deleting &sk->sk_bind_node ?
Because if ->subscriptions is non-zero, it must be on a list, and therefore
pprev must be non-NULL.
^ permalink raw reply
* Re: [PATCH net-next v3 2/2] ipv4: implement igmp_qrv sysctl to tune igmp robustness variable
From: Flavio Leitner @ 2014-09-02 18:51 UTC (permalink / raw)
To: Hannes Frederic Sowa; +Cc: netdev
In-Reply-To: <b24ad902c532616c7683565df3a0fcc9d925da9c.1409665378.git.hannes@stressinduktion.org>
On Tue, Sep 02, 2014 at 03:49:26PM +0200, Hannes Frederic Sowa wrote:
> As in IPv6 people might increase the igmp query robustness variable to
> make sure unsolicited state change reports aren't lost on the network. Add
> and document this new knob to igmp code.
>
> RFCs allow tuning this parameter back to first IGMP RFC, so we also use
> this setting for all counters, including source specific multicast.
>
> Also take over sysctl value when upping the interface and don't reuse
> the last one seen on the interface.
>
> Cc: Flavio Leitner <fbl@redhat.com>
> Signed-off-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
> ---
Thanks again,
Acked-by: Flavio Leitner <fbl@redhat.com>
^ permalink raw reply
* Re: [PATCH net-next v3 1/2] ipv6: add sysctl_mld_qrv to configure query robustness variable
From: Flavio Leitner @ 2014-09-02 18:50 UTC (permalink / raw)
To: Hannes Frederic Sowa; +Cc: netdev
In-Reply-To: <3f38fb1c0c5544de4cea8bb13e0adcbba17bebd3.1409665378.git.hannes@stressinduktion.org>
On Tue, Sep 02, 2014 at 03:49:25PM +0200, Hannes Frederic Sowa wrote:
> This patch adds a new sysctl_mld_qrv knob to configure the mldv1/v2 query
> robustness variable. It specifies how many retransmit of unsolicited mld
> retransmit should happen. Admins might want to tune this on lossy links.
>
> Also reset mld state on interface down/up, so we pick up new sysctl
> settings during interface up event.
>
> IPv6 certification requests this knob to be available.
>
> I didn't make this knob netns specific, as it is mostly a setting in a
> physical environment and should be per host.
>
> Cc: Flavio Leitner <fbl@redhat.com>
> Signed-off-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
> ---
Thanks very much Hannes!
Acked-by: Flavio Leitner <fbl@redhat.com>
^ permalink raw reply
* Re: spam anyone?
From: David Miller @ 2014-09-02 18:49 UTC (permalink / raw)
To: xiaodi; +Cc: netdev
In-Reply-To: <54056E17.10606@sjtu.edu.cn>
From: Larry Xiao <xiaodi@sjtu.edu.cn>
Date: Tue, 02 Sep 2014 15:13:27 +0800
> I didn't see anyone complain about spam before.
> So I don't know if it's only me receiving many spams like this one.
>
> (I don't know if this email will go through .. )
It happens, the best I can do is continue teaching bogofilter about
it.
^ permalink raw reply
* Re: RTNL: assertion failed at net/ipv6/addrconf.c (1699)
From: Cong Wang @ 2014-09-02 18:40 UTC (permalink / raw)
To: Hannes Frederic Sowa
Cc: Sabrina Dubroca, Tommi Rantala, David S. Miller, Alexey Kuznetsov,
James Morris, Hideaki YOSHIFUJI, Patrick McHardy, netdev, LKML,
trinity, Dave Jones
In-Reply-To: <1409681914.978561.162800349.5A7FA784@webmail.messagingengine.com>
On Tue, Sep 2, 2014 at 11:18 AM, Hannes Frederic Sowa
<hannes@stressinduktion.org> wrote:
> Those ASSERT_RTNLs were misplaced and only caught the callers mostly
> from addrconf.c. I don't mind getting reports from stable kernel users
> and fixing those, too (or help fixing those). ASSERT_RTNL is not
> dangerous.
>
> We had a long history in not correctly using rtnl lock in ipv6/multicast
> code and those wrongfully placed ASSERT_RTNLs were my bad when I fixed
> the duplicate address detection handling.
>
> If enough multicast addresses are subscribed to an interface we might
> again get those splats because enabling promisc mode on an interface
> will also check for rtnl lock.
>
Sure, I never doubt adding ASSERT_RTNL() is helpful, I just still think
this should be for net-next, or at least a separated patch. I don't want
my patch to be blamed in others' "Fixes:". :)
^ permalink raw reply
* Re: RTNL: assertion failed at net/ipv6/addrconf.c (1699)
From: Cong Wang @ 2014-09-02 18:37 UTC (permalink / raw)
To: Eric Dumazet
Cc: Hannes Frederic Sowa, Sabrina Dubroca, Tommi Rantala,
David S. Miller, Alexey Kuznetsov, James Morris,
Hideaki YOSHIFUJI, Patrick McHardy, netdev, LKML, trinity,
Dave Jones
In-Reply-To: <1409682093.26422.5.camel@edumazet-glaptop2.roam.corp.google.com>
On Tue, Sep 2, 2014 at 11:21 AM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> On Tue, 2014-09-02 at 11:15 -0700, Cong Wang wrote:
>
>> That is what we do when backporting patches, I can do that if David asks
>> me to backport it, but you know for netdev that is David's work.
>>
>> (I am not saying I don't want to help him, I just want to point out the fact.
>> I am very pleased to help David for stable backports as long as he asks)
>
> Problem is : your patch submission do not identify bug origin.
>
> You claim you want to help, but you do not provide the basic thing that
> _really_ helps.
>
> The proper way to identify bug origin is to add in the headers one
> line :
>
> Fixes: 12-digit-SHA1 ("patch title")
>
Since when "Fixes:" tag becomes mandatory for a stable patch?
At least netdev-FAQ is not updated. ;-) I 100% agree "Fixes:"
is helpful when backporting patches, but it is not mandatory currently.
For this patch, I was too lazy to dig the history, it looks like this is
caused by the following commit:
commit c15b1ccadb323ea50023e8f1cca2954129a62b51
Author: Hannes Frederic Sowa <hannes@stressinduktion.org>
Date: Thu Mar 27 18:28:07 2014 +0100
ipv6: move DAD and addrconf_verify processing to workqueue
Thanks.
^ permalink raw reply
* Re: [PATCH net] core: Don't attempt to load the "" driver.
From: Cong Wang @ 2014-09-02 18:25 UTC (permalink / raw)
To: David Laight; +Cc: Linux Netdev List, David Miller
In-Reply-To: <063D6719AE5E284EB5DD2968C1650D6D174856B9@AcuExch.aculab.com>
On Tue, Sep 2, 2014 at 6:48 AM, David Laight <David.Laight@aculab.com> wrote:
> While the applications shouldn't be calling an SIOCxxx ioctl with ifr_name[0] == 0
> the kernel shouldn't be tracing the error either.
>
Why don't we reject this empty string? It doesn't look like a valid one.
I assume this is for compatibility?
^ permalink raw reply
* Re: RTNL: assertion failed at net/ipv6/addrconf.c (1699)
From: Eric Dumazet @ 2014-09-02 18:21 UTC (permalink / raw)
To: Cong Wang
Cc: Hannes Frederic Sowa, Sabrina Dubroca, Tommi Rantala,
David S. Miller, Alexey Kuznetsov, James Morris,
Hideaki YOSHIFUJI, Patrick McHardy, netdev, LKML, trinity,
Dave Jones
In-Reply-To: <CAHA+R7PY=Pu7VKA3wp1DxeJRckxPNWL-xfv2XXkpaY935kt61A@mail.gmail.com>
On Tue, 2014-09-02 at 11:15 -0700, Cong Wang wrote:
> That is what we do when backporting patches, I can do that if David asks
> me to backport it, but you know for netdev that is David's work.
>
> (I am not saying I don't want to help him, I just want to point out the fact.
> I am very pleased to help David for stable backports as long as he asks)
Problem is : your patch submission do not identify bug origin.
You claim you want to help, but you do not provide the basic thing that
_really_ helps.
The proper way to identify bug origin is to add in the headers one
line :
Fixes: 12-digit-SHA1 ("patch title")
^ permalink raw reply
* Re: RTNL: assertion failed at net/ipv6/addrconf.c (1699)
From: Hannes Frederic Sowa @ 2014-09-02 18:18 UTC (permalink / raw)
To: Cong Wang
Cc: Sabrina Dubroca, Tommi Rantala, David S. Miller, Alexey Kuznetsov,
James Morris, Hideaki YOSHIFUJI, Patrick McHardy, netdev, LKML,
trinity, Dave Jones
In-Reply-To: <CAHA+R7Mwy1SCpEUgGf9BdLEro6N-3t89KU=xL0iuzgOppXELrQ@mail.gmail.com>
On Tue, Sep 2, 2014, at 20:04, Cong Wang wrote:
> On Tue, Sep 2, 2014 at 10:58 AM, Hannes Frederic Sowa
> <hannes@stressinduktion.org> wrote:
> > Hi Cong,
> >
> > On Tue, Sep 2, 2014, at 18:50, Cong Wang wrote:
> >> On Fri, Aug 29, 2014 at 6:51 PM, Hannes Frederic Sowa
> >> <hannes@stressinduktion.org> wrote:
> >> >
> >> > Also rtnl_lock and rcu_read_lock compose in that order, so we don't need
> >> > to change dev_get_by_flags, but as this is the only user it sure is
> >> > possible. RCU locked version is just easier composeable, so I wouldn't
> >> > touch that if needed in future, just also take rcu lock as before.
> >>
> >> There is no point to keep RCU read lock if we have rtnl lock,
> >> I don't know why you don't want to change dev_get_by_flags(),
> >> it is pretty easy to do since it only has one caller.
> >
> > I definitely don't have a problem cleaning this up in net-next. I wanted
> > a minimal patch for stable because I didn't check history where and when
> > additional users of dev_get_by_flags_rcu were removed.
>
> `git grep` should show you we only have one caller. Apparently we don't
> care about any out-of-tree module.
Sure, I don't care about out-of-tree modules either. I just wanted to
make it easier to backport. Current patch is almost headache free to
backport.
> >> Even if you really need RCU in future, you are always welcome
> >> to bring it back when you do, sorry we should never be blocked by
> >> code NOT merged yet.
> >>
> >> >
> >> > Also we should move ASSERT_RTNL checks from addrconf_join_solict to
> >> > ipv6_dev_mc_inc/dec.
> >> >
> >>
> >> Make it another patch.
> >
> > It is just one logical change, moving ASSERT_RTNLs to places where they
> > better catch invalid callstacks.
> >
>
> Conflicts with what you claimed above. :)
Those ASSERT_RTNLs were misplaced and only caught the callers mostly
from addrconf.c. I don't mind getting reports from stable kernel users
and fixing those, too (or help fixing those). ASSERT_RTNL is not
dangerous.
We had a long history in not correctly using rtnl lock in ipv6/multicast
code and those wrongfully placed ASSERT_RTNLs were my bad when I fixed
the duplicate address detection handling.
If enough multicast addresses are subscribed to an interface we might
again get those splats because enabling promisc mode on an interface
will also check for rtnl lock.
Bye,
Hannes
^ permalink raw reply
* [PATCH net-next] net: systemport: update UMAC_CMD only when link is detected
From: Florian Fainelli @ 2014-09-02 18:17 UTC (permalink / raw)
To: netdev; +Cc: davem, Florian Fainelli
When we bring the interface down, phy_stop() will schedule the PHY
state machine to call our link adjustment callback. By the time we do so,
we may have clock gated off the SYSTEMPORT hardware block, and this will
cause bus errors to happen in bcm_sysport_adj_link():
Make sure that we only touch the UMAC_CMD register when there is an
actual link. This is safe to do for two reasons:
- updating the Ethernet MAC registers only make sense when a physical
link is present
- the PHY library state machine first set phydev->link = 0 before
invoking phydev->adjust_link in the PHY_HALTED case
This is a similar fix to the GENET one:
c677ba8b3c47650358572091ed8a6af50bfca877 ("net: bcmgenet: update
UMAC_CMD only when link is detected").
Fixes: 80105befdb4b ("net: systemport: add Broadcom SYSTEMPORT Ethernet MAC driver")
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
drivers/net/ethernet/broadcom/bcmsysport.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/broadcom/bcmsysport.c b/drivers/net/ethernet/broadcom/bcmsysport.c
index 8f91de169663..662cf2222873 100644
--- a/drivers/net/ethernet/broadcom/bcmsysport.c
+++ b/drivers/net/ethernet/broadcom/bcmsysport.c
@@ -1071,16 +1071,19 @@ static void bcm_sysport_adj_link(struct net_device *dev)
if (!phydev->pause)
cmd_bits |= CMD_RX_PAUSE_IGNORE | CMD_TX_PAUSE_IGNORE;
- if (changed) {
+ if (!changed)
+ return;
+
+ if (phydev->link) {
reg = umac_readl(priv, UMAC_CMD);
reg &= ~((CMD_SPEED_MASK << CMD_SPEED_SHIFT) |
CMD_HD_EN | CMD_RX_PAUSE_IGNORE |
CMD_TX_PAUSE_IGNORE);
reg |= cmd_bits;
umac_writel(priv, reg, UMAC_CMD);
-
- phy_print_status(priv->phydev);
}
+
+ phy_print_status(priv->phydev);
}
static int bcm_sysport_init_tx_ring(struct bcm_sysport_priv *priv,
--
1.9.1
^ permalink raw reply related
* Re: RTNL: assertion failed at net/ipv6/addrconf.c (1699)
From: Cong Wang @ 2014-09-02 18:15 UTC (permalink / raw)
To: Eric Dumazet
Cc: Hannes Frederic Sowa, Sabrina Dubroca, Tommi Rantala,
David S. Miller, Alexey Kuznetsov, James Morris,
Hideaki YOSHIFUJI, Patrick McHardy, netdev, LKML, trinity,
Dave Jones
In-Reply-To: <1409681487.26422.1.camel@edumazet-glaptop2.roam.corp.google.com>
On Tue, Sep 2, 2014 at 11:11 AM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> On Tue, 2014-09-02 at 11:04 -0700, Cong Wang wrote:
>> On Tue, Sep 2, 2014 at 10:58 AM, Hannes Frederic Sowa
>
>> > I definitely don't have a problem cleaning this up in net-next. I wanted
>> > a minimal patch for stable because I didn't check history where and when
>> > additional users of dev_get_by_flags_rcu were removed.
>>
>> `git grep` should show you we only have one caller. Apparently we don't
>> care about any out-of-tree module.
>
> Point is : you did not check if some stable versions had more callers.
>
> Its very nice you checked current version, but it is not enough for a
> stable candidate.
That is what we do when backporting patches, I can do that if David asks
me to backport it, but you know for netdev that is David's work.
(I am not saying I don't want to help him, I just want to point out the fact.
I am very pleased to help David for stable backports as long as he asks)
^ permalink raw reply
* Re: RTNL: assertion failed at net/ipv6/addrconf.c (1699)
From: Eric Dumazet @ 2014-09-02 18:11 UTC (permalink / raw)
To: Cong Wang
Cc: Hannes Frederic Sowa, Sabrina Dubroca, Tommi Rantala,
David S. Miller, Alexey Kuznetsov, James Morris,
Hideaki YOSHIFUJI, Patrick McHardy, netdev, LKML, trinity,
Dave Jones
In-Reply-To: <CAHA+R7Mwy1SCpEUgGf9BdLEro6N-3t89KU=xL0iuzgOppXELrQ@mail.gmail.com>
On Tue, 2014-09-02 at 11:04 -0700, Cong Wang wrote:
> On Tue, Sep 2, 2014 at 10:58 AM, Hannes Frederic Sowa
> > I definitely don't have a problem cleaning this up in net-next. I wanted
> > a minimal patch for stable because I didn't check history where and when
> > additional users of dev_get_by_flags_rcu were removed.
>
> `git grep` should show you we only have one caller. Apparently we don't
> care about any out-of-tree module.
Point is : you did not check if some stable versions had more callers.
Its very nice you checked current version, but it is not enough for a
stable candidate.
^ permalink raw reply
* [Patch net] ipv6: fix rtnl lock assertion failure in ipv6_sock_ac_join()
From: Cong Wang @ 2014-09-02 18:07 UTC (permalink / raw)
To: netdev; +Cc: Cong Wang, David S. Miller, Hannes Frederic Sowa, Sabrina Dubroca
Tommi reported the following RTNL lock assertion failure:
[ 77.297196] RTNL: assertion failed at net/ipv6/addrconf.c (1699)
[ 77.298080] CPU: 0 PID: 4842 Comm: trinity-main Not tainted 3.17.0-rc2+ #30
[ 77.299039] Hardware name: Bochs Bochs, BIOS Bochs 01/01/2011
[ 77.299789] ffff88003d76a618 ffff880026133c50 ffffffff8238ba79
ffff880037c84520
[ 77.300829] ffff880026133c90 ffffffff820bd52b 0000000000000000
ffffffff82d86c40
[ 77.301869] 0000000000000000 00000000f76fd1e1 ffff8800382d8000
ffff8800382d8220
[ 77.302906] Call Trace:
[ 77.303246] [<ffffffff8238ba79>] dump_stack+0x4d/0x66
[ 77.303928] [<ffffffff820bd52b>] addrconf_join_solict+0x4b/0xb0
[ 77.304731] [<ffffffff820b031b>] ipv6_dev_ac_inc+0x2bb/0x330
[ 77.305498] [<ffffffff820b0060>] ? ac6_seq_start+0x260/0x260
[ 77.306257] [<ffffffff820b05fe>] ipv6_sock_ac_join+0x26e/0x360
[ 77.307046] [<ffffffff820b0429>] ? ipv6_sock_ac_join+0x99/0x360
[ 77.307798] [<ffffffff820cdd60>] do_ipv6_setsockopt.isra.5+0xa70/0xf20
This is due to we don't hold rtnl lock when calling addrconf_join_solict()
in ipv6_sock_ac_join(). So hold rtnl lock instead of RCU lock here,
after all it is not a hot path.
BTW, mcast _might_ have similar problem, but I don't touch it since no one
reports so far.
Reported-by: Tommi Rantala <tt.rantala@gmail.com>
Tested-by: Sabrina Dubroca <sd@queasysnail.net>
Cc: David S. Miller <davem@davemloft.net>
Cc: Hannes Frederic Sowa <hannes@stressinduktion.org>
Signed-off-by: Sabrina Dubroca <sd@queasysnail.net>
Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
---
include/linux/netdevice.h | 4 ++--
net/core/dev.c | 13 +++++++------
net/ipv6/anycast.c | 22 +++++++++++-----------
3 files changed, 20 insertions(+), 19 deletions(-)
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 38377392..71838bb 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -2074,8 +2074,8 @@ void __dev_remove_pack(struct packet_type *pt);
void dev_add_offload(struct packet_offload *po);
void dev_remove_offload(struct packet_offload *po);
-struct net_device *dev_get_by_flags_rcu(struct net *net, unsigned short flags,
- unsigned short mask);
+struct net_device *__dev_get_by_flags(struct net *net, unsigned short flags,
+ unsigned short mask);
struct net_device *dev_get_by_name(struct net *net, const char *name);
struct net_device *dev_get_by_name_rcu(struct net *net, const char *name);
struct net_device *__dev_get_by_name(struct net *net, const char *name);
diff --git a/net/core/dev.c b/net/core/dev.c
index ab9a165..343847a 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -897,23 +897,24 @@ struct net_device *dev_getfirstbyhwtype(struct net *net, unsigned short type)
EXPORT_SYMBOL(dev_getfirstbyhwtype);
/**
- * dev_get_by_flags_rcu - find any device with given flags
+ * __dev_get_by_flags - find any device with given flags
* @net: the applicable net namespace
* @if_flags: IFF_* values
* @mask: bitmask of bits in if_flags to check
*
* Search for any interface with the given flags. Returns NULL if a device
* is not found or a pointer to the device. Must be called inside
- * rcu_read_lock(), and result refcount is unchanged.
+ * rtnl_lock(), and result refcount is unchanged.
*/
-struct net_device *dev_get_by_flags_rcu(struct net *net, unsigned short if_flags,
- unsigned short mask)
+struct net_device *__dev_get_by_flags(struct net *net, unsigned short if_flags,
+ unsigned short mask)
{
struct net_device *dev, *ret;
+ ASSERT_RTNL();
ret = NULL;
- for_each_netdev_rcu(net, dev) {
+ for_each_netdev(net, dev) {
if (((dev->flags ^ if_flags) & mask) == 0) {
ret = dev;
break;
@@ -921,7 +922,7 @@ struct net_device *dev_get_by_flags_rcu(struct net *net, unsigned short if_flags
}
return ret;
}
-EXPORT_SYMBOL(dev_get_by_flags_rcu);
+EXPORT_SYMBOL(__dev_get_by_flags);
/**
* dev_valid_name - check if name is okay for network device
diff --git a/net/ipv6/anycast.c b/net/ipv6/anycast.c
index 2101832..484ed23 100644
--- a/net/ipv6/anycast.c
+++ b/net/ipv6/anycast.c
@@ -77,7 +77,7 @@ int ipv6_sock_ac_join(struct sock *sk, int ifindex, const struct in6_addr *addr)
pac->acl_next = NULL;
pac->acl_addr = *addr;
- rcu_read_lock();
+ rtnl_lock();
if (ifindex == 0) {
struct rt6_info *rt;
@@ -90,11 +90,11 @@ int ipv6_sock_ac_join(struct sock *sk, int ifindex, const struct in6_addr *addr)
goto error;
} else {
/* router, no matching interface: just pick one */
- dev = dev_get_by_flags_rcu(net, IFF_UP,
- IFF_UP | IFF_LOOPBACK);
+ dev = __dev_get_by_flags(net, IFF_UP,
+ IFF_UP | IFF_LOOPBACK);
}
} else
- dev = dev_get_by_index_rcu(net, ifindex);
+ dev = __dev_get_by_index(net, ifindex);
if (dev == NULL) {
err = -ENODEV;
@@ -136,7 +136,7 @@ int ipv6_sock_ac_join(struct sock *sk, int ifindex, const struct in6_addr *addr)
}
error:
- rcu_read_unlock();
+ rtnl_unlock();
if (pac)
sock_kfree_s(sk, pac, sizeof(*pac));
return err;
@@ -171,11 +171,11 @@ int ipv6_sock_ac_drop(struct sock *sk, int ifindex, const struct in6_addr *addr)
spin_unlock_bh(&ipv6_sk_ac_lock);
- rcu_read_lock();
- dev = dev_get_by_index_rcu(net, pac->acl_ifindex);
+ rtnl_lock();
+ dev = __dev_get_by_index(net, pac->acl_ifindex);
if (dev)
ipv6_dev_ac_dec(dev, &pac->acl_addr);
- rcu_read_unlock();
+ rtnl_unlock();
sock_kfree_s(sk, pac, sizeof(*pac));
return 0;
@@ -198,7 +198,7 @@ void ipv6_sock_ac_close(struct sock *sk)
spin_unlock_bh(&ipv6_sk_ac_lock);
prev_index = 0;
- rcu_read_lock();
+ rtnl_lock();
while (pac) {
struct ipv6_ac_socklist *next = pac->acl_next;
@@ -211,7 +211,7 @@ void ipv6_sock_ac_close(struct sock *sk)
sock_kfree_s(sk, pac, sizeof(*pac));
pac = next;
}
- rcu_read_unlock();
+ rtnl_unlock();
}
static void aca_put(struct ifacaddr6 *ac)
@@ -331,7 +331,7 @@ int __ipv6_dev_ac_dec(struct inet6_dev *idev, const struct in6_addr *addr)
return 0;
}
-/* called with rcu_read_lock() */
+/* called with rtnl_lock() */
static int ipv6_dev_ac_dec(struct net_device *dev, const struct in6_addr *addr)
{
struct inet6_dev *idev = __in6_dev_get(dev);
--
1.8.3.1
^ permalink raw reply related
* Re: RTNL: assertion failed at net/ipv6/addrconf.c (1699)
From: Cong Wang @ 2014-09-02 18:04 UTC (permalink / raw)
To: Hannes Frederic Sowa
Cc: Sabrina Dubroca, Tommi Rantala, David S. Miller, Alexey Kuznetsov,
James Morris, Hideaki YOSHIFUJI, Patrick McHardy, netdev, LKML,
trinity, Dave Jones
In-Reply-To: <1409680684.972417.162793869.03CF8A61@webmail.messagingengine.com>
On Tue, Sep 2, 2014 at 10:58 AM, Hannes Frederic Sowa
<hannes@stressinduktion.org> wrote:
> Hi Cong,
>
> On Tue, Sep 2, 2014, at 18:50, Cong Wang wrote:
>> On Fri, Aug 29, 2014 at 6:51 PM, Hannes Frederic Sowa
>> <hannes@stressinduktion.org> wrote:
>> >
>> > Also rtnl_lock and rcu_read_lock compose in that order, so we don't need
>> > to change dev_get_by_flags, but as this is the only user it sure is
>> > possible. RCU locked version is just easier composeable, so I wouldn't
>> > touch that if needed in future, just also take rcu lock as before.
>>
>> There is no point to keep RCU read lock if we have rtnl lock,
>> I don't know why you don't want to change dev_get_by_flags(),
>> it is pretty easy to do since it only has one caller.
>
> I definitely don't have a problem cleaning this up in net-next. I wanted
> a minimal patch for stable because I didn't check history where and when
> additional users of dev_get_by_flags_rcu were removed.
`git grep` should show you we only have one caller. Apparently we don't
care about any out-of-tree module.
>
>> Even if you really need RCU in future, you are always welcome
>> to bring it back when you do, sorry we should never be blocked by
>> code NOT merged yet.
>>
>> >
>> > Also we should move ASSERT_RTNL checks from addrconf_join_solict to
>> > ipv6_dev_mc_inc/dec.
>> >
>>
>> Make it another patch.
>
> It is just one logical change, moving ASSERT_RTNLs to places where they
> better catch invalid callstacks.
>
Conflicts with what you claimed above. :)
^ permalink raw reply
* Re: [net-next PATCH 0/3] qdisc bulk dequeuing and utilizing delayed tailptr updates
From: Tom Herbert @ 2014-09-02 18:04 UTC (permalink / raw)
To: Jesper Dangaard Brouer
Cc: David S. Miller, Linux Netdev List, Florian Westphal,
Hannes Frederic Sowa, Daniel Borkmann
In-Reply-To: <20140902143254.1918.8419.stgit@dragon>
On Tue, Sep 2, 2014 at 7:35 AM, Jesper Dangaard Brouer
<brouer@redhat.com> wrote:
> This patchset uses DaveM's recent API changes to dev_hard_start_xmit(),
> from the qdisc layer, to implement dequeue bulking.
>
> Open questions:
>
> - For now set bulk limit to 8 packets, don't want to stress the driver
> avail ring_buffer space.
>
Please get limit from BQL also, see:
[PATCH net-next] net: Functions to report space available in device TX queues
> - We are not doing proper accounting for weight_p/quota in __qdisc_run(). Do we care?
>
> - Is the (!skb->next) check in dequeue necessary?
>
> - Do we need some checks in dev_requeue_skb() as we could be requeuing a SKB list?
>
> - Should we rename dequeue_skb() to dequeue_skb_list() ?
>
>
> Based on top of:
> commit 364a9e93243d ("sock: deduplicate errqueue dequeue")
>
> ---
>
> Jesper Dangaard Brouer (3):
> qdisc: sysctl to adjust bulk dequeue limit
> qdisc: bulk dequeue support for qdiscs with TCQ_F_ONETXQUEUE
> qdisc: adjustments for API allowing skb list xmits
>
>
> include/net/sch_generic.h | 2 ++
> net/core/sysctl_net_core.c | 9 +++++++++
> net/sched/sch_generic.c | 33 ++++++++++++++++++++++++++++-----
> 3 files changed, 39 insertions(+), 5 deletions(-)
>
> --
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: RTNL: assertion failed at net/ipv6/addrconf.c (1699)
From: Hannes Frederic Sowa @ 2014-09-02 17:58 UTC (permalink / raw)
To: Cong Wang
Cc: Sabrina Dubroca, Tommi Rantala, David S. Miller, Alexey Kuznetsov,
James Morris, Hideaki YOSHIFUJI, Patrick McHardy, netdev, LKML,
trinity, Dave Jones
In-Reply-To: <CAHA+R7ODa+LrdwSrS7HcBfwwq8_fpj-Ld0OCDtcOqkpCdZCvxg@mail.gmail.com>
Hi Cong,
On Tue, Sep 2, 2014, at 18:50, Cong Wang wrote:
> On Fri, Aug 29, 2014 at 6:51 PM, Hannes Frederic Sowa
> <hannes@stressinduktion.org> wrote:
> >
> > Also rtnl_lock and rcu_read_lock compose in that order, so we don't need
> > to change dev_get_by_flags, but as this is the only user it sure is
> > possible. RCU locked version is just easier composeable, so I wouldn't
> > touch that if needed in future, just also take rcu lock as before.
>
> There is no point to keep RCU read lock if we have rtnl lock,
> I don't know why you don't want to change dev_get_by_flags(),
> it is pretty easy to do since it only has one caller.
I definitely don't have a problem cleaning this up in net-next. I wanted
a minimal patch for stable because I didn't check history where and when
additional users of dev_get_by_flags_rcu were removed.
> Even if you really need RCU in future, you are always welcome
> to bring it back when you do, sorry we should never be blocked by
> code NOT merged yet.
>
> >
> > Also we should move ASSERT_RTNL checks from addrconf_join_solict to
> > ipv6_dev_mc_inc/dec.
> >
>
> Make it another patch.
It is just one logical change, moving ASSERT_RTNLs to places where they
better catch invalid callstacks.
Bye,
Hannes
^ permalink raw reply
* Re: [PATCH net-next] core: Simplify logic in dev_load().
From: Sergei Shtylyov @ 2014-09-02 17:49 UTC (permalink / raw)
To: David Laight, Linux Netdev List, David Miller
In-Reply-To: <063D6719AE5E284EB5DD2968C1650D6D174856D6@AcuExch.aculab.com>
Hello.
On 09/02/2014 05:51 PM, David Laight wrote:
> The code in dev_load() is convoluted.
> Return early and remove the 'no_module' variable.
You forgot to sign off on the patch, so it can't be applied.
WBR, Sergei
^ permalink raw reply
* Re: [PATCH net v2] tg3: prevent ifup/ifdown during PCI error recovery
From: Prashant Sreedharan @ 2014-09-02 17:32 UTC (permalink / raw)
To: Ivan Vecera; +Cc: netdev, Michael Chan
In-Reply-To: <1409574117-19435-1-git-send-email-ivecera@redhat.com>
On Mon, 2014-09-01 at 14:21 +0200, Ivan Vecera wrote:
> The patch fixes race conditions between PCI error recovery callbacks and
> potential ifup/ifdown.
>
> First, if ifup (tg3_open) is called between tg3_io_error_detected() and
> tg3_io_resume() then tp->timer is armed twice before expiry. Once during
> tg3_open() and again during tg3_io_resume(). This results in BUG
> at kernel/time/timer.c:945.
>
> Second, if ifdown (tg3_close) is called between tg3_io_error_detected()
> and tg3_io_resume() then tg3_napi_disable() is called twice without
> a tg3_napi_enable between. Once during tg3_io_error_detected() and again
> during tg3_close(). The tg3_io_resume() then hangs on rtnl_lock().
>
> v2: Added logging messages per Prashant's request
>
> Cc: Prashant Sreedharan <prashant@broadcom.com>
> Cc: Michael Chan <mchan@broadcom.com>
>
> Signed-off-by: Ivan Vecera <ivecera@redhat.com>
Acked-by: Prashant Sreedharan <prashant@broadcom.com>
--
^ permalink raw reply
* Re: [PATCH net-next 2/2] sunvnet: Re-check for a VIO_DESC_READY data descriptor after short udelay()
From: Raghuram Kothakota @ 2014-09-02 17:33 UTC (permalink / raw)
To: Sowmini Varadhan; +Cc: David Miller, netdev
In-Reply-To: <20140902165659.GH31516@oracle.com>
On Sep 2, 2014, at 9:56 AM, Sowmini Varadhan <sowmini.varadhan@oracle.com> wrote:
> On (09/02/14 09:43), Raghuram Kothakota wrote:
>> We could optimize this a bit by not wait for normal traffic, I mean,
>> non-burst traffic
>> and apply the retry only when we detect a stream of packets?
>
> How could you tell the difference efficiently? You'd need to
> track some kind of history/state for inter-packet arrival time.
> All seems like over-kill (more useful to go and optimize other
> parts of the system, such as do less work in interrupt context).
>From what I see, the vnet_walk_rx() picks up packets in a while loop,
we could count the number of packets picked up in that loop and use
that count as a method to determine if we need to apply this retry or not.
That is, retry only if that counter is > x, that may avoid waiting for cases
where peer sent one packet only? It may be worth trying it
and see if it still keeps up the improvement that you saw.
-Raghuram
>
>> We could detect a stream based on how many packets are picked
>> up in this function, picking up 3 or more could be considered as a stream,
>> of course tune based on testing.
>>
>> You probably tried it already, but checking to see if you tried with
>> less number
>> of iterations, we could reduce the iterations if the numbers are equally good
>> with less iterations.
>
> yes, the 3 * 4 micro-seconds was arrived at heuristically.
>
> --Sowmini
>
^ permalink raw reply
* Re: [PATCH net-next 2/2] sunvnet: Re-check for a VIO_DESC_READY data descriptor after short udelay()
From: Sowmini Varadhan @ 2014-09-02 16:56 UTC (permalink / raw)
To: Raghuram Kothakota; +Cc: David Miller, netdev
In-Reply-To: <665366D4-384B-45E1-B371-239EA87F5A15@oracle.com>
On (09/02/14 09:43), Raghuram Kothakota wrote:
> We could optimize this a bit by not wait for normal traffic, I mean,
> non-burst traffic
> and apply the retry only when we detect a stream of packets?
How could you tell the difference efficiently? You'd need to
track some kind of history/state for inter-packet arrival time.
All seems like over-kill (more useful to go and optimize other
parts of the system, such as do less work in interrupt context).
> We could detect a stream based on how many packets are picked
> up in this function, picking up 3 or more could be considered as a stream,
> of course tune based on testing.
>
> You probably tried it already, but checking to see if you tried with
> less number
> of iterations, we could reduce the iterations if the numbers are equally good
> with less iterations.
yes, the 3 * 4 micro-seconds was arrived at heuristically.
--Sowmini
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox