* [PATCH net-next 4/4] bonding: Do not ignore notifications for ARP-work-queue
@ 2015-03-12 5:54 Mahesh Bandewar
2015-03-12 16:12 ` Nikolay Aleksandrov
0 siblings, 1 reply; 3+ messages in thread
From: Mahesh Bandewar @ 2015-03-12 5:54 UTC (permalink / raw)
To: Jay Vosburgh, Andy Gospodarek, Veaceslav Falico,
Nikolay Aleksandrov, David Miller
Cc: Mahesh Bandewar, Maciej Zenczykowski, netdev, Eric Dumazet
This patch adds code to reschedule the ARP-work (aggressively)
to handle the notifications before resuming the regular cycle.
Signed-off-by: Mahesh Bandewar <maheshb@google.com>
---
drivers/net/bonding/bond_main.c | 38 ++++++++++++++++++++++----------------
1 file changed, 22 insertions(+), 16 deletions(-)
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 54ecb7a22bae..882974d543d2 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -2814,17 +2814,20 @@ static void bond_activebackup_arp_mon(struct work_struct *work)
arp_work.work);
bool should_notify_peers = false;
bool should_notify_rtnl = false;
- int delta_in_ticks;
+ unsigned long delta_in_ticks;
delta_in_ticks = msecs_to_jiffies(bond->params.arp_interval);
if (!bond_has_slaves(bond))
goto re_arm;
- rcu_read_lock();
-
should_notify_peers = bond_should_notify_peers(bond);
+ if (bond_get_notif_pending(bond, BOND_ARP_NOTIF)) {
+ rcu_read_lock();
+ goto eval_arp_probe;
+ }
+ rcu_read_lock();
if (bond_ab_arp_inspect(bond)) {
rcu_read_unlock();
@@ -2841,25 +2844,28 @@ static void bond_activebackup_arp_mon(struct work_struct *work)
rcu_read_lock();
}
+eval_arp_probe:
should_notify_rtnl = bond_ab_arp_probe(bond);
rcu_read_unlock();
re_arm:
- if (bond->params.arp_interval)
- queue_delayed_work(bond->wq, &bond->arp_work, delta_in_ticks);
-
if (should_notify_peers || should_notify_rtnl) {
- if (!rtnl_trylock())
- return;
-
- if (should_notify_peers)
- call_netdevice_notifiers(NETDEV_NOTIFY_PEERS,
- bond->dev);
- if (should_notify_rtnl)
- bond_slave_state_notify(bond);
-
- rtnl_unlock();
+ if (!rtnl_trylock()) {
+ delta_in_ticks = 1;
+ bond_set_notif_pending(bond, BOND_ARP_NOTIF, 1);
+ } else {
+ if (should_notify_rtnl)
+ bond_slave_state_notify(bond);
+ if (should_notify_peers)
+ call_netdevice_notifiers(NETDEV_NOTIFY_PEERS,
+ bond->dev);
+ rtnl_unlock();
+ bond_set_notif_pending(bond, BOND_ARP_NOTIF, 0);
+ }
}
+
+ if (bond->params.arp_interval)
+ queue_delayed_work(bond->wq, &bond->arp_work, delta_in_ticks);
}
/*-------------------------- netdev event handling --------------------------*/
--
2.2.0.rc0.207.ga3a616c
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH net-next 4/4] bonding: Do not ignore notifications for ARP-work-queue
2015-03-12 5:54 [PATCH net-next 4/4] bonding: Do not ignore notifications for ARP-work-queue Mahesh Bandewar
@ 2015-03-12 16:12 ` Nikolay Aleksandrov
2015-03-13 2:03 ` Mahesh Bandewar
0 siblings, 1 reply; 3+ messages in thread
From: Nikolay Aleksandrov @ 2015-03-12 16:12 UTC (permalink / raw)
To: Mahesh Bandewar, Jay Vosburgh, Andy Gospodarek, Veaceslav Falico,
David Miller
Cc: Maciej Zenczykowski, netdev, Eric Dumazet
On 03/12/2015 06:54 AM, Mahesh Bandewar wrote:
> This patch adds code to reschedule the ARP-work (aggressively)
> to handle the notifications before resuming the regular cycle.
>
> Signed-off-by: Mahesh Bandewar <maheshb@google.com>
> ---
> drivers/net/bonding/bond_main.c | 38 ++++++++++++++++++++++----------------
> 1 file changed, 22 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
> index 54ecb7a22bae..882974d543d2 100644
> --- a/drivers/net/bonding/bond_main.c
> +++ b/drivers/net/bonding/bond_main.c
> @@ -2814,17 +2814,20 @@ static void bond_activebackup_arp_mon(struct work_struct *work)
> arp_work.work);
> bool should_notify_peers = false;
> bool should_notify_rtnl = false;
> - int delta_in_ticks;
> + unsigned long delta_in_ticks;
>
> delta_in_ticks = msecs_to_jiffies(bond->params.arp_interval);
>
> if (!bond_has_slaves(bond))
> goto re_arm;
>
> - rcu_read_lock();
> -
> should_notify_peers = bond_should_notify_peers(bond);
> + if (bond_get_notif_pending(bond, BOND_ARP_NOTIF)) {
> + rcu_read_lock();
> + goto eval_arp_probe;
> + }
>
> + rcu_read_lock();
^^^^^^^
Since rcu_read_lock() is acquired in both cases, why don't you leave it
where it is now ? Then you'll be able to save a line and drop the { }
on the "if" above.
> if (bond_ab_arp_inspect(bond)) {
> rcu_read_unlock();
>
> @@ -2841,25 +2844,28 @@ static void bond_activebackup_arp_mon(struct work_struct *work)
> rcu_read_lock();
> }
>
> +eval_arp_probe:
> should_notify_rtnl = bond_ab_arp_probe(bond);
^^^^^
Keep in mind that bond_ab_arp_probe() calls bond_arp_send_all() each time
if we have an active slave. We could be sending ARP requests each tick
until rtnl gets acquired.
> rcu_read_unlock();
>
> re_arm:
> - if (bond->params.arp_interval)
> - queue_delayed_work(bond->wq, &bond->arp_work, delta_in_ticks);
> -
> if (should_notify_peers || should_notify_rtnl) {
> - if (!rtnl_trylock())
> - return;
> -
> - if (should_notify_peers)
> - call_netdevice_notifiers(NETDEV_NOTIFY_PEERS,
> - bond->dev);
> - if (should_notify_rtnl)
> - bond_slave_state_notify(bond);
> -
> - rtnl_unlock();
> + if (!rtnl_trylock()) {
> + delta_in_ticks = 1;
> + bond_set_notif_pending(bond, BOND_ARP_NOTIF, 1);
> + } else {
> + if (should_notify_rtnl)
> + bond_slave_state_notify(bond);
> + if (should_notify_peers)
> + call_netdevice_notifiers(NETDEV_NOTIFY_PEERS,
> + bond->dev);
> + rtnl_unlock();
> + bond_set_notif_pending(bond, BOND_ARP_NOTIF, 0);
> + }
> }
> +
> + if (bond->params.arp_interval)
> + queue_delayed_work(bond->wq, &bond->arp_work, delta_in_ticks);
> }
>
> /*-------------------------- netdev event handling --------------------------*/
>
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH net-next 4/4] bonding: Do not ignore notifications for ARP-work-queue
2015-03-12 16:12 ` Nikolay Aleksandrov
@ 2015-03-13 2:03 ` Mahesh Bandewar
0 siblings, 0 replies; 3+ messages in thread
From: Mahesh Bandewar @ 2015-03-13 2:03 UTC (permalink / raw)
To: Nikolay Aleksandrov
Cc: Jay Vosburgh, Andy Gospodarek, Veaceslav Falico, David Miller,
Maciej Zenczykowski, netdev, Eric Dumazet
On Thu, Mar 12, 2015 at 9:12 AM, Nikolay Aleksandrov <nikolay@redhat.com> wrote:
> On 03/12/2015 06:54 AM, Mahesh Bandewar wrote:
>> This patch adds code to reschedule the ARP-work (aggressively)
>> to handle the notifications before resuming the regular cycle.
>>
>> Signed-off-by: Mahesh Bandewar <maheshb@google.com>
>> ---
>> drivers/net/bonding/bond_main.c | 38 ++++++++++++++++++++++----------------
>> 1 file changed, 22 insertions(+), 16 deletions(-)
>>
>> diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
>> index 54ecb7a22bae..882974d543d2 100644
>> --- a/drivers/net/bonding/bond_main.c
>> +++ b/drivers/net/bonding/bond_main.c
>> @@ -2814,17 +2814,20 @@ static void bond_activebackup_arp_mon(struct work_struct *work)
>> arp_work.work);
>> bool should_notify_peers = false;
>> bool should_notify_rtnl = false;
>> - int delta_in_ticks;
>> + unsigned long delta_in_ticks;
>>
>> delta_in_ticks = msecs_to_jiffies(bond->params.arp_interval);
>>
>> if (!bond_has_slaves(bond))
>> goto re_arm;
>>
>> - rcu_read_lock();
>> -
>> should_notify_peers = bond_should_notify_peers(bond);
>> + if (bond_get_notif_pending(bond, BOND_ARP_NOTIF)) {
>> + rcu_read_lock();
>> + goto eval_arp_probe;
>> + }
>>
>> + rcu_read_lock();
> ^^^^^^^
> Since rcu_read_lock() is acquired in both cases, why don't you leave it
> where it is now ? Then you'll be able to save a line and drop the { }
> on the "if" above.
>
OK, I was under false impression of not-needing-rcu for
should_notify_peers() and hence removed the rcu_read_lock() from the
original location. I'll reinstate.
>
>> if (bond_ab_arp_inspect(bond)) {
>> rcu_read_unlock();
>>
>> @@ -2841,25 +2844,28 @@ static void bond_activebackup_arp_mon(struct work_struct *work)
>> rcu_read_lock();
>> }
>>
>> +eval_arp_probe:
>> should_notify_rtnl = bond_ab_arp_probe(bond);
> ^^^^^
> Keep in mind that bond_ab_arp_probe() calls bond_arp_send_all() each time
> if we have an active slave. We could be sending ARP requests each tick
> until rtnl gets acquired.
>
well, that will not be a good behavior however seldom it would be.
I'll update code to avoid that in the next patch-set.
>> rcu_read_unlock();
>>
>> re_arm:
>> - if (bond->params.arp_interval)
>> - queue_delayed_work(bond->wq, &bond->arp_work, delta_in_ticks);
>> -
>> if (should_notify_peers || should_notify_rtnl) {
>> - if (!rtnl_trylock())
>> - return;
>> -
>> - if (should_notify_peers)
>> - call_netdevice_notifiers(NETDEV_NOTIFY_PEERS,
>> - bond->dev);
>> - if (should_notify_rtnl)
>> - bond_slave_state_notify(bond);
>> -
>> - rtnl_unlock();
>> + if (!rtnl_trylock()) {
>> + delta_in_ticks = 1;
>> + bond_set_notif_pending(bond, BOND_ARP_NOTIF, 1);
>> + } else {
>> + if (should_notify_rtnl)
>> + bond_slave_state_notify(bond);
>> + if (should_notify_peers)
>> + call_netdevice_notifiers(NETDEV_NOTIFY_PEERS,
>> + bond->dev);
>> + rtnl_unlock();
>> + bond_set_notif_pending(bond, BOND_ARP_NOTIF, 0);
>> + }
>> }
>> +
>> + if (bond->params.arp_interval)
>> + queue_delayed_work(bond->wq, &bond->arp_work, delta_in_ticks);
>> }
>>
>> /*-------------------------- netdev event handling --------------------------*/
>>
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2015-03-13 2:04 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-03-12 5:54 [PATCH net-next 4/4] bonding: Do not ignore notifications for ARP-work-queue Mahesh Bandewar
2015-03-12 16:12 ` Nikolay Aleksandrov
2015-03-13 2:03 ` Mahesh Bandewar
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).