* Re: [PATCH net] ipv6: fix headroom calculation in udp6_ufo_fragment
From: David Miller @ 2013-11-06 3:10 UTC (permalink / raw)
To: hannes; +Cc: Saran.Neti, netdev, pshelar, tsl-vulnerability-research
In-Reply-To: <20131105014127.GG8832@order.stressinduktion.org>
From: Hannes Frederic Sowa <hannes@stressinduktion.org>
Date: Tue, 5 Nov 2013 02:41:27 +0100
> Commit 1e2bd517c108816220f262d7954b697af03b5f9c ("udp6: Fix udp
> fragmentation for tunnel traffic.") changed the calculation if
> there is enough space to include a fragment header in the skb from a
> skb->mac_header dervived one to skb_headroom. Because we already peeled
> off the skb to transport_header this is wrong. Change this back to check
> if we have enough room before the mac_header.
>
> This fixes a panic Saran Neti reported. He used the tbf scheduler which
> skb_gso_segments the skb. The offsets get negative and we panic in memcpy
> because the skb was erroneously not expanded at the head.
>
> Reported-by: Saran Neti <Saran.Neti@telus.com>
> Cc: Pravin B Shelar <pshelar@nicira.com>
> Signed-off-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
Applied and queued up for -stable, thanks Hannes.
^ permalink raw reply
* Re: [PATCH net-next] ipv4: introduce new IP_MTU_DISCOVER mode IP_PMTUDISC_INTERFACE
From: Hannes Frederic Sowa @ 2013-11-06 3:11 UTC (permalink / raw)
To: David Miller; +Cc: netdev, fweimer
In-Reply-To: <20131105.215750.2106258274923826040.davem@davemloft.net>
On Tue, Nov 05, 2013 at 09:57:50PM -0500, David Miller wrote:
> From: Hannes Frederic Sowa <hannes@stressinduktion.org>
> Date: Tue, 5 Nov 2013 02:24:17 +0100
>
> > Sockets marked with IP_PMTUDISC_INTERFACE won't do path mtu discovery,
> > their sockets won't accept and install new path mtu information and they
> > will always use the interface mtu for outgoing packets. It is guaranteed
> > that the packet is not fragmented locally. But we won't set the DF-Flag
> > on the outgoing frames.
> >
> > Florian Weimer had the idea to use this flag to ensure DNS servers are
> > never generating outgoing fragments. They may well be fragmented on the
> > path, but the server never stores or usees path mtu values, which could
> > well be forged in an attack.
> >
> > (The root of the problem with path MTU discovery is that there is
> > no reliable way to authenticate ICMP Fragmentation Needed But DF Set
> > messages because they are sent from intermediate routers with their
> > source addresses, and the IMCP payload will not always contain sufficient
> > information to identify a flow.)
> >
> > Recent research in the DNS community showed that it is possible to
> > implement an attack where DNS cache poisoning is feasible by spoofing
> > fragments. This work was done by Amir Herzberg and Haya Shulman:
> > <https://sites.google.com/site/hayashulman/files/fragmentation-poisoning.pdf>
> >
> > This issue was previously discussed among the DNS community, e.g.
> > <http://www.ietf.org/mail-archive/web/dnsext/current/msg01204.html>,
> > without leading to fixes.
> >
> > This patch depends on the patch "ipv4: fix DO and PROBE pmtu mode
> > regarding local fragmentation with UFO/CORK" for the enforcement of the
> > non-fragmentable checks. If other users than ip_append_page/data should
> > use this semantic too, we have to add a new flag to IPCB(skb)->flags to
> > suppress local fragmentation and check for this in ip_finish_output.
> >
> > Many thanks to Florian Weimer for the idea and feedback while implementing
> > this patch.
> >
> > Cc: David S. Miller <davem@davemloft.net>
> > Suggested-by: Florian Weimer <fweimer@redhat.com>
> > Signed-off-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
>
> Ok I changed my mind and decided to apply this, thanks.
>
> BTW, what about IPV6? Even if ipv6 doesn't need this facility, for
> whatever reason, these IP_PMTUDISC_* values are shared with ipv6
> (via the IPV6_MTU_DISCOVER socket option) so we should at least make
> sure that we do something reasonable if the new value happens to get
> passed in.
Of course I will look after IPv6. ;)
IPPROTO_IP setsockopt level does change the value of inet_sk->pmtudisc.
The IPPROTO_IPV6 would change the pmtudisc in ipv6_pinfo. So we currently
don't accept this option on IPv6 sockets (-EINVAL) and when applied with
IPPROTO_IP level it will change the sockets behaviour for IPv4 mapped
sockets only. So nothing will break with this change.
It should be easy to do and I plan to send the changes this week. pmtudisc
in ipv6_pinfo has only 2 bits so I will have to increase the socket size
a bit.
Thank you,
Hannes
^ permalink raw reply
* Re: [PATCH net] net: sch_red: Fix race between timer and red_destroy()
From: David Miller @ 2013-11-06 3:16 UTC (permalink / raw)
To: subramanian.vijay; +Cc: netdev, shemminger, eric.dumazet
In-Reply-To: <1383617296-20273-1-git-send-email-subramanian.vijay@gmail.com>
From: Vijay Subramanian <subramanian.vijay@gmail.com>
Date: Mon, 4 Nov 2013 18:08:16 -0800
> Make sure timer does not fire once qdisc is destroyed.
>
> Signed-off-by: Vijay Subramanian <subramanian.vijay@gmail.com>
I do not understand how this problem can trigger.
del_timer_sync() waits for all instances of the timer handler which
are running to complete, and then it deletes the timer from any of
the scheduled timer lists.
Your patch is making the mod_timer() in the timer handler conditional,
and this really shouldn't be necessary.
The whole point of del_timer_sync() is to address these kinds of
situations.
^ permalink raw reply
* Re: [PATCH] ipv6: drop the judgement in rt6_alloc_cow()
From: David Miller @ 2013-11-06 3:17 UTC (permalink / raw)
To: hannes; +Cc: duanj.fnst, netdev
In-Reply-To: <20131105212344.GM8832@order.stressinduktion.org>
From: Hannes Frederic Sowa <hannes@stressinduktion.org>
Date: Tue, 5 Nov 2013 22:23:44 +0100
> On Tue, Nov 05, 2013 at 01:34:53PM +0800, Duan Jiong wrote:
>>
>> Now rt6_alloc_cow() is only called by ip6_pol_route() when
>> rt->rt6i_flags doesn't contain both RTF_NONEXTHOP and RTF_GATEWAY,
>> and rt->rt6i_flags hasn't been changed in ip6_rt_copy().
>> So there is no neccessary to judge whether rt->rt6i_flags contains
>> RTF_GATEWAY or not.
>
> This is ok.
>
> Acked-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
Applied, thanks everyone.
^ permalink raw reply
* Re: [RESEND PATCH net-next] virtio-net: switch to use XPS to choose txq
From: David Miller @ 2013-11-06 3:22 UTC (permalink / raw)
To: jasowang; +Cc: netdev, virtualization, linux-kernel, mst
In-Reply-To: <1383646785-10446-1-git-send-email-jasowang@redhat.com>
From: Jason Wang <jasowang@redhat.com>
Date: Tue, 5 Nov 2013 18:19:45 +0800
> We used to use a percpu structure vq_index to record the cpu to queue
> mapping, this is suboptimal since it duplicates the work of XPS and
> loses all other XPS functionality such as allowing user to configure
> their own transmission steering strategy.
>
> So this patch switches to use XPS and suggest a default mapping when
> the number of cpus is equal to the number of queues. With XPS support,
> there's no need for keeping per-cpu vq_index and .ndo_select_queue(),
> so they were removed also.
>
> Cc: Rusty Russell <rusty@rustcorp.com.au>
> Cc: Michael S. Tsirkin <mst@redhat.com>
> Acked-by: Rusty Russell <rusty@rustcorp.com.au>
> Acked-by: Michael S. Tsirkin <mst@redhat.com>
> Signed-off-by: Jason Wang <jasowang@redhat.com>
> ---
> Resend since the previous complie warning disappears after commit
> 3573540cafa4296dd60f8be02f2aecaa31047525
> (netif_set_xps_queue: make cpu mask const).
Yep, looks great, applied. Thanks Jason!
^ permalink raw reply
* Re: gso: Attempt to handle mega-GRO packets
From: Herbert Xu @ 2013-11-06 4:07 UTC (permalink / raw)
To: Eric Dumazet
Cc: Ben Hutchings, David Miller, christoph.paasch, netdev, hkchu,
mwdalton
In-Reply-To: <1383702347.4291.152.camel@edumazet-glaptop2.roam.corp.google.com>
On Tue, Nov 05, 2013 at 05:45:47PM -0800, Eric Dumazet wrote:
>
> Hmm, I do not think fskb has the headers in the general case.
Only GRO produces such packets, see the changeset where I added
frag_list support to skb_setgment
89319d3801d1d3ac29c7df1f067038986f267d29
> It might work in the GRO case only.
Any chance you guys can test this patch out?
Thanks!
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* Re: gso: Attempt to handle mega-GRO packets
From: Eric Dumazet @ 2013-11-06 4:23 UTC (permalink / raw)
To: Herbert Xu
Cc: Ben Hutchings, David Miller, christoph.paasch, netdev, hkchu,
mwdalton
In-Reply-To: <20131106040717.GA15711@gondor.apana.org.au>
On Wed, 2013-11-06 at 12:07 +0800, Herbert Xu wrote:
> On Tue, Nov 05, 2013 at 05:45:47PM -0800, Eric Dumazet wrote:
> >
> > Hmm, I do not think fskb has the headers in the general case.
>
> Only GRO produces such packets, see the changeset where I added
> frag_list support to skb_setgment
Nope, I already mentioned this :
Please take a look at 2613af0ed18a11d5c566a81f9a6510b73180660a
("virtio_net: migrate mergeable rx buffers to page frag allocators")
I do not see why skb_segment() would be tied to GRO -> GSO, with
the property of each page frag being exactly MSS sized.
We use it in TCP stack with page frags of any size.
skb_segment() needs to iterate properly on all pages frags,
the one found in the first skb, and the ones found on frag_list skbs.
^ permalink raw reply
* Re: gso: Attempt to handle mega-GRO packets
From: Herbert Xu @ 2013-11-06 4:28 UTC (permalink / raw)
To: Eric Dumazet
Cc: Ben Hutchings, David Miller, christoph.paasch, netdev, hkchu,
mwdalton
In-Reply-To: <1383711785.4291.156.camel@edumazet-glaptop2.roam.corp.google.com>
On Tue, Nov 05, 2013 at 08:23:05PM -0800, Eric Dumazet wrote:
>
> Nope, I already mentioned this :
>
> Please take a look at 2613af0ed18a11d5c566a81f9a6510b73180660a
> ("virtio_net: migrate mergeable rx buffers to page frag allocators")
This patch should simply be reverted. You guys steam-rolled over
the virtio_net people's concerns that this seriously impacts virt
performance.
Cheers,
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* Re: [PATCH net-next 0/3] IPsec improvements
From: David Miller @ 2013-11-06 4:48 UTC (permalink / raw)
To: mathias.krause; +Cc: steffen.klassert, herbert, dmitry.tarnyagin, netdev
In-Reply-To: <cover.1381923854.git.mathias.krause@secunet.com>
From: Mathias Krause <mathias.krause@secunet.com>
Date: Tue, 5 Nov 2013 14:54:08 +0100
> This series moves pskb_put() to the core code -- making the code
> duplication in caif obsolete (patches 1 and 2).
>
> Additionally does this series optimize the IPsec receive path in patch 3
> by allowing skb_cow_data() to leave the buffer fragmented. I noticed the
> linearization to be a bottleneck when doing some VPN gateway benchmarks.
> Linearization of the buffer isn't needed in the receive path as the
> crypto API (and all other users of skb_cow_data) can handle sg.
>
> With patch 3 applied I was able to increase the throughput of an IPsec
> gateway from 7.12 Gbit/s to 7.28 Gbit/s.
Please deal with the feedback given to you by Ben about referring to
variables with the '@' prefix consistently in comments, then resend
this entire patch series.
Thanks.
^ permalink raw reply
* Re: [PATCH net] qeth: avoid buffer overflow in snmp ioctl
From: David Miller @ 2013-11-06 4:51 UTC (permalink / raw)
To: blaschka; +Cc: netdev, stable
In-Reply-To: <20131105141400.GA29477@tuxmaker.boeblingen.de.ibm.com>
From: Frank Blaschka <blaschka@linux.vnet.ibm.com>
Date: Tue, 5 Nov 2013 15:14:00 +0100
> + if (req_len > QETH_BUFSIZE - IPA_PDU_HEADER_SIZE
> + - sizeof(struct qeth_ipacmd_hdr)
> + - sizeof(struct qeth_ipacmd_setadpparms_hdr))
> + return -EINVAL;
Please fix the indentation and parenthesize the more complicated right
side of the expression, it should be something like:
if (req_len > (QETH_BUFSIZE - IPA_PDU_HEADER_SIZE -
sizeof(struct qeth_ipacmd_hdr) -
sizeof(struct qeth_ipacmd_setadpparms_hdr)))
Thanks.
^ permalink raw reply
* Re: gso: Attempt to handle mega-GRO packets
From: Eric Dumazet @ 2013-11-06 5:20 UTC (permalink / raw)
To: Herbert Xu
Cc: Ben Hutchings, David Miller, christoph.paasch, netdev, hkchu,
mwdalton
In-Reply-To: <20131106042858.GA15745@gondor.apana.org.au>
On Wed, 2013-11-06 at 12:28 +0800, Herbert Xu wrote:
> On Tue, Nov 05, 2013 at 08:23:05PM -0800, Eric Dumazet wrote:
> >
> > Nope, I already mentioned this :
> >
> > Please take a look at 2613af0ed18a11d5c566a81f9a6510b73180660a
> > ("virtio_net: migrate mergeable rx buffers to page frag allocators")
>
> This patch should simply be reverted. You guys steam-rolled over
> the virtio_net people's concerns that this seriously impacts virt
> performance.
>
Have you followed netdev traffic lately ?
virtio_net performance is better than before, and we did no revert,
thank you.
I am really tired, I'll fix skb_segment() tomorrow.
^ permalink raw reply
* INVESTMENT PARTNERSHIP
From: Mrs. Nkoana Maite Mashabane @ 2013-11-06 5:42 UTC (permalink / raw)
To: net_lm, netcomp3000, netbizpak, netballpk, netabuse, netadmin,
netcourt, netdev, net, netcom_dps, netbizn, net.country,
netcruiser, netcom, netadmin, netbsd-docs-ru, netbehaviour,
netadmin, netbytes, net, netbsd-soccvs, netbeans-info, netadmin,
net, net, netadm, net, netadm, netchatcomp, netbazar, netangels,
net96, net_central, net-abuse, netcontact, netddang, netcom,
net-artidaya, netbank, netannounce, netcell, netbank, net,
netcomp, netabuse, netcompd, netcom, net818, net, net1
Hello,
I am Mrs. Maite Nkoana Mashabane, am the Minister of International Relations and Cooperation of South Africa, Please view my profile. http://www.gcis.gov.za/gcis/gcis_profile.jsp?id=1480.
I would like you to indicate your interest to receive the transfer of $3.5 Million. I will like you to stand as the beneficiary of the said fund.
However, I will be resigning soon and want to establish any type of machinery tools devices or agriculture or even real estate under my own registered company. I have the sum to invest. How can I come in contact with you in partnership/investment transaction?
I want you to assist me in investing the money in your country.
I will also like you to send me the following information below:
(1)Full names:
(2)Private phone number:
(3)Current residential address:
(4)Your Country:
(5)Occupation:
(6)Age and Sex:
Please I do not need to remind you of the need for absolute Confidentiality if this transaction must succeed. YOU MUST NOT CALL ME! If you do not feel comfortable with this transaction, do not hesitate to discontinue.
I will give you more detail's as soon as you confirm your willingness to assisting me.
Mrs. Nkoana Maite Mashabane
^ permalink raw reply
* spurious ip forwarding inside namespaces
From: Alexei Starovoitov @ 2013-11-06 6:32 UTC (permalink / raw)
To: Isaku Yamahata, Nicolas Dichtel; +Cc: David S. Miller, netdev
Hi All,
commit 06a23fe31c
"core/dev: set pkt_type after eth_type_trans() in dev_forward_skb()"
and refactoring 64261f230a
"dev: move skb_scrub_packet() after eth_type_trans()"
are forcing pkt_type to be PACKET_HOST when skb traverses veth.
which means that ip forwarding will kick in inside netns
even if skb->eth->h_dest != dev->dev_addr
The way we hit the problem is the following:
create 4 or more of netns with veth and attach them to a bridge.
disable learning on a bridge (via setageing 0), so it will flood
packets to all namespaces.
Since ip forwarding and icmp redirect by default is on,
single icmp request packet from one netns creates a storm of packets,
since every netns receives it and incorrectly does ip forward on it
and also incorrectly sends icmp redirect which are multiplied once
again by flooding in the bridge.
ping -c1 generates 1 outgoing packet and ~13K incoming packets with
just 4 netns.
Running any tcp traffic between such namespaces becomes impossible
and ksoftirqd cpu usage goes to 100% on all cpus.
Please suggest how to fix this cleanly.
Thanks
Alexei
^ permalink raw reply
* [PATCH net-next 0/9] bonding: rebuild the lock use for bond monitor
From: Ding Tianhong @ 2013-11-06 6:52 UTC (permalink / raw)
To: Jay Vosburgh, Andy Gospodarek, David S. Miller,
Nikolay Aleksandrov, Veaceslav Falico, Netdev
Now the bond slave list is not protected by bond lock, only by RTNL,
but the monitor still use the bond lock to protect the slave list,
it is useless, there were three way to fix the protect problem:
1. add bond_master_upper_dev_link() and bond_upper_dev_unlink()
in bond->lock, but it is unsafe to call call_netdevice_notifiers()
in write lock.
2. remove unused bond->lock for monitor function, only use the exist
rtnl lock(), it will take performance loss in fast path.
3. use RCU to protect the slave list, of course, performance is better,
but in slow path, it is ignored.
obviously the solution 1 is not fit here, I will consider the 2 and 3
solution. My principle is simple, if in fast path, RCU is better,
otherwise in slow path, both is well, if RTNL is necessary for other
reason, no need to add RCU, if the RTNL is only used for slave list,
I will use RCU to instead of RTNL for performance.
the second problem is the curr_slave_lock for bond, it is too old and
unwanted in many place, because the curr_active_slave would only be
changed in 3 place:
1. enslave slave.
2. release slave.
3. change active slave.
all above were already holding bond lock, RTNL and curr_slave_lock
together, it is tedious and no need to add so mach lock, the RTNL
is enough, when the other cpu to read the curr_active_slave
for parallel, RTNL or RCU is both well for it, so after the patch set,
I will recheck the curr_slave_lock and determane what to do for it,
this time, only for monitor work well.
for the stability, I did not change the logic for the monitor,
all change is clear and simple, I have test the patch set for lockdep,
it work well and stability.
Best Regards
Ding Tianhong
Ding Tianhong (9):
bonding: remove the no effect lock for bond_select_active_slave()
bonding: rebuild the lock use for bond_mii_monitor()
bonding: rebuild the lock use for bond_alb_monitor()
bonding: rebuild the lock use for bond_loadbalance_arp_mon()
bonding: rebuild the lock use for bond_activebackup_arp_mon()
bonding: use RTNL instead of bond lock for
bond_3ad_state_machine_handler()
bonding: remove unwanted lock for bond_option_active_slave_set()
bonding: remove unwanted lock for bond enslave and release
bonding: remove unwanted lock for bond_store_primary_xxx()
drivers/net/bonding/bond_3ad.c | 15 ++--
drivers/net/bonding/bond_alb.c | 55 +++----------
drivers/net/bonding/bond_main.c | 160 +++++++++----------------------------
drivers/net/bonding/bond_options.c | 4 -
drivers/net/bonding/bond_sysfs.c | 8 --
5 files changed, 53 insertions(+), 189 deletions(-)
--
1.8.2.1
^ permalink raw reply
* [PATCH net-next 8/9] bonding: remove unwanted lock for bond enslave and release
From: Ding Tianhong @ 2013-11-06 6:53 UTC (permalink / raw)
To: Jay Vosburgh, Andy Gospodarek, David S. Miller,
Nikolay Aleksandrov, Veaceslav Falico, Netdev
The bond_change_active_slave() and bond_select_active_slave()
was no longer need bond lock and curr_slave_lock, the enslave
and release will always in RTNL, so remove the unwanted lock.
Signed-off-by: Ding Tianhong <dingtianhong@huawei.com>
---
drivers/net/bonding/bond_main.c | 19 +------------------
1 file changed, 1 insertion(+), 18 deletions(-)
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index dc17a85..d2d70dc 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -1565,13 +1565,8 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev)
bond_compute_features(bond);
bond_set_carrier(bond);
- if (USES_PRIMARY(bond->params.mode)) {
- read_lock(&bond->lock);
- write_lock_bh(&bond->curr_slave_lock);
+ if (USES_PRIMARY(bond->params.mode))
bond_select_active_slave(bond);
- write_unlock_bh(&bond->curr_slave_lock);
- read_unlock(&bond->lock);
- }
pr_info("%s: enslaving %s as a%s interface with a%s link.\n",
bond_dev->name, slave_dev->name,
@@ -1590,19 +1585,11 @@ err_detach:
bond_hw_addr_flush(bond_dev, slave_dev);
vlan_vids_del_by_dev(slave_dev, bond_dev);
- write_lock_bh(&bond->lock);
if (bond->primary_slave == new_slave)
bond->primary_slave = NULL;
if (bond->curr_active_slave == new_slave) {
bond_change_active_slave(bond, NULL);
- write_unlock_bh(&bond->lock);
- read_lock(&bond->lock);
- write_lock_bh(&bond->curr_slave_lock);
bond_select_active_slave(bond);
- write_unlock_bh(&bond->curr_slave_lock);
- read_unlock(&bond->lock);
- } else {
- write_unlock_bh(&bond->lock);
}
slave_disable_netpoll(new_slave);
@@ -1743,13 +1730,9 @@ static int __bond_release_one(struct net_device *bond_dev,
* will interfere.
*/
write_unlock_bh(&bond->lock);
- read_lock(&bond->lock);
- write_lock_bh(&bond->curr_slave_lock);
bond_select_active_slave(bond);
- write_unlock_bh(&bond->curr_slave_lock);
- read_unlock(&bond->lock);
write_lock_bh(&bond->lock);
}
--
1.8.2.1
^ permalink raw reply related
* [PATCH net-next 5/9] bonding: rebuild the lock use for bond_activebackup_arp_mon()
From: Ding Tianhong @ 2013-11-06 6:52 UTC (permalink / raw)
To: Jay Vosburgh, Andy Gospodarek, David S. Miller,
Nikolay Aleksandrov, Veaceslav Falico, Netdev
The bond_activebackup_arp_mon() use the bond lock for read to
protect the slave list, it is no effect, and the RTNL is still
called for bond_ab_arp_monitor(), so move the RTNL to the top
of the monitor to protect the whole function is a clear way,
I also remove the curr_slave_lock for bond_select_active_slave(),
RTNL is enough here.
Signed-off-by: Ding Tianhong <dingtianhong@huawei.com>
---
drivers/net/bonding/bond_main.c | 57 +++++++++++------------------------------
1 file changed, 15 insertions(+), 42 deletions(-)
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index b875dff..dc17a85 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -2545,7 +2545,7 @@ static int bond_ab_arp_inspect(struct bonding *bond)
* Called to commit link state changes noted by inspection step of
* active-backup mode ARP monitor.
*
- * Called with RTNL and bond->lock for read.
+ * Called with RTNL hold.
*/
static void bond_ab_arp_commit(struct bonding *bond)
{
@@ -2608,9 +2608,7 @@ static void bond_ab_arp_commit(struct bonding *bond)
do_failover:
ASSERT_RTNL();
block_netpoll_tx();
- write_lock_bh(&bond->curr_slave_lock);
bond_select_active_slave(bond);
- write_unlock_bh(&bond->curr_slave_lock);
unblock_netpoll_tx();
}
@@ -2620,7 +2618,7 @@ do_failover:
/*
* Send ARP probes for active-backup mode ARP monitor.
*
- * Called with bond->lock held for read.
+ * Called with RNTL held.
*/
static void bond_ab_arp_probe(struct bonding *bond)
{
@@ -2628,8 +2626,6 @@ static void bond_ab_arp_probe(struct bonding *bond)
struct list_head *iter;
bool found = false;
- read_lock(&bond->curr_slave_lock);
-
if (bond->current_arp_slave && bond->curr_active_slave)
pr_info("PROBE: c_arp %s && cas %s BAD\n",
bond->current_arp_slave->dev->name,
@@ -2637,12 +2633,9 @@ static void bond_ab_arp_probe(struct bonding *bond)
if (bond->curr_active_slave) {
bond_arp_send_all(bond, bond->curr_active_slave);
- read_unlock(&bond->curr_slave_lock);
return;
}
- read_unlock(&bond->curr_slave_lock);
-
/* if we don't have a curr_active_slave, search for the next available
* backup slave from the current_arp_slave and make it the candidate
* for becoming the curr_active_slave
@@ -2702,51 +2695,31 @@ void bond_activebackup_arp_mon(struct work_struct *work)
struct bonding *bond = container_of(work, struct bonding,
arp_work.work);
bool should_notify_peers = false;
- int delta_in_ticks;
-
- read_lock(&bond->lock);
- delta_in_ticks = msecs_to_jiffies(bond->params.arp_interval);
+ if (!rtnl_trylock())
+ goto re_arm;
- if (!bond_has_slaves(bond))
+ if (!bond_has_slaves(bond)) {
+ rtnl_unlock();
goto re_arm;
+ }
should_notify_peers = bond_should_notify_peers(bond);
- if (bond_ab_arp_inspect(bond)) {
- read_unlock(&bond->lock);
-
- /* Race avoidance with bond_close flush of workqueue */
- if (!rtnl_trylock()) {
- read_lock(&bond->lock);
- delta_in_ticks = 1;
- should_notify_peers = false;
- goto re_arm;
- }
-
- read_lock(&bond->lock);
-
+ if (bond_ab_arp_inspect(bond))
bond_ab_arp_commit(bond);
- read_unlock(&bond->lock);
- rtnl_unlock();
- read_lock(&bond->lock);
- }
-
bond_ab_arp_probe(bond);
-re_arm:
- if (bond->params.arp_interval)
- queue_delayed_work(bond->wq, &bond->arp_work, delta_in_ticks);
+ if (should_notify_peers)
+ call_netdevice_notifiers(NETDEV_NOTIFY_PEERS, bond->dev);
- read_unlock(&bond->lock);
+ rtnl_unlock();
- if (should_notify_peers) {
- if (!rtnl_trylock())
- return;
- call_netdevice_notifiers(NETDEV_NOTIFY_PEERS, bond->dev);
- rtnl_unlock();
- }
+re_arm:
+ if (bond->params.arp_interval)
+ queue_delayed_work(bond->wq, &bond->arp_work,
+ msecs_to_jiffies(bond->params.arp_interval));
}
/*-------------------------- netdev event handling --------------------------*/
--
1.8.2.1
^ permalink raw reply related
* [PATCH net-next 6/9] bonding: use RTNL instead of bond lock for bond_3ad_state_machine_handler()
From: Ding Tianhong @ 2013-11-06 6:53 UTC (permalink / raw)
To: Jay Vosburgh, Andy Gospodarek, David S. Miller,
Nikolay Aleksandrov, Veaceslav Falico, Netdev
The bond_3ad_state_machine_handler() is a slow path, it use the bond
lock to protect the bond slave list and slave port, the slave list
could not be protect by bond lock anymore, so I need to use RTNL or
RCU instead of bond lock, but if I remove the bond lock, the
bond_3ad_state_machine_handler() may use the slave port when bond
releasing the slave, it will occur problems.
As the bond_3ad_unbind_slave() only protected by bond lock or RTNL,
so add RCU is not a good solution as it could not protect the slave
port, so RTNL is fit here.
Signed-off-by: Ding Tianhong <dingtianhong@huawei.com>
---
drivers/net/bonding/bond_3ad.c | 15 +++++++--------
1 file changed, 7 insertions(+), 8 deletions(-)
diff --git a/drivers/net/bonding/bond_3ad.c b/drivers/net/bonding/bond_3ad.c
index 187b1b7..a4d190e 100644
--- a/drivers/net/bonding/bond_3ad.c
+++ b/drivers/net/bonding/bond_3ad.c
@@ -2068,18 +2068,19 @@ void bond_3ad_state_machine_handler(struct work_struct *work)
struct slave *slave;
struct port *port;
- read_lock(&bond->lock);
+ if (!rtnl_trylock()) {
+ queue_delayed_work(bond->wq, &bond->ad_work, ad_delta_in_ticks);
+ return;
+ }
- //check if there are any slaves
if (!bond_has_slaves(bond))
goto re_arm;
- // check if agg_select_timer timer after initialize is timed out
+ /* check if agg_select_timer timer after initialize is timed out */
if (BOND_AD_INFO(bond).agg_select_timer && !(--BOND_AD_INFO(bond).agg_select_timer)) {
slave = bond_first_slave(bond);
port = slave ? &(SLAVE_AD_INFO(slave).port) : NULL;
- // select the active aggregator for the bond
if (port) {
if (!port->slave) {
pr_warning("%s: Warning: bond's first port is uninitialized\n",
@@ -2093,7 +2094,6 @@ void bond_3ad_state_machine_handler(struct work_struct *work)
bond_3ad_set_carrier(bond);
}
- // for each port run the state machines
bond_for_each_slave(bond, slave, iter) {
port = &(SLAVE_AD_INFO(slave).port);
if (!port->slave) {
@@ -2114,7 +2114,7 @@ void bond_3ad_state_machine_handler(struct work_struct *work)
ad_mux_machine(port);
ad_tx_machine(port);
- // turn off the BEGIN bit, since we already handled it
+ /* turn off the BEGIN bit, since we already handled it */
if (port->sm_vars & AD_PORT_BEGIN)
port->sm_vars &= ~AD_PORT_BEGIN;
@@ -2122,9 +2122,8 @@ void bond_3ad_state_machine_handler(struct work_struct *work)
}
re_arm:
+ rtnl_unlock();
queue_delayed_work(bond->wq, &bond->ad_work, ad_delta_in_ticks);
-
- read_unlock(&bond->lock);
}
/**
--
1.8.2.1
^ permalink raw reply related
* [PATCH net-next 2/9] bonding: remove bond read lock for bond_mii_monitor()
From: Ding Tianhong @ 2013-11-06 6:52 UTC (permalink / raw)
To: Jay Vosburgh, Andy Gospodarek, David S. Miller,
Nikolay Aleksandrov, Veaceslav Falico, Netdev
The bond_mii_monitor() still use bond lock read to protect
bond_has_slaves() and bond_miimon_inspect(), it is no effect,
so I move the RTNL to the top of the function to protect the
whole monitor, of course, if the bond state did not changed,
the monitor still calling RTNL, it may bring more performance
loss, but as a slow path, it is negligible.
also in bond_miimon_commit(), I remove the unwanted curr_slave_lock
when calling the bond_select_active_slave(), the RTNL is enough.
Signed-off-by: Ding Tianhong <dingtianhong@huawei.com>
---
drivers/net/bonding/bond_main.c | 44 ++++++++++++-----------------------------
1 file changed, 13 insertions(+), 31 deletions(-)
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 9c9803c..98171eb 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -2074,9 +2074,7 @@ static void bond_miimon_commit(struct bonding *bond)
do_failover:
ASSERT_RTNL();
block_netpoll_tx();
- write_lock_bh(&bond->curr_slave_lock);
bond_select_active_slave(bond);
- write_unlock_bh(&bond->curr_slave_lock);
unblock_netpoll_tx();
}
@@ -2098,47 +2096,31 @@ void bond_mii_monitor(struct work_struct *work)
bool should_notify_peers = false;
unsigned long delay;
- read_lock(&bond->lock);
-
delay = msecs_to_jiffies(bond->params.miimon);
- if (!bond_has_slaves(bond))
+ if (!rtnl_trylock()) {
+ delay = 1;
goto re_arm;
+ }
- should_notify_peers = bond_should_notify_peers(bond);
-
- if (bond_miimon_inspect(bond)) {
- read_unlock(&bond->lock);
-
- /* Race avoidance with bond_close cancel of workqueue */
- if (!rtnl_trylock()) {
- read_lock(&bond->lock);
- delay = 1;
- should_notify_peers = false;
- goto re_arm;
- }
+ if (!bond_has_slaves(bond)) {
+ rtnl_unlock();
+ goto re_arm;
+ }
- read_lock(&bond->lock);
+ should_notify_peers = bond_should_notify_peers(bond);
+ if (bond_miimon_inspect(bond))
bond_miimon_commit(bond);
- read_unlock(&bond->lock);
- rtnl_unlock(); /* might sleep, hold no other locks */
- read_lock(&bond->lock);
- }
+ if (should_notify_peers)
+ call_netdevice_notifiers(NETDEV_NOTIFY_PEERS, bond->dev);
+
+ rtnl_unlock();
re_arm:
if (bond->params.miimon)
queue_delayed_work(bond->wq, &bond->mii_work, delay);
-
- read_unlock(&bond->lock);
-
- if (should_notify_peers) {
- if (!rtnl_trylock())
- return;
- call_netdevice_notifiers(NETDEV_NOTIFY_PEERS, bond->dev);
- rtnl_unlock();
- }
}
static bool bond_has_this_ip(struct bonding *bond, __be32 ip)
--
1.8.2.1
^ permalink raw reply related
* [PATCH net-next 3/9] bonding: rebuild the lock use for bond_alb_monitor()
From: Ding Tianhong @ 2013-11-06 6:52 UTC (permalink / raw)
To: Jay Vosburgh, Andy Gospodarek, David S. Miller,
Nikolay Aleksandrov, Veaceslav Falico, Netdev
The bond_alb_monitor use bond lock to protect the bond slave list,
it is no effect here, we need to use RTNL or RCU to replace bond lock,
the bond_alb_monitor will called 10 times one second, so I think
RTNL is fit here, and move the rtnl to the top of the function, the
RTNL is being used when rlb_enabled is true.
remove the unwanted curr_slave_lock, because the curr_active_slave
will not be changed out the RTNL, so it is safe to sending the learning
packets.
Signed-off-by: Ding Tianhong <dingtianhong@huawei.com>
---
drivers/net/bonding/bond_alb.c | 33 ++++++---------------------------
1 file changed, 6 insertions(+), 27 deletions(-)
diff --git a/drivers/net/bonding/bond_alb.c b/drivers/net/bonding/bond_alb.c
index 7870e4e..4ed7fe4 100644
--- a/drivers/net/bonding/bond_alb.c
+++ b/drivers/net/bonding/bond_alb.c
@@ -1491,11 +1491,13 @@ void bond_alb_monitor(struct work_struct *work)
struct list_head *iter;
struct slave *slave;
- read_lock(&bond->lock);
+ if (!rtnl_trylock())
+ goto re_arm;
if (!bond_has_slaves(bond)) {
bond_info->tx_rebalance_counter = 0;
bond_info->lp_counter = 0;
+ rtnl_unlock();
goto re_arm;
}
@@ -1506,24 +1508,17 @@ void bond_alb_monitor(struct work_struct *work)
if (bond_info->lp_counter >= BOND_ALB_LP_TICKS(bond)) {
/* change of curr_active_slave involves swapping of mac addresses.
* in order to avoid this swapping from happening while
- * sending the learning packets, the curr_slave_lock must be held for
- * read.
+ * sending the learning packets, the RTNL or curr_slave_lock
+ * must be held for read.
*/
- read_lock(&bond->curr_slave_lock);
-
bond_for_each_slave(bond, slave, iter)
alb_send_learning_packets(slave, slave->dev->dev_addr);
- read_unlock(&bond->curr_slave_lock);
-
bond_info->lp_counter = 0;
}
/* rebalance tx traffic */
if (bond_info->tx_rebalance_counter >= BOND_TLB_REBALANCE_TICKS) {
-
- read_lock(&bond->curr_slave_lock);
-
bond_for_each_slave(bond, slave, iter) {
tlb_clear_slave(bond, slave, 1);
if (slave == bond->curr_active_slave) {
@@ -1534,8 +1529,6 @@ void bond_alb_monitor(struct work_struct *work)
}
}
- read_unlock(&bond->curr_slave_lock);
-
bond_info->tx_rebalance_counter = 0;
}
@@ -1544,16 +1537,6 @@ void bond_alb_monitor(struct work_struct *work)
if (bond_info->primary_is_promisc &&
(++bond_info->rlb_promisc_timeout_counter >= RLB_PROMISC_TIMEOUT)) {
- /*
- * dev_set_promiscuity requires rtnl and
- * nothing else. Avoid race with bond_close.
- */
- read_unlock(&bond->lock);
- if (!rtnl_trylock()) {
- read_lock(&bond->lock);
- goto re_arm;
- }
-
bond_info->rlb_promisc_timeout_counter = 0;
/* If the primary was set to promiscuous mode
@@ -1562,9 +1545,6 @@ void bond_alb_monitor(struct work_struct *work)
*/
dev_set_promiscuity(bond->curr_active_slave->dev, -1);
bond_info->primary_is_promisc = 0;
-
- rtnl_unlock();
- read_lock(&bond->lock);
}
if (bond_info->rlb_rebalance) {
@@ -1587,10 +1567,9 @@ void bond_alb_monitor(struct work_struct *work)
}
}
+ rtnl_unlock();
re_arm:
queue_delayed_work(bond->wq, &bond->alb_work, alb_delta_in_ticks);
-
- read_unlock(&bond->lock);
}
/* assumption: called before the slave is attached to the bond
--
1.8.2.1
^ permalink raw reply related
* [PATCH net-next 7/9] bonding: remove unwanted lock for bond_option_active_slave_set()
From: Ding Tianhong @ 2013-11-06 6:53 UTC (permalink / raw)
To: Jay Vosburgh, Andy Gospodarek, David S. Miller,
Nikolay Aleksandrov, Veaceslav Falico, Netdev
The bond_option_active_slave_set() is always in RTNL, bond lock
and curr_slave_lock is no longer need here, so remove it to clean
the function.
Signed-off-by: Ding Tianhong <dingtianhong@huawei.com>
---
drivers/net/bonding/bond_options.c | 4 ----
1 file changed, 4 deletions(-)
diff --git a/drivers/net/bonding/bond_options.c b/drivers/net/bonding/bond_options.c
index 9a5223c..7694343 100644
--- a/drivers/net/bonding/bond_options.c
+++ b/drivers/net/bonding/bond_options.c
@@ -101,8 +101,6 @@ int bond_option_active_slave_set(struct bonding *bond,
}
block_netpoll_tx();
- read_lock(&bond->lock);
- write_lock_bh(&bond->curr_slave_lock);
/* check to see if we are clearing active */
if (!slave_dev) {
@@ -135,8 +133,6 @@ int bond_option_active_slave_set(struct bonding *bond,
}
}
- write_unlock_bh(&bond->curr_slave_lock);
- read_unlock(&bond->lock);
unblock_netpoll_tx();
return ret;
}
--
1.8.2.1
^ permalink raw reply related
* [PATCH net-next 1/9] bonding: remove the no effect lock for bond_select_active_slave()
From: Ding Tianhong @ 2013-11-06 6:52 UTC (permalink / raw)
To: Jay Vosburgh, Andy Gospodarek, David S. Miller,
Nikolay Aleksandrov, Veaceslav Falico, Netdev
The bond slave list was no longer protected by bond lock and only
protected by RTNL or RCU, so anywhere that use bond lock to protect
slave list is meaningless.
The curr_active_slave could only be changed in 3 place:
1. enslave slave.
2. release slave.
3. change_active_slave.
all above place were holding bond lock, RTNL and curr_slave_lock together,
it is tedious and meaningless, only RTNL is enough, so remove the
bond lock and curr_slave_lock for change_active_slave.
when read the curr_active_slave, if it has RTNL yet, no need to add any
protection, otherwise we could use RCU dereference to ensure that no
problems occurs.
there are several place calling bond_select_active_slave() and
bond_change_active_slave(), the next step I will clean these place
and remove the no effect lock.
Signed-off-by: Ding Tianhong <dingtianhong@huawei.com>
---
drivers/net/bonding/bond_alb.c | 22 +++-------------------
drivers/net/bonding/bond_main.c | 26 ++------------------------
2 files changed, 5 insertions(+), 43 deletions(-)
diff --git a/drivers/net/bonding/bond_alb.c b/drivers/net/bonding/bond_alb.c
index 0287240..7870e4e 100644
--- a/drivers/net/bonding/bond_alb.c
+++ b/drivers/net/bonding/bond_alb.c
@@ -470,7 +470,7 @@ static void rlb_teach_disabled_mac_on_primary(struct bonding *bond, u8 addr[])
/* slave being removed should not be active at this point
*
- * Caller must hold bond lock for read
+ * Caller must hold rtnl.
*/
static void rlb_clear_slave(struct bonding *bond, struct slave *slave)
{
@@ -512,13 +512,9 @@ static void rlb_clear_slave(struct bonding *bond, struct slave *slave)
_unlock_rx_hashtbl_bh(bond);
- write_lock_bh(&bond->curr_slave_lock);
-
if (slave != bond->curr_active_slave) {
rlb_teach_disabled_mac_on_primary(bond, slave->dev->dev_addr);
}
-
- write_unlock_bh(&bond->curr_slave_lock);
}
static void rlb_update_client(struct rlb_client_info *client_info)
@@ -1680,15 +1676,10 @@ void bond_alb_handle_link_change(struct bonding *bond, struct slave *slave, char
* If new_slave is NULL, caller must hold curr_slave_lock or
* bond->lock for write.
*
- * If new_slave is not NULL, caller must hold RTNL, bond->lock for
- * read and curr_slave_lock for write. Processing here may sleep, so
- * no other locks may be held.
+ * If new_slave is not NULL, caller must hold RTNL, Processing
+ * here may sleep, so no other locks may be held.
*/
void bond_alb_handle_active_change(struct bonding *bond, struct slave *new_slave)
- __releases(&bond->curr_slave_lock)
- __releases(&bond->lock)
- __acquires(&bond->lock)
- __acquires(&bond->curr_slave_lock)
{
struct slave *swap_slave;
@@ -1722,9 +1713,6 @@ void bond_alb_handle_active_change(struct bonding *bond, struct slave *new_slave
tlb_clear_slave(bond, swap_slave, 1);
tlb_clear_slave(bond, new_slave, 1);
- write_unlock_bh(&bond->curr_slave_lock);
- read_unlock(&bond->lock);
-
ASSERT_RTNL();
/* in TLB mode, the slave might flip down/up with the old dev_addr,
@@ -1749,15 +1737,11 @@ void bond_alb_handle_active_change(struct bonding *bond, struct slave *new_slave
/* swap mac address */
alb_swap_mac_addr(swap_slave, new_slave);
alb_fasten_mac_swap(bond, swap_slave, new_slave);
- read_lock(&bond->lock);
} else {
/* set the new_slave to the bond mac address */
alb_set_slave_mac_addr(new_slave, bond->dev->dev_addr);
- read_lock(&bond->lock);
alb_send_learning_packets(new_slave, bond->dev->dev_addr);
}
-
- write_lock_bh(&bond->curr_slave_lock);
}
/*
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index a141f40..9c9803c 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -691,15 +691,11 @@ static void bond_set_dev_addr(struct net_device *bond_dev,
*
* Perform special MAC address swapping for fail_over_mac settings
*
- * Called with RTNL, bond->lock for read, curr_slave_lock for write_bh.
+ * Called with RTNL.
*/
static void bond_do_fail_over_mac(struct bonding *bond,
struct slave *new_active,
struct slave *old_active)
- __releases(&bond->curr_slave_lock)
- __releases(&bond->lock)
- __acquires(&bond->lock)
- __acquires(&bond->curr_slave_lock)
{
u8 tmp_mac[ETH_ALEN];
struct sockaddr saddr;
@@ -708,11 +704,7 @@ static void bond_do_fail_over_mac(struct bonding *bond,
switch (bond->params.fail_over_mac) {
case BOND_FOM_ACTIVE:
if (new_active) {
- write_unlock_bh(&bond->curr_slave_lock);
- read_unlock(&bond->lock);
bond_set_dev_addr(bond->dev, new_active->dev);
- read_lock(&bond->lock);
- write_lock_bh(&bond->curr_slave_lock);
}
break;
case BOND_FOM_FOLLOW:
@@ -724,9 +716,6 @@ static void bond_do_fail_over_mac(struct bonding *bond,
if (!new_active)
return;
- write_unlock_bh(&bond->curr_slave_lock);
- read_unlock(&bond->lock);
-
if (old_active) {
memcpy(tmp_mac, new_active->dev->dev_addr, ETH_ALEN);
memcpy(saddr.sa_data, old_active->dev->dev_addr,
@@ -755,8 +744,6 @@ static void bond_do_fail_over_mac(struct bonding *bond,
pr_err("%s: Error %d setting MAC of slave %s\n",
bond->dev->name, -rv, new_active->dev->name);
out:
- read_lock(&bond->lock);
- write_lock_bh(&bond->curr_slave_lock);
break;
default:
pr_err("%s: bond_do_fail_over_mac impossible: bad policy %d\n",
@@ -839,9 +826,6 @@ static bool bond_should_notify_peers(struct bonding *bond)
* If @new's link state is %BOND_LINK_BACK we'll set it to %BOND_LINK_UP,
* because it is apparently the best available slave we have, even though its
* updelay hasn't timed out yet.
- *
- * If new_active is not NULL, caller must hold bond->lock for read and
- * curr_slave_lock for write_bh.
*/
void bond_change_active_slave(struct bonding *bond, struct slave *new_active)
{
@@ -909,16 +893,10 @@ void bond_change_active_slave(struct bonding *bond, struct slave *new_active)
bond_should_notify_peers(bond);
}
- write_unlock_bh(&bond->curr_slave_lock);
- read_unlock(&bond->lock);
-
call_netdevice_notifiers(NETDEV_BONDING_FAILOVER, bond->dev);
if (should_notify_peers)
call_netdevice_notifiers(NETDEV_NOTIFY_PEERS,
bond->dev);
-
- read_lock(&bond->lock);
- write_lock_bh(&bond->curr_slave_lock);
}
}
@@ -943,7 +921,7 @@ void bond_change_active_slave(struct bonding *bond, struct slave *new_active)
* - The primary_slave has got its link back.
* - A slave has got its link back and there's no old curr_active_slave.
*
- * Caller must hold bond->lock for read and curr_slave_lock for write_bh.
+ * Caller must hold RTNL.
*/
void bond_select_active_slave(struct bonding *bond)
{
--
1.8.2.1
^ permalink raw reply related
* [PATCH net-next 9/9] bonding: remove unwanted lock for bond_store_primary_xxx()
From: Ding Tianhong @ 2013-11-06 6:53 UTC (permalink / raw)
To: Jay Vosburgh, Andy Gospodarek, David S. Miller,
Nikolay Aleksandrov, Veaceslav Falico, Netdev
The bond_select_active_slave() in bond_store_primary_xxx() is
already protected by RTNL, the bond lock and curr_slave_lock is
no effect here, so remove the unwanted lock.
Signed-off-by: Ding Tianhong <dingtianhong@huawei.com>
---
drivers/net/bonding/bond_sysfs.c | 8 --------
1 file changed, 8 deletions(-)
diff --git a/drivers/net/bonding/bond_sysfs.c b/drivers/net/bonding/bond_sysfs.c
index 47749c9..8f6b713 100644
--- a/drivers/net/bonding/bond_sysfs.c
+++ b/drivers/net/bonding/bond_sysfs.c
@@ -1068,8 +1068,6 @@ static ssize_t bonding_store_primary(struct device *d,
if (!rtnl_trylock())
return restart_syscall();
block_netpoll_tx();
- read_lock(&bond->lock);
- write_lock_bh(&bond->curr_slave_lock);
if (!USES_PRIMARY(bond->params.mode)) {
pr_info("%s: Unable to set primary slave; %s is in mode %d\n",
@@ -1107,8 +1105,6 @@ static ssize_t bonding_store_primary(struct device *d,
"but it has not been enslaved to %s yet.\n",
bond->dev->name, ifname, bond->dev->name);
out:
- write_unlock_bh(&bond->curr_slave_lock);
- read_unlock(&bond->lock);
unblock_netpoll_tx();
rtnl_unlock();
@@ -1156,11 +1152,7 @@ static ssize_t bonding_store_primary_reselect(struct device *d,
new_value);
block_netpoll_tx();
- read_lock(&bond->lock);
- write_lock_bh(&bond->curr_slave_lock);
bond_select_active_slave(bond);
- write_unlock_bh(&bond->curr_slave_lock);
- read_unlock(&bond->lock);
unblock_netpoll_tx();
out:
rtnl_unlock();
--
1.8.2.1
^ permalink raw reply related
* [PATCH net-next 4/9] bonding: rebuild the lock use for bond_loadbalance_arp_mon()
From: Ding Tianhong @ 2013-11-06 6:52 UTC (permalink / raw)
To: Jay Vosburgh, Andy Gospodarek, David S. Miller,
Nikolay Aleksandrov, Veaceslav Falico, Netdev
The bond_loadbalance_arp_mon() use the bond lock to protect the
bond slave list, it is no effect, so I could use RTNL or RCU to
replace it, the bond_select_active_slave() need RTNL, so whatever
the RCU use, the RTNL is need to add, so I choose the RTNL to
protect the whole monitor, I still remove the curr_slave_lock for
it, because RTNL is enough here.
Signed-off-by: Ding Tianhong <dingtianhong@huawei.com>
---
drivers/net/bonding/bond_main.c | 14 ++++++--------
1 file changed, 6 insertions(+), 8 deletions(-)
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 98171eb..b875dff 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -2376,10 +2376,13 @@ void bond_loadbalance_arp_mon(struct work_struct *work)
struct list_head *iter;
int do_failover = 0;
- read_lock(&bond->lock);
+ if (!rtnl_trylock())
+ goto re_arm;
- if (!bond_has_slaves(bond))
+ if (!bond_has_slaves(bond)) {
+ rtnl_unlock();
goto re_arm;
+ }
oldcurrent = bond->curr_active_slave;
/* see if any of the previous devices are up now (i.e. they have
@@ -2454,20 +2457,15 @@ void bond_loadbalance_arp_mon(struct work_struct *work)
if (do_failover) {
block_netpoll_tx();
- write_lock_bh(&bond->curr_slave_lock);
-
bond_select_active_slave(bond);
-
- write_unlock_bh(&bond->curr_slave_lock);
unblock_netpoll_tx();
}
+ rtnl_unlock();
re_arm:
if (bond->params.arp_interval)
queue_delayed_work(bond->wq, &bond->arp_work,
msecs_to_jiffies(bond->params.arp_interval));
-
- read_unlock(&bond->lock);
}
/*
--
1.8.2.1
^ permalink raw reply related
* [PATCH] ipv6: replace RTF_ROUTEINFO with RTF_ADDRCONF in rt6_get_route_info()
From: Duan Jiong @ 2013-11-06 7:26 UTC (permalink / raw)
To: David Miller; +Cc: netdev
As the rfc 4191 said, the Router Preference and Lifetime values in a
::/0 Route Information Option should override the preference and lifetime
values in the Router Advertisement header. But when the kernel deals with
a ::/0 Route Information Option, the rt6_get_route_info() always return
NULL, that means that overriding will not happen, because those default
routers were added without flag RTF_ROUTEINFO in rt6_add_dflt_router().
In order to match those default routers, we can replace RTF_ROUTEINFO
with RTF_ADDRCONF in rt6_get_route_info().
Signed-off-by: Duan Jiong <duanj.fnst@cn.fujitsu.com>
---
net/ipv6/route.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index 04e17b3..549aa3b 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -1940,7 +1940,8 @@ static struct rt6_info *rt6_get_route_info(struct net *net,
for (rt = fn->leaf; rt; rt = rt->dst.rt6_next) {
if (rt->dst.dev->ifindex != ifindex)
continue;
- if ((rt->rt6i_flags & (RTF_ROUTEINFO|RTF_GATEWAY)) != (RTF_ROUTEINFO|RTF_GATEWAY))
+ if ((rt->rt6i_flags & (RTF_ADDRCONF|RTF_GATEWAY)) !=
+ (RTF_ADDRCONF|RTF_GATEWAY))
continue;
if (!ipv6_addr_equal(&rt->rt6i_gateway, gwaddr))
continue;
--
1.8.3.1
^ permalink raw reply related
* Re: [PATCH] wcn36xx: Fix logging macro with unnecessary semicolon
From: Eugene Krasnikov @ 2013-11-06 7:49 UTC (permalink / raw)
To: Joe Perches
Cc: John W. Linville, wcn36xx, linux-wireless, netdev,
linux-kernel@vger.kernel.org
In-Reply-To: <1383684000.4387.42.camel@joe-AO722>
Hm... when it comes to semicolon the patch seems to be good. When it
comes to dynamic debugging i think we should have a separate
discussion about that.
I personally like the whole idea about dynamic debug but if you want
to change it i would suggest to have some kind of framework for all
ath drivers(or maybe all wireless drivers). Because obviously you can
find common code in every driver that defines it's own debug
functions/debug levels and so on. Why not to make a framework with
standard API/levels?
On Tue, Nov 5, 2013 at 8:40 PM, Joe Perches <joe@perches.com> wrote:
> The wcn36xx_err macro should not end in a semicolon as
> there are 2 consecutive semicolons in the preprocessed
> output.
>
> Remove it.
>
> Neaten the logging macros to use a more common style.
> Convert to use pr_debug to enable dynamic debug.
>
> Signed-off-by: Joe Perches <joe@perches.com>
> ---
> drivers/net/wireless/ath/wcn36xx/wcn36xx.h | 28 +++++++++++++---------------
> 1 file changed, 13 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/net/wireless/ath/wcn36xx/wcn36xx.h b/drivers/net/wireless/ath/wcn36xx/wcn36xx.h
> index 58b6383..d157bb7 100644
> --- a/drivers/net/wireless/ath/wcn36xx/wcn36xx.h
> +++ b/drivers/net/wireless/ath/wcn36xx/wcn36xx.h
> @@ -53,22 +53,20 @@ enum wcn36xx_debug_mask {
> WCN36XX_DBG_ANY = 0xffffffff,
> };
>
> -#define wcn36xx_err(fmt, arg...) \
> - printk(KERN_ERR pr_fmt("ERROR " fmt), ##arg);
> -
> -#define wcn36xx_warn(fmt, arg...) \
> - printk(KERN_WARNING pr_fmt("WARNING " fmt), ##arg)
> -
> -#define wcn36xx_info(fmt, arg...) \
> - printk(KERN_INFO pr_fmt(fmt), ##arg)
> -
> -#define wcn36xx_dbg(mask, fmt, arg...) do { \
> - if (wcn36xx_dbg_mask & mask) \
> - printk(KERN_DEBUG pr_fmt(fmt), ##arg); \
> +#define wcn36xx_err(fmt, ...) \
> + pr_err("ERROR " fmt, ##__VA_ARGS__)
> +#define wcn36xx_warn(fmt, ...) \
> + pr_warn("WARNING " fmt, ##__VA_ARGS__)
> +#define wcn36xx_info(fmt, ...) \
> + pr_info(fmt, ##__VA_ARGS__)
> +#define wcn36xx_dbg(mask, fmt, ...) \
> +do { \
> + if (wcn36xx_dbg_mask & mask) \
> + pr_debug(fmt, ##__VA_ARGS__); \
> } while (0)
> -
> -#define wcn36xx_dbg_dump(mask, prefix_str, buf, len) do { \
> - if (wcn36xx_dbg_mask & mask) \
> +#define wcn36xx_dbg_dump(mask, prefix_str, buf, len) \
> +do { \
> + if (wcn36xx_dbg_mask & mask) \
> print_hex_dump(KERN_DEBUG, pr_fmt(prefix_str), \
> DUMP_PREFIX_OFFSET, 32, 1, \
> buf, len, false); \
>
>
--
Best regards,
Eugene
^ 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