* Re: [PATCH net-next-2.6 3/3] bonding,ipv4,ipv6,vlan: Handle NETDEV_BONDING_FAILOVER like NETDEV_NOTIFY_PEERS
From: Jay Vosburgh @ 2011-04-19 19:12 UTC (permalink / raw)
To: Ben Hutchings
Cc: David Miller, Andy Gospodarek, Patrick McHardy, netdev,
Brian Haley
In-Reply-To: <1303153792.2857.32.camel@bwh-desktop>
Ben Hutchings <bhutchings@solarflare.com> wrote:
>On Fri, 2011-04-15 at 17:30 -0700, Jay Vosburgh wrote:
>> Ben Hutchings <bhutchings@solarflare.com> wrote:
>>
>> >It is undesirable for the bonding driver to be poking into higher
>> >level protocols, and notifiers provide a way to avoid that. This does
>> >mean removing the ability to configure reptitition of gratuitous ARPs
>> >and unsolicited NAs.
>>
>> In principle I think this is a good thing (getting rid of some
>> of those dependencies, duplicated code, etc).
>>
>> However, the removal of the multiple grat ARP and NAs may be an
>> issue for some users. I don't know that we can just remove this (along
>> with its API) without going through the feature removal process.
>
>Right.
>
>> As I recall, the multiple gratuitous ARP stuff was added for
>> Infiniband, because it is dependent on the grat ARP for a smooth
>> failover.
>>
>> There is also currently logic to check the linkwatch link state
>> to wait for the link to go up prior to sending a grat ARP; this is also
>> for IB.
>
>Why would we activate a slave without link up? Perhaps if the previous
>active slave is removed?
It's special sauce for Infiniband; I don't recall the details
except that the submitter said that without it the initial gratuitous
ARP could be lost. I didn't (and still don't) have IB hardware to test
this on.
Ideally, I'd like to test the current situation (no checking of
linkwatch) for IB and see if the linkwatch trick is still necessary. It
may have been due to some behavior of the device driver that is now
different. For now, though, keeping it (as you've done) just in case
seems prudent.
>> Brian Haley added the unsolicited NAs; I've added him to the cc
>> so perhaps he (or somebody else) can comment on the necessity of keeping
>> the ability to send multiple NAs.
>[...]
>
>How about restoring the parameters like this:
I think the patch below is better than Brian's suggestion (new
single parameter) because it won't break existing configurations, even
though it is doing magic under the covers. If the magic is a real
concern, we could add logic to detect when both parameters are used and
set to different values, but I'm not really that worked up about it as
long as the magic is clearly documented.
>---
>From: Ben Hutchings <bhutchings@solarflare.com>
>Date: Mon, 18 Apr 2011 19:36:48 +0100
>Subject: [PATCH net-next-2.6] ipv4,ipv6,bonding: Restore control over number of peer notifications
>
>For backward compatibility, we should retain the module parameters and
>sysfs attributes to control the number of peer notifications
>(gratuitous ARPs and unsolicited NAs) sent after bonding failover.
>Also, it is possible for failover to take place even though the new
>active slave does not have link up, and in that case the peer
>notification should be deferred until it does.
>
>Change ipv4 and ipv6 so they do not automatically send peer
>notifications on bonding failover. Change the bonding driver to send
>separate NETDEV_NOTIFY_PEERS notifications when the link is up, as
>many times as requested. Since it does not directly control which
>protocols send notifications, make num_grat_arp and num_unsol_na
>aliases for a single parameter.
>
>Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
>---
> Documentation/networking/bonding.txt | 31 ++++++++----------
> drivers/net/bonding/bond_main.c | 59 ++++++++++++++++++++++++++++++++++
> drivers/net/bonding/bond_sysfs.c | 26 +++++++++++++++
> drivers/net/bonding/bonding.h | 2 +
> net/ipv4/devinet.c | 1 -
> net/ipv6/ndisc.c | 1 -
> 6 files changed, 101 insertions(+), 19 deletions(-)
>
>diff --git a/Documentation/networking/bonding.txt b/Documentation/networking/bonding.txt
>index e27202b..511b4e5 100644
>--- a/Documentation/networking/bonding.txt
>+++ b/Documentation/networking/bonding.txt
>@@ -1,7 +1,7 @@
>
> Linux Ethernet Bonding Driver HOWTO
>
>- Latest update: 23 September 2009
>+ Latest update: 18 April 2011
>
> Initial release : Thomas Davis <tadavis at lbl.gov>
> Corrections, HA extensions : 2000/10/03-15 :
>@@ -585,25 +585,22 @@ mode
> chosen.
>
> num_grat_arp
>-
>- Specifies the number of gratuitous ARPs to be issued after a
>- failover event. One gratuitous ARP is issued immediately after
>- the failover, subsequent ARPs are sent at a rate of one per link
>- monitor interval (arp_interval or miimon, whichever is active).
>-
>- The valid range is 0 - 255; the default value is 1. This option
>- affects only the active-backup mode. This option was added for
>- bonding version 3.3.0.
>-
> num_unsol_na
>
>- Specifies the number of unsolicited IPv6 Neighbor Advertisements
>- to be issued after a failover event. One unsolicited NA is issued
>- immediately after the failover.
>+ Specify the number of peer notifications (gratuitous ARPs and
>+ unsolicited IPv6 Neighbor Advertisements) to be issued after a
>+ failover event. As soon as the link is up on the new slave
>+ (possibly immediately) a peer notification is sent on the
>+ bonding device and each VLAN sub-device. This is repeated at
>+ each link monitor interval (arp_interval or miimon, whichever
>+ is active) if the number is greater than 1.
>+
>+ These notifications are now generated by the ipv4 and ipv6 code
>+ and the numbers of repetitions cannot be set independently.
I think it would be good for the long run to be specific about
"now," i.e., bump the bonding version and specify that here.
>- The valid range is 0 - 255; the default value is 1. This option
>- affects only the active-backup mode. This option was added for
>- bonding version 3.4.0.
>+ The valid range is 0 - 255; the default value is 1. These options
>+ affect only the active-backup mode. These options were added for
>+ bonding versions 3.3.0 and 3.4.0 respectively.
>
> primary
>
>diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
>index 5cd4766..c9043db 100644
>--- a/drivers/net/bonding/bond_main.c
>+++ b/drivers/net/bonding/bond_main.c
>@@ -89,6 +89,7 @@
>
> static int max_bonds = BOND_DEFAULT_MAX_BONDS;
> static int tx_queues = BOND_DEFAULT_TX_QUEUES;
>+static int num_peer_notif = 1;
> static int miimon = BOND_LINK_MON_INTERV;
> static int updelay;
> static int downdelay;
>@@ -111,6 +112,10 @@ module_param(max_bonds, int, 0);
> MODULE_PARM_DESC(max_bonds, "Max number of bonded devices");
> module_param(tx_queues, int, 0);
> MODULE_PARM_DESC(tx_queues, "Max number of transmit queues (default = 16)");
>+module_param_named(num_grat_arp, num_peer_notif, int, 0644);
>+MODULE_PARM_DESC(num_grat_arp, "Number of peer notifications to send on failover event");
>+module_param_named(num_unsol_na, num_peer_notif, int, 0644);
>+MODULE_PARM_DESC(num_unsol_na, "Number of peer notifications to send on failover event");
Perhaps add "alias of num_grat_arp" here so it shows up in modinfo?
In any event, with the version number thing I mentioned above, I
think this is a reasonable way to proceed.
-J
Signed-off-by: Jay Vosburgh <fubar@us.ibm.com>
> module_param(miimon, int, 0);
> MODULE_PARM_DESC(miimon, "Link check interval in milliseconds");
> module_param(updelay, int, 0);
>@@ -1080,6 +1085,21 @@ static struct slave *bond_find_best_slave(struct bonding *bond)
> return bestslave;
> }
>
>+static bool bond_should_notify_peers(struct bonding *bond)
>+{
>+ struct slave *slave = bond->curr_active_slave;
>+
>+ pr_debug("bond_should_notify_peers: bond %s slave %s\n",
>+ bond->dev->name, slave ? slave->dev->name : "NULL");
>+
>+ if (!slave || !bond->send_peer_notif ||
>+ test_bit(__LINK_STATE_LINKWATCH_PENDING, &slave->dev->state))
>+ return false;
>+
>+ bond->send_peer_notif--;
>+ return true;
>+}
>+
> /**
> * change_active_interface - change the active slave into the specified one
> * @bond: our bonding struct
>@@ -1147,16 +1167,28 @@ void bond_change_active_slave(struct bonding *bond, struct slave *new_active)
> bond_set_slave_inactive_flags(old_active);
>
> if (new_active) {
>+ bool should_notify_peers = false;
>+
> bond_set_slave_active_flags(new_active);
>
> if (bond->params.fail_over_mac)
> bond_do_fail_over_mac(bond, new_active,
> old_active);
>
>+ if (netif_running(bond->dev)) {
>+ bond->send_peer_notif =
>+ bond->params.num_peer_notif;
>+ should_notify_peers =
>+ bond_should_notify_peers(bond);
>+ }
>+
> write_unlock_bh(&bond->curr_slave_lock);
> read_unlock(&bond->lock);
>
> netdev_bonding_change(bond->dev, NETDEV_BONDING_FAILOVER);
>+ if (should_notify_peers)
>+ netdev_bonding_change(bond->dev,
>+ NETDEV_NOTIFY_PEERS);
>
> read_lock(&bond->lock);
> write_lock_bh(&bond->curr_slave_lock);
>@@ -2555,6 +2587,7 @@ void bond_mii_monitor(struct work_struct *work)
> {
> struct bonding *bond = container_of(work, struct bonding,
> mii_work.work);
>+ bool should_notify_peers = false;
>
> read_lock(&bond->lock);
> if (bond->kill_timers)
>@@ -2563,6 +2596,8 @@ void bond_mii_monitor(struct work_struct *work)
> if (bond->slave_cnt == 0)
> goto re_arm;
>
>+ should_notify_peers = bond_should_notify_peers(bond);
>+
> if (bond_miimon_inspect(bond)) {
> read_unlock(&bond->lock);
> rtnl_lock();
>@@ -2581,6 +2616,12 @@ re_arm:
> msecs_to_jiffies(bond->params.miimon));
> out:
> read_unlock(&bond->lock);
>+
>+ if (should_notify_peers) {
>+ rtnl_lock();
>+ netdev_bonding_change(bond->dev, NETDEV_NOTIFY_PEERS);
>+ rtnl_unlock();
>+ }
> }
>
> static __be32 bond_glean_dev_ip(struct net_device *dev)
>@@ -3178,6 +3219,7 @@ 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);
>@@ -3190,6 +3232,8 @@ void bond_activebackup_arp_mon(struct work_struct *work)
> if (bond->slave_cnt == 0)
> goto re_arm;
>
>+ should_notify_peers = bond_should_notify_peers(bond);
>+
> if (bond_ab_arp_inspect(bond, delta_in_ticks)) {
> read_unlock(&bond->lock);
> rtnl_lock();
>@@ -3209,6 +3253,12 @@ re_arm:
> queue_delayed_work(bond->wq, &bond->arp_work, delta_in_ticks);
> out:
> read_unlock(&bond->lock);
>+
>+ if (should_notify_peers) {
>+ rtnl_lock();
>+ netdev_bonding_change(bond->dev, NETDEV_NOTIFY_PEERS);
>+ rtnl_unlock();
>+ }
> }
>
> /*-------------------------- netdev event handling --------------------------*/
>@@ -3568,6 +3618,8 @@ static int bond_close(struct net_device *bond_dev)
>
> write_lock_bh(&bond->lock);
>
>+ bond->send_peer_notif = 0;
>+
> /* signal timers not to re-arm */
> bond->kill_timers = 1;
>
>@@ -4644,6 +4696,12 @@ static int bond_check_params(struct bond_params *params)
> use_carrier = 1;
> }
>
>+ if (num_peer_notif < 0 || num_peer_notif > 255) {
>+ pr_warning("Warning: num_grat_arp/num_unsol_na (%d) not in range 0-255 so it was reset to 1\n",
>+ num_peer_notif);
>+ num_peer_notif = 1;
>+ }
>+
> /* reset values for 802.3ad */
> if (bond_mode == BOND_MODE_8023AD) {
> if (!miimon) {
>@@ -4833,6 +4891,7 @@ static int bond_check_params(struct bond_params *params)
> params->mode = bond_mode;
> params->xmit_policy = xmit_hashtype;
> params->miimon = miimon;
>+ params->num_peer_notif = num_peer_notif;
> params->arp_interval = arp_interval;
> params->arp_validate = arp_validate_value;
> params->updelay = updelay;
>diff --git a/drivers/net/bonding/bond_sysfs.c b/drivers/net/bonding/bond_sysfs.c
>index 259ff32..58fb3e9 100644
>--- a/drivers/net/bonding/bond_sysfs.c
>+++ b/drivers/net/bonding/bond_sysfs.c
>@@ -874,6 +874,30 @@ static DEVICE_ATTR(ad_select, S_IRUGO | S_IWUSR,
> bonding_show_ad_select, bonding_store_ad_select);
>
> /*
>+ * Show and set the number of peer notifications to send after a failover event.
>+ */
>+static ssize_t bonding_show_num_peer_notif(struct device *d,
>+ struct device_attribute *attr,
>+ char *buf)
>+{
>+ struct bonding *bond = to_bond(d);
>+ return sprintf(buf, "%d\n", bond->params.num_peer_notif);
>+}
>+
>+static ssize_t bonding_store_num_peer_notif(struct device *d,
>+ struct device_attribute *attr,
>+ const char *buf, size_t count)
>+{
>+ struct bonding *bond = to_bond(d);
>+ int err = kstrtou8(buf, 10, &bond->params.num_peer_notif);
>+ return err ? err : count;
>+}
>+static DEVICE_ATTR(num_grat_arp, S_IRUGO | S_IWUSR,
>+ bonding_show_num_peer_notif, bonding_store_num_peer_notif);
>+static DEVICE_ATTR(num_unsol_na, S_IRUGO | S_IWUSR,
>+ bonding_show_num_peer_notif, bonding_store_num_peer_notif);
>+
>+/*
> * Show and set the MII monitor interval. There are two tricky bits
> * here. First, if MII monitoring is activated, then we must disable
> * ARP monitoring. Second, if the timer isn't running, we must
>@@ -1572,6 +1596,8 @@ static struct attribute *per_bond_attrs[] = {
> &dev_attr_lacp_rate.attr,
> &dev_attr_ad_select.attr,
> &dev_attr_xmit_hash_policy.attr,
>+ &dev_attr_num_grat_arp.attr,
>+ &dev_attr_num_unsol_na.attr,
> &dev_attr_miimon.attr,
> &dev_attr_primary.attr,
> &dev_attr_primary_reselect.attr,
>diff --git a/drivers/net/bonding/bonding.h b/drivers/net/bonding/bonding.h
>index 77180b1..5bf3b89 100644
>--- a/drivers/net/bonding/bonding.h
>+++ b/drivers/net/bonding/bonding.h
>@@ -149,6 +149,7 @@ struct bond_params {
> int mode;
> int xmit_policy;
> int miimon;
>+ u8 num_peer_notif;
> int arp_interval;
> int arp_validate;
> int use_carrier;
>@@ -229,6 +230,7 @@ struct bonding {
> rwlock_t lock;
> rwlock_t curr_slave_lock;
> s8 kill_timers;
>+ u8 send_peer_notif;
> s8 setup_by_slave;
> s8 igmp_retrans;
> #ifdef CONFIG_PROC_FS
>diff --git a/net/ipv4/devinet.c b/net/ipv4/devinet.c
>index acf553f..5345b0b 100644
>--- a/net/ipv4/devinet.c
>+++ b/net/ipv4/devinet.c
>@@ -1203,7 +1203,6 @@ static int inetdev_event(struct notifier_block *this, unsigned long event,
> break;
> /* fall through */
> case NETDEV_NOTIFY_PEERS:
>- case NETDEV_BONDING_FAILOVER:
> /* Send gratuitous ARP to notify of link change */
> inetdev_send_gratuitous_arp(dev, in_dev);
> break;
>diff --git a/net/ipv6/ndisc.c b/net/ipv6/ndisc.c
>index 6f7d491..a51fa74c 100644
>--- a/net/ipv6/ndisc.c
>+++ b/net/ipv6/ndisc.c
>@@ -1746,7 +1746,6 @@ static int ndisc_netdev_event(struct notifier_block *this, unsigned long event,
> fib6_run_gc(~0UL, net);
> break;
> case NETDEV_NOTIFY_PEERS:
>- case NETDEV_BONDING_FAILOVER:
> ndisc_send_unsol_na(dev);
> break;
> default:
>--
>1.7.4
>
>
>--
>Ben Hutchings, Senior Software Engineer, Solarflare
>Not speaking for my employer; that's the marketing department's job.
>They asked us to note that Solarflare product names are trademarked.
---
-Jay Vosburgh, IBM Linux Technology Center, fubar@us.ibm.com
^ permalink raw reply
* Re: [PATCH 0/3] netfilter: netfilter fixes for 2.6.39-rc4
From: David Miller @ 2011-04-19 19:05 UTC (permalink / raw)
To: kaber; +Cc: netfilter-devel, netdev
In-Reply-To: <1303224705-17400-1-git-send-email-kaber@trash.net>
From: kaber@trash.net
Date: Tue, 19 Apr 2011 16:51:42 +0200
> Hi Dave,
>
> following are three netfilter fixes for 2.6.39-rc4, containing:
>
> - a fix for the bitmap:ip,mac set type to require the src/dst parameter
> to be set to src, from Jozsef
>
> - a fix to make --del-set of the SET target work, from Jozsef
>
> - a patch to fix the order in which sets are dumped, from Jozsef
>
> Please pull from:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/kaber/nf-2.6.git master
Pulled, thanks!
^ permalink raw reply
* Re: 2.6.38 dev_watchdog WARNING
From: Ben Hutchings @ 2011-04-19 18:49 UTC (permalink / raw)
To: tim.gardner; +Cc: netdev
In-Reply-To: <4DADC8F2.9050700@canonical.com>
On Tue, 2011-04-19 at 11:40 -0600, Tim Gardner wrote:
> I'm seeing a lot of these kinds of bugs: WARNING: at
> /build/buildd/linux-2.6.38/net/sched/sch_generic.c:256
> dev_watchdog+0x213/0x220()
>
> The kernel is 2.6.38.2 plus Ubuntu cruft.
>
> A spot check of the 200+ hits on this string indicates they are
> primarily due to these drivers:
>
> ipheth
> atl1c
> sis900
> r8169
>
> As far as I can tell the warning happens when link is down on the media
> (and has never been link UP) and are sent a transmit packet which never
> completes. Is there a net/core or net/sched requirement to which these
> drivers do not conform ? Are they not correctly indicating link status?
The watchdog fires when the software queue has been stopped *and* the
link has been reported as up for over dev->watchdog_timeo ticks.
The software queue should be stopped iff the hardware queue is full or
nearly full. If the software queue remains stopped and the link is
still reported up, then one of these things is happening:
1. The link went down but the driver didn't notice
2. TX completions are not being indicated or handled correctly
3. The hardware TX path has locked up
4. The link is stalled by excessive pause frames or collisions
5. Timeout is too low and/or low watermark is too high
(there may be other explanations)
I think the watchdog is primarily meant to deal with case 3, though all
of cases 1-3 may be worked around by resetting the hardware.
Ben.
--
Ben Hutchings, Senior Software Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply
* Re: [PATCH 0/3] netfilter: netfilter update
From: David Miller @ 2011-04-19 18:28 UTC (permalink / raw)
To: kaber; +Cc: netfilter-devel, netdev
In-Reply-To: <1303223492-13549-1-git-send-email-kaber@trash.net>
From: kaber@trash.net
Date: Tue, 19 Apr 2011 16:31:29 +0200
> following is a small netfilter update for net-next, containing:
>
> - a patch from Eric to remove some atomic ops in the *tables fastpath
>
> - removal of an unnecessary ifdef from IPVS by Simon
>
> - SCTP and UDPLITE support for ipset, from Jozsef
>
> Please pull from:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/kaber/nf-next-2.6.git master
Pulled, thanks!
^ permalink raw reply
* Re: 2.6.38 dev_watchdog WARNING
From: Ben Greear @ 2011-04-19 18:20 UTC (permalink / raw)
To: tim.gardner; +Cc: netdev
In-Reply-To: <4DADC8F2.9050700@canonical.com>
On 04/19/2011 10:40 AM, Tim Gardner wrote:
> I'm seeing a lot of these kinds of bugs: WARNING: at
> /build/buildd/linux-2.6.38/net/sched/sch_generic.c:256
> dev_watchdog+0x213/0x220()
>
> The kernel is 2.6.38.2 plus Ubuntu cruft.
>
> A spot check of the 200+ hits on this string indicates they are
> primarily due to these drivers:
>
> ipheth
> atl1c
> sis900
> r8169
>
> As far as I can tell the warning happens when link is down on the media
> (and has never been link UP) and are sent a transmit packet which never
> completes. Is there a net/core or net/sched requirement to which these
> drivers do not conform ? Are they not correctly indicating link status?
r8169 doesn't report link right for me...
No idea about the rest.
Ben
>
> rtg
--
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc http://www.candelatech.com
^ permalink raw reply
* Re: ip_rt_bug questions.
From: Dave Jones @ 2011-04-19 18:13 UTC (permalink / raw)
To: David Miller; +Cc: netdev
In-Reply-To: <20110418.220419.226797064.davem@davemloft.net>
On Mon, Apr 18, 2011 at 10:04:19PM -0700, David Miller wrote:
> From: Dave Jones <davej@redhat.com>
> Date: Mon, 18 Apr 2011 17:59:23 -0400
>
> > On Mon, Apr 18, 2011 at 02:50:23PM -0700, David Miller wrote:
> >
> > > BTW, if you could modify this thing to spit out a stack
> > > trace (probably by using WARN_ON() or similar) that will
> > > probably show us where the bug is coming from.
> >
> > Ok, I'll add that, and see if I can reproduce it again.
>
> Thanks a lot Dave.
So I hit that twice in the same hour yesterday, but haven't been
able to reproduce it since. My fuzzer must have been dependant
upon something I can't influence (memory contents perhaps, I pass
around random pointers).
I'll keep the WARN_ON in there in case I do stumble across it again.
Dave
^ permalink raw reply
* 2.6.38 dev_watchdog WARNING
From: Tim Gardner @ 2011-04-19 17:40 UTC (permalink / raw)
To: netdev
I'm seeing a lot of these kinds of bugs: WARNING: at
/build/buildd/linux-2.6.38/net/sched/sch_generic.c:256
dev_watchdog+0x213/0x220()
The kernel is 2.6.38.2 plus Ubuntu cruft.
A spot check of the 200+ hits on this string indicates they are
primarily due to these drivers:
ipheth
atl1c
sis900
r8169
As far as I can tell the warning happens when link is down on the media
(and has never been link UP) and are sent a transmit packet which never
completes. Is there a net/core or net/sched requirement to which these
drivers do not conform ? Are they not correctly indicating link status?
rtg
--
Tim Gardner tim.gardner@canonical.com
^ permalink raw reply
* Re: [PATCH] usbnet: Resubmit interrupt URB more often
From: Paul Stewart @ 2011-04-19 17:51 UTC (permalink / raw)
To: Ben Hutchings; +Cc: netdev, davem
In-Reply-To: <1303233066.2988.14.camel@bwh-desktop>
Thanks, Ben. I've made some changes. usbnet.c is a little bit a of
mess style-wise, and I let myself get sucked in. :-)
--
Paul
On Tue, Apr 19, 2011 at 10:11 AM, Ben Hutchings
<bhutchings@solarflare.com> wrote:
>
> On Tue, 2011-04-19 at 09:35 -0700, Paul Stewart wrote:
> [...]
> > index 4342bd9..e4dbb29 100644
> > --- a/drivers/usb/core/urb.c
> > +++ b/drivers/usb/core/urb.c
> > @@ -295,7 +295,9 @@ int usb_submit_urb(struct urb *urb, gfp_t mem_flags)
> > struct usb_host_endpoint *ep;
> > int is_out;
> >
> > - if (!urb || urb->hcpriv || !urb->complete)
> > + if (urb->hcpriv)
> > + return -EALREADY;
> > + if (!urb || !urb->complete)
> > return -EINVAL;
> [...]
>
> The test for !urb must come before the test on urb->hcpriv.
>
> Also, the kernel coding style does not allow '//' comments. Please use
> scripts/checkpatch.pl to check for style and other common errors.
>
> Ben.
>
> --
> Ben Hutchings, Senior Software Engineer, Solarflare
> Not speaking for my employer; that's the marketing department's job.
> They asked us to note that Solarflare product names are trademarked.
>
^ permalink raw reply
* [PATCHv2] usbnet: Resubmit interrupt URB more often
From: Paul Stewart @ 2011-04-19 17:44 UTC (permalink / raw)
To: netdev; +Cc: davem, bhutchings
In-Reply-To: <1303233066.2988.14.camel@bwh-desktop>
I previously sent a patch to resubmit the interrupt URB when
coming out of suspend. I haven't seen much activity on the
list about it, and thought I'd send a slight variant of this
change. This one unconditionally resubmits the interrupt urb
in usbnet_bh. The consequences for resubmitting the URB often
are not large. In most HCI cases this just means usb_submit_urb
returns immediately and leaves the previous request outstanding.
Doing things this way allows us to avoid keeping track of the
URB transmit status, which may change silently over suspend-
resume transitions and is not tracked in any way currently by
usbnet.
I've designed this change on two types of systems: the first
class of system leaves USB devices powered during suspend.
This might silently cause the interrupt URB to disappear (or at
least not be resubmitted in intr_complete). Resubmission after
system resume will prevent this from causing problems.
The second class of device are those which shut down the device
during suspend. During a suspend-resume cycle, the device is
re-enumerated at system resume, and for whatever reason
usbnet_resume may be called on the device during the call-tree
from usbnet_open-> usb_autopm_get_interface, which may cause a
race where the first change above may cause the bh to submit the
interrupt urb before usbnet_open() does. As a result, I've added
an EALREADY check and a fix to urb.c to send one.
Signed-off-by: Paul Stewart <pstew@chromium.org>
Cc: David S. Miller <davem@davemloft.net>
Cc: Ben Hutchings <bhutchings@solarflare.com>
---
drivers/net/usb/usbnet.c | 14 +++++++++++++-
drivers/usb/core/urb.c | 5 +++--
2 files changed, 16 insertions(+), 3 deletions(-)
diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c
index 02d25c7..1718898 100644
--- a/drivers/net/usb/usbnet.c
+++ b/drivers/net/usb/usbnet.c
@@ -636,7 +636,12 @@ static int usbnet_open (struct net_device *net)
/* start any status interrupt transfer */
if (dev->interrupt) {
retval = usb_submit_urb (dev->interrupt, GFP_KERNEL);
- if (retval < 0) {
+ if (retval == -EALREADY) {
+ /*
+ * It is not an error if interrupt urb is alredy active
+ */
+ retval = 0;
+ } else if (retval < 0) {
if (netif_msg_ifup (dev))
deverr (dev, "intr submit %d", retval);
goto done;
@@ -1065,6 +1070,10 @@ static void usbnet_bh (unsigned long param)
if (dev->txq.qlen < TX_QLEN (dev))
netif_wake_queue (dev->net);
}
+
+ /* Re-submit interrupt urb (doesn't hurt to retry) */
+ if (netif_running(dev->net))
+ usb_submit_urb(dev->interrupt, GFP_KERNEL);
}
@@ -1285,6 +1294,9 @@ int usbnet_suspend (struct usb_interface *intf, pm_message_t message)
* wake the device
*/
netif_device_attach (dev->net);
+ /* Stop interrupt urbs while in suspend */
+ if (dev->interrupt)
+ usb_kill_urb(dev->interrupt);
}
return 0;
}
diff --git a/drivers/usb/core/urb.c b/drivers/usb/core/urb.c
index 4342bd9..ff85f50 100644
--- a/drivers/usb/core/urb.c
+++ b/drivers/usb/core/urb.c
@@ -295,8 +295,10 @@ int usb_submit_urb(struct urb *urb, gfp_t mem_flags)
struct usb_host_endpoint *ep;
int is_out;
- if (!urb || urb->hcpriv || !urb->complete)
+ if (!urb)
return -EINVAL;
+ if (!urb->complete || urb->hcpriv)
+ return -EALREADY;
dev = urb->dev;
if ((!dev) || (dev->state < USB_STATE_DEFAULT))
return -ENODEV;
@@ -807,4 +809,3 @@ int usb_anchor_empty(struct usb_anchor *anchor)
}
EXPORT_SYMBOL_GPL(usb_anchor_empty);
-
--
1.7.3.1
^ permalink raw reply related
* [PATCHv6] usbnet: Resubmit interrupt URB if device is open
From: Paul Stewart @ 2011-04-19 17:44 UTC (permalink / raw)
To: netdev-u79uwXL29TY76Z2rM5mHXA
Cc: stern-nwvwT67g6+6dFdvTe/nMLpVzexx5G7lz,
linux-usb-u79uwXL29TY76Z2rM5mHXA, oliver-GvhC2dPhHPQdnm+yROfE0A,
davem-fT/PcQaiUtIeIZ0/mPfg9Q, bhutchings-s/n/eUQHGBpZroRs9YW3xA,
linux-usb-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <Pine.LNX.4.44L0.1104221129530.1877-100000-IYeN2dnnYyZXsRXLowluHWD2FQJk+8+b@public.gmane.org>
Resubmit interrupt URB if device is open. Use a flag set in
usbnet_open() to determine this state. Also kill and free
interrupt URB in usbnet_disconnect().
Signed-off-by: Paul Stewart <pstew-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
---
drivers/net/usb/usbnet.c | 11 +++++++++++
include/linux/usb/usbnet.h | 1 +
2 files changed, 12 insertions(+), 0 deletions(-)
diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c
index 02d25c7..b4572c3 100644
--- a/drivers/net/usb/usbnet.c
+++ b/drivers/net/usb/usbnet.c
@@ -643,6 +643,7 @@ static int usbnet_open (struct net_device *net)
}
}
+ set_bit(EVENT_DEV_OPEN, &dev->flags);
netif_start_queue (net);
if (netif_msg_ifup (dev)) {
char *framing;
@@ -1105,6 +1106,9 @@ void usbnet_disconnect (struct usb_interface *intf)
if (dev->driver_info->unbind)
dev->driver_info->unbind (dev, intf);
+ usb_kill_urb(dev->interrupt);
+ usb_free_urb(dev->interrupt);
+
free_netdev(net);
usb_put_dev (xdev);
}
@@ -1285,6 +1289,9 @@ int usbnet_suspend (struct usb_interface *intf, pm_message_t message)
* wake the device
*/
netif_device_attach (dev->net);
+
+ /* Stop interrupt URBs */
+ usb_kill_urb(dev->interrupt);
}
return 0;
}
@@ -1297,6 +1304,10 @@ int usbnet_resume (struct usb_interface *intf)
if (!--dev->suspend_count)
tasklet_schedule (&dev->bh);
+ /* resume interrupt URBs */
+ if (test_bit(EVENT_DEV_OPEN, &dev->flags))
+ usb_submit_urb(dev->interrupt, GFP_NOIO);
+
return 0;
}
EXPORT_SYMBOL_GPL(usbnet_resume);
diff --git a/include/linux/usb/usbnet.h b/include/linux/usb/usbnet.h
index ba09fe8..d148cca 100644
--- a/include/linux/usb/usbnet.h
+++ b/include/linux/usb/usbnet.h
@@ -64,6 +64,7 @@ struct usbnet {
# define EVENT_RX_MEMORY 2
# define EVENT_STS_SPLIT 3
# define EVENT_LINK_RESET 4
+# define EVENT_DEV_OPEN 5
};
static inline struct usb_driver *driver_of(struct usb_interface *intf)
--
1.7.3.1
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [PATCHv5] usbnet: Resubmit interrupt URB once if halted
From: Paul Stewart @ 2011-04-19 17:44 UTC (permalink / raw)
To: netdev-u79uwXL29TY76Z2rM5mHXA
Cc: stern-nwvwT67g6+6dFdvTe/nMLpVzexx5G7lz,
linux-usb-u79uwXL29TY76Z2rM5mHXA, oliver-GvhC2dPhHPQdnm+yROfE0A,
davemloft.net-hpIqsD4AKlfQT0dZR+AlfA,
bhutchings-s/n/eUQHGBpZroRs9YW3xA,
linux-usb-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <Pine.LNX.4.44L0.1104211436540.1939-100000-IYeN2dnnYyZXsRXLowluHWD2FQJk+8+b@public.gmane.org>
Resubmit interrupt URB if device is open. Use a flag set in
usbnet_open() to determine this state. Also kill and free
interrupt URB in usbnet_disconnect().
Signed-off-by: Paul Stewart <pstew-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
---
drivers/net/usb/usbnet.c | 14 ++++++++++++++
include/linux/usb/usbnet.h | 1 +
2 files changed, 15 insertions(+), 0 deletions(-)
diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c
index 02d25c7..c7cf4af 100644
--- a/drivers/net/usb/usbnet.c
+++ b/drivers/net/usb/usbnet.c
@@ -643,6 +643,7 @@ static int usbnet_open (struct net_device *net)
}
}
+ set_bit(EVENT_DEV_OPEN, &dev->flags);
netif_start_queue (net);
if (netif_msg_ifup (dev)) {
char *framing;
@@ -1105,6 +1106,11 @@ void usbnet_disconnect (struct usb_interface *intf)
if (dev->driver_info->unbind)
dev->driver_info->unbind (dev, intf);
+ if (dev->interrupt) {
+ usb_kill_urb(dev->interrupt);
+ usb_free_urb(dev->interrupt);
+ }
+
free_netdev(net);
usb_put_dev (xdev);
}
@@ -1285,6 +1291,10 @@ int usbnet_suspend (struct usb_interface *intf, pm_message_t message)
* wake the device
*/
netif_device_attach (dev->net);
+
+ /* Stop interrupt URBs */
+ if (dev->interrupt)
+ usb_kill_urb(dev->interrupt);
}
return 0;
}
@@ -1297,6 +1307,10 @@ int usbnet_resume (struct usb_interface *intf)
if (!--dev->suspend_count)
tasklet_schedule (&dev->bh);
+ /* resume interrupt URBs */
+ if (dev->interrupt && test_bit(EVENT_DEV_OPEN, &dev->flags))
+ usb_submit_urb(dev->interrupt, GFP_NOIO);
+
return 0;
}
EXPORT_SYMBOL_GPL(usbnet_resume);
diff --git a/include/linux/usb/usbnet.h b/include/linux/usb/usbnet.h
index ba09fe8..d148cca 100644
--- a/include/linux/usb/usbnet.h
+++ b/include/linux/usb/usbnet.h
@@ -64,6 +64,7 @@ struct usbnet {
# define EVENT_RX_MEMORY 2
# define EVENT_STS_SPLIT 3
# define EVENT_LINK_RESET 4
+# define EVENT_DEV_OPEN 5
};
static inline struct usb_driver *driver_of(struct usb_interface *intf)
--
1.7.3.1
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [PATCHv4] usbnet: Resubmit interrupt URB once if halted
From: Paul Stewart @ 2011-04-19 17:44 UTC (permalink / raw)
To: netdev-u79uwXL29TY76Z2rM5mHXA
Cc: linux-usb-u79uwXL29TY76Z2rM5mHXA, davem-fT/PcQaiUtIeIZ0/mPfg9Q,
stern-nwvwT67g6+6dFdvTe/nMLpVzexx5G7lz,
greg-U8xfFu+wG4EAvxtiuMwx3w
In-Reply-To: <BANLkTi=N3T-V8VNOcbKu6COKvbEHqMoAog-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
Set a flag if the interrupt URB completes with ENOENT as this
occurs legitimately during system suspend. When the
usbnet_resume is called, test this flag and try once to resubmit
the interrupt URB.
This version of the patch moves the urb submit directly into
usbnet_resume. Is it okay to submit a GFP_KERNEL urb from
usbnet_resume()?
Signed-off-by: Paul Stewart <pstew-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
---
drivers/net/usb/usbnet.c | 13 ++++++++++++-
include/linux/usb/usbnet.h | 1 +
2 files changed, 13 insertions(+), 1 deletions(-)
diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c
index 02d25c7..3651a48 100644
--- a/drivers/net/usb/usbnet.c
+++ b/drivers/net/usb/usbnet.c
@@ -482,6 +482,7 @@ static void intr_complete (struct urb *urb)
case -ESHUTDOWN: /* hardware gone */
if (netif_msg_ifdown (dev))
devdbg (dev, "intr shutdown, code %d", status);
+ set_bit(EVENT_INTR_HALT, &dev->flags);
return;
/* NOTE: not throttling like RX/TX, since this endpoint
@@ -1294,9 +1295,19 @@ int usbnet_resume (struct usb_interface *intf)
{
struct usbnet *dev = usb_get_intfdata(intf);
- if (!--dev->suspend_count)
+ if (!--dev->suspend_count) {
tasklet_schedule (&dev->bh);
+ /* resubmit interrupt URB if it was halted by suspend */
+ if (dev->interrupt && netif_running(dev->net) &&
+ netif_device_present(dev->net) &&
+ test_bit(EVENT_INTR_HALT, &dev->flags)) {
+ clear_bit(EVENT_INTR_HALT, &dev->flags);
+ usb_submit_urb(dev->interrupt, GFP_KERNEL);
+ }
+ }
+}
+
return 0;
}
EXPORT_SYMBOL_GPL(usbnet_resume);
diff --git a/include/linux/usb/usbnet.h b/include/linux/usb/usbnet.h
index ba09fe8..6c4b5f8 100644
--- a/include/linux/usb/usbnet.h
+++ b/include/linux/usb/usbnet.h
@@ -64,6 +64,7 @@ struct usbnet {
# define EVENT_RX_MEMORY 2
# define EVENT_STS_SPLIT 3
# define EVENT_LINK_RESET 4
+# define EVENT_INTR_HALT 5
};
static inline struct usb_driver *driver_of(struct usb_interface *intf)
--
1.7.3.1
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [PATCHv3] usbnet: Resubmit interrupt URB once if halted
From: Paul Stewart @ 2011-04-19 17:44 UTC (permalink / raw)
To: netdev-u79uwXL29TY76Z2rM5mHXA
Cc: linux-usb-u79uwXL29TY76Z2rM5mHXA, davem-fT/PcQaiUtIeIZ0/mPfg9Q,
stern-nwvwT67g6+6dFdvTe/nMLpVzexx5G7lz,
greg-U8xfFu+wG4EAvxtiuMwx3w
In-Reply-To: <20110420.012431.104074243.davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>
Set a flag if the interrupt URB completes with ENOENT as this
occurs legitimately during system suspend. When the usbnet_bh
is called after resume, test this flag and try once to resubmit
the interrupt URB.
Signed-off-by: Paul Stewart <pstew-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
---
drivers/net/usb/usbnet.c | 8 ++++++++
include/linux/usb/usbnet.h | 1 +
2 files changed, 9 insertions(+), 0 deletions(-)
diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c
index 02d25c7..9ac4bae 100644
--- a/drivers/net/usb/usbnet.c
+++ b/drivers/net/usb/usbnet.c
@@ -482,6 +482,7 @@ static void intr_complete (struct urb *urb)
case -ESHUTDOWN: /* hardware gone */
if (netif_msg_ifdown (dev))
devdbg (dev, "intr shutdown, code %d", status);
+ set_bit(EVENT_INTR_HALT, &dev->flags);
return;
/* NOTE: not throttling like RX/TX, since this endpoint
@@ -1065,6 +1066,13 @@ static void usbnet_bh (unsigned long param)
if (dev->txq.qlen < TX_QLEN (dev))
netif_wake_queue (dev->net);
}
+
+ // try once to resume interrupt URBs if they were halted before
+ if (dev->interrupt && netif_running(dev->net) &&
+ test_bit(EVENT_INTR_HALT, &dev->flags)) {
+ clear_bit(EVENT_INTR_HALT, &dev->flags);
+ usb_submit_urb(dev->interrupt, GFP_KERNEL);
+ }
}
diff --git a/include/linux/usb/usbnet.h b/include/linux/usb/usbnet.h
index ba09fe8..6c4b5f8 100644
--- a/include/linux/usb/usbnet.h
+++ b/include/linux/usb/usbnet.h
@@ -64,6 +64,7 @@ struct usbnet {
# define EVENT_RX_MEMORY 2
# define EVENT_STS_SPLIT 3
# define EVENT_LINK_RESET 4
+# define EVENT_INTR_HALT 5
};
static inline struct usb_driver *driver_of(struct usb_interface *intf)
--
1.7.3.1
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* Re: DSCP values in TCP handshake
From: Mikael Abrahamsson @ 2011-04-19 17:38 UTC (permalink / raw)
To: Matt Mathis; +Cc: Stephen Hemminger, Joe Buehler, Eric Dumazet, netdev
In-Reply-To: <BANLkTi=fDdoF=kSfY=9ER4iR3M27L=CLXw@mail.gmail.com>
On Tue, 19 Apr 2011, Matt Mathis wrote:
> Please do. This missing spec is one of the things that makes Less than
> Best Effort (aka scavenger service) unusable. Only the client knows if
> they are fetching data in the background. The server doesn't care.
I emailed with Fred Baker and the below text seems to be where it ended
regarding reflexive marking of packets. If someone wants this changed,
they need to bring it up with the IETF.
<http://www.ietf.org/proceedings/54/220.htm>
"Fred Baker presented draft-ietf-ieprep-packet-marking-policy-00.txt
Van Jacobsen presented a concern for Kathy Nichols, Diffserv co-chair. The
concern was that the DSCP is intended to identify traffic supported by a
service, as opposed to traffic of a certain application type. He also
worried that a specific BCP indicating the facilities used to support such
services might be misused by regulators to mandate ISP services.
Specifically, he wanted a statement between the requirements document and
a document suggesting a specific configuration identifying the fact that
inter-service-provider signaling as specified in this draft is certainly
specified but is not commonly implemented.
Christian Huitema and another speaker also commented that while a cookbook
such as this was an interesting useful document, it didn't seem to
directly stem from emergency as a context.
Rei Atarashi presented draft-ietf-ieprep-reflexive-dscp-00.txt
Van basically felt that the notion of a default response from the host was
inappropriate; this is something every network should provide a policy
for. "
--
Mikael Abrahamsson email: swmike@swm.pp.se
^ permalink raw reply
* Re: [PATCH] usbnet: Resubmit interrupt URB more often
From: Ben Hutchings @ 2011-04-19 17:11 UTC (permalink / raw)
To: Paul Stewart; +Cc: netdev, davem
In-Reply-To: <20110419164703.5A6A82052B@glenhelen.mtv.corp.google.com>
On Tue, 2011-04-19 at 09:35 -0700, Paul Stewart wrote:
[...]
> index 4342bd9..e4dbb29 100644
> --- a/drivers/usb/core/urb.c
> +++ b/drivers/usb/core/urb.c
> @@ -295,7 +295,9 @@ int usb_submit_urb(struct urb *urb, gfp_t mem_flags)
> struct usb_host_endpoint *ep;
> int is_out;
>
> - if (!urb || urb->hcpriv || !urb->complete)
> + if (urb->hcpriv)
> + return -EALREADY;
> + if (!urb || !urb->complete)
> return -EINVAL;
[...]
The test for !urb must come before the test on urb->hcpriv.
Also, the kernel coding style does not allow '//' comments. Please use
scripts/checkpatch.pl to check for style and other common errors.
Ben.
--
Ben Hutchings, Senior Software Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply
* Re: [Bugme-new] [Bug 33502] New: Caught 64-bit read from uninitialized memory in __alloc_skb
From: Christoph Lameter @ 2011-04-19 17:10 UTC (permalink / raw)
To: Eric Dumazet
Cc: Andrew Morton, netdev, bugzilla-daemon, bugme-daemon,
casteyde.christian, Vegard Nossum, Pekka Enberg
In-Reply-To: <1303183217.4152.49.camel@edumazet-laptop>
On Tue, 19 Apr 2011, Eric Dumazet wrote:
> }
> }
>
> -#ifdef CONFIG_CMPXCHG_LOCAL
> +#if defined(CONFIG_CMPXCHG_LOCAL) && \
> + !defined(CONFIG_KMEMCHECK) && !defined(DEBUG_PAGEALLOC)
> +#define SLUB_USE_CMPXCHG_DOUBLE
> +#endif
> +
> +#ifdef SLUB_USE_CMPXCHG_DOUBLE
> #ifdef CONFIG_PREEMPT
> /*
Ugg.. Isnt there some way to indicate to kmemcheck that a speculative
access is occurring?
^ permalink raw reply
* Re: [Bugme-new] [Bug 33502] New: Caught 64-bit read from uninitialized memory in __alloc_skb
From: Christoph Lameter @ 2011-04-19 17:09 UTC (permalink / raw)
To: Eric Dumazet
Cc: Andrew Morton, netdev, bugzilla-daemon, bugme-daemon,
casteyde.christian, Vegard Nossum, Pekka Enberg
In-Reply-To: <1303182557.4152.48.camel@edumazet-laptop>
On Tue, 19 Apr 2011, Eric Dumazet wrote:
> get_freepointer(s, object) can access to freed memory and kmemcheck
> triggers the fault, while this_cpu_cmpxchg_double() would presumably
> detect a change of tid and would not perform the freelist/tid change.
Sounds right. The new lockless patchset for slub that uses a locked
cmpxchg16b will make this behavior even more common since it will do more
speculative accesses.
^ permalink raw reply
* Re: [PATCH] bonding: 802.3ad - fix agg_device_up
From: Jay Vosburgh @ 2011-04-19 16:55 UTC (permalink / raw)
To: Jiri Bohac; +Cc: netdev, Andy Gospodarek, Stephen Hemminger
In-Reply-To: <20110419120955.GA14302@midget.suse.cz>
Jiri Bohac <jbohac@suse.cz> wrote:
>The slave member of struct aggregator does not necessarily point
>to a slave which is part of the aggregator. It points to the
>slave structure containing the aggregator structure, while
>completely different slaves (or no slaves at all) may be part of
>the aggregator.
>
>The agg_device_up() function wrongly uses agg->slave to find the state of the
>aggregator. Use agg->lag_ports->slave instead. The bug has been
>introduced by commit 4cd6fe1c6483cde93e2ec91f58b7af9c9eea51ad.
>
>Signed-off-by: Jiri Bohac <jbohac@suse.cz>
One additional note: port->slave can be NULL when the slave is
transitioning in or out of the bond, but not when the port is part of an
aggregator, so this usage should be safe.
-J
Signed-off-by: Jay Vosburgh <fubar@us.ibm.com>
>diff --git a/drivers/net/bonding/bond_3ad.c b/drivers/net/bonding/bond_3ad.c
>index 494bf96..31912f1 100644
>--- a/drivers/net/bonding/bond_3ad.c
>+++ b/drivers/net/bonding/bond_3ad.c
>@@ -1482,8 +1482,11 @@ static struct aggregator *ad_agg_selection_test(struct aggregator *best,
>
> static int agg_device_up(const struct aggregator *agg)
> {
>- return (netif_running(agg->slave->dev) &&
>- netif_carrier_ok(agg->slave->dev));
>+ struct port *port = agg->lag_ports;
>+ if (!port)
>+ return 0;
>+ return (netif_running(port->slave->dev) &&
>+ netif_carrier_ok(port->slave->dev));
> }
>
> /**
>--
>Jiri Bohac <jbohac@suse.cz>
>SUSE Labs, SUSE CZ
>
^ permalink raw reply
* Re: A patch you wrote some time ago (aka: "[patch 41/54] ICMP: Fix icmp_errors_use_inbound_ifaddr sysctl")
From: Chris Wright @ 2011-04-19 16:54 UTC (permalink / raw)
To: Alexander Hoogerhuis; +Cc: Chris Wright, linux-kernel, netdev, kaber
In-Reply-To: <4DADBBBC.6020803@boxed.no>
* Alexander Hoogerhuis (alexh@boxed.no) wrote:
> I hope you (or anyone else) can spare half a minute to have a quick
> look at a patch you wrote a few years ago:
>
> >http://lkml.org/lkml/2007/6/8/124
I actually did not write that patch, rather added it to the -stable tree.
Patrick (CCd) wrote it.
> I've been tracking down a case of ICMP Redirects originating from
> the wrong IPs, and as far I can tell, you patch is the last to touch
> this code (net/ipv4/icmp.c:507):
>
> > if (rt->fl.iif && net->ipv4.sysctl_icmp_errors_use_inbound_ifaddr)
> > dev = dev_get_by_index_rcu(net, rt->fl.iif);
> >
> >if (dev)
> > saddr = inet_select_addr(dev, 0, RT_SCOPE_LINK);
> >else
> > saddr = 0;
>
> In a plain world this would work, but I have come across a case that
> seems to be not handled by this.
>
> I have two machines set up with VRRP to act as routers out of a
> subnet, and they have IPs x.x.x.13/28 and x.x.x.14/28, with VRRP
> holding on to x.x.x.1/28.
>
> If a node in x.x.x.0/28 needs to get a ICMP redirect from x.x.x.1/28
> (to reach another subnet behind a different gateway in x.x.x.0/28),
> then the source IP on the ICMP redirect is chosen as the primary IP
> on the interface that the packet arrived at.
>
> This is as far as I can tell from RFCs and colleagues fine for most
> things after you're routed one hop or more, but in the case of ICMP
> redirect it means that the redirect is not adhered to by the client,
> as it will get the reidrect from x.x.x.13/28, not x.x.x.1/28.
>
> inet_select_addr seems to be explicitly looking for the primary IP
> in all cases (./net/ipv4/devinet.c:875), and in the case of sending
> ICMP recdirect when in an VRRP setup, that would not work well. It
> should try to match the actual inbound IP.
>
> Judging by the comments from your patch I am not sure if the source
> IP that triggers the ICMP redirect is available at this point any
> more.
>
> The way I understand it should pick adress is this way:
>
> > if (rt->fl.iif && net->ipv4.sysctl_icmp_errors_use_inbound_ifaddr)
> > dev = dev_get_by_index_rcu(net, rt->fl.iif);
> >
> > if (dev == fl.iif)
> > saddr = iph->daddr;
> >
> > if (dev != fl.iif)
> > saddr = inet_select_addr(dev, 0, RT_SCOPE_LINK);
> > else
> > saddr = 0;
>
> I.e. if we are replying to something that is from a local network
> segment, then iph->daddr would be a more correct source. My C skill
> is prehistoric so what I've written likely is far from correct, but
> the general gist is that there is a special case for replying to
> something local.
>
> As it stands today (I'm on 2.6.35.11), ICMP redirects when using
> VRRP are broken, and I'm hoping I may have found out why. :)
>
> mvh,
> A
> --
> Alexander Hoogerhuis | http://no.linkedin.com/in/alexh
> Boxed Solutions AS | +47 908 21 485 - alexh@boxed.no
> "Given enough eyeballs, all bugs are shallow." -Eric S. Raymond
^ permalink raw reply
* [PATCH] usbnet: Resubmit interrupt URB more often
From: Paul Stewart @ 2011-04-19 16:35 UTC (permalink / raw)
To: netdev; +Cc: davem
I previously sent a patch to resubmit the interrupt URB when
coming out of suspend. I haven't seen much activity on the
list about it, and thought I'd send a slight variant of this
change. This one unconditionally resubmits the interrupt urb
in usbnet_bh. The consequences for resubmitting the URB often
are not large. In most HCI cases this just means usb_submit_urb
returns immediately and leaves the previous request outstanding.
Doing things this way allows us to avoid keeping track of the
URB transmit status, which may change silently over suspend-
resume transitions and is not tracked in any way currently by
usbnet.
I've designed this change on two types of systems: the first
class of system leaves USB devices powered during suspend.
This might silently cause the interrupt URB to disappear (or at
least not be resubmitted in intr_complete). Resubmission after
system resume will prevent this from causing problems.
The second class of device are those which shut down the device
during suspend. During a suspend-resume cycle, the device is
re-enumerated at system resume, and for whatever reason
usbnet_resume may be called on the device during the call-tree
from usbnet_open-> usb_autopm_get_interface, which may cause a
race where the first change above may cause the bh to submit the
interrupt urb before usbnet_open() does. As a result, I've added
an EALREADY check and a fix to urb.c to send one.
Signed-off-by: Paul Stewart <pstew@chromium.org>
Cc: David S. Miller <davem@davemloft.net>
---
drivers/net/usb/usbnet.c | 12 +++++++++++-
drivers/usb/core/urb.c | 5 +++--
2 files changed, 14 insertions(+), 3 deletions(-)
diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c
index 02d25c7..3b3c169 100644
--- a/drivers/net/usb/usbnet.c
+++ b/drivers/net/usb/usbnet.c
@@ -636,7 +636,10 @@ static int usbnet_open (struct net_device *net)
/* start any status interrupt transfer */
if (dev->interrupt) {
retval = usb_submit_urb (dev->interrupt, GFP_KERNEL);
- if (retval < 0) {
+ if (retval == -EALREADY) {
+ // It is not an error if interrupt urb is alredy active
+ retval = 0;
+ } else if (retval < 0) {
if (netif_msg_ifup (dev))
deverr (dev, "intr submit %d", retval);
goto done;
@@ -1065,6 +1068,10 @@ static void usbnet_bh (unsigned long param)
if (dev->txq.qlen < TX_QLEN (dev))
netif_wake_queue (dev->net);
}
+
+ // Re-submit interrupt urb (doesn't hurt to retry)
+ if (netif_running (dev->net))
+ usb_submit_urb (dev->interrupt, GFP_KERNEL);
}
@@ -1285,6 +1292,9 @@ int usbnet_suspend (struct usb_interface *intf, pm_message_t message)
* wake the device
*/
netif_device_attach (dev->net);
+ // Stop interrupt urbs while in suspend
+ if (dev->interrupt)
+ usb_kill_urb(dev->interrupt);
}
return 0;
}
diff --git a/drivers/usb/core/urb.c b/drivers/usb/core/urb.c
index 4342bd9..e4dbb29 100644
--- a/drivers/usb/core/urb.c
+++ b/drivers/usb/core/urb.c
@@ -295,7 +295,9 @@ int usb_submit_urb(struct urb *urb, gfp_t mem_flags)
struct usb_host_endpoint *ep;
int is_out;
- if (!urb || urb->hcpriv || !urb->complete)
+ if (urb->hcpriv)
+ return -EALREADY;
+ if (!urb || !urb->complete)
return -EINVAL;
dev = urb->dev;
if ((!dev) || (dev->state < USB_STATE_DEFAULT))
@@ -807,4 +809,3 @@ int usb_anchor_empty(struct usb_anchor *anchor)
}
EXPORT_SYMBOL_GPL(usb_anchor_empty);
-
--
1.7.3.1
^ permalink raw reply related
* [PATCH] net: tun: convert to hw_features
From: Michał Mirosław @ 2011-04-19 16:13 UTC (permalink / raw)
To: netdev; +Cc: Rusty Russell
This changes offload setting behaviour to what I think is correct:
- offloads set via ethtool mean what admin wants to use (by default
he wants 'em all)
- offloads set via ioctl() mean what userspace is expecting to get
(this limits which admin wishes are granted)
- TUN_NOCHECKSUM is ignored, as it might cause broken packets when
forwarded (ip_summed == CHECKSUM_UNNECESSARY means that checksum
was verified, not that it can be ignored)
If TUN_NOCHECKSUM is implemented, it should set skb->csum_* and
skb->ip_summed (= CHECKSUM_PARTIAL) for known protocols and let others
be verified by kernel when necessary.
TUN_NOCHECKSUM handling was introduced by commit
f43798c27684ab925adde7d8acc34c78c6e50df8:
tun: Allow GSO using virtio_net_hdr
Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
---
drivers/net/tun.c | 63 +++++++++++++++++++++--------------------------------
1 files changed, 25 insertions(+), 38 deletions(-)
diff --git a/drivers/net/tun.c b/drivers/net/tun.c
index f5e9ac0..ade3cf9 100644
--- a/drivers/net/tun.c
+++ b/drivers/net/tun.c
@@ -123,6 +123,9 @@ struct tun_struct {
gid_t group;
struct net_device *dev;
+ u32 set_features;
+#define TUN_USER_FEATURES (NETIF_F_HW_CSUM|NETIF_F_TSO_ECN|NETIF_F_TSO| \
+ NETIF_F_TSO6|NETIF_F_UFO)
struct fasync_struct *fasync;
struct tap_filter txflt;
@@ -451,12 +454,20 @@ tun_net_change_mtu(struct net_device *dev, int new_mtu)
return 0;
}
+static u32 tun_net_fix_features(struct net_device *dev, u32 features)
+{
+ struct tun_struct *tun = netdev_priv(dev);
+
+ return (features & tun->set_features) | (features & ~TUN_USER_FEATURES);
+}
+
static const struct net_device_ops tun_netdev_ops = {
.ndo_uninit = tun_net_uninit,
.ndo_open = tun_net_open,
.ndo_stop = tun_net_close,
.ndo_start_xmit = tun_net_xmit,
.ndo_change_mtu = tun_net_change_mtu,
+ .ndo_fix_features = tun_net_fix_features,
};
static const struct net_device_ops tap_netdev_ops = {
@@ -465,6 +476,7 @@ static const struct net_device_ops tap_netdev_ops = {
.ndo_stop = tun_net_close,
.ndo_start_xmit = tun_net_xmit,
.ndo_change_mtu = tun_net_change_mtu,
+ .ndo_fix_features = tun_net_fix_features,
.ndo_set_multicast_list = tun_net_mclist,
.ndo_set_mac_address = eth_mac_addr,
.ndo_validate_addr = eth_validate_addr,
@@ -628,8 +640,7 @@ static __inline__ ssize_t tun_get_user(struct tun_struct *tun,
kfree_skb(skb);
return -EINVAL;
}
- } else if (tun->flags & TUN_NOCHECKSUM)
- skb->ip_summed = CHECKSUM_UNNECESSARY;
+ }
switch (tun->flags & TUN_TYPE_MASK) {
case TUN_TUN_DEV:
@@ -1094,6 +1105,10 @@ static int tun_set_iff(struct net *net, struct file *file, struct ifreq *ifr)
goto err_free_sk;
}
+ dev->hw_features = NETIF_F_SG | NETIF_F_FRAGLIST |
+ TUN_USER_FEATURES;
+ dev->features = dev->hw_features;
+
err = register_netdevice(tun->dev);
if (err < 0)
goto err_free_sk;
@@ -1158,18 +1173,12 @@ static int tun_get_iff(struct net *net, struct tun_struct *tun,
/* This is like a cut-down ethtool ops, except done via tun fd so no
* privs required. */
-static int set_offload(struct net_device *dev, unsigned long arg)
+static int set_offload(struct tun_struct *tun, unsigned long arg)
{
- u32 old_features, features;
-
- old_features = dev->features;
- /* Unset features, set them as we chew on the arg. */
- features = (old_features & ~(NETIF_F_HW_CSUM|NETIF_F_SG|NETIF_F_FRAGLIST
- |NETIF_F_TSO_ECN|NETIF_F_TSO|NETIF_F_TSO6
- |NETIF_F_UFO));
+ u32 features = 0;
if (arg & TUN_F_CSUM) {
- features |= NETIF_F_HW_CSUM|NETIF_F_SG|NETIF_F_FRAGLIST;
+ features |= NETIF_F_HW_CSUM;
arg &= ~TUN_F_CSUM;
if (arg & (TUN_F_TSO4|TUN_F_TSO6)) {
@@ -1195,9 +1204,8 @@ static int set_offload(struct net_device *dev, unsigned long arg)
if (arg)
return -EINVAL;
- dev->features = features;
- if (old_features != dev->features)
- netdev_features_change(dev);
+ tun->set_features = features;
+ netdev_update_features(tun->dev);
return 0;
}
@@ -1262,12 +1270,9 @@ static long __tun_chr_ioctl(struct file *file, unsigned int cmd,
case TUNSETNOCSUM:
/* Disable/Enable checksum */
- if (arg)
- tun->flags |= TUN_NOCHECKSUM;
- else
- tun->flags &= ~TUN_NOCHECKSUM;
- tun_debug(KERN_INFO, tun, "checksum %s\n",
+ /* [unimplemented] */
+ tun_debug(KERN_INFO, tun, "ignored: set checksum %s\n",
arg ? "disabled" : "enabled");
break;
@@ -1316,7 +1321,7 @@ static long __tun_chr_ioctl(struct file *file, unsigned int cmd,
break;
#endif
case TUNSETOFFLOAD:
- ret = set_offload(tun->dev, arg);
+ ret = set_offload(tun, arg);
break;
case TUNSETTXFILTER:
@@ -1595,30 +1600,12 @@ static void tun_set_msglevel(struct net_device *dev, u32 value)
#endif
}
-static u32 tun_get_rx_csum(struct net_device *dev)
-{
- struct tun_struct *tun = netdev_priv(dev);
- return (tun->flags & TUN_NOCHECKSUM) == 0;
-}
-
-static int tun_set_rx_csum(struct net_device *dev, u32 data)
-{
- struct tun_struct *tun = netdev_priv(dev);
- if (data)
- tun->flags &= ~TUN_NOCHECKSUM;
- else
- tun->flags |= TUN_NOCHECKSUM;
- return 0;
-}
-
static const struct ethtool_ops tun_ethtool_ops = {
.get_settings = tun_get_settings,
.get_drvinfo = tun_get_drvinfo,
.get_msglevel = tun_get_msglevel,
.set_msglevel = tun_set_msglevel,
.get_link = ethtool_op_get_link,
- .get_rx_csum = tun_get_rx_csum,
- .set_rx_csum = tun_set_rx_csum
};
--
1.7.2.5
^ permalink raw reply related
* [BUG] bnx2x: bnx2x_set_pbd_csum() random accesses
From: Eric Dumazet @ 2011-04-19 16:11 UTC (permalink / raw)
To: Dmitry Kravkov, Eilon Greenstein; +Cc: netdev
Hi guys
bnx2x_set_pbd_csum() / bnx2x_set_pbd_csum_e2() seem to read
tcp_hdrlen(skb) even for non TCP frames ?
Also, (skb_network_header(skb) - skb->data) is signed, so
(skb_network_header(skb) - skb->data) / 2 is a bit expensive...
Thanks
^ permalink raw reply
* Re: Hight speed data sending from custom IP out of kernel
From: juice @ 2011-04-19 16:02 UTC (permalink / raw)
To: monstr, netdev
In-Reply-To: <4DAD76F1.40309@monstr.eu>
Hi!
I can see you are probably going to run into CPU performance problems, but
it depends a lot on the type of traffic you are going to send.
My system requires quite fast processor, but even more important is to
have a network interface card that really supports the full speed of
gigabit ethernet line. The reason for that is that my test traffic
includes streams of very small packets that cause a lot of overhead in
processing.
Most of my test traffic is UDP, but it does not really matter what the
higher layers of the traffic are, this scheme operates on the ethernet
layer and does not care about payload structure.
I tried several NIC:s before i settled using Intel 82576 cards with the
igb driver. If you have less capable interface card, your small packet
performance is going to be a lot poorer.
Using that card I can get to full speed GE line rate even with 64byte
packets, but if you want to send larger packets, say close to 1500byte
then almost any NIC will work OK for you.
You can download the module code and the userland seeding application from
my svn server at https://toosa.swagman.org/svn/streamgen
The streamseed userland application requires libpcap-dev to build
correctly but the streamgen module is self-sufficent.
There is not a lot of documentation, and the module is still "work in
progress" as I am going to fix it to work with more than one interface at
the same time when I get to do it. Currently it can only use one interface
on the sending host machine.
- Juice -
> Hi Juice,
>
> juice wrote:
>> Hi Michal.
>>
>> How fast do you need to send the data?
>
> It sounds weird but as fast as possible. There is no specific limit
> because I
> want to create demo and test it on various hw configuration which I can
> easily
> create on FPGA. For now the bottleneck is Microblaze cpu. It can run from
> 50MHz
> till 170-180MHz. We also support both endians and have two hw IP
> cores(10/100/1000) which I can use.
>
>> I have an application where I send test stream out to GE line and can
>> fill
>> the total capacity of the ethernet regardless of the packet size.
>
> What cpu do you use?
>
>>
>> The test stream I am sending is stored in kernel memory, and therefore
>> is
>> limited by the amount of free memory. 200M is no problem.
>
> Is it UDP or TCP?
>
>>
>> The solution I am using is loosely based on the pktgen module, except
>> that
>> my module can load a wireshark capture from userland program and then
>> send
>> it from ethernet interface in wire speed.
>
> Sound good. Would it be possible to see it and test it?
>
> Thanks,
> Michal
>
>
>>
>> - Juice -
>>
>>
>>> Hi,
>>> I would like to create demo for high speed data sending from custom IP
>> through
>>> the ethernet. I think the best description is that there are dmaable
>>> memory
>>> mapped registers or just memory which store data I want to send (for
>> example 200MB).
>>> Linux should handle all communication between target(probably server)
>> and
>>> host
>>> (client) but data in the packets should go from that custom IP and
>>> can't go
>>> through the kernel because of performance issue.
>>> Ethernet core have own DMA which I could use but the question is if
>> there
>>> is any
>>> option how to convince the kernel that data will go directly from
>>> memory
>> mapped
>>> registers and the kernel/driver/... just setup dma BD for headers and
>> second for
>>> data.
>>> Do you have any experience with any solution with passing data
>> completely
>>> out of
>>> kernel?
>>> Thanks,
>>> Michal
>>> --
>>> Michal Simek, Ing. (M.Eng)
>>> w: www.monstr.eu p: +42-0-721842854
>>> Maintainer of Linux kernel 2.6 Microblaze Linux -
>>> http://www.monstr.eu/fdt/
>>> Microblaze U-BOOT custodian
>>> --
>>> 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
>>
>>
>>
>>
>
>
> --
> Michal Simek, Ing. (M.Eng)
> w: www.monstr.eu p: +42-0-721842854
> Maintainer of Linux kernel 2.6 Microblaze Linux -
> http://www.monstr.eu/fdt/
> Microblaze U-BOOT custodian
>
^ permalink raw reply
* Re: [PATCH] net: xen-netback: convert to hw_features
From: Michał Mirosław @ 2011-04-19 15:25 UTC (permalink / raw)
To: Ian Campbell; +Cc: netdev@vger.kernel.org, xen-devel@lists.xensource.com
In-Reply-To: <1303226148.5997.233.camel@zakaz.uk.xensource.com>
On Tue, Apr 19, 2011 at 04:15:48PM +0100, Ian Campbell wrote:
> On Tue, 2011-04-19 at 14:43 +0100, Michał Mirosław wrote:
> > On Tue, Apr 19, 2011 at 02:39:00PM +0100, Ian Campbell wrote:
> > > On Tue, 2011-04-19 at 14:30 +0100, Michał Mirosław wrote:
> > > > On Tue, Apr 19, 2011 at 02:17:53PM +0100, Ian Campbell wrote:
> > > > > I fixed it with the following, I also moved the !can_sg MTU clamping
> > > > > into a set_features hook (like we do with netfront). Am I right that
> > > > > this pattern copes with changes to SG via ethtool etc better? I think
> > > > > it's more future proof in any case.
> > > > This looks wrong. Even if SG is turned on, you might get big skbs which
> > > > are linearized. There is a difference in SG capability and SG offload
> > > > status and as I see it the capability is what you need to test for MTU.
> > > So the existing stuff in drivers/net/xen-netfront.c is wrong too?
> > Looks like it. But I don't really know what are the real constraints for MTU.
> > What I know is that SG even if turned on needs not be used (and currently
> > it's not e.g. if checksum offload is disabled).
> The interesting case is the opposite one, isn't it? IOW if NETIF_F_SG is
> disabled but the frontend/backend agree that they have the capability to
> handle >PAGE_SIZE skbs
Then the driver might get bigger skbs but they won't ever be fragmented.
> In my experience, the normal reason for disabling the NETIF_F_SG offload
> status is that the underlying capability is somehow buggy, otherwise is
> there any reason to turn it off?
Some features depend on others to function or on some hardware/software state.
Though in most cases the reason is the one you wrote (capability also includes
what driver has implemented).
Best Regards,
Michał Mirosław
^ permalink raw reply
* [PATCH 3/3] IPVS: init and cleanup restructuring.
From: Hans Schillstrom @ 2011-04-19 15:25 UTC (permalink / raw)
To: horms, ja, ebiederm, lvs-devel, netdev, netfilter-devel
Cc: hans.schillstrom, Hans Schillstrom
In-Reply-To: <1303226705-29178-1-git-send-email-hans@schillstrom.com>
This patch tries to restore the initial init and cleanup
sequences that was before name space patch.
The number of calls to register_pernet_device have been
reduced to one for the ip_vs.ko
Schedulers still have their own calls.
This patch adds a function __ip_vs_service_cleanup()
and a throttle or actually on/off switch for
the netfilter hooks.
The nf hooks will be enabled when the first service is loaded
and disabled when the last service is removed or when a
name space exit starts.
Signed-off-by: Hans Schillstrom <hans@schillstrom.com>
---
include/net/ip_vs.h | 17 +++++++
net/netfilter/ipvs/ip_vs_app.c | 15 +-----
net/netfilter/ipvs/ip_vs_conn.c | 20 ++++----
net/netfilter/ipvs/ip_vs_core.c | 86 ++++++++++++++++++++++++++++++++------
net/netfilter/ipvs/ip_vs_ctl.c | 66 ++++++++++++++++++++++-------
net/netfilter/ipvs/ip_vs_est.c | 14 +-----
net/netfilter/ipvs/ip_vs_proto.c | 11 +----
net/netfilter/ipvs/ip_vs_sync.c | 13 +----
8 files changed, 161 insertions(+), 81 deletions(-)
diff --git a/include/net/ip_vs.h b/include/net/ip_vs.h
index d516f00..558e490 100644
--- a/include/net/ip_vs.h
+++ b/include/net/ip_vs.h
@@ -791,6 +791,7 @@ struct ip_vs_app {
/* IPVS in network namespace */
struct netns_ipvs {
int gen; /* Generation */
+ int throttle; /* Instead of nf unreg */
/*
* Hash table: for real service lookups
*/
@@ -1089,6 +1090,22 @@ ip_vs_control_add(struct ip_vs_conn *cp, struct ip_vs_conn *ctl_cp)
atomic_inc(&ctl_cp->n_control);
}
+/*
+ * IPVS netns init & cleanup functions
+ */
+extern int __ip_vs_estimator_init(struct net *net);
+extern int __ip_vs_control_init(struct net *net);
+extern int __ip_vs_protocol_init(struct net *net);
+extern int __ip_vs_app_init(struct net *net);
+extern int __ip_vs_conn_init(struct net *net);
+extern int __ip_vs_sync_init(struct net *net);
+extern void __ip_vs_conn_cleanup(struct net *net);
+extern void __ip_vs_app_cleanup(struct net *net);
+extern void __ip_vs_protocol_cleanup(struct net *net);
+extern void __ip_vs_control_cleanup(struct net *net);
+extern void __ip_vs_estimator_cleanup(struct net *net);
+extern void __ip_vs_sync_cleanup(struct net *net);
+extern void __ip_vs_service_cleanup(struct net *net);
/*
* IPVS application functions
diff --git a/net/netfilter/ipvs/ip_vs_app.c b/net/netfilter/ipvs/ip_vs_app.c
index 7e8e769..51f3af7 100644
--- a/net/netfilter/ipvs/ip_vs_app.c
+++ b/net/netfilter/ipvs/ip_vs_app.c
@@ -576,7 +576,7 @@ static const struct file_operations ip_vs_app_fops = {
};
#endif
-static int __net_init __ip_vs_app_init(struct net *net)
+int __net_init __ip_vs_app_init(struct net *net)
{
struct netns_ipvs *ipvs = net_ipvs(net);
@@ -585,26 +585,17 @@ static int __net_init __ip_vs_app_init(struct net *net)
return 0;
}
-static void __net_exit __ip_vs_app_cleanup(struct net *net)
+void __net_exit __ip_vs_app_cleanup(struct net *net)
{
proc_net_remove(net, "ip_vs_app");
}
-static struct pernet_operations ip_vs_app_ops = {
- .init = __ip_vs_app_init,
- .exit = __ip_vs_app_cleanup,
-};
-
int __init ip_vs_app_init(void)
{
- int rv;
-
- rv = register_pernet_device(&ip_vs_app_ops);
- return rv;
+ return 0;
}
void ip_vs_app_cleanup(void)
{
- unregister_pernet_device(&ip_vs_app_ops);
}
diff --git a/net/netfilter/ipvs/ip_vs_conn.c b/net/netfilter/ipvs/ip_vs_conn.c
index 36cd5ea..f8d6702 100644
--- a/net/netfilter/ipvs/ip_vs_conn.c
+++ b/net/netfilter/ipvs/ip_vs_conn.c
@@ -1251,30 +1251,30 @@ int __net_init __ip_vs_conn_init(struct net *net)
{
struct netns_ipvs *ipvs = net_ipvs(net);
+ EnterFunction(2);
atomic_set(&ipvs->conn_count, 0);
proc_net_fops_create(net, "ip_vs_conn", 0, &ip_vs_conn_fops);
proc_net_fops_create(net, "ip_vs_conn_sync", 0, &ip_vs_conn_sync_fops);
+ LeaveFunction(2);
return 0;
}
-static void __net_exit __ip_vs_conn_cleanup(struct net *net)
+void __net_exit __ip_vs_conn_cleanup(struct net *net)
{
+ EnterFunction(2);
/* flush all the connection entries first */
ip_vs_conn_flush(net);
proc_net_remove(net, "ip_vs_conn");
proc_net_remove(net, "ip_vs_conn_sync");
+ LeaveFunction(2);
}
-static struct pernet_operations ipvs_conn_ops = {
- .init = __ip_vs_conn_init,
- .exit = __ip_vs_conn_cleanup,
-};
int __init ip_vs_conn_init(void)
{
int idx;
- int retc;
+ EnterFunction(2);
/* Compute size and mask */
ip_vs_conn_tab_size = 1 << ip_vs_conn_tab_bits;
ip_vs_conn_tab_mask = ip_vs_conn_tab_size - 1;
@@ -1309,18 +1309,18 @@ int __init ip_vs_conn_init(void)
rwlock_init(&__ip_vs_conntbl_lock_array[idx].l);
}
- retc = register_pernet_device(&ipvs_conn_ops);
-
/* calculate the random value for connection hash */
get_random_bytes(&ip_vs_conn_rnd, sizeof(ip_vs_conn_rnd));
+ LeaveFunction(2);
- return retc;
+ return 0;
}
void ip_vs_conn_cleanup(void)
{
- unregister_pernet_device(&ipvs_conn_ops);
+ EnterFunction(2);
/* Release the empty cache */
kmem_cache_destroy(ip_vs_conn_cachep);
vfree(ip_vs_conn_tab);
+ LeaveFunction(2);
}
diff --git a/net/netfilter/ipvs/ip_vs_core.c b/net/netfilter/ipvs/ip_vs_core.c
index a7bb81d..dc27fdf 100644
--- a/net/netfilter/ipvs/ip_vs_core.c
+++ b/net/netfilter/ipvs/ip_vs_core.c
@@ -1343,6 +1343,10 @@ ip_vs_in_icmp(struct sk_buff *skb, int *related, unsigned int hooknum)
return NF_ACCEPT; /* The packet looks wrong, ignore */
net = skb_net(skb);
+ /* Name space in use ? */
+ if (net->ipvs->throttle)
+ return NF_ACCEPT;
+
pd = ip_vs_proto_data_get(net, cih->protocol);
if (!pd)
return NF_ACCEPT;
@@ -1563,6 +1567,8 @@ ip_vs_in(unsigned int hooknum, struct sk_buff *skb, int af)
}
net = skb_net(skb);
+ if (net->ipvs->throttle)
+ return NF_ACCEPT;
/* Protocol supported? */
pd = ip_vs_proto_data_get(net, iph.protocol);
if (unlikely(!pd))
@@ -1588,7 +1594,6 @@ ip_vs_in(unsigned int hooknum, struct sk_buff *skb, int af)
}
IP_VS_DBG_PKT(11, af, pp, skb, 0, "Incoming packet");
- net = skb_net(skb);
ipvs = net_ipvs(net);
/* Check the server status */
if (cp->dest && !(cp->dest->flags & IP_VS_DEST_F_AVAILABLE)) {
@@ -1879,24 +1884,73 @@ static int __net_init __ip_vs_init(struct net *net)
{
struct netns_ipvs *ipvs;
+ EnterFunction(2);
ipvs = net_generic(net, ip_vs_net_id);
if (ipvs == NULL) {
pr_err("%s(): no memory.\n", __func__);
return -ENOMEM;
}
+ /* Hold the beast until a service is registerd */
+ ipvs->throttle = -1;
ipvs->net = net;
/* Counters used for creating unique names */
ipvs->gen = atomic_read(&ipvs_netns_cnt);
atomic_inc(&ipvs_netns_cnt);
net->ipvs = ipvs;
+
+ if ( __ip_vs_estimator_init(net) < 0)
+ goto estimator_fail;
+
+ if (__ip_vs_control_init(net) < 0)
+ goto control_fail;
+
+ if (__ip_vs_protocol_init(net) < 0)
+ goto protocol_fail;
+
+ if (__ip_vs_app_init(net) < 0)
+ goto app_fail;
+
+ if (__ip_vs_conn_init(net) < 0)
+ goto conn_fail;
+
+ if (__ip_vs_sync_init(net) < 0)
+ goto sync_fail;
+
+ LeaveFunction(2);
printk(KERN_INFO "IPVS: Creating netns size=%zu id=%d\n",
sizeof(struct netns_ipvs), ipvs->gen);
return 0;
+/*
+ * Error handling
+ */
+
+sync_fail:
+ __ip_vs_conn_cleanup(net);
+conn_fail:
+ __ip_vs_app_cleanup(net);
+app_fail:
+ __ip_vs_protocol_cleanup(net);
+protocol_fail:
+ __ip_vs_control_cleanup(net);
+control_fail:
+ __ip_vs_estimator_cleanup(net);
+estimator_fail:
+ return -ENOMEM;
}
static void __net_exit __ip_vs_cleanup(struct net *net)
{
- IP_VS_DBG(10, "ipvs netns %d released\n", net_ipvs(net)->gen);
+ net->ipvs->throttle = -1;
+ EnterFunction(2);
+ __ip_vs_sync_cleanup(net);
+ __ip_vs_service_cleanup(net); /* ip_vs_flush() with locks */
+ __ip_vs_conn_cleanup(net);
+ __ip_vs_app_cleanup(net);
+ __ip_vs_protocol_cleanup(net);
+ __ip_vs_control_cleanup(net);
+ __ip_vs_estimator_cleanup(net);
+ LeaveFunction(2);
+ IP_VS_DBG(2, "ipvs netns %d released\n", net_ipvs(net)->gen);
}
static struct pernet_operations ipvs_core_ops = {
@@ -1913,10 +1967,7 @@ static int __init ip_vs_init(void)
{
int ret;
- ret = register_pernet_device(&ipvs_core_ops); /* Alloc ip_vs struct */
- if (ret < 0)
- return ret;
-
+ EnterFunction(2);
ip_vs_estimator_init();
ret = ip_vs_control_init();
if (ret < 0) {
@@ -1944,41 +1995,50 @@ static int __init ip_vs_init(void)
goto cleanup_conn;
}
+ ret = register_pernet_device(&ipvs_core_ops); /* Alloc ip_vs struct */
+ if (ret < 0)
+ goto cleanup_sync;
+
ret = nf_register_hooks(ip_vs_ops, ARRAY_SIZE(ip_vs_ops));
if (ret < 0) {
pr_err("can't register hooks.\n");
- goto cleanup_sync;
+ goto cleanup_net;
}
pr_info("ipvs loaded.\n");
+ LeaveFunction(2);
+
return ret;
+cleanup_net:
+ unregister_pernet_device(&ipvs_core_ops); /* free ip_vs struct */
cleanup_sync:
ip_vs_sync_cleanup();
- cleanup_conn:
+cleanup_conn:
ip_vs_conn_cleanup();
- cleanup_app:
+cleanup_app:
ip_vs_app_cleanup();
- cleanup_protocol:
+cleanup_protocol:
ip_vs_protocol_cleanup();
ip_vs_control_cleanup();
- cleanup_estimator:
+cleanup_estimator:
ip_vs_estimator_cleanup();
- unregister_pernet_device(&ipvs_core_ops); /* free ip_vs struct */
return ret;
}
static void __exit ip_vs_cleanup(void)
{
+ EnterFunction(2);
nf_unregister_hooks(ip_vs_ops, ARRAY_SIZE(ip_vs_ops));
+ unregister_pernet_device(&ipvs_core_ops); /* free ip_vs struct */
ip_vs_sync_cleanup();
ip_vs_conn_cleanup();
ip_vs_app_cleanup();
ip_vs_protocol_cleanup();
ip_vs_control_cleanup();
ip_vs_estimator_cleanup();
- unregister_pernet_device(&ipvs_core_ops); /* free ip_vs struct */
pr_info("ipvs unloaded.\n");
+ LeaveFunction(2);
}
module_init(ip_vs_init);
diff --git a/net/netfilter/ipvs/ip_vs_ctl.c b/net/netfilter/ipvs/ip_vs_ctl.c
index 08715d8..6534ca3 100644
--- a/net/netfilter/ipvs/ip_vs_ctl.c
+++ b/net/netfilter/ipvs/ip_vs_ctl.c
@@ -69,6 +69,11 @@ int ip_vs_get_debug_level(void)
}
#endif
+
+/* Protos */
+static void __ip_vs_del_service(struct ip_vs_service *svc);
+
+
#ifdef CONFIG_IP_VS_IPV6
/* Taken from rt6_fill_node() in net/ipv6/route.c, is there a better way? */
static int __ip_vs_addr_is_local_v6(struct net *net,
@@ -345,6 +350,9 @@ static int ip_vs_svc_unhash(struct ip_vs_service *svc)
svc->flags &= ~IP_VS_SVC_F_HASHED;
atomic_dec(&svc->refcnt);
+ /* No more services then no need for input */
+ if (atomic_read(&svc->refcnt) == 0)
+ svc->net->ipvs->throttle = -1;
return 1;
}
@@ -480,7 +488,6 @@ __ip_vs_unbind_svc(struct ip_vs_dest *dest)
}
}
-
/*
* Returns hash value for real service
*/
@@ -1214,6 +1221,8 @@ ip_vs_add_service(struct net *net, struct ip_vs_service_user_kern *u,
write_unlock_bh(&__ip_vs_svc_lock);
*svc_p = svc;
+ /* Now whe have a service - full throttle */
+ ipvs->throttle = 0;
return 0;
@@ -1472,6 +1481,41 @@ static int ip_vs_flush(struct net *net)
return 0;
}
+/*
+ * Delete service by {netns} in the service table.
+ * Called by __ip_vs_cleanup()
+ */
+void __ip_vs_service_cleanup(struct net *net)
+{
+ unsigned hash;
+ struct ip_vs_service *svc, *tmp;
+
+ EnterFunction(2);
+ /* Check for "full" addressed entries */
+ for (hash = 0; hash<IP_VS_SVC_TAB_SIZE; hash++) {
+ write_lock_bh(&__ip_vs_svc_lock);
+ list_for_each_entry_safe(svc, tmp, &ip_vs_svc_table[hash],
+ s_list) {
+ if (net_eq(svc->net, net)) {
+ ip_vs_svc_unhash(svc);
+ /* Wait until all the svc users go away. */
+ IP_VS_WAIT_WHILE(atomic_read(&svc->usecnt) > 0);
+ __ip_vs_del_service(svc);
+ }
+ }
+ list_for_each_entry_safe(svc, tmp, &ip_vs_svc_fwm_table[hash],
+ f_list) {
+ if (net_eq(svc->net, net)) {
+ ip_vs_svc_unhash(svc);
+ /* Wait until all the svc users go away. */
+ IP_VS_WAIT_WHILE(atomic_read(&svc->usecnt) > 0);
+ __ip_vs_del_service(svc);
+ }
+ }
+ write_unlock_bh(&__ip_vs_svc_lock);
+ }
+ LeaveFunction(2);
+}
/*
* Zero counters in a service or all services
@@ -3593,6 +3637,7 @@ int __net_init __ip_vs_control_init(struct net *net)
int idx;
struct netns_ipvs *ipvs = net_ipvs(net);
+ EnterFunction(2);
ipvs->rs_lock = __RW_LOCK_UNLOCKED(ipvs->rs_lock);
/* Initialize rs_table */
@@ -3619,6 +3664,7 @@ int __net_init __ip_vs_control_init(struct net *net)
if (__ip_vs_control_init_sysctl(net))
goto err;
+ LeaveFunction(2);
return 0;
err:
@@ -3626,10 +3672,11 @@ err:
return -ENOMEM;
}
-static void __net_exit __ip_vs_control_cleanup(struct net *net)
+void __net_exit __ip_vs_control_cleanup(struct net *net)
{
struct netns_ipvs *ipvs = net_ipvs(net);
+ EnterFunction(2);
ip_vs_trash_cleanup(net);
ip_vs_stop_estimator(net, &ipvs->tot_stats);
__ip_vs_control_cleanup_sysctl(net);
@@ -3637,13 +3684,9 @@ static void __net_exit __ip_vs_control_cleanup(struct net *net)
proc_net_remove(net, "ip_vs_stats");
proc_net_remove(net, "ip_vs");
free_percpu(ipvs->tot_stats.cpustats);
+ LeaveFunction(2);
}
-static struct pernet_operations ipvs_control_ops = {
- .init = __ip_vs_control_init,
- .exit = __ip_vs_control_cleanup,
-};
-
int __init ip_vs_control_init(void)
{
int idx;
@@ -3657,12 +3700,6 @@ int __init ip_vs_control_init(void)
INIT_LIST_HEAD(&ip_vs_svc_fwm_table[idx]);
}
- ret = register_pernet_device(&ipvs_control_ops);
- if (ret) {
- pr_err("cannot register namespace.\n");
- goto err;
- }
-
smp_wmb(); /* Do we really need it now ? */
ret = nf_register_sockopt(&ip_vs_sockopts);
@@ -3682,8 +3719,6 @@ int __init ip_vs_control_init(void)
return 0;
err_net:
- unregister_pernet_device(&ipvs_control_ops);
-err:
return ret;
}
@@ -3691,7 +3726,6 @@ err:
void ip_vs_control_cleanup(void)
{
EnterFunction(2);
- unregister_pernet_device(&ipvs_control_ops);
ip_vs_genl_unregister();
nf_unregister_sockopt(&ip_vs_sockopts);
LeaveFunction(2);
diff --git a/net/netfilter/ipvs/ip_vs_est.c b/net/netfilter/ipvs/ip_vs_est.c
index 759163e..508cce9 100644
--- a/net/netfilter/ipvs/ip_vs_est.c
+++ b/net/netfilter/ipvs/ip_vs_est.c
@@ -192,7 +192,7 @@ void ip_vs_read_estimator(struct ip_vs_stats_user *dst,
dst->outbps = (e->outbps + 0xF) >> 5;
}
-static int __net_init __ip_vs_estimator_init(struct net *net)
+int __net_init __ip_vs_estimator_init(struct net *net)
{
struct netns_ipvs *ipvs = net_ipvs(net);
@@ -203,24 +203,16 @@ static int __net_init __ip_vs_estimator_init(struct net *net)
return 0;
}
-static void __net_exit __ip_vs_estimator_exit(struct net *net)
+void __net_exit __ip_vs_estimator_cleanup(struct net *net)
{
del_timer_sync(&net_ipvs(net)->est_timer);
}
-static struct pernet_operations ip_vs_app_ops = {
- .init = __ip_vs_estimator_init,
- .exit = __ip_vs_estimator_exit,
-};
int __init ip_vs_estimator_init(void)
{
- int rv;
-
- rv = register_pernet_device(&ip_vs_app_ops);
- return rv;
+ return 0;
}
void ip_vs_estimator_cleanup(void)
{
- unregister_pernet_device(&ip_vs_app_ops);
}
diff --git a/net/netfilter/ipvs/ip_vs_proto.c b/net/netfilter/ipvs/ip_vs_proto.c
index f7021fc..eb86028 100644
--- a/net/netfilter/ipvs/ip_vs_proto.c
+++ b/net/netfilter/ipvs/ip_vs_proto.c
@@ -316,7 +316,7 @@ ip_vs_tcpudp_debug_packet(int af, struct ip_vs_protocol *pp,
/*
* per network name-space init
*/
-static int __net_init __ip_vs_protocol_init(struct net *net)
+int __net_init __ip_vs_protocol_init(struct net *net)
{
#ifdef CONFIG_IP_VS_PROTO_TCP
register_ip_vs_proto_netns(net, &ip_vs_protocol_tcp);
@@ -336,7 +336,7 @@ static int __net_init __ip_vs_protocol_init(struct net *net)
return 0;
}
-static void __net_exit __ip_vs_protocol_cleanup(struct net *net)
+void __net_exit __ip_vs_protocol_cleanup(struct net *net)
{
struct netns_ipvs *ipvs = net_ipvs(net);
struct ip_vs_proto_data *pd;
@@ -349,11 +349,6 @@ static void __net_exit __ip_vs_protocol_cleanup(struct net *net)
}
}
-static struct pernet_operations ipvs_proto_ops = {
- .init = __ip_vs_protocol_init,
- .exit = __ip_vs_protocol_cleanup,
-};
-
int __init ip_vs_protocol_init(void)
{
char protocols[64];
@@ -382,7 +377,6 @@ int __init ip_vs_protocol_init(void)
REGISTER_PROTOCOL(&ip_vs_protocol_esp);
#endif
pr_info("Registered protocols (%s)\n", &protocols[2]);
- return register_pernet_device(&ipvs_proto_ops);
return 0;
}
@@ -393,7 +387,6 @@ void ip_vs_protocol_cleanup(void)
struct ip_vs_protocol *pp;
int i;
- unregister_pernet_device(&ipvs_proto_ops);
/* unregister all the ipvs protocols */
for (i = 0; i < IP_VS_PROTO_TAB_SIZE; i++) {
while ((pp = ip_vs_proto_table[i]) != NULL)
diff --git a/net/netfilter/ipvs/ip_vs_sync.c b/net/netfilter/ipvs/ip_vs_sync.c
index 1aeca1d..e911f03 100644
--- a/net/netfilter/ipvs/ip_vs_sync.c
+++ b/net/netfilter/ipvs/ip_vs_sync.c
@@ -1664,7 +1664,7 @@ int stop_sync_thread(struct net *net, int state)
/*
* Initialize data struct for each netns
*/
-static int __net_init __ip_vs_sync_init(struct net *net)
+int __net_init __ip_vs_sync_init(struct net *net)
{
struct netns_ipvs *ipvs = net_ipvs(net);
@@ -1678,24 +1678,17 @@ static int __net_init __ip_vs_sync_init(struct net *net)
return 0;
}
-static void __ip_vs_sync_cleanup(struct net *net)
+void __ip_vs_sync_cleanup(struct net *net)
{
stop_sync_thread(net, IP_VS_STATE_MASTER);
stop_sync_thread(net, IP_VS_STATE_BACKUP);
}
-static struct pernet_operations ipvs_sync_ops = {
- .init = __ip_vs_sync_init,
- .exit = __ip_vs_sync_cleanup,
-};
-
-
int __init ip_vs_sync_init(void)
{
- return register_pernet_device(&ipvs_sync_ops);
+ return 0;
}
void ip_vs_sync_cleanup(void)
{
- unregister_pernet_device(&ipvs_sync_ops);
}
--
1.7.2.3
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox